mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 22:13:59 +01:00
b0fa2ef9bfb16571d4d760fba4012e63ba55e68b
12
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
07a2941f0c |
fix(recovery): reclaim verified inactive provider caches on full disks (#19192)
Recover from a full-disk workspace save by reclaiming only a verified inactive provider installation under the existing source and maintenance authority. Commit the preparation inventory in Core before changing cache contents. Bind interrupted cleanup to stable filesystem identities and keep small staged generation markers when directory birth times are unavailable. Skip read-only or permission-denied assets and stop freeing blocks once marker space is available. Preserve task files, Git, session history and uncertain action receipts. Only the normal saved-workspace and provider-stop receipts permit continuation. Validated with 446 complete recovery regressions on the repair revision, subsequent focused recovery and transport suites, typecheck/build, and all exact-head hosted CI. Greptile reviewed the final head at 5/5 with no unresolved findings. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
46fb933b01 |
fix(recovery): keep the stop receipt when a file-preserving restart fails (#19181)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs can execute in a remote sandbox. When a run fails, Paperclip stops the sandbox and records the provider's stop receipt on the environment lease. > - System workspace recovery can restart that stopped sandbox to save its files. Before the restart, it removed the stop receipt from the lease. > - If the restart failed and the provider could not stop the sandbox after that, the lease had no receipt. Continuation admission requires a stop receipt on every remote lease of the failed run. So every Retry and every comment waited for an environment that never stopped. > - This pull request keeps the original receipt until the provider confirms the restart. It restores the receipt when the restart fails and the sandbox cannot stop. A restart that fails because the sandbox disk is full now ends recovery with a fresh-workspace outcome. > - The benefit is that a failed file-preserving restart no longer blocks the task permanently. Saved input and Retry continue in a fresh isolated workspace, and the original sandbox stays retained. ## Linked Issues or Issue Description No public issue exists. This is a bug report in the format of `bug_report.yml`. **What happened?** 1. A native run failed because its Daytona sandbox filled its 10 GB disk. 2. Paperclip stopped the sandbox normally and recorded the stop receipt on the environment lease. 3. Some minutes later, system workspace recovery tried to restart the sandbox to preserve its files. Before the restart, `prepareStoppedSandboxForRecovery` deleted the stop receipt. 4. The restart failed on the full disk. The sandbox went into the provider's `error` state. 5. The stop-only teardown after the failed restart also failed, because the provider cannot stop a sandbox in that state. The lease stayed in `pending_cleanup` with no receipt. The cleanup sweep retried the same failing stop with backoff. 6. `admitExplicitNativeContinuation` requires a provider stop receipt on every remote lease of the failed run. Every Retry and every comment was deferred with "Waiting for the previous environment to stop. Your message will start automatically." The message never started. Also, after recovery was exhausted, a Retry replaced the exhausted generation with a new one. That generation repeated the same restart, and the Retry still reported success. **Expected behavior** A lease must not lose its stop receipt while the real sandbox state is unknown or not running. When the file-preserving restart cannot succeed, recovery must end in a state that lets the task continue, for example in a fresh isolated workspace. **Steps to reproduce** 1. Use a native run with a retained remote sandbox lease that has a `stopped` receipt and a pending workspace recovery. 2. Make the provider resume fail (for example with `no space left on device`), and make the provider stop fail. 3. Run `recoverRetainedWorkspace` until recovery is exhausted. 4. Save a user comment on the task and run the deferred-comment admission. 5. Before this change, the lease is `pending_cleanup` without a receipt. The comment stays deferred with reason `remote_cleanup`, and no successor run starts. **Paperclip version or commit** Observed on a hosted staging build. Reproduced on `master` at `6452e964a36` with the new tests in this PR. **Deployment mode** Hosted, with the Daytona sandbox provider plugin. The defect is in core server recovery code, so it is not specific to one adapter. ## What Changed - `server/src/services/sandbox-stop-and-retain.ts` - `prepareStoppedSandboxForRecovery` still writes the stop-only intent and moves the lease to `pending_cleanup` before the provider call. It now keeps the prior `remoteExecutionTermination` (and `sandboxStopAndRetainReceipt`, if present) as `priorStop` inside the `sandboxStopAndRetain` intent. - New `confirmSandboxRecoveryRestart` removes `priorStop` when the provider resume returns. From that point, the sandbox may be running, and only a fresh stop can settle the lease. - New `restoreSandboxStopAfterFailedRestart` restores the prior receipts and the `released` / `success` lease state. It works only while `priorStop` is present, so only when no restart was confirmed. It checks the restored receipt with `hasRemoteTerminationReceipt` first. It uses the same attempt fence as `settleStopOnlyCleanup`, and it writes through `writeEnvironmentCleanup` so dispatch and cleanup wake up. - `settleStopOnlyCleanup` now shares that attempt fence (`stopOnlyAttempt`). Its behavior does not change. - `server/src/services/workspace-recovery.ts` - After `resumeWorkspaceRecoveryLease` returns, recovery confirms the restart before any other check. - If the restart is not confirmed, recovery still tries the stop-only teardown first. A successful stop records a fresh receipt, as before. If the stop also fails, recovery restores the prior receipt instead of leaving the lease without one. - A later attempt that finds a pending stop-only lease (for example after a process restart during the reopen) uses the same rule when its stop fails. - A resume error that reports a full sandbox disk (`No space left on device` or `Disk quota exceeded`) becomes `source_unavailable` / `use_alternative_strategy`. The generation is exhausted at once, and the existing fresh-workspace continuation applies. Other resume errors keep the existing bounded retry policy (`provider_unavailable`, at most six attempts in 30 minutes). Because the receipt stays, that policy also ends in a state that lets the task continue. - `server/src/services/task-retry.ts`: Retry binds to an exhausted generation with a `use_alternative_strategy` failure and no fresh attempt, the same way it binds to a complete or running generation. It does not create a new generation that repeats a step the policy already rejected. The Retry continues in a fresh isolated workspace. An explicit workspace recovery request (`requestWorkspaceRecovery`) still starts a new generation. - `doc/task-recovery.md`: describes the receipt handling around the reopen and the Retry binding. - `server/src/services/workspace-recovery.test.ts`: four new tests (five cases) with the real database and mocked provider calls. ### Design choice I kept the existing write-ahead fence. The lease still leaves `released` and shows no receipt while the provider reopen is in progress. So no admission, reuse or destructive cleanup can act in that window, and a crash still leaves a stop-only obligation. The original receipt is not deleted. It stays in the stop-only intent and is discarded only when the provider confirms the restart. This makes one simple rule: the suspended receipt exists only while no restart has been confirmed. I did not use "restore only when the provider confirms that the sandbox is not running". The Daytona plugin reports a sandbox in the `error` state as `active` in `inspectLease`, and its `stopLease` cannot stop that state. So with the current plugin, no provider confirmation exists for this exact failure, and the hold would stay. A separate change to the Daytona plugin's `stopLease` can recover an errored sandbox before it stops it. This PR does not depend on that change. If that change lands and its recovery succeeds, the existing stop attempt records a fresh receipt, and the restore path is not used. I also did not change continuation admission to accept a lease in `pending_cleanup`. Many gates treat pending cleanup as unfinished work (`hasPendingRunCleanup`, the remote stop check and others). A change there would be wider and more sensitive. The restored receipt is the provider's original receipt, bound to the same company, run, lease, provider, allocation and plugin. No process ID or run status takes its place. ## Verification All commands ran from `server/` in a clean worktree of `master` on macOS (Apple Silicon), with the embedded PostgreSQL test database. Vitest ran from the real path. - Baseline on `master` (`6452e964a36`), before any change: `npx vitest run src/services/workspace-recovery.test.ts` → exit 0, 331 passed. - The new tests fail on `master`. I reverted only the three source files and kept the new tests: - `npx vitest run src/services/workspace-recovery.test.ts -t "file-preserving restart fails|without repeating a restart|keeps the stop obligation once|interrupted restart that the provider"` → exit 1, 4 failed, 1 passed. - In the comment cases, no successor run starts (`expected [] to have a length of 1`). A temporary log (not committed) showed the lease in `pending_cleanup` in both cases. In the `unknown_failure` case, the comment was deferred with `remote_cleanup` / "Waiting for the previous environment to stop. Your message will start automatically." This is the same hold as in the incident. - The guard test "keeps the stop obligation once the provider has confirmed a restart" passes on `master`, as expected. It makes sure that the fix does not restore a receipt after a confirmed restart. - With this change, the same command → exit 0, 5 passed. - `npx vitest run src/services/workspace-recovery.test.ts` on the committed head → exit 0, 336 passed (331 existing and 5 new). - Neighboring suites (`task-retry`, `explicit-native-continuation`, `heartbeat/recovery`, `environment-runtime`, and the other server test files that import the changed modules: `live-recovery-work`, `retained-native-session-cleanup-work`, `execution-control-work`, `legacy-workspace-restore-recovery`, `deferred-execution-receipt-work`, `issue-agent-mutation-ownership-routes`, `native-workspace-export-retry`, `native-cancelled-workspace-preservation`, `native-workspace-warm-reuse`, `native-workspace-reconstruction`, `native-failed-workspace-recovery`, `heartbeat/workspace-preparation`) → exit 0, 16 files, 1456 passed. - `npx vitest run src/__tests__/chat-channels.integration.test.ts src/__tests__/slack-conversation-lifecycle.test.ts src/services/native-runtime/preserved-native-response-wait.integration.test.ts` (other importers of `task-retry`) → exit 0, 3 files, 2074 passed. - `pnpm --filter @paperclipai/server typecheck` → exit 0. - Not run: the full `pnpm -r typecheck`, `pnpm test:run` and `pnpm build` repo checks. CI runs them. No live Daytona probe ran. New tests: - `lets saved input continue after a file-preserving restart fails on a sandbox that cannot stop` (`disk_full`, `unknown_failure`): resume fails and stop fails. The lease keeps its original receipt and is `released` / `success`. Recovery is exhausted (after one attempt for a full disk, and after six attempts for an unknown failure). The saved comment starts one successor run with `fresh_isolated_workspace` readiness. The source files stay in the retained sandbox. - `continues a Retry in a fresh workspace without repeating a restart that cannot succeed`: after a full-disk exhaustion, Retry keeps the same generation and binds its intent. The restart is not called again, and the Retry starts one run in a fresh isolated workspace. - `keeps the stop obligation once the provider has confirmed a restart`: resume returns, a later check fails, and the stop fails. The lease stays `pending_cleanup` without a receipt, and the intent has no `priorStop`. - `restores the stop receipt after an interrupted restart that the provider never confirmed`: the lease starts in `pending_cleanup` with a suspended receipt, as after a process restart during the reopen. The stop fails, the receipt is restored, and the attempt continues to the normal reopen. Assumption and limits (runtime dependency rule): the provider calls are mocked, and the database is real. The tests do not exercise the real Daytona error text. The full-disk check matches the patterns that Paperclip already uses for sandbox disk-full errors. If the provider error text does not match, recovery uses the bounded retry path, which now also ends in a fresh-workspace continuation. The change adds one database write after each confirmed reopen and one after a failed reopen and stop. It adds no polling. ## Risks - A resume can fail after the provider started the sandbox, for example when the reply is lost. If the stop-only teardown then also fails, recovery restores a `stopped` receipt for a sandbox that can be running. The old run's processes do not survive the earlier stop, so the receipt is still correct as proof that the old execution ended. But the retained sandbox can run until the provider stops it or a later recovery or cleanup stops it. Before this change, the same case caused a permanent hold. - A full-disk restart failure now exhausts recovery at once. The task continues in a fresh workspace without the failed run's files. The files stay in the retained sandbox. An operator can still request workspace recovery explicitly after the disk is freed. - Retry no longer renews an exhausted generation that has a `use_alternative_strategy` failure and no fresh attempt. For destroyed or missing sources, the result is the same as before but without the extra attempt. - No schema or migration change. A suspended receipt is one extra field in the existing `sandboxStopAndRetain` metadata. Older servers ignore the field. The intent equality checks compare the stored value, so a mixed-version deploy keeps working. - #18820 also changes `server/src/services/workspace-recovery.test.ts`. The two PRs touch different tests. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (model ID `claude-opus-5-5`), running as a Claude Code agent with tool use (shell, file reads and edits, local test and typecheck runs) and extended reasoning. The runtime does not expose the context window size to the agent. ## 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 (affected suites; full repo runs are left to CI, as noted 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 - [ ] 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 Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
53735db7de |
fix(retry): resume verified preparation failures (#19004)
## Thinking Path > - Paperclip must preserve saved agent work while proving earlier execution stopped. > - A deliberate task Retry can observe a newer failure while an older preparation still holds execution. > - That older preparation can also have a finalized empty object log despite never reaching the provider. > - Binding only the latest run and requiring every log field to be absent keeps valid recovery deferred. > - This change follows the exact persisted predecessor chain and binds current Retry authority to eligible holds. > - It recognizes only an exact finalized empty-log receipt within the existing strict preparation proof. > - One fresh user turn can then start with saved work and the existing ownership, stop and accounting checks. ## Linked Issues or Issue Description Refs #18989, #18860 and #18997. The first waits for predecessor cleanup during automatic dispatch. The second selects the committed native session owner. The third retains admitted Retry authority when no source recovery action exists. None passes current board Retry intent to a held failed predecessor. A search of open repairs found no duplicate. ROADMAP.md was checked; this is a recovery bug fix. **What happened?** A failed native preparation retains a legacy run row and an inherited session. Its remote workspace is later saved and verified. An automatic successor also fails during preparation. Board Retry observes that newer run, but the effective hold belongs to its predecessor. The Retry does not bind the predecessor's recovery generation. The native historical-source guard then keeps the request deferred. **Expected behavior** Carry the deliberate Retry through its verified same-company, same-task and same-agent persisted predecessor chain. Start one fresh user turn after every source passes current execution, stop and saved-work checks. Keep incomplete or unrelated proof held. Preserve prior work and recorded action outcomes. **Steps to reproduce** 1. Fail native preparation with an inherited session and a remote workspace. 2. Finish its real workspace recovery and stop receipts. 3. Save a newer failed automatic successor with `retryOfRunId` bound to that source. 4. Submit ordinary task Retry for the newer run. 5. Before the change, no successor is admitted. The request remains deferred behind the older hold. **Paperclip version or commit** The regression fails on master `039e0dad2c51c80251e1302a19c5566e039c357a`, which includes #18989. **Deployment mode** Source checkout on macOS arm64 with real PostgreSQL, HTTP task Retry and the assembled heartbeat service. The remote filesystem boundary uses temporary files. Hosted rollout and an original-task Retry are not yet verified. This PR incorporates the reviewed empty-log follow-up #19021, preserving its commits and regression controls. Both parts are required for hosted preparation recovery, so the combined revision is qualified and deployed together. ## What Changed - Accept a finalized zero-byte `s3_segments` log only when its reference binds the exact company, agent and run, its digest matches empty content, compression is false, and no output progress or nonempty excerpt exists. Add a real-recorder HTTP regression and 14 negative controls. Unknown or nonempty logs stay held. - Read a complete persisted Retry chain in one recursive query, limited to 32 runs. Reject missing, cyclic, newer, unfinished, cross-company, cross-task and cross-agent sources. Transcript hints grant no authority. - Bind eligible historical recovery records to the current Retry intent. Keep complete or running recovery budgets. Leave successful ancestors and mismatched recovery-source records unchanged. - Recheck that same chain when selecting an ancestor's fresh-user strategy. - Admit only a held ancestor through the shared native preparation proof. Keep the closed preparation failure codes, no-provider state, profile, input, result, output and accounting checks. Treat inherited session input as prior context and accept the exact bounded host start marker. Verify current ownership, saved work and stop receipts before forcing a fresh session. - Add HTTP and PostgreSQL regressions and document the rule. - Give the HTTP Retry nudge and explicit test worker the same real heartbeat facade and provider boundary. Establish the revoked catalog receipt before either can dispatch. ## Verification - Combined integration head `91f021f233151192f69dd7a035d083abcc67c47c` requires fresh complete CI and review; qualification is in progress. - Repair validation: all **six** recovered preparation provider-boundary cases pass. The catalog cases still require the exact access refusal, zero provider inputs and zero provider turns. The success case still checks preserved file bytes and one native turn. - [Previous exact-head CI](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785508136) failed the catalog case with `remote_workspace_transport_unsupported`. The route created a default heartbeat that could race the injected worker and allocate outside the controlled transport. The repaired test shares its real heartbeat across both dispatchers and holds the agent start lock until the revoked catalog receipt is committed. It tracks and drains both Retry nudges during teardown. It changes no production access check or expected refusal. - The first broad local reproduction hit a full local disk. It passed 201 cases and failed 86, including explicit manifest free-space errors. Generated build caches were removed from these isolated checkouts. The owning run passed 670/671 cases. Its one failure was the saved-Files loader refusing insufficient temporary disk space. After cleanup, that unchanged test passes in isolation. The final shared-facade and start-lock version passes all six dispatch cases. Both legacy continuation suites pass all 29 cases. The final fixture teardown also retires deferred input before removing recovery proof, so global queue sweeps cannot revisit earlier cases. All seven dispatch and saved-Files controls pass together after that change. No disk reserve, assertion or timeout changed. The initial local typecheck also stopped on `ENOSPC` while materializing provider assets. Server typecheck now passes after cleanup. No test timeout or reserve was lowered. - Current-head CI is complete: 52 successful checks, three expected skips, and a successful security status. Both required gates (`ci / e2e` and `ci / verify`) passed on `0adc2e38103533dd45bcc56f957ac019642c57b1`. [Full CI evidence](https://github.com/paperclipai/paperclip/actions/runs/41898305521). Greptile scored final head `0adc2e38103533dd45bcc56f957ac019642c57b1` 5/5 with no blocking findings or unresolved threads. The earlier verification below describes the previous head. - RED on unchanged master `039e0dad2c`: the certified-preparation sibling passes, while the inherited-session case admits zero successors instead of one. A second isolated RED on latest base `8f06bd4e17`, including #18997, uses the exact final three-node test fixture. The HTTP route returns 202, then the test observes zero successors instead of one. All baseline production files remain unchanged. The initial dependency/setup failures are separate from these behavioral failures. - The positive cases cover the failed-failed-successful chain. They keep the completed result unchanged, verify the original saved file and validate fresh-session continuation authority. Repeated HTTP input and concurrent resume calls keep one successor. - PostgreSQL controls accept a complete 32-run chain. A 33-run chain and a newer-created predecessor return no usable ancestry and do not bind the older recovery record. - Safety controls cover company, task and agent scope, conflicting canonical bindings, missing and cyclic edges, transcript hints, live execution and controllers, unknown stop, failed saving, unsafe writer state, Stop, actor and generation changes, independent holds and mismatched recovery records. - Ancestor controls retain provider-event, native-input, session-checkpoint, controller-recovery, result, process, runtime, accounting and unknown-result guards. Host start and warning prefixes use exact origins, sequence and payloads. A fifth extra event is rejected. - The first complete owning run found 22 introduced admission failures. The initial historical-native allowance also applied to exact-source Retry. It admitted contradictory preparation evidence. The fix now requires a canonical ancestor and the shared strict startup proof. The original direct-source assertions remain unchanged. - The next focused run passed 82 cases, including both catalogue-warning controls. One new result fixture lacked the database's required owner tuple. Two existing controls reached their original 30-second limit. The fixture now uses the real native task and completion-contract owner tuple. A controlled nine-case run passes the result case, both unchanged timed-out controls, three failure-code controls and all three positives. The earlier timeout causes remain undetermined. All failed-run logs are retained. - Complete owning suites passed on previous head `137a8eb3febf41052e112fc3c4ffe1abb92bcab7`: all 675 tests across workspace recovery, task Retry, explicit continuation and legacy continuation. The command exits successfully with normal teardown. This includes every earlier introduced admission failure and the two unchanged timeout controls. Its base includes the merged #18997 authority repair and #18982 rolling-copy packaging repair. The rebase retained both test imports at their one adjacent-import conflict. - Node 24.19.0 and pnpm 9.15.4. Module boundaries and `git diff --check` pass. The initial PostgreSQL start lacked package library links after install with scripts disabled. The documented hydration restored the isolated dependency; that setup failure was not a behavioral red. - Previous-head [full typecheck](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785506215) and [full build](https://github.com/paperclipai/paperclip/actions/runs/41883421120/job/126785506094) passed. Earlier attempts received synchronized runner shutdown signals and a lost-runner annotation. The shutdown initiator remains unknown. A CI owner used the documented hosted-runner failover for attempt three and restored the original routing value after capture. That attempt exposed the catalog failure above. Source workflows and check requirements remain unchanged. The current head now has green CI, Greptile 5/5, no unresolved review threads, and no merge conflicts. Hosted rollout and the original task are not yet verified. - Combined recovery proof: 24 of 24 cases passed on this exact head in 43.13 seconds, with normal teardown and unchanged assertions and limits. They cover the real recorder, all 14 empty-output rejection controls, ancestor and direct Retry, saved messages, the configured native provider, and verified stop. The targeted TypeScript graph also passed; the preceding integration step passed 95 safety/provider/message cases and all 49 task Retry cases. - The incorporated #19021 was independently red/green with the real recorder: unchanged predecessor code returned HTTP 202 but admitted zero successors. Its 18 focused cases, all 690 owning recovery cases, full typecheck/build, 1,904 local chat cases, complete CI and Greptile 5/5 passed before integration. The unchanged CLI startup lane passed one justified retry; no hosted acceptance is inferred from those results. - Additional local qualification of the unchanged workspace fixture alongside the native bootstrap fixture passed 288 of 289 cases. One manual-admission HTTP case ended with `socket hang up` after 451 ms, without an assertion or HTTP status. That unchanged case passed once in isolation at the original 40-second limit, with normal teardown, and passed in exact `0adc` CI in 17.2 seconds. The earlier disconnect cause remains unknown; no assertion or timeout changed. - An earlier CI attempt failed the newly merged native ingress fixture because its shared Node command failed strict permission qualification. The current base includes #19020's owned executable fixture repair; no production permission check was relaxed. - Integration with current master had one documentation conflict. An automatic test merge also duplicated an app declaration; that syntax failure was retained and fixed by reusing the configured real HTTP app. Both new message-state assertions remain intact. Workflows and lockfile retain master's contents. ## Risks - A damaged or overlong chain remains held. The helper does not truncate it and grant partial authority. - The new query runs only for a deliberate Retry or its ancestor strategy check. It adds no background polling, table, credential or provider policy. - A fresh intent is not stop or file-recovery proof. The existing owners retain those checks. Prior effects remain fenced, and no old result is replayed. - Local tests do not prove hosted rollout. Live acceptance must use the served fixed revision and the task's original saved work. ## Model Used OpenAI GPT-6 through Codex for the original change. OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, for the merge and CI repair. Both used repository tools and PostgreSQL verification. Context-window limits were not exposed by the harness. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
|
25b5a072cc |
fix(recovery): let messages continue recovered preparation (#19026)
## Thinking Path > - Paperclip must preserve work while allowing people to continue failed tasks. > - A preparation failure can retain an ID from an earlier provider session. > - Workspace recovery and host settlement already prove that the failed attempt never dispatched a provider. > - The preparation classifier also required that inherited session ID to be absent. > - That extra condition kept new messages, saved messages, and Retry behind a resolved recovery hold. > - Classify execution from the existing execution evidence and admit one fresh turn through normal gates. ## Linked Issues or Issue Description Refs #18908, #19004, and #19021. The latter two address historical Retry ancestry and empty object logs. This repair addresses a current source with an inherited session and ordinary user messages; it needs neither ancestry nor an object log. ROADMAP.md and related open PRs were checked. **What happened?** Sending a message to a failed task saved it in the queue, but did not start another run. The failed native preparation had recovered files and durable host settlement, plus `sessionIdBefore` from prior work. The classifier treated that input as execution evidence and kept the recovery hold. **Expected behavior** Admit the new user turn once the existing saved-work and stop checks pass. Preserve previous failures and files. Show the current run instead of the old error, and consume messages when execution starts. **Steps to reproduce** Fail Runner preparation before runtime selection. Preserve its remote workspace. Retain an inherited `sessionIdBefore`. Send an ordinary message or click Retry. The three new HTTP regressions accept the request but admit zero successors before the repair. **Paperclip version or commit** Base `3bd52a9fa0`. **Deployment mode** Authenticated hosted Runner. Regression verification uses macOS arm64, Node 26.4.0, real PostgreSQL, HTTP routes, and real temporary workspace archives. The external sandbox transport and native provider boundary are controlled in the execution fixture. ## What Changed - Stop requiring the inherited session input to be absent from otherwise verified failed preparation. - Cover new messages, previously saved messages, Retry, request replay, one successor, recovered files, and current task presentation. - Exercise the existing preparation safety controls with inherited context, including a new output-session refusal. - Exercise native execution and finalization with the inherited session input. - Document the continuation rule. ## Verification - RED: all three new HTTP cases admitted zero successors before the production change. - All three final HTTP cases pass. The 41 preparation safety cases, including the new output-session refusal, pass with inherited context. - The native execution fixture passes with inherited context. It reads restored files, writes the continuation result, runs one provider turn, persists the accepted result, empties the queue, and resolves the notice. - Initial positive assertions expected an empty queue while the fixture deliberately held the agent's slot, then expected a null optional wait field. The final assertions use the actual queue contract and retain execution and presentation checks. - `pnpm build`, `node scripts/typecheck-low-memory.mjs`, module boundaries, and `git diff --check` pass locally. The final rebased head `56d2ef26` passes the four HTTP/native journey cases. All 634 tests in the four complete owning recovery, Retry and continuation suites pass. Current-head full CI passes: 53 successful checks and three expected skips, with no failed or pending checks. The full local command has the documented timeout below. - The full local command passed its first three chat shards, then stopped in shard four on a 15-second timeout in `answers each finding thread in its own run instead of one unattestable batch` (267 other shard cases passed). The exact unchanged test passes alone (about nine seconds of test work) and all three full CI chat groups pass on this head. It uses a custom adapter, so the changed native-preparation proof is inapplicable. The timeout cause remains unknown; the full local command is not claimed green and its later groups were not reached. - Initial CI attempt: all other checks passed, but server shard 2/12 failed on the unchanged `idle-completed-history.test.ts` owned HTTP runtime test. The ten-second readiness probe returned ECONNREFUSED. The source and runtime have no diff from the base and this test does not use failed-preparation admission. The exact local case passes; the underlying readiness cause remains unknown. Logs and annotations are retained at [the failed job](https://github.com/paperclipai/paperclip/actions/runs/41897539206/job/126811385017). The single failed-job retry passed on the same head, including the dependent verify aggregate. This proves recovery of that check; the original readiness cause remains unknown. - Greptile reviewed the exact head `56d2ef26` at 5/5 with no actionable findings and zero unresolved threads. Hosted recovery remains unverified until the repair reaches the serving revision. ## Risks The risk is mistaking earlier context for a stopped current execution. Current provider session, process, controller, native input/result/output, accounting, exact lease stop, workspace epoch/generation/writer, actor, assignment, pause and approval checks remain enforced. No migration, new polling, API, credentials or execution policy is added. ## Model Used OpenAI GPT-6 through Codex, using reasoning, code editing, shell tools, and browser diagnostics. The exact serving snapshot and context window 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 (634 owning-suite cases and final-head journey cases; full-local timeout is 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> |
||
|
|
2518ac85ab |
fix(recovery): continue verified failed native preparation (#18908)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task Retry keeps a durable request while the system stops old work and saves its files. > - Native preparation can fail after it allocates a workspace, but before it starts a provider. > - Completed preservation can resolve the action while the original Retry still waits. > - The old classifier rejected every allocated failed preparation. The source check also returned before its canonical Retry fallback. > - This pull request accepts only complete server proof and moves the existing exact-source fallback before that return. > - The same Retry can then start one normal continuation. Current access and unknown execution still hold work. > - Saving files and admitting that continuation must use the same parent-row lock order; the composed head includes the supporting preservation fix. ## Linked Issues or Issue Description **What happened?** Task Retry accepted a failed native preparation and saved its request. The workspace had complete preservation and stop receipts. The request still waited for execution recovery and created no continuation. **Expected behavior** The same deliberate Retry must continue after the system verifies all saved files and source leases. It must preserve current task, actor, repository, Stop and execution checks. **Steps to reproduce** 1. Record a native-adapter claim that fails during unresolved legacy preparation, before provider execution. 2. Finish real workspace preservation and exact lease cleanup. 3. Submit normal HTTP task Retry and reconcile its canonical request. 4. On the base commit, no run is linked to that request. With this change, one continuation is linked to it. **Paperclip version or commit** Reproduced on master `d7283e875772ca8c684dc86e471e6b48d543b7b1`. **Deployment mode** Source checkout with real local PostgreSQL and filesystem archives. Object storage, sandbox transport and the native provider backend are controlled in the final execution test. This is not a live model or hosted object-storage claim. **Prerequisite: #18918** at `4c1d7769e6cfa9536a67c4e3b536cef155f19966`. This head includes that parent through normal history-preserving merges while retaining `master` as its target. The parent owns remote workspace save, prepare and provision lock order, legacy import reservation and publication, their concurrency regressions, and the database lock-order documentation. Merge the parent before this PR; no deployment is implied. Related: Refs #18892 for interrupted preparation. Refs #18882 for the separate connected-chat setup path. Refs #18898 for already-admitted cleanup waits. This change does not depend on #18882 or #18898. ## What Changed - Accept allocated failed preparation only with the server error and durable settlement records, no execution/session/checkpoint/output/result/coordinator evidence, all exact remote source leases stopped, and verified saved workspace proof. - Recognize only the host's closed accounting scope and typed pre-provider unsupported-harness warning. Unknown accounting, provider metrics and arbitrary diagnostics remain holds. - Check the canonical failed-preparation source before an empty resolved-action list can return. Reuse the normal locked continuation checks. - Add HTTP, preservation-worker and native execution coverage, including source failures produced by real heartbeat workspace preparation. Keep one canonical request and one provider turn. Preserve the original failure and real archive files. - Restore storage, environment and experimental settings even if native test setup or owned async cleanup fails. - Cover changed Stop, source, actor, controller, provider evidence, snapshot generation, writer and lease receipts. Keep current unavailable-catalog denial before provider startup. - Share one bounded host-settlement receipt reader between failed-native preparation and existing-directory readiness. Keep each caller's accounting, profile and additional warning restrictions. - Include parent #18918's company/task/run-before-workspace ordering for save, prepare and provision. Keep legacy import publication out of reverse native-owner waits with exact NOWAIT refusal. Retain foreign keys, ownership checks, uploaded work and transaction-aware notifications. - Document the failed-preparation continuation and parent lock-order contracts. ## Verification - Causal base comparison: the normal HTTP/preservation regression fails on the canonical request's missing successor. Both existing interrupted-preparation controls pass; the same cases pass with the classifier/fallback repair. - Actual heartbeat-produced catalog failures reproduce RED before the host-accounting fix, both with and without the real typed unsupported-harness warning. No source failure records are patched in those cases. - The native execution case keeps `paperclip_runner`, native runtime selection and the requested Codex model. A controlled native session reads real restored `saved.txt`, writes `continued.txt`, emits an accepted result and finishes through the real finalizer once. Only the external native backend is controlled. - Current unavailable-catalog admission and suspended authenticated membership remain denials before provider work. Extra provider/session/accounting material, Stop, newer source, writer/generation and lease-receipt mismatches remain held. - Both settlement consumers pass 53 focused real PostgreSQL/filesystem/native-boundary cases on Node24.22.2. Existing-directory warning presentation/provenance and durable-measurement restrictions are preserved. - Parent #18918 proves real PostgreSQL lock cycles through actual save, prepare, provision, reservation and import publication owners. Its native status peer test proves a busy refusal without a deadlock. Immutable task binding, writer/epoch fences, same-journal retry and unexpected-error rollback controls retain uploaded objects and original files. The original spontaneous peer identity is not claimed. - Historical head `6204f565` passed all 54 CI checks, but its local affected run passed only 572/573 with a real PostgreSQL preservation deadlock and its Paperclip review was 3/5. Those results remain historical evidence; this composed head includes the explicit parent repair. - Production-composed head `93a02d4cf74f4d8d2cc6c1b62884bc58280500bf`, Node24.22.2 / pnpm9.15.4: full `pnpm build` and recursive `pnpm -r typecheck` passed. The six affected recovery, continuation, Retry, migration and concurrency files passed 724 tests. Opt-in remote workspace integration passed 65/65 and ownership lifecycle passed 16/16, with no skips. - Final head `65dcd6c6a601448b5a436b7a7f2c84b8cfde761b` adds only the parent's unsupported-host test guard. Its 18 concurrency cases and generated server test-layer typecheck passed on the exact head. Production bytes match the qualified head above. - Exact-head CI passed on `65dcd6c6`: 54 success, 3 skipped, 1 neutral; no failed or pending jobs. Greptile reviewed this exact commit at 5/5 with no actionable issue and zero open threads. - The current-head Paperclip assessment is registered and queued with no run at the latest observation. It has not delivered a current-head score. The earlier 3/5 assessment and its resolved findings remain history. No extra review request or hosted task replay was used to force delivery. ## Risks - The new allocated-source proof is conservative. Extra or missing execution material keeps work held and needs inspection. - This changes admission after completed preservation. It does not certify unknown provider effects or restore repository access. - Administrative import can now return a named busy hold while an execution row is owned. It retains the archive and journal for a later administrative retry. - There is no schema migration or new API. Stop, task ownership, current actor access, budget and approval checks retain their normal owners. ## Model Used OpenAI Codex, GPT-6 family. The exact runtime model ID and context window are not exposed in this agent session. The agent used reasoning, tool calls, code edits, real local PostgreSQL and filesystem tests. A separate Codex agent reviewed the admission and execution boundaries. ## 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 - [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> |
||
|
|
585b4e6af8 |
fix(chat): preserve deferred Retry source batches (#17883)
## Thinking Path > - Paperclip manages agent work through tasks and governed execution. > - Connected conversations retain the source and authority for each request. > - Task Retry pins the full saved GitHub input batch before it waits for cleanup. > - Resuming that wait could append an independent board queue after authorization. > - Dispatch then rejected the changed batch before the provider started. > - This pull request keeps the authorized batch intact and restores certified undelivered input from historical failed attempts. > - Each board message retains a separate turn and its own admission checks. ## Linked Issues or Issue Description **What happened?** A deferred GitHub task Retry could consume a separate board message while promoting its saved receipt. The new run then failed with `chat_failed_run_retry_not_authorized`. A second Retry could also fail because the historical run retained the mixed batch. **Expected behavior** Retry must preserve the source batch it authorized. Separate board input must keep its own execution path. Historical input may be restored only when durable evidence proves that the rejected attempt never started a provider. **Steps to reproduce** 1. Retain ten authorized GitHub requests behind a failed native turn. 2. Start task Retry while the predecessor still lacks its exact termination receipt. 3. Save an independent board message. 4. Complete cleanup and resume the same Retry receipt. 5. Observe the mixed batch rejected during dispatch. Retry that failed attempt to reproduce the second refusal. Refs #17725 for the existing saved-input Retry contract. This change repairs the later queue merge and its retained failed state. Related #17878 covers snapshot continuation and later turns in recovered directories; its workspace repair can be reviewed independently. ## What Changed - Keep deferred chat Retry promotion scoped to its exact receipt. Ordinary board Retry coalescing retains its existing behavior. - Let a new explicit task Retry restore a historical mixed batch only after strict never-started, source, ownership, allocation, board-comment and prior-delivery checks. - Preserve failed-run history and original budget-skipped receipts. Record restored queue ownership in activity. - Add production queue and HTTP-route regressions. Update the execution contract and verification maps. ## Verification - Before the repair, the deferred ten-input batch failed before identity initialization. The same regression now reaches the real initializer with the exact GitHub batch. The independent board message then reaches its own initializer in a separate turn. - Before historical recovery, ordinary task Retry refused the persisted mixed failed batch. The repair passes that case without changing the failed run. HTTP duplicate requests retain one Retry owner. An additional compatibility probe restored the old queue candidate predicate and removed all historical mixed-state fixture writes: the real producer created the exact batch and the complete historical HTTP recovery case passed (1/1). The fixed-source historical fixture now uses the production serializers. - The affected Retry group passes 86 cases on commit `325c00158df5569f5b42731cda4ecc4584909d99`, including already-coalesced external receipts. Refusals cover edited sources, active controllers, unresolved allocations, changed receipts, foreign external receipt ownership and prior delivery of either board or GitHub input, including immutable budget-skipped receipts. - Full workspace `pnpm -r typecheck` and `pnpm build` pass with these source changes. Map, module-boundary and diff checks pass. - Full local `pnpm test:run` was started. The host cannot create additional PostgreSQL semaphores; database-dependent suites fail or skip at startup. The affected database group runs against a separate migrated PostgreSQL 17.10 test database with Node 24.22.0 on macOS arm64. - The regression deliberately stops after the real identity initializer to avoid provider allocation. It proves HTTP, queue, source authorization, identity and separate continuation behavior. It does not claim a live model result, GitHub publication, deployment or recovery of the original hosted task. - Current head `325c00158df5569f5b42731cda4ecc4584909d99`: all checks are terminal with no failures or pending checks; required `ci / verify` and `ci / e2e` pass. Conditional Storybook checks are skipped. Greptile and Paperclip each reviewed this exact commit at 5/5. Both earlier review threads are resolved. Paperclip retains one optional robustness suggestion for batch ordering; the verified historical producer order remains a strict recovery contract. ## Risks Restoring input could repeat work if the prior turn had executed. The existing strict bootstrap proof rejects provider output, native state, process or controller evidence, and allocations. Restoration also checks current external authority, unchanged board revisions and other run history. An uncertain executor remains held. No migration, provider fallback or relaxation of dispatch validation is included. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository inspection, code editing and test execution. The exact runtime model ID and context-window size are not exposed in this session. A separately shared regression supplied independent reproduction evidence. ## 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 (affected database regressions; full local suite has the host limitation described 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> |
||
|
|
208a093d19 |
fix(chat): preserve resumed review waits and retry newer saved input (#17725)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/36/changes/paperclip/36 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review agents publish findings and wait for an authorized reply or head change. > - Review tools could retain the requester after resume while completion lost that same wait proof. > - A yielded report could then queue its own continuation or repeat a missing-wait error. > - Ordinary Retry could also select an older failed turn while a newer saved request had no admission. > - This change shares the existing authority proofs and canonical admission owners. > - Current access, publication, execution and budget gates remain required. ## Linked Issues or Issue Description Refs #17318. Builds on the canonical saved-input retry owner in #17733. Related #16550 covers deferred legacy comments after ownership waits. The separate recovery repair in #17724 owns the cleanup-to-saved-input handoff; this PR does not add a second admission owner. A resumed review could publish an assessment, then turn its `response_wake` into an immediate `issue_status_changed` wake. A provider that omitted its continuation could instead repeat a completion error. A newer linked-human message saved during a budget hold could also remain unstarted because Retry selected the older failed source. ## What Changed - Resolve passive review waits through the accepted requester ledger used by GitHub tools. Require the latest reviewed head, exact company, task, agent, repository and processed publication under current access. - Fill omitted/null yielded continuation from that server proof. Permit an inherited normal tree resume only through its accepted parent identity. Permit an ordinary follow-up only after this run has a processed useful reply linked to its exact inbound message. Reject acknowledgements, partial assessments, pending or uncertain writes as that reply proof. - Recheck the same run/review/head/source proof during feedback, finalization and final status commit. Preserve explicit malformed-input rejection and Done policy. Add no monitor permission or publication grant. - Keep heartbeat and publication projections narrow. Preserve both published Copilot v62 identities and generate source-bound v63 from the pinned merged base. No v63 live qualification is claimed. - Certify the structured isolated-directory binding failure only at its pre-allocation boundary and only through canonical task Retry. Other workspace validation reasons and unknown effects remain denied. - Route Retry to eligible newer saved input through the existing canonical owner. Preserve the old result and skipped receipt. Require current principal, source, App, runtime fence, budget, native ownership and exact cleanup proof. - Recover absent historical App metadata only through an earlier trusted processed response receipt and an immutable non-null endpoint claim. Explicit malformed or conflicting metadata stays denied. - Revalidate execution through the exact claimed wake, coalesced task Retry and resolved user-continuation receipt after admission. A second normal Retry can recover the exact never-started authorization failure with unchanged saved input. Require empty execution evidence, exact diagnostics and no allocated or active executor. - Explain when a recorded result prevents exact replay. Ordinary board Retry directs a new task message to continue from saved work. Inherited requester identity alone never grants replay. - Keep accepted board-message preparation failures with the canonical task Retry owner. Certify the exact unchanged comment, original user wake, accepted same-user identity and absence of provider execution. Pin that source, recheck it at admission, and bind identity initialization to the coalesced intent and exact claimed successor receipt. Uncertifiable sources retain an actionable fresh-message refusal. - Return safe named authority categories for board Retry refusals. Update execution documentation and capability maps. Add no schema or polling loop. ## Verification - A fresh external review request exposed a separate isolated-directory preparation failure before allocation. Its first normal Retry saved an intent but admitted no successor. This follow-up certifies only that exact structured preparation boundary through the existing canonical Retry owner, with trusted failed settlement and no executor, lease, result or output. A real HTTP/queue regression reproduces the missing successor before the repair. Twenty-three current-source safety cases and the final 98-case Retry/HTTP group pass, including a fresh click after a denied intent, source revocation, accepted results and the older direct API remaining denied. The warning case uses actual configuration preparation and production event append; unknown warning shape or provider provenance stays denied. Full workspace typecheck, build, maps and diff checks pass. Renewed hosted normal Retry now succeeds on the final combined source: it preserves the exact saved request and linked requester, starts one native successor in a fresh attempt workspace, publishes the current-head 2/5 assessment and commits In review with passive response wake. - A later hosted normal Retry published a low-score assessment but lost its accepted source identity because the previous attempt failed before identity initialization. It repeated continuation errors, then submitted Done. This is failed acceptance, not a successful qualification. The initializer now retains the exact source only after execution-time failed-chat Retry authorization; generic Retry and Done policy are unchanged. A real queue/initializer regression fails with a null source on the previous code and passes with the newer saved source. Two real database completion cases verify that source yields the authoritative wait and commits In review. Renewed hosted normal Retry now retains the accepted source and exposes the authoritative current-review wait. It completed after two semantic feedback corrections (wrong contract revision, then wrong criterion IDs); the correct contract was present in emitted input. The third finish was accepted and committed In review, with no missing-wait loop or Done escalation. - Repeated board-message preparation failures now walk their canonical intent, failed dispatch and accepted parent identity, rechecking each contained predecessor. A production queue regression fails the second ordinary HTTP Retry before the repair and passes afterward. Changed source, contradictory execution evidence and cyclic receipts remain denied; traversal stops after twenty links. - New inherited omission cases failed before the repair. The final full PostgreSQL 18 wait/status suite passes **252 tests** under Node 24.22. It covers omitted/null input, malformed-first recovery, same-head resume, exact-source replies, changed heads, revoked access, pending/uncertain/mixed publications, final commit revalidation, no duplicate review and no self-wake. - **15 existing publication tests passed before the pinned merge** on a separate fresh database. They retain default partial-delivery and failure-fallback behavior. Context reads remain below the unchanged 16 KiB heartbeat and publication response limits with large saved output present. - Saved-input tests cover a real signed PR webhook, production accounting-budget refusal, normal policy recovery and canonical Retry. Historical producer-shaped tests prove the missing-App case red, then check trusted prior receipts and malformed, late, revoked or mismatched proof. A broader order-coupled test was reproduced on the prior source; its global fault injector now targets its own authorization query. The 43-case sequence passes without changing production behavior or its safety assertions. - New signed-ingress and real heartbeat execution tests reproduced the admitted-successor failure, then the next normal Retry failure. The final pinned merged source passes 249 full GitHub review/publication and Retry cases on a fresh database, including all 49 saved-input cases. A separate 74-case Retry/HTTP run also passes. Exact dispatch negatives cover source, company, agent, task, successor, receipt, pause, assignment, accepted success, controller and cleanup proof. Bootstrap contradictions remain denied. The canonical Retry suite passes 16 cases through its supported external PostgreSQL test connection. - The normal board HTTP route reproduced the unusable inherited-result refusal. It now returns actionable guidance without starting a successor. The actual UI Retry hook and banner tests pass 12 cases. - A production-shaped accepted board comment followed by a pre-provider qualification failure reproduced HTTP 409 before the repair. The final real HTTP/queue test certifies one canonical successor and preserves the accepted identity. It rejects changed source, actor, task, agent, result, controller, process, profile, allocation and receipt evidence. A post-acceptance source change keeps the same intent pending. Execution revalidation rejects removed or mismatched proof. The probe stops deliberately after production identity initialization; it does not claim provider execution. The final source passes the 75-case Retry group, the focused two-case board/inherited-policy group and 19 canonical Retry cases. Full workspace typecheck, build and map checks pass. - Full workspace `pnpm -r typecheck` and `pnpm build` pass. Map, module-boundary and diff checks pass. Full local `pnpm test:run` remains unverified. The 256-case explicit-native suite cannot initialize its packaged PostgreSQL because libzstd is absent; an existing command-scoped fallback did not resolve it. Current-head remote full-suite checks are required. Merged-source full GitHub+Retry (249), separate Retry/HTTP (74), full wait/status (252), profile/history and completion (75), and visible Retry hook/banner (12) checks pass. The fresh owned PostgreSQL 18 databases applied all 377 migrations. - Actual Rust Runner, OpenCode 1.18.34 and exact DeepSeek used unchanged production-generated synthetic contexts. Resume finished with an explicit server wait. The author-reply trial omitted continuation twice: its first report failed the existing empty-criteria check; the corrected second report still omitted continuation and received the exact server wait. It completed once with no missing-wait loop or Done escalation. Controller acknowledgments were synthetic; these probes do not establish hosted publication or board state. - Hosted combined testing proved automatic admission of the exact saved author reply, a useful GitHub answer and committed passive wait. It required two malformed-continuation rejections before a nullish fallback succeeded. A later normal Pause/Resume completed in one finish with explicit response_wake and no new review/comment. The admitted normal Retry then exposed the execution guard gap repaired here. Those remain checkpoint results. On the final combined source, renewed normal Retry completed actual native provider work and GitHub publication, committed the passive wait, released ownership and cleaned up its environment. A subsequent ordinary unmentioned author reply woke once, made no GitHub tool calls or new assessment, and completed its first finish with an explicit response wake. Both exact leases expired with successful cleanup and the queue was empty. However, final destination inspection found a separate automatic fallback overwrote the existing summary with a generic no-final-response message. External summary preservation therefore remains failed acceptance; the separate publication repair in #17726 owns that producer. Full combined publication preservation has not passed. These hosted runs include the pinned-base native additions; they do not qualify Copilot v63. - Current-head automated review is 5/5 with zero unresolved threads. All current-head CI gates passed (54 successful checks/contexts, four skipped, none pending). The earlier login-cleanup idle-work assertion also reproduced locally while its controlled baseline passed; its cause remains unknown, but the exact assertion passed in the fresh current-head CI shard. No timeout, assertion or production workaround was added. This PR is ready for review within its source, authority, native finalization, state, wake and cleanup scope. The independent automatic fallback publication regression is tracked by #17726; no complete combined workflow pass is claimed. ## Risks - The latest saved review defines the durable head boundary. Ingress must record a new head before admission. A reply receipt proves delivery only for its exact saved source; a shared GitHub comment ID alone cannot authorize this wait. - Historical App recovery cannot infer identity without direct proof or an earlier valid server response receipt. It never replays that response. - This does not infer review-score policy from instructions or override Done. No deployment, merge or repair of historical tasks is claimed by the PR. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, repository inspection, code editing and test execution. The context-window size is 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 (focused database and native suites; full local suite limitation described 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 (verified on immutable latest head) - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (verified on the latest head) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
338bf75761 |
fix(chat): admit task Retry after GitHub reconnect (#17733)
## Thinking Path > - Paperclip manages agents and their tasks. > - Task Retry must preserve saved input and enforce current access. > - A GitHub reconnect invalidates old runtime callbacks. > - Retry reused that old callback authority and rejected valid new intent. > - Retry also replayed an older failed request when a follow-up was already saved. > - This change admits a deliberate Retry under current authority and transfers saved input once. ## Linked Issues or Issue Description Refs #16965, #17053 and #17317. These repairs preserve work and contain old executors. This change fixes the subsequent chat admission boundary. A search found no overlapping open reconnect/admission fix. **What happened?** Ordinary task Retry returned `chat_failed_run_retry_not_authorized` after the same GitHub App reconnected. The endpoint was healthy, but the saved request still carried its old runtime generation. A newer queued follow-up also made Retry reject the older failed request instead of continuing the follow-up. No successor was admitted. HTTP qualification found two related defects. Concurrent clicks could omit the canonical chat action from a response. Signed-in board dispatch also passed an incomplete task resource to the permission resolver, which rejected a permitted mutation. **Expected behavior** A deliberate Retry checks current authority for the same App, sender, repository, conversation and task. It continues saved input once. A later reconnect, changed App, revoked access or changed source prevents dispatch. Old runtime callbacks remain invalid. **Steps to reproduce** 1. Admit a GitHub request and record a failed native run. 2. Save a follow-up while execution is blocked. 3. Reconnect the same GitHub App. 4. Click ordinary task Retry. 5. Before this change, admission returns 409. Without reconnect, the newer input alone also causes 409. **Paperclip version or commit** Baseline source: `41c42895fa862c586ff841f8c50200d93ffc83a4`. **Deployment mode** Source checkout on macOS. Regressions use real PostgreSQL transactions and the assembled chat, HTTP route and heartbeat admission services. Provider boundaries are simulated. No hosted recovery is claimed. ## What Changed - Record the admitting App identity for new GitHub deliveries. Historical deliveries can use their original working-comment binding. Reauthorize deliberate Retry under a pinned current generation and credential-reference fingerprint. - Select the complete saved-input queue from the failed run's actual conversation before replaying the older failed batch. Keep exact-run Retry bounded while ordinary task Retry supports larger queues. Transfer its receipts with the dispatch transaction. Recover the same ownership after restart or a lost response. Release unadmitted input after a definitive rejection. - Preserve canonical chat action IDs across concurrent clicks. Supply current task status and assignment fields to board mutation checks. - Keep exact-run Retry, stale webhook, containment, source integrity, publication, pause and budget checks. Update recovery contracts and verification recipes. ## Verification - Red: all three new PostgreSQL regressions failed on unchanged production code. Reconnect failed at chat authorization. Newer input failed at the obsolete-source guard. The combined case also failed. - Green: 61 selected chat integration cases passed. These include 27 new cases and 34 existing cases for containment, retained-source recovery, exact batches and source revocation. - A further red/green PostgreSQL regression reproduces the 51-message overflow identified during review. All 51 messages reach one successor, including a later Retry of that successor. A separate 52-message consumed-batch regression also passes. - A competing conversation retains its own deferred message. Distinct automatic policy contexts are checked independently; revoking only the later policy blocks the whole unadmitted batch. Batch validation still rejects a later message with a changed App binding or deleted source. A real PostgreSQL query capture reduced the 51-message preparation from 1,103 calls to fewer than 180. - All 16 durable task Retry tests passed. - The real HTTP route returns 202 and admits one queued successor for concurrent deliveries. Fault injection covers rollback during input handoff, response loss after dispatch commit, and permission loss when resuming an existing durable wait. A wait only adopts input after current admission succeeds; a one-shot settlement marker keeps status projection read-only. A new deliberate Retry works after another reconnect or a failed successor. - Current-App, repository, sender, source edit/delete, reassignment, endpoint pause and board-permission controls passed. Historical deliveries remain unchanged. - Final-head CI: 55 checks passed, three expected skips. The unchanged minute-bucket rate-limit assertion passed locally against PostgreSQL and on its one CI rerun. Greptile reviewed the final commit at 5/5 with no unresolved threads. - Workspace typecheck and full build passed. Map alignment, module boundaries, UI token gates and `git diff --check` passed. - Hosted task recovery and a live provider turn remain unverified. The local tests hold agent capacity to inspect admission without starting a model or publishing a GitHub response. ## Risks - A deliberate GitHub Retry can admit old saved input under current connection authority. It requires the same App binding and current access. It pins that admission; it does not rewrite the historical delivery or make stale callbacks valid. - The durable chat queue has an optional admission callback. It transfers input only when the transaction contains a valid queued execution or saved wait. Other chat consumers have no callback and retain their behavior. - No migration, Cloud dependency, deployment, configuration change or workspace cleanup is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source inspection, code execution and GitHub tools. The exact serving variant 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 - [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> |
||
|
|
3f1ad6b25b |
fix(chat): resume task Retry after retained-source containment (#17317)
## Thinking Path > - Paperclip manages agent work through task and execution ownership. > - A task Retry must retain the user's request while the system verifies an old executor. > - Connected conversations also retain their external principal and accepted source batch. > - Chat Retry admission required containment before it could save a wait for that recovery. > - A saved chat receipt could also remain deferred after containment became available. > - This pull request lets the exact recovery-bound request wait and resume through its chat owner. > - Normal admission still checks containment, current access and policy before one successor starts. ## Linked Issues or Issue Description **What happened?** An ordinary task Retry on a connected conversation failed with `failed_chat_retry_not_authorized` while its retained remote executor still needed verification. The new recovery generation existed, but the chat action failed before it could save a dispatch wait. This occurred after the repairs in #17267 were confirmed live. The local regression found a second gap. A processed chat action with a deferred wake was not reconsidered after containment. Re-entering normal admission also tried to insert its existing durable receipt again. A historical hold for the same failed run could also hide the current Retry's workspace strategy, so the request still waited after the old executor was contained. Preparation could then read that sibling instead of the recovery record that owned the fresh successor, losing the fresh-workspace decision. **Expected behavior** Save the exact authorized Retry while its matching recovery generation verifies the executor. Once containment is recorded, reconsider the same chat receipt through normal admission. Start one successor with the original source batch. Keep publication, principal, pause and budget checks. **Steps to reproduce** 1. Admit a GitHub or Telegram request and record a failed native run with a retained remote lease. 2. Leave the executor's termination unconfirmed and exhaust its prior recovery generation. 3. Use ordinary task Retry to create a new recovery generation. 4. Process the connected Retry. The old admission code fails instead of retaining a wait. 5. With a deferred receipt, record exact containment and reconcile again. The old dispatch path leaves it stranded. **Paperclip version or commit** Reproduced from `412914cfa78d5abefcc34d3eb8d4bb86e461caac`. The branch is rebased onto current master. **Deployment mode** Hosted tenant for the reported failure. Isolated local PostgreSQL and the assembled heartbeat/chat services for the regression. Refs #17267 and #16965. This change does not add a Cloud dependency or change the absence protocol. ## What Changed - Permit a saved admission wait only for the exact current task Retry and active source recovery generation. - Keep promotion and execution dependent on independent containment proof. - Bind a Retry to active automatic recovery without replacing its generation, attempt count or deadline. - Share the current exact Retry strategy across historical holds for the same run. Retain each hold's stop and effect checks. - Select the canonical recovery record that proves the committed fresh successor during workspace preparation. - Let task receipt reconciliation return the same deferred action to its chat owner. - Recheck current board and external authority, then update the existing wake atomically with its successor. - Preserve the exact chat source batch. Keep generic board-comment filtering unchanged. - Add GitHub and Telegram cases for waiting, containment, one successor, stale generations, source edits, endpoint pause and supersession. - Update recovery documentation and capability maps. ## Verification - Red: real PostgreSQL tests reproduced the admission rejection for GitHub and Telegram. Six active-recovery cases then failed on the generation-ID assumption. Two same-run historical-hold cases remained deferred after containment on the old strategy lookup. A preparation regression then returned no readiness when the historical hold was stored first. - Green: 73 selected chat integration tests and 394 continuation, task Retry, workspace recovery, recovery-state and workspace-preparation tests passed. The chat tests use the real chat admission and heartbeat queue. Unrelated capacity stays occupied so no real provider starts. They assert no executor before containment, one successor afterward, unchanged source leases while waiting, preserved recovery budgets, and denial after authority changes. A separate source hold still blocks work. Both sibling insertion orders pass through the committed-readiness lookup and real workspace preparation: the new directory has independent Git storage and files, and unpublished old work remains intact. - `pnpm -r typecheck`, `node scripts/check-maps.mjs` and `git diff --check` passed after the review fixes. The source build passed on `f6660efa48e37a80cd9857217a537606de85b5c8`. - The local host reached its PostgreSQL semaphore limit. Successful runs used a test-only loader to reduce disposable cluster connection and worker limits. No repository database helper or application setting changed. - The full local `pnpm test:run` began before the final review edits, failed in its first server phase and was stopped after those failures. Later phases did not run. This is not a full local pass. The transient Retry-notice case passes after fixture isolation; the four Discord close-owned publication cases also pass in isolation. Other broad-run failure causes are not established. - Exact-head CI and security checks pass on `f6660efa48e37a80cd9857217a537606de85b5c8`; Greptile is 5/5 with zero unresolved review threads. Optional Paperclip Review completed neutral with a 5/5 static assessment of all 11 changed files and no findings; it did not run tests or hosted acceptance. All 57 current-head checks are terminal: 54 success, two skipped and the optional neutral review, with no failures or pending checks. - The real hosted workflow confirmed that the deployed absence repair recorded containment. A later ordinary UI Retry was accepted and saved a durable request, but it remains behind the historical source hold. The useful existing review remains unchanged. This follow-up is not deployed; a live successful successor and final task disposition remain unverified. No missing files are claimed recovered. ## Risks - The existing task reconciliation sweep can restage a deferred chat action. This reuses its receipt and generation; it does not grant new execution or publication authority. - Dispatch must keep the board Retry identity separate from the connected request's principal. The regression checks these bindings and preserves current chat authorization at each phase. - Historical holds reuse a strategy only from the locked same-run recovery that names the current Retry intent. A different run, malformed recovery record or independent source hold remains fenced. - Unknown executors without a matching active recovery still fail admission. Promotion and execution require containment even when admission saved a wait. - No schema or API shape changes. No credential changes, manual deployment or service restart. ## Model Used OpenAI GPT-6 through Codex, with repository tools, code execution and browser verification. The exact served model variant and context window 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 - [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> |
||
|
|
4c57da3708 |
fix(recovery): continue Retry after verified sandbox absence (#17267)
## Thinking Path > - Paperclip manages AI agents and their work. > - Task Retry must reconcile a stopped native execution before starting another one. > - A retained sandbox can disappear while workspace preservation is pending. > - A tenant-key error does not prove absence, especially after credential rotation. > - This pull request accepts only a short-lived Cloud observation bound to the exact source and current preservation owner. > - Normal Retry admission can then continue in a fresh workspace without claiming that missing files were recovered. ## Linked Issues or Issue Description **What happened?** A native run stops with workspace preservation pending. Its Daytona allocation no longer exists. Recovery tries to reopen it, then cannot confirm a stop. The ordinary Retry remains deferred behind cleanup and repeats a transient failure. **Expected behavior** Cloud must independently confirm absence in the recorded provider organization. Core must verify that proof and settle executor containment under the current physical owner. Normal Retry admission must enforce all existing gates and create at most one successor. The old run must remain failed and missing files must remain unavailable. **Steps to reproduce** 1. Persist a failed native run with a retained Daytona lease and an exhausted workspace-recovery generation. 2. Remove the exact provider allocation so reopen and stop cannot confirm it. 3. Request ordinary task Retry. 4. Observe that recovery reports `provider_unavailable` and leaves the Retry deferred. **Paperclip version/commit** Reproduced on source based on `6d44b80a5edbcc9f69bfc3d520994e70aa9fab6d` with the prior preservation and Retry repairs present. **Deployment mode** Cloud tenant with native Runner and a retained Daytona source. The local acceptance uses real Core, Cloud, PostgreSQL, signatures, native Runner and UI, with deterministic model and provider HTTP boundaries. Refs #16962, #16965, #17053. These repairs cover completion proposals, durable Retry and retained-source reopening. They do not establish absence of a missing provider allocation. Related #16809 preserves orphan sandbox leases; this change requires independent proof before settling an absent allocation. ## What Changed - Add an HTTPS client for a purpose-separated, host-bound Cloud sandbox-absence capability. - Verify signature, audience, stack, expiry, source identities and current physical-owner token. - Settle only executor absence with a metadata and owner compare-and-set. Preserve source protection and record that workspace contents are unavailable. - Leave successor creation, approval, pause, budget and uncertain-effect checks to normal Retry admission. - Document the protocol and update the recovery maps. ## Verification - Red: the new persisted-state regression returned `provider_unavailable` and scheduled recovery on the unfixed code. Expected: `source_unavailable` with a contained old execution. - Green: 384 focused tests pass across workspace recovery, signed absence, explicit continuation, ordinary Retry, workspace preparation and provider-owned fresh workspaces. - Signature tests reject forged, expired and mismatched proofs. HTTP failures or missing configuration grant no authority. - Two missing allocations are both reconciled before Retry. A simulated interruption reuses the first durable absence receipt. - Concurrent Retry admission produces one queued fresh successor. It records no successful old result and no recovered-file claim. - Browser acceptance at this change: one ordinary Retry, two provider absence reads for two retained sources, one fresh native execution, useful confirmation and Done after reload. The old run remains failed. Existing saved review and newer local file bytes remain unchanged. The deterministic model does not perform external publication; this is local protocol acceptance, not live provider recovery. - `pnpm typecheck`, `pnpm build`, and `node scripts/check-maps.mjs` pass. - A complete local `pnpm test:run` pass was not obtained. The first run hit a 15-second timeout in an unchanged status-card test. It was stopped to apply review fixes before the remaining phases ran. The unchanged status-card suite then passed 25/25 in isolation with the repository short temporary-path convention. - CI exposed a chat nonduplication assertion that compared unordered database rows. The assertion now sorts both complete row sets by run ID. Its focused test passes. - All 58 current-head checks are terminal on `1e2e881f6b6b1ae1fbfe1f2b2922657ce46e1470`: 55 success, two unaffected Storybook skips, and one neutral advisory review. Required CI, build, E2E and security checks pass. Greptile is 5/5 with no unresolved threads. The optional Paperclip review is neutral advisory 5/5 with nine-file source coverage and no findings; it did not independently run tests. - One unchanged native-runtime test timed out at five seconds on its first CI attempt. Its suite passed 33/33 in isolation; a single retry of that lane passed without code changes. ## Risks - The companion Cloud endpoint must ship before this consumer. Older or unavailable Cloud versions return no proof and retain the current hold. - A valid absence receipt permits a fresh workspace attempt. It cannot restore missing remote files, establish task success or authorize repeating an uncertain external effect. - This PR has not been deployed. The original task remains unrecovered pending rollout and reconciliation of its existing Retry. - No schema, credential, provider-resource or fleet changes are included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source inspection, code execution and browser tools. The exact backend variant and context-window size are not exposed by 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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (focused scope above; full local run limitation recorded) - [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> |
||
|
|
eaacadeba3 |
fix(recovery): let preservation owners reopen retained sources (#17053)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery must save retained workspace files before ordinary execution can resume. > - The environment runtime blocks access to retained sources to prevent unsafe reuse or deletion. > - The recovery worker also used that ordinary resume path. > - The guard returned no lease, so recovery reported an invalid source binding before it could save files. > - This pull request gives the physical preservation owner a separate, source-bound resume path. > - Ordinary execution and destructive cleanup keep their existing guards. ## Linked Issues or Issue Description Refs #16965. **What happened?** A retained workspace with a workspaceRestoreRecovery marker could not recover through the real environment runtime. The runtime returned null before calling the provider. Recovery classified this as source_binding_invalid. Earlier recovery fixtures replaced the runtime facade, so they did not exercise this guard. **Expected behavior** The verified preservation owner can reopen its exact stopped allocation to save files. Other callers remain blocked. Recovery preserves newer local files and does not start another agent or repeat a completed external action. **Steps to reproduce** 1. Seed a terminal source run with a retained sandbox, a recovery action, and the workspaceRestoreRecovery lease marker. 2. Give the source a verified stop receipt. 3. Call recoverRetainedWorkspace through environmentRuntimeService. 4. Before this change, the recovery action returns to blocked with source_binding_invalid. No files are copied. **Paperclip version or commit** Reproduced on master at |
||
|
|
d33b40a5c3 |
fix: recover saved work and provide reliable task Retry (#16965)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The control plane must retain work when an execution or workspace export fails. > - Retained-workspace holds can leave saved input waiting with no usable Retry action. > - Users cannot verify process ownership or repair provider allocations. > - This pull request gives system recovery a durable budget and gives each deliberate Retry a durable intent. > - Safe recovery can preserve files, settle an accepted result, or prepare an independent workspace. > - One task state keeps infrastructure diagnostics out of the ordinary conversation while preserving real permission and approval gates. ## Linked Issues or Issue Description **What happened?** A task with a retained workspace can show recovery, failed-run and saved-message warnings at the same time. Historical runs without native workspace descriptors cannot use the native restore path. Old recovery metadata can prevent a new attempt even when an independent workspace would be safe. **Expected behavior** Paperclip owns bounded infrastructure retries and file preservation. A user can request another attempt with ordinary Retry. Thinking appears immediately. The service preserves completed effects and current user controls. It does not claim that starting fresh recovered the old files. **Steps to reproduce** 1. Retain a failed run and its workspace after export or process-stop failure. 2. Save a follow-up while the recovery hold is active. 3. Open the task and try to continue without infrastructure credentials or operator certification. Refs #16906, #16835, #16883. This extends their recovery and receipt contracts. Related work: #16962 and #16963. ## What Changed - Add task-level Retry with company-scoped authorization, all-status idempotency and durable admission before physical work. Keep exact-run Retry compatible. - Add typed recovery phases, finite persisted backoff, historical adoption and durable local failure diagnostics. - Preserve native workspaces or verified historical snapshots. Admit an independent fresh workspace only after execution containment. Keep old-source preservation and cleanup obligations separate. - Carry completed publication and API receipts forward. Fence uncertain mutation retries across replacement runs. Preserve connected-chat identity and authorization. - Re-enter normal dispatch for saved input. Reuse accepted results through the existing finalizer. - Use one task interruption projection across the thread, composer and task list. Show optimistic Thinking, ordinary Retry and genuine user gates. - Add PostgreSQL, filesystem, API, browser-fixture and controlled provider coverage. Document the behavior and update the feature/access maps. ## Verification - All GitHub Actions CI jobs pass on `6d8c6d27d50586d8548b31d03b47f6d73a0ee11d`: typecheck, all server/chat/workspace and serialized suites, Runner checks, eight browser shards, canary and build. The required merge gate passes. - Greptile reviewed the final commit at 5/5, with no actionable findings or unresolved review threads. Its two earlier P1 findings are fixed. - Full recursive typecheck and build pass locally on the final commit. All 16 task Retry cases pass against isolated real PostgreSQL. The unrelated-success case and both stale Retry cases fail on the previous code and pass after the fix. - The corrected gateway fixture passes its uncertain-mutation and independent-read case. Four real HTTP API cases pass for definite 403/422 rejection, completed-effect reuse, and uncertain-effect fences across deliberate and automatic retries. These focused gateway/API checks used a temporary database lifecycle harness on an existing healthy local PostgreSQL process. The standard repository helper is unchanged. - Earlier qualification covers 291 task UI cases, saved-file copyback and ancestry, current Agent Chat ownership races, nine chat Retry API browser cases, and six assembled browser recovery fixtures. A real API/PostgreSQL/native-runner journey showed Thinking in 25 ms despite a delayed response, admitted one successor in a separate workspace, retained original files, and survived reload. - The controlled Daytona test interrupts export, joins the controller, restarts it in a separate process, and verifies one agent start, one external sentinel operation, retained file contents and no premature cleanup. It does not establish abrupt-kill or live LLM/GitHub acceptance. - The full local `pnpm test:run` was attempted and was not green: an OAuth row-lock timing assertion failed, and a later suite exhausted macOS PostgreSQL semaphores. The ten OAuth timing cases passed on rerun against a separate database. Later embedded database initialization failures remain recorded; the final CI matrix passes. - Security scanning passed on production-code commit `63953edf069b111c6becc3144dffa6deb52f68b5`. Its only successor changes one test-fixture identity line. The final scan is neutral/inconclusive because its pinned merge-base checkout failed or timed out. Both rerun API requests returned 404, so no fresh successful scan is claimed. Socket and Snyk checks pass on the final head. - The optional Paperclip Review remains queued behind an earlier run in workspace finalization. Its prior completed assessment was incomplete because a comparison tool was blocked by the platform filter. It is not counted as a completed code review. GitHub does not require this check for merge. - Hosted acceptance remains pending deployment. Local and CI results do not establish recovery of a hosted task or its retained files. ## Risks - Recovery changes span admission, physical ownership and dispatch. Incorrect evidence could permit overlapping work. Existing physical locks and authoritative stop requirements remain in place. - The migration adds concurrent indexes. It requires the existing top-level migration runner and retries incomplete index creation safely. - A fresh attempt can complete while old-source preservation remains unresolved. The service retains that obligation and does not report those files as recovered. - Unknown tool mutations remain blocked until their authoritative receipt owner resolves them. Generic upstream tools have no universal result-query protocol. - Recovery and diagnostics stay inside the stack. No Cloud incident endpoint, notification worker or new outbound reporting channel is included. ## Model Used OpenAI Codex, GPT-6 family. The runtime does not expose a more specific model version or context-window size. The model used reasoning, shell execution, source editing and browser tools. No model subagents were used for this implementation. ## 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 (focused suites; the full local run limitation is 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> |