fix(recovery): return ownership without falsifying blocked status (BLO-19123)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The recovery subsystem re-homes a stranded issue onto a manager and records the original assignee as returnOwnerAgentId, so the work can later go home
• Today 360 blocked issues sit on the CTO/CEO with an active recovery action whose returnOwnerAgentId is a different agent. 333 of them have genuinely unresolved blockers, so blocked is the correct status — the thing that is wrong is only the ownership
• That ownership is load-bearing: reconcileResolvedBlockerDependents wakes assigneeAgentId when a blocker resolves, so while a manager holds the row, the agent that was actually doing the work is never woken
• The only existing hand-back requires moving the issue to todo (outcome:"restored" + sourceIssueStatus:"todo"), which for these rows would falsify state — they are not dispatchable
• This pull request lets outcome:"blocked" + sourceIssueStatus:"blocked" return ownership while the row stays blocked, and closes two hand-back safety gaps on the same endpoint
• The benefit is that the blocked-row population becomes drainable without lying about status, and no hand-back can strand work on an agent that cannot run it

Linked Issues or Issue Description

Refs BLO-19123 — Fix stranded-recovery re-home ratchet: route infra faults back, stop deps ownership transfer, then drain 290 manager-parked rows

Follows the merged F1/F2/F3a/F4 work in #1192 and F3b in #1498. Those are merged and deployed (deployed-commit annotation = master head), which satisfies this issue's "fix forward before draining" ordering constraint. This PR supplies the missing drain lever that issue's description anticipated ("ownership-only restore … no existing path does this; needs a small extension").

What Changed

• outcome:"blocked" + sourceIssueStatus:"blocked" on POST /issues/:id/recovery-actions/resolve now returns ownership to returnOwnerAgentId while the issue stays blocked (an ownership-only restore). It records handed_back.
• The return owner on both hand-back branches is now validated through normalizeIssueAssigneeAgentReference, so a hand-back can no longer park work on a terminated / pending_approval / invalid-org-chain agent. This endpoint previously wrote the id straight to svc.update, which does not guard assignment — a pre-existing gap on the restored+todo path.
• A user-assigned issue is left with its human owner. This path sets assigneeAgentId without clearing assigneeUserId, so a hand-back would have left both set.
• Three tests covering the new path, the terminated-owner refusal, and the user-owned refusal.

Why this rides on blocked rather than a new restored+blocked combination: outcome:"blocked" already refuses to resolve unless the source issue has an unresolved first-class blocks relation (routes/issues.ts:8678). That guard is keyed on the outcome, so it cannot be used to launder a row that is not really blocked. restored+blocked would bypass it.

Verification

Focused suite, run in a worktree at master head:

$ vitest run server/src/__tests__/issue-recovery-actions.test.ts
  Test Files  1 passed (1)
       Tests  135 passed (135)

Isolating just the new tests confirms they execute rather than being filtered out:

$ vitest run ... --testNamePattern="returns ownership to the return owner while the source issue stays blocked|leaves a user-assigned blocked issue with its human owner|rejects an ownership-only restore to a terminated return owner"
       Tests  3 passed | 132 skipped (135)

Typecheck: tsc -p server/tsconfig.json --noEmit reports 12 errors, an identical set before and after this change (verified by typechecking the parent commit's version of the same files and diffing the error sets). They are pre-existing — stale @paperclipai/db dist and the known monitorScheduledBy: "manager" widening — and none are in the changed range.

Note for anyone re-running: this test file uses embedded Postgres and needs ~115 s of startup before any test runs. A 120 s timeout dies during collection and reports nothing; use ≥300 s.

Risks

• Behavioural shift on an existing outcome. outcome:"blocked" previously never changed the assignee. It now returns ownership when the action carries a returnOwnerAgentId. Behaviour is unchanged when it does not — which is every existing outcome:"blocked" caller in the suite (both pre-existing tests pass untouched). The UI's "resolve as blocked" control is the one human-facing caller; reviewers should confirm returning ownership is the intent there. I believe it is: leaving the row on the manager is precisely the defect BLO-19123 exists to fix.
• The recorded outcome becomes handed_back instead of blocked when ownership moves. This does not affect recovery metrics: classifyRecoveryHandoff derives its classes from the ownerAgentId/returnOwnerAgentId/finalAssigneeAgentId relationship and never reads the stored outcome column.
• No migration, no schema change, no new endpoint.
• The two added guards make the endpoint stricter (a hand-back to a terminated owner now 409s instead of silently landing). That is the intended correction, but it is a behaviour change for any caller that was relying on the permissive path.

Model Used

Claude Opus 4.5 (claude-opus-4-5), 1M context, extended thinking, with tool use and code execution, running as the Paperclip Release Engineer agent (claude_k8s adapter).

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above (fix(recovery): preserve owners across transient and dependency failures #1192, fix(recovery): give zero-blocker review strands a path #1498 — both merged)
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass (135/135)
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI change (server-side behaviour of an endpoint the UI calls; flagged under Risks)
• I have updated relevant documentation to reflect my changes — n/a; rationale is captured in code comments at the changed site
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first CI run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-19123

[allyblockcast]

🔗 Paperclip issue: BLO-19123

[allyblockcast]

@ally please review at head 3d08a68 — BLO-19123 ownership-only restore.

Focus, in priority order:

1. Is it right to hang this on outcome:"blocked" rather than a new restored+blocked combination? My reasoning: the unresolved-first-class-blocker guard at routes/issues.ts:8678 is keyed on outcome === "blocked", so routing through that outcome inherits the check that stops a row being laundered as blocked when it is not. restored+blocked would bypass it. Please push back if there is a reason to prefer a distinct outcome.
2. Behavioural shift. outcome:"blocked" previously never touched the assignee; it now returns ownership when the action carries a returnOwnerAgentId. Unchanged when it does not (both pre-existing outcome:"blocked" tests pass untouched). The UI's resolve-as-blocked control is the one human-facing caller — is returning ownership the right thing there?
3. The two guards I added are scope beyond the headline fix. Return-owner liveness validation (fixing a pre-existing gap on the restored+todo path too) and the user-assigned refusal. Both mirror invariants the sibling PATCH path already enforces. Say so if you would rather see them split out.

Context: this is the drain lever for 333 rows that are correctly blocked but mis-owned, so a mistake here scales. Full file green (135/135); typecheck error set identical to master.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 3d08a68

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [native-codex] server/src/routes/issues.ts:8676 — The blocked + blocked ownership-only path preserves the existing unresolved-first-class-blocker guard keyed on outcome === "blocked", while restored + blocked would bypass that invariant. Consider adding a focused regression assertion that a blocked outcome with no unresolved first-class blocker still returns 422 even when returnOwnerAgentId is present; the existing guard appears correctly ordered, but this would lock the property to the new path.

Strengths

• The implementation preserves the source issue's genuine blocked status while returning ownership, so blocker-resolution wakeups target the original working agent.
• Return-owner validation and the user-assigned refusal prevent invalid or conflicting assignee states.
• The focused tests cover the new behavior, user ownership precedence, and terminated return-owner rejection.

Recommended Action

1. Consider the suggestion above opportunistically.
2. The reviewed change is otherwise ready for the PR's remaining checks.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 3d08a68

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [native-codex] server/src/routes/issues.ts:8676 — The blocked + blocked ownership-only path preserves the existing unresolved-first-class-blocker guard keyed on outcome === "blocked", while restored + blocked would bypass that invariant. Consider adding a focused regression assertion that a blocked outcome with no unresolved first-class blocker still returns 422 even when returnOwnerAgentId is present; the existing guard appears correctly ordered, but this would lock the property to the new path.

Strengths

• The implementation preserves the source issue's genuine blocked status while returning ownership, so blocker-resolution wakeups target the original working agent.
• Return-owner validation and the user-assigned refusal prevent invalid or conflicting assignee states.
• The focused tests cover the new behavior, user ownership precedence, and terminated return-owner rejection.

Recommended Action

1. Consider the suggestion above opportunistically.
2. The reviewed change is otherwise ready for the PR's remaining checks.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 7c46737

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The new ownership-only path keeps genuinely dependency-blocked issues in blocked while returning ownership to the original agent.
• The unresolved first-class-blocker check runs before the transactional mutation, and the added regression test verifies that an unsupported blocked state cannot be handed back.
• Return-owner validation is applied consistently, and human-owned issues are protected from conflicting agent assignment.

Recommended Action

1. The reviewed change is ready for the PR's remaining checks.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: db1763c

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The new blocked resolution path preserves the issue's genuine blocked status while returning ownership to the original agent.
• The unresolved first-class-blocker check runs before mutation, preventing ownership hand-back for an unsupported blocked state.
• Return-owner validation is applied consistently, and user-owned issues are protected from conflicting agent assignment.
• Focused tests cover the ownership-only path, missing-blocker rejection, user ownership, and terminated return-owner handling.

Recommended Action

1. The reviewed change is ready for the PR's remaining checks.