mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
DottaandClaude
30403781b9
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run survives a control-plane restart through restart recovery, which resumes the interrupted provider turn > - Restart recovery charges a provider attempt only when the provider did work that could be lost, and it stops after three charged attempts > - From controller generation 12 on, every restart charged an attempt before that work rule was checked > - In a busy restart wave, one long task sees 12 to 15 restarts, most caused by other tasks. A restart that hit before the provider accepted a turn was charged, the budget ran out, and a person had to click Retry > - This pull request keeps the work rule past generation 12 for restarts that a durable record proves did not start the provider, and bounds a crash loop with a count of consecutive restarts without provider progress > - The benefit is that a long task survives many unrelated restarts without a manual Retry, while a run that never makes progress still stops ## Linked Issues or Issue Description Refs #20625 (same restart recovery path). No public issue exists for this case. **What happened?** `restartResumeKeepsAttempt` returned "charge" for every restart once the run reached `NATIVE_PROGRESS_CREDIT_GENERATION_LIMIT` (12) controller generations. It did this before it read the work-based outcome. In a local restart chaos wave (L6, seed 9106, task t03, Claude B5 with RR1) the run reached generation 13. The last restart interrupted a resume whose runner started but whose provider had not accepted a turn. That restart was charged at the last attempt, so the claim failed with `execution_recovery_budget_exhausted` and `native_restart_recovery_blocked`. The task needed a manual Retry. **Expected behavior** A restart that a durable record proves did not start provider work does not spend an attempt, also past generation 12. A crash loop that never makes provider progress still stops automatically. When it stops, the task shows a Retry that works. **Steps to reproduce** Run a native task past controller generation 12. Make the last restart hit after `native.process_start_requested` but before `turn.accepted`, with the provider attempt at 3. The new integration test `does not charge a restart past the generation limit whose resume never started the provider` reproduces the failure before the fix. **Paperclip version or commit** `master` ated5e7493bb. ## What Changed - `restartResumeKeepsAttempt` now returns `{ keepsAttempt, noProgressRestarts }`. A new helper judges the interrupted attempt as `provider_not_started`, `provider_progressed` or `charged`, with the same proofs as before. - `noProgressRestarts` is 0 when the interrupted attempt is proven to have made provider progress. Otherwise it is the previous claim's count plus one. A clean-drain claim carries the previous count. - Before generation 12 the decision does not change. - From generation 12 on, a restart keeps the attempt only when the interrupted attempt is proven `provider_not_started` and the count is below the new `NATIVE_NO_PROGRESS_RESTART_LIMIT` (12). Provider work (progress, or an accepted turn without progress) and unprovable cases still charge. - Each restart claim writes `noProgressRestarts` into its `recovery_history` entry. No schema change: `recovery_history` is JSONB. - Claims saved by earlier releases have no count. They are read as zero. - Integration tests: the three generation-cap remote tests now check the new bound. New tests cover the unstarted restart past the limit, the consecutive-limit stop with a working Retry, the progress reset, an accepted turn without progress, an unrecorded previous claim, and an old-format claim. - The durable-ownership plan (section F) records the rule. **Why 12.** It equals the existing generation limit (`PROVIDER_ATTEMPT_CREDIT_GENERATION_LIMIT`). The count includes restarts before generation 12. Thus a run that never makes provider progress is bounded exactly as before: the existing test `bounds a crash loop that never starts the provider` passes unchanged. Only proven progress resets the count. In the failing wave, the task had made progress shortly before its last restart, so its count was 1. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/heartbeat-native-restart-suspension.test.ts` - Red before the fix: with only the new constant added to the old code, the 9 new and changed tests fail. With the fix, they pass. The existing crash-loop, progress and unstarted-resume tests pass without change. - The consecutive-limit test checks that the stopped run is `terminal_failure` / `native_restart_recovery_blocked` with no lease owner and nothing scheduled, that `taskInterruptionPresentation` shows `failed` with `canRetry: true`, and that `requestTaskRetry` accepts the Retry. - `cd server && npx tsc --noEmit -p .` - Full local run of the three suites above: 322 passed, 14 skipped. - Live: local restart chaos wave L6 (seed 9106, concurrency 3, `--env local --adapters codex,claude --workloads A1,B5,C1 --faults RR1,RR2,RR3 --tasks 6`) at7f7803ab0c: 6 of 6 tasks passed, and no task needed Retry. Task t03 (Claude B5 RR1) reached controller generation 13 again. The claim at generation 13 resumed the run (`resume_dead_runner`), and the server log has no `execution_recovery_budget_exhausted`. The same task on master was blocked at that claim and needed Retry. ## Risks - A run that makes provider progress between restarts past generation 12 still charges each of those restarts. This is the same as before, by design. - An upgrade or a mid-run deploy reads old claims as count zero. A run that was already in a no-progress loop can get up to 12 more proven-unstarted restarts before the bound applies. It cannot be charged where the old rule did not charge. - A rollback ignores the new field and restores the old rule. - Past generation 12 the claim now reads the event log, as it already does before generation 12. This is not on the turn-start path. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, local test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] 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 - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>