mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-17 01:25:26 +02:00
DottaandPaperclip
5edf55d735
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runner sessions can stay warm between turns in a reusable sandbox. > - Heartbeat stops that sandbox when a turn fails or is cancelled. > - The native executor treated every returned terminal result as a successful warm release. > - A failed session could therefore retain a transport for a stopped sandbox. > - This pull request retires unsuccessful sessions before heartbeat stops their sandbox. > - A retry can start without inheriting that stale transport. Successful turns stay warm. ## Linked Issues or Issue Description **What happened?** A structured failed or cancelled terminal result did not throw. The host kept its native session in the warm cache even though heartbeat stopped the reusable sandbox. Later session retirement could use the stopped provider transport and reject the retry. **Expected behavior** Retire the unsuccessful session and collect its managed files before returning to heartbeat. Keep successful sessions warm. **Steps to reproduce** 1. Run a native session with a warm lifecycle and a reusable sandbox. 2. Return a structured failed or cancelled result from the provider. 3. Stop the sandbox after the executor returns. 4. Retry with a changed native session identity. 5. Verify that the previous session was closed before step 3 and is not closed again during retry. **Paperclip version or commit** Developed from `b54b2dc35`, the current `origin/master` at implementation time. **Deployment mode** Native runner with a reusable Daytona sandbox. The same warm-session release path also serves local providers. Related work: #14735 added incremental managed-file checkpoints for warm turns. #14734 preserves tool outcomes during shutdown. This change fixes the host's handling of unsuccessful terminal results. ## What Changed - Retain a warm session only when its terminal run state is `succeeded`. - Use the existing failed-session retirement path for structured failures and cancellations. - Preserve the existing checkpoint-before-retirement path, including when provider shutdown fails. Collect stopped files after successful shutdown, including edits made during shutdown. - Retire the warm owner if the checkpoint or its receipt callback rejects, then propagate the initiating error. - Add ten regression cases for failure and cancellation, with and without managed files. They check cleanup order, cache removal, retry with a new session identity, and edits preserved when close rejects. - Document the lifecycle rule. ## Verification - The failed and cancelled managed-file regressions fail on unpatched master because the provider is not closed. - All 516 native executor tests pass, including ten new regressions. - `pnpm -r typecheck` and `pnpm build` pass. `pnpm test:run` finished its general-server group with 14,537 passed, 75 skipped, and one existing failure in `native-session-resume.test.ts` (`retainedNativeCleanupJournalMatches`, line 1125). A separate run using the unmodified `origin/master` executor fails the same assertion. The command stops before later local groups; all equivalent CI groups pass on this head. - All 56 PR checks are successful or intentionally skipped. Greptile reviewed this head at 5/5 with no remaining findings. - A real Daytona public-API probe forced a Codex authentication failure, waited for a confirmed sandbox stop, corrected the credential, and retried successfully in the same resumed sandbox. It verified the agent note in canonical storage and deleted the test sandbox. - Pumpkin staging on this exact commit: forced a structured authentication failure, confirmed sandbox stop, corrected the credential, and retried successfully in that same sandbox. Three successful turns kept the same live runner PID/start ticks, native session, runner instance, and provider session. Canonical downloads verified the note, all 8 MiB binary bytes, and deletion persistence. Subsequent checkpoints hashed/copied only 52 and 78 bytes. After sandbox deletion, canonical files still matched. Temporary secrets, agent configuration, and test policy were cleaned up or restored. - Unpatched live probes with both 5-second and 90-second retry delays also succeeded. The unit regressions prove incorrect retention; the original stopped-lease exception was not reproduced in those probes. The staging recovery campaign used the public retry API after an explicit reset of the disposable task’s already-deleted old sandbox/session. The UI did not expose a Retry control on the inspected task or run detail views. - Temporary API-key authentication was used for the campaign. This change does not repair the original managed-subscription authentication 401. ## Risks - Failed and cancelled turns now close their provider sooner. Successful warm-turn behavior is unchanged. - Cleanup uses the existing failed-session path. Its existing handling of cleanup errors is unchanged. - No database migration, API contract change, or dependency change is included. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, code execution, and test analysis. The exact served model ID and context-window size are not exposed in this session. ## 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 (516 relevant executor tests; the full local suite has the independently reproduced baseline failure documented above) - [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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>