Skip to content

fix(unassign): base the unassign label set on live GitHub labels instead of the cached row - #1377

Merged
joryirving merged 2 commits into
mainfrom
courier/misospace/dispatch/issue-1366
Oct 23, 2026
Merged

joryirving merged 2 commits into
mainfrom
courier/misospace/dispatch/issue-1366

Conversation

@itsmiso-ai

Copy link
Copy Markdown
Contributor

What

POST /api/issues/unassign computed the label set for its updateIssueLabels call — 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

  • The live read happens inside the per-issue mutation anchor (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.
  • The remote write stays inside the fenced write (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.
  • Live-read failure → audit row with errorMessage: "Could not verify live GitHub labels: ...", success: false, beforeLabels = cached row, then 503 — field-for-field the shape src/app/api/issues/actions/route.ts uses.

Invariants this change keeps, and every other path enforcing the same rule

  1. A full-label-set PUT (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 in src/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.ts and src/lib/issue-capacity-handoff.ts — their label decisions re-read live labels via the forge/claim-gate; unchanged here.
    • src/lib/groomer/run.ts also calls updateIssueLabels; it is a separate subsystem outside this PR's scope and is not modified (no claim is made about its snapshot handling here).
  2. Claim/unclaim paths never replace the whole set — they use targeted addIssueLabel/removeIssueLabel (src/app/api/issues/claim/route.ts, src/lib/issue-claim.ts); unchanged.
  3. Every remote write on this route stays inside the fenced write under the per-issue anchor (fix(ownership): guard the remote agent/* label write in POST /api/issues/unassign #1361) — unchanged placement; the live read was added inside the anchor, after the identity-400 and before the fenced write.
  4. Fail closed on forge-read failure: no remote write, no cache write, failure audit row, 503 — matches the assign route; pinned by a new test.
  5. The cache mirror (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 passed
  • npm run test (full suite) → 5279 passed, 26 skipped, 1 todo
  • npm run lint → 0 errors (1 pre-existing unrelated warning in src/app/login/page.tsx)
  • npm run typecheck → clean

CI checks are pending (not observed from this run).

Tests added/changed

  • Live-not-cache derivation for both unassign_agent and unassign_owner (live-only label survives the PUT; cache and audit mirror the live-derived set).
  • 400 without writes when the agent label is gone live but still cached.
  • 503 fail-closed shape (no remote/cache write, failure audit, cached row as beforeLabels baseline).
  • Read-ordering inside the anchor (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 beforeLabels when 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.

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 head 4fcae471.
  • Satisfied — req-54667ad772c7: getLiveIssueLabels is called inside withIssueMutationGuard, after the issue identity check, and the new ordering test observes anchor-entered → issue-read → live-labels-read.
  • Satisfied — req-9aefc8e166ee: The full-set PUT is derived from the live label snapshot rather than issue.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 getLiveIssueLabels into 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 joryirving left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@joryirving
joryirving merged commit 6fd6613 into main Oct 23, 2026
13 checks passed
@joryirving
joryirving deleted the courier/misospace/dispatch/issue-1366 branch October 23, 2026 18:14
@its-miso its-miso Bot mentioned this pull request Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(unassign): base the unassign label set on live GitHub labels instead of the cached row

2 participants