Repository navigation
fix(unassign): base the unassign label set on live GitHub labels instead of the cached row - #1377
Conversation
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M3.1-Flash (anthropic) — primary route · pr-reviewer-action v3.7.0
Recommendation: approve — no blocking or major findings.
Requirement Coverage
- Satisfied —
req-e589bfe1ad7c: Lint and typecheck CI checks passed on head4fcae471. - Satisfied —
req-54667ad772c7:getLiveIssueLabelsis called insidewithIssueMutationGuard, after the issue identity check, and the new ordering test observesanchor-entered → issue-read → live-labels-read. - Satisfied —
req-9aefc8e166ee: The full-set PUT is derived from the live label snapshot rather thanissue.labels; tests cover live-only label preservation for both agent and owner unassignment.
Linked Issue Fit
All PR 1366 acceptance criteria are implemented: live labels determine the written set, a forge-read failure returns 503 without remote or cache writes while creating a failure audit, identity mismatches do not reach GitHub, and the remote write remains inside runFencedRemoteWrite.
Claims Falsify
- Claim 1 — held: 12/12 listed items: the live read is inside the mutation anchor and follows the body/anchor identity check; no missed sibling call site was found in the unassign path.
- Claim 2 — held: 12/12 listed PR-scoped items: the live snapshot flows through
getLiveIssueLabelsinto the PUT, with cache and audit mirroring the derived result; the explicitly out-of-scope groomer caller was not represented as compliant by this PR.
Sources
Verified src/app/api/issues/unassign/route.ts, its test file, src/lib/claim-gate.ts, src/lib/assignment-conflicts.ts, the sibling assign/claim paths, ownership reconciliation, capacity handoff, src/lib/issue-claim.ts, and the mutation-guard interface.
Tool Harness Findings
The 24-call harness inspection confirmed the producer (fetchIssue) → live label mapping → guarded route decision → fenced PUT → cache/audit persistence chain, and found no relevant counterexample; the harness exhausted its budget before broader investigation, but no unresolved issue affecting this scoped change remained.
joryirving
left a comment
There was a problem hiding this comment.
Secondary hard-blocker review on exact head 4fcae471f74c20227dcde8ce7845f2fd2a215685: APPROVE. The unassign route now derives its full-set GitHub PUT from a live label snapshot taken inside the per-issue mutation anchor and after canonical body/issue identity validation, so unrelated labels added since the last cache sync are preserved. Forge-read failure fails closed with 503, zero remote/cache writes, and a failure audit; successful writes remain inside runFencedRemoteWrite, with cache/audit persistence only after the remote write succeeds. Tests cover both agent and owner removal, live-only label preservation, stale-cache/no-live-target behavior, read ordering, and the fail-closed shape. Primary review and all exact-head checks are green. No independent ownership/correctness blocker found.
What
POST /api/issues/unassigncomputed the label set for itsupdateIssueLabelscall — a FULL-label-set PUT — from the cached Prisma row. A label added on GitHub since the last sync was silently stripped by the next unassign. The route now reads the live label set through the same forge capability the sibling assign route uses (getLiveIssueLabels,src/lib/claim-gate.ts) and derives the written set from it, failing closed with a 503 (failure audit row, zero writes) when GitHub is unreachable — the route already could not succeed without GitHub, so the live read costs no availability.Fixes #1366
How
withIssueMutationGuard) and after the body/anchor identity check, so the snapshot the fenced PUT applies is taken while the holder owns the issue, and a mismatched body never reaches GitHub.runFencedRemoteWrite, fix(ownership): guard the remote agent/* label write in POST /api/issues/unassign #1361); the cache mirror and the success audit follow a successful write as before.errorMessage: "Could not verify live GitHub labels: ...",success: false,beforeLabels= cached row, then 503 — field-for-field the shapesrc/app/api/issues/actions/route.tsuses.Invariants this change keeps, and every other path enforcing the same rule
updateIssueLabels) must apply a snapshot of the LIVE GitHub label set taken under the issue's mutation anchor, never the cached row (the rule stated insrc/lib/issue-claim.ts).POST /api/issues/unassign— kept: this PR adds the live read + fail-closed 503 (this diff).POST /api/issues/actions(assign,src/app/api/issues/actions/route.ts) — already keeps it (Claim gate reads stale cache — GitHub agent/* claims can be lost to sync lag #1037 live read + 503); unchanged here, and the new failure shape is copied from it.src/lib/ownership-reconciliation.tsandsrc/lib/issue-capacity-handoff.ts— their label decisions re-read live labels via the forge/claim-gate; unchanged here.src/lib/groomer/run.tsalso callsupdateIssueLabels; it is a separate subsystem outside this PR's scope and is not modified (no claim is made about its snapshot handling here).addIssueLabel/removeIssueLabel(src/app/api/issues/claim/route.ts,src/lib/issue-claim.ts); unchanged.prisma.issue.update) only runs after a successful fenced remote write — unchanged.Validation (local receipts, head 4fcae47)
./node_modules/.bin/vitest run src/app/api/issues/unassign/route.test.ts→ 29/29 passednpm run test(full suite) → 5279 passed, 26 skipped, 1 todonpm run lint→ 0 errors (1 pre-existing unrelated warning insrc/app/login/page.tsx)npm run typecheck→ cleanCI checks are pending (not observed from this run).
Tests added/changed
unassign_agentandunassign_owner(live-only label survives the PUT; cache and audit mirror the live-derived set).beforeLabelsbaseline).anchor-entered → issue-read → live-labels-read); mismatched-body 400 now asserts the forge is never reached.Handoff
Settled decisions: failure audit keeps the cached row as
beforeLabelswhen the live read fails (live snapshot never landed — same as assign); the success/guard-lost audits now record the live snapshot. Live read placed after the identity check so wrong-body requests never hit GitHub. Independent review pass: approve (one minor test gap — owner live divergence — fixed in 4fcae47). Remaining external verification: CI.