feat(lifecycle): persist idempotent claim and status transition evidence (#1202)

[itsmiso-ai]

What

Fixes #1202. Adds IssueLifecycleEvent, the durable worker-neutral event stream for a work item: claim, release, status, completion and merge facts in one append-only table that belongs to the Issue it describes.

Issue holds only current state and AuditLog is per-request, per-writer and not deduplicated, so neither can tell a first claim from a third attempt — and a later claim-to-merge read must not infer the first claim from issue creation. Every writer goes through one contract (src/lib/issue-lifecycle.ts), so the taxonomy, the idempotency rules and the timestamps cannot drift per route.

New surface only: one model, one additive migration, one lib module, docs. No API response, worker contract, token or model field changes.

Writers traced and instrumented

Every status writer was traced before instrumenting; each now records through the shared contract:

Writer	Site	Event
Claim	src/app/api/issues/claim/route.ts	claim
Unclaim + stale-AgentWork sweep	src/lib/issue-claim.ts (shared release core, both callers)	claim_released
Board drag	src/app/api/issues/move/route.ts	status_changed
Manual / agent status	src/app/api/issues/status/route.ts	status_changed
Operator groom	src/app/api/issues/groom/route.ts	status_changed
Hosted groomer plan application	src/lib/groomer/run.ts	status_changed, issue_closed
Reconciler	src/app/api/issues/reconcile/route.ts	status_changed, pr_merged, issue_closed
Issue sync + closed-issue reconciliation	src/lib/issue-sync.ts (makePrismaIssueStore, shared makePrismaReconcileIssueStore)	status_changed, issue_closed, issue_reopened
GitHub webhook	src/app/api/issues/webhook/route.ts	status_changed
Worker task report	src/app/api/agents/[agentName]/tasks/report/route.ts, src/lib/report-close.ts	attempt_failed, completed

api:claim, api:unclaim, api:move, api:status, api:groom, stale-recovery, hosted-groomer, reconciler, sync, webhook and agent-report are the recorded sources; each row keeps its source, actor, agent, optional prior/new status, source identity, occurrence time and observation time.

Idempotency — two rules, because the writers are not equally repeatable

1. Transitions, not writes. A Dispatch-authored write records only a real transition (a claim also counts when ownership changed hands without the status moving — a force-claim between agents moves no label). A redelivered claim of an already-claimed issue, a drag back into the same column, a set-status to the status already carried and a grooming plan that changed no status/* label all record nothing, while claim → release → reclaim keeps the first claim and both attempts.
2. Source identity. An observed event carries the source's own identity as a unique dedupeKey and upserts with an empty update, so a GitHub redelivery or a repeated sync collapses onto the row that already exists without rewriting its timestamps: webhook webhook|<repo>|<n>|<delivery id>|<label>, sync sync|<repo>|<n>|<kind>|<from>|<to>|<issue updated_at>, reconciler reconciler|<repo>|<n>|<kind>|<pr>|<merged_at>.

Durability across the GitHub/local boundary

Dispatch mutates GitHub and its own cache in two separate systems, so an event write cannot be transactional with the GitHub label change. Two rules close that gap, one per direction:

• Evidence before mirror (sync). A synced issue records its evidence before the cache row is updated, so a failed event write aborts that mirror instead of leaving a cache that has already moved past the evidence. The next run derives the same transition from the unchanged row and upserts onto the same source-derived key — one row, not a duplicate, and nothing to reconstruct. recordSyncedIssueEvents therefore propagates instead of swallowing.
• Mirrored-state fallback (Dispatch-authored writes). Claim and set-status mutate GitHub first; if the local transaction then rolls back, a retry reads live labels that show no change. Both routes now pass their cached label set as mirroredBefore, and the recorder prefers the observed labels whenever they show the change, falling back to the mirror only when they do not — marking such a row recoveredFromMirroredState. A redelivered claim of an already-committed issue still records nothing, because the committed cache row already equals the post-write labels. Decision logic stays on the live GitHub labels throughout.

Invariants this change keeps

1. Existing API/worker behaviour is unchanged. Every response shape, status code, audit row, lease, AgentWork write and GitHub label write is as before; the event is an additional write next to it. Two nuances follow from the durability rules above, and both reuse failure shapes the code already had: an API-owned write commits its event inside the same transaction as the state it describes (claim, move, status, groom, unclaim), and a failed evidence write now also defers a sync mirror for that issue to the next run — the sync core already caught and reported a throwing update, so a repo with a failing lifecycle insert is reported errored and retried rather than silently mirrored.
2. GitHub remains the status-label authority. No writer changes what GitHub says; the lifecycle table is evidence, never a source of truth. Events are never replayed onto Issue, so a late or out-of-order observation cannot move the present status.
3. Observation is never backdated. An observed event keeps the source's own occurrence time (merged_at, issue updated_at, closed_at) in occurredAt and the wall clock in observedAt; a Dispatch-authored write has occurredAt == observedAt. No reader may infer a first claim from the absence of rows, because the migration is backfill-free — stated explicitly in the docs.
4. The distinctions the issue asks for stay separate. claim_released ≠ attempt_failed ≠ completed ≠ issue_closed ≠ pr_merged. A merge is recorded only from an observed merged PR in the reconciler, keyed by PR number and merged_at — never from a worker reporting success. A release that deliberately retains status/in-review records the unchanged status rather than implying the work finished. A groomer close records its own issue_closed under the application key, because it mirrors state=closed in the same run and sync could otherwise never observe the closure.
5. No per-attempt cost regression on sync. The repo full name a synced event needs comes from the findIssue lookup the sync core already performs (kept in a one-entry map consumed by the update), so a sync still makes no extra query per issue; the reconcile store's single repo lookup moved ahead of the update rather than being added.

Other code paths that touch the same state

• transitionIssueStatus (src/lib/issue-status.ts) is untouched: it is the GitHub-side swap, and the event is recorded from the before/after label sets the caller actually wrote.
• refreshSingleIssue and the groom route's post-write refresh are unchanged: a refresh mirrors GitHub state and its change is picked up as the next sync event, which is correct — the grooming decision itself already recorded its own transition.
• next-task/admission reads status/* labels and the lease table, neither of which this change alters, so claimability and admission behaviour are unaffected. Admission writes no status label, so it records nothing.
• src/app/api/issues/label, /unlabel, /unassign and /actions change non-status labels only and record nothing.

Local validation (run against the integrated change on this branch)

cwd /workspace, revision c20a430, node_modules installed from package-lock.json. prisma generate was re-run after the schema change in the first commit; the review fixes touch no schema.

• npm run typecheck (tsc --noEmit) → clean.
• npm run lint (eslint .) → 0 errors; 1 pre-existing warning in src/app/login/page.tsx, unrelated to this change.
• npm test (vitest run) → 177 files passed / 3 skipped; 4387 passed | 16 skipped | 1 todo, 0 failed.
• npx next build → succeeded (production build, routes emitted).
• npx prisma validate → schema valid.

New tests: the contract unit tests (src/lib/issue-lifecycle.test.ts: transition guard, claim guard incl. force-claim and redelivery, the mirrored-state fallback and its limitation, occurrence-vs-observation, upsert idempotency, sync collapse, close/reopen distinctness, failure propagation), plus per-writer tests in the claim, status, unclaim/release, stale-sweep (new src/lib/stale-work.test.ts), move, groom, webhook, reconcile, both sync stores, report and report-close suites — covering a failed insert and its retry for each, a repeated delivery, an absent-label unlabel with and without a delivery id, a grooming noop/partial, a groomer close followed by a sync, and every known status writer.

One unrelated pre-existing flake surfaced during this run and is filed separately as #1257: src/lib/pr-fix-surfacing.test.ts asserts the rendered comment body does not contain 99, which collides with the millisecond field of its own timestamp footer. It failed once on a full-suite run and passed three targeted reruns unchanged; that file is not touched by this PR.

Remaining external verification (not claimed green here)

• CI is pending and left to the operator. Nothing in this body claims a check passed beyond the local runs above. In particular prisma migrate deploy was not run locally — this sandbox has no Postgres — so the new migration is unverified against a live database until CI's db:deploy step and the integration tests (src/lib/sync-lock.integration.test.ts, src/app/api/agents/[agentName]/tasks/report/route.integration.test.ts, src/lib/groomer/apply.integration.test.ts) run. Coverage thresholds are unchanged from the measured baseline; this change adds covered code only.
• The earlier green run on 1953df9 does not carry over to c20a430; the checks on this head have not been observed.
• AI review and human review are not awaited; later findings are follow-up work.

Known limitations, documented in the PR's docs rather than fixed here:

1. Dispatch's own label writes also generate GitHub webhook deliveries, so a delivery processed before the originating request mirrored its change can record the same move once as api:* and once as webhook. Both rows carry identical fromStatus/toStatus, so a reader deduplicating consecutive identical transitions sees one.
2. The mirrored-state fallback reads the cache as a witness of the pre-write state, which holds only when the cache was in step with GitHub. If sync lag left it behind, a recovered row's fromStatus can describe the last mirrored state rather than the state the claim displaced; the two are indistinguishable at retry time, and separating them needs an intent record written durably before the GitHub mutation. Every such row is marked recoveredFromMirroredState, so a reader can tell a recovered row from an observed move, and no status change is lost — sync independently records the real GitHub-side move.

[joryirving]

Reviewed Dispatch #1247 against issue #1202 and actual GitHub/Prisma claim, sync, webhook, stale-claim and hosted groomer paths at exact head 1953df9. The additive schema and shared recorder are good foundations; six correctness defects remain, including the four that the automated reviewer independently flagged and two source-grounded omissions. CI, Security Audit, PR Smoke, Image Build and AI PR Review all completed green on this head; GitHub reports mergeable. Green gates are not proof of the new durability failure cases.

P1: External GitHub success + DB failure permanently loses claim history. In src/app/api/issues/claim/route.ts:204-238, GitHub gets the agent and in-progress labels before the local transaction. A failed recordClaimEvent rolls back the cache/event, not GitHub; a retry reads the new live labels as before-state and suppresses the first claim because nothing changes. Introduce a recoverable, idempotent durable intent/event or reconciliation path bridging external success and failed local commit. Pin the GitHub-success, failed-event-insert, retry scenario.

P1: Sync mirroring permanently swallows event insert failures. makePrismaIssueStore.updateIssue in src/lib/issue-sync.ts:480-500 first commits the mirrored issue update, then calls recordSyncedIssueEvents with errors caught/logged inside recordLifecycleEventQuietly. The next sync sees already-updated state and derives no transition. This can silently omit externally authored first claims/status moves/closes forever. makePrismaReconcileIssueStore has the same ordering. Make the event atomic with the mirror or persist a retryable source observation independent of current-state diffs; test insert failure then repeated sync yields exactly one event.

P2: Hosted groomer records non-transitions. internal writer at src/lib/groomer/run.ts:1232-1245 calls recordLifecycleEventQuietly unconditionally rather than shared recordStatusTransition. Noop and partial plans that alter other fields only create bogus status_changed rows with identical from/to. Guard against unchanged status and test both cases.

P2: Webhook absent-label removal emits a false status transition. src/app/api/issues/webhook/route.ts:206 tests nextLabels !== currentLabels by reference. The unlabeled branch calls .filter() which returns a new array even if the status label wasn't present, causing false removal events; no delivery ID can make them repeat. Test missing-label removal, with/without X-GitHub-Delivery.

P2: Stale-release retry records a second claim_released. The actual preserveStatus release at src/lib/issue-claim.ts:180 writes an event without a dedupe key; a crash after that commit but before AgentWork finalization results in allowMissingAgent repair writing another event with a different key at :158. Fix using one per-release stable identity across initial and recovery writes, without collapsing a legitimate later reclaim/re-release; test release → crash → repair and release → reclaim → release.

P2: Hosted groomer closes an issue without recording issue_closed. src/lib/groomer/run.ts:1140-1143 mirrors Issue.state=closed when applied.closed, but the only lifecycle insert at :1232 is a status_changed record. The next sync sees before.state=closed and never emits the closure. Record an idempotent issue_closed for confirmed groomer closure (separate from kanban done), test groomer-close then sync.

Please fix the six cases before merge. They all sit within #1202's required append-only claim/status/closure and retry semantics, not the later #1203 read UI/merge backfill. No schema migration data-loss issue was found in the additive SQL.

[joryirving]

P1: GitHub labels change before this transaction. If the event insert fails, the GitHub mutation survives while this transaction rolls back. A retry reads the already-claimed live labels and recordClaimEvent sees no transition, permanently losing the first claim. Make this cross-boundary state change recoverable and add GitHub-success/DB-failure/retry regression.

[itsmiso-ai]

Fixed in c20a430.

The retry no longer loses the claim. The GitHub write still happens before the transaction, but the event's before-state is no longer taken only from the live labels: recordClaimEvent now accepts mirroredBefore — the cache row, which is written inside that same transaction and therefore still holds the pre-claim set after a rollback.

src/lib/issue-lifecycle.ts resolves the before-state once, for both recordStatusTransition and recordClaimEvent: prefer the observed labels whenever they already show the change, and fall back to the mirrored set only when they do not. The recovered row carries metadata.recoveredFromMirroredState: true.

• Attempt 1: GitHub applies, event insert fails, transaction rolls back.
• Attempt 2: observed labels show nothing (already claimed), mirrored set shows status/ready → the claim is recorded with the real before-state.

A redelivered claim of an already-committed issue still records nothing, because the committed cache row already equals the post-write labels. The decision logic — done check, conflict analysis, buildNewLabels, transitionIssueStatus — is untouched and still reads the live GitHub labels.

src/app/api/issues/status/route.ts had the identical shape and is fixed the same way, so a set-status that fails after the GitHub swap is also recoverable.

Regression tests: src/app/api/issues/claim/route.test.ts (GitHub-success / failed-insert / retry) and src/app/api/issues/status/route.test.ts, plus the contract cases in src/lib/issue-lifecycle.test.ts.

One honest limitation, now documented in docs/issue-lifecycle-events.md and pinned by a test: the mirrored row is a witness of the pre-write state only when the cache was in step with GitHub. If sync lag left it behind, a recovered row's fromStatus can describe the last mirrored state rather than the state this claim displaced — the two are indistinguishable at retry time, and separating them would need an intent record written durably before the GitHub mutation. Every such row is marked recoveredFromMirroredState, so a reader can tell a recovered row from an observed move; no status change is lost either way, because sync independently records the real GitHub-side move.

Re: "move/status/groom use the same external-write-before-local-event pattern and should be reviewed together" — checked. move and groom already pass the cached label set as before rather than live labels, so neither had the shape that loses evidence; only claim and status did, and both are fixed.

[joryirving]

P1: issue.update has already committed on line 481 before recordSyncedIssueEvents, which logs/swallow DB insert failures internally. Next sync sees no new label/state delta, so the missing event can never be reconstructed. Require atomic mirror/evidence or a durable replay path independent of comparing to the updated cache; test event-insert failure followed by successful sync.

[itsmiso-ai]

Fixed in c20a430.

Two changes, one per half of the finding.

The ordering. Both makePrismaIssueStore.updateIssue and makePrismaReconcileIssueStore.updateIssue now record the evidence before prisma.issue.update. A failed event write therefore aborts that issue's mirror write instead of landing a cache that has already moved past the evidence.

The swallow. recordSyncedIssueEvents now calls recordLifecycleEvent directly, so it propagates. The quiet wrapper was the reason a failure could only cost a log line; with the mirror behind the evidence, that would be the wrong trade.

Why this converges rather than retrying forever: when the event write fails, the cache row is unchanged, so the next sync derives the same transition from the same before-state and lands on the same source-derived dedupeKey. All three kinds are stable across that re-derivation — status_changed keys on the issue's updated_at, issue_closed on closed_at (falling back to updated_at), issue_reopened on updated_at.

Blast radius, stated plainly. A throwing updateIssue is caught by the per-repo handler in syncIssuesForRepos and by the per-issue handler in reconcileClosedIssues, so a failing lifecycle insert now marks that repo errored instead of mirroring it. That is the same shape the code already had for a failed mirror write — no new failure mode — and it is self-healing on the next run, including during a rolling deploy where the new table may briefly not exist yet. The alternative (mirror first, log the failure) is what made the row unrecoverable, since the next sync derives nothing from an unchanged source.

Regression tests: src/lib/issue-sync.test.ts now pins the ordering for both stores — a rejected event write leaves issue.update uncalled, and the retry produces exactly one row per identity (3 attempts, 2 identities for the reconcile close, which records both status_changed and issue_closed).

[joryirving]

P2 (two paths): This unconditional status_changed insert occurs even when a noop/partial groom plan changed no status label, fabricating identical from/to transitions; use recordStatusTransition. Separately, when applied.closed sets cached Issue.state=closed at lines 1140-43, this writer never emits issue_closed. Sync then sees already-closed cached state and cannot observe the closure either. Add a distinct, idempotent issue_closed event for confirmed groomer closes, and regress both no-status-change and close→sync.

[itsmiso-ai]

Both halves fixed in c20a430.

Non-transitions. The unconditional recordLifecycleEventQuietly(status_changed) is gone. The groomer now goes through a new recordStatusTransitionQuietly — recordStatusTransition with the same catch-and-warn guarantee the groomer needs, since a run whose mutations already landed must not fail over its evidence row — passing before: analyzed.labels / after: applied.labels, the two sets the application actually had. So a noop, and a partial that landed something before the status step, record nothing instead of a row with identical from/to.

The close. applied.closed now records its own issue_closed, keyed hosted-groomer|<repo>|<n>|issue_closed|<applicationKey> and written independently of the transition. That is what makes the close observable at all: the groomer mirrors state=closed in the same run, so by the time sync looks, the cache already reads closed and the closure is unobservable. applicationKey is stable across a replayed application (it is derived from the pinned plan/evidence digest, not from the run), so a retry collapses onto the same row.

Net effect for a plan that closes without moving the status label: one issue_closed, no status_changed. For a plan that moves the status without closing: one status_changed, no issue_closed.

Regression tests in src/lib/groomer/run.test.ts: the noop case, the partial case, and the close-then-sync case.

[joryirving]

P2: .filter() returns a fresh array even when the unlabeled status label was absent. nextLabels !== currentLabels is then true by reference and records a phantom status change; with no delivery ID repeats may persist multiple rows. Check actual label membership/value change and test absent-label redelivery.

[itsmiso-ai]

Fixed in c20a430.

The unlabeled branch now returns the same array reference when the label is absent:

: currentLabels.includes(labelName)
  ? currentLabels.filter((label) => label !== labelName)
  : currentLabels;

Array.prototype.filter always allocates, which is why nextLabels !== currentLabels was true by reference even when the contents were identical. With membership-preserving references the existing gate becomes exact for all four action/label combinations — labeled/present and unlabeled/absent return the same reference and record nothing, while the two real mutations return a new array and record.

This also removes a pointless cache write: an absent-label delivery no longer calls issue.update with a fresh-but-identical label array.

On "with no delivery ID can also insert this false row repeatedly" — with no false transition there is nothing to repeat. The no-delivery-id path is still covered for genuine mutations, which remain unkeyed by design (GitHub sent no identity to key on).

Regression tests in src/app/api/issues/webhook/route.test.ts: unlabeled for a status label that is not in the cache, with and without an X-GitHub-Delivery id — both assert no issueLifecycleEvent write at all — plus the existing real-removal case still recording exactly one event.

[joryirving]

P2: This real preserveStatus release uses an unkeyed claim_released event. A crash after it commits but before retiring AgentWork retries through the already-unassigned branch, which writes another differently-keyed claim_released. Share a stable per-release identity across initial release and repair, and test release→crash→repair without swallowing a future distinct release.

[itsmiso-ai]

Fixed in c20a430.

releaseIssueClaim now takes an optional releaseKey — a stable identity for this release attempt — and builds one dedupe key from it that both paths share:

const releaseDedupeKey = options.releaseKey
  ? lifecycleDedupeKey("claim-released", issue.id, agentLabel, options.releaseKey)
  : null;

The real preserveStatus release and the allowMissingAgent repair both use it, so release → crash → repair is one row instead of two differently-keyed rows.

src/lib/stale-work.ts supplies it from the AgentWork row being released: lifecycleDedupeKey("agent-work", work.id). That row is the claim epoch, and it is stable across a retry of the same sweep — the retry re-enters the STALE + staleClaimReleasePending branch for the same row. So a reclaim-and-release cycle gets a new AgentWork row and therefore a new key, and a genuine second release is never collapsed into the first. That is the "without swallowing a future distinct release" requirement, and it is why the epoch identity has to come from AgentWork rather than from the issue's labels or a timestamp.

Without a releaseKey the behaviour is byte-for-byte unchanged: the unclaim route keeps its "already-released" repair key and its real release stays unkeyed, so no other caller's semantics move.

Regression tests: src/lib/issue-claim.test.ts covers release → repair (one row), release → reclaim → release across two epochs (two rows), and the unchanged no-releaseKey path; a new src/lib/stale-work.test.ts drives the sweep end to end — crash after the event commits, before the AgentWork row is finalized, then retry — and asserts exactly one claim_released.

[itsmiso-ai]

Review follow-up — all six cases addressed in c20a430

Both reviews (its-saffron[bot], joryirving) are addressed; four findings were raised twice, independently, and all six are fixed. Each has a reply on its own thread with the mechanism and the test that pins it. Summary of the second-pass commit:

#	Finding	Fix
1	GitHub labels change before the claim transaction; a failed insert + retry suppressed the event (claim/route.ts:221)	mirroredBefore on the shared contract — claim and status routes pass the cached label set, which survives the rollback; recorder prefers observed labels, falls back to the mirror, marks the row recoveredFromMirroredState
2	Sync committed the mirror before a quietly-swallowed event insert (issue-sync.ts:489)	Both sync stores record evidence before issue.update; recordSyncedIssueEvents propagates, so a failure defers that mirror and the next run re-derives onto the same dedupe key
3	Groomer recorded status_changed for noop/partial plans (groomer/run.ts:1232)	Goes through the shared guard via a new recordStatusTransitionQuietly, keeping the groomer's never-fail-over-evidence guarantee
4	Groomer close emitted no issue_closed, and sync could never observe it (groomer/run.ts:1140)	Idempotent issue_closed keyed by applicationKey, recorded independently of the transition
5	filter() made nextLabels !== currentLabels true for an absent-label unlabel (webhook/route.ts:206)	Membership-preserving references make the existing gate exact; also removes a pointless cache write
6	Real preserveStatus release was unkeyed, so a retry duplicated it (issue-claim.ts:180)	Caller-supplied releaseKey = the AgentWork row (one claim epoch, stable across a sweep retry), shared by the release and its repair; a reclaim gets a new row and therefore a new key

On #2 specifically, since it changes a failure shape: a throwing updateIssue is already caught per repo in syncIssuesForRepos and per issue in reconcileClosedIssues, so a failing lifecycle insert now reports that repo errored instead of mirroring it — self-healing on the next run, and no new failure mode. It is the deliberate trade against leaving an unrecoverable gap.

On #1 and #2 together: I checked move and groom for the same external-write-before-local-event shape, as suggested. Both already pass the cached label set as before rather than live labels, so neither had the gap; only claim and status did.

One limitation I did not fix, stated rather than buried: the mirrored-state fallback treats the cache as a witness of the pre-write state, which holds only when the cache was in step with GitHub. Under sync lag a recovered row's fromStatus can describe the last mirrored state rather than the state the claim actually displaced — the two are indistinguishable at retry time, and separating them would require an intent record written durably before the GitHub mutation (a second table). Every recovered row is marked recoveredFromMirroredState, so a reader can separate it from an observed move, and nothing is lost because sync independently records the real GitHub-side move. It is documented in docs/issue-lifecycle-events.md and pinned by a test. Happy to take the durable-intent route in a follow-up if you would rather have it exact here.

Local validation on this head (cwd /workspace, revision c20a430): npm run typecheck clean · npm run lint 0 errors, 1 pre-existing warning in src/app/login/page.tsx · npm test 177 files passed / 3 skipped, 4387 passed | 16 skipped | 1 todo, 0 failed · npx next build succeeded · npx prisma validate valid. prisma migrate deploy was still not run (no Postgres in this sandbox).

CI on c20a430 has not been observed and is left to the operator — the earlier green run was on 1953df9 and does not carry over. An unrelated pre-existing flake in src/lib/pr-fix-surfacing.test.ts surfaced during this run and is filed as #1257; that file is untouched here.

A second adversarial pass over the final diff found no further code defects and no vacuous tests; its two remaining observations (no direct test for the reconcile store's ordering, and this limitation) are both now covered.

Superseded by a newer automated review for this pull request.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: qwen3.8-flash-next-chat (openai) — fallback (primary failed) · pr-reviewer-action v3.4.1

Recommendation

Approve. The PR introduces a durable, idempotent lifecycle event stream (IssueLifecycleEvent) as requested in issue PR 1202. The migration is additive and safe. The implementation correctly handles the complex durability requirements (e.g., "Evidence before mirror" for sync, mirroredBefore fallback for claims) and addresses previous review feedback regarding idempotency keys and phantom events.

Findings

No blocking issues found. The code follows repository conventions and the linked issue's acceptance criteria.

Required Checks Disposition

1. Review migration for data loss risk: Satisfied. The migration 20261013120000_add_issue_lifecycle_events creates a new table with no changes to existing tables or columns. It is purely additive.
2. Test migration on a copy of production schema: Not Applicable. This is a standard additive Prisma migration. CI runs Database migrations and Database integration checks which validate the schema against a fresh database. Testing on a specific production copy is not feasible in this context and not required for additive changes.
3. Review for path traversal vulnerabilities: Not Applicable. The PR modifies API routes and database logic but does not handle file system paths or user-supplied file inputs. The path_handling_changes risk flag appears to be a false positive from the classifier based on generic keywords or unrelated file touches.
4. Test with edge-case paths (null bytes, symlinks): Not Applicable. Same as above; no file system interaction is introduced.

Linked Issue Fit

The PR fully implements issue PR 1202:

• Schema: Adds IssueLifecycleEvent with required fields (issueId, kind, source, actor, occurredAt, observedAt, dedupeKey).
• Writers: Instruments all specified writers (claim, unclaim, move, status, groom, groomer, reconcile, sync, webhook, report).
• Idempotency: Implements two rules: transition-based guards for Dispatch-authored writes and source-identity-based upserts for observed events.
• Durability: Handles the GitHub/Local boundary via mirroredBefore (claims) and "record before mirror" (sync).
• Acceptance Criteria:
  • Claim → release → reclaim retains history: Verified by tests in src/lib/issue-claim.test.ts.
  • No duplicate events on redelivery: Verified by dedupe keys in webhook/sync/reconcile.
  • Existing behavior unchanged: No API response changes; tests confirm existing flows still pass.

Standards Compliance

• Prisma Schema: Follows strict relations (onDelete: Cascade).
• Code Standards: Uses shared contract src/lib/issue-lifecycle.ts to prevent drift. Error handling uses error instanceof Error pattern where applicable (in the recorder). Validation is present.
• Documentation: New doc docs/issue-lifecycle-events.md explains taxonomy and failure modes. AGENTS.md updated with subsystem map entry.

Thread Dispositions

• Thread PRRT_kwDOSZt6uM6sFrwt (src/lib/issue-claim.ts): Fixed. The author added releaseKey support to releaseIssueClaim using the AgentWork row ID as a stable identity, ensuring retries collapse onto one event. Tests in src/lib/stale-work.test.ts verify this.
• Thread PRRT_kwDOSZt6uM6sFrwn (src/app/api/issues/webhook/route.ts): Fixed. The unlabeled branch now preserves array reference if the label is absent, preventing phantom transitions. Tests in src/app/api/issues/webhook/route.test.ts cover absent-label redelivery.
• Thread PRRT_kwDOSZt6uM6sFrwj (src/lib/groomer/run.ts): Fixed. Groomer now uses recordStatusTransitionQuietly which respects the transition guard (no-op plans record nothing) and emits distinct issue_closed events keyed by application key. Tests in src/lib/groomer/run.test.ts verify both cases.

Human Review Dispositions

• Review 5510858147 (joryirving): Addressed.
  • P1 Claim History Loss: Addressed via mirroredBefore fallback in claim/route.ts. If the transaction fails after GitHub update, the retry uses the cached pre-state to reconstruct the event, marked recoveredFromMirroredState.
  • P1 Sync Event Swallowing: Addressed by reordering issue-sync.ts to record events before updating the cache. If event recording fails, the cache update is aborted, allowing the next sync run to re-derive and record the event.

Outstanding Human Change Requests

• @​joryirving's change request (1953df9, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

[itsmiso-ai]

Takeover verification — no further code work needed

I took over this PR to settle the state on head c20a430. I did not change code: nothing I found required it. Recording the receipts so a follow-up run does not have to re-derive this.

Published state (verified, not inferred). PR open and non-draft, head c20a4303b1f64e4a15f5d0e2e906c0caa55afc42 on courier/misospace/dispatch/issue-1202, identical to local HEAD and to origin/<branch> — nothing unpushed, and nothing published from any other branch.

CI on this head: 13/13 green (Docker Build, Docker Build (MCP), npm audit, Tests, Lint, Workflow lint, Coverage, Database migrations, Typecheck, smoke, Build, Database integration, review). This supersedes the body's "the checks on this head have not been observed" — that was true when written and is no longer. It also resolves two caveats the body left open: the migration is verified against a live Postgres (Database migrations), and the integration tests (sync-lock, tasks/report, groomer/apply) did run (Database integration).

Review feedback, all addressed. The human CHANGES_REQUESTED review and the dismissed bot review are both on the stale head 1953df9. Its five threads each carry an author reply describing the c20a430 fix, and I independently verified each fix is genuinely present in the code rather than merely asserted:

Finding	Verified at HEAD
Claim loses evidence on GitHub-success/DB-failure/retry	issue-lifecycle.ts:242 resolveBeforeState shared by both writers, flag recoveredFromMirroredState; claim/route.ts:242 and status/route.ts:157 pass mirroredBefore
Sync mirrors before a swallowed event insert	issue-sync.ts:495 before :509 and :586 before :596; recordSyncedIssueEvents has no try/catch (only the *Quietly wrappers swallow, and sync uses neither)
Groomer records noop/partial as transitions	groomer/run.ts:1241 recordStatusTransitionQuietly with before: analyzed.labels / after: applied.labels; the unconditional insert is gone
Groomer close emits no issue_closed	groomer/run.ts:1264-1281 emits its own row keyed hosted-groomer|…|issue_closed|applicationKey
Absent-label unlabel records a phantom change	webhook/route.ts:185 returns currentLabels itself when the label is absent, making the reference gate exact
Real preserveStatus release unkeyed	issue-claim.ts:138 one releaseDedupeKey("claim-released", …) shared by the release (:197) and the repair (:174); stale-work.ts:168 supplies it from the AgentWork row

The AI reviewer (its-saffron[bot]) has since APPROVED on c20a430 with review_result: clean.

One thing I re-checked and want on the record. A first read suggested move and groom still had the claim/status defect class, since neither passes mirroredBefore. They do not. move (move/route.ts:63) and groom (groom/route.ts:107) both derive before from the cache row read before the transaction, and a rollback leaves that row untouched — so a retry re-derives the same transition and records it. The reconciler does use live GitHub labels, but it never writes the cache labels at all, so the next sync independently observes the GitHub-side move. The defect class is closed, not half-closed. No change needed.

Local validation, run by me against the integrated work on this head (cwd /workspace, revision c20a430, node_modules freshly installed from package-lock.json; prisma generate ran via postinstall):

• npm run typecheck (tsc --noEmit) → clean, no output.
• npm run lint (eslint .) → 0 errors, 1 pre-existing warning in src/app/login/page.tsx (untouched by this PR).
• npm test (vitest run) → 177 files passed / 3 skipped; 4387 passed | 16 skipped | 1 todo, 0 failed.

Still pending / owned by others — explicitly not claimed green by me: human review. joryirving's CHANGES_REQUESTED stands until they re-review c20a430; I left all five threads unresolved because resolving another reviewer's threads is their call, not mine. The documented mirrored-state limitation stands as stated (rows marked recoveredFromMirroredState; exact separation would need a durable intent record — a follow-up, not a blocker). I did not run prisma migrate deploy locally (no Postgres in this sandbox); CI covered it instead.

Leaving this open for operator verification. I am not merging.

[itsmiso-ai]

Courier: already addressed (run courier-a6e7e528e2ebe19a8bfc6df0e6f72c40)

PR #1247 was already published on the run branch courier/misospace/dispatch/issue-1202 at head c20a430, which is identical to local HEAD and origin/ (nothing unpushed, nothing published from another branch). The PR is open and non-draft. All review feedback present is already addressed: joryirving's CHANGES_REQUESTED review and the dismissed its-saffron[bot] review are both on the stale head 1953df9, and each of their five threads carries an author reply describing a c20a430 fix. I independently verified in code that each fix is genuinely present, not merely asserted: shared resolveBeforeState with recoveredFromMirroredState in src/lib/issue-lifecycle.ts:242 used by both writers with mirroredBefore passed at claim/route.ts:242 and status/route.ts:157; sync records evidence before the mirror at src/lib/issue-sync.ts:495 before :509 and :586 before :596 with recordSyncedIssueEvents propagating; the groomer's unconditional status_changed is replaced by recordStatusTransitionQuietly at src/lib/groomer/run.ts:1241 and applied.closed emits its own issue_closed keyed by applicationKey at :1264-1281; src/app/api/issues/webhook/route.ts:185 returns currentLabels itself for an absent-label unlabel; src/lib/issue-claim.ts:138 shares one claim-released dedupe key between the real release and the repair with stale-work.ts:168 supplying it from the AgentWork row. its-saffron[bot] has APPROVED on c20a430 with review_result clean. CI on this head is 13/13 green including Database migrations and Database integration, which also resolves the two caveats the PR body left open. I re-checked one flagged concern and found no gap: move/route.ts:63 and groom/route.ts:107 both derive before from the cache row read before the transaction, which a rollback leaves untouched, so a retry re-derives the transition; the reconciler uses live GitHub labels but never writes cache labels, so sync independently recovers. Local validation run by me against the integrated work on c20a430 (cwd /workspace, node_modules freshly installed from package-lock.json): npm run typecheck clean; npm run lint 0 errors with 1 pre-existing warning in the untouched src/app/login/page.tsx; npm test 177 files passed / 3 skipped, 4387 passed | 16 skipped | 1 todo, 0 failed. No code change was required, so nothing was committed or pushed by me. I left a takeover verification comment on the PR with these receipts (#1247 (comment)). Left open and not merged for operator verification; human re-review of c20a430 remains pending and I did not resolve another reviewer's threads.

[joryirving]

Re-reviewed exact head c20a430 against #1202 and the six issues in my previous REQUEST_CHANGES. APPROVED. Claim's recordClaimEvent receives the cached mirroredBefore to repair GitHub-success/DB-failure with no false idempotent suppression; issue sync and closed reconciliation now durably record source-derived upsert evidence before the cache mirror so a failed insert preserves retry eligibility. Hosted grooming uses the transition guard and emits distinct idempotent issue_closed for a confirmed close. Missing-label webhook redelivery no longer fabricates a removal by array reference. Stale claim release and already-released repair share the per-AgentWork stable releaseKey, without suppressing a later distinct release. The schema remains additive and worker-neutral with no read API or merge-backfill scope creep. Exact-head CI, security audit, image build, PR Smoke and AI review all passed; mergeable.