Skip to content

fix(recovery): stop escalating dependency-blocked issues as stranded (BLO-19124) - #1062

Open
allyblockcast[bot] wants to merge 1 commit into
masterfrom
cto/blo-19124-suppress-dependency-blocked-strands
Open

fix(recovery): stop escalating dependency-blocked issues as stranded (BLO-19124)#1062
allyblockcast[bot] wants to merge 1 commit into
masterfrom
cto/blo-19124-suppress-dependency-blocked-strands

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Aug 5, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The recovery subsystem sweeps assigned issues whose live execution disappeared and escalates them as stranded_assigned_issue — moving them to blocked and handing them to a recovery owner
  • issue_dependencies_blocked sits in NON_RETRYABLE_CONTINUATION_ERROR_CODES alongside genuine failures like agent_not_invokable and budget_exhausted, so the sweep escalates it unconditionally
  • But that code is not a failure — it is the dispatcher declining to run an issue whose blockers are still open, i.e. a DAG node correctly waiting its turn
  • Measured on one inbox: 158 of 161 active recovery actions were this code, and all 158 had a genuinely unresolved blocker — so 98% of the recovery queue was false positives
  • Escalating them is worse than a no-op: escalateStrandedAssignedIssue reassigns the issue to a recovery owner, so the dependency wake later fires at an agent that no longer owns the work
  • This pull request adds a guard that re-evaluates dependency readiness at sweep time and skips the escalation while the dispatcher's own gate would still refuse
  • The benefit is that correctly-sequenced work stops being parked in blocked under a recovery owner, and the recovery queue is left holding only real strands

Linked Issues or Issue Description

Refs: BLO-19124 — https://paperclip.blockcast.net/BLO/issues/BLO-19124

Problem or motivation

issue_dependencies_blocked is emitted by the dispatcher's dependency gate
(heartbeat.tscancelQueuedRunForBlockedDependencies) when
listDependencyReadiness reports an issue is not dependency-ready. Its own
cancellation reason promises "Paperclip will wake the assignee when blockers
resolve"
— that wake arrives via listWakeableBlockedDependents once the blocker
closes. It is a wait, not a lost execution path.

Because the code is a member of NON_RETRYABLE_CONTINUATION_ERROR_CODES, the
stranded-issue sweep treated it as a non-retryable failure and escalated every
occurrence: blocked status, a recovery action, and reassignment to a recovery
owner. On a single inbox that produced 158 of 161 active recovery actions, every
one of which had a genuinely unresolved blocker and nothing an owner could act on.
The oldest such action had been active for seven weeks.

Proposed solution

Re-evaluate readiness at sweep time and skip the escalation when the dispatcher's
gate would still refuse for the same reason. Keep escalating when the issue is
dependency-ready, because "dependency-blocked with nothing blocking it" is a real
defect that must stay visible.

Alternatives considered

Removing issue_dependencies_blocked from NON_RETRYABLE_CONTINUATION_ERROR_CODES
entirely was rejected: that Set also governs "do not burn retry attempts", which is
correct behaviour for a dependency wait. Suppressing only the escalation is the
narrower change and leaves the retry semantics untouched.

Trusting the failing run's recorded evidence instead of re-querying was also
rejected — the evidence can be minutes to days stale, and a blocker that has since
closed must not be silently skipped.

What Changed

  • server/src/services/recovery/service.ts: added a guard ahead of the
    non_retryable escalation branch that calls issuesSvc.listDependencyReadiness
    for the issue and continues without escalating when isDependencyReady is
    false. Uses the same readiness function as the dispatcher gate that produced the
    error code, so the two cannot disagree.
  • Extracted the issue_dependencies_blocked literal into
    DEPENDENCY_BLOCKED_ERROR_CODE and referenced it from
    NON_RETRYABLE_CONTINUATION_ERROR_CODES, so the Set membership and the new guard
    cannot drift apart.
  • Added a dependencyWaitSkipped counter to the sweep result so suppressed waits
    are observable rather than being folded anonymously into skipped.
  • server/src/__tests__/heartbeat-process-recovery.test.ts: two paired regression
    tests (see Verification).

Verification

CI is the signal for this PR. pnpm install did not converge in this workspace
(>20 min, node_modules never populated), so I could not complete a local vitest
run and have left that checklist box unchecked rather than claim a green I do not
have. The relevant job is the server test suite over
server/src/__tests__/heartbeat-process-recovery.test.ts.

The two new tests are deliberately disjoint, so neither a blanket suppression nor
the current blanket escalation can pass both:

  1. "does not escalate a dependency-blocked continuation while a blocker is still
    open"
    — asserts dependencyWaitSkipped === 1, escalated === 0, the issue
    stays in_progress with its original assigneeAgentId, and no recovery issue
    or comment is created.
  2. "still escalates a dependency-blocked continuation when nothing is actually
    blocking it"
    — same error code, blocker done, asserts
    dependencyWaitSkipped === 0 and escalated === 1.

Reverting the guard should fail (1); widening it to suppress on the error code
alone should fail (2). Reviewers: please confirm both fail in those two directions
rather than taking the pairing on trust.

Field evidence for the premise (CEO inbox, 2026-08-05): 166 blocked issues,
143 active recovery actions, all stranded_assigned_issue. 128 of 143 (89.5%)
carry latestRunErrorCode: issue_dependencies_blocked
, and every one of those 128
reports unresolvedBlockerCount >= 1. Two sampled at random and verified against
the authoritative blockedBy hydration:

  • BLO-9658 → blocked by BLO-9663, status todo, assigned to a human — while
    its recovery action instructs an agent owner to "fix the runtime/adapter
    failure"
    . Its timeoutAt elapsed 3 days ago and the row is still active.
  • BLO-4296 → blocked by BLO-8329, status blocked.

Both are real explicit blocks edges, so both are suppressed by this guard.

Risks

Low, and deliberately fail-closed. The guard only ever suppresses an escalation,
never creates one, and only for a single error code. If the readiness lookup returns
no entry for the issue the guard does not fire and the pre-existing escalation path
runs unchanged.

The one behavioural shift worth naming: an issue that is genuinely stuck behind a
blocker that is itself dead will no longer be surfaced by this sweep. That case was
never served by this path anyway — the escalation reassigned it to an owner who
could not act — and it needs a blocker-liveness check on the root rather than
retry-exhaustion on the child. Tracked as follow-up scope on BLO-19124.

Model Used

Claude Opus 5 (claude-opus-5), 1M context, extended thinking, with tool use and
code execution via Claude Code.

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
  • 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 — not claimed: pnpm install did not converge in this workspace, so CI is the first real run of these tests
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots — n/a, server-only
  • I have updated relevant documentation to reflect my changes
  • 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 first review
  • I will address all Greptile and reviewer comments before requesting merge

…(BLO-19124)

`issue_dependencies_blocked` is the dispatcher declining to run an issue whose
blockers are still open — a DAG node correctly waiting its turn, not a lost
execution path. Its own cancellation reason promises "Paperclip will wake the
assignee when blockers resolve", and that wake arrives via
listWakeableBlockedDependents once the blocker closes.

Because the code is a member of NON_RETRYABLE_CONTINUATION_ERROR_CODES, the
stranded-issue sweep treated it as a non-retryable failure and escalated every
occurrence to `blocked` with a recovery action and a recovery owner. Measured on
one inbox: 158 of 161 active recovery actions were this code, all 158 with a
genuinely unresolved blocker. Escalating is worse than a no-op —
escalateStrandedAssignedIssue reassigns the issue, so the dependency wake then
fires at an agent that no longer owns the work.

Re-evaluate readiness at sweep time via the same listDependencyReadiness the
dispatcher gate used, and skip the escalation while that gate would still refuse.
Keep escalating when the issue IS dependency-ready, because "dependency-blocked
with nothing blocking it" is a real defect and is exactly the blocked-with-zero-
blockers state this ticket forbids.

The Set membership stays: it also governs "do not burn retry attempts", which is
correct for a wait. Only the escalation is suppressed.

Refs: BLO-19124

Co-Authored-By: Claude <noreply@anthropic.com>
@allyblockcast

allyblockcast Bot commented Aug 5, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-8329
🔗 Paperclip issue: BLO-4296
🔗 Paperclip issue: BLO-9658
🔗 Paperclip issue: BLO-19124
🔗 Paperclip issue: BLO-9663

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 5, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-8329
🔗 Paperclip issue: BLO-4296
🔗 Paperclip issue: BLO-9658
🔗 Paperclip issue: BLO-19124
🔗 Paperclip issue: BLO-9663

@allyblockcast

allyblockcast Bot commented Aug 5, 2026

Copy link
Copy Markdown
Author

Ally — Consolidated PR Review

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

Important Issues (1)

  • [gstack/review + native-codex] server/src/services/recovery/service.ts:6257 — A dependency-ready result is not sufficient evidence that the prior cancellation is defective. When the last blocker transitions to done, the issue update commits before listWakeableBlockedDependents queues the normal issue_blockers_resolved wake. A concurrent stranded sweep can therefore observe isDependencyReady === true before hasActiveExecutionPath can see that wake, fall through here, and escalate/reassign the issue. The same happens after a genuinely lost wake, before the existing resolved-blocker reconciliation gets a chance to repair it. This recreates the ownership damage this PR is intended to stop precisely when dependencies resolve.
    • Preserve a grace period based on the latest blocker completion (matching the lost-wake reconciliation window), or atomically confirm/queue the dependency-resolved wake before escalating. Add a regression test for a newly completed blocker with no wake visible yet; the issue must remain with its original assignee rather than escalate.

Suggestions (1)

  • [pr-review-toolkit] server/src/__tests__/heartbeat-process-recovery.test.ts:4555 — The “No recovery action” assertion checks recovery issues and comments but not issueRecoveryActions. Query that table as nearby recovery tests do so the test directly proves the stated invariant.

Strengths

  • The guard uses current dependency state rather than stale run evidence.
  • The paired tests cover unresolved and already-resolved blocker states, and the dedicated counter makes skips observable.
  • The change keeps tenant scoping and parameterized readiness queries intact.

Recommended Action

  1. Close the blocker-resolution race before merge.
  2. Add the direct recovery-action assertion while updating the regression coverage.

This PR is authored by app/allyblockcast, so the App cannot review its own PR. This exact head must be reopened under an independent author before an allyblockcast[bot] App approval is possible.

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.

0 participants