Repository navigation
Diagnose provider identity collisions and approve audited GitHub tag repairs - #167
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThis PR adds an offline GitHub/provider identity collision explorer plus a tightly constrained, SDK-audited GitHub tag-detach repair flow, exposed through both CLI and typed SDK APIs, with conservative ownership checks, approval digests, lock/revision validation, recovery/idempotency guarantees, and extensive synthetic and public-corpus validation. Sequence diagram for approved GitHub identity tag repairsequenceDiagram
participant Agent
participant CLI as pm_github_identity
participant SDK as IdentitySDK
participant Reader as readRefreshItems
participant Planner as planGithubIdentityRepair
participant Writer as commitItemMutations
participant History as NativeAuditHistory
Agent->>CLI: --identity --owner --detach
CLI->>Planner: planGithubIdentityRepair(pmRoot, identity, owner, detach)
Planner->>Reader: readRefreshItems(pmRoot)
Reader-->>Planner: complete item graph
Planner->>SDK: previewItemMutations(transaction)
SDK-->>Planner: validated plan and approval digest
Planner-->>CLI: IdentityRepairPlan
Agent->>CLI: --approve digest
CLI->>SDK: applyGithubIdentityRepair(request)
SDK->>Reader: readRefreshItems(pmRoot)
Reader-->>SDK: current graph and revisions
SDK->>Writer: commitItemMutations(updated_at match)
Writer->>History: append audited tag-detach event
History-->>Writer: durable transaction receipt
Writer-->>CLI: changed transaction
Flow diagram for offline identity diagnosis and scoped repairflowchart TD
A[pm github identity] --> B[readRefreshItems complete tracker copy]
B --> C[Build identity graph]
C --> D{Inspect exact identity evidence}
D -->|diagnosis only| E[Return nodes edges counts pagination]
D -->|owner and detach supplied| F[planGithubIdentityRepair]
F --> G{Ownership and repair checks pass}
G -->|no| H[Typed refusal with remedy]
G -->|yes| I[SDK semantic preview]
I --> J[Approval digest]
J --> K{Exact digest approved}
K -->|no| L[No mutation]
K -->|yes| M[Writer-lock recheck and updated_at match]
M --> N[Detach selected GitHub tag]
N --> O[Append native audit event]
O --> P[Idempotent replay or receipt]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
Orchestrator review (codex review agent, reproduced findings; handed to the feature agent):
VERDICT: BLOCK
FINDINGS:
- [blocking] identity.ts:151 — Recovery accepts fabricated audit plans without validating their approval binding — a forged history marker with a nonexistent owner and arbitrary digest replaces tags with unapproved values while returning
changed:false. - [blocking] identity.ts:122 — Comma serialization changes retained tags beyond the approved repair — a stored tag
keep,meremains intact in the preview but becomes separatekeepandmetags after apply. - [blocking] identity.ts:100 — Ownership validation ignores conflicting
github_numbermetadata on the selected owner — an owner declaring issue 1’s URL but issue number 2 still authorizes removing another item’s issue 1 tag.
SUMMARY: The PR adds offline identity diagnosis and audited, digest-approved tag repairs. All 15 supplied identity tests pass, but they miss these three reproduced approval, tag-preservation, and ownership defects.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
VERDICT: MERGE
FINDINGS:
- none
SUMMARY: Follow-up at 474dbd9 addresses all three blocking findings: recovery revalidates the graph digest, retained tags and the SDK journal so forged audit plans never authorize writes; tag detach uses the SDK removeTags so a stored comma tag survives byte-identical; an owner with a conflicting github_number is refused before preview and apply. Three named regression tests with revert proofs (20/20 identity tests), gates reported green, mergeable_state clean.
Result
Agents can diagnose conflicting provider identities offline with
pm github identityand the typedexploreGithubIdentities,planGithubIdentityRepairandapplyGithubIdentityRepairSDK exports. A copied prose example appears as a mention; a structured provider link retains provider ownership evidence. Exactexternal_ref, GitHub tags, native source ownership and contradictory identities on one item remain visible.Before, imports refused ambiguous ownership but offered no local exploration or reviewed repair surface. Now agents receive compact TOON/JSON graphs with counts and pagination, typed refusals with remedies, and an explicit approval digest for one narrowly scoped tag detach.
Design and scope
Reuses certified complete SDK reads, the existing isolated refresh reader, real SDK mutation previews, durable item transactions, optimistic
updated_atmatching and native append-only audit. Approval is rechecked under the writer lock. Recovery reconstructs the pre-repair graph to verify the approval digest and exact retained tags, and requires the original SDK journal. The native audit comment alone cannot authorize a repair. SDK transactions remain responsible for recovery, compensation and idempotent replay. Ordinary input files are copied before SDK cache writes; linked roots/entries refuse. No git executable or network is needed.Repairs remove only the selected conflicting GitHub tag. Ambiguous native/source metadata refuses apply. The pinned SDK cannot clear core
external_refthrough its item mutation surface, so those repairs returnunsupported_repairwith an owning-provider remedy. Jira/Linear semantics and core metadata validation remain authoritative. No dependency pins, coverage thresholds or release workflows changed.Acceptance evidence
step_appliedinterruption/recovery and byte-identical repeat apply.Blocking review fixes
stale_approvalbefore any writes. Tests cover nonexistent owners, altered plans/retained tags and missing journals.keep,metagremoveTagsfor the exact GitHub provenance tags. The regression verifies preview, stored literal tag and byte-identical replay.github_numberambiguous_identitybefore writes; consistent issue-number metadata is also exercised by the successful comma-tag repair.Each regression was run with only its corresponding function reverted to the pre-fix implementation; tests and the other fixes remained unchanged:
applyGithubIdentityRepairforged audit plans with nonexistent owners never authorize recovery writestransactionapproved tag detach preserves a stored comma tag byte-identicalkeepandmeinstead ofkeep,me.planan owner with a conflicting github_number cannot authorize detaching another itemAll functions were restored before validation. Proofs are recorded on the linked pm item.
Real public corpus
Read-only non-git archive of public
unbraind/pm-githubmain394129e3a8a607bfc13dd8acc8adfe4704165da2, with git absent from PATH: 296 files, 145 all-status items, 9 identities, 0 collisions, 3 prose mentions, 3 returned nodes/edges, no continuation. Compact JSON: 642 bytes; TOON: 551 bytes. All file bytes and inventory unchanged. Zero real collisions is zero-case evidence; synthetic trackers supply nonempty repair evidence. Fingerprint and method:docs/identity-collisions-validation.md.Validation
Final pushed head
474dbd9004216e0bd43fd3a92d09532f7855c57f: CI run 39853785873 passed, including all four jobs and both native Bun suites. These are measured counts from the completed job logs on that exact head.npm run release:checknpm run release:checkbun run release:checkbun run release:checkSeparate
npm run test:bunsuites passed 1021 / 0 / 0 across 48 files on each Node matrix leg. Full release suites also have zero cancellations and TODO tests. No thresholds, exclusions, dependency pins or workflows changed.Local
node --test test/identity.test.tspassed 20/20, zero failures/skips/cancellations/TODO, throughpm test pm-github-identity-collisions --run --progress --timeout 600. Focusedidentity.tscoverage is exact 100/100/100/100. Build, typecheck, test typecheck, lint, docstring, zero-duplication, privacy, changelog checks and isolated strict tracker health passed. Full gates ran in CI because of shared-host CPU load. Public-corpus validation was rerun and reproduced the measurements above without source writes.Fix commit:
474dbd9004216e0bd43fd3a92d09532f7855c57fincludes source, regressions, docs, and append-only pm evidence together. PM item: pm-github-identity-collisions. It remains in progress and the implementation claim is released for orchestrator verification. The journal must be retained for recovery; core external_ref clearing remains an explicitly refused pinned-SDK limitation.