Skip to content

fix(recovery): skip stranded recovery while the company is paused - #13443

Open
MrBlackTongue wants to merge 6 commits into
paperclipai:masterfrom
MrBlackTongue:fix/recovery-skip-paused-company
Open

MrBlackTongue wants to merge 6 commits into
paperclipai:masterfrom
MrBlackTongue:fix/recovery-skip-paused-company

Conversation

@MrBlackTongue

@MrBlackTongue MrBlackTongue commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The recovery service watches the board for stranded assigned issues and repairs them
  • An operator can pause a company; while the pause holds, no run can start for any agent in it
  • budgets.getInvocationBlock reports a paused company as an invocation block, and reconcileStrandedAssignedIssues reads every block as "over budget"
  • The sweep then moves each assigned issue to blocked with a recovery action, although nothing is stranded and nobody abandoned the work
  • The damage outlives the pause, because a resumed company does not reopen a blocked issue: blocked has no dispatch queue, no monitor and no blocker of its own
  • This pull request skips an issue whose company is paused instead of escalating it
  • The benefit is that a pause stays a reversible operator control, and the board does not get a wall of false blocked cards after each pause

Linked Issues or Issue Description

No public issue exists. The problem is described here with the bug report fields.

What happened?

While a company is paused (companies.status = 'paused'), reconcileStrandedAssignedIssues moves assigned issues to blocked. Each issue gets a recovery action and the comment "Paperclip cannot safely continue automatic recovery because the original recovery target is over budget." The cause is budgets.getInvocationBlock, which returns a block for a paused company. The recovery sweep reads every block as budget exhaustion.

When the operator resumes the company, the issues stay blocked. A blocked issue has no dispatch queue entry, no monitor and no blocker, so it never wakes again without manual repair.

Expected behavior

A paused company is a deliberate stop of all work. The sweep must leave its assigned issues in their current status and count them as skipped. Recovery must start again after the resume.

Steps to reproduce

  1. Create a company with one agent and one issue in todo that is assigned to the agent.
  2. Let the last run of that issue fail, so the issue has no live run.
  3. Set the company to paused.
  4. Run the stranded-issue recovery sweep.
  5. The issue moves to blocked with the "over budget" comment. Expected: the issue stays in todo.

Paperclip version or commit

Reproduced on master at 0e9b24c.

Deployment mode

Local single-host install.

Agent adapter(s) involved

Not adapter-specific (core bug). The sweep never reaches an adapter, because the pause blocks every invocation.

Database mode

PostgreSQL.

Observed scale

On one paused company, 28 issues moved to blocked in a single sweep at the start of the pause, and 36 over the whole pause window. Four of them were watchdog issues that had been created six seconds earlier.

What Changed

  • getInvocationBlock now names the cause of the block it returns: company_paused for a paused company, budget_exhausted for every hard-stop. A caller no longer has to infer the cause from a second read of the company row, which can observe a later resume.
  • reconcileStrandedAssignedIssues now skips an issue whose company is paused. The budget branch reads cause from the block it already holds, so the decision and the observation come from one read. The top of the per-issue pass also reads the live company row, which stops the rest of the sweep from working on a paused company.
  • Added a regression test that seeds a stranded assigned issue, pauses its company, runs the sweep and asserts the issue stays todo with no recovery action and no comment.
  • Added a second regression test for the mid-sweep window: the company is paused from inside getInvocationBlock, so the pause starts after the candidate query. The issue must still stay in_progress.

Verification

Перебазировано на master 00ad9503fe3f5e4589143f79bba489ec91f976bb, текущий коммит 3da76f7c6246e706cfb5ac6faa740df77504b56a:

  • Полная сборка pnpm -r build и проверка типов pnpm -r typecheck прошли.
  • В трёх файлах тестов (budgets-service, heartbeat-process-recovery, deferred-execution-receipt-work): 479 успешных тестов и 7 падений сценариев Git/рабочих каталогов.
  • Все 7 падений повторились с budgets.ts и recovery/service.ts из текущего master без исправления PR. Тесты и проверки не ослаблялись.
  • Все три регрессионных сценария паузы компании прошли отдельно (3/3): компания уже на паузе, пауза во время обхода, возобновление сразу после чтения причины блокировки.
  • Сохранены новая обработка временного ожидания расчёта расходов и ветки admittedRecovery из master; причина company_paused проверяется до определения владельца бюджетной блокировки.
  • GitHub подтверждает отсутствие конфликта (MERGEABLE). CI текущего коммита завершён: 57 успешных проверок, 3 штатно пропущены, падений и ожидающих проверок нет. Greptile: успешно, 5/5 на текущем HEAD; открытых тредов нет.
  • По замечанию Greptile удалены только добавленные комментарии к коду; esbuild подтвердил идентичность исполняемого JavaScript до и после удаления. Три регрессии паузы повторно прошли перед отправкой. Исправленный тред закрыт.
  • Миграций нет.

Локальная сборка runner выполнена с CARGO_TARGET_DIR, явно указывающим на runner/target данной копии: упаковщик вендора ожидает бинарник в этом каталоге.

Risks

Low risk. The change only adds a skip path. It narrows automatic recovery while a company is paused, which is the intended state of a paused company: no run can start there in any case. The extra cost is one indexed primary-key read per candidate issue, plus one more on the budget branch only.

Model Used

Перебазирование и проверка совместимости: GPT-6.1 Sol через Codex. Историческая атрибуция исходной реализации ниже.

Claude Opus 5 (claude-opus-5), extended thinking, with tool use and code execution through the Claude Code CLI.

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 not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change (e.g. docs/..., fix/...) and contains no internal Paperclip ticket id or instance-derived details
  • I have run tests locally and they pass (7 baseline failures documented above)
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

Повтор CI: пустой коммит 3da76f7c6246e706cfb5ac6faa740df77504b56a, дерево идентично 55b16bc97. Предыдущий прогон упал в несвязанном cloud-auth.spec.ts при чтении освобождённого ответа. Повторный CI полностью зелёный, включая cloud-auth; Greptile — 5/5.

@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

I cannot add reviewers from a fork. @cryppadotta @nickyleach @im0xMagnus — you have recent commits in server/src/services/recovery/service.ts. Please review when you have time, or point me to the right owner.

The change is one skip path in reconcileStrandedAssignedIssues: a paused company stops all runs, so its assigned issues are not stranded and must not be escalated to blocked. The regression test fails on master and passes with the fix.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with no new actionable issues found.

Summary

Recovery now leaves assigned issues unchanged while their company is paused.

  • Paused companies keep stranded assigned issues in their current state.

Reviews (17) · Last reviewed commit: "ci: retry unrelated cloud auth e2e failu..." · Reviewed by Greptile

Comment thread server/src/services/recovery/service.ts Resolved
@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

Fixed the Greptile P1 in 3018109.

Pause snapshot can go stale — valid. The snapshot was taken once before the per-issue pass, so a pause that started during the sweep was invisible to the skip while getInvocationBlock already reported the company block. That issue still took the permanent escalation.

The snapshot is gone. isCompanyPaused reads the live companies row, and it runs twice: at the top of the per-issue pass, and again immediately before the budget branch writes blocked. The second read is the decisive one, so there is no window between a stale read and the destructive write.

New regression test: skips a stranded assigned issue when its company is paused during the sweep. It pauses the company from inside getInvocationBlock, which is exactly the mid-sweep moment described in the review. Both tests pass with the fix. Without the second read the new test fails with expected 1 to be +0 on result.escalated, so it guards the reported race and not only the fix.

Cost: one indexed primary-key read per candidate issue, plus one more on the budget branch only.

Comment thread server/src/services/recovery/service.ts Resolved
@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

Fixed the second Greptile P1 in 4aa9b3c, and it was a fair call: inferring the cause of the block from a later company read let a resume between the two reads turn a pause into a false budget verdict.

The block now names its own cause. getInvocationBlock returns cause: "company_paused" for a paused company and cause: "budget_exhausted" for every hard-stop. Stranded recovery reads that field from the block it already holds, so the decision and the observation come from one read and nothing can slip between them. The second company read on the escalation path is gone.

Tests: budgets-service.test.ts (7 passed, expectations updated for the new field) and the two pause cases in heartbeat-process-recovery.test.ts (2 passed).

One more note on the previous run. ci / General tests (server (3/5)) failed in src/__tests__/workspace-runtime.test.ts → "persists provisioning before starting and excludes provision time from readiness timeout", with TypeError: .toMatch() expects to receive a string, but got object on startingRow.providerRef. That file is untouched by this PR, and the same shard passed on the first commit of this branch, so it looks like a flake rather than a regression from here. ci / verify failed only because it aggregates that shard. This push re-runs both.

@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

Status after the review round: Greptile is green on 4aa9b3c, and 30 of 32 checks pass.

The two red ones are an unrelated flaky shard and the gate that aggregates it:

  • run 1 — ci / General tests (server (3/5)): workspace-runtime.test.ts → "persists provisioning before starting and excludes provision time from readiness timeout", TypeError: .toMatch() expects to receive a string, but got object.
  • run 2 — ci / General tests (server (1/5)): chat-channels.integration.test.ts → "orders reversed GitHub edit and delete callbacks behind their durable root", expected false to be true.

Two different files, neither touched by this branch, and both tests pass locally on this branch head (vitest run … -t "…", 1 passed each). A fork contributor cannot re-run a failed job, so 1db5d93 is an empty commit that only re-triggers the workflow. If the shard goes red again on an unrelated file, please tell me whether you want me to chase it here or in a separate issue.

@joinwell52-AI

Copy link
Copy Markdown

Your explanation of the second-read resume race was useful for our recovery research. We ran a bounded branch probe at 0e9b24c and 1db5d93: unchanged getInvocationBlock and recovery-branch code, scripted database rows, and an escalation recorder—not PostgreSQL or a full recovery sweep.

Across seven scenarios, the candidate skipped already-paused and mid-pass-paused companies while preserving escalation for an active company's actual budget exhaustion and an agent budget hard-stop. We also resumed the company immediately after obtaining a company_paused block: the candidate still skipped using that observed cause, rather than reinterpreting the now-active company as evidence of a budget failure.

That helped us see the discriminator as evidence attached to an observation, not merely a nicer error label. Would an explicit resume-after-block regression be useful alongside the two pause tests? It would preserve the distinction you described if a later refactor is tempted to re-read company status again.

Probe, pinned versions, and saved branch observations

@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

@joinwell52-AI Yes, it is useful. Added in ff46620: skips a stranded assigned issue when its company resumes right after the pause block.

The test pauses the company inside getInvocationBlock, after the per-issue pause check has already passed, and resumes it right after the block is returned. So the sweep holds a company_paused block while the company row already says active. It asserts escalated === 0, skipped === 1, the issue keeps its status, and no recovery action is written.

I checked that it guards the distinction and not only the current code: with the cause check replaced by a fresh isCompanyPaused read, the test fails with expected 1 to be +0 on result.escalated, which is exactly the false budget verdict. With the fix it passes.

The branch is also rebased onto current master. The only conflict was in budgets.ts against #17046 (prepared scope reads, unpricedUsageBlocks, budgetHeld); cause is now attached to those upstream return paths unchanged in meaning. The two empty re-run commits are dropped, so the branch is one commit again. Locally budgets-service.test.ts and the three pause cases in heartbeat-process-recovery.test.ts pass (38 passed in the filtered run).

@MrBlackTongue

Copy link
Copy Markdown
Contributor Author

The only red check on ff46620 was ci / e2e shard (6/8): tests/e2e/board-attachment-receipts.spec.ts:293 (classic=true) timed out on the composer while the "Paperclip announcements" dismiss handler intercepted the action. This PR only touches server/src/services/recovery/service.ts and server/src/services/budgets.ts (+ their tests), so it looks like an unrelated flake; the same spec passed with classic=false in the same shard. I can't rerun jobs from a fork, so I pushed an empty commit (5737bf3) to retrigger CI.

MrBlackTongue and others added 4 commits November 7, 2026 21:09
Rebased onto current master as one commit. Same change as before: a paused
company is a deliberate stop, so the stranded sweep skips its assigned issues
instead of escalating them, and the invocation block carries its cause from
the same read.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@MrBlackTongue
MrBlackTongue force-pushed the fix/recovery-skip-paused-company branch from 5d13a04 to d1d5d36 Compare November 7, 2026 19:12
Comment thread server/src/services/budgets.ts Resolved

This branch has not been deployed

No deployments
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.

2 participants