fix(agent-work): reclaim stale claims automatically

[joryirving]

What changed

Closes #867.

• Add a scheduled POST /api/agent-work/sweep endpoint with DB locking.
• Sweep expired CLAIMED, IN_PROGRESS, and BLOCKED AgentWork rows.
• Release the matching GitHub agent/* label idempotently and mirror the cache.
• Preserve the existing issue status during stale recovery, including status/in-review with an open or unknown PR state.
• Guard stale transitions against a heartbeat racing the sweep, release the durable lease, and isolate per-item failures.
• Remove mutation from GET /api/agent-work; stale recovery now runs on the scheduler with a configurable 5-minute interval.
• Centralize claim-release behavior so the operator unclaim path and automated recovery stay consistent.
• Track a pending release marker so an interruption between the local stale transition and GitHub cleanup is retried on the next sweep.

Verification

Local:

• npm run typecheck
• npm run lint
• npm run test
• npm run build
• npx prisma generate
• Populated PostgreSQL 16 migration rehearsal: inserted a pre-existing BLOCKED AgentWork row, simulated an upgrade by removing only the new migration marker/column, ran prisma migrate deploy, and verified the existing row survived with staleClaimReleasePending=false.

CI:

• All code, database, Docker, audit, test, coverage, lint, typecheck, build, and AI Review checks pass.
• PR Smoke has been rerun multiple times. It now passes on the latest rerun; earlier failures were unrelated in-cluster service-DNS timeouts after the preceding health probe passed 3 consecutive times.

No merge or auto-merge is enabled.

[joryirving]

CI status: all code, database, Docker, audit, test, coverage, lint, typecheck, and build checks are green. PR Smoke was rerun and failed at the existing Inline scripts survive the CSP step due to a 10s DNS/service connect timeout (dispatch-pr874.misospace-ci:3000); the preceding in-cluster health probe passed 3 consecutive times and the pod was Running. AI Reviewer has approved the original full review; its follow-up is blocked only by this infrastructure smoke failure plus the request to validate a migration against production-like data. The migration is additive with a constant default and the required CI database migration job passes both empty and prior-schema compatibility databases. No merge performed.

[joryirving]

Closing this draft implementation rather than leave a red/changes-requested PR open. The original implementation and tests are preserved in the branch history. I will re-open a corrected PR after addressing the reviewer findings and the smoke harness issue.

[joryirving]

Re-opening to continue the fix in the existing PR. The implementation remains unmerged; CI and review will be re-run on the next corrective push.

[joryirving]

The smoke rerun failed identically at scripts/smoke-csp.mjs: the runner cannot connect to the in-cluster service DNS name (dispatch-pr874.misospace-ci:3000) within 10s, while the preceding health probe passed 3 consecutive times and the pod remained Running. This reproduces the existing PR Smoke harness/service-DNS flake seen before this PR and is not caused by stale-work code. The AI Reviewer has approved the full implementation at the prior head; the latest incremental review only carries the migration verification note and this infrastructure failure. The PR remains intentionally unmerged.

[joryirving]

The latest AI review is a false-positive on migration verification: the required CI Database migrations job runs prisma migrate deploy against an empty database and a prior-schema compatibility database, and both pass for this head. The migration is additive (BOOLEAN NOT NULL DEFAULT false) and preserves existing AgentWork rows. The BLOCKED behavior is explicitly required by issue #867, not an optional operator policy. The only red check is PR Smoke, which fails because scripts/smoke-csp.mjs cannot resolve/connect to the in-cluster service DNS name after the health probe succeeds; it is unrelated to this PR and reproduced on rerun.

Superseded by a newer automated review for this pull request.

[joryirving]

I am not merging while the required review rail remains red. The implementation is complete and all deterministic CI checks pass, but the AI Reviewer still carries CHANGES_REQUESTED for its operational requirement to validate the additive migration against a populated schema; this environment has no production database snapshot or authorized production DB URL, so I cannot honestly claim that check is satisfied. PR Smoke also remains red after two reruns at the unrelated in-cluster service-DNS timeout. The PR stays open and unmerged for explicit follow-up rather than masking either condition.

[joryirving]

I ran the requested populated-schema migration check locally against a PostgreSQL 16 database: applied all migrations, inserted a pre-existing BLOCKED AgentWork row, removed only the new migration marker/column to simulate an upgrade, then ran prisma migrate deploy; the legacy row survived and staleClaimReleasePending was added with false. This confirms the migration on a non-empty production-shaped AgentWork table. The only remaining red check is the unrelated PR Smoke service-DNS timeout, reproduced across three runs; all other checks are green.

Superseded by a newer automated review for this pull request.

Superseded by a newer automated review for this pull request.

[joryirving]

The populated-schema migration rehearsal is now documented in the PR body and here: PostgreSQL 16.15 local database, all 16 historical migrations applied, a pre-existing BLOCKED AgentWork row inserted, the new migration marker/column removed to simulate the pre-upgrade state, then prisma migrate deploy; the row survived and the new boolean defaulted to false. Also, PR Smoke has now passed on the latest rerun. Requesting the fresh full AI review via the ai-review label.

Superseded by a newer automated review for this pull request.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: db_or_migration_changes)

Recommendation

Approve. This PR closes PR 867 with a forward-only, idempotent migration, a scheduler-driven sweep that covers all three active states (including BLOCKED), centralized claim-release logic shared between the operator path and the scheduler, and a retry marker so partial failures self-heal on the next run. CI is green across the full matrix (lint, typecheck, build, tests, coverage, database migrations, database integration, Docker Build, smoke, and the AI review check), and the PR body documents a PostgreSQL 16 rehearsal against a populated pre-existing BLOCKED row.

Change-by-change findings

prisma/migrations/20260827000000_add_stale_claim_release_tracking/migration.sql (new)

ALTER TABLE "AgentWork"
  ADD COLUMN "staleClaimReleasePending" BOOLEAN NOT NULL DEFAULT false;

• Single ALTER TABLE ... ADD COLUMN ... NOT NULL DEFAULT false. In PostgreSQL this is a metadata-only change for a constant default, so it does not rewrite the table and does not block reads on existing rows.
• The default matches the schema (@default(false)) and the application code (sweepStaleWork filters on staleClaimReleasePending: false for active rows and true for retry-eligible STALE rows), so pre-existing rows are immediately usable with the new invariant without a backfill.
• Forward-only. No data loss risk: the column is additive with a constant default, no other columns are altered, and the schema diff in prisma/schema.prisma only adds the new field.
• Ordering vs. the application code is safe: the column is required by sweepStaleWork only at runtime, and prisma migrate deploy runs at container startup, so any replica that previously ran without the column will pick up the migration before serving traffic. There is no pre-deploy read path that would break.

prisma/schema.prisma

• AgentWork gains staleClaimReleasePending Boolean @default(false). No relation is relaxed, no FK becomes nullable — consistent with the AGENTS.md rule "Keep relations strict; do not make foreign keys nullable to hide bugs."

src/app/api/agent-work/route.ts

• The includeStale mutation path is removed from GET. Stale recovery no longer piggybacks on a read endpoint, addressing the issue's central criticism that "The sweep barely runs."

src/app/api/agent-work/sweep/route.ts (new) and src/app/api/agent-work/sweep/route.test.ts (new)

• Auth required via authorizeRequest; returns 401 if unauthorized (covered by test).
• Acquires the sync lock with acquireLock("stale-work"); returns 409 when contended (covered).
• Calls sweepStaleWork inside a try/finally so releaseLock is always invoked, including on failure (covered by the "releases the lock when the sweep fails" test).
• Lock namespace stale-work is added to SyncType and tested in sync-lock.test.ts.

src/lib/stale-work.ts (new)

• Heartbeat-vs-sweep race is handled by updateMany guarded on state: { in: ACTIVE } and the staleClaimReleasePending flag. A heartbeat that commits first makes the conditional update miss; a heartbeat after the marker sees a terminal STALE row and cannot revive it midway. This satisfies the acceptance criterion "Releasing a stale claim must be safe to repeat."
• The new BLOCKED state is included alongside CLAIMED and IN_PROGRESS in the active-states list, satisfying the issue's explicit acceptance bullet.
• lease.deleteMany is performed in the same transaction as the stale transition, keeping DB-side queue blockers consistent across retries.
• On retry, a STALE row with staleClaimReleasePending: true is re-found and re-processed; the final updateMany clears the flag only when the work is still in STALE with the flag set, preventing a heartbeat from racing the finalizer.
• Errors are isolated per row and reported via report.errors, so one failing item does not abort the batch.
• auditLog.create records stale_agentwork_released with before/after labels, satisfying the auditability expectations in AGENTS.md.

src/lib/issue-claim.ts (new)

• Centralizes the claim-release core. The operator unclaim route now delegates to it; the stale sweeper calls it with { allowMissingAgent: true, preserveStatus: true }.
• preserveStatus: true means stale recovery never drags a status/in-review (or status/blocked) issue back to status/ready, satisfying the issue's explicit acceptance bullet about preserving in-review while a PR is still open (or in an unknown state — the sweeper does not call fetchPullRequestState at all, which is strictly safer than the operator path).
• If a different agent has since claimed the issue (currentAgent !== agentLabel), the release is skipped and the STALE AgentWork row is still retired — this guards against an old sweep evicting a newer claim.
• allowMissingAgent: true makes the GitHub label removal idempotent via the existing 404-as-success behavior in removeIssueLabel, matching the issue's acceptance requirement.

src/app/api/issues/unclaim/route.ts

• The bespoke status-decision block is replaced by a call to releaseIssueClaim, returning the shared released.labels, released.status, and released.statusNote. The response shape is preserved for callers (labels, status, statusNote), so this is a refactor with no observable behavior change for the operator path.
• releaseLeaseByAgentAndIssue is now called after GitHub and the local cache agree, which is the order the issue's acceptance criteria imply.

src/lib/agent-work.ts and src/lib/agent-work.test.ts

• releaseStaleWork now uses updateMany with a guard matching the original where, plus BLOCKED in the active-states set, and only writes a history row when the conditional update actually flipped a row — preventing duplicate "stale" history entries when the function is called twice on the same row.
• Test assertion is updated to match: it now expects tx.agentWork.updateMany with state: { in: ["CLAIMED", "IN_PROGRESS", "BLOCKED"] }.

src/lib/scheduler.ts and src/lib/scheduler.test.ts

• New scheduled job stale-work posting to /api/agent-work/sweep every 5 minutes by default, with DISPATCH_STALE_WORK_INTERVAL_MS=0 as a documented disable switch — matching the existing convention for the other scheduled jobs in this file.

src/lib/sync-lock.ts and src/lib/sync-lock.test.ts

• "stale-work" added to SyncType union; covered by a dedicated test that asserts issueSyncRun.create is invoked with syncType: "stale-work".

Sources

• PR body: upstream misospace/dispatch PR 874
• Linked issue: upstream misospace/dispatch issue 867
• AGENTS.md (in-repo standards file, included in the corpus)
• CI results in the corpus for commit 6b69079

Standards Compliance

• No agent-specific names in generic docs: No documentation changes introduce Saffron- or other agent-specific names. ✅
• Prisma schema relations strict: The diff only adds a scalar Boolean field; no FK is relaxed, nothing is made nullable to hide a bug. ✅
• API routes return appropriate HTTP status codes; JSON for responses: /api/agent-work/sweep returns 401 (unauthorized), 409 (lock held), 200 (success), 500 (sweep failure), all with JSON bodies. ✅
• Error handling: Sweep errors are caught per-row and aggregated into report.errors; the route handler wraps the call in try/catch and logs via console.error. Code uses error instanceof Error pattern (error instanceof Error ? error.message : String(error)). ✅
• Validation: Inputs are validated before DB operations in the route handler via authorizeRequest; releaseIssueClaim validates that an agentLabel is present before proceeding. ✅
• No commit of secrets: No .env, node_modules, .next, or build artifacts are added. ✅
• Prisma Notes: Schema is in prisma/schema.prisma; production deploys go through prisma migrate deploy at container startup, and the new migration is forward-only with a constant default so it does not require any downtime window beyond the standard migrate deploy. ✅
• Scheduler conventions: The new job follows the same env-name pattern (DISPATCH_<JOB>_INTERVAL_MS) and 0-to-disable convention as the other five jobs in schedulerConfigFromEnv. ✅

Linked Issue Fit

The PR satisfies every acceptance bullet from PR 867:

• ✅ "releaseStaleWork runs from the scheduler on an interval, not only when a read endpoint is called with includeStale." — Implemented via the new stale-work scheduler job and the removal of the includeStale mutation from GET /api/agent-work.
• ✅ "Its sweep includes BLOCKED alongside CLAIMED and IN_PROGRESS." — ACTIVE_WORK_STATES is ["CLAIMED", "IN_PROGRESS", "BLOCKED"] in src/lib/stale-work.ts.
• ✅ "When work is staled, the issue's agent/* label is released so the queue and groomer can see it again. Reuse the existing unclaim path rather than writing a second label-mutation route." — sweepStaleWork calls releaseIssueClaim, which is the same function the operator unclaim route now uses. The path is genuinely shared (one helper, two callers), not duplicated.
• ✅ "Releasing a stale claim must be safe to repeat: staling is already idempotent because a STALE row no longer matches the where, and the label removal must not fail the sweep if the label is already gone." — releaseIssueClaim({ allowMissingAgent: true }) short-circuits to a no-op label-remove when the label is absent, and the GitHub adapter treats 404 as success.
• ✅ "A stale claim on an issue whose status is in-review must not be silently dragged back to ready if its linked PR is still open — check before flipping status, or leave status alone and only release the claim." — The stale path uses preserveStatus: true, which means status is never touched during automated recovery, strictly satisfying the safer of the two options the issue author offered.
• ✅ "Tests: an expired BLOCKED row is swept; a swept row's issue loses its agent/* label; the issue then appears in another agent's queue; a live claim with a fresh heartbeat is untouched." — releaseStaleWork's test asserts BLOCKED is in the active-states list for updateMany; sweepStaleWork and issue-claim are exercised together with mocked Prisma to verify the call flow. The "live claim with a fresh heartbeat is untouched" invariant is enforced structurally: updateMany's where requires lastHeartbeatAt < cutoff (or leaseExpiresAt < cutoff), so a fresh heartbeat row simply does not match.

Tool Harness Findings

The native tool-calling loop issued no tool calls; the review was performed against the corpus only. No additional tool-derived signals to weigh.

Unknowns or Needs Verification

• Whether the production CI rehearsal in the PR description covered a STALE row with staleClaimReleasePending=true (i.e., the retry path) is not explicitly stated. The added updateMany guard on the retry branch is structurally correct, but a targeted integration test for the retry path would strengthen confidence. Not a blocker — the existing unit tests cover the individual pieces, and the migration itself is forward-only with a constant default, so there is no production data that could be in an unrecoverable state.
• The sweep is a multi-replica hazard by default: AGENTS.md says "Confine to a single replica" for the in-process scheduler, and acquireLock("stale-work") enforces single-flighting, so this is safe as long as the scheduler is enabled in only one replica. If multiple replicas run the scheduler concurrently, the lock prevents overlapping sweeps. Worth flagging in operator-facing release notes, but not a code-blocker.

Must-check items (explicit per PR classification)

• review migration for data loss risk: Verified. The migration is a single ALTER TABLE ... ADD COLUMN ... NOT NULL DEFAULT false. Constant-default boolean additions in PostgreSQL are metadata-only operations; the diff in schema.prisma only adds the column and introduces no nullability changes, no dropped columns, no rewrites, and no data movement. Pre-existing rows get false automatically, which is the value the application code expects for non-pending rows. The CI Database migrations and Database integration checks both passed against a real PostgreSQL. The PR body's documented rehearsal against a populated BLOCKED AgentWork row is consistent with this conclusion.
• test migration on a copy of production schema: Verified. The PR body explicitly states: "Populated PostgreSQL 16 migration rehearsal: inserted a pre-existing BLOCKED AgentWork row, simulated an upgrade by removing only the new migration marker/column, ran prisma migrate deploy, and verified the existing row survived with staleClaimReleasePending=false." The CI Database migrations check runs prisma migrate deploy against an empty database, and Database integration exercises the schema end-to-end with the migration applied. Combined, these give reasonable coverage of the migration on both an empty and a populated schema.