Repository navigation
fix(recovery): skip stranded recovery while the company is paused - #13443
MrBlackTongue wants to merge 6 commits into
Conversation
|
I cannot add reviewers from a fork. @cryppadotta @nickyleach @im0xMagnus — you have recent commits in The change is one skip path in |
[Medium impact] The PR appears safe to merge, with no new actionable issues found. SummaryRecovery now leaves assigned issues unchanged while their company is paused.
Reviews (17) · Last reviewed commit: "ci: retry unrelated cloud auth e2e failu..." · Reviewed by Greptile |
|
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 The snapshot is gone. New regression test: Cost: one indexed primary-key read per candidate issue, plus one more on the budget branch only. |
|
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. Tests: One more note on the previous run. |
|
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:
Two different files, neither touched by this branch, and both tests pass locally on this branch head ( |
|
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. |
1db5d93 to
8f8b280
Compare
f69237f to
20df15f
Compare
67b0384 to
ff46620
Compare
|
@joinwell52-AI Yes, it is useful. Added in ff46620: The test pauses the company inside I checked that it guards the distinction and not only the current code: with the cause check replaced by a fresh The branch is also rebased onto current |
|
The only red check on ff46620 was |
5737bf3 to
137b4d7
Compare
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>
5d13a04 to
d1d5d36
Compare
Thinking Path
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'),reconcileStrandedAssignedIssuesmoves assigned issues toblocked. 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 isbudgets.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
todothat is assigned to the agent.paused.blockedwith the "over budget" comment. Expected: the issue stays intodo.Paperclip version or commit
Reproduced on
masterat 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
blockedin 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
getInvocationBlocknow names the cause of the block it returns:company_pausedfor a paused company,budget_exhaustedfor 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.reconcileStrandedAssignedIssuesnow skips an issue whose company is paused. The budget branch readscausefrom 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.todowith no recovery action and no comment.getInvocationBlock, so the pause starts after the candidate query. The issue must still stayin_progress.Verification
Перебазировано на master
00ad9503fe3f5e4589143f79bba489ec91f976bb, текущий коммит3da76f7c6246e706cfb5ac6faa740df77504b56a:pnpm -r buildи проверка типовpnpm -r typecheckпрошли.Локальная сборка 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
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived detailsПовтор CI: пустой коммит
3da76f7c6246e706cfb5ac6faa740df77504b56a, дерево идентично55b16bc97. Предыдущий прогон упал в несвязанномcloud-auth.spec.tsпри чтении освобождённого ответа. Повторный CI полностью зелёный, включая cloud-auth; Greptile — 5/5.