mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
2eede8b3183f877f9afbf06655aa1eee07590d28
2860
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude
|
2eede8b318 |
fix(native): give a relaunched settled runner its admitted provider pack (#20683)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - After a server restart or a same-run retry, a remote native run reattaches to its runner in the sandbox (`reattach_remote_runner`). > - When the controller saved the runner artifact, the executor prepares that run as an adoption. It skips provider-pack selection, because a live adopted runner keeps its launched provider. > - The reattach probe can instead find the runner settled and exited (`verifyRemoteRunnerOrSettledExit`). Then nothing is adopted, and the transport relaunches the runner from its durable state. That relaunch must start the provider again, so it needs the provider-pack launch inputs. > - The executor did not give them. OpenCode failed with "OpenCode executable is missing from the provider pack". ACPX failed with "ACPX launch profile omitted its provider-pack authority or verified artifact digests". Both ended the run. > - This pull request selects the saved (admitted) provider-pack authority on that settled-exit relaunch and verifies it as an exact installed identity, as an ordinary reattach does. > - The benefit is that a restart or a same-run retry can resume an exited runner with the same provider pack, instead of failing. ## Linked Issues or Issue Description **What happened?** On a hosted staging instance, two OpenCode runs and one ACPX (Claude) run were interrupted by a deploy (SIGTERM) or ended a turn without a result. The server scheduled a same-run retry that reattached to the existing remote runner. The runner had exited cleanly (`runner_remote_process_exited lifecycle=suspended`). The retry then failed within three seconds with `runner_remote_provider_artifact_incompatible`. **Steps to reproduce** 1. Prepare a remote OpenCode or ACPX run with a persisted runner artifact (`controllerPersistence.runnerArtifact`). 2. Reattach with `restartRecovery.kind = "reattach_remote_runner"`, where the runner process is not running and its state is `suspended` and settled. 3. Observe that the transport options have no `adoptExistingRunner`, no `providerPackAuthorityDigest` and no `opencodeCommand` / `acpxSidecarSha256`. **Expected behavior** The relaunch gets the admitted provider pack's launch inputs, or it fails with a clear pack-identity error. ## What Changed - `createRunnerdBackend` (native session executor): when a persisted-runner reattachment finds no live runner to adopt, select the saved provider-pack authority (`reattach: true`), so `expectedProviderPackManifest` is the admitted pack. - `prepareRemoteRunnerAssets` no longer returns early for that relaunch. It verifies the installed pack as an exact identity and sets the active pack root, the same path as a reattach without a persisted artifact. - A live adopted runner is unchanged: it still skips pack selection. - The test `uses the bundled Linux image authority for remote opencode|claude-x64` now also covers the settled-exit relaunch for OpenCode and ACPX. ## Verification - RED on `origin/master` (`c444da7d05`): the extended test fails for `opencode` and `claude-x64` (the relaunch options have no provider-pack inputs). - GREEN with this change: `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-session-executor.test.ts -t "uses the bundled Linux image authority"` (14 passed). - `pnpm --filter @paperclipai/server typecheck` passes. - `pnpm verify`: see the result below. ## Risks - Upgrade safety: the relaunch reads the saved authority, so a newer controller does not select a newer pack for an older session. It requires an exact installed identity. If the sandbox no longer has that pack, the relaunch fails with a pack-identity error, and the existing Retry path applies, as before. - No change to a live adoption, to fresh runs or to the normal path before `turn.start`. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, read-only staging API reads). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude
|
e33ede9a8a |
fix(native): settle a restart-orphaned write_document from its transactional receipt (#20674)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run calls Paperclip tools such as `write_document`. The runner saves each tool input durably before the server runs the tool, and the server saves a result receipt > - A control-plane restart can cut a `write_document` call after the runner saved its input and before any result was saved > - Recovery then has no proof of the outcome. A call from an earlier server process settles as "unknown outcome" (#20628). A call the current server process admitted stays pending, close cannot prove that the provider drained, and the session quarantines (`native_session_cleanup_quarantined`). A person must click Retry > - `write_document` saves its revision and its idempotency receipt in one database transaction, under the run and issue row locks. The revision is also compare-and-swap on `baseRevisionId`. So the receipt is a durable outcome record: present means applied, absent means not applied > - This pull request makes recovery use that proof. Recovery runs the same receipt-guarded write. It replays the saved result, or it applies the write exactly once > - The benefit is that an orphaned `write_document` settles with its real result. The document is never written twice, and the run does not need Retry for this call ## Linked Issues or Issue Description **What happened?** Daytona restart-chaos wave `w-20261110T174930-s9205`, task t01 (codex, workload C1, fault RR2). A restart cut a `write_document` call. The close settlement showed `semanticTools.pending: [{ operationId: "write_document", callId: "exec-8b79ab78-…" }]` and `providerDrained: false`. The session quarantined with `native_session_cleanup_quarantined`, and the task needed Retry. Since #20628, a call that an earlier server process admitted settles as `tool_call_interrupted_by_restart` with an unknown outcome. The agent then has to guess whether the write applied. **Expected behavior** When a durable record proves the outcome, recovery uses it. A write that applied returns its saved result. A write that did not apply runs once. When the record cannot prove it, the call stays as it is today. **Steps to reproduce** 1. On master, run `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/paperclip-runner-tool-authority.test.ts -t "orphaned"`. 2. `authority.recover()` returns `null` for a `write_document` that has no receipt, so the call stays unsettled. Refs #20628, #20639, #20507 ## What Changed - `PaperclipRunnerToolAuthority.recover()` now settles `write_document` through its transactional receipt (`#recoverTransactionalWrite`). It runs the same receipt-guarded mutation as an ordinary call. If the receipt exists, it returns the saved result and writes nothing. If no receipt exists, it applies the write once. - A definite refusal that rolls the transaction back (a 4xx such as a stale `baseRevisionId`, a reused idempotency key, invalid input) settles as a tool error. Any other failure leaves the call unsettled, as before. - The set of operations with this proof is explicit: `TRANSACTIONAL_RECEIPT_OPERATIONS` contains only `write_document`. Other tools keep their current recovery rules. - New test: an orphaned `write_document` with no receipt applies once; a second recovery and a late original call replay the receipt; an applied write recovers its saved result; a stale base settles as a tool error with no change. - Updated test: a reused idempotency key with other input now settles as an `paperclip_runner_tool_idempotency_conflict` tool error instead of staying pending. The receipt proves that this call can never apply under that key. - Docs: `packages/paperclip-runner/docs/durable-recovery.md` (semantic operations that outlive a turn) and a new design note, `doc/plans/2026-11-10-durable-turn-start-and-tool-call-records.md`. The note also covers the second change (an ACPX turn start cut off before the prompt reached the provider), which comes in a separate pull request. ## Verification - Red on master, green here: `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/paperclip-runner-tool-authority.test.ts` (43 passed). - `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts`: 178 passed. One unrelated ACPX timing test failed once under load and passed when run alone. - Server typecheck: `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2 (a runner doc changed), so required CI supplies the broad coverage. - Live restart-chaos wave: see the comment on this pull request. ## Risks - Recovery can now apply a write that never applied before the restart. This is the call the agent made, applied once. The write is compare-and-swap on `baseRevisionId`, so it cannot overwrite a revision that someone wrote after the agent read the document. That case settles as a conflict error. - Upgrade safety: no saved shape changes. Receipts saved by the previous release have the same shape. A server on the previous release keeps its current recovery for these calls. - Time to first token: no change. This code runs only during recovery. - No runner source changes, so no new runner image is needed. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file editing) in Claude Code. Extended reasoning was used. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude
|
cec903c6f0 |
fix(native): retire a finished remote owner whose gone Runner left a delivery unacknowledged (#20677)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native follow-up run on a remote sandbox first proves that the previous per-turn owner finished. Then it retires that owner's controller journal and starts a new Runner. > - The proof requires a settled journal. A command that the Runner did not acknowledge holds the owner. > - On staging, a per-turn owner committed its result and its Daytona sandbox was destroyed after the turn (`paperclip_runner_destroy_after_turn`, cleanup `success`). Its journal still held a `run.attach` or a `semantic_tool.result` that the Runner never acknowledged. > - The next run failed with `prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(run.attach.pending)` (or `semantic_tool.result.pending`). A Retry or a new message gets the same journal, so it fails the same way. The task stays blocked. > - Retirement already excuses the Runner's own lifecycle commands when the prior sandbox lease ended on another sandbox. This pull request applies the same rule to deliveries that only that gone Runner could receive. > - The benefit is that these follow-ups start by themselves. Commands that carry user intent still hold the owner. ## Linked Issues or Issue Description **What happened?** A census of staging failures (48 h, Nov 9–10) found 6 failed runs with `prior_owner_terminal_state_indeterminate` and `retirement_refused=journal_commands_unsettled(run.attach.pending | semantic_tool.result.pending)`. The last one was at 18:56 UTC on Nov 10. In two examples that I checked: - The previous run succeeded. Its result was committed and its coordinator lease was released. - Its lease policy was `ephemeral`. The lease is `expired`, with `failureReason=paperclip_runner_destroy_after_turn` and `cleanupStatus=success`. - The follow-up got a new sandbox (a different provider lease) and refused in about 11 s. It started no Runner. The task became `blocked`, and the next action was "Send a message to continue". **Expected behavior** When the record proves that the previous Runner is gone, an undelivered attachment, tool answer or read cannot run. The follow-up retires the finished owner and starts a new Runner. Retirement replays no command. **Steps to reproduce** 1. Finish a per-turn remote native run whose controller journal keeps a `run.attach` or `semantic_tool.result` command in `pending`. 2. Let the sandbox lease of that run end (`expired` or `released`) on its own sandbox. 3. Start a follow-up run that gets a new sandbox. 4. Observe `retirement_refused=journal_commands_unsettled(run.attach.pending)`. **Paperclip version or commit** Staging builds of Nov 10. The refusal still reproduces on `master` at `48c6e996da` (the new tests fail there). ## What Changed - `native-session-executor.ts` (`readSettledRunnerdProjection`): when the prior run's sandbox leases ended on another sandbox (`remoteRunnerGone`, the existing proof), unfinished `run.attach`, `semantic_tool.result`, `session.snapshot` and `session.goal.get` commands no longer hold the retirement. These join the lifecycle commands that the rule already excused. - In the same case, a journaled `semantic_tool.result` answers its `semantic_tool.input`. The controller journals the answer after the tool ran. Only its delivery was outstanding. - No other proof changes. The prior run must still be terminal, with a committed result, a released coordinator lease, no recovery state, a stopped or fenced controller, a delivered `run.terminal`, and no live work after it. Quarantine copies must pass the same checks. - `doc/execution-semantics.md`: describe the rule. ## Verification - `server/src/services/native-runtime/native-recovered-owner-followup.test.ts` (real Postgres): - New: "retires a finished remote owner with an unacknowledged pending-attach / pending-tool-result when its sandbox lease ended on another sandbox". It fails on `master` with the exact staging errors `journal_commands_unsettled(run.attach.pending)` and `journal_commands_unsettled(semantic_tool.result.pending)`. It passes with this change, and the log shows the retirement. - Held cases stay held: a lease that is still retained, a lease on this run's sandbox, no lease record, and a tool input with no journaled answer (`journal_tool_input_unanswered`). - New: "keeps a stopped local owner held by an unacknowledged run.attach". A local Runner is not gone by record. - Whole file: 95 passed. - Neighbouring suites pass: `explicit-native-continuation.test.ts` and `native-controller-recovery.integration.test.ts` (460 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2: a global CI input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Live proof: not done. The redeploy chaos scenario (K15) is not on `master` yet. Tests only. ## Risks - The rule applies only when every sandbox lease of the prior run is `released` or `expired`, has a release time, and names a different provider lease than this run's. A reusable sandbox that this run resumed keeps the existing hold. - A command that carries user intent (`turn.start`, `turn.steer`, `request.resolve`, `interaction.receipt`, `external_provider.operation`, `session.goal.set`/`clear`, `session.close`, `run.cancel` and others) still holds the owner. - Retirement archives the old journal under `continuity-breaks` and replays nothing, as before. - No persisted shape changes. Old saved journals are read by the same rule. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
ad0543ae90 |
test(recovery): re-arm a capture-policy save whose finished coordinator names a lapsed owner (#20672)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Workspace recovery re-arms a save that an older capture policy exhausted (#20569), so the files are saved without a person > - On staging, one save of this kind was not re-armed after the deploy. Its export hold also carries a workspace finalization block, and it has a resolved no-replay sibling for the same run > - #20661 found that discovery skipped a hold when the run's finished coordinator still named a lease owner whose lease had lapsed. The same discovery clause gates the capture-policy re-arm > - This pull request adds a regression test for that exact capture-policy shape. It shows that #20661 alone makes the worker re-arm and save it > - The benefit is that the capture-policy path keeps this guarantee in later changes ## Linked Issues or Issue Description Refs #20661 Refs #20569 **What happened?** An exhausted capture-policy save (`export_interrupted`, cause `provider_error`, 6 of 6 attempts, phase `preserve_files`) was not re-armed on the worker's first pass after #20569 was deployed. **What did you expect to happen?** The worker re-arms the save once under the current capture policy, saves the files, and returns the hold to the system. **Steps to reproduce** 1. Exhaust a save under capture policy 1 with `export_interrupted`. Add a workspace finalization block and a resolved no-replay sibling for the same run. 2. Leave a lapsed `native-finalizer:` lease owner on the run's `terminal_failure` coordinator. 3. Run the workspace recovery worker. Before #20661, `nextWorkspaceRecoveryAt` returns null and the hold is never selected. ## What Changed - `workspace-recovery-capture-policy.test.ts`: add "re-arms a save whose finished coordinator still names a lapsed finalizer owner". The test uses the staging record: export hold with `historicalWorkspaceSource` and `workspaceFinalizationBlock`, a resolved no-replay sibling, waiting user input, and a lapsed finalizer owner. It drives the worker's own discovery loop until the worker is idle. - No production code changes. ## Verification - Red: with the source files from before #20661 (`git checkout 686893636d^ -- <the four source files>`), the new test fails. `nextWorkspaceRecoveryAt` returns null, so the worker is never woken. - Green: on current master, `pnpm --filter @paperclipai/server exec vitest run src/services/workspace-recovery-capture-policy.test.ts` passes (5 tests). The file passed 4 runs in a row. - The test asserts that the save completes under policy 2, the hold returns to the system, `releasedLapsedOwner` is recorded, and the lapsed owner is released. ## Risks - Low risk. This change adds a test only. - Previous-release story: the test uses rows in the shape an older release saved, with no capture-policy marker. No migration is needed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, browser GET reads) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
e9db5e0d03 |
perf(github): read the inherited review waits of an issue list in one pass (#20673)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - An agent can review a GitHub pull request. After the agent publishes the review, the task can wait in review for the next GitHub reply. A later run of the same task can inherit this wait. > - Issue lists and inbox attention show these waits. For each waiting task, the server reads the inherited GitHub review wait: the source comment, the latest review, and the review publication. > - The server did these reads one time for each waiting task. A list request with N waiting tasks sent N copies of the prepared review statement and N copies of the publication read. > - On a hosted database, the prepared review statement ran about 270,000 times in about 2.5 hours (up to 87 calls per second). The run identity read before it ran the same number of times. The list read that calls it ran only about 1,400 times, so each list read started about 190 review reads on average. No other caller reads the waits of many runs in a loop. > - This pull request reads the reviews and the publications of all waits of one list request with one statement each. Each wait gets the same result as before. > - The benefit is about 200 times fewer review statements and fewer pool connections for each issue list. ## Linked Issues or Issue Description Refs #20400 (shares review-wait reads between the callers of one request). Refs #20551 (authorizes the waits of one issue page with one read; this change does the same for the inherited GitHub review reads of that page). **What happened** `listExternalReviewWaits` called `readInheritedGitHubReviewWait` one time for each candidate wait, inside `Promise.all`. Each call sent a run identity read, a source-message read, the prepared `inherited_github_review_wait` statement, and a publication read. A list with N waits sent each statement N times. **Expected behavior** One list request sends one review statement and one publication read for each company. The result of each wait does not change. **Steps to reproduce** 1. Put many tasks of one company in review with an inherited GitHub review wait. 2. Open the issue list or the inbox. 3. Count the statements of the database. The review statement runs one time for each waiting task. ## What Changed - `server/src/services/chat-channels/github/review-wait.ts`: - Split `readInheritedGitHubReviewWait` into shared steps: the source comment read, the review checks, the publication key, and the published result. The single-run read uses the same prepared statement and the same checks as before. - Add `readInheritedGitHubReviewWaits(db, runs)`. It reads the source comment of each run with the same lineage check as before. Then, for each company, one `DISTINCT ON` statement reads the latest review for each (source comment, task, agent) key, with the same joins, filters and order as the prepared statement. One read finds the processed publications. Each run gets the same result as a separate call: `null`, `revoked`, or `published`. - `server/src/services/native-runtime/external-review-wait.ts`: `listExternalReviewWaits` uses the batch read. The rest of the list and its authorization do not change. - `server/src/__tests__/github-review-wait.test.ts`: a new test reads 18 runs in one batch and compares each result with a separate single-run read. The runs include each revocation case (connection disabled or inactive, review state, review agent, repository, repository id, thread, publication, identity, company), a newer unreviewed head, no identity, no task, an endpoint of another agent, a run in another company and a duplicate run. The test also makes sure that the batch sends one review statement and one publication read for each company. The existing revocation tests also check the batch read. - `server/src/__tests__/helpers/github-review-wait.ts`: the seed helper can add a wait to an existing company and agent. ## Verification - `pnpm vitest run server/src/__tests__/github-review-wait.test.ts server/src/__tests__/github-review-check-state.test.ts server/src/__tests__/issue-list-review-wait-authorization.integration.test.ts server/src/__tests__/blocked-inbox-review-wait-scope.test.ts` - Focused local run of every test that imports the changed modules or the seed helper (7 files, 335 tests passed), including `external-chat-wait.integration.test.ts`, `external-review-wait.test.ts` and `legacy-execution-recovery.test.ts`. The change to the shared seed helper makes `pnpm verify` select the full server suite, so CI supplies that coverage. - Local measurement on real PostgreSQL (embedded PostgreSQL, `ANALYZE` after seeding). One company and agent, each wait has 5 reviews and 10 outbound messages. "Before" is the previous list code: one `readInheritedGitHubReviewWait` call for each run in `Promise.all`. "After" is `readInheritedGitHubReviewWaits`. The runs alternate order in each pair. Both return equal results in every sample. | Waits | Version | Statements | Review statements | Median ms | Min–max ms | Samples | | --- | --- | --- | --- | --- | --- | --- | | 200 | before | 800 | 200 | 106.8 | 98.0–117.5 | 10 | | 200 | after | 402 | 1 | 44.3 | 41.6–50.9 | 10 | | 20 | before | 80 | 20 | 9.8 | 8.6–14.7 | 12 | | 20 | after | 42 | 1 | 6.3 | 5.3–7.0 | 12 | - `EXPLAIN (ANALYZE, BUFFERS)` of the batch review statement for 200 waits: index scans on each join, 6.7 ms execution. The single statement: 0.05 ms execution and about 2.7 ms planning for each call. - The run identity read and the source-message read still run one time for each wait. They are cheap on the hosted database (about 0 ms each), and they keep the lineage check for each run. ## Risks - Low risk. The batch statement uses the same joins, filters and order as the prepared statement, and `DISTINCT ON` keeps the first row for each key. The array filters can also match a mix of two keys. The code uses only rows of exact requested keys. - The single-run read and its prepared statement do not change. The finalizer, the status committer, and the attachment reuse paths still use it. - Errors do not change: a 403 from the lineage check gives `revoked`, and other errors are thrown. - No migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (code reading, editing, shell, tests). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
c444da7d05 |
perf(run-events): stop blocking checkpoint FK checks during event appends (#20670)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run sends a stream of runner events. The server appends each event to the run log. The runner controller then saves a checkpoint for recovery. > - Each checkpoint save inserts the new `event:N` row and updates the `cursor` row in `native_runner_checkpoints`. The new row has a foreign key to the run row in `heartbeat_runs`. > - The event append locks the same run row `FOR UPDATE`. That lock blocks the `FOR KEY SHARE` lock that the foreign key check takes. So each checkpoint save waits for the event append that is in progress. > - On a hosted database, the two-row checkpoint upsert had the largest total SQL time. The one-row (cursor-only) upsert, which has no foreign key check, was much cheaper. > - This pull request changes the append lock to `FOR NO KEY UPDATE`. Appenders still serialize. Child foreign key checks no longer wait. > - The benefit is shorter checkpoint saves, shorter waits before each runner ACK, and less lock wait on the database. ## Linked Issues or Issue Description **What existing behavior does this improve?** The controller checkpoint save for native runs (`nativeRunnerCheckpointWriter`), and the native run-event append (`appendHeartbeatRunEvent`). **Subsystem affected** Server run-event log and native runtime checkpoint persistence. **Current behavior** On one hosted database, Neon query performance showed this statement as the largest total: `insert into native_runner_checkpoints ... values (...), (...) on conflict (run_id, part) do update`. It ran 6,582 times at 62.6 ms (412 s) in the first 38 minutes after a compute restart, and 9,435 times at 31.8 ms (300 s) about two hours later. An earlier day showed 100,650 calls at 102 ms. The one-row form of the same upsert ran 965 times at 13.4 ms. The runner already writes only changed records (`captureCheckpoint` compares plaintext), so unchanged material is not rewritten. The two-row form is a new event row plus the cursor. Only the new row fires the foreign key check. `appendHeartbeatRunEvent` locks the run row `FOR UPDATE` for each native event, in a transaction with several statements. `FOR UPDATE` conflicts with `FOR KEY SHARE`, so the check waits for that transaction. **Proposed behavior** `appendHeartbeatRunEvent` locks the run row `FOR NO KEY UPDATE`. This mode still conflicts with other appenders, `FOR SHARE`, `FOR UPDATE`, deletes and updates. It does not conflict with `FOR KEY SHARE`. The append never changes key columns: `runUpdate` is limited to process fields, and the native and retry-exhaustion paths reject it. The fast path already uses a plain `UPDATE`, which takes the same lock mode. The execution-admission adapter already locks this row `FOR NO KEY UPDATE`. **Reason and benefit** The wait goes in both directions. A checkpoint save waits for an append. An append also waits for an open checkpoint transaction that holds `FOR KEY SHARE`. Both waits stop. The controller sends each ACK only after its checkpoint commits, so shorter saves also shorten the event loop. ## What Changed - `server/src/services/heartbeat-run-events.ts`: lock the run row `FOR NO KEY UPDATE` instead of `FOR UPDATE` in the transactional append path. Add a comment that names the contract. - `heartbeat-run-event-atomic-append.test.ts`: add a regression test. An append transaction stays open. A checkpoint row insert for the same run must finish under a 2 s `lock_timeout`. A second appender must still wait. After release, the sequence is 1, 2 with no gap and no duplicate. The existing test with 40 concurrent appenders on two connections still proves contiguous sequence numbers. Update the existing lock-statement assertion to the new clause. - Add `scripts/benchmark-run-event-checkpoint-lock.ts`. It replays a native event stream with the real append and the real checkpoint writer on separate connections. It times each checkpoint save for a baseline append module and the current one. ## Verification - Red: with the old `FOR UPDATE` clause, the new test fails with `canceling statement due to lock timeout`. With the change it passes. - `npx vitest run src/__tests__/heartbeat-run-event-atomic-append.test.ts src/__tests__/heartbeat-run-event-sequencing.test.ts src/services/native-runtime/native-runner-checkpoint-authority.integration.test.ts src/__tests__/native-recovery-notice-work.test.ts` (server): 4 files, 42 tests passed. - `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm verify` exits 2, because a new script under `scripts/` is a global CI input. Required CI supplies the broad coverage. Local benchmark. Embedded PostgreSQL 18.1 on a laptop. 400 events per pass, 2 KiB event payload, 6 KiB header, real encryption. Order: warm-up, baseline, current, baseline, current. `HOLD_MS` keeps each append transaction open longer. It stands in for hosted round trips. | Hold | Pass | Median save | p90 | Max | Saves > 2 ms | Total save time | | --- | --- | --- | --- | --- | --- | --- | | 5 ms | Baseline (1) | 8.16 ms | 9.50 ms | 129.48 ms | 396 / 400 | 3,551 ms | | 5 ms | Current (2) | 1.44 ms | 2.08 ms | 12.66 ms | 44 / 400 | 677 ms | | 5 ms | Baseline (3) | 8.12 ms | 9.64 ms | 22.08 ms | 398 / 400 | 3,453 ms | | 5 ms | Current (4) | 1.50 ms | 2.37 ms | 10.54 ms | 80 / 400 | 700 ms | | 0 ms | Baseline (1) | 1.71 ms | 2.50 ms | 11.05 ms | 105 / 400 | 780 ms | | 0 ms | Current (2) | 0.97 ms | 1.42 ms | 10.28 ms | 22 / 400 | 458 ms | | 0 ms | Baseline (3) | 1.49 ms | 2.02 ms | 6.45 ms | 41 / 400 | 667 ms | | 0 ms | Current (4) | 0.91 ms | 1.24 ms | 9.80 ms | 19 / 400 | 420 ms | - Event order: the controller frame chain awaits each event's append and its local commit, but not its checkpoint save. The ACK goes through `#sendJson`, which queues the save and does not block the next frame. Only semantic inputs and command results await the save. So the save for event N overlaps the append of event N+1. Streaming events (`item.delta`) also go through the same locked append, with no checkpoint save, so a save can also overlap them. The benchmark uses this order. - Control without overlap (`SERIAL=1`, each save finishes before the next append, 5 ms hold): baseline median 1.15 / 1.07 ms, current 1.03 / 1.00 ms. There is no material difference. The gain comes only from the removed lock wait. - Results are the same in every pass. The script asserts that all event rows, the header and the cursor are stored, and that the run-log sequence is contiguous. - Command: see the script header. It needs a baseline copy of `heartbeat-run-events.ts` from the base revision. ## Risks - Low risk. The SQL change is one lock clause. Appenders still serialize with each other, with `FOR UPDATE` and `FOR SHARE` lockers, and with updates and deletes of the run row. Run deletion and key changes take `FOR UPDATE`, so they still wait for an append, and an append still waits for them. - The only new concurrency is with `FOR KEY SHARE` lockers. In this code base those are foreign key checks of child inserts. The one explicit `FOR KEY SHARE` read is on `issues`, not on `heartbeat_runs`. A foreign key check only needs the run row and its key to exist. The append does not delete the row or change its key. - The hosted cause is inferred. Neon shows calls and mean time only. We did not read lock waits on the hosted database. The local proof shows the mechanism and the size of the effect. A new query-performance window after deployment must confirm the result. - Persisted run state: no saved run, session, event or checkpoint shape changes. State saved by the previous release carries forward unchanged. The lock mode applies only inside one transaction, so a deploy during a run needs no special case. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, local PostgreSQL). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
81c52e3666 |
perf(chat): find terminal GitHub responses by issue run indexes (#20671)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The GitHub chat channel posts a terminal response when an agent run for a GitHub request stops. A background sweep finds acknowledged response comments (`processed`, not final) whose issue is blocked or whose latest run is terminal > - For each candidate, the sweep reads the status of the latest run of the assigned agent for the issue. It matches runs with `coalesce(native_issue_id::text, context_snapshot->>'issueId')` > - No index has that expression. So PostgreSQL reads every run of the agent for each candidate, and it decompresses the large legacy `context_snapshot` of each run > - On a hosted test database, this statement ran 13 times at a mean of 11 s in a 38-minute window (149 s in total), with single runs of 16 s and 41 s > - This pull request splits the run match into its two disjoint branches. Each branch uses an existing issue index > - The benefit is that each candidate reads a few index entries instead of the agent's full run history. The returned rows do not change, and no migration is necessary ## Linked Issues or Issue Description **What existing behavior does this improve?** The discovery page query in `registerGitHubTerminalResponseWork` in `server/src/services/chat-channels/github/terminal-response-work.ts`. The query runs when the sweep reconstructs its candidates, and once or twice for each candidate check. **Subsystem affected** GitHub chat channel terminal-response recovery. **Current behavior** The `WHERE` clause has `issues.status = 'blocked' or (select r.status from heartbeat_runs r where r.company_id = ... and r.agent_id = ... and coalesce(r.native_issue_id::text, r.context_snapshot->>'issueId') = <issue id> order by r.created_at desc, r.id desc limit 1) in (<terminal statuses>)`. The planner can only use `heartbeat_runs_company_agent_started_idx`. It reads all runs of the agent, then evaluates the `coalesce` on each row. That decompresses the context of each legacy run. The cost is the number of candidates times the number of agent runs. **Proposed behavior** The subquery takes the latest run from two branches and then the latest of the two: - runs with `native_issue_id = <issue id>`, through `heartbeat_runs_company_native_issue_contract_id_uq` - runs with `native_issue_id is null and context_snapshot->>'issueId' = <issue id>`, through `heartbeat_runs_company_ctx_issue_created_idx` The two branches are disjoint, and together they are exactly the rows where the `coalesce` is equal to the issue ID. Both branches and the outer query use the same order (`created_at desc, id desc`). So the selected run is the same, including ties. **Reason and benefit** This statement was the largest single-statement cost in the hosted window after the earlier database fixes. A local reproduction is about 490x faster on the default fixture, and the gain grows with the agent's run history. **Breaking changes** None. The API, the candidate rules and the results do not change. ## What Changed - Rewrite the latest-run status subquery in the GitHub terminal-response discovery page as two index-backed branches (native binding, and legacy context without a native binding). - Add two integration tests for the sweep. In the first, newer runs of the same agent that do not bind the issue (one with a native binding to another issue, but a legacy context that names this issue) must not hide the stopped run. In the second, a newer active run with a native binding to the issue keeps the request waiting. - Add `scripts/benchmark-github-terminal-response-sweep.ts`. It captures the SQL that the real worker generates at a baseline commit and at `HEAD`, compares all rows, and measures both. ## Verification Cause and thesis. The cost comes from the `coalesce` run match, which has no index and makes each candidate scan the agent's run history. Thesis: if each precedence branch uses its own issue index, the cost of the page no longer depends on the agent's run count, and the rows stay identical. The benchmark confirms the thesis. Local SQL benchmark: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-github-terminal-response-sweep.ts --baseline-ref=7b33384446 ``` Fixture (synthetic): two companies, each with one GitHub endpoint and one assigned agent. Each issue has one GitHub response comment. The first N comments are acknowledged and not final; the others are final. Every tenth issue is blocked. Runs alternate between native binding and legacy context, and each run has a 4 KiB incompressible context. The fixture also has these edge cases: newer active runs with a native binding to another issue but a legacy context that names this issue, active runs that tie on `created_at` with the latest run (between native and legacy runs), and newer active runs of another agent. The benchmark compares every column of every candidate row (the page limit is lifted for this check). Embedded PostgreSQL 18.1 on an Apple laptop. The hosted database is Neon with different compute, so absolute times are local SQL measurements only. Each size has 2 warmups and then alternates the baseline and the treatment. | Size | Runs (both companies) | Candidate rows equal | Baseline median (range) | Treatment median (range) | Speedup | | --- | --- | --- | --- | --- | --- | | Small: 200 issues, 3 runs each, 20 candidates, 7 samples | 1,372 | 26 of 26 | 18.7 ms (17.7–20.8) | 3.8 ms (3.3–4.3) | 4.9x | | Default: 1,500 issues, 8 runs each, 150 candidates, 7 samples | 25,300 (156 MB) | 200 of 200 | 3,697 ms (2,875–4,032) | 7.6 ms (6.6–9.6) | 490x | | Large: 3,000 issues, 10 runs each, 400 candidates, 5 samples | 62,600 (382 MB) | 538 of 538 | 57,055 ms (48,343–66,079) | 31 ms (16–79) | 1,800x | Sizes are set with `BENCH_ISSUES`, `BENCH_RUNS_PER_ISSUE`, `BENCH_CANDIDATES` and `BENCH_ITERATIONS`. The timed query is the real first page (limit 100). Plan (default size): the baseline runs `SubPlan` 270 times. Each run is a `Bitmap Heap Scan` on `heartbeat_runs_company_agent_started_idx` that removes about 12,000 rows by filter, for about 8.2 million buffer reads in total. The treatment runs an `Append` of two `Limit` index scans (`heartbeat_runs_company_native_issue_contract_id_uq` and `heartbeat_runs_company_ctx_issue_created_idx`), for about 3,800 buffer reads in total. Tests: - `cd server && npx vitest run src/__tests__/chat-channels.integration.test.ts -t "terminal GitHub"`: 13 passed, including the 2 new tests. - Mutation check: when the legacy branch does not exclude runs with a native binding, the first new test fails. It passes with the fix. - `cd server && npx tsc --noEmit`: passed. ## Risks - Low risk. The change is one SQL subquery. The match is the same set of runs: `native_issue_id::text = <uuid text>` is equal to `native_issue_id = <uuid>`, and a null `native_issue_id` makes the `coalesce` use the legacy context. - The candidate set itself is not bounded by this change. Acknowledged responses that never become final stay in the candidate set. Each candidate is now cheap, but the count still grows with that history. - `readGitHubResponseRun` in `response-run-state.ts` uses the same `coalesce` match. The sweep calls it once for each candidate row, and other GitHub paths also call it. Each call is a separate, smaller statement. This pull request does not change it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Used through Claude Code with tool use (shell, file edits, and local PostgreSQL benchmarks). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
c14f2fe45d |
fix(native): read the scheduled-resume evidence the resume reconciler claimed (#20656)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #20645 lets a local same-run scheduled resume send the one same-conversation continuation for a turn lost on restore. It applies only when durable records prove three facts: the previous runner stopped, the attempt was admitted within the budget, and it is the same checkpointed conversation. > - It read the third proof from the coordinator's saved `failure_detail` while the phase was `retryable_failure`. > - In production, the resume reconciler (`claimNativeSessionResumptions`) claims the run first. It moves the coordinator to `observed` and clears `failure_detail`. So the executor never saw that evidence, and #20645 refused every real scheduled resume. > - The restart-chaos L7 wave showed this live. Codex B5 TM2 hit the exact path (`runner.drain` timeout, scheduled resume, `provider_turn_lost_on_restore`), sent no continuation, and needed Retry. > - This pull request makes the reconciler record that evidence durably when it claims the run, and makes the executor read it from there. > - The benefit is that #20645 works on the real path. The proof rule is unchanged. ## Linked Issues or Issue Description Refs #20645 **What happened?** Wave `w-20261110T212307-s9107` (L7, seed 9107, local, master plus #20645), task codex B5 TM2: - `native_session_recovery_failed: runner_local_connect_failed: PRP command runner.drain timed out`; - `retryable_failure` with `recoveryMode: "exact_checkpoint_resume"`, `recoverable`, `providerSessionEstablished` and `checkpointExists` all set; - `native.local_process_stopped`; - attempt 2 started; - `turn.failed` with `provider_turn_lost_on_restore` for the same turn ID that the checkpoint was running. No continuation was sent. The run failed with `native_provider_terminal_failed` and was preserved with `uncertain_provider_action`. **Expected behavior** The proven scheduled resume sends the single continuation. **Steps to reproduce** The new integration test below. The live path is timing-dependent (the L7 command in #20645). ## What Changed - `native-finalization-reconciler.ts`: when it claims a same-run resume, the reconciler appends one `same_run_resume` entry to `recovery_history`. The append is bounded like other history writes, and it runs in the same update that clears the failure. The entry copies `recoveryMode`, `recoverable`, `providerSessionEstablished`, `checkpointExists`, `successorRunId` and `replacementDenied` from the saved failure, with the controller boot and provider attempt. - `scheduled-resume-continuation.ts`: the evidence for the third proof now comes from either of two durable records: - the saved failure, when the executor claims a `retryable_failure` coordinator directly (unchanged); or - a reconciler claim: an `observed` coordinator whose last history entry is a `same_run_resume` entry naming the executor's own controller boot. The two other proofs are unchanged: the stop receipt, and an admitted attempt from 1 to 3. - `native-session-executor.ts`: passes the coordinator's last history entry and the controller boot to the decision. - `doc/execution-semantics.md`: describes the recorded evidence. Readers of `recovery_history` that look at the last entry do so only for restart claims, Stop recovery and shutdown suspension. Restart recovery's previous-claim lookup skips non-claim dispositions, so a `same_run_resume` entry does not change those paths. **Still refused** (tested): - every #20645 case; - the last history entry is not the resume claim, or there is no history; - the resume claim was recorded by another boot; - the claimed evidence is a bootstrap retry, has no checkpoint, or records a replacement; - a reconciler-claimed resume without a stop receipt. ## Verification - `native-scheduled-resume-continuation.integration.test.ts` now also runs through the real reconciler claim (`claimNativeSessionResumptions`) before `executePaperclipNativeSession`. On master the reconciler-claimed allowed case fails (`resumeInterruptedTurn` is false). With this change it is true, and the claim admits attempt 2. A reconciler-claimed resume without a stop receipt still passes false. - `scheduled-resume-continuation.test.ts`: decision cases for the reconciler-claim entry. - `native-session-resumption.test.ts` passes. 54 tests pass across the three files. - `cd server && npx tsc --noEmit` passes. - Live: not yet. The failing wave above is the reproduction. A rerun is timing-dependent. ## Risks - One more `recovery_history` entry per scheduled resume, inside the existing 20-entry bound. - The proof rule itself is unchanged. This only reads the same evidence from the record where it now survives. - No schema change. A run claimed by an earlier build has no `same_run_resume` entry, so it keeps the existing failure: refused, with Retry offered as today. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
48c6e996da |
fix(native): continue a task whose quarantined completed turn's holds settle (#20662)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a remote run starts, workspace preparation attaches the verified saved work of earlier runs of the same task. > - Preparation failed with `remote_workspace_recovery_receipt_invalid` when any earlier run had a recovery receipt that it could not verify. > - A refusal cannot change the saved state, so every Retry failed in the same way, and the task stayed blocked. Recoveries must fall back automatically. > - This pull request skips an unverifiable source. It attaches nothing unverified and starts from the workspace's newest verified save. > - The benefit is that Retry and continuations start again, while strict receipt checks still decide what is attached, and no saved work is changed. ## Linked Issues or Issue Description Refs #20631 **What happened?** On staging, after the deploy that included #20631, a user pressed Retry on a task whose previous run failed with `native_controller_recovery_missing_evidence:runner_not_settled`. The new run failed in setup in under a minute with `setup_failed` / `remote_workspace_recovery_receipt_invalid` (`executionRecovery: bootstrap`, `providerWorkStarted: false`). The task stayed blocked. Another Retry would fail the same way. #20631 accepted one shape of this refusal (a source that saved only the snapshot its own restore loaded). This task had a different shape. **Expected behavior** A Retry starts. A source whose receipt cannot be verified is not attached, and its saved state stays intact for workspace recovery. **Steps to reproduce** 1. Let a remote native run fail before provider work and leave a recovery receipt or restore hold that does not verify (for example a receipt on an unsaved reference, a receipt from another epoch, or a saved reference without a receipt). 2. Press Retry on the task. 3. See the new run fail in setup with `remote_workspace_recovery_receipt_invalid`. **Paperclip version or commit** Staging deploy that includes `e3bc4ca798`. The gap remains on `master` at `a3b34a9d88`. ## What Changed - `attachPreservedRemoteWorkspaceSnapshots` no longer throws for an unverifiable source. Each of the five refusal shapes now skips the source and logs `remote_workspace_recovery_receipt_invalid` with a reason: `receipt_without_reference`, `reference_unreadable`, `receipt_on_unsaved_reference`, `receipt_unverified`, and `receipt_snapshot_missing` (was `remote_workspace_recovery_snapshot_missing`). - `retainRemoteRecoveryAttachments` no longer throws for a retained recovered copy that it cannot verify. It does not publish that copy again. The files stay in the workspace. - The writer check (`remote_workspace_recovery_writer_changed`), the workspace access check, and snapshot read and digest failures still refuse. - `doc/execution-semantics.md` describes the fallback rule. ## Verification - Red on unfixed code: `remote-workspace-recovery-receipt-fallback.test.ts` (moved out of `workspace-recovery-preparation.test.ts` to keep that file under 1,000 lines) "starts a Retry from the newest verified save when a predecessor has %s" (6 shapes: `receipt_without_reference`, `unreadable_reference_with_hold`, `receipt_on_prepared_reference`, `superseded_snapshot_with_hold`, `receipt_from_other_epoch`, `receipt_for_other_run`). All 6 failed with `remote_workspace_recovery_receipt_invalid`. The source is a native run that failed with `runner_not_settled`, and the writer is a Retry (`retryOfRunId`, `taskRetryRequestId`) that follows the real ancestry query. - Green: the same 6 tests pass. They also assert that nothing is attached, and that the source run row, its workspace operations, and its snapshot row are unchanged. - Updated safety tests: the canonical-attachment fault matrix now expects no attachment for receipt faults, and still expects a refusal for a missing or corrupt archive and for a wrong company or task. The retained-history test expects an unverifiable retained copy to be left out of the published list, while verified copies stay published. - `npx vitest run src/services/remote-workspace-recovery-receipt-fallback.test.ts src/services/workspace-recovery-preparation.test.ts`: 91 passed. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Not verified live on Daytona. I could not read the staging rows for this task: the browser had no free tab. The tests cover every refusal shape of this function, so the exact staging shape is one of them. ## Risks - Previous-release state: the rule reads the same rows as before (run reference, result hold, workspace operations, recovery actions, snapshots). Rows that earlier releases saved carry forward. No migration. A blocked task recovers on its next Retry or wake. - A Retry can now start without files from an unverifiable source. Those files are not lost: the source's snapshot, restore hold, receipts and recovery action stay unchanged, and workspace recovery still owns them. The skip is logged with a named reason. - A retained copy that fails verification is dropped from the published list. Its directory stays in the workspace and is no longer retired automatically. - A missing or corrupt snapshot archive still refuses. A permanently lost archive therefore still blocks attach. This PR does not change that. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
8f6ca25976 |
fix(workspaces): skip unverifiable recovery sources instead of failing Retry (#20666)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a remote run starts, workspace preparation attaches the verified saved work of earlier runs of the same task. > - Preparation failed with `remote_workspace_recovery_receipt_invalid` when any earlier run had a recovery receipt that it could not verify. > - A refusal cannot change the saved state, so every Retry failed in the same way, and the task stayed blocked. Recoveries must fall back automatically. > - This pull request skips an unverifiable source. It attaches nothing unverified and starts from the workspace's newest verified save. > - The benefit is that Retry and continuations start again, while strict receipt checks still decide what is attached, and no saved work is changed. ## Linked Issues or Issue Description Refs #20631 **What happened?** On staging, after the deploy that included #20631, a user pressed Retry on a task whose previous run failed with `native_controller_recovery_missing_evidence:runner_not_settled`. The new run failed in setup in under a minute with `setup_failed` / `remote_workspace_recovery_receipt_invalid` (`executionRecovery: bootstrap`, `providerWorkStarted: false`). The task stayed blocked. Another Retry would fail the same way. #20631 accepted one shape of this refusal (a source that saved only the snapshot its own restore loaded). This task had a different shape. **Expected behavior** A Retry starts. A source whose receipt cannot be verified is not attached, and its saved state stays intact for workspace recovery. **Steps to reproduce** 1. Let a remote native run fail before provider work and leave a recovery receipt or restore hold that does not verify (for example a receipt on an unsaved reference, a receipt from another epoch, or a saved reference without a receipt). 2. Press Retry on the task. 3. See the new run fail in setup with `remote_workspace_recovery_receipt_invalid`. **Paperclip version or commit** Staging deploy that includes `e3bc4ca798`. The gap remains on `master` at `a3b34a9d88`. ## What Changed - `attachPreservedRemoteWorkspaceSnapshots` no longer throws for an unverifiable source. Each of the five refusal shapes now skips the source and logs `remote_workspace_recovery_receipt_invalid` with a reason: `receipt_without_reference`, `reference_unreadable`, `receipt_on_unsaved_reference`, `receipt_unverified`, and `receipt_snapshot_missing` (was `remote_workspace_recovery_snapshot_missing`). - `retainRemoteRecoveryAttachments` no longer throws for a retained recovered copy that it cannot verify. It does not publish that copy again. The files stay in the workspace. - The writer check (`remote_workspace_recovery_writer_changed`), the workspace access check, and snapshot read and digest failures still refuse. - `doc/execution-semantics.md` describes the fallback rule. ## Verification - Red on unfixed code: `remote-workspace-recovery-receipt-fallback.test.ts` (moved out of `workspace-recovery-preparation.test.ts` to keep that file under 1,000 lines) "starts a Retry from the newest verified save when a predecessor has %s" (6 shapes: `receipt_without_reference`, `unreadable_reference_with_hold`, `receipt_on_prepared_reference`, `superseded_snapshot_with_hold`, `receipt_from_other_epoch`, `receipt_for_other_run`). All 6 failed with `remote_workspace_recovery_receipt_invalid`. The source is a native run that failed with `runner_not_settled`, and the writer is a Retry (`retryOfRunId`, `taskRetryRequestId`) that follows the real ancestry query. - Green: the same 6 tests pass. They also assert that nothing is attached, and that the source run row, its workspace operations, and its snapshot row are unchanged. - Updated safety tests: the canonical-attachment fault matrix now expects no attachment for receipt faults, and still expects a refusal for a missing or corrupt archive and for a wrong company or task. The retained-history test expects an unverifiable retained copy to be left out of the published list, while verified copies stay published. - `npx vitest run src/services/remote-workspace-recovery-receipt-fallback.test.ts src/services/workspace-recovery-preparation.test.ts`: 91 passed. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Not verified live on Daytona. I could not read the staging rows for this task: the browser had no free tab. The tests cover every refusal shape of this function, so the exact staging shape is one of them. ## Risks - Previous-release state: the rule reads the same rows as before (run reference, result hold, workspace operations, recovery actions, snapshots). Rows that earlier releases saved carry forward. No migration. A blocked task recovers on its next Retry or wake. - A Retry can now start without files from an unverifiable source. Those files are not lost: the source's snapshot, restore hold, receipts and recovery action stay unchanged, and workspace recovery still owns them. The skip is logged with a named reason. - A retained copy that fails verification is dropped from the published list. Its directory stays in the workspace and is no longer retired automatically. - A missing or corrupt snapshot archive still refuses. A permanently lost archive therefore still blocks attach. This PR does not change that. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
9296c74e8d |
fix(native): let a restart claim take back its lapsed lease at budget admission (#20650)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a control-plane restart, restart recovery claims an interrupted native run (`resume_dead_runner`) and grants a 20-minute lease. Budget admission then re-checks that the claim still owns the run before the resumed run may start provider work. > - A host sleep, or the restart-chaos fault TM2, can make that stored lease look expired while its owner is alive. #20625 lets the owner take back such a lease when the row still shows exactly its own claim. That rule did not cover budget admission. > - So a claim whose lease lapsed before dispatch was refused with `Native recovery no longer owns provider dispatch`. The run failed with `adapter_failed`, recovery held it with `native_continuation_requires_reconciliation`, and the user had to click Retry. > - This pull request applies the #20625 rule in budget admission: the claim takes its lease back through the existing re-acquire condition. > - The benefit is that a run resumed after a restart survives a time jump before dispatch with no Retry. Every fence and every budget check stays the same. ## Linked Issues or Issue Description Refs #20625, #20474 **What happened?** Restart-chaos wave L7 (seed 9107, local) on a branch with #20625, task codex B5 TM2. A TM3 restart claimed the run (`resume_dead_runner`, generation 2). TM2 then aged every database time by one hour before the resume reached dispatch. Budget admission (`lockRunBudgetRecoveryOwnerInTransaction`) required a live lease and threw `Native recovery no longer owns provider dispatch`. The run failed with `adapter_failed`, recovery stopped with `native_continuation_requires_reconciliation` / `preserve_without_replay_v1`, and the task needed Retry. **Expected behavior** The claim's own lapsed lease is taken back, and the resume is admitted when the budget allows it. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex,claude --workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1 --concurrency 3 --tasks 6 --seed 9107` (timing-dependent). ## What Changed - `native-execution-lease.ts`: two new helpers. Both use the #20625 own-claim rule. Neither takes a lock out of order. - `readOwnNativeRecoveryClaimLease` (read-only) returns `live`, `lapsed_own_claim` or `lost`. It compares the lease against the database clock. A lapsed lease counts as `lapsed_own_claim` only when all of these hold: - the same lease owner; - this process's controller boot, pid and process start; - a non-null `recovery_state`; - `nativeExecutionLeaseHeldCondition({ remote: false, controllerGeneration })`, which applies the #20625 own-claim rule and every #20474 untouched check; - no non-local environment lease on the run. - `extendOwnNativeRecoveryClaimInTransaction` extends such a lease by the local execution TTL, with one conditional UPDATE on the same predicate. - `budget-reservations.ts`: - `lockRunBudgetRecoveryOwnerInTransaction` still refuses another owner, and refuses a lapsed lease unless it is the run's own untouched claim. It no longer locks the run row, so the documented coordinator → issue → run order holds. - The lease is extended only at the end of an admitted reservation (`reserveRunBudget`) or an admitted dispatch recheck (`recheckRunBudgetReservationInTransaction`). At those points the caller already holds the run row lock. A refused admission, for example one over budget, leaves the lease lapsed. - `heartbeat-native-restart-suspension.test.ts`: the test "still settles a recovery whose claim merely expired" encoded "expired means not the owner". #20474 and #20625 changed that rule for an untouched own claim. The test now has two cases: - an expired claim that another actor recorded on (`awaiting_evidence`) still settles with no provider session; - an untouched expired claim is dispatched. - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: Step 1 lists the new user of the rule. ## Verification - New `server/src/__tests__/native-recovery-dispatch-lease-reacquire.test.ts`: - `reserveRunBudget` takes back the claim's own lapsed lease and admits the resume. This fails before the change. - An over-budget resume is still refused, and its lease stays lapsed. - Fenced variants each throw `Native recovery no longer owns provider dispatch` and leave the row unchanged: a graceful-shutdown suspension, an appended failure settlement, `awaiting_evidence`, `blocked`, a claim from an earlier generation, another boot, a remote sandbox lease, and another lease owner. - At the dispatch gate (`recheckRunBudgetReservationInTransaction`), an admitted own claim gets its lease extended, and a fenced claim is refused with the row unchanged. - Related suites pass (308 tests): `native-execution-lease-reacquire.test.ts`, `closed-native-receipt-accounting.test.ts`, `heartbeat-native-predispatch-retry.test.ts`, `run-dispatch/adapters/postgres.test.ts`, `heartbeat-native-restart-suspension.test.ts` and `heartbeat-restart-claim-lapse.test.ts`. - `cd server && npx tsc --noEmit` passes. - Live: one L7 wave (seed 9107) ran on the first commit of this branch (`w-20261110T212307-s9107`). It logged 0 `Native recovery no longer owns provider dispatch` refusals, against 1 in the previous wave. Codex B5 TM2 got past dispatch, but it still needed Retry for another reason: after a `runner.drain` timeout, the scheduled same-run resume did not send the continuation of #20645. That case is tracked separately. The other five tasks passed. ## Risks - This widens when a restart claim can take back its own lapsed lease, and only at budget admission. It is local only. It needs the same owner, boot and generation, and the row must show exactly that claim. A claim that any other actor touched still fails as before. - Budget hard-stop behaviour is unchanged. The lease is extended only after budget admission passes. - No schema change and no migration. Rows saved by earlier releases carry the same history shape. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
de7ceac0de |
fix: preserve compatible historical Codex WEB evidence (#20665)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native session migration preserves local evidence in encrypted archives. > - Each WEB receipt binds the exact provider directory and exclusion policy. > - The Codex credential refresh change added two excluded files and changed that policy hash. > - A new reader then rejected old receipts even when their facts, files, skills, and authority still matched. > - This pull request accepts the exact prior Codex policy while checking archive paths against the current credential policy. > - Operators can keep valid saved evidence across the upgrade without copying it again. ## Linked Issues or Issue Description Refs: #19814, #20646. **What happened?** A reader upgrade rejected saved Codex WEB evidence because the profile hash changed when `auth-refresh.json` and `auth-refresh.json.tmp` became excluded credentials. The other receipt bindings still matched. This blocked the native harness migration gate. **Expected behavior** The exact known prior profile remains readable when the current archive path checks accept every saved entry. An archive that includes either newly excluded credential file must still fail. Unknown profiles and changed authority must fail. **Steps to reproduce** 1. Preserve WEB evidence with the frozen prior Codex profile and no credential refresh files. 2. Read its unchanged receipt with the current profile. 3. Observe that the old reader returns false solely because the profile hash changed. **Paperclip version or commit** Reproduced against `a3b34a9d88`; final branch is based on `e9343d8501`. ## What Changed - Accept the exact prior Codex profile hash only when the complete current profile has the reviewed successor hash and the provider is Codex. - Keep the current entry and excluded-path checks. Keep facts, controller, archive, skill, and ownership checks. - Add a frozen prior policy fixture with its literal hash. Test old receipt reads, both credential files, unknown policies, changed authority, and current writes. ## Verification - Real PostgreSQL baseline: the frozen old-policy receipt test failed because the reader returned false. The unchanged fixture passes with this fix. - All 18 selected PostgreSQL cases pass on final head after the master rebase: six new cases and twelve existing WEB archive and authority guards (14.93 seconds). - Full layered server typecheck passes on final head after the rebase. The module boundary check passes. - A single read-only native-only candidate check on the deployed dependency graph validated all 54 retained scopes. The 29 prior failures passed, and the other 25 stayed valid. The check made no storage calls or data writes. It did not replace final deployed enrollment acceptance. - Final head `14112d0e3abb6e318a86fe9799d95bddd2dbf76b` passes all [required CI checks](https://github.com/paperclipai/paperclip/actions/runs/42919867108): 55 successful checks, four expected skips, and one neutral review. The legacy security status also passes. Exact-head Greptile review is 5/5 with no open review threads. ## Risks The compatibility branch accepts only the two reviewed policy hashes. A future policy change can require another explicit compatibility decision. The reader still rejects saved credential refresh files. It does not rewrite receipts or change execution authority. No stored schema or operator command changes. ## Model Used OpenAI Codex, GPT-6 family. The exact serving model ID and context-window capacity are not exposed in this session. Used reasoning, source editing, shell execution, and tool calls. ## 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> |
||
|
DottaandPaperclip
|
aa2a7be8b4 |
fix: recognize provider-owned Runner workspace storage (#20663)
## Thinking Path > - Paperclip manages agent work and preserves its execution state. > - A server replacement must retain every workspace that depends on the old server disk. > - Runner task environments can own their workspace outside the Core server. > - These environments save an adapter-managed workspace with a runnerTaskWorkspaceOwner binding. > - Readiness and maintenance treated that provider path as a Core filesystem dependency. > - This change validates the saved owner and shares that classification between both paths. > - Local, malformed, and foreign workspace bindings still block enrollment. ## Linked Issues or Issue Description Related enrollment safeguards: #18835. A GitHub search found no duplicate fix for this provider-owned class. **What happened?** Volume-free readiness returned workspaces_require_migration for an adapter-managed Runner workspace owned by a plugin task environment. Maintenance also selected it and failed with workspace_provider_unsupported. The provider owned the path, which did not exist on the Core server. **Expected behavior** A valid provider-owned workspace does not require migration from the Core server disk. Ordinary local workspaces and invalid ownership bindings remain blockers. **Steps to reproduce** 1. Run a paperclip_runner agent in a plugin task environment. 2. Retain its adapter-managed workspace and complete runnerTaskWorkspaceOwner binding. 3. Read startup eligibility for volume-free enrollment. 4. Observe the workspace blocker before this fix. 5. Run workspace maintenance and observe workspace_provider_unsupported for the same row. **Paperclip version or commit** Reproduced on |
||
|
Devin FoleyandPaperclip
|
e9343d8501 |
fix(heartbeat): release task Retries held by a drain as soon as it ends (#20640)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a person clicks Retry on a stopped task, Paperclip saves the
request as a held wake and dispatches it right away, unless admission is
held. Admission is held during a task drain, for example while a
deployment restarts the server
> - A Retry held by a drain is released later by the deferred-receipt
worker. That worker shares one queue with every other held wake: comment
waits, execution waits, and interrupted queues
> - On a busy instance that queue holds hundreds of passive waits that
come due every 30 seconds. A Retry is checked in due-time order behind
all of them. Ending a drain does not wake the worker either
> - In a real case, a Retry clicked during a deployment drain was still
held 40 minutes later, after the drain expired and the server restarted.
It ran only when the person clicked Retry again
> - This pull request checks operator Retries ahead of passive waits,
and wakes the worker as soon as a drain stops or expires
> - The benefit is that a Retry held by a drain starts within one worker
pass after the drain ends
## Linked Issues or Issue Description
**What happened?**
A person clicked Retry on a blocked task 14 seconds after a deployment's
task drain started. The Retry was saved as `deferred_issue_execution`,
and the immediate dispatch returned early because of the drain. The
drain had a 20-minute TTL, and the deployment restarted the server about
6 minutes in. The held Retry still did not start, and its `updated_at`
never changed. Forty minutes later the person clicked Retry again, and
the new request started a run and merged with the held one.
That instance had 390 held wakes: 271 execution waits and 116 others,
against 6 task Retries. The execution waits come due every 30 seconds.
Measured on the instance, the deferred-receipt worker took about 7
minutes to get through that backlog once.
**Expected behavior**
A Retry that a drain held starts soon after the drain stops or expires,
or after the server restarts, whatever the size of the passive backlog.
**Steps to reproduce**
1. Start a non-idle task drain (`POST /instance/task-drain`).
2. Click Retry on a stopped task. The receipt is
`deferred_issue_execution`.
3. Stop the drain. On `master`, the deferred-receipt worker only notices
on its next 30-second re-check. It then checks the Retry in due-time
order behind every passive wait that came due before it.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
686893636d |
fix(recovery): release a lapsed owner of a finished coordinator so stop-proof holds re-arm (#20661)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Workspace recovery saves the files of a run whose copy-back failed, and it holds the shared workspace until the save is done > - #20295 re-arms a save that was exhausted with `agent_stop_unconfirmed`, so the run's stop is proven again and the files are saved without a person > - On staging, four holds of this shape were never re-armed. Their runs ended six days ago, their sandbox leases are released with a provider stop receipt, and the servers that ran them were replaced many times. Their tasks stay blocked, and so do the parent tasks that wait for them > - The finished coordinator of each run still names a lease owner whose lease lapsed. Discovery treats that owner as "waiting for the abandoned-owner settlement", but that settlement handles only observed (unfinished) coordinators. Nothing ever releases the owner, so the worker never selects the hold, the re-arm refuses it, and the attempt's stop check refuses it too > - This pull request makes a lapsed owner of a finished coordinator count as stopped. The re-arm releases it in the same transaction, and then the new attempt proves the sandbox stop and saves the files as #20295 designed > - The benefit is that these holds settle on the worker's next pass after deploy, without a migration and without an administrator ## Linked Issues or Issue Description Refs #20295 **What happened?** An exhausted workspace save (`native_workspace_sync_out_retry_exhausted`, last failure `agent_stop_unconfirmed`) stayed blocked for six days. The task showed "Paperclip tries again automatically", but no re-arm was ever recorded (no `stopProofRecovery` marker, no `issue.workspace_recovery_rearmed` activity), and the shared workspace stayed held by the failed run. **What did you expect to happen?** The recovery worker re-arms the save once the run is provably over, saves the dirty files, and frees the workspace so the task and its parent can continue. **Steps to reproduce** 1. Exhaust a workspace save with `agent_stop_unconfirmed` (one attempt, window spent). 2. Leave a lease owner on the run's `terminal_failure` coordinator with an expired lease (or no expiry), as a finalizer pass of a replaced server does. 3. Run the workspace recovery worker. `nextWorkspaceRecoveryAt` returns null, discovery does not select the hold, and `rearmExhaustedStopProof` refuses it. ## What Changed - `workspace-recovery-stop-proof.ts`: add `lapsedFinishedCoordinatorOwner` and `releaseLapsedCoordinatorOwner`. A lease owner on a finished coordinator (committed, terminal failure, applied) whose lease lapsed, or that has no expiry, does not block `sourceRunProvablyOver`. The stop-proof re-arm clears that owner with a compare-and-swap in its locked transaction, and records `releasedLapsedOwner` in the marker and the activity. - `workspace-recovery-capture-policy.ts`: the capture-policy re-arm uses the same rule, because it shares the same check and discovery clause. - `workspace-recovery-fallback-discovery.ts`: the discovery clause still waits for the abandoned-owner settlement for a stale owner of an unfinished coordinator, but it no longer excludes a hold whose finished coordinator has a lapsed owner. A live lease still defers the re-arm to its expiry. - `system-recovery-state.ts`: add `FINISHED_COORDINATOR_PHASES`. - `doc/execution-semantics.md`: document the rule. - Tests: two red-then-green regressions in the staging shape (an expired finalizer lease, and an owner without an expiry saved by an older release), one strictness case (a live lease on a finished coordinator keeps the hold and is not released), and the existing stale-owner case now uses an observed coordinator, which still waits for the abandoned-owner settlement. ## Verification - Red before the fix: `pnpm --filter @paperclipai/server exec vitest run src/services/workspace-recovery-stop-proof.test.ts` failed the two new staging-shape cases (`nextWorkspaceRecoveryAt` returned null: the worker was never woken for the hold). - Green after the fix: `workspace-recovery-stop-proof.test.ts` and `workspace-recovery-capture-policy.test.ts` pass (15 tests). The staging-shape cases assert that the hold is re-armed, the save completes, the dirty files are saved, the workspace hold is free, and the lapsed owner is released. - `pnpm verify`: exit 2 (broader verification required, because `doc/execution-semantics.md` is a global CI input). Focused suites pass: `workspace-recovery-stop-proof`, `workspace-recovery-capture-policy`, `workspace-recovery-fallback`, `workspace-recovery-retry-copyback` (113 tests). Required CI supplies the broad coverage. - Staging evidence (read-only GET requests): the run is `failed`, finished six days ago, `controllerLeaseExpiresAt` is null, its only lease is `released` with `cleanupStatus: success`, a `stopped` termination receipt and a stop-and-retain receipt. The action's evidence has no re-arm marker, and four holds of the same shape were not touched after #20295 was deployed. ## Risks - Previous-release story: rows saved by earlier releases carry the lapsed owner and no marker. They are selected and re-armed on the worker's first pass after deploy. No migration. - Strictness stays where an owner could be live: a live lease on any coordinator defers the re-arm to its expiry, and a stale owner of an unfinished (observed) coordinator still waits for the abandoned-owner settlement, which proves its controller stopped. The new attempt still requires a provider stop receipt for the allocation before it reads files. - Clearing a lapsed owner matches the existing token-lease contract: an expired finalizer or cleanup lease is free for the next claim, and a holder's later write fails its token compare-and-swap. - Low risk otherwise: the change is limited to the two exhausted-hold re-arms and their discovery clause. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, browser GET reads) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
f30ac226d6 |
Keep project environment mappings independent of shared defaults (#20651)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A project supplies work context and repository storage to task runs. > - Creation selected a storage owner but did not retain the selected environment on the project. > - Configuration stored an environment inside checkout policy, but policy reads and run selection dropped it. > - Projects therefore continued to follow shared agent and instance defaults. > - This pull request saves placement on each project and uses it for new runs. > - The benefit is an independent Project to Environment association with the existing storage, trust, and recovery safeguards. ## Linked Issues or Issue Description Refs #20459 and #11644. Follow-up to #20587. The reported journey is Project > Configuration: changing one project's environment must not change another project's mapping or shared defaults. Related open PR #18176 repairs placement through the legacy workspace-policy field. This follow-up introduces a separate project association, saves creation defaults, and migrates saved legacy references. It overlaps that PR's placement, permission, and recovery safeguards. The changes should be reviewed together. #9588 concerns environment-variable response privacy and does not address placement. ROADMAP.md has no separate project-association work planned. ## What Changed - Add nullable `projects.environment_id` and expose `Project.environmentId`. Save the effective creation default once on the project. Keep environment placement separate from checkout/isolation policy. - Read and write Environment directly in Project Configuration. Keep the control available with isolated workspaces disabled. Each form reads its project's value, even when environment choices share a query cache. - Offer environments compatible with the current storage owner. Change the target and storage together when the existing storage selector attaches a core project to a plugin. Preserve retained-data restrictions and validate the observed storage owner again on save. - Resolve new task runs from their organizational project's target before agent and instance defaults. File sources from another project do not change placement. Explicit low-trust sandbox choices and enforced sandbox policies retain precedence. External provider placement remains separate. - Restore admitted native runs from their owned environment lease. Later project edits apply to new admissions. Require workspace-write permission for environment changes and clears. Check company bindings before runtime admission and during lease creation. - Migrate valid saved policy references into the project column. Normalize legacy API writes. Count and clear matching project references when an environment is deleted. Update API and behavior documentation. - Refresh the maintained SDK source registry for the two changed shared files. Runtime pins are unchanged. ## Verification - [Hosted CI](https://github.com/paperclipai/paperclip/actions/runs/42908045146) is green on `8b3b7d5b5aed51fc4c8e9c2e3506508bbaf9e00c`: **60 successful checks**, three skipped gates, no failures, and `ci / verify` passed. GitHub reports no merge conflicts. Standard Greptile scored the same commit **5/5** with both review threads resolved. - Source checkout on macOS arm64, Node 26.12.0, with fresh isolated PostgreSQL. No production data or live provider calls. - The two-project component test changes the first form with isolation disabled and a shared query cache. The second form keeps its target. Storage changes update only the selected project. A saved target remains selected while the list loads, and an explicit clear remains possible. - API integration creates two projects from the same default, patches one, reads both, and checks that agent and instance defaults remain unchanged. A later instance-default change does not rewrite either saved project target. Tests cover unavailable/incompatible choices, anonymous and cross-company denials, and storage preconditions. - The migration test runs the actual SQL against an existing project row and verifies its environment and disabled checkout policy survive. Deletion accounting and clearing affect only matching project references. - The final guard set passes **53 tests across four files**: `project-environments.test.ts`, `environment-selection-route-guards.test.ts`, `heartbeat/environment-selection.test.ts`, and `ProjectProperties.environment.test.tsx`. All **64 environment-run-orchestrator tests** pass, including direct lease acquisition without an extra caller argument. The broader affected set passed **109 tests across eight files** before the additional admission guards. These sets overlap. - UI typecheck, migration numbering/safety, token gates, module boundaries, query-error rendering, and diff checks pass. `pnpm verify --explain` returns exit 2 and requires broad CI because shared schema/runtime paths and behavior documentation changed. Broad coverage is delegated to current-head CI. - Early iterations failed due to a legacy parser that drops the environment field, a fixture without real company membership, and incorrect expectations for privacy-preserving 404 denials. The service now reads legacy references before normalization, the API fixture has real membership, and denial assertions match the existing privacy contract. The orchestrator mock also needed the reused company-binding guard in its service contract. The focused suites above pass after those corrections. - Local server typecheck remains blocked by installed ACPX, Slack, AI SDK, and Runner type mismatches in unchanged dependency-facing code. It reports no errors in the changed production paths. This is not a full local typecheck or suite pass. Hosted CI supplies full current-head coverage. - SDK source-proof and privacy qualification passes **56 tests across three files**, including the real built SDK. The maintained source registry matches the committed revision; runtime pins are unchanged. - Review reproduced two failures: an incoming policy without a storage owner could be validated before the existing owner was restored, and uppercase legacy UUIDs missed migration matching. Preserve the owner before validation and compare normalized UUID text. Both regressions now pass. The post-fix project/storage/orchestrator batch passes **107 tests**: `node node_modules/vitest/vitest.mjs run server/src/__tests__/project-environments.test.ts server/src/__tests__/project-workspace-provider.integration.test.ts server/src/__tests__/environment-run-orchestrator.test.ts --silent`. - The first hosted run failed build, typecheck, and canary at the same `run-completion.test.ts` call because lease acquisition required a new argument. The lease method now enforces binding internally. Its existing caller compiles without a new field, and the orchestrator tests prove the binding flag reaches the runtime. The full server check no longer reports that error; the local dependency errors above remain. ## Risks - Apply the normal database migration before serving the new revision. The migration adds a nullable foreign key and copies valid legacy project selections. It does not move repository data or resize provider storage. The five newest schema snapshots are retained. - New projects retain their creation target when shared defaults change. Existing projects without a saved selection continue to inherit defaults. A null target explicitly opts into inheritance. - Existing saved project selections now affect placement. Missing, archived, incompatible, and foreign-company targets fail validation; they do not silently choose another storage owner. A running admitted process keeps its original owned lease. - Local dependency limitations are listed above. No live-provider, deployed-browser, or production rollout qualification is claimed. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The exact serving snapshot 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> |
||
|
DottaandClaude
|
30403781b9 |
fix(native): keep crediting proven unstarted restarts past the generation limit (#20647)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run survives a control-plane restart through restart recovery, which resumes the interrupted provider turn > - Restart recovery charges a provider attempt only when the provider did work that could be lost, and it stops after three charged attempts > - From controller generation 12 on, every restart charged an attempt before that work rule was checked > - In a busy restart wave, one long task sees 12 to 15 restarts, most caused by other tasks. A restart that hit before the provider accepted a turn was charged, the budget ran out, and a person had to click Retry > - This pull request keeps the work rule past generation 12 for restarts that a durable record proves did not start the provider, and bounds a crash loop with a count of consecutive restarts without provider progress > - The benefit is that a long task survives many unrelated restarts without a manual Retry, while a run that never makes progress still stops ## Linked Issues or Issue Description Refs #20625 (same restart recovery path). No public issue exists for this case. **What happened?** `restartResumeKeepsAttempt` returned "charge" for every restart once the run reached `NATIVE_PROGRESS_CREDIT_GENERATION_LIMIT` (12) controller generations. It did this before it read the work-based outcome. In a local restart chaos wave (L6, seed 9106, task t03, Claude B5 with RR1) the run reached generation 13. The last restart interrupted a resume whose runner started but whose provider had not accepted a turn. That restart was charged at the last attempt, so the claim failed with `execution_recovery_budget_exhausted` and `native_restart_recovery_blocked`. The task needed a manual Retry. **Expected behavior** A restart that a durable record proves did not start provider work does not spend an attempt, also past generation 12. A crash loop that never makes provider progress still stops automatically. When it stops, the task shows a Retry that works. **Steps to reproduce** Run a native task past controller generation 12. Make the last restart hit after `native.process_start_requested` but before `turn.accepted`, with the provider attempt at 3. The new integration test `does not charge a restart past the generation limit whose resume never started the provider` reproduces the failure before the fix. **Paperclip version or commit** `master` at |
||
|
DottaandClaude Opus 5.5
|
4f33bb3578 |
fix(workspaces): release the Blocked status of a sibling-settled no-replay hold (#20648)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a remote workspace save fails, recovery actions hold the task until the run's files are saved > - #20641 lets a resolved no-replay hold settle when a sibling action of the same run saved the files > - Review found that the settlement did not release the task's Blocked status, and that a settled hold with a historical source stayed selectable > - A new test with the waiting user comment also showed that the worker skipped the second hold of the same run in every pass, so it never settled > - #20641 was merged before these fixes were pushed > - This pull request completes #20641 with those fixes > - The benefit is that the task leaves Blocked and continues without a person ## Linked Issues or Issue Description Refs #20641, Refs #20569 **What happened?** #20641 merged at its first head. The review follow-ups for it were pushed later and are not on master: - A settled no-replay hold that owned the task's Blocked status (`nativeFailureBlock`) left the task `blocked`. - A hold settled by a sibling stayed selectable through `workspaceRecoverySourceEligible` when it named a `historicalWorkspaceSource`. - With saved user input waiting, the export hold stayed selectable and came first in each worker pass. `reconcileRetainedWorkspaces` skipped the other hold of the same run, so the no-replay hold never settled. **Expected behavior** After the save, the no-replay hold settles in the same or next pass. The task returns to `todo` when that hold owns its Blocked status. The settled hold is never selected again. **Steps to reproduce** 1. Use the staging-shape test in `workspace-recovery-capture-policy.test.ts`, with a waiting "try again" comment. 2. Run the worker until nothing is due. 3. On master, the worker never goes idle and the no-replay hold stays `resolved`/`blocked`. ## What Changed - `workspace-recovery-fallback.ts`: new shared `holdOwnsBlockedStatus`, used by `continueFromLastSavedWorkspace` and `settleSiblingRecoveredHold`. A settled no-replay hold that owns the Blocked status returns the task to `todo`. A newer decision's Blocked status is kept. An export hold's status stays with its own result finalization. - `workspace-recovery-source.ts`: `workspaceRecoverySourceEligible` excludes a resolved hold that carries `siblingWorkspaceRecovery`. - `workspace-recovery.ts`: `reconcileRetainedWorkspaces` runs several holds of one run one after another in that run's job, instead of skipping the later ones. - `workspace-recovery-capture-policy.test.ts`: the staging test now has the waiting comment, and a variant with `nativeFailureBlock` and `historicalWorkspaceSource`. It checks `todo`, no re-selection, and no repeat handoff in a later pass. - `doc/execution-semantics.md`: documents these rules. ## Verification - Red, with each rule disabled: the status stayed `blocked`; the settled hold was selected again; the worker never went idle (starvation). - Green: `npx vitest run --root server` on 14 workspace recovery suites (including `workspace-recovery-capture-policy`, `-fallback`, `-admission`, `-source`, `execution-blocker-read`, `preserved-native-response-wait.integration`): all passed. - `npx tsc --noEmit -p server/tsconfig.json`: clean. ## Risks - Several holds of one run now run in sequence in one job. Concurrency per run is unchanged (one job per run). - A no-replay hold releases the Blocked status only when it still owns it. Otherwise the status is kept. - Previous-release story: no new persisted state. Holds saved by earlier releases are handled on the first worker pass after deploy. > 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 - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
805531cc71 |
fix(native): never leave a cleanup-gated task without Retry or an owner (#20639)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs close their provider session at the end of each attempt.
When close cannot prove that the runner stopped, the runner keeps a
cleanup gate (`native_session_cleanup_quarantined`) in server memory
> - #20572 hid Retry for every gated run and said "Paperclip finishes
the cleanup automatically". Only a remote environment release settles
the gate. A local gate that needs an operator never settles, and a
server restart loses it
> - A restart-chaos wave on master found two tasks that rested blocked
with no Retry, no scheduled retry and no recovery owner. A person could
not continue them
> - This pull request makes the run's own recovery retire the gate when
the host proves the old processes stopped. It also offers Retry whenever
nothing settles the gate, and it makes that Retry work
> - The benefit is that a cleanup-gated task either continues by itself
or shows a Retry that starts. It never rests without both
## Linked Issues or Issue Description
Refs #20572, #20578
**What happened?**
Restart-chaos wave `w-20261110T185144-s9107` (local, claude, fault TM2:
the server is paused with SIGSTOP for about 33 s). Tasks t04 and t05
completed their provider turn. The runner checkpoint owner lapsed while
the server was paused (`native_runner_checkpoint_owner_lost`). The
session close then failed with `native_session_close_unrecoverable:
runner did not durably suspend before checkpoint`. The provider had
drained and no tool call was pending. The runner kept an
operator-required cleanup gate. Thirty seconds later the run's own
same-run recovery verified that the runner process and group had stopped
(`native.local_process_stopped`). The gate still refused the recovery,
and the run ended `failed` with `native_session_cleanup_quarantined`.
The task showed `canRetry: false` and a cleanup wait until the deadline.
Nothing owned that wait.
**Expected behavior**
When the old runner and provider processes are proven stopped, the run's
own recovery continues. When nothing settles the gate, the task offers a
Retry that starts.
**Steps to reproduce**
1. Run restart-chaos L7: `--env local --adapters codex,claude
--workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1
--concurrency 3 --tasks 6 --seed 9107`.
2. Observe a claude TM2 task rest `blocked` with `canRetry=false`,
`retryWait=true` and no scheduled retry.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude
|
37c50f70c2 |
fix(native): continue a turn lost on restore in a proven same-run resume (#20645)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run keeps its provider conversation when its local runner dies. When the restored runner finds that the active turn was lost, it reports `provider_turn_lost_on_restore` and does not invent a result. > - After a control-plane restart, the admitted restart attempt may send one continuation in that same conversation (#15297). The agent then reconciles unfinished work. The task is not resent. > - A runner can also be lost without a server restart, for example when `runner.drain` times out. The run then resumes itself after the retry delay from its exact checkpoint. That path never allowed the continuation, because the executor passed `resumeInterruptedTurn` only for a `resume_dead_runner` restart claim. > - So the lost turn failed with `native_provider_terminal_failed`, automatic replacement was refused (`uncertain_provider_action`), and the user had to click Retry. > - This pull request allows the same single continuation for that resume, but only when durable records prove the same three facts that a restart claim proves. > - The benefit is that a runner lost mid-turn continues in the same conversation with no Retry. No case that is refused today becomes allowed without that proof. ## Linked Issues or Issue Description Refs #15297, #20625 **What happened?** Restart-chaos wave L7 (seed 9107, local), task codex B5 TM2, run on master after #20625. A TM3 restart resumed the run (attempt 1). During the TM2 freeze the TS runtime lost its runner: `native_session_recovery_failed: runner_local_connect_failed: PRP command runner.drain timed out`. The executor saved `retryable_failure` with `recoveryMode: "exact_checkpoint_resume"`. After the delay, recovery verified the runner process stopped (`native.local_process_stopped`). The same run then resumed as attempt 2 (`native.process_start_requested`, `providerAttempt: 2`). The restored runner reported `turn.failed` with `provider_turn_lost_on_restore` (`recoverable: true`). No continuation was sent. The run failed with `native_provider_terminal_failed`, recovery stopped with `uncertain_provider_action` / `preserve_without_replay_v1`, and the task needed Retry. **Expected behavior** The run sends the one same-conversation continuation, exactly as it does after a restart claim, and finishes without Retry. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex,claude --workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1 --concurrency 3 --tasks 6 --seed 9107` (timing-dependent: the drain timeout must land during an active turn). ## What Changed - New `server/src/services/native-runtime/scheduled-resume-continuation.ts`. It decides whether a same-run scheduled resume may continue a lost turn. Each of the three proofs comes from a durable record: 1. **The previous runner is stopped.** The latest process record of the run in `heartbeat_run_events` (`native.process_start_requested`, `native.process_identity_recorded`, `native.local_process_stopped`) must be `native.local_process_stopped`. Its `processPid` must equal its `processGroupId`, and neither the pid nor the group may be alive now. This is the existing `readNativeLocalProcessStop` check, read in the claim transaction before this attempt records its own `native.process_start_requested`. 2. **The attempt was admitted under the bounded budget.** The executor's claim moves `native_run_finalizations` from phase `retryable_failure` to `observed`. It charges a provider attempt, and the claim refuses any attempt above 3 (`native_session_retry_exhausted`). The decision also requires `1 <= attempt <= NATIVE_PROVIDER_RECOVERY_ATTEMPT_LIMIT`. 3. **It is the same provider conversation.** The saved `failure_detail` must show `recoveryMode: "exact_checkpoint_resume"`, `recoverable: true`, `providerSessionEstablished: true` and `checkpointExists: true`, with no `successorRunId` or `replacementDenied`. At run time the existing runtime guards still apply: the backend must recover the checkpointed session (`recovered`, not a continuity-break replacement), and the lost turn must equal the checkpoint's `activeTurnId` and the snapshot's interrupted turn. The provider kinds stay codex, acpx and opencode, with no compaction, goal control, held result or pending runtime request. - `native-session-executor.ts`: computes that decision in the coordinator claim. It passes `resumeInterruptedTurn` when either the restart claim is `resume_dead_runner` (unchanged) or the decision allows it. - `doc/execution-semantics.md`: describes the rule and the refused cases. **Cases that stay refused** (each has a test in `scheduled-resume-continuation.test.ts`): - a restart-recovery claim (it keeps its own existing rule); - a remote execution target; - a coordinator that is not `retryable_failure`, including a fresh run; - `recoveryMode` `bootstrap_retry` or `ambiguous_state`, or no saved failure detail; - a failure that is not recoverable; - no established provider session; - no checkpoint; - a recorded replacement (`successorRunId` or `replacementDenied`); - an attempt outside 1..3; - no stop receipt, a stop receipt superseded by a newer launch, a stopped process that is still alive, or a run that never had a process. The runtime-side refusals are unchanged and already covered by `native-session-runtime.test.ts`: real provider errors, historical error-null failures, and no restart permission. ## Verification - `cd server && npx vitest run src/services/native-runtime/scheduled-resume-continuation.test.ts`: 22 tests, covering the decision table and the stop receipt in embedded Postgres. - `cd server && npx tsc --noEmit` passes. - Live: not proven. An L7 wave on this branch (seed 9107) did not reproduce the drain-timeout path, so this pull request has unit and database tests only. In that wave the same cell (codex B5 TM2) needed Retry for a different reason: TM2 aged its restart-recovery claim lease before dispatch, and budget admission refused with `Native recovery no longer owns provider dispatch`. This pull request does not change that path. Four other tasks passed. Claude C1 TM2 rested blocked without a Retry button, which is a separate cleanup-quarantine issue. ## Risks - This allows one more path to send the same-conversation continuation. It is limited to local runs. It needs the same proofs as a restart claim, and it uses the attempt the resume already charged. It does not change the generation cap or `restartResumeKeepsAttempt`. - A rolling deploy reads `failure_detail` and stop events written by earlier releases. Those fields and events already have this shape, so an older saved failure is judged by the same rule. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
353945c86d |
fix(tool-gateway): let agents create gateway tokens without receiving the value (#20613)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Tool gateways let external MCP clients use a company's tools with a
gateway client token
> - Since delegated connection management (#17737), an agent can create
and revoke gateway client tokens for its responsible person
> - #20581 stopped agents from getting the one-time token value, but it
did this by making token creation person-only
> - The product decision is that agents keep their delegated authority
to create and revoke tokens
> - This pull request restores agent token creation, but the agent never
receives the token value: the person reveals the value once on the
gateway's Tokens page
> - The benefit is that an agent can do the rotation work for the
person, and the secret still never enters chat, run events, logs or tool
output
## Linked Issues or Issue Description
Refs #17737
Refs #20581
**What happened**
After #20581, `POST /api/tool-gateway/gateways/{gatewayId}/tokens`
returns 403 `gateway_token_requires_person` to every agent caller.
Agents lost the delegated authority to create gateway client tokens that
#17737 gave them. Before #20581, an agent that created a token received
the plaintext value in its tool result and could repeat it in chat.
**Expected behavior**
An agent with delegated connection management can create and revoke
gateway client tokens. The agent never receives a token value. The
responsible person gets the value once, on the gateway's Tokens page.
Person callers keep today's behavior.
**Steps to reproduce**
1. Give an agent delegated connection management for a responsible
person with `tools:admin`.
2. As the agent, call `POST
/api/tool-gateway/gateways/{gatewayId}/tokens` (for example through
`call_api`).
3. See 403 `gateway_token_requires_person`.
## What Changed
- Agent create: `POST /api/tool-gateway/gateways/:gatewayId/tokens` from
an agent with delegated authority returns 201 with token metadata only
(`id`, `tokenPrefix`, `expiresAt`, `pendingReveal: true`), plus
`tokensPath`, a `reveal_credential` `requiredUserAction` for the
responsible person, and a `nextStep` instruction. The saved row has no
usable value: its stored hash is of a value that is generated and then
discarded. Gateway authentication also rejects `pendingReveal` rows.
- Person reveal: new person-only `POST
/api/tool-gateway/gateway-tokens/:tokenId/reveal`. It generates the
value, stores its hash, clears `pendingReveal` and returns the value
once. A conditional update makes a second or concurrent reveal return
409. Only the token's responsible person can reveal it (403
`gateway_token_reveal_requires_responsible_person`). Agent callers get
403 `gateway_token_requires_person`. A token revoked before reveal
cannot be revealed.
- Why a reveal that generates the value: no stored plaintext existed to
reveal, and storing one (even encrypted) for later display adds a secret
at rest. Generating the value at reveal time keeps the existing one-time
display and stores only a hash.
- Schema: `tool_mcp_gateway_tokens.pending_reveal boolean not null
default false` (migration `0460_gateway_token_pending_reveal`). Shared
`ToolMcpGatewayToken.pendingReveal`, and `ConnectionRequiredUserAction`
gets `reveal_credential` / `gateway_token`.
- UI: the Tokens page shows a "Waiting for you to reveal" section for
pending tokens with a "Reveal token" button. The value appears in the
existing "New token — copy now" one-time block. Pending tokens show the
status "Not revealed".
- Agent gateway create: the 201 for agent callers also includes
`gatewayPath`, `tokensPath` and a `nextStep` that tells the agent to
link the saved gateway.
- The agent create-token `nextStep` and the catalog say not to revoke
the old token in the same turn: its client keeps working on it until the
person installs the new value.
- The reveal update and its `tool_gateway.token_revealed` activity entry
commit in one transaction. The Tokens page offers Reveal only for tokens
that act for the signed-in person.
- API reference: new `docs/api/tool-gateway-tokens.md` (create, reveal,
revoke) in the docs nav.
- Revoke stays allowed for agents with delegated authority (unchanged
route).
- API catalog: create-gateway, create-token, reveal and revoke
descriptions now say who acts, that agents never receive values, the
page paths, and that the old token is revoked only after the client
works with the new one.
- Agent access map: create-token policy note and a new reveal entry.
- Evals `gateways-profiles.create-gateway` and
`gateways-profiles.rotate-token`:
- Remove the keyword grader (`human|operator|admin|manage`) and the
handoff text checks. Grade saved state, rendered links and secret
absence. A plain reply or a question card is accepted.
- Remove the setup probes that called the create routes as the agent.
Under delegated authority, those probes created a real gateway
("Forbidden agent gateway"), which failed create-gateway before the
agent ran.
- create-gateway: exactly one gateway for the intended profile and
company, created by the agent (or by the person after a handoff); the
person opens it from a rendered link; attribution matches the creator;
any agent-created token is still pending reveal; no token value in chat,
cards or run events.
- rotate-token: after the agent's turn, both setup tokens still work and
any replacement the agent created is pending the person's reveal; the
person follows the rendered Tokens page link and reveals (or issues) the
value in a private browser; the new value works before the old token is
revoked; the unrelated client keeps access; no token value appears
anywhere, including a regex scan for any full `pcgw_<id>.<secret>`
value.
- The changed scenario context and outcomes are recorded in
`chat-full-catalog-testops-manifest.json` `changedFields` and pinned in
the manifest test. The inventory digest, descriptors (`oracleVersion`
v2, `sourceFiles`) and SDK public input pins are updated. The unused
`external-gateway-handoff.ts` module is removed.
- Fixture realism: the create-gateway profile now allows a real read
tool (an empty profile made the agent ask which apps to add instead of
finishing), and the rotation tokens get a neutral owner note instead of
"Disposable rotation fixture" (the agent read that note as an eval
hint).
- Shared private-browser helpers (`external-private-browser.ts`,
`external-private-browser-worker.ts`) get one optional `pendingTokenId`
input and one `revealedPending` receipt field.
## Verification
- Red: on `master` route code, the new integration tests `lets agents
create and revoke gateway client tokens without ever receiving a token
value` and `reveals an agent-created gateway token value only to its
responsible person` fail (403 `gateway_token_requires_person`).
- Green: `cd server && npx vitest run
src/__tests__/connection-management.integration.test.ts -t gateway` (25
passed). The test checks that the agent `call_api` result has no `token`
field and no token-shaped value, that the agent cannot reveal, that the
person reveals once (second reveal 409) and the value authenticates,
that agent revoke works and stops the value, that a token revoked before
reveal never gets a value, and that person create still returns the
value.
- `npx vitest run src/__tests__/tool-gateway.test.ts
src/__tests__/openapi-routes.test.ts
src/services/native-runtime/runner-api.test.ts` (2247 passed).
- `cd ui && npx vitest run
src/pages/apps/gateways/panels/TokensPanel.test.tsx
src/pages/apps/gateways/gateway-helpers.test.ts` (16 passed); `pnpm
check:token-gates` clean.
- Eval graders and catalog: `pnpm exec vitest run --config
tests/runner-e2e/vitest.config.ts` on the gateway oracle tests,
`external.test.ts`, catalog consistency/contract, TestOps
export/retained, phase/clock callers and SDK input/type tests (all
pass); `node --test` on the TestOps manifest, adapter, contract,
streaming and reconcile tests (all pass); `node
tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`
(unchanged).
- `node scripts/check-maps.mjs` passes. `tsc --noEmit` for `server`,
`ui` and `tests/runner-e2e`: no errors in changed files.
- Paid end-to-end run of both eval cases, run
[42872921890](https://github.com/paperclipai/paperclip/actions/runs/42872921890)
on
|
||
|
DottaandPaperclip
|
2150820bd8 |
perf(native): parallelize durability eligibility checks (#20646)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A web server must prove that retained run state is durable before volume-free enrollment. > - The proof reads selected native sessions and validates their existing recovery receipts. > - Serial database checks made a valid inventory take 59.665 seconds. > - The signed preparation request has a 55-second limit. > - This pull request overlaps independent reads within fixed limits. > - The benefit is a faster eligibility check with the same preservation rules. ## Linked Issues or Issue Description Refs #20599 and #20601. A search found no duplicate of this remaining serial-read bottleneck. **What happened?** An actual full startup eligibility check returned eligible with no blockers in 59.665 seconds. Controller checks took 44.134 seconds and harness checks took 8.592 seconds. Signed preparation timed out at 55.002 seconds. **Expected behavior** A durable instance can complete signed preparation within its existing deadline. **Steps to reproduce** 1. Retain many native task sessions with database-backed controller checkpoints and valid filesystem-independence receipts. 2. Run the full startup eligibility check. 3. Observe serial checkpoint and receipt reads consume the signed request budget. ## What Changed - Check the existing terminal-controller metadata and checkpoint-existence predicate within the existing eight-reader source batch. - Validate harness independence receipts with at most four concurrent readers per existing page. - Stop admitting more harness reads after an invalid result or error. Join all started reads before returning or throwing. - Keep large controller, full-checkpoint and legacy archive reads serial. - Add real PostgreSQL regressions for overlap, concurrency limits and failure settlement. ## Verification - Genuine controller regression RED: all 16 checkpoint reads ran, but peak overlap was one. GREEN: 10 focused PostgreSQL controls passed, including bounded overlap, joined failures, active and warm handoffs, and failed-source preservation. The other 45 cases were filtered, not claimed as passes. - Genuine harness regression RED: overlap was one instead of four. GREEN: all three new PostgreSQL controls passed. Six existing authority, replacement and refusal controls also passed. - Final head `c059e031`: the same 10 controller and 9 harness PostgreSQL controls passed after rebasing onto current master and refreshing Runner artifacts. `node node_modules/typescript/bin/tsc --noEmit -p server/tsconfig.json` also passed. The first `pnpm exec` attempt could not start the compiler because package-manager bootstrap fetches failed; it is not counted as a typecheck pass. - `git diff --check` passed. - Actual private-process acceptance with deployed `8e4c5e81` dependencies and only the two changed owners: the complete check passed in 37.596 seconds with no blockers. Nine runs were running before and after; 1,750 sessions were selected. This is 17.4 seconds inside the unchanged request limit. The earlier complete baseline was 59.665 seconds on 1,745 selected sessions, so these are observed timings on a changing dataset, not a controlled exact speedup. No serving process was patched. - The first private candidate check stopped on a native-controller blocker in 18.216 seconds. A targeted follow-up found no blocker without any preservation or business write. Its cause was not identified. It is retained as a nonpass and is not a speedup result. - Current-head Greptile review is 5/5 with no actionable findings. Required CI is running: https://github.com/paperclipai/paperclip/actions/runs/42898013184. ## Risks - The scans now use more simultaneous database reads. Limits remain eight for controller source readers and four for harness proofs. - Invalid or failed proofs still refuse eligibility. All started readers settle before failure leaves the scan. - Previous-release state carries forward unchanged. This changes no stored shape, checkpoint identity, lease rule, schema, deadline or provider protocol. - Large legacy payload reads remain serial, so their existing memory bound is unchanged. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. This is a measured performance defect in the existing enrollment path. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, code execution and parallel agents. The runtime does not expose the exact model variant or context-window size. ## 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 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 - [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> |
||
|
DottaandClaude Opus 5.5
|
4398a723c0 |
fix(native): settle tool callbacks and holds that a server restart orphaned (#20628)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs send provider tool calls to the server as durable
semantic inputs. The server runs the tool and saves the result in the
controller journal
> - Close must prove that every admitted tool call has a saved result.
Otherwise it reports `native_session_settlement_incomplete` and the
runner holds a cleanup gate (`native_session_cleanup_quarantined`)
> - A server restart in the middle of a tool call leaves its input
without a result. The new controller asks the tool authority for a
receipt. Tools without a receipt (assigned app tools,
`connections_search`) return nothing, so the input stays pending forever
> - Runs that already failed this way kept no-replay holds for a turn
that had completed. The task stayed blocked with no Retry and no
continuation
> - Restarts are normal. Work that a restart interrupts must settle by
itself, not wait for a person
> - This pull request settles such a callback as interrupted, with an
unknown outcome, when its owner is provably gone, and settles the holds
that earlier releases left once their controller is gone. It never runs
the tool or the turn again
> - The benefit is that close drains after a restart, and tasks that
earlier releases blocked this way continue on their own
## Linked Issues or Issue Description
Refs #20572 (that PR fixed the presentation of this quarantine. Its
Risks section names this runner drain rule as a separate follow-up.)
Refs #20641 (it settles a resolved no-replay hold when a sibling action
saved the run's files. This PR covers a different case: the hold's own
recovery saved the files, and its run's turn completed. Both use the
same recovery receipt, `completedWorkspaceRecoveryCondition`.)
**What happened?**
A remote native run started an assigned app tool call. The server
restarted 3 seconds later. The new server adopted the runner, and the
provider turn completed about 9 minutes later. The tool call never got a
saved result. Close reported `native_session_settlement_incomplete` with
`runnerSuspended: true`, `providerDrained: false` and one pending
callback. The session quarantined, and the same-run recovery was refused
at admission. A second run showed the same shape with a
`connections_search` callback after a later restart.
The first run then had two resolved-but-blocked holds with
`preserve_without_replay_v1` and an unknown action outcome: the
cleanup-quarantine hold, and a continuation hold whose workspace
recovery saved the files. The task view said "Saved work recovered. Send
a message to continue." with no Retry and no continuation. The task
waits on a child task and stays blocked.
**Expected behavior**
After a restart, a tool call without a saved result settles
automatically. A durable receipt is replayed. Without a receipt, the
agent is told that the outcome is unknown. Close proves the drain, and
no cleanup gate is left. Holds that earlier releases left for a
completed turn settle without a person, and the task continues when its
own wait ends.
**Steps to reproduce**
1. Start a native run and call an assigned app tool.
2. Restart the server while the tool call runs.
3. Let the adopted turn complete, then close the session. Close reports
`native_session_settlement_incomplete` and quarantines. The task stays
blocked by the no-replay holds.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
d06f43b14f |
fix(workspaces): settle a resolved no-replay hold once a sibling saved its files (#20641)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a remote workspace save fails, recovery actions hold the task until the files are saved > - One failed run can have two holds: its export hold, and a no-replay hold that a later "try again" opened for the same run > - The no-replay hold is recorded resolved-but-blocked, and it is the task's execution blocker > - #20569 lets the export hold retry under the new symlink capture policy and save the files > - The sibling settlement that should then release the no-replay hold only looked at active holds, so the task stayed blocked with no working Retry > - This pull request lets the sibling settlement and its discovery include resolved no-replay holds > - The benefit is that the task continues without a person once the files are saved ## Linked Issues or Issue Description Refs #20569 **What happened?** On staging, a native run on a Daytona sandbox failed to save its workspace (`remote_workspace_archive_unsafe_symlink`). Its export hold (`native_workspace_sync_out_retry_exhausted`) was exhausted after 6 attempts and handed to the board. A user comment then opened a second hold for the same run (`native_continuation_requires_reconciliation`, `preserve_without_replay_v1`). That hold also failed 6 times and was recorded `resolved` with outcome `blocked`. The task's execution blocker is this second hold. **Expected behavior** After #20569 is deployed, the worker re-arms the export hold and saves the files. The no-replay hold of the same run then settles, and the task is no longer blocked. **Steps to reproduce** 1. Create both holds with the exact staging evidence (see the new test `stagingIncident`). 2. Run the recovery worker until nothing is due. 3. Before this change, the export hold completes, but the no-replay hold stays `resolved`/`blocked` and `getExecutionBlocker` still returns it with `canRetry: false`. ## What Changed - `workspace-recovery-fallback.ts` (`settleSiblingRecoveredHold`): a resolved no-replay hold (`isRetainedFileHold`) can now settle when a verified receipt of a sibling action saved its run's files. Settling removes `automaticRecovery`, so the hold stops blocking the task. It starts no turn. - `workspace-recovery-fallback-discovery.ts`: the worker discovers such a resolved hold when a sibling of the same run is recovered. - New test in `workspace-recovery-capture-policy.test.ts` with the exact staging shape of both holds: board owner, `active_run_watchdog`, system recovery policy version 1 without a capture policy version, stage `restoring_files`, `export_interrupted` with cause `provider_error`, 6/6 attempts, `historicalWorkspaceSource` and `workspaceRestoreRecovery` lease ids. - `doc/execution-semantics.md`: states that a resolved no-replay hold also settles from a sibling save. ## Verification - Red before the fix: the new test failed. The no-replay hold stayed `resolved`/`blocked`, and it stayed the execution blocker. - Green after the fix: `npx vitest run --root server src/services/workspace-recovery-capture-policy.test.ts` (3 passed). The test drives the worker's own discovery (`nextWorkspaceRecoveryAt` + `reconcileRetainedWorkspaces`) until nothing is due. It checks: the export hold is re-armed once and saves the files; the no-replay hold is `resolved`/`restored` by the sibling; `getExecutionBlocker` returns null; the continuation hook ran when the task was free; no new run was created; the worker goes idle. - Related suites: `workspace-recovery-fallback`, `-admission`, `-stop-proof`, `-source`, `-preparation`, `-preparation-gates`, `execution-blocker-read`, `preserved-native-response-wait.integration`: 409 passed. - `npx tsc --noEmit -p server/tsconfig.json`: clean. - `pnpm verify` exits 2 (global doc input), so CI supplies the broad coverage. ## Risks - A resolved no-replay hold is now released when another action's verified receipt saved the same run's files. The receipt check is unchanged, so an unverified sibling still does not release it. - Settling starts no turn. Waiting input and the run's own result continue the task through normal admission, which still checks every other hold. - Previous-release story: holds saved by earlier releases need no migration. The worker discovers them on its first pass after deploy. > 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 - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
e3bc4ca798 |
fix(native): settle warm follow-ups that a restart left behind (#20631)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native warm runs on a remote sandbox continue the saved session of the previous run. Admission first proves what happened to the previous Runner. > - A deploy restarted the staging server while warm Runners were idle. Three follow-up runs then failed before provider work, each with a different refusal, and their tasks stayed blocked. > - The refusals asked for proof that a restart had made impossible. A restart is a product failure, so recovery must settle it automatically and keep the strict checks for real uncertainty. > - This pull request accepts the proof that the restart leaves behind, in three places: the warm owner on a replaced sandbox, the suspended Runner with an undelivered suspend receipt, and the restore hold of a run that saved nothing new. > - The benefit is that these follow-up runs start, and state saved by older releases carries forward without a migration. ## Linked Issues or Issue Description Refs #20567, #20346 **What happened?** On staging, a deploy replaced the webserver process at about 18:04 UTC on 2026-11-10 (lease controller boot `348ec0bb` on one deployment, then boot `3e84c195` on a new deployment that started at 18:04:16). Three runs of one Codex agent on Daytona failed between 18:08 and 18:15: 1. A warm follow-up got a newly created sandbox. The previous warm Runner stayed `ready` in its own retained sandbox. Live adoption denied it with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate` (`remote-warm-session-recovery.ts:127`), because only a resumed sandbox may be adopted. The per-turn retirement path refused warm owners (`lifecycle_not_per_turn`). 2. A warm follow-up resumed the same sandbox and directory. The previous Runner had been suspended by the shutdown, but the controller stopped before it acknowledged the suspend result. The Runner kept its terminal fence (`pendingTerminalDelivery` for `runner.suspend`). The read refused it with `native_controller_recovery_missing_evidence:runner_not_settled` (`native-session-executor.ts:7032`). 3. A continuation into a fresh workspace failed with `remote_workspace_recovery_receipt_invalid` (`remote-workspace-recovery-attachments.ts:60`). Its predecessor had restored the committed save, failed before provider work, and kept a restore hold with no recovery receipt. Its saved reference named the snapshot it restored. **Expected behavior** After a restart, the next run continues. It retires a finished owner that nobody can adopt, reads a finished suspension, and skips a source that left no unsaved files. Real uncertainty still refuses with a named reason. **Steps to reproduce** 1. Run a warm native Codex turn on a reusable Daytona sandbox and let it commit. 2. Restart the server (deploy) while the Runner is idle. 3. Start the next turn. Get a new sandbox (case 1), or resume the same sandbox after the shutdown suspended the Runner (case 2). 4. For case 3, continue a task whose previous run restored the last save and failed before provider work, into a fresh workspace. **Paperclip version or commit** Staging before `2e3cbe57` (the runs predate that deploy). The gaps remain on `master` at `9b21dcbd`. ## What Changed - `inspectRemoteWarmSessionRecovery` returns `null` when this run's sandbox is `created` or `replacement`. Such a sandbox holds no Runner to adopt. The caller then uses ordinary admission. - `inspectFinishedPerTurnRunnerdOwner` also retires a **warm** owner on a remote sandbox runner when this run's sandbox is `created` or `replacement`. It uses all the existing proofs: terminal run, committed result, released coordinator lease, stopped or fenced controller, settled journal, and quarantine copies. Local warm Runners and resumed sandboxes keep the live path. - `restoreRemoteSuspendedRunnerCheckpoint` accepts a suspended Runner whose only open fence is its own undelivered `runner.suspend` receipt. The fence must name `runner.suspend` with lifecycle `suspended`, it must be the last controller command, and the restored journal must not name that command differently. A provider cleanup, unsent events, a turn's terminal fence or a warm transition still refuse with `runner_not_settled`. A later authorized Runner process reconciles the receipt, as the Runner already does. - `attachPreservedRemoteWorkspaceSnapshots` skips a source with a receipt-less restore hold when its saved reference names the exact committed snapshot that its own succeeded `remote_workspace_restore` loaded at the same epoch. Any other saved snapshot still needs the canonical receipt. - `doc/execution-semantics.md` describes the three rules. ## Verification - Red (unfixed code), each in the exact staging shape: - `native-recovered-owner-followup.test.ts` "retires a finished warm owner left behind by a restart when this run's sandbox is created/replacement" failed with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. - `native-controller-recovery.integration.test.ts` "defers a remote warm continuation…" failed: live adoption rejected a created/replacement sandbox with the same error. - `native-controller-recovery.integration.test.ts` "reads a resumed sandbox's suspended Runner with an undelivered suspend receipt" failed with `native_controller_recovery_missing_evidence:runner_not_settled`. It reads through a sandbox runner that refuses paths outside the workspace base. - `workspace-recovery-preparation.test.ts` "decides a receipt-less restore hold of a source that restored_unchanged" failed with `remote_workspace_recovery_receipt_invalid`. - Green: the same tests pass. Focused files pass: `native-recovered-owner-followup.test.ts` and `native-controller-recovery.integration.test.ts` (104 tests), `workspace-recovery-preparation.test.ts` (85 tests). - Safety cases (strict checks stay strict): - A warm owner on a resumed sandbox is not retired. - A warm owner with an unsettled journal (pending command, unanswered tool input) stays held and names `retirement_refused=journal_…`. - A warm owner whose finalization lease is still held stays held. - A turn terminal fence, a provider cleanup, unsent events, a fence behind a later command, or a journal that names the command differently still fail with `runner_not_settled` and cache nothing. - A restore-hold source that saved another snapshot, whose restore did not succeed, or whose restore was at another epoch still fails with `remote_workspace_recovery_receipt_invalid`. - Neighbouring suites pass: `native-session-executor.test.ts`, `plugin-runner-release-resume.integration.test.ts`, `explicit-native-continuation.test.ts` and `native-harness-filesystem-independence-continuation.integration.test.ts` (1786 tests). The last one asserted the old warm refusal on a created sandbox. It now shows that the warm follow-up resumes the same provider session for the new run. - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` returns exit 2, because a global doc input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. ## Risks - Previous-release state: all three rules read state that earlier releases already save (lease outcome, coordinator rows, Runner state in the sandbox, journal commands, run results and workspace operations). Rows saved by older releases carry forward. No migration. - Case 1 starts a new Runner for a warm session whose old Runner may still exist, idle, in its retained sandbox. Its controller is gone and its result is committed, so it can accept no new commands. The provider session follows the existing resume-or-replace policy. - Case 2 trusts a suspend fence only in the exact shape that the Runner writes before it sends the result. Other fences still refuse. - After case 1, a warm follow-up resumes from its own warm checkpoint. That checkpoint carries no replacement policy, so a failed provider resume is still reported as `native_session_recovery_failed` and is not replaced. This is the existing warm behavior. - Case 3 skips a source only when it provably saved nothing beyond a committed snapshot. That snapshot stays in its own workspace. - The three tasks that already failed stay blocked until their next wake or a user message. This PR does not add an automatic continuation for a completed no-replay hold. A follow-up can add that. - `native-runner-restart-recovery.integration.test.ts` (runs only with a runner container) no longer probes a replacement sandbox, because that probe now retires the owner by design. - Not verified live on Daytona. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, read-only browser access to staging). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
24661683db |
fix(recovery): bootstrap a remote follow-up that a restart cut before its Runner launched (#20642)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a control-plane restart, restart recovery classifies each claimed native run: reattach a live Runner, resume from a checkpoint, or bootstrap again when durable records prove no provider work > - A follow-up run on a continued session gets a rebound admission checkpoint. It names the new run, but it keeps the previous turn's provider session ID > - Recovery read that copied provider session as this run's provider work. A follow-up killed after `native.process_start_requested` but before its remote Runner launched was classified `reattach_remote_runner` > - The reattach then had no Runner to find, failed with `native_continuation_requires_reconciliation`, and the task needed Retry > - This pull request stops the copied session from counting as this run's work, and keeps the existing durable proofs as the authority > - The benefit is that these restarts bootstrap again by themselves and do not repeat provider work ## Linked Issues or Issue Description Refs #20561, #20555 **What happened?** Two Daytona restart-chaos tasks needed a Retry click: - Wave `w-20261110T173243-s9202` t02 (claude C1, K2): the follow-up run was killed at `stage.asset.home`, after `native.process_start_requested`. Recovery chose `reattach_remote_runner`. Verification failed with `native.remote_runner_recovery_failed {stage: runner_state, reason: state_read_failed}` (`runner_remote_state_unavailable: exit=1`): `runner-state.json` did not exist because the Runner never launched. The storage fault window (ST7) had ended 30 s before, so it was not the cause. - Wave `w-20261110T174930-s9205` t04 (codex C1, RR1): the scheduled retry of a `process_lost` run was killed at `session.checkpoint.restore`, after `native.process_start_requested`. Recovery chose `reattach_remote_runner`. Verification failed with `native_remote_recovery_lease_mismatch: stage=runner_verification field=identity_run_id`: the local Runner identity belonged to the previous run, because this run's Runner never started. **Expected behavior** A run whose durable records prove that no Runner received a provider turn bootstraps again without a person. **Steps to reproduce** 1. Start a remote follow-up run on a continued session (for example, a comment after a completed turn). 2. Kill the server after `native.process_start_requested` and before the remote Runner launches. 3. Observe `reattach_remote_runner`, a failed verification and a Retry banner. **Paperclip version or commit** master close to `6d6b2d0e` (waves); still reproduced by the new tests on master `90dd864bff`. ## What Changed - `native-restart-recovery.ts`, no controller checkpoint for the run (`remote_runner_never_received_commands`): a follow-up with `sessionIdBefore` qualifies when it carries only its continued-session checkpoint, and a predecessor's settled Runner identity (`carriesInheritedRunnerIdentity`) no longer disqualifies it. A recorded process identity, a Runner-sent event, a provider event or a pending request still keeps the run held. - `native-restart-recovery.ts`, controller checkpoint exists: the existing Postgres proof (`nativeRunnerCheckpointRemoteBootstrapProof`) now also runs when the run carries only a rebound checkpoint whose provider session a settled earlier run of the same session saved (`carriesSettledPredecessorSession`). The copied provider session then does not count as this run's provider evidence. The controller checkpoint stays the authority: any turn command or turn event keeps the run held. - `doc/execution-semantics.md`: describe the rule. ## Verification - `server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts` (real Postgres): - "restarts a follow-up whose controller never saved a checkpoint (s9205 t04)" and the variant "with its predecessor's remote runner identity": red on master (`reattach_remote_runner`), green now. - "restarts a follow-up whose saved controller checkpoint proves no provider turn (s9202 t02)": red on master, green now. - Held cases stay held: own recorded Runner identity, a carried active turn, a provider session that no settled earlier run saved, a `turn.start` command, a projected `turn.started` event. - Whole file: 171 passed. - `npx tsc --noEmit -p server` passes. - The executor's existing `bootstrap_incomplete` path handles both shapes: a scoped root that belongs to the previous run goes through the existing prior-owner verification, and a root of this run restored from the controller checkpoint goes through `runnerdStateProvesIncompleteBootstrap` / `runnerdStateProvesNoRemoteProviderTurn`. ## Risks - A run with any recorded Runner process, Runner-sent event, provider event, turn command or pending request keeps today's reattach or hold. - A bootstrap claim charges one provider attempt, as today. The bounded limit (3) still applies. - No persisted shape changes. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude
|
ed5e7493bb |
fix(native): take back a lapsed lease held under the run's own restart claim (#20625)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an execution lease on its `native_run_finalizations` row. The owner renews it every 5 minutes. After a host sleep (or the restart-chaos time fault TM2), the stored lease looks lapsed although the owner is alive. > - #20474 lets the same owner take back a lapsed lease when no other actor recorded anything on the row. One of the checks is "`recovery_state` is null". > - Restart recovery writes `recovery_state` (`resuming_session`, `awaiting_runner_reattach` or `bootstrap_incomplete`) in the same update that grants the lease. The executor keeps that value until a result is accepted. > - So a run that a restart resumed can never take back its own lapsed lease. Renewal interrupts the turn, and failure settlement is refused with `native_session_lease_lost`. The run fails with `adapter_failed`, recovery holds it with `preserve_without_replay_v1`, and the user must click Retry. > - This pull request counts the claim's own record as untouched when the row still shows exactly that claim and nothing recorded since. > - The benefit is that a resumed run survives a later host sleep with no Retry. Every real takeover still fences the late owner out. ## Linked Issues or Issue Description Refs #20474 **What happened?** The zero-retry restart-chaos matrix (local wave L7, seed 9107: TM1, TM2, TM3 with 1 h aging) had 3 of 6 tasks that needed Retry. All three were Codex runs. In each run, a TM3 kill made restart recovery resume the run (`resume_dead_runner`, generation 2). The runner restored the conversation, reported `provider_turn_lost_on_restore`, and the same-conversation restart continuation started as designed. Then the TM2 fault aged every database time by one hour. The execution lease lapsed while its owner was alive. The run then failed with `native_session_lease_lost` (`adapter_failed`), and recovery stopped with `native_continuation_requires_reconciliation` / `preserve_without_replay_v1`. One run had already proposed its result and completed its turn. The Retry click worked every time. **Expected behavior** A run whose lease lapsed only because time jumped, and that no other actor claimed, continues with no Retry. This must also be true after a restart resumed that run. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex,claude --workloads A1,B5,C1 --faults TM1,TM2,TM3 --time-age-hours 1 --concurrency 3 --tasks 6 --seed 9107` ## What Changed - `server/src/services/native-runtime/native-execution-lease.ts`: the "untouched" part of `nativeExecutionLeaseHeldCondition` now also accepts the run's own local restart-recovery claim. All of these must be true: - the last `recovery_history` entry is that claim: same controller boot and generation as the row; - its disposition matches the current `recovery_state` (`resume_dead_runner` → `resuming_session`, `reattach_existing_runner` → `awaiting_runner_reattach`, `bootstrap_incomplete` → `bootstrap_incomplete`); - `recovery_request_id` equals the request that claim recorded. Any later actor appends a history entry (shutdown suspension, failure settlement), moves the state (`awaiting_evidence`, `blocked`), or replaces the owner, boot or generation. Each of these still fences the late owner out. All other untouched checks are unchanged. - `native-execution-lease-reacquire.test.ts`: new cases for each claim kind, a requested claim, failure settlement under the claim, and eight fenced variants. - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: Step 1 lists the new rule. ## Verification - Red/green: `cd server && npx vitest run src/services/native-runtime/native-execution-lease-reacquire.test.ts`. Without the source change, 5 new cases fail (the three claim kinds, the requested claim, and failure settlement). With it, all 37 pass. - Related suites pass: `native-execution-lease.test.ts`, `heartbeat-restart-claim-lapse.test.ts`, `heartbeat-native-orphan-sweep-lease-race.test.ts`, `native-runner-restart-lease-renewal.integration.test.ts`, `native-runner-restart-preparation.integration.test.ts`. - `cd server && npx tsc --noEmit` passes. - Live: the same L7 wave (seed 9107, local) on this branch. The server log has 0 `native_session_lease_lost` (the baseline had 3). The two tasks whose cells failed this way in the baseline (codex C1 TM1, codex C1 TM3) now pass with no Retry. Two other tasks needed Retry for different causes, which this pull request does not change: codex B5 TM2 (a `runner.drain` timeout, then a same-run retry that may not continue a turn lost on restore) and claude C1 TM2 (`ACPX session is not open` on `turn.start` until the retry budget ran out). The L6 wave (seed 9106) also ran on this branch: 5 of 6 passed. Its one failure is `execution_recovery_budget_exhausted` and is not related to leases. ## Risks - The rule widens when an owner may take back its own lapsed lease. It applies only to local runs, before a result, with the same owner token, attempt, boot, pid, process start and generation. A claim that any other actor changed afterwards does not match. - Rows saved by earlier releases carry the same history entry shape, so a run claimed before a deploy is handled the same way. A deploy changes the boot, so the new process never takes back the old process's lease. - No schema change and no migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). Extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
fd6e71a7ff |
fix(native): restore a warm backup when its source sandbox is idle (#20627)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs on remote sandboxes keep a warm sandbox and a verified harness backup so the next run continues the same provider session > - A run that starts while the previous run's workspace export is still saving gets a new (created) sandbox lease, because reuse skips an exporting sandbox > - When that run prepares the harness, it finds a backup but no recorded reusable lease, and it fails closed with `runner_harness_state_mismatch: backup_without_reusable_lease` > - #20278 accepted a destroyed source and a settled restore hold as retired backup sources, but not an idle warm source that is still `retained` for reuse > - This pull request accepts an idle warm source under the same proofs, claims it so it is never resumed again, and covers the resumed lease that the failed run leaves behind > - The benefit is that these recoveries continue the same session automatically instead of blocking the task ## Linked Issues or Issue Description Refs #20278 **What happened?** A monitor wake started about 90 seconds after the previous run on the same task finished. The previous run's warm sandbox lease was `retained` with policy `reuse_by_environment`, but its workspace export was still saving. Reuse skipped it and created a new sandbox. Workspace realization waited for the export (about 136 s) and restored the saved files. Then harness preparation found the previous run's backup (its lease stamp names this exact manifest, and no provider work followed it). The source lease was not destroyed and was not a restore hold, so the run failed with `native_session_recovery_failed: Error: runner_harness_state_mismatch: backup_without_reusable_lease` and the task was blocked. **What did you expect?** The run continues the same provider session. The idle source sandbox holds exactly the backed-up state, so restoring the backup into the new sandbox is safe. **Steps to reproduce** 1. Finish a warm native run on a remote sandbox. Its workspace export is still saving. 2. Start the next run on the same task before the export settles. 3. Reuse skips the exporting sandbox and creates a new lease. Harness preparation fails with `backup_without_reusable_lease`. 4. A retry resumes that new lease, which has no harness state, and fails with `resumed_unavailable`. ## What Changed - `native-retired-backup-source.ts`: an idle warm source is a third retired backup source. It must be `released` or `retained` with policy `reuse_by_environment`, its own run must be terminal and not the current run, and it must have no cleanup in flight, no restore hold or pending shutdown settlement, and no pending native workspace export. The backup stamp, single-owner chain, and later-provider-work checks still apply. - Before the restore, the idle source is claimed with a compare-and-swap on the checked row. The claim moves its `reusableSandboxLease` scope into a `reusableSandboxLeaseSuperseded` receipt. Reuse never resumes that older state again, and the next acquisition treats it as obsolete and destroys it. If a run resumed or changed the source first, the claim matches no row and the backup is not restored. - `environments.ts`: a cross-run reuse handoff now requires that the source row has no superseded receipt. The supersede claim and the handoff compete on the same row, so an acquisition that selected the source before the claim cannot resume it afterward. - The proof now rejects a backup whose source is the target allocation itself (`targetProviderLeaseId`). - `native-session-executor.ts`: a resumed lease whose sandbox has no harness state at all (`unavailable`) restores the backup under the same proof. This recovers the lease that the failed run left behind, so rows saved by older releases recover after deploy without a migration. - `doc/execution-semantics.md`: documents the idle-source rule, the supersede claim, and the resumed stateless lease. ## Verification - Red before the fix, green after: - `native-session-executor.test.ts` "restores a warm backup into a created lease only when an idle warm source finished its run and its export" failed with `backup_without_reusable_lease`. - "... a resumed lease never held the session and the warm source is idle" failed with `resumed_unavailable`. - `native-retired-backup-source.integration.test.ts` (real Postgres) "authorizes an idle retained/released warm source and claims it as superseded" failed (returned false). - Integration test "refuses a reuse handoff of a source superseded while its provider resume was pending" fails without the handoff guard and passes with it. - Safety rows stay refused: source run still running, workspace export pending, cleanup in flight, restore into its own allocation, stamp names another checkpoint, provider work after the backup, two host leases for one allocation, claim lost to another run, and a resumed lease that lost its own state. Existing destroyed-source and restore-hold rows are unchanged. - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-retired-backup-source.integration.test.ts src/services/native-runtime/remote-warm-session-recovery.test.ts` passes (1408 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2 because a global CI input (`doc/execution-semantics.md`) changed. Required CI supplies the broad coverage. ## Risks - Previous-release story: no persisted shape changes. Lease rows saved by earlier releases carry forward. The new `reusableSandboxLeaseSuperseded` metadata key is additive. - The idle source is destroyed at the next acquisition for its workspace. Its state is the verified backup, which the restored lease now holds, so no work is lost. - Not covered: when the source cannot be proven retired or idle (for example a live owner or later provider work), the run still fails closed with the same named code. An automatic fresh-session fallback for that case is not in this PR. - The root trigger (reuse skips a sandbox while its export saves, then waits for that export) is unchanged. This PR does not change the reuse lookup. > 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 - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, with tool use and code execution in Claude Code. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
76c787fffc |
fix(runner): report monitor notes length limits (#20604)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents use task monitors to resume a future check. > - The monitor schema limits notes to 500 characters. > - The field description did not state the limit. Rejection showed only the schema path. > - This pull request adds the limit to the description and to safe length errors. > - Agents can correct notes before they retry a rejected call. ## Linked Issues or Issue Description Related prior fix: #16868. **What happened?** A native `set_task_monitor` call with 501 characters in `monitor.notes` failed with a path that ended in `notes/maxLength`. The error did not state 500. The schema already had `maxLength: 500`, but the field description did not state it. **Expected behavior** State the limit in the field description and in the rejection. Accept 500 characters. Reject 501 characters before dispatch. Preserve a saved monitor when a replacement is invalid. **Steps to reproduce** 1. Supply a future `monitor.nextCheckAt` and 501 characters in `monitor.notes`. 2. Read the native provider error. 3. Before this fix, it shows the constraint path without the maximum. **Paperclip version or commit** Reproduced on `7368a0b15c2f02107778a961144d86e2eb59a952` in a Linux source checkout, with Node 25.9.0 and Rust 1.97.1. ## What Changed - Use one local value for the published notes limit and its description. The server test checks agreement with the persisted shared validator. Keep the standalone Runner package boundary. - Read the numeric maximum from the authorized JSON Schema validation error. Keep submitted values private and preserve the 512-byte feedback bound. - Test the published schema and real bridge dispatch path at 500 and 501 characters. Test saved contents and rejection without monitor changes with real PostgreSQL. - Refresh generated contracts and document the limit. - Sync two SDK proof lock identities after the reviewed master lock refresh in #20615. This fixes the remaining CI proof mismatch. Keep compiler, parser, Zod, and all rejection checks unchanged. ## Verification - Final head: `bac60234ab3a8fc7043a90c7f9c81b02e250428b`. - Red: the server regression failed because the field description omitted `500`. The Rust regression failed because the error omitted `maximum 500 characters`. - Local passes after the final base update: 44 PostgreSQL monitor/scheduler tests, 14 real-source Rust bridge tests, 11 cache/Docker guard tests, and 56 SDK proof tests. Runner TypeScript build and generated contract checks pass. - The focused Rust check compiles the actual source in a small Cargo workspace. The full Rust CI suites also pass on Linux and Windows. - Final-head [PR CI run](https://github.com/paperclipai/paperclip/actions/runs/42881633978) passes. This includes typecheck, build, Runner support, Runner Rust and Vitest, server tests, serialized tests, and all eight e2e shards. - Final-head [Windows Rust run](https://github.com/paperclipai/paperclip/actions/runs/42881633419) passes. - Greptile: 5/5 on the final head, with no open findings. There are 61 successful checks and five conditional skips. The separate Paperclip Review remains queued; its comment says work is paused. No CI or test check is pending or failed. - `pnpm verify` returned exit 2 and selected broader CI. The full local suite was not run. Final-head CI supplies that coverage. Earlier failed attempts were repaired and are not counted as passes. ## Risks - Low risk. The existing 500-character bound, scheduling semantics, and permissions stay in place. - Length feedback uses only a schema path and numeric bound. It does not print submitted text. - The Runner binary and tool contracts must reach an instance for the new guidance to apply. This PR does not deploy or merge. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The runtime does not expose a more specific model ID or context window size. ## 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> |
||
|
DottaandClaude Opus 5.5
|
38944cff1a |
fix(environments): retry and safely replace a reusable sandbox that will not resume (#20626)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task that runs on a reusable (warm) cloud sandbox resumes the same sandbox on the next wake > - When a follow-up arrives while the provider is auto-stopping or archiving the idle sandbox, the resume can fail > - Each failed attempt waited the provider's whole five-minute start deadline and then failed with `reusable_sandbox_resume_failed`, even when the workspace was saved and nothing lived only on the sandbox > - Two such runs in a row made a user wait for many minutes and recorded two failed runs > - This pull request bounds the resume, retries a state change inside the run, and replaces the sandbox from the saved snapshot when that is safe > - The benefit is that recovery works automatically and the first turn is not delayed by minutes ## Linked Issues or Issue Description **What happened?** Two consecutive runs failed before provider dispatch with: `Reusable sandbox lease "<id>" could not be resumed (cause=provider_error); the lease was preserved and no replacement was created.` Each run took about 5.5 minutes (320s and 330s) in preparation. The third run resumed the same sandbox from `stopped` in 4.7 seconds after the provider settled. The previous run had ended with a clean final save and a verified harness backup. **Steps to reproduce** 1. Let a task's reusable sandbox go idle so that the provider auto-stops or archives it. 2. Send a follow-up while the provider still reports the sandbox as changing state. 3. The Daytona worker waits in `ensureSandboxStarted` until its 300s start deadline and throws "is still <state> at its start deadline". The host does not classify that as transient and it has no safe fallback, so the run fails. **Expected behavior** The run retries briefly. If the sandbox still cannot start and nothing lives only on it, the run continues on a replacement sandbox that restores the saved workspace. **Root cause** `server/src/services/environment-runtime.ts` (reusable resume block, about line 1820): the resume deadline was the provider's full start deadline (300s plus RPC margin), the transient classifier did not recognize a state transition, and the catch always threw `ReusableSandboxResumeError`. #20212 waits out a state change inside the same 300s start deadline, so it covers the start but not the time budget or the fallback. ## What Changed - Add `server/src/services/reusable-sandbox-resume-replacement.ts`: - `REUSABLE_SANDBOX_RESUME_BUDGET_MS` (90s): the total resume time for one run, retries included. - `sandboxStateTransitionFailure`: "state change in progress" and "is still <state> at its start deadline" are transient. - `retireUnresumableReusableSandboxLease`: replace only when the lease is `released`/`retained` with no live owner, the task workspace is clean with a committed saved snapshot and no other writer, and a `paperclip_runner` session has a verified lease-bound harness backup. Access refusals (401/403) and an unavailable worker stay strict. After the replacement lease (which carries the harness backup) is recorded, the old lease moves to `pending_cleanup` (reason `resume_failed`) in one guarded write, so the existing cleanup sweep destroys the old sandbox and no provider delete runs before the first turn. A failed creation or a restart before that write leaves the old lease `retained` for the next attempt. - `environment-runtime.ts`: cap the resume deadline at the budget, treat a state transition as transient, and call the replacement policy before failing. A replacement records `sandboxLeaseAcquisition: { outcome: "replacement", reason: "resume_failed" }`; workspace preparation then restores the saved snapshot. - `remote-workspace-transport.ts`: add the closed failure label `sandbox_state_transition`, so a strict failure names the cause instead of `provider_error`. - `doc/execution-semantics.md`: document the budget, the in-run retry, the replacement conditions, and the strict cases. - Update the two slow-start tests: the resume RPC timeout is now 90s instead of 330s/150s. A 61s start still resumes the same sandbox. ## Verification - Red before the fix (new file `server/src/__tests__/environment-runtime-reusable-resume-replacement.test.ts`, staging shape: lease created by the real acquisition path, then retained after a clean save with a backup stamp, 300s provider deadline, worker refusal with no status): 3 tests failed with `could not be resumed (cause=provider_error); the lease was preserved and no replacement was created.` - Green after the fix: 10/10 pass, including a case where the replacement creation fails and the next run replaces the sandbox with the backup carried forward. The safe case replaces the sandbox, keeps every resume RPC at or below 90s, makes no synchronous destroy call, and moves the old lease to `pending_cleanup`. A single state-change refusal resumes the same sandbox inside the run. Six strict cases keep the lease and fail with `cause=sandbox_state_transition`: unsaved work, no saved version, an uncommitted saved version, a live owner, no verified backup, and a 403. - `pnpm exec vitest run src/__tests__/environment-runtime.test.ts` (197 pass), `heartbeat-retry-scheduling`, `legacy-execution-recovery`, `remote-workspace-transport` (251 pass). - `tsc -p server/tsconfig.json --noEmit` passes. - `pnpm verify` exits 2 (a doc change is a global CI input). Required CI supplies the broad coverage. - Not run: a live Daytona run on staging. The provider message is kept off the run record, so the exact transition state on staging (stopping or archiving) is inferred from timing, not observed. ## Risks - A strict resume now gives up after 90s instead of 300s. The provider start continues in the background, and the bounded bootstrap retry resumes the same sandbox. A stopped sandbox resumes in seconds and an archived one in about 25s on Daytona, so 90s leaves headroom. - A replacement leaves the old sandbox to the cleanup sweep. If the old start finishes later, the sweep still destroys it. The guarded write only claims a `released`/`retained` lease with no live owner. - Upgrade safety: no new persisted fields are required. Leases saved by older releases are read with the same rule; one without a verified backup stays strict. The replacement reason `resume_failed` already exists in the acquisition outcome. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
070d6f83fd |
perf(realtime): share permission reads among queued live events (#20624)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - A task page shows the output of a running agent as it is produced.
> - The server sends each output event to the browser on the company
WebSocket.
> - Before each send, the server checks permissions and redacts secrets.
Each event did its own sequential database reads.
> - When output arrives faster than those reads complete, the queue
grows. The text reaches the browser later and later, and the server then
closes the connection.
> - This pull request lets the events that are already in the queue
share one set of reads.
> - The benefit is that live output keeps pace with the run, and a
revoked permission still applies to the next event.
## Linked Issues or Issue Description
**What happened?**
On a hosted instance, the live output of a running task sometimes
appeared very slowly. A probe WebSocket on the company event stream
received events 0.5 seconds after their creation time at the start.
After 30 seconds the delay was 23 seconds. The server delivered about 6
events each second. Then it closed the socket with code 1013.
The delivery loop for a connection is serial. For each transcript event
it ran a membership read, a scope decision, a run lookup and a task read
decision. Native run events added a settings read and a secret registry
read. A burst of 200 events issued 1,401 SQL statements.
**Expected behavior**
Live output reaches the browser at the rate the run produces it. The
cost of the permission checks does not grow with the number of events in
a burst. A revoked permission stops delivery immediately.
**Steps to reproduce**
1. Use a deployment that authenticates users, with a database that has a
network round trip.
2. Start several agent runs that produce output at the same time.
3. Open the task page of one run and read the live output.
4. Compare the creation time of each `heartbeat.run.log` or
`heartbeat.run.event` message with its arrival time.
5. The delay grows continuously until the server closes the socket with
code 1013.
**Paperclip version or commit**
`master` at
|
||
|
DottaandPaperclip
|
89ec4e7614 |
feat(links): add cached GitHub pull request previews (#20593)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task conversations contain links to pull requests and other work. > - A URL alone cannot tell the operator whether a PR is open, merged, or blocked by conflicts. > - Fetching every link at render time wastes API calls and can expose private data through a shared cache. > - This pull request adds a demand-driven links service and a GitHub PR adapter. > - Each viewer and company has a separate cache. Repeated links share one request and result. > - Operators get context when they hover or focus a link. Normal links stay useful when a provider is unavailable. ## Linked Issues or Issue Description **Subsystem affected** Task Markdown links, shared link metadata, and the server GitHub API boundary. **Problem or motivation** PR links have compact labels and neutral icons. They do not provide a rich hover card or a common status source across repeated mentions. A background resolver cannot borrow a person's GitHub connection because it has no viewer identity. **Proposed solution** Recognize supported links locally. Request metadata after a delayed hover or keyboard focus. Show a card at the first line of the link, aligned with its icon. Share the result across all mentions in the current viewer and company. Keep the exact original navigation URL. The first adapter supports GitHub pull requests. Open is green, merged is purple, and merge conflicts are orange. Draft, closed, unknown, loading and unavailable states remain quiet. **Alternatives considered** Eager page scans would spend API quota on links the operator does not inspect. Company-wide caching of personal GitHub results would cross the credential boundary. This change uses neither approach. **Roadmap alignment** ROADMAP.md was checked. This extends existing task links and the GitHub resolver. It adds no deployment adapter or new background worker. Searches of open and closed PRs for link previews and PR hover cards found no matching implementation. ## What Changed - Add host-owned link recognition and a normalized preview contract. The service selects an adapter by provider. - Add a board-only company read endpoint. It checks company access before cache reads and sends a private, no-store response. - Use the current viewer's eligible GitHub connection. A configured connection that cannot supply credentials fails closed. With no managed connection, reuse existing company credentials or public access. Instance credentials retain the public-repository check. - Reuse the GitHub external-object resolver for API transport and response parsing. Add title and mergeability to its normalized snapshot without changing its existing state contract. - Coalesce requests by company, viewer and canonical PR URL. Bound cached entries and concurrent work. Cache success for one minute and failures for a short period or the provider retry window. - Add an abortable request deadline. Timed-out work stays in the concurrency budget until it settles. No periodic worker or idle polling is added. - Add a shared browser query and anchored preview card. Repeated Markdown links observe the same result. There are no automatic retries, window-focus refreshes, toasts or connection prompts. - Add ten production-component Storybook examples and real pointer/keyboard browser checks. Document the API and verification route. ## Verification - Linux source checkout: 233 focused tests passed in nine suites. They cover recognition, fixed API destinations, provider states, access failures, credential selection, viewer/company isolation, request coalescing, expiry, cache bounds, concurrency bounds and mounted OpenAPI coverage. - A live public GitHub smoke returned this PR's correct title and open state. Two equivalent URLs shared one real network request. Personal connection access was not part of that smoke. - A controlled HTTP server stalls a real response body. The test proves cancellation while another lookup completes. - UI typecheck and the server layered typecheck passed. The UI production build passed. - Token gates and query policy gates passed. All 30 map/catalog support checks passed. The three new server test files also passed a focused TypeScript check. - All 27 focused browser checks passed on the reviewed source tree. They cover light/dark themes, narrow layouts, open/merged/conflict/draft/closed/loading/unavailable/failed states, fixed positioning during pointer movement, exact navigation and keyboard access. A real browser HTTP interception counts one deferred lookup for two equivalent PR links. - The first browser attempt lacked Chromium. A later combined build/test attempt exhausted the sandbox's 8 GiB memory limit and killed Storybook. Those attempts supply no pass evidence. Chromium is installed, and the final browser checks run with the build stopped. One initial Storybook dependency reload interrupted a streaming check. All eight existing inline-link cases passed on the warmed server, followed by a complete 27-case pass. - Synthetic Storybook pointer events did not exercise the native hover boundary. The assertions now run through real Playwright pointer and keyboard actions. The fixture stories retain the production components. - Review fixes preserve the Draft label when GitHub reports both draft and conflicts. They also preserve neutral link colors in dark mode. The 25 affected service checks pass, including the added combined-state regression. Browser checks compare unknown, draft and closed links with the muted theme color. - The first CI policy run rejected a literal retry opt-out. The code now uses the existing explicit zero-retry helper. A new test-spy type error was also corrected. These failures are fixed; their earlier runs remain failed. - `pnpm verify --explain` returned exit 2 because the browser test path selects broad CI. No full local suite is claimed. The final CI run supplies that coverage. - The previous head `437913c797963aa113bb82e2ae8b836969317ef9` passed CI and received a 5/5 Greptile score. All four review threads are resolved. Master then advanced. The current head `ae2a1c2adbd61a621cca9bdd160561952613df6e` resolves the source-manifest conflict against master `9b21dcbdda01296349f3cfe0a3a6891e79df6130`. Fresh CI and review are running. - The previous CI run found a lost class on decorated PR links and a stale SDK source manifest. Both are fixed. The existing CommentThread integration test and 26 focused link service tests pass. All 45 SDK source/privacy tests pass. All eight source-manifest helper tests pass. The manifest helper changed only source identities and added the two new shared files. Runtime, compiler, parser and library pins stay unchanged. - A later review found that expired timeouts could reuse a settled promise. The service now removes settled requests from its shared-request map and counts unfinished provider work separately. The new regression failed before the fix and passes after it. The concurrency-bound and real stalled-HTTP tests also pass. - UI typecheck and token gates pass after these fixes. The browser rerun needed Chromium in the new session. That first attempt did not exercise the UI. The complete 27-case browser rerun passed before the master merge with retries disabled. All 27 browser cases also passed on the merge revision with retries disabled. - After the master merge, all 84 focused link, OpenAPI and integration tests pass. All 55 SDK source/privacy/manifest tests pass. UI typecheck and token gates pass again. The source manifest retains master's reviewed runtime pins and includes both sets of source changes. GitHub confirms the PR can merge cleanly. The preflight helper rebuilt the shared package and plugin SDK; Cargo is unavailable locally. Final CI supplies Rust coverage. - The latest master update changed the reviewed report-lock policy but left the SDK lock hash at its previous value. The SDK source/evidence checks reproduced the mismatch. This change sets only the SDK lock hash to the authenticated output of the existing master policy and patch. Compiler, parser and Zod pins stay unchanged. All 56 SDK source/evidence/manifest tests now pass. All 84 focused link/API/integration tests, UI typecheck and token gates also pass on the updated source. ## Risks - This cache is process-local. Separate server replicas can make separate GitHub requests. - A cached result can remain visible for its short lifetime after GitHub changes. A later hover refreshes expired data. - GitHub can report unknown mergeability. The UI shows conflicts only when GitHub explicitly reports them. - Personal preview data stays in viewer/company caches. It is not written to company external-object records or persisted browser query storage. - Stories simulate the API. They do not prove a live GitHub login or private-repository access. - No schema migration is required. ## Model Used OpenAI Codex, based on GPT-6. The runtime does not expose the exact serving model ID or context-window limit. Used reasoning, repository inspection, source editing, shell execution and browser tools. No subagents were used. ## 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> |
||
|
DottaandClaude Opus 5.5
|
42d4499489 |
fix(daytona): force a stuck stop before starting a retained sandbox (#20609)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On Daytona, a follow-up task resumes the agent's retained sandbox before the agent starts > - The host stops an idle sandbox gracefully. A process that ignores SIGTERM can leave Daytona in `stopping` for many minutes > - Resume waited for that state change until its 300 s start deadline. It never escalated, so each attempt failed after five minutes, and the bounded retries did the same > - On staging, a user comment waited about 21 minutes before the agent started. Two resume attempts each used 299.7 s in the plugin dispatch > - This pull request forces a stuck stop once, then starts the sandbox within the same deadline. It also traces the wait with the observed state > - The benefit is that a stuck stop costs about 30 s instead of failing the run, and the next case is diagnosable from the trace ## Linked Issues or Issue Description **What happened?** A user commented on a task that used a retained Daytona sandbox. The first run and its first retry each failed after about 5 min 20 s with `reusable_sandbox_resume_failed (cause=provider_error)`. The Honeycomb trace shows `execution.plugin_dispatch` at 299.7 s in both. Inside it there is only `lease.lookup` (44 ms) and `lease.channel_cleanup`, and no `lease.start`. So the whole budget went to `waitOutSandboxTransition`, which polled a sandbox that stayed in a transition state. The later attempt, after a control-plane restart, found the sandbox `stopped`. Its `lease.start` took 1.3 s. A transition that ends in `stopped` is a stop. **Expected behavior** A sandbox that stays `stopping` for longer than the host's own graceful-stop budget is forced to stop, then starts. The run does not fail on the start deadline. **Steps to reproduce** 1. Retain a Daytona sandbox whose graceful stop does not finish (Daytona reports `stopping`). 2. Resume or realize the lease. 3. Before this change, the call polls until the 300 s deadline and fails with "is still stopping at its start deadline". ## What Changed - `packages/plugins/sandbox-providers/daytona/src/plugin.ts`: `waitOutSandboxTransition` tracks how long the sandbox stays `stopping`. After `STUCK_STOPPING_FORCE_AFTER_MS` (30 s) it calls `sandbox.stop(seconds, true)` once, then keeps polling. A refused force stop keeps the old wait within the same deadline. - The wait is a `lease.wait_transition` span. The force stop is a `lease.force_stop` child span. - `lease-tracing.ts`: the closed `paperclip.sandbox.startup.state` label also records `starting`, `stopping`, `restoring`, `archiving` and `resizing`. Before, these were all `other`. - `server/src/services/plugin-host-services.ts`: the host admits the two new span names and the new closed state values. - `doc/observability.md`: documents the new spans and states. ## Verification - Red/green: with the plugin change stashed, the three new stuck-`stopping` tests (realize, refused force stop, resume) fail by hanging to the deadline. With the change they pass. The "settles on its own" test confirms that no force stop happens for a normal stop. - `vitest run` in `packages/plugins/sandbox-providers/daytona`: 14 files, 618 tests passed (8 skipped). `tsc --noEmit` passed. - `server/src/__tests__/plugin-host-services-span.test.ts`: red before the allowlist change, 25/25 after. - `pnpm verify` exited 2 (the doc is a global CI input), so CI supplies the broad coverage. - Not verified on a live Daytona sandbox. Hosted acceptance needs the change deployed to staging. ## Risks - The force stop sends SIGKILL to processes in a sandbox that is already stopping and is about to start. The disk persists across a stop, so the retained workspace is kept. A graceful stop that is still running past 30 s is killed. The host's own graceful stop budget is 30 s, so the host had already given up on it. - Daytona can refuse a stop while a state change is in progress. Then the code waits as before, so the result is no worse than today. - The root cause is inferred from the trace (a transition that ends in `stopped`). The new state label confirms or refutes it the next time it happens. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, browser, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
9b21dcbdda |
refactor: remove SPEC docs and repoint spec citations (#20565)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors and agents read the docs under `doc/` before they change code > - `doc/SPEC.md` and `doc/SPEC-implementation.md` were out of date. `doc/SPEC-implementation.md` became an append-only log: in 60 days it got about 1,645 new lines and lost 181, across 140 commits > - #20557 changed `AGENTS.md` so the code and its tests are the source of truth, and the specs are only background > - This pull request removes the two spec docs. It moves only the sections that live code comments cite into focused docs, and it repoints all other references > - The benefit is one less stale document that agents read and append to ## Linked Issues or Issue Description Refs #20557 (merged). This PR is the follow-up that removes the docs. It is based on `master`, which already contains #20557. **Issue type** Outdated content. The docs are out of date and conflict with the code. **Where is the issue?** `doc/SPEC.md`, `doc/SPEC-implementation.md`, and the code comments and docs that cite them. **What's wrong?** `doc/SPEC.md` and `doc/SPEC-implementation.md` are out of date, but agents still read them and append a paragraph with each fix. **Suggested fix** Delete both files. Move the sections that code cites into focused docs. Repoint the references. ## What Changed - Deleted `doc/SPEC.md` and `doc/SPEC-implementation.md`. - Moved these spec sections, because live code comments cite them: - §9.3.1 "Shared default-open issue writes" → new section in `doc/agent-permission-defaults.md`. - §9.8.1 "Issue-thread interaction resolver contract" → `doc/execution-semantics.md`, "Issue-thread interaction resolution". That section already had most of the contract. I added only the missing parts: the `supersedeOnUserComment` default, the canonical policy list, the addressee and cap rules, the rule that surfaces show the server's effective audience, and the cross-issue budget charge. The short policy summary at the top of the section now points to the list, so the definitions occur one time only. - §9.9 "Task Watchdog Authority Contract" → new "Authority contract" section in `doc/TASK-WATCHDOG.md`. It holds the allowed and disallowed mutation lists and the deferred capabilities. The atomic recovery batch, restoration verification, and interaction rules were already in `doc/execution-semantics.md` §11, so the new section links there and does not repeat them. I did not move the "Downstream acceptance criteria" checklist or the sentence about the CTO and Security sign-off. These were implementation-time notes; the tests now hold that coverage. - §9.10 "Company Skill Policy Contract" → new `doc/company-skill-policy.md`. No focused doc had this contract. I did not move the "Required regression tests" phase checklist. - Did not move any other part of the specs. - Repointed code comments that cited spec sections: - `packages/shared/src/issue-write-denial.ts` (§9.3) → `doc/agent-permission-defaults.md` - `server/src/routes/issues.ts` (§9.9) → `doc/TASK-WATCHDOG.md`, "Authority contract" - `server/src/routes/issues.ts` (§9.3, §9.8.1 cross-issue budget) → `doc/agent-permission-defaults.md` - `ui/src/lib/interaction-audience.ts` and `ui/src/components/InteractionGovernancePanel.tsx` (§9.8.1) → `doc/execution-semantics.md` - `ui/src/lib/skill-policy-denial.ts`, `ui/src/pages/CompanySkills.tsx`, `ui/src/pages/CompanySkills.production.tsx`, `ui/src/pages/SkillStudio.tsx` (§9.10) → `doc/company-skill-policy.md` - `server/src/services/recovery/ownership-invariant-report.ts` (monitor deadline wait path) → `doc/execution-semantics.md` §8. That section already lists the one-shot monitor as a valid wait path. - Resynced the git blob pin for `packages/shared/src/issue-write-denial.ts` in `tests/runner-e2e/source-sdk-public-inputs.json`. The comment edit changed the file's hash, and the Runner support suite checks that pin. I used `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --write`; only that one pin changed. - Updated `AGENTS.md` §1 so it does not name the deleted files. - Removed the links to the deleted files from `doc/plugins/PLUGIN_SPEC.md`. - Updated the `doc-maintenance` agent skill (`.agents/skills/doc-maintenance/`) so it no longer audits or stages `doc/SPEC.md`. - Removed or replaced references in `maps/README.md`, `maps/feature-map/README.md`, `maps/storybook-map/README.md`, `doc/PRODUCT.md`, `doc/CLIPHUB.md`, `doc/DEPLOYMENT-MODES.md`, `doc/DEVELOPING.md`, `doc/evals.md`, `doc/spec/agent-runs.md`, `doc/AGENTCOMPANIES_SPEC_INVENTORY.md`, `doc/connections/CONNECTOR-PLAYBOOK.md`, `doc/plugins/ideas-from-opencode.md`, `doc/execution-semantics.md`, `doc/TASK-WATCHDOG.md`, and `.devin/wiki.json`. - In `doc/execution-semantics.md`, the "admission is independent of cleanup and accounting" reference now states the rule inline: only a hard stop, an explicit pause, or an inactive company refuses admission. - Kept these references as they are: dated plans in `doc/plans/` and `docs/plans/`, the dated log `doc/logs/2026-05-24-cli-api-parity-e2e-log.md`, `releases/`, the transcript fixture `ui/src/fixtures/runTranscriptFixtures.ts` (it uses the filename only as sample text), and `docs/specs/external-task-protocol.md` (it refers to the Symphony project's own `SPEC.md`). ## Verification - `git grep -n "SPEC-implementation\|SPEC\.md"` (without `releases/`, and ignoring `PLUGIN_SPEC.md` citations) finds only the kept references listed above. - `grep -rn "§9\.\(3\|8\|9\|10\)" ui/src server/src packages/shared/src` finds no results. - `node scripts/check-maps.mjs`: "Maps aligned: 35 areas, 192 capability IDs". - `pnpm check:token-gates`: "All gates clean". - `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/source-sdk-public-type.test.ts tests/runner-e2e/source-sdk-evidence.test.ts`: 45 passed. These failed in CI before the pin resync. - `pnpm verify`: exit 2 ("Broader verification required") because `AGENTS.md` is a global CI input. Required CI supplies the broad coverage for this revision. - All code changes are comments only. No runtime behavior changes. The title uses `refactor:` because comments in source files changed. ## Risks - Low risk. The change removes docs and edits comments only. - A reader can lose context from spec sections that I did not move. The text stays in git history. The code and its tests are the source of truth. - External links to the two deleted files will return 404. > 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 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edits). Standard context window. ## 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 checks above; CI supplies broad coverage) - [x] I have added or updated tests where applicable (not applicable: docs and comments only) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
3bf7af6c77 |
perf(plugins): read the lifecycle inbox from per-resource cursors (#20576)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins get agent, project and agent runtime changes from the `resource_lifecycle_events` journal. Each plugin acknowledges events one resource at a time. > - #20405 made the inbox page read linear in the company journal size. The journal has no retention, so each page still reads every event of the company to find the unacknowledged ones: about 1.7 s per 1M events. > - The delivery worker reads a page again after each acknowledgment. The idle-sleep check also reads the whole journal when all events are acknowledged. `acknowledge` checks every older event of the resource. > - On a hosted instance, the page read ran 181 times in one window at about 1 s each. > - This pull request adds a per-plugin, per-resource cursor. Reads start at the cursor, so they do not read acknowledged history again. > - The benefit is that page, idle-check and acknowledgment cost follow the number of resources and pending events, not the journal size. Delivery order and results stay the same. ## Linked Issues or Issue Description Refs #20405 **What happened?** The plugin lifecycle inbox page query (`select ... from "resource_lifecycle_events" ... DISTINCT ON ...`) ran 181 times at about 1 s each on a hosted database in one window. Each acknowledgment causes another full read of the company journal. The idle-sleep check (`hasPending`) reads the full journal when all events are acknowledged. `acknowledge` reads the full history of the resource. **Expected behavior** A page read, the idle-sleep check and an acknowledgment must not read acknowledged history again. Their cost must not grow with the journal size. **Steps to reproduce** 1. Give a company 400 agents with 2,500 `agent_runtime` events each (1M events). 2. Acknowledge the first 2,000 events of each agent for one plugin. 3. Call `pluginLifecycleInbox(db, pluginId).list(companyId, 100, undefined, ["agent", "project", "agent_runtime"])`. 4. The read takes about 1.7 s. `hasPending` takes about 240 ms. **Paperclip version or commit** `8d2bf78323` (master). **Deployment mode** Hosted (PostgreSQL). Local repro uses embedded PostgreSQL 18. ## What Changed - `packages/db/src/schema/plugin_lifecycle_resource_cursors.ts`: new table `plugin_lifecycle_resource_cursors`. It has one row per plugin, company, resource type and resource id. `acknowledged_through_id` means: every event of that resource at or below this id is acknowledged by the plugin. Plugin and company deletion cascade. - Migration `0462_plugin_lifecycle_resource_cursors`: creates the table and carries existing acknowledgments forward (see Risks for the previous-release story). - `server/src/services/plugin-lifecycle-inbox.ts`: - `list` and `hasPending` use a backward loose index scan on `resource_lifecycle_events_resource_order_idx`. It visits each resource stream once and gets its latest event id. A stream whose latest event is at or below its cursor is settled and is not read again. For other streams, the read gets one head: the unacknowledged creation, else the first unacknowledged transition above the cursor. Then it applies `afterId`, id order and the limit, as before. - `acknowledge` reads only the events between the cursor and the acknowledged event. It still writes the per-event `plugin_lifecycle_acknowledgments` row. It moves the cursor only over a contiguous acknowledged prefix. A creation recorded after a pending transition does not move the cursor. - An event counts as acknowledged when it is at or below the cursor, or when it has an acknowledgment row. Thus rows written by an earlier release, and gaps in old history, keep their current behavior. - `server/src/__tests__/resource-lifecycle-events.test.ts`: new database tests for the migration carry-forward, gaps, late creations, cursor reads, the rollback rows and the plugin-delete cascade. All tests from #20405 still pass without change. - `scripts/benchmark-plugin-lifecycle-inbox.ts`: runs the migration backfill on the fixture and times it. Adds an acknowledgment case. Runs `VACUUM ANALYZE`, because hosted tables are autovacuumed. - `doc/DATABASE.md`: describes the cursor, its read cost and the migration. - The delivery worker (`plugin-lifecycle-delivery.ts`) is not changed. ## Verification Thesis: the cost is a read of the whole company journal (page, idle check) or the whole resource history (acknowledgment). If a per-resource cursor marks the acknowledged prefix, each read can skip that prefix. Then the cost follows the number of resource streams and pending events. The rows must stay equal. Local SQL timings: embedded PostgreSQL 18.1 on Apple Silicon, synthetic data, warm cache, one warmup, 7 samples, baseline and treatment order alternate per sample. The machine was shared with other jobs (load average about 35), so spread is wide. Values are median (min to max) in ms. Rows are equal (`assert.deepEqual`) for every list and probe case. Fixture A: 400 agents x 2,500 runtime events, 2,000 acknowledged per agent. 1,020,440 events, 800,420 acknowledgments. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,700 (1,633-2,438) | 11.5 (9.5-12.2) | 148x | | page of 100, agent and project | 3.3 (2.5-5.4) | 4.2 (3.6-6.7) | 0.8x (slower) | | idle-sleep check | 238 (174-367) | 2.7 (2.1-3.4) | 88x | | acknowledge one head | 15.3 (9.4-30.1) | 20.3 (12.9-29.9) | neutral (noise) | | migration backfill | - | 9.3 s | - | Fixture B: 40 agents x 25,000 runtime events, 24,000 acknowledged per agent (long history). 1,020,080 events. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,943 (1,642-4,979) | 4.4 (2.9-5.6) | 444x | | page of 100, agent and project | 3.5 (2.1-5.7) | 4.6 (2.8-12.0) | 0.8x (slower) | | idle-sleep check | 234 (220-243) | 2.8 (1.9-6.5) | 85x | | acknowledge one head | 64.6 (48.5-71.6) | 5.0 (2.6-7.1) | 13x | | migration backfill | - | 4.6 s | - | Fixture C: 5,000 agents x 200 runtime events, 190 acknowledged per agent (many resources). 1,025,040 events. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,867 (1,649-1,975) | 76.9 (66.2-497) | 24x | | page of 100, agent and project | 10.3 (8.7-12.4) | 21.6 (20.2-23.2) | 0.5x (slower) | | idle-sleep check | 220 (201-298) | 2.8 (2.0-5.3) | 78x | | acknowledge one head | 4.9 (3.4-8.6) | 6.0 (5.2-7.6) | neutral | | migration backfill | - | 15.6 s | - | `EXPLAIN (ANALYZE, BUFFERS)` for the all-types page on fixture A: before 2,488 ms, 13,597 shared hits plus temp files; after 7.0 ms, 7,926 shared hits. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=400 --per-agent=2500 --acked=2000 --samples=7 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=40 --per-agent=25000 --acked=24000 --samples=7 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=5000 --per-agent=200 --acked=190 --samples=7 cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/project-instance-secrets.test.ts src/__tests__/agent-runtime-hooks.test.ts pnpm --filter @paperclipai/db check:migrations ``` - `resource-lifecycle-events.test.ts`, `plugin-lifecycle-delivery.test.ts` and `project-instance-secrets.test.ts`: 86 passed. `agent-runtime-hooks.test.ts`: 22 passed. - `tsc --noEmit -p server`: pass. `check:migrations`: pass. ## Risks - **Schema change.** Migration `0462` adds one table and two foreign keys to `plugins` and `companies`. It adds no index on an existing table. The backfill reads the acknowledgment table once. It took 4.6 s to 15.6 s on 1M-event fixtures with 0.8M to 1M acknowledgments. - **Migration order.** The number `0462` is reserved after open PRs that use `0459` to `0461` (#20571, #20568, #20573). Merge this PR after them. Before merge, rebase onto master and chain the snapshot after the latest migration. - **Previous-release story.** - Upgrade: the backfill stores, per plugin and resource, the end of the contiguous acknowledged prefix. A cursor never passes an unacknowledged event, so no event is lost. Acknowledged events above a gap keep their rows, so no event is delivered again. A test runs the migration statement on rows that only an earlier release wrote and checks that `list` returns the same rows before and after. - Rolling deploy: an earlier release that still runs writes only acknowledgment rows. The new reads honor those rows. - Rollback: the new code still writes every acknowledgment row, so an earlier release sees the same progress. - **Ordering assumption.** The cursor is safe because writers record a resource's events while they hold that resource's row lock (`paperclip_record_agent_runtime` locks the agent row; agent status and project writers document the lock). Thus a later commit for the same resource always has a higher id. A writer that breaks this contract could commit an event below the cursor, and that event would not be delivered. A lower id from a different resource is still delivered (existing test). - **Slower path.** A page that asks only for `agent` and `project` streams is 1 to 11 ms slower. These streams are short, so the old full read was cheap, and the new read visits each stream once. The cost is bounded by the number of resources. - **Many resources.** The page cost grows with the number of resource streams of the requested types. At 5,000 agents a page is about 77 ms, compared with about 1.9 s before. - The hosted planner can choose a different plan. The hosted improvement is not yet measured. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL benchmarks). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
19e938415a |
fix(runtime): omit Codex refresh handoffs from backups (#20614)
## Thinking Path > - Managed ACPX Codex retains its final refresh after provider exit so Core can save it. > - Shutdown saves a checkpoint before Core removes the live credential handoff. > - Checkpoint profiles must exclude the handoff while retaining resumable session history. ## Linked Issues or Issue Description Follow-up to #20580 and its credential-backup review finding. Refs #20457. ## What Changed - Exclude `auth-refresh.json` and `auth-refresh.json.tmp` from the shared Codex checkpoint profile, covering native and ACPX homes. - Extend real checkpoint-copy coverage to ACPX Codex, both handoff filenames, and native Codex. Keep the live files and session history intact. ## Verification - A real ACPX checkpoint-copy test failed before the exclusion fix; Grok passed. - Five targeted real-copy/profile tests pass, including after applying the change to current master. - `git diff --check` and local secret/PII diff review pass. - Full regression coverage runs in GitHub CI at the maintainer's request. No live provider credentials were used. ## Risks - Two entries added to an existing exclusion list; no runtime lifecycle, schema, persisted identity, or session format changes. Saved sessions remain readable. Existing backup restoration uses the same exclusion profile. - Provider credential cleanup still follows final writeback; excluding a backup entry does not delete the live handoff. ## Model Used - OpenAI GPT-6 through Codex, with tool-assisted implementation and verification. The runtime does not expose a more specific model ID or context-window size. ## 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> |
||
|
DottaandClaude Opus 5.5
|
86cc3e3a11 |
fix(workspaces): save remote workspaces that contain outside symlinks (#20569)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents on a remote sandbox (for example Daytona) save their workspace after each run, and recovery retries a failed save > - The save refused the whole workspace when one symlink was absolute or pointed out of the workspace > - Ordinary agent work makes such links: a headless Chromium profile links `SingletonSocket` by absolute path into `/tmp` > - Recovery then spent its full budget on the same refusal, handed the hold to the board, and the task stayed blocked > - This pull request keeps confined links, records and omits outside links, and retries saves that an older policy exhausted > - The benefit is that these saves succeed and blocked tasks continue without a person ## Linked Issues or Issue Description **What happened?** On staging, a native run (OpenCode on `paperclip_runner`, Daytona sandbox) built Storybook and drove Chromium with `--user-data-dir=<workspace>/.review-chrome`. Chromium wrote `.review-chrome/SingletonSocket -> /tmp/.org.chromium.Chromium.*/SingletonSocket`. The save failed three times with `remote_workspace_archive_unsafe_symlink` (`native_workspace_sync_out_retry_exhausted`). System recovery then tried six more times. It recorded each failure as `export_interrupted` with cause `provider_error`, became `exhausted`, and gave the hold to the board. The task and its parent stayed blocked. **Expected behavior** The save succeeds for an ordinary workspace that contains symlinks. Links inside the workspace stay links. Absolute and escaping links are dropped and recorded, and are never followed. Holds that an earlier release exhausted recover automatically after deploy. **Steps to reproduce** 1. Create a workspace with `node_modules/.bin/tool -> ../tool/cli.js`, an absolute link to a file outside the workspace, and a link `escape -> ../outside/file`. 2. Capture it with `captureRemoteWorkspaceArchive` (or the serialized sandbox helper). 3. Before this change, the capture fails with `remote_workspace_archive_unsafe_symlink`. ## What Changed - `remote-workspace-snapshot-archive.ts`: capture records an absolute or escaping symlink in the manifest `excluded` list and omits it, instead of failing the save. It never follows a link. Confined links are kept. Restore validation is unchanged and still refuses an archive that names an outside link. - Added `REMOTE_WORKSPACE_CAPTURE_POLICY_VERSION = 2`. Each new system recovery generation records `capturePolicyVersion`. Generations without the field count as version 1. - New `workspace-recovery-capture-policy.ts`: an exhausted save whose last failure was in `preserve_files` (`export_interrupted`) under an older capture policy gets one new bounded generation. Restore refusals (`archive_unsafe`) are not re-armed, because restore validation is unchanged. It shares the run-over guard (`sourceRunProvablyOver`) with the stop-proof re-arm. It writes a `capturePolicyRecovery` marker and `issue.workspace_recovery_rearmed` activity with reason `capture_policy_changed`. The new generation has the current version, so the rule cannot repeat for it. - The recovery worker discovers these holds (`capturePolicyRearmPendingCondition` in `nextWorkspaceRecoveryAt` and the obligations query). - Docs: `doc/native-workspace-finalization-recovery.md` (symlink policy) and `doc/execution-semantics.md` (capture-policy re-arm). Previous-release story: archives saved by the previous release never contain outside links, and restore of them is unchanged. Recovery state saved by the previous release has no `capturePolicyVersion`; it is read as version 1 and, if exhausted in `preserve_files`, is re-armed once on the first worker pass after deploy. No migration is needed. A capture already in flight at the cutover uses the helper that its operation installed; the next attempt installs the new helper. ## Verification Red before the fix (same tests, unfixed code): - `remote-workspace-snapshot-archive.test.ts` "keeps confined links and drops outside links" (local and serialized helper): failed with `remote_workspace_archive_unsafe_symlink`. - `remote-workspace-transport.test.ts` "saves a workspace with outside links through the sandbox helper" (split and Daytona one-shot output): failed with `remote_workspace_archive_unsafe_symlink`. - `workspace-recovery-capture-policy.test.ts` "retries once under the new policy": failed, because the worker did not discover the exhausted hold (`nextWorkspaceRecoveryAt` returned null). Green after the fix: - `npx vitest run --root server src/services/remote-workspace-snapshot-archive.test.ts src/services/remote-workspace-transport.test.ts src/services/workspace-recovery-capture-policy.test.ts`: 271 passed. - The recovery test uses the exact staging evidence shape (6 attempts, `export_interrupted`, `provider_error`, board owner, no policy version). It ends `complete` and `recovered`, saves a snapshot, and leaves the source untouched. - Safety cases: the archive does not contain the outside bytes; restore refuses a hand-made archive with an escaping link; a hold exhausted under the current policy is not re-armed and no sandbox resume is attempted. - `npx tsc --noEmit -p server/tsconfig.json`: clean. - `pnpm verify`: see CI for the broad selection. ## Risks - A tracked symlink that is absolute or escaping is now omitted. After restore it shows as a deletion in `git status`. Before, the whole save failed. - On deploy, every hold exhausted in `preserve_files` with `export_interrupted` gets one more bounded recovery generation, including holds that failed for provider reasons. This is one generation per hold per policy version. - The manifest exclusion label stays `paperclip.remote-workspace-exclusions.v1`. Workspaces that the old policy accepted produce the same manifest, so unchanged-snapshot reuse still works. > 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 - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use and code execution. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
8e4c5e81b9 |
perf(issues): read created-from tasks by index and inbox review waits by page (#20573)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The task page shows "Created from this task". The inbox shows blocked-inbox attention for a page of tasks. > - On a hosted database, the created-from list ran about 4,000 times in one afternoon at about 499 ms each. The task page sends it each time a task opens. > - Its filter was an `OR` of three provenance paths over the list's own rows. PostgreSQL could not use an index for it. It read every task of the company and tested each one, with the list's other per-row conditions (such as the chat conversation state subquery). > - The blocked-inbox attention map also read external review waits for every in-review task of the company, not only for the tasks of the page. > - This pull request reads created-from tasks from three index probes, and reads review waits only for the in-review tasks that can change a finding of the page. > - The benefit is less database work on each task open and each inbox page. The responses stay the same. ## Linked Issues or Issue Description Refs #20400 (shares review-wait reads between the attention maps of one request). Refs #20551 (authorizes the review waits of a page with one read; this change narrows the waits that the page asks for, and adds a test that pins the batched read). Refs #13241 (adds the created-from list). **What happened** - `GET /api/companies/:id/issues?createdFromIssueId=…` used `createdFromIssueCondition`. Its filter was `origin_issue_id = source OR (origin_issue_id IS NULL AND origin_run_id IN source runs) OR (… AND EXISTS creation activity of a source run)`. The `OR` stops index use, so the statement read every task of the company. The list's other conditions, which include the chat conversation state subquery, ran for each task before the task was excluded. - `listIssueBlockedInboxAttentionMap` read review waits for every in-review task of the company graph. The caller uses the findings of the page rows only. **Expected behavior** The created-from list reads only the created tasks. The blocked-inbox map reads review waits only for in-review tasks that the liveness classifier can read for a page row. The rows, the order, and the attention values do not change. **Steps to reproduce** 1. Use a company with tens of thousands of tasks, runs, and activity rows. 2. Open a task page. Look at the created-from list statement: it reads the full task set of the company. 3. Open an inbox page with blocked-inbox attention. Look at the review-wait read: it gets the ID of every in-review task of the company. ## What Changed - `server/src/services/issue-creation-origin.ts`: `createdFromIssueCondition` is now `issues.id <> source AND issues.id IN (three branches)`. Each branch starts from an index: - recorded origin task: `issues_company_origin_issue_idx`; - legacy origin run: the source task's runs (`heartbeat_runs_company_origin_issue_idx`), then the new partial index on `issues (company_id, origin_run_id)`; - creation activity: `activity_log_run_id_idx` for the source runs, then the task by primary key. The precedence does not change: a recorded origin task wins, the run path applies only without one, and the activity path applies only without both. The activity path matches only an `entity_id` that is the canonical text of the task ID, as before (it also compares `id::text = entity_id`). - `packages/db`: add `issues_company_legacy_origin_run_idx` on `issues (company_id, origin_run_id) WHERE origin_issue_id IS NULL AND origin_run_id IS NOT NULL`. Migration `0461` creates it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS` and is registered with the other concurrent migrations. `doc/DATABASE.md` documents the index and the benchmark command. - `server/src/services/issues.ts`: add `blockedInboxReviewWaitIssueIds`. It returns the in-review tasks in the blocker closure of the page rows. The blocked-inbox map reads review waits only for these tasks. The liveness classifier reads a wait only for a row or for a blocker that it reaches from a row, and the caller reads only the findings of the rows. The classifier does not change. - Tests: - `issue-created-from-routes.test.ts`: new cases for precedence in both directions, a source-run comment, an activity row whose task has another origin run, a non-canonical and a non-UUID activity entity ID, the source's own creation activity, and other-company rows. - `blocked-inbox-review-wait-scope.test.ts`: the narrowed set for chains, cycles, shared blockers and off-page tasks. The rows get the same findings with the narrowed waits as with the waits of every in-review task, and each narrowed wait is needed. - `issue-list-review-wait-authorization.integration.test.ts`: on real PostgreSQL with two authorized waits, `listExternalReviewWaits` and one issue-list request with blocked-inbox attention each send one bound-run message-link statement. With the per-wait authorization from before #20551, both tests fail (2 statements). - `scripts/benchmark-created-from-issues.ts`: local benchmark. It loads the baseline `issues.ts` and `issue-creation-origin.ts` from a pinned commit, asserts identical complete row payloads, and reports alternating timings and `EXPLAIN (ANALYZE, BUFFERS)`. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/issue-created-from-routes.test.ts src/__tests__/blocked-inbox-review-wait-scope.test.ts src/__tests__/issue-list-review-wait-authorization.integration.test.ts src/__tests__/agent-task-hierarchy-depth.test.ts src/__tests__/issue-blocker-attention.test.ts src/__tests__/issue-list-assignee-filter-routes.test.ts src/__tests__/issues-parent-id-alias.test.ts src/__tests__/batched-chat-authorization.integration.test.ts src/__tests__/issue-liveness.test.ts` passes (126 tests). One timing test in `issue-list-assignee-filter-routes` failed one time under machine load and passed on rerun. - `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. - `pnpm verify` asks for broader verification because the change adds a file under `scripts/`. Required CI supplies that coverage. - Benchmark: `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-created-from-issues.ts --baseline-ref=bd30ef8510` Fixture (synthetic, local PostgreSQL 18.1): one company with 30,000 tasks (10,628 with a recorded origin task, 10,445 with only a legacy origin run), 60,030 runs with wide context snapshots (145 MB), and 261,079 activity rows; a second company with 10,000 tasks and 20,000 runs. The busy source task has 63 created tasks through all three paths, plus decoys. The quiet source task has 3. The UI's page: `sortField=id`, `asc`, `limit=500`, routine executions included. Both versions return identical complete payloads. 9 alternating pairs after 2 warm-up pairs: | Case | List statement before | List statement after | List call before, median (min to max) | List call after, median (min to max) | | --- | --- | --- | --- | --- | | Busy source (63 tasks) | 234.9 ms, 47,437 buffers | 0.90 ms, 437 buffers | 156.0 ms (114.5 to 480.7) | 31.6 ms (19.9 to 62.7) | | Quiet source (3 tasks) | 90.7 ms, 47,413 buffers | 0.25 ms, 46 buffers | 126.1 ms (90.6 to 300.4) | 10.5 ms (7.2 to 26.1) | The list call time includes the enrichment reads (labels, runs, attention), which do not change. Inbox page (50 open tasks, blocked-inbox attention): the review-wait read goes from 2 statements with 1,215 parameters (every in-review task of the company) to 1 statement with 31 parameters. The page time does not change in this fixture (median 801 ms before, 873 ms after, both with a wide spread), because the fixture has no review waits and the other graph reads of the map are most of the time. With real waits, the server does an inherited GitHub review read for each wait it finds, so the gain grows with the number of off-page waits. Migration timing (local PostgreSQL 18.1): `0461` (measured as the same statement under its earlier number) on 500,000 tasks (846 MB, 20 companies, one third with a legacy origin run) takes 1.6 s. The index is 12 MB. ## Risks - The new index adds a small write cost to tasks with only an origin run. New tasks with a source task record `origin_issue_id`, so the index holds mostly older tasks. The migration does not lock writes. - The created-from rewrite must keep the same rows. The new tests and the benchmark payload check cover precedence, company scope, and ID text forms. Visibility and read filters are the list's other conditions and do not change. - The review-wait narrowing depends on the liveness classifier reading waits only for rows and their blocker chains. A unit test compares the findings of the rows with full and narrowed waits. A future classifier change that reads waits of other tasks must update `blockedInboxReviewWaitIssueIds`. The function comment says this. - The local timings are synthetic. Hosted acceptance needs a fresh query-statistics window after deployment. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edits, local PostgreSQL) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
12f70d0d4c |
Keep Slack form custody and receipt transfer durable (#19825)
## Thinking Path > - Paperclip manages work for AI-agent companies. > - Chat connections deliver provider messages and user answers. > - Slack requires a response within its original three-second deadline. > - Forms and their delivery receipts must survive retries and tenant restarts. > - This change uses the existing connector queue and durable form owners. > - Current authorization, exact receipt checks and normal ACK rules stay in force. > - The tenant can settle useful work while keeping the connection enabled. ## Linked Issues or Issue Description Refs #19019 and #19348. Companion Cloud PR: paperclipai/paperclip-cloud#1250. ## What Changed - Preserve form grants, sessions, encrypted evidence, terminal receipts and company-authorized Retry. - Check initial form enrollment before the provider send. Keep a typed preparation wait only while an exact current enrollment obligation exists. - Refund the attempt only when the claimed attempt and credential lease still match and provider invocation has not started. Possible or accepted sends retain uncertain-delivery custody. An ordinary error with the same text still uses the normal five-attempt limit. - Notify the existing publication worker when normal enrollment commits. Reconstruct pending work from durable state after a lost reply or owner restart. - Page settled history before proving idle safety. Keep unfinished, malformed and foreign receipts visible beyond the first 1,000 rows. - Keep Slack callback, answer, receipt and ACK rules in their provider owners. Support body-bound Cloud idle capabilities when a complete catalog exceeds the header budget. - Add focused regression coverage, an enabled quiet/reactivation recipe and explicit fixture port checks. - Keep SDK evidence tests tied to a normally built reviewed reference, with a separate strict refusal for actual unreviewed CI outputs. ## Verification App head: `6284d6881b5dc38a9ee149bcbde35e69a67dd8ca`, including actual master `dff8bed007e01c32f3826144b893ec80603d1335`. Companion Cloud head: `f83b38187dc7e6b2e219fe865dc16d268cc66fdb`, including actual Cloud master `b1dcc3421a91efffee631a61536cf48f357c22fc`. Cloud #1250 is merged at `1d07d81f23d4627ab7b6dcf49176b4128a2f8f9b` after exact-head passing checks and complete Greptile 5/5. Current exact-head CI typecheck, build, tests and aggregate verification are green; the complete edited Greptile review is 5/5 and all 13 review threads and nested comment pages are resolved. - Both latest enrollment findings were reproduced against the preceding product. The pure readiness control returned no wait for the first enrollment. The real-PostgreSQL control observed a provider call while that enrollment was still staged. - The repaired source passes three whole files: 21 readiness controls, 80 real-PostgreSQL milestone controls and 37 tenant-actor middleware controls. All 138 pass with zero excluded or TODO cases. The SQL case reconstructs the worker through seven waits, preserves the attempt budget, recovers a controlled lost enrollment reply, and commits one provider-port call and one exact grant. Replaying the selected attempt does not duplicate them. Authorization, SDK and enrollment-response ports are controlled in that SQL case. - The same files cover ordinary five-attempt exhaustion, accepted and possible sends, stale ownership, malformed bindings, attention states and current actor restrictions. One synthetic mismatched-binding parameter uses generation 2 against current generation 1; it is not evidence for an older-generation control. - The normal server-package typecheck passes at the measured source. It runs the normal Runner TypeScript, protocol, SDK and declaration prerequisites and all seven main App programs. The separate declaration configuration is also recorded. A worker union-narrowing error was found, fixed with an explicit callable/null branch, and verified by that normal compiler. No cast or test relaxation was used. - The normal merge of current master preserves all 78 owned paths and 15,783 unrelated master entries and modes. The three measured test files and their changed feature paths are unchanged. New upstream DB code adds only the normal nonunique concurrent native-session index and its migration classification. Shared and adapter-utils changes alter unrelated prompt or denial wording. The 138-case proof carries for those unchanged behaviors; it is not reported as a rerun after the merge. The normal Shared/SDK ensure, DB build, adapter-utils build and complete server-package check also pass at the current merged head. Their fresh inventory contains 8,143 outputs; canonical SQL and journal copies match. This build did not apply the new index migration. - Earlier real-PostgreSQL retained-history coverage reproduced the missing proof with 1,005 settled rows. Its repair retains the existing two-second/64-query inspection budget, current endpoint bound and exact owner snapshot. The 46 source-bound controls cover unfinished and invalid tails, exact cross-page action references, Retry, rollback and lost receipt responses. - Earlier enabled quiet/reactivation coverage produced ordinary useful work, kept configuration enabled, accepted fresh signed work plus a duplicate, and produced exactly one second useful reply. Its two App-worker windows lasted at least 30 seconds with zero measured PG writes, connection or outbound activity after settlement. That evidence stays tied to its measured revision. A missing worker EXIT marker limits that earlier proof; parent evidence does not replace it. - Earlier three-second callback, terminal ACK, current-question race and complete body-size controls remain source-bound. The 500-owner/1,000-binding body measured 313,875 bytes with a 618-byte JWT; signature tampering failed closed. - Final managed-provider acceptance now passes all five fresh Node boots: A/A, A/AB, B/A rejection, B/AB recovery, B/B final. Two independently configured, challenged, verified, linked and enrolled Slack identities exercise ACK and retirement debt separately. Normal provider factory/materialization delivers the configuration through a controlled external stack-provider boundary. The registered opening returns HTTP 200 in 21 ms within the original 3-second deadline. Two answers persist, normal ACK is accepted, exact retirement and lost acceptance/GC replies recover, and the original canonical snapshot remains unchanged without duplicate provider sends or submissions. - The separate complete committed-preflight case passes. A committed `update_attempted` response is lost before SDK entry; normal service-owner reconstruction after the original lease/deadline recovers the same useful update and one answer without a second trigger. This reconstructs service owners within one App process; the separate five fresh Node boots cover process restart and managed signing-key rotation. - Assertion-time PG socket reports balance 15 opens/15 closes across the five boots and 5/5 in preflight, peak 2 and zero live sockets or unexpected attempts. Worker terminal EXIT markers were missing and are not replaced by parent EXIT markers. These are domain/recovery and assertion-time connection proofs, not a new whole-worker terminal quiet measurement. All known owners/groups closed; independent SQL audits were empty; only successful fixture databases were removed. Earlier failed fixture databases/logs remain retained. No product guard, deadline or assertion was weakened. Exact runtime source and normal outputs are unchanged by the three test files in the current follow-up, so these source-bound proofs carry without repeating those journeys. - The preceding App CI ran 11,312 support cases: 11,289 passed, three SDK-evidence cases failed, and 20 external cases skipped. Normal CI dependency setup resolved six stale Slack patch-hash references and produced an unreviewed lock. The public SDK proof correctly refused that lock; the tests incorrectly assumed the CI output was a reviewed reference. The three-file test repair keeps the protected lock policy, source/compiler/parser/Zod pins, scanner and redaction rules unchanged. It builds an owned reviewed reference using the existing normal Shared/SDK producers for the positive seal test, while a separate negative requires actual unreviewed-build privacy refusal and no retained SDK bytes. It does not relabel the CI build as certified or admit a new lock hash. - All four whole SDK support files pass locally: public type 30, evidence 16, public-input synchronizer 8 and lock preparation 10, totaling 64 with zero excluded/TODO cases in 28.73 seconds. The genuine fresh reference build/positive seal and separate invalidated-reference privacy refusal passed within their original 120-second bounds. Original installed dependencies, SDK outputs/cache and runtime source stayed unchanged; normal global setup produced only its own Runner JavaScript outputs. PostgreSQL was denied throughout, all four worker markers were observed, and the owned process group/private test root closed. The local canonical-lock negative mutates only its own reference; fresh remote CI also passes the actual resolved-CI-lock branch: all 16 evidence and 30 public-type tests pass, including the genuine reviewed-reference positive and the strict actual-unreviewed-build refusal. The upstream synchronizer file runs its current 10 cases in CI. - The first complete CI attempt on this head also exposed two separate 30-second timeouts: one real reporter-artifact test and the `agent-test-environment` module-loading hook. Full original logs remain preserved. Their exact test, sanitizer and evidence source blobs are unchanged from the preceding head; inspection found no deterministic setup cause. After the original run finished, one retry of only the failed jobs passed on the same synthetic merge `168851e210671ebe97e7a778e8cf891807e80502`: Runner support reports 11,304 passes and 20 external skips; serialized shard 6 reports all 395 passes, including all 67 route bodies. No source, timeout or assertion changed. This proves recovery; the original causes remain unexplained. - The final pre-merge tree audit uses actual master `17136a6c25ef6cb7c816a29140a4c48c2c75fb31` (refreshed after the earlier `85dec046` audit). The expected normal squash preserves all 81 owned paths and 15,816 unrelated master entries and modes. Only OpenAPI has edits from both sides, and its complete content composition preserves the new project route and token help alongside this PR's API entries. The shared inbound selection changes preserve the same candidate set; new concurrent indexes, Native/workspace behavior and shutdown changes remain upstream behavior. The scoped form proof uses process agents and does not qualify the new Native/sandbox branches. The latest question-schema change still accepts the explicit version-1 payload exercised here. The last master delta also changes managed AI/Native credential refresh and writeback; those branches are outside the measured process-agent form paths. Existing 8,143 local outputs remain tied to their measured revision; no new whole-tree output equality or deployed suspension is claimed. - Reproduce the public path with `doc/testing/slack-cloud-qualification.md`, the companion Cloud checkout, Node 24/22 and fresh owned fixture databases. Aggregate local `pnpm -r typecheck`, `pnpm test:run` and `pnpm build` were not rerun as one command set. Current exact-head required CI and the complete Greptile 5/5 review now pass; the full edited summaries and all paginated threads were read. Local worker quiet does not prove deployed host or Neon suspension. Photon and Discord remain separate work; this PR completes the described Slack slice. ## Risks Use compatible App and Cloud readers before transferring custody. Signatures prove authenticity; current authorization and exact ownership still control execution. Preserve old targets, secrets and terminal facts until their normal owners release them. Keep the managed seal identity stable while custody remains. The required Slack adapter patch remains pinned. Upstream's concurrent index uses the normal top-level migration path. ## Model Used OpenAI `gpt-6.1-sol` implemented and tested the changes. A GPT-6-based Codex managing agent reviewed and integrated them. The host does not expose the managing agent's exact model ID or context-window sizes. The agents used code execution and test tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used, its version and tool capabilities - [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 linked public related issues or PRs - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run the local tests listed above and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green for the current head - [x] Greptile is 5/5 with no open findings for the current head - [x] I will address all reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
1805a1d247 |
perf(workspaces): probe workspace run bindings through indexes (#20568)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The terminal workspace sweep checks each Git workspace for recent activity and for live consumers before it expires or moves the workspace > - These checks find the runs of a workspace with one SQL scope, `expirationScope`. The scope used one `OR` over all run bindings, and one branch decoded the context JSON of each run > - PostgreSQL could not use an index for that `OR`. Each check read every run of the company and its large context from TOAST. On a hosted database, each call took 2 to 5 seconds, about 5,200 s in total in one window > - This pull request splits the scope into one index probe per binding and adds one concurrent expression index for the legacy `executionWorkspaceId` context key > - The benefit is that each check reads a few index entries instead of all company runs. The cleanup decisions do not change ## Linked Issues or Issue Description **What existing behavior does this improve?** The workspace activity and cleanup-safety checks: `gitWorkspaceLastActivityAt`, `isGitWorkspaceExpirationEligible`, `workspaceMoveEligible` and the orphan-folder consumer check. All of them use `expirationScope` in `server/src/services/git-workspace-expiration.ts`. **Subsystem affected** Execution workspace cleanup in the server (`git-workspace-expiration.ts`, `isolated-repository-rehome.ts`, `isolated-task-directory-retention.ts` through the shared scope) and the `heartbeat_runs` schema in `packages/db`. **Current behavior** The `relevant_runs` CTE selects company runs where any of these is true: `issue_id` or `native_issue_id` is in the issue tree or the bound issues, the run is a bound issue's checkout or execution run, or the context JSON has a matching `issueId`, `taskId` or `executionWorkspaceId`. The planner does one sequential scan of the company runs and reads each context from TOAST. The cost grows with the run history of the company, not with the size of the workspace. **Proposed behavior** Each binding is a separate index probe. The `UNION` of run IDs goes to a primary-key probe. The SQL gives the same rows. **Reason and benefit** Local SQL measurements show a 34x to 169x decrease in time. The decrease grows with the run history (see Verification). ## What Changed - `expirationScope` builds `relevant_runs` from a `UNION` of index probes, one for each binding. The key sets go in as arrays (`= ANY(ARRAY(...))`). With `IN (subquery)`, the planner chose a hash join over a full scan, because it does not count the cost of reading TOASTed contexts. - The legacy context keys use `context_snapshot ->> 'key'`. This expression matches the existing indexes `heartbeat_runs_company_ctx_issue_created_idx` and `heartbeat_runs_company_ctx_task_created_idx`. For a non-object context, `->>` gives `NULL`, as the strict JSONPath decode did. Such a run can still match through a native binding. - New partial index `heartbeat_runs_company_ctx_execution_workspace_idx` on `(company_id, context_snapshot ->> 'executionWorkspaceId') WHERE context_snapshot ? 'executionWorkspaceId'`. The query repeats the predicate. For an array context, `?` can be true, but `->>` gives `NULL`, so the result does not change. The index is partial so that it serves only this probe. A full index that starts with `company_id` made the planner choose it for an unrelated company-scoped query. Migration 0460 builds it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration is in the list of migrations that run outside a transaction. - New tests: a legacy `issueId` or `executionWorkspaceId` binding on a run that is bound to a different issue still blocks cleanup. A plan test with `enable_seqscan = off` checks that no binding scans `heartbeat_runs` and that the three context indexes are used. - The benchmark script `scripts/benchmark-workspace-activity.ts` gets a `--runs-per-workspace` option. Unrelated fixture runs now have distinct context IDs, as real history does. - `doc/DATABASE.md` describes the probes and the new index. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/git-workspace-activity.test.ts src/__tests__/native-harness-backup-expiration.test.ts`: 132 of 132 tests pass. The second file checks that the native session index stays the planner's choice for its query. These include the existing route matrix (root, child, bound, native, context issue, context task, context workspace, checkout, execution run), exactly-once counts, malformed and uppercase contexts, company isolation, controller leases and lease vetoes. - `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. - Benchmark (local SQL measurement, PostgreSQL 18.1, synthetic fixture, 8 workspaces per sample, before and after alternate in each iteration). The script compares the complete ordered caller results and the complete `relevant_runs` multisets of the baseline and treatment. They are equal in every run. `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-activity.ts --baseline-ref=origin/master --runs-per-workspace=<N>` | Fixture | Check | Before median (min–max) | After median (min–max) | Speedup | | --- | --- | --- | --- | --- | | 4,800 runs, 330 MB | last activity | 523 ms (460–598) | 12 ms (8–15) | 44x | | 4,800 runs, 330 MB | eligibility | 452 ms (432–473) | 13 ms (11–15) | 34x | | 4,800 runs, 330 MB | idle eligibility | 465 ms (442–573) | 12 ms (10–13) | 40x | | 24,000 runs, 1.7 GB | last activity | 2,240 ms (2,214–2,454) | 13 ms (11–14) | 169x | | 24,000 runs, 1.7 GB | eligibility | 2,245 ms (2,097–2,377) | 17 ms (14–19) | 133x | | 24,000 runs, 1.7 GB | idle eligibility | 2,196 ms (2,179–2,381) | 15 ms (13–18) | 143x | Before: one `Seq Scan` on `heartbeat_runs`. After: `heartbeat_runs_company_issue_created_idx`, `heartbeat_runs_company_native_issue_contract_id_uq`, the three context indexes and `heartbeat_runs_pkey`. - Index build time: on a synthetic table with 200,000 runs and 8 KiB contexts (1.8 GB), the full form of this index took 3.7 s to build concurrently and used 15 MB. The partial index reads the same rows and holds only runs that have the key. ## Risks - Cleanup safety: a wrong "allowed" result could delete an active workspace. The scope keeps every binding of the old query. The benchmark checks exact equality of the run sets, and the existing tests check each binding route and each veto. - Migration: the index build reads every run context once. It runs concurrently and does not block writes. Until migration 0460 builds the index, the new query is still correct but the `executionWorkspaceId` probe scans. - Plan choice: the array form makes index probes the cheapest plan for a small tree. A very large issue tree gives a large array, but each element is still one index probe. - The sweep frequency does not change. This pull request reduces only the cost of each check. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
17136a6c25 | fix(runtime): persist managed Runner credential refreshes across execution targets (#20580) | ||
|
DottaandPaperclip
|
85dec04677 |
fix(native): validate active controller durability during eligibility (#20601)
## Thinking Path > - Paperclip controls agents while their work runs outside the web server. > - A web replacement must retain each controller's durable recovery state. > - Modern controllers commit that state to the database before sending work. > - Their local controller file is only an external-persistence identity marker. > - Disk eligibility still required recovery metadata that can arrive after a run finishes. > - This pull request verifies the complete active database authority instead. > - It also bounds concurrent source reads so a large history does not require serial network waits. ## Linked Issues or Issue Description Refs: #20311, #20584, #20599. **What happened?** A live native run can have a complete database controller checkpoint and no terminal session snapshot yet. The disk-preservation check treats its local identity marker as a local-only obligation. The check also resolves selected sources one at a time. **Expected behavior** Accept a complete, bound active recovery authority. Refuse missing, partial, stale or foreign evidence. Keep all execution and enrollment guards. **Steps to reproduce** Use the new active-controller regression. Start a real Runner controller with the database checkpoint writer. Keep the business run active and omit its terminal snapshot metadata. The previous check returns a local obligation even after the full checkpoint commits. ## What Changed - Require the exact external-persistence marker and a complete decoded encrypted checkpoint for active runs. Match run, agent, task, session, Runner and workspace identities. For warm reuse, also permit an older marker only when its full older checkpoint matches that marker and its exact earlier successful run has a committed accepted result in the same company, agent, task, session, Runner and workspace. Keep the current live writer mandatory. Read the marker again after validation. - Accept that proof only with its fenced live writer and task binding, or the existing exact saved recovery snapshot. The saved-snapshot path still requires the full active journal. - Keep newer remote workspace authority strict. A malformed reference cannot fall back to an older prepared reference. Conflicting plugin task authority remains blocked. - Resolve selected sources at most eight at a time for read-only eligibility. Join every started reader before reporting an error. Keep source order, pagination, predicates and all write paths unchanged. Offline migration and prestaging still resolve sources one at a time. - Add real PostgreSQL regressions for active durability, partial and foreign state, expired ownership, previous prepared-reference state, post-result state and reader failure joining. ## Verification - Canonical Node24 preflight passed in74.2s. It built the current Runner inputs and published the cache. No shared Cargo target or hosted database was used. - Genuine active regression RED: the previous check returned true when false was required. The real checkpoint writer had committed the complete authority and replaced the local file with the Runner marker. Treatment passed, including the refusal controls. - Genuine source-reader RED: sixteen retained pointers used one reader at a time. Treatment used at most eight and joined all eight when the first reader failed. With a controlled20ms reader delay and real PostgreSQL selection, the small fixture changed from497.5ms to85.3ms. This is a bounded-concurrency proof, not a production speedup measurement. - All54 cases in the affected controller migration integration file passed in76.07s, with no skips. These include existing failed-controller, archived-controller, inline database and previous-session preservation cases. - Server TypeScript passed. The normal merge of current master kept both changed files byte-for-byte identical to the54-case proof. Final post-merge focused checks passed two of two in14.83s (52 other cases intentionally unselected); server TypeScript and module boundaries passed. Comment-only correction743 removed all new code comments required by CONTRIBUTING. A later warm-reuse regression reproduced the remaining false blocker through the real Runner rotateRunIdentity API. The bounded treatment uses one exact older-run lookup and full old/current checkpoint validation; it adds no Runner format, release or lineage service. Final normal-merge head `c62d6eb1755cf99b6def9563ba69854dba17de42` includes the new0459 baseline. All three focused PostgreSQL cases passed in21.37s (52 existing cases intentionally unselected); fresh server TypeScript and module boundaries passed. The prior54-case regression evidence is retained; no full matrix was repeated. - `git diff --check` passed. Final-head CI passed:56 checks succeeded and4 were skipped; the separate Paperclip Review advisory was still queued. All required test, build, typecheck and ci/verify gates passed on c62. [Final required workflow](https://github.com/paperclipai/paperclip/actions/runs/42868279612). Fresh [Greptile review](https://github.com/paperclipai/paperclip/pull/20601#issuecomment-6774537057) is5/5 on exact head `c62d6eb1755cf99b6def9563ba69854dba17de42`, with no open review threads. ```sh node scripts/agent-preflight.mjs --skip-rust-debug node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/native-runtime/native-controller-migration.integration.test.ts --disableConsoleIntercept pnpm --filter @paperclipai/server exec tsc --noEmit ``` ## Risks - This reports preservation evidence. It does not grant attachment, execution, replacement or enrollment authority. The normal live-owner and admission gates still apply. - A partial old or current checkpoint, stale local controller state, identity mismatch or uncertain owner stays blocked. Warm-marker proof also rejects a foreign agent, newer source, later completion, rejected result, per-turn scope or expired current writer. No source, checkpoint, session, lease, archive or provider state is changed by the check. - The eight readers add bounded database concurrency during an explicit eligibility request. There is no new background scan, cache, row filter or timeout change. Live enumeration time still needs deployed measurement. - The plugin task-lease branch uses the producer's environment lease row ID, as native-session-executor and existing diskless adoption coverage require. A direct active-eligibility positive for that plugin branch is a coverage limit; existing mismatch/conflicting-authority checks remain. The advisory found no must-fix defect, and this does not change the current tested revision. - Saved formats and terminal migration behavior remain unchanged. Older prepared workspace references remain supported. A fully saved result can still pass before the run-status update, but its entire active checkpoint must decode. - The observed staging run finished naturally and became valid on the existing terminal path during investigation. That observation is not deployed verification of the new active path. ## Model Used OpenAI GPT-6 (Codex), with reasoning, tool use and code execution. This runtime does not expose a more specific model ID or context-window size. ## 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> |
||
|
DottaandClaude Opus 5.5
|
1b7ec13acb |
perf(native): default remote sandbox runs to a warm, reusable sandbox for measured harnesses (#20048)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can run in remote sandboxes (Daytona), where each turn needs
the task's workspace and a runner inside the sandbox
> - With a default environment, every remote turn got a brand-new
sandbox: the whole workspace was uploaded again and a new runner
started, even for a follow-up comment
> - In a paperclip-sized workspace that made a follow-up take 30 s to
first token, against 8–10 s with a reusable warm sandbox
> - This pull request makes an agent that never chose a lifecycle run
warm on a sandbox for the measured harnesses (Codex, OpenCode, Claude),
so a follow-up keeps its sandbox, runner and workspace
> - The default applies only where warm can work: a sandbox provider
that keeps reusable leases, and a managed AI account that supports a
warm session. Everywhere else the run is per-turn, exactly as before
> - The benefit is fast remote follow-ups by default, without
re-uploading a large repository every turn
## Linked Issues or Issue Description
**What happened?**
On Daytona, with a fresh paperclip clone as the project workspace, a
follow-up comment took 30 s to its first token. The run log showed
`environment: fresh/created` on every follow-up. It spent 13.3 s in
`workspace_sync.transfer`, a full upload into the new sandbox, and 8–9 s
in `runner.session.resume`.
**Steps to reproduce**
1. Create a Daytona environment with the defaults: lifecycle inherited,
lease reuse off.
2. Run a Codex task in a large repository, then post a follow-up.
3. The follow-up gets a new sandbox, uploads the whole workspace again
and starts a new runner.
**Expected behavior**
A follow-up on the same task reuses the live sandbox, its runner and its
workspace (plan rule 2: no full re-upload per turn).
## What Changed
- **One rule for the default.** `defaultsRemoteSandboxRunToWarm`
(`native-runtime/native-warm-capability.ts`) decides the remote-sandbox
warm default for both the lease (environment view) and the runner
(native session policy), in one vocabulary (runtime backend plus ACPX
agent). It is applied only when no lifecycle was chosen anywhere:
admitted session, environment `runnerLifecycleMode`, or agent
`lifecycleMode`. It holds only when:
1. the harness is measured (Codex, OpenCode, ACPX Claude);
2. the sandbox provider keeps reusable leases (Daytona, CreateOS and
boxd declare them; E2B, Modal and the built-in `fake` do not); and
3. with a managed AI account, that harness supports a managed warm
session (`supportsManagedNativeWarmSession`: Codex).
- **The harness resolves as the runtime resolves it.**
`resolveWarmProviderSelection` uses
`resolvePaperclipRunnerProviderProfile`, so an omitted `provider` is
Codex and ACPX without an agent is Claude, in both decisions. A config
the runtime would refuse gets no default.
- **Lease side** (`runner-environment-lifecycle.ts`,
`environment-run-orchestrator.ts`): for an eligible run only, the
orchestrator asks the provider before acquisition
(`providerSupportsReusableLeases` in `environment-runtime.ts`: the
acquisition rule, one registry lookup for a plugin provider, no
worker-ready wait). With support, the run-scoped view sets `reuseLease:
true` and forwards the warm idle window to the provider's auto-stop. The
shared environment is never rewritten. Every other run resolves exactly
as on master, without the probe.
- **Runner side** (`native-lifecycle.ts`, `runtime-selection.ts`): the
native policy applies the same rule to the acquired lease
(`reusableLeaseConfigured && effectiveCapabilities.reusableLeases`). If
acquisition downgrades the lease anyway, the run falls back to per-turn;
a default never produces
`runner_warm_environment_requires_reusable_lease`.
- **Idle window.** The remote default uses the window the environment
view forwards to the provider: an inheriting environment's
`runnerIdleTimeoutMs`, then the agent's `idleTimeoutMs`, then 5 minutes.
The sandbox execution target now carries the environment's window
(`runnerIdleTimeoutMs`, an optional field in `adapter-utils`'s execution
target) so the runner and the provider auto-stop agree. An explicit warm
choice and the local default are unchanged.
- Unchanged: explicit agent choices, environments that pin a lifecycle,
plugin-task environments, admitted sessions (`persistedPolicy`), and
local behavior.
- `tests/restart-chaos`: the vCPU budget check counts `--daytona-inherit
--lifecycle default` sandboxes as retained, because they now default to
warm.
- `doc/plans/2026-11-08-large-workspace-speed.md`: the measurements, the
ranked waits, and the gate.
## Verification
- Unit and integration tests, one per rule:
- Rule 1 (provider gate): `native-lifecycle.test.ts` (ephemeral lease →
per-turn), `runner-environment-lifecycle.test.ts` (no reusable leases →
environment unchanged, no warm window),
`environment-runtime-reusable-lease-probe.test.ts` (`fake`, an
undeclared plugin, a declared plugin whose worker does not verify the
reuse methods, and a plugin without a running worker read false),
`runtime-selection.test.ts` ("defaults an agent without a lifecycle on
...": reusable → warm/keep_running; reuse configured on an unsupported
provider → per_turn/destroy_after_turn; ephemeral → per_turn),
`environment-run-orchestrator.test.ts` (probe result decides
`reuseLease` and the forwarded window).
- Rule 2 (explicit configuration wins): explicit per-turn agent, pinned
environment, admitted session, configured `idleTimeoutMs`, managed
Claude per-turn, managed Codex warm; the orchestrator does not probe
when a lifecycle is explicit.
- Rule 3 (omitted provider): `{}` resolves to Codex in the environment
view; `runtime-selection.test.ts` "agrees with the environment view's
lease for ..." feeds the environment view's lease into selection for
Codex, a provider-less Codex agent, OpenCode, ACPX Claude, ACPX without
an agent, and Copilot (per-turn), and checks both decisions agree. A
plugin-task target stays out of the default.
- The two suites that failed on the previous head
(`runtime-selection.test.ts`,
`workspace-recovery-preparation-dispatch.test.ts`) pass locally.
Locally, 6 connector-skill cases in `runtime-selection.test.ts` fail
with `ENOENT .claude/skills/slack/SKILL.md` because this checkout is a
nested worktree; they do not touch this change and pass in CI.
- End-to-end check of this default on Daytona (before the gate was
added; Daytona keeps reusable leases, so the gate does not change this
path): a user-default environment (lifecycle inherited, lease reuse
unset, `--daytona-inherit`), a fresh paperclip clone, built from the
image's source revision with this change (branch measure/remote-warm).
| | Before (per-turn sandbox) | After (this change) |
|---|---|---|
| Follow-up, Codex | 30.4 s | 7.6 s |
| Follow-up, Claude | about 30 s | 11.0 s |
| First follow-up after an idle K1, Codex | 45.9 s | 20.3 s |
| First follow-up after an idle K1, Claude | 38.3 s | 18.3 s |
The run-log reuse records show the follow-ups reusing the workspace,
environment, runner and provider session, with no
`workspace_sync.transfer`. First turns still upload the workspace once
(14.8 s and 26.1 s). Every sandbox was cleaned up.
- Daytona restart chaos wave (K1/K2/K4 with a mid-run comment,
inheriting environment, agents with no lifecycle, runner image built
from this branch): wave `w-20261110T171438-s20048`, seed 20048, on
|
||
|
DottaandClaude
|
c37eacbee3 |
fix(native): Retry re-finalizes a saved result that ran out of finalization retries (#20578)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs save an accepted result. Then a finalizer applies that
result to the task.
> - Finalization has its own budget of 3 failed attempts (#20555). When
the budget runs out, the coordinator goes to `terminal_failure` with
`native_finalization_retry_exhausted`.
> - For a provider-succeeded run, the task then rests `in_review` with a
board recovery card. The task shows "resolved" and offers no Retry. The
user is stuck.
> - The ordinary Retry is also wrong here: it starts a new provider run,
which repeats work the agent already finished.
> - This pull request adds two things. First, contention failures on
such a result get two longer automatic delays before the board gets the
task. Second, when the state is still exhausted, the task offers Retry,
and that Retry applies the same saved result again.
> - The benefit: fewer tasks need a Retry, and when one does, Retry
works and does no duplicate provider work.
## Linked Issues or Issue Description
Refs #20555 (it names this gap under Risks).
**What happened?**
In restart-chaos wave `w-20261110T044338-s20385`, task t03 ended with a
succeeded run, an accepted result, and a coordinator in
`terminal_failure` / `native_finalization_retry_exhausted`. The issue
stayed `in_review` with a board-owned `active_run_watchdog` card and
`canRetry=false`. #20555 stopped the early exhaustion. A real exhaustion
still leaves the same stuck state. Coordinators that older builds
already left in this state are stuck the same way.
**Expected behavior**
Transient finalization failures recover by themselves when that is safe.
When they do not, the task offers Retry. That Retry re-finalizes the
saved result. It does not start a provider run and does not charge a
provider attempt.
**Steps to reproduce**
1. Let a native run save an accepted result and end `succeeded`.
2. Make each finalization of that result fail until the budget of 3 runs
out.
3. Before this fix, the task presentation is `resolved` with
`canRetry=false`, and the board card has no action that applies the
result. The new test
`server/src/__tests__/native-exhausted-finalization-retry.test.ts`
reproduces this.
**Paperclip version or commit**
master at
|
||
|
Nicky LeachandPaperclip
|
98ce93dd4e |
Choose project storage from compatible environments (#20587)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Projects supply repository files to agent runs through core staging or a storage plugin. > - A storage plugin's default claim silently attached new projects even when their agents used a different environment. > - An environment name or driver type does not establish shared storage access. The existing binding and task-driver contracts do. > - This pull request groups creation choices by that contract and selects storage from the creator's effective environment. > - The benefit is a visible, validated choice and a clear error before an incompatible managed workspace operation. ## Linked Issues or Issue Description Fixes #20459. Related: #20156 covers a separate provider-failure diagnostic. The PR search found no other fix for #20459. ROADMAP.md has no duplicate project-creation change. ## What Changed - Show **Available in** when configured environments require different project storage. Each option names the environments that can use the same storage owner. Hide the selector when all choices are compatible. - Derive compatibility from repository staging and managed binding ownership. Local, SSH, and sandbox environments use core staging. Managed storage requires a registered task driver from the same installed storage plugin. Separate storage plugins remain incompatible. Unknown plugin contracts and the fake sandbox are excluded. - Follow the creating agent's environment, or the person's primary agent, then the instance default and local fallback. Apply managed-only and forced Kubernetes policy. The product has no separate user or company environment-default field today. - Add a company-authorized discovery endpoint and an optional creation `environmentId`. Validate the choice again on save. Store the storage owner without overriding agent run environments. Preserve explicit API storage choices and existing project ownership. - Keep the hidden repository-free private-task project independent of environment availability. Submit the observed storage owner as `expectedStorageProviderPluginId` so a changed contract fails at save without setting a workspace policy. Instance task-isolation defaults stay inherited. - Reject an incompatible managed workspace before a provider operation. Name the storage and selected environment in the error. Add regression tests, API documentation, creation-story fixtures, and minimal corrections to obsolete map text. ## Verification - **102 tests passed across 7 files:** project environment integration, installed storage policy, repository persistence, project environment routes, project creation telemetry routes, OpenAPI, and NewProjectDialog. Run with `node node_modules/vitest/vitest.mjs run server/src/__tests__/project-environments.test.ts server/src/__tests__/project-workspace-provider.integration.test.ts server/src/__tests__/project-repositories-persistence.test.ts server/src/__tests__/project-routes-env.test.ts server/src/__tests__/project-goal-telemetry-routes.test.ts server/src/__tests__/openapi-routes.test.ts ui/src/components/NewProjectDialog.test.tsx --silent`. - The review fixes cover both archived-default private-task cases and changed-storage rejection. With both instance isolation flags enabled, a new core repository project keeps a null policy and resolves to an isolated Git worktree. The complete affected set passes all 102 tests. - The CI query-error policy failure was fixed by using the shared query view. The form and shared query-view suites pass **24 tests**, including cached choices during a transient refetch failure. `node scripts/check-query-error-rendering.mjs --enforce --summary` reports zero findings. - CI exposed stale SDK source hashes and heartbeat fixtures that paired plugin storage with Local. Refreshed exactly four shared-source hashes with the maintained generator (runtime pins unchanged). All **53 SDK privacy/source-proof tests**, including real built SDK output, pass. All **7 corrected managed-storage heartbeat scenarios** pass; the fixtures declare the owning plugin's task environment and substitute transport at the existing execution seam. - Shared and UI TypeScript checks pass. The UI production build passes. `pnpm check:token-gates`, `node scripts/check-maps.mjs`, `node scripts/check-module-boundaries.mjs`, and diff checks pass. - Tested UI states: compatible choices, incompatible groups, default within a group, manual override, create failure, loading, discovery failure and retry, and an unavailable default. Tests use the production form and existing design tokens. - The integration tests use real isolated PostgreSQL on macOS arm64 with Node 26.12.0. They cover user/agent defaults, storage persistence, policy restrictions, company and authentication denials, and rejection before a provider call. Discovery uses saved configuration and makes no worker or provider requests. - Local verification limits: `pnpm -r typecheck` and `pnpm build` stop in unchanged adapter-utils code because the installed ACPX types lack `inheritProcessEnv`. A direct server check also finds an unchanged AI SDK type-version mismatch. The existing heartbeat test `inherits the instance default environment across companies while preserving explicit agent overrides` times out at line 518 on both this change and unmodified master with the same dependencies. The full `pnpm test:run` attempt passed all six chat shards (1,948 tests), then the Git workspace expiration suite reported 14 failures. I stopped that broad run. Two sampled failures reproduce on unchanged master with the same `unknown_worktree_git_files` refusal; the other expiration failures were not independently compared. A later broad workspace-preparation run also hit workspace validation failures and was stopped; the seven corrected fixture scenarios pass independently. These limits do not establish a full local suite pass. Hosted CI remains required. - Initial missing-package and loopback-denial failures were resolved by repairing isolated dependency links, building the SDK, and running database tests with loopback access. No deployed or live-provider execution is claimed. ## Risks - Creation defaults now follow the effective environment. A plugin's `defaultForProjects` claim alone no longer selects storage. This is the intended behavior change. - Compatibility covers repository project storage. It does not guarantee arbitrary host paths, retained-workspace moves, adapter/image support, credentials, or worker readiness. Matching provider configs cannot prove a cross-plugin storage contract. - A person's primary agent supplies the personal default because there is no separate personal environment setting. An unavailable configured default requires a valid explicit choice or a configuration repair. - No migration runs. Existing projects keep their storage owner. This change does not detach existing managed data or provide a storage migration. - Full local verification has the limits listed above. CI and standard Greptile review are pending. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The exact serving snapshot and context-window size were 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> |
||
|
DottaandPaperclip
|
1c78d023c6 |
perf(native): narrow backup eligibility to selected runs (#20599)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Server replacement checks whether retained native sessions still depend on local harness data. > - This check joins selected task sessions, runs, backup stamps, and durable backup records. > - PostgreSQL can start from backup leases and scan an issue's full run history before applying the selected-run condition. > - A measured instance spent 9.56 seconds in that selector and exceeded an 8-second statement limit. > - This pull request selects the exact current run identities first, then applies the existing lease and backup checks. > - The same receipt validation and eligibility decisions remain in place. ## Linked Issues or Issue Description **What happened?** Startup eligibility timed out while selecting native harness backup obligations. The plan scanned historical runs for each lease before checking the task session's selected run. **Expected behavior** The selector should use the selected run identities without scanning unrelated run history. It must return the same session, scope, company, and receipt flag. **Steps to reproduce** Use a database with retained native task sessions, many historical runs on the same issues, and version-two backup stamps. Run startup eligibility with an 8-second statement timeout. The lease-first plan can exceed that limit. **Paperclip version or commit** The local baseline is `d0c30e69a9cea912db7c5290c5d585fdd63ec228`. **Deployment mode** Self-hosted server with PostgreSQL. ## What Changed - Keep `AS MATERIALIZED` as an intentional join-order fence: build selected task-session and last-run identities before joining backup leases. - Keep only narrow identity columns in that materialized set. Read the JSON receipt flag through the selected run's primary key after the lease and current-backup filters, so profiles for unrelated identities do not widen the intermediate rows. - Preserve company and agent bindings, exact normalized-session text comparison, current-backup exclusion, scope deduplication, ordering, and the 100-row page. - Add real PostgreSQL regressions for full ordered-row equivalence, nullable JSON receipt flags, malformed and uppercase session bindings, cross-company rows, duplicate stamps, page tails, and invalid receipts. - Existing saved state carries forward under the same validation rules. No schema or persisted identity changes are required. ## Verification - The two focused PostgreSQL regressions passed locally. Command: `pnpm exec vitest run server/src/services/native-runtime/native-harness-backup-obligations.integration.test.ts --silent`. - Worktree preflight passed. The focused regressions (2/2) and `node server/scripts/typecheck-layers.mjs` passed on final head `c739353b22` after rebase. - One bounded read-only pair on PostgreSQL 17.11 returned the same 54 complete ordered rows: baseline 9,564 ms, candidate 298 ms (32.1 times faster). The plan changed from lease-to-issue-history scans to selected-run primary-key lookups and a hash join. - A synthetic PostgreSQL 18.1 fixture already chose an efficient baseline plan. Across five alternating warm pairs, its median was 104.6 ms before and 109.4 ms after. This neutral control used 22,800 runs, 14,100 leases, 3,173 task sessions, and 5,460 backups; all 54 complete rows matched. The empty control also matched. - These measurements cover the selector. A later read-only assembled-call probe on a newer serving release substituted only this selector: the selector took 544 ms for 54 rows, while the full eligibility call took 56.1 seconds and still reported a controller migration blocker. Other checks remain outside this PR; this PR does not claim to bring the whole request within its deadline. - Required CI and fresh review results will be recorded here before handoff. ## Risks - Materialization adds a selected-identity pass. Databases that already choose an efficient plan may see little benefit, as the local neutral control shows. - Pagination and receipt validation must stay exact. The regression crosses the 100-row boundary and checks that an invalid final scope still blocks eligibility. ## Model Used OpenAI Codex, GPT-6 family. The exact model ID and context window are unavailable in this session. The agent used reasoning, code editing, shell execution, PostgreSQL diagnostics, and GitHub tools. ## 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> |
||
|
DottaandClaude Opus 5.5
|
580ec41110 |
fix(workspaces): run private tasks parked in a personal project (#20589)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A private task without a project is parked in its owner's "My private tasks" project > - The spec says an issue-level grant can expose a task without exposing its containing project > - Workspace setup for a run also required project read on the task's project > - So an agent that could read its private task still failed at setup, before any work, when the task was in "My private tasks" > - This pull request accepts task access for that personal project when the project supplies no workspace files > - The benefit is that a person can make a chat or task private and the agent can still work on it, without gaining access to the person's other private tasks ## Linked Issues or Issue Description **What happened** A person made their chat with the CEO agent private. The chat moved into the person's "My private tasks" project. The next CEO run failed at execution setup in about one second with "Project is private and the actor is not an access member." **Expected behavior** The CEO can work in a private chat (or on a private task) that it can read. The CEO does not get access to the person's other private tasks. **Steps to reproduce** 1. Open a chat with an agent and make the chat private (the chat has no project). 2. Send a message to the agent. 3. See the run fail at setup with `deny_project_private` on the person's "My private tasks" project. ## What Changed - `assertTaskWorkspaceExecutionAccess` marks the task's own project as a container when it is not also the selected workspace or source project. - `assertTaskWorkspaceSourceProjectAccess` accepts a `deny_project_private` decision for such a container only when it is a personal "My private tasks" project with no project workspaces. Task read is already authorized earlier in the same check. - Shared private projects, personal projects with workspaces, workspace and source projects, and low-trust boundary denials are not changed. ## Verification - Red then green: `cd server && npx vitest run src/__tests__/agent-project-access.test.ts` (new test `runs a private task in its owner's personal project without granting that project` fails on `master` with the eval's error; 12 passed with the change). The test also checks that the personal project stays unreadable, that a shared private project still needs membership, and that a personal project with a workspace still needs membership. - `npx vitest run src/__tests__/workspace-preparation-history.test.ts src/__tests__/project-workspace-root-provenance.test.ts src/__tests__/task-workspace-child-trust.test.ts src/__tests__/task-workspace-source-copy-privacy.test.ts src/__tests__/privacy-production-review.test.ts` (81 passed). - `tsc --noEmit -p server`: no errors in the changed files. ## Risks - The change relaxes one authorization check. It applies only to the task's own personal project, only after task read is allowed, only for the private-project denial reason, and only when that project has no project workspaces. A project read through any other path is not changed. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
accf22076f |
fix(search): apply task privacy to artifact results (#20595)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Private tasks are visible only to the people and agents that have access > - Company search already filters task results by the caller's task read condition > - Artifact results (agent documents, work products and attachments) in search, and the company artifacts list, did not use that condition > - So a company member could find a private task's document title and snippet through search > - This pull request applies the same task read condition to artifact results > - The benefit is that private task outputs stay private in search and in the artifacts list ## Linked Issues or Issue Description **What happened** An agent saved a document on a private task. A different company member without access to that task searched for a word in the document. The search returned the document as an artifact result with a snippet. **Expected behavior** Search and the artifacts list show artifacts only from tasks the caller can read. **Steps to reproduce** 1. Create a private task and let an agent save a document on it. 2. Sign in as a company member who cannot read the task. 3. Search for a word in the document, or open the company artifacts list. 4. See the private document in the results. ## What Changed - `company-search.ts`: artifact counts and artifact rows apply `options.issueReadCondition`. - `routes/companies.ts`: `GET /companies/:companyId/artifacts` passes the caller's `issueReadSqlCondition` to the artifacts service. ## Verification - Red then green: `cd server && npx vitest run src/__tests__/company-search-service.test.ts -t "private task artifacts"` (fails on `master` with the private document in the outsider's results). - Red then green: `npx vitest run src/__tests__/company-artifacts-service.test.ts -t "caller can read"`. - `npx vitest run src/__tests__/company-search-service.test.ts src/__tests__/company-search-extract-service.test.ts src/__tests__/company-search-extract-routes.test.ts src/__tests__/company-artifacts-service.test.ts` (all pass). - `tsc --noEmit -p server`: no errors in the changed files. ## Risks - Low risk. The read condition is the same one task results use. Callers with full access (local board, instance admin, privacy mode off) get `true` and see no change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude
|
7368a0b15c |
fix(recovery): re-queue native runs that fail on a transient database fault before provider work (#20561)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native (Paperclip Runner) runs prepare a task, pass a final dispatch
gate, and only then start a provider.
> - A native run can fail before any provider work starts. Examples are
a Postgres serialization or deadlock error (`40001`, `40P01`) at budget
admission, or a transient database error during setup.
> - Today such a run ends `adapter_failed` or `setup_failed`. `failRun`
writes the "no provider work" record only for legacy runs, and the
bounded retry paths do not retry these codes for native runs. The
release then opens the native hold
(`native_continuation_requires_reconciliation`), and a person must click
Retry.
> - The goal is that 0% of tasks need a Retry click after a restart or a
storage/DB fault. Duplicate provider work is worse than a Retry click,
so the fix must only act when the server can prove no provider work
started.
> - This pull request records that proof for native runs and sends a
transient database failure to the existing uncharged, bounded
`interrupted_preparation` retry lane.
> - The benefit is that these runs re-queue by themselves, and runs with
any sign of provider work keep today's hold.
## Linked Issues or Issue Description
**What happened?**
In restart-chaos waves with database pressure, a native run failed at
budget admission with exhausted serialization retries (`40001`/`40P01`).
The run event history had no `native.process_start_requested` event.
About 15 s later the server recorded "Automatic recovery stopped" with
cause `native_continuation_requires_reconciliation`. The task stayed
blocked until a person clicked Retry.
**Expected behavior**
A native run that failed before any provider work, on a fault that
clears by itself, re-queues automatically. The attempt is bounded and
does not spend the failure budget. A persistent failure still ends
failed, with a working Retry.
**Steps to reproduce**
1. Start a native (Paperclip Runner) task.
2. Make budget admission or a setup database write fail with SQLSTATE
`40001` or `40P01` until its bounded retries are exhausted.
3. See the run end `adapter_failed`/`setup_failed`, the task go to
`blocked`, and a `native_continuation_requires_reconciliation` recovery
action that waits for Retry.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
6ed6b0754b |
perf(recovery): probe recovery source runs and stale-continuation runs by key (#20584)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The workspace recovery worker saves the files of failed runs. After each pass, it reads the time of its next deadline (`nextWorkspaceRecoveryAt`) > - That read finds the source run, its finalizer and its leases with `id::text = evidence->>'runId'`. A text comparison cannot use a key. Each exhausted hold read every run, finalizer and lease of its company > - On a hosted staging database this statement used about 4 s per call (21 calls in the sample window). The cost grows with exhausted holds multiplied by the company's run count > - The ownership reconciler's stale-continuation check (`checkStaleDroppedComments`) has the run lookup shape that #20412 fixed in the lost-wake check: one OR across the legacy context and the native column > - This pull request compares the run id as a canonical uuid, and it gives the stale-continuation check the per-binding run probes from #20412 > - The benefit is the same recovery decisions with index-driven work: 11 s to 93 ms for the deadline read on a representative local fixture ## Linked Issues or Issue Description Refs #20412 **What happened?** The workspace recovery deadline read was one of the most expensive statements that remained on a hosted staging database after the fixes in #20393, #20405, #20386, #20412, #20391 and #20400. Query statistics showed about 4 s per call. The statement text starts with `select case when (issue_recovery_actions.…`. The statement evaluates the retained-source fallback and stop-proof conditions for each recovery action. These conditions read the source run (`heartbeat_runs`), its finalizer (`native_run_finalizations`) and its leases (`environment_leases`) with `x.id::text = evidence->>'runId'`. The cast prevents the use of the primary key. PostgreSQL reads all rows of the company for each exhausted hold. The worker's discovery (`retainedWorkspaceCandidates`) uses the same conditions. The stale-continuation check matched a task's runs with `(context_snapshot->>'issueId' = issue OR native_issue_id = issue)`. With the OR, PostgreSQL can join runs only by company. It also hashed every wake request of the company for the stale-wake test. **Expected behavior** Each condition uses a key lookup. The worker and the reconciler make the same decisions for the same saved state. **Steps to reproduce** 1. Seed a company with a deep run history and a few thousand workspace recovery actions. Let a few hundred be exhausted holds that stay active. 2. Call `nextWorkspaceRecoveryAt`. 3. The call takes seconds, and its time grows with holds multiplied by runs. The benchmark below shows this. **Paperclip version or commit** Baseline: `8d2bf78323`. **Deployment mode** Server with PostgreSQL. The local proof uses disposable PostgreSQL 18.1. Staging uses hosted PostgreSQL. ## What Changed - `workspace-recovery-fallback-discovery.ts`: the source run, finalizer and lease probes compare `id = canonicalExecutionUuid(evidence->>'runId')`. The helper returns null unless the text is the canonical lowercase uuid form. Thus the probe matches exactly the rows that `id::text = evidence->>'runId'` matched. The helper already exists in `execution-identity.ts`. - `nextWorkspaceRecoveryAt` and worker discovery use these conditions. Their SQL changes only in these probes. - `checkStaleDroppedComments` reads in two stages. The first stage keeps comments that a stale-continuation cancellation dropped and that no open wake carries. The second stage applies the run probes. The run probes use the `taskRuns` helper from #20412, which is now shared by both checks. The stale-wake lookup ends with `offset 0`, so it stays a per-comment index probe. - The final `order by c.created_at limit N`, the projection and the delivery check after the query do not change. - Add `scripts/benchmark-recovery-status-checks.ts`. It loads the changed files from an explicit baseline commit and from the working tree. Before it records time, it compares every changed condition for every recovery action, the complete service results and the positional SQL rows. - Add tests: each run binding in the stale-continuation check (native, legacy, live run on either binding, wrong agent, wrong company, failed run, run outside the comment time, closed by a person, open wake). Also a test that a hold with a non-canonical saved run id is not discovered, as before. - Document both checks and the benchmark in `doc/DATABASE.md`. - No schema change, index, migration, or new startup work. ## Verification - `pnpm exec vitest run src/__tests__/ownership-reconciliation-report.test.ts src/services/workspace-recovery-fallback.test.ts src/services/workspace-recovery-stop-proof.test.ts src/__tests__/heartbeat-held-comment-wake.test.ts --maxWorkers=1` (from `server/`). - `pnpm --filter @paperclipai/server typecheck`. - `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-recovery-status-checks.ts --baseline-ref=8d2bf78323 --samples=7` Benchmark setup: PostgreSQL 18.1 (embedded, local), `shared_buffers` 128 MB, `work_mem` 4 MB. All data is synthetic, with md5-hex filler. Each fixture has two companies (full size and a quarter). Two warmup rounds, then 7 timed samples for each arm, in alternating order. Each sample runs the real service function and asserts the same result. These are local SQL measurements on a shared machine, so the spread is wide. Workspace recovery deadline (`nextWorkspaceRecoveryAt`). About 8% of actions are exhausted holds that stay active. Some actions save a non-canonical (uppercase) run id. Before timing, the benchmark compares six conditions (fallback pending, due time, due; re-arm pending, due time, due) for every action of the fixture. | Fixture | Runs / actions / pending holds | Baseline median (min–max) | Treatment median (min–max) | Shared buffers | | --- | --- | --- | --- | --- | | small | 6,500 / 255 / 36 | 50.9 ms (47–76) | 6.8 ms (6–8) | 19,133 → 1,075 | | representative | 78,750 / 3,813 / 588 | 11,098 ms (8,277–12,790) | 93 ms (35–145) | 3,493,417 → 24,902 | | deep history | 191,250 / 3,813 / 588 | 40,146 ms (31,190–52,521) | 40 ms (36–230) | 13,491,792 → 26,911 | Stale-continuation check (`checkStaleDroppedComments`). The fixture is pessimistic: more than half of recent comments on done tasks have a stale cancelled wake. The table shows statement time from `EXPLAIN ANALYZE` with default settings, and one statement run with `enable_nestloop = off` (the company-only hash plan). The result is checked as equal in both modes. | Fixture | Runs / recent comments / reported | Statement, baseline → treatment | Hash plan, baseline → treatment | | --- | --- | --- | --- | | small | 3,792 / 76 / 24 | 3.2 → 1.5 ms | 5.5 → 3.8 ms | | representative | 75,406 / 1,125 / 200 (limit) | 34 → 26 ms | 66 → 25 ms | | busy hour | 152,031 / 5,625 / 200 (limit) | 328 → 153 ms | 2,302 → 165 ms | - Neutral path: the complete stale-continuation service call stays about the same (busy hour 750 → 655 ms median). The per-comment delivery check after the query (`deliveredQueuedCommentIds`) is most of that time. This change does not touch it. ## Risks - Low risk. Every predicate keeps the same logic. Only the probe shape and the evaluation order change. The benchmark compares each changed condition for every recovery action, including non-canonical saved run ids, and the complete results of both checks. - Previous-release story: no saved state, schema or identity changes. Holds saved by older releases are read with the same rules, so they carry forward unchanged. - Rows with the same comment `created_at` have no defined order, as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |