mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
d5c1f48180d56c83b87f89bb74f73caeb4a7104d
2160
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
d5c1f48180 |
perf(native): index the backup lease lookup by scope digest (#20571)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native harness backup expiration removes old session backups. Before it removes a backup, it finds the environment leases that reference the backup root > - This lookup reads leases by run ID, by the `metadata.nativeHarnessBackup.sessionScopeSha256` digest, or by the identity's own lease ID, in one `OR` query > - The metadata arm has no index. So PostgreSQL reads every lease of the company, and the metadata of each lease, for each lookup > - On a hosted test database, this statement ran 21,736 times at a mean of 158 ms in one window. After other fixes, it still ran 2,364 times at a mean of 121 ms > - This pull request adds a small partial expression index on the digest > - The benefit is that each lookup reads a few index entries instead of all company leases. The query text and the result do not change ## Linked Issues or Issue Description **What existing behavior does this improve?** The lease lookup in `backupDependencies` in `server/src/services/native-runtime/native-harness-backup-expiration.ts`. `reconcileBackup` calls it for each backup root when the expiration worker loads or checks a root. **Subsystem affected** Native harness backup expiration and the `environment_leases` table schema. **Current behavior** The query is `company_id = $1 and (heartbeat_run_id in (...) or metadata->'nativeHarnessBackup'->>'sessionScopeSha256' = $n or id = $m)`. The run arm and the ID arm have indexes. The digest arm has no index. A `BitmapOr` plan needs an index for every arm, so the planner reads all company leases and evaluates the JSON expression on each row. Lease metadata holds sandbox details, so these reads are large. **Proposed behavior** The partial index `environment_leases_native_backup_scope_idx` keys leases by the digest. It holds only leases that have a backup stamp. Every arm of the `OR` now has an index, and the planner combines three index probes. The SQL and the returned lease IDs do not change. **Reason and benefit** This statement was in the top ten by total database time on a loaded hosted database. A local reproduction shows a query that is more than 6,000x faster and reads 9 buffers instead of about 436,000. **Breaking changes** None. The API, the lease-matching rules and the results do not change. ## What Changed - Add the partial expression index `environment_leases_native_backup_scope_idx` on `(metadata->'nativeHarnessBackup'->>'sessionScopeSha256')`, where that value is not null. - Add migration `0459`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration runner runs it as a top-level statement, the same as migrations 0454–0458. - Add a regression test. It checks the lease-matching rules: run leases, a lease with only the backup stamp, and the identity's own lease match. Another digest, a non-string stamp, a top-level key with the same name, and the same digest in another company do not match. The test also checks that the custom plan and a forced generic plan of the generated SQL use `BitmapOr` with the new index and no sequential scan. - Add `scripts/benchmark-environment-lease-backup-lookup.ts`. It captures the SQL that the real service generates and measures it with and without the index. - Document the index in `doc/DATABASE.md`. ## Verification Cause and thesis. The cost comes from the one `OR` arm without an index, which forces a scan of all company leases and their metadata. Thesis: an index on the digest lets the planner use a `BitmapOr` of three index probes, so the cost no longer depends on the company's lease count. The results stay identical. The benchmark confirms the thesis. I did not change the SQL. With the index, the existing `OR` query already gets the index plan. A `UNION` rewrite would not help without the index, because the digest arm would still scan the company. Local SQL benchmark: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-environment-lease-backup-lookup.ts ``` Fixture (synthetic): one company with 100,000 leases with 4 KiB of incompressible metadata each. 30% of the leases have a backup stamp for another session. A second company has 20,000 leases. The table is 678 MB. The backup root has 3 run leases, 1 lease with only the stamp, and 1 identity lease. The same digest is also in the other company. Embedded PostgreSQL 18 on an Apple laptop, which other work loaded heavily. The hosted database is Neon with different compute, so absolute times are local SQL measurements only. The script measures in the order: no index, index, no index, index. Each measurement has 2 warmups and 11 samples of `EXPLAIN ANALYZE` execution time and of the full `reconcileBackup` call. | Measurement | Without index | With index | | --- | --- | --- | | Lease query execution, run 1 | median 196 ms (138–562 ms) | median 0.010 ms (0.007–0.017 ms) | | Lease query execution, run 2 | median 185 ms (144–295 ms) | median 0.030 ms (0.020–0.045 ms) | | `reconcileBackup` wall time (includes file reads), runs 1 and 2 | median 188 ms and 216 ms | median 1.4 ms and 4.2 ms | | Plan | `Gather > Seq Scan` | `Bitmap Heap Scan > BitmapOr` of `heartbeat_run_idx`, the new index and `pkey` | | Buffers | 365,880 hit + 70,050 read | 9 hit | These numbers are from head `65bd9d7ac6`. The final head only renumbers the migration to 0459 after rebases; the SQL, the index and the service code are the same. Two earlier runs on the same fixture, SQL and index gave 236–425 ms without the index and 0.023–0.033 ms with the index. The machine had heavy load from other work, so the spread without the index is wide. Output check: in all four measurements, the service returned the same 5 lease IDs, and the complete query rows were equal. Index build (`CREATE INDEX CONCURRENTLY` from the migration file): 0.8–2.4 s for the 678 MB table over all runs. Index size: 3.4 MB with 30% of leases stamped. Tests: - `cd server && npx vitest run src/__tests__/native-harness-backup-expiration.test.ts`: 109 passed. The new test failed when I dropped the index before the plan check (confirmed red), and it passes with the index. - `pnpm --filter @paperclipai/db check:migrations`: passed. - `pnpm --filter @paperclipai/db typecheck` and `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm verify`: it asks for broader coverage (exit 2) because of the new script. See the CI results for this head. ## Risks - Migration time. A concurrent index build reads the full `environment_leases` table and does not block writes. Locally, the build took 0.8–2.4 s for 678 MB. The time grows linearly with the table size. I did not time it on a production-size clone. I expect a few seconds to tens of seconds on a large instance, which is in the 1–2 minute budget. This estimate is not proven. - If the build stops, PostgreSQL can leave an invalid index. The migration runner repairs invalid concurrent indexes when it runs again, and `IF NOT EXISTS` makes a rerun safe. - The index expression must stay textually equal to the query expression. If someone changes the JSON path, the planner cannot use the index. The new test catches this for custom and generic plans. - The index is partial, so ANALYZE does not collect statistics for the expression. The planner uses the default equality estimate. On the large fixture and in the small test, the planner still selects the index plan. A company with very few leases can still get a sequential scan, which is cheap. - Each lease write with a backup stamp writes one more small index entry. The write cost is small. - This change does not reduce how often the worker calls the lookup. It makes each call cheap. The other digest lookups in backup retention (`native-harness-backup-retention.ts`) and session retention checks can also use the index. - Previous-release story: no persisted state changes. The index is derived from existing lease metadata. - The hosted improvement is not verified until deployment. After deployment, compare a new query-statistics window for this statement. ## 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> |
||
|
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> |
||
|
DottaandClaude Opus 5.5
|
be0d100a8b |
fix(native): settle cleanup-gate refusals after a completed turn without a dead-end Retry (#20572)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs execute a provider turn, then close the provider session
and prove it stopped
> - When close cannot prove that the old runner, provider and tools
stopped, the runner holds a cleanup gate
(`native_session_cleanup_quarantined`) that refuses the next session on
that task
> - A run finished its provider turn and saved its work. A server
restart had left one tool callback without a saved result, so close
could not prove the drain, and the gate refused the run's own same-run
recovery 30 seconds later
> - The task then showed "Couldn't finish this attempt" with a Retry
button, but the retry route refuses this error code with 409. The saved
error text told an operator to act
> - This pull request makes the task projection, the execution blocker,
the executor and the retry route agree, and replaces the operator text
with plain wording
> - The benefit is that finished work no longer looks like a failed
attempt, and the UI never offers a Retry that the server refuses
## Linked Issues or Issue Description
**What happened?**
A remote native run completed its provider turn and saved its comments
and documents. A server restart during the turn left one semantic tool
callback without a saved result. Session close then reported
`native_session_settlement_incomplete` (runner suspended, provider drain
unproven), and the runner kept an operator-required cleanup gate. The
same-run disposition recovery was refused at admission, and the run
ended `failed` with `native_session_cleanup_quarantined`.
**Expected behavior**
The task should show its real state (here, waiting on a child task). It
should not show a failure banner or a Retry that the server refuses. No
text should ask an administrator to act.
**Steps to reproduce**
1. Start a remote native run and restart the server while a tool
callback runs.
2. Let the resumed turn complete without a semantic result, so a
same-run recovery is scheduled.
3. Observe the refusal at admission, the "Couldn't finish this attempt"
banner, and a Retry that returns 409.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
d82e5efeb9 |
perf(ui): cut repeated reads while a task is open (#20563)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The board UI keeps an open task current with a shared company
websocket and with polling fallbacks
> - In a busy company, one open task sent 35 to 60 API requests in 17
seconds, and each request waits on the database
> - Most of these requests were repeated reads: task summaries 22 times,
the task run list 11 times, task browsers 6 times, and the 60 KB agent
list 6 times
> - The cause is not the open task. Other agents' run and task events
invalidate company-wide lists on every 300 ms batch, and the task view
polls runs and browsers on fixed intervals even while the websocket is
open
> - This pull request spaces company-wide refreshes, and it lets the
websocket replace run and browser polling while it is connected
> - The benefit is about 79% fewer reads from an open task in a busy
company, with the same polling fallback when the connection is down
## Linked Issues or Issue Description
Refs #17608
Refs #9627
**What happened?**
An open task in a busy company sent the same reads again and again. In
one 17-second visit, the UI requested `issues/summaries` 22 times, the
task live runs 11 times, the task browsers 6 times, and the company
agent list 6 times. The company live runs and the task's child lists
(`parentId`, `createdFromIssueId`, `descendantOf`) were requested 2 to 3
times each.
**Expected behavior**
A busy company must not make an open task refetch company-wide lists on
every event batch. While the live connection is open, the UI must use
events, not fixed fast intervals, for task runs and browsers. The UI
must keep its polling fallback when the connection is closed.
**Steps to reproduce**
1. Open a task in a company where other agents run work.
2. Keep the task open for 17 seconds.
3. Count the API requests in the browser network panel.
**Paperclip version or commit**
Based on master at
|
||
|
DottaandClaude
|
84007892e9 |
refactor: move AGENTS.md section detail to docs and rules, add a bug-fixing section (#20585)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - `AGENTS.md` is the instruction file that each human and AI contributor reads before a change > - Pull request #20557 shortened the file, but several sections still held detail that applies to only some changes > - The scoped-rules section told agents to read each matching rule in full, and most changes match five rules > - The core engineering rules mixed three product invariants with process conventions > - This pull request moves that detail to the docs and to scoped rules, and adds a section on how to fix a bug > - The benefit is a 211-line file that states what applies to every change and points to the rest ## Linked Issues or Issue Description Refs #20557 **Issue type** Outdated or unclear contributor documentation. **Where is the issue?** `AGENTS.md` at the repository root, and `rules/`. **What's wrong?** - The scoped-rules section made agents read the full body of each matching rule before a change. The rule bodies total about 47 KB. - Sections 4, 6, 8, 9 and 10 repeated detail that belongs in `doc/DEVELOPING.md`, `doc/DATABASE.md`, a route rule, `DESIGN.md` and `CONTRIBUTING.md`. - Section 4 had a database reset command for `data/pglite`. That directory does not exist. - Section 5 had seven rules. Four of them were process conventions or path-specific detail. - The file did not say how to fix a bug. **Suggested fix** Make the rule headers the index. Move section detail to the document or rule that owns it. Keep only the three invariants in section 5. Add a bug-fixing section. ## What Changed - The scoped-rules section now tells agents to list the rule headers with one `grep` command. Agents read a rule body only for a design decision in its area, when the description is not sufficient, or when a reviewer cites it. - `rules/README.md` says that each `description` must state the required behavior in one sentence. - `rules/pull-request-review.mdc` has a description that states its requirements. - Section 4 (dev setup) and section 11 (pull requests) now point to `doc/DEVELOPING.md` and `CONTRIBUTING.md`. - The schema change workflow moved from section 6 to `doc/DATABASE.md`. - The API and auth expectations moved from section 8 to the new `rules/api-route-expectations.mdc`. - Section 9 links to `DESIGN.md` for design rules. `AGENTS.md` no longer tells contributors to run `pnpm check:token-gates`. - Added section 10, "Fixing a Bug": reproduce the error, write a failing test, write the fix, see the test pass, and verify the fix. - Section 5 now has three rules: company scope, contract synchronization, and the control-plane invariants. - The company-scope exceptions moved to the new `rules/company-scope-exceptions.mdc`. - Removed the strategic-docs rule. The plan-doc convention moved to the repo map. The artifact rule moved to section 7. - Removed rule 7. Five code comments that cited `AGENTS.md §5.7` now cite `rules/data-paths.mdc`. - Regenerated the SDK public source pin, because one of those comments is in `packages/shared/src/crash-points.ts`. - Added both new rules to `.greptile/files.json`. - The API route rule applies only to routes that read or write company data. Its status code list is a list of common codes. - The company-scope rule lists the sidebar company order as an instance-wide exception. - The `honeycomb-tracing` skill now points to `rules/data-paths.mdc` for the Telemetry workflow. ## Verification - Open `AGENTS.md` and confirm that each link resolves. A local script checked this. - Run `grep -E '^(description|globs|alwaysApply):' rules/*.mdc` and confirm that each rule prints a description. - Run `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`. It passes locally. - `packages/shared/src/crash-points.test.ts` passed locally before the rebase onto `master`. - Five source files changed in comments only. No other test suite ran locally. ## Risks - An agent that reads only rule descriptions can miss a detail in a rule body. Reviewers still apply the full rules. - Pull requests and the definition of done are now sections 11 and 12. Sections 1 to 9 keep their numbers. - Dated plan documents under `doc/plans/` still cite `AGENTS.md §5.7`. They are historical records. - Pull request #20565 also changes `AGENTS.md`. The second one to merge needs a conflict fix in section 1 and section 2. ## Model Used - Claude (Anthropic), in Claude Code with tool use. - `claude-fable-5-2`. ## 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 - [ ] 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> |
||
|
Nicky LeachandPaperclip
|
2e3cbe57a4 |
fix(native): preserve parked Runners across controller restarts (#20586)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native execution parks a released sandbox Runner for a later turn. > - The warm idle timer detaches Core and leaves Runner retention to the host. > - Controller shutdown instead stops recently parked Runners through checkpoint cleanup. > - This makes reuse depend on whether the idle timer fired before shutdown. > - This pull request uses the same detach operation for idle expiry and shutdown. > - The benefit is consistent Runner retention without a new retention or recovery mechanism. ## Linked Issues or Issue Description Fixes: #20012. Related: #20005 unified cached and adopted process reuse. Closed #19975 included this shutdown change within a broader retention proposal. ## What Changed - Detach confirmed released entries during controller shutdown. Keep busy, unreleased, and close-marked entries on the existing cleanup path. - Share the detach operation with idle expiry and cached-owner handoff. - Record that a local handle has started detaching. Later cleanup cannot fall back to stopping its Runner after a partial detach failure. - Preserve explicit environment deletion, release proof, admission, and process-reuse checks. - Add Codex and Claude restart, detach-failure, and environment-deletion regressions. Add real Runner adoption cases after the restart sweep, including managed GitHub rebinding. - Document the lifetime rule and make the shutdown warning cover both detach and checkpoint failures. ## Verification - Final source revision: `e4bb6ecbd7`. Local environment: macOS arm64, Node 26.12.0. - New controller-restart and detach-failure tests fail against unchanged master: 4 expected failures. Old shutdown calls close or reports success instead of reporting detach failure. - Focused executor tests: 22 passed. - Full native session executor suite: 1,328 passed. - `cd server && node scripts/typecheck-layers.mjs`: passed all source and test layers. - `node scripts/check-module-boundaries.mjs` and `git diff --check`: passed. - Manual diff review and added-line credential/PII scan: passed. - `cd server && node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/plugin-runner-release-resume.integration.test.ts --bail=1 --reporter=verbose`: 14 passed on this revision. This includes new Codex and Claude restart cases and managed GitHub rebinding. - Failure history: an earlier concurrent run had 4 passes and 10 failures. Its first failure was the existing Claude cached case exceeding 60 seconds. Later cases encountered a leftover prototype spy, recursive calls, and startup timeouts. No source changed before the passing full rerun. A clean worktree of unchanged master (`e01fe7f84a`) passed all 11 original cases. The first timeout's exact cause remains unknown; the later pass does not establish that cause. - Standard Greptile review: 5/5 on `e4bb6ecbd7bd965b5f56507d2c42c580e8a1d547`, with no actionable findings. - `node scripts/local-verify.mjs --explain` returned exit 2 because the execution-semantics document requires broad CI. Required CI will supply broad typecheck, test, and build coverage. - A hosted deployment and an actual controller process replacement have not been run. The added integration cases exercise the shutdown sweep and a new adopting transport against a real Runner. ## Risks - SIGINT and SIGTERM preserve confirmed parked Runners, including ordinary service stops. The host still owns eviction and retention. - Detach failure retains a fenced local owner and reports failure. Core does not infer permission to stop the Runner from that failure. - Reuse remains conditional on current admission, process configuration, and host availability. - No persisted schema, checkpoint format, launch identity, or migration changes. State from the previous release uses the existing adoption or verified restore paths. ## Model Used - OpenAI Codex, GPT-6, with reasoning, repository inspection, tool use, and local test execution. The exact deployment identifier 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 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
|
765ccd8691 |
perf(chat): index chat-admission checks by company and payload issue id (#20562)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Chat connections save each external message as a `chat_actions` row
before the server admits it as an agent wakeup.
> - The server checks if an issue holds chat input that it did not
admit. Recovery skips such an issue. Manual task start refuses it.
> - On a hosted instance, the recovery read with this check used the
most total database time: 488 calls at about 1 s each. The manual-start
check ran 155 times at about 0.9 s.
> - The check matches `chat_actions.payload->>'issueId'` against the
issue. No index covers this value, so each call reads the full
`chat_actions` table and its payloads.
> - This pull request adds one concurrent partial index for the check.
The check SQL does not change.
> - The benefit is less background database load, so request reads wait
less. The results stay the same.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The chat-admission check `unadmittedChatWakeupCondition`. Recovery uses
it in the per-issue candidate read. Manual task start uses it in the
held-input check.
**Subsystem affected**
Server chat admission, issue recovery, manual task start, and the
database schema.
**Current behavior**
The check finds `inbound_wakeup` actions by `company_id` and
`payload->>'issueId'`. Two nested checks find `failed_run_retry` actions
and later `inbound_wakeup` actions on the same company and payload issue
ID. `chat_actions` has no index on `company_id` or on the payload issue
ID. Each call reads the full table and reads each payload. On a hosted
instance, the two statements used about 630 s of database time in one
statistics window.
**Proposed behavior**
The check and its nested checks read a small set of rows from one index.
The admission result does not change.
**Reason and benefit**
The hosted database had many concurrent active queries. Request reads
waited behind background work such as this check.
**Breaking changes**
None. The migration adds an index only.
## What Changed
- Migration `0458_chat_admission_issue_index` builds
`chat_actions_company_issue_kind_idx` with `CREATE INDEX CONCURRENTLY IF
NOT EXISTS`. It runs through the existing top-level statement path,
which repairs an invalid index on retry.
- Keys: `(company_id, payload->>'issueId', kind, status)`.
- Partial: `kind IN ('inbound_wakeup', 'failed_run_retry')`.
- One index serves the outer input lookup, the failed-run retry lookup,
and the later-input lookup. All three match on the same company and
payload issue ID.
- The Drizzle schema declares the index. The snapshot chain moves to
`0458` and keeps five snapshots.
- New regression test
`server/src/__tests__/chat-admission-condition.test.ts` pins the
admission semantics on real PostgreSQL:
- Pending and failed input without a receipt holds the issue. Received,
processed and cancelled input does not. Other action kinds do not.
- A skipped receipt, a receipt for another agent, or a receipt in
another company does not admit input. A failed or cancelled run still
counts as admission.
- The issue ID is compared as company-scoped text. Uppercase, array,
null, other-issue and other-company values do not match.
- Only an admitted retry in the same conversation and issue that lists
the action releases skipped input.
- Only later admitted input in the same conversation, issue and session
generation releases a failed input. Pending input stays held.
- The check reads `chat_actions` only through the new index.
- `scripts/benchmark-chat-admission-index.ts` measures both caller
statements with and without the index and fails if the rows differ.
- `scripts/benchmark-chat-admission-index-migration.ts` times the
migration through the normal migration runner.
- `doc/DATABASE.md` documents the index and the benchmark commands.
## Verification
Local SQL measurements on synthetic data. PostgreSQL 18.1 (embedded),
default settings. The fixture has 100,995 `chat_actions` rows (662 MB):
60,000 GitHub actions with 6 KiB incompressible payloads, 2,000 issues
with 20 admitted inbound messages each (3 KiB payloads), and 1,000
failed-run retries. Each case has 2 warmups and 9 samples of `EXPLAIN
ANALYZE` execution time. Runs alternate: no index, index, no index,
index.
| Caller statement | Case | No index, median (range) | Index, median
(range) |
| --- | --- | --- | --- |
| Manual-start held check | quiet issue | 3,093 ms (2,150–3,519) / 3,586
ms (2,375–11,235) | 0.036 ms (0.034–0.052) / 0.040 ms (0.033–0.047) |
| Manual-start held check | admitted input | 3,555 ms (3,310–8,080) /
5,205 ms (3,353–9,525) | 0.049 ms / 0.052 ms |
| Manual-start held check | unadmitted input | 3,002 ms (2,864–3,221) /
4,179 ms (3,200–6,585) | 0.051 ms / 0.044 ms |
| Manual-start held check | failed, later admitted | 3,586 ms / 3,722 ms
| 0.066 ms / 0.054 ms |
| Manual-start held check | admitted retry | 2,733 ms / 4,953 ms | 0.049
ms / 0.053 ms |
| Recovery candidate read | quiet issue | 43 ms (41–48) / 96 ms (44–219)
| 0.047 ms / 0.055 ms |
| Recovery candidate read | admitted input | 203 ms (48–575) / 174 ms
(44–314) | 0.055 ms / 0.058 ms |
| Recovery candidate read | unadmitted input | 66 ms / 144 ms | 0.053 ms
/ 0.054 ms |
- Buffers per call fall from about 40,000 shared hits to 5–23.
- Locally, the recovery read without the index uses a hashed full scan
of `chat_actions`. The held check uses a full scan per call. On the
hosted instance both took about 1 s per call. The local fixture is
smaller than the hosted table.
- Correctness: complete positional rows match across all four runs for
all six cases (quiet, admitted, unadmitted, failed then admitted,
retried, other-company decoy). The script asserts the expected held
result for each case.
- Migration timing: 600,001 `chat_actions` rows (1,759 MB), 300,000 of
them in the index scope. First run through `applyPendingMigrations`: 4.2
s. Rerun after the history row is removed: 0.5 s. Concurrent build
alone: 3.4 s. Index size: 30 MB.
Commands:
```sh
pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-chat-admission-index.ts
OTHER_PAYLOAD_KIB=2 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-chat-admission-index-migration.ts
cd server && npx vitest run src/__tests__/chat-admission-condition.test.ts # 6 passed
pnpm --filter @paperclipai/db check:migrations # passed
pnpm --filter @paperclipai/server typecheck # passed
```
## Risks
- The migration builds one index concurrently. It does not block writes.
On the 1.76 GB local fixture it took about 4 s. A larger hosted table
takes longer, but the build does not lock out reads or writes.
- Writes to `inbound_wakeup` and `failed_run_retry` actions maintain one
more index. Status updates on these rows were already not HOT updates,
because other partial indexes use `status`.
- The planner has no expression statistics for a partial index. The new
test pins that the check uses the index on an analyzed table with
unrelated rows.
- The check SQL does not change, so admission behavior does not change.
> 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
|
96ab5e3d38 |
fix(recovery): read a retained Runner from its own allocation and continue unverifiable no-replay holds from the last save (#20567)
<!-- 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 a saved session in the next run. The next run first reads the retained Runner lifecycle from the sandbox. > - #20346 abandons the workspace epoch of a run that died during preparation. The next run then restores the last save into a new directory of the same sandbox. > - The retained-sandbox binding check required the old and the new directory to be the same. The warm follow-up failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding` before provider work. > - The failed run's no-replay hold then refused its files with `allocation_ownership_ambiguous`. The task stayed blocked. The text said that Retry "succeeds only after Paperclip is updated", so no click could help. > - This pull request binds the Runner read to the directory that the retained lease recorded, and lets the existing retained-source fallback settle these holds from the last save. > - The benefit is that a moved workspace continues its session, and a hold that cannot be verified continues automatically instead of dead-ending. ## Linked Issues or Issue Description Refs #20346 **What happened?** On staging, a warm native continuation ran about 10 minutes after a deploy. The run before it died during preparation (database deadlock), so its workspace epoch was abandoned. The continuation restored the last save into a new allocation directory of the same Daytona sandbox. It then failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding`, because the retained lease of the committed source run recorded the old directory. The failure recorded a no-replay hold (`native_continuation_requires_reconciliation`, resolved with outcome `blocked`). Workspace recovery refused the source with `allocation_ownership_ambiguous`, because the sandbox's reusable-lease chain had predecessors in another directory and a non-native preparation-only run. The task stayed blocked with "Retry runs the check again and succeeds only after Paperclip is updated." **Expected behavior** The continuation reads the retained Runner state from the directory where that Runner wrote it, and continues. When retained files cannot be verified, the task continues from the last committed save without a person. **Steps to reproduce** 1. Run a warm native session on a reusable remote sandbox and let it commit. 2. Fail the next run during preparation, so that its epoch is abandoned (#20346). 3. Start the next warm continuation. It restores the last save into a new allocation directory and fails with `retained_sandbox_binding`. 4. See the hold refuse its source with `allocation_ownership_ambiguous` and stay blocked. **Paperclip version or commit** Staging `c874cfa4dacc` (includes #20346). ## What Changed - `assertNativeRetainedSandboxBinding` now returns the directory that the retained lease recorded. It accepts a different target directory only when both are canonical sibling allocations of the same execution workspace under the sandbox's `.paperclip-workspaces` (`<workspaceId>` or `<workspaceId>-<epoch>-<leaseId>`). Same sandbox (`providerLeaseId`), environment, run, issue and committed owner are still required. Other directories stay refused. - `restoreRemoteSuspendedRunnerCheckpoint` reads the Runner state from that recorded directory. A moved source must be suspended and settled. A Runner left in the old allocation is never adopted live. - The retained-source fallback (`continueFromLastSavedWorkspace`) and its discovery now also cover the native no-replay hold (`native_continuation_requires_reconciliation`, active or resolved with outcome `blocked`, replay blocked for the same run). Rows saved by earlier releases are discovered without migration. - The fallback owns the Blocked status of such a hold only while the recorded `nativeFailureBlock` status version still matches the task. Otherwise it keeps the newer block and starts no continuation. - The refusal text no longer says "succeeds only after Paperclip is updated", and it promises no fallback. - `doc/execution-semantics.md` describes both rules. ## Verification - Red (unfixed code): `native-controller-recovery.integration.test.ts` "binds a retained Runner read to the source's directory after the workspace moved within the same sandbox" failed with `native_controller_recovery_missing_evidence:retained_sandbox_binding` at `native-controller-recovery.ts:145`, the staging error. - Red (unfixed code): `workspace-recovery-fallback.test.ts` "continues a resolved-but-blocked native no-replay hold from the last save, discovered by the worker" failed: the worker found no due work for the exact staging row. The newer-block safety case and the message case in `system-recovery-state.test.ts` also failed. - Provider boundary: a new test drives `restoreRemoteSuspendedRunnerCheckpoint` through a sandbox runner that refuses paths outside the sandbox workspace base. The suspended case reads `runner-state.json` from the source directory, never the moved root. The ready case is refused with `runner_not_settled` and caches nothing. - Green: `native-controller-recovery.integration.test.ts`, `workspace-recovery-fallback.test.ts` and `system-recovery-state.test.ts` pass (45 tests). The neighbouring suites `workspace-recovery-retry-copyback`, `workspace-recovery-preparation-gates` and `workspace-recovery-stop-proof` pass (135 tests). - Safety cases: another workspace, another parent, a non-canonical path or name (`-0-`, trailing text), `/unrelated` and another sandbox stay refused. A hold whose task moved to a newer Blocked decision keeps the block and starts no continuation. Exactly one continuation is delivered. Native replacement is denied for the abandoned source, so no second executor starts. - `npx tsc --noEmit -p server/tsconfig.json` passes. `pnpm verify` returns exit 2 (a global doc input changed), so required CI supplies the broad coverage. ## Risks - Previous-release state: rows saved by earlier releases carry forward without migration. The binding reads existing lease metadata. The fallback discovers existing resolved-but-blocked holds by their existing evidence fields. - The binding now accepts a sibling allocation directory. The read still binds to the exact sandbox, run, environment and committed owner, and it reads only the source's recorded directory. A non-suspended Runner in a moved allocation is refused, not adopted. - The fallback abandons the unverified epoch and continues from the last committed save with a fresh session. In the staging shape the run did no provider work after it restored that save, so no work is lost. Unverified files are attached to the task when they can be read, as before. - Not verified live: a real warm Codex resume after a moved allocation 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 - [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
|
faaae003a9 |
perf(chat): find the conversation head from open delivery work (#20564)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - External chat messages go into a durable delivery queue. Each
conversation drains in provider order.
> - Each drain step reads the head of one conversation. The head query
filters one endpoint's deliveries by state and thread.
> - On a hosted instance, this query ran about 10,000 times at about 300
ms each. The database was busy, and request reads waited behind
background work.
> - PostgreSQL read every processed delivery of the endpoint and its
`normalized_event` value to find a few open rows.
> - This pull request finds the endpoint's open delivery IDs first and
fetches only those rows.
> - The benefit is less background database load. The selected head and
the delivery order do not change.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The inbound conversation head query
(`earliestOpenConversationDelivery`).
**Subsystem affected**
Server chat inbound delivery queue.
**Current behavior**
The query has `endpoint_id = $1 and (state in ('received', 'retry',
'processing') or (state = 'processed' and <pending inbound wake>))`, a
thread filter on `normalized_event`, and a sort on `normalized_event`
keys. No index matches the `or`. PostgreSQL scans all deliveries of the
endpoint. For each row it reads `normalized_event` to test the thread.
It also reads all of `chat_actions` to build the pending-wake set. A
busy endpoint keeps all of its processed history, so the cost grows with
that history.
**Proposed behavior**
The query selects `id in (<open delivery IDs of the endpoint>)`. That
set is a `union all` of two small reads:
- received, retry and processing deliveries of the endpoint, through the
existing `chat_deliveries_work_idx`;
- processed deliveries that an issued or processing `inbound_wakeup`
action of the same endpoint (and company) still owns.
PostgreSQL fetches only those rows by primary key, then applies the same
reaction, thread and order rules.
**Reason and benefit**
The cost no longer grows with processed history. On a local fixture with
60,000 processed deliveries, the head query reads 1,309 shared buffers
instead of 262,630.
**Breaking changes**
None. There is no schema change, migration, API change, timer or poll
change.
Related work: #17971 changed the pending-wake predicate of the same
queue. #20400 added the index for the other chat delivery statement in
the same report (`select state ...
normalized_event->'githubAutomatic'->'context'->>'deliveryId'`). That
statement already uses the #20400 index on master, so this pull request
does not change it.
## What Changed
- `delivery-ledger.ts`: add `openConversationDeliveryIds(endpointId)`.
It returns the endpoint's open delivery IDs. It selects the same set as
the old state predicate for that endpoint.
- `conversation-queue.ts`: the conversation head query uses `id in
(openConversationDeliveryIds(endpointId))` in place of the state `or`.
The thread filter, reaction exclusion, order, limit and lifecycle
dependency read do not change.
- `inbound-queue-discovery.test.ts`: add a real PostgreSQL test. It
compares the new set with the old predicate on three endpoints. It
covers each state, issued and processing wakes, duplicate wakes, wakes
of non-processed deliveries, processed, other-kind and null-delivery
actions, an action on a different endpoint of the same company, and
reactions. It also checks that the real captured head query returns the
oracle's first row.
- `benchmark-inbound-queue-discovery.ts`: add
`--endpoint-history=<rows>` (a busy endpoint with 4 KiB payloads) and
`--only=<fixture>`. Add a work budget: the treatment head query must
visit at most 100 delivery rows on the history fixtures.
- `doc/DATABASE.md`: describe the head query and the benchmark command.
## Verification
Local SQL measurements. PostgreSQL 17.10 (Postgres.app, macOS arm64),
disposable database, synthetic data, default settings with 256 MB shared
buffers. The host was under heavy load from other work, so timings are
noisy. Shared buffers and visited rows are stable. Each arm has two
warmups and five alternating samples (median [min..max]). Baseline:
`origin/master` at `8d2bf78323`.
| Fixture | Query | Baseline | Treatment | Shared buffers | Delivery
rows visited | Output |
| --- | --- | ---: | ---: | ---: | ---: | --- |
| 60,000 processed, 4 KiB | head | 290.09 ms [220.21..554.19] | 7.34 ms
[5.85..7.68] | 262,630 → 1,309 | 60,002 → 4 | Equal |
| 20,000 processed, 4 KiB | head | 276.02 ms [211.77..322.12] | 34.61 ms
[4.45..194.15] | 67,618 → 486 | 20,002 → 4 | Equal |
| 1,500 processed, 32 KiB | head | 15.88 ms [13.66..21.79] | 2.29 ms
[1.64..7.75] | 10,602 → 80 | 1,502 → 4 | Equal |
| 20 processed | head | 1.49 ms [0.92..3.66] | 1.85 ms [1.01..1.97] | 2
→ 45 | 22 → 968 | Equal |
- The deadline and ready selectors do not change. Their buffers are
identical in both arms; their timings move only with host noise.
- The small control is neutral. On a near-empty table, PostgreSQL picks
a sequential scan for the open-state branch. This costs about 45
buffers.
- The 60,000-row run passes `--require-work-budget`. The baseline
exceeds the budget (`baselineExceedsWorkBudget: true`). This is the
red/green proof. There is no timing threshold in CI.
- On the treatment, the remaining head cost is the scan of
`chat_actions` for pending wakes of the endpoint. The baseline also
reads all of `chat_actions` for its hashed subplan.
- `pnpm exec vitest run src/__tests__/inbound-queue-discovery.test.ts`
(server): 6 tests pass, including the new equivalence test.
- `chat-channels.integration.test.ts` filtered with `-t
"reorder|orders|FIFO|conversation drain|lease renewal|receipt
order|edit|lifecycle"`: 148 assembled chat tests pass. They cover Slack,
Teams, Telegram, Discord and GitHub provider-order drains, head-of-line
barriers, lease loss, lifecycle edits and wake retries.
- `pnpm exec tsc --noEmit -p tsconfig.json` (server) passes.
Reproduce:
```sh
pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-inbound-queue-discovery.ts --baseline-ref=8d2bf78323 --endpoint-history=60000 --only=endpoint-history --require-work-budget
```
Set `PAPERCLIP_TEST_POSTGRES_ADMIN_URL` to use an existing disposable
PostgreSQL server. The benchmark uses synthetic data only.
## Risks
- Query plans depend on data and settings. These local numbers do not
prove hosted request latency. A follow-up statistics window after deploy
must confirm the gain.
- The pending-wake branch still scans the endpoint's `chat_actions`. A
partial index on pending `inbound_wakeup` actions would remove that
scan. I did not add it, to avoid a migration.
- Low behavior risk. The new set equals the old predicate for each
endpoint, and the order expression is unchanged. Tests compare both on
the same rows.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell,
file edit) 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
- [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
|
56aa160a4e |
perf(native): index heartbeat runs by company and native session (#20560)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs share a native session. Retention, recovery, resume and archive code reads all runs of one session from `heartbeat_runs`. > - On a hosted instance, the native retention lookup ran about 67,000 times in one day at about 22 ms, and about 10,000 times at about 11 ms in a later window. > - No index has `native_session_id`. Each read walks the agent's or the company's run history and filters the rows. > - This pull request adds one concurrent partial index on `(company_id, native_session_id)`. The SQL does not change. > - The benefit is less background database load. Request reads wait less, and the results stay the same. ## Linked Issues or Issue Description **What existing behavior does this improve?** The native session-run lookups. The largest caller is `sessionRetentionBlocker` in native harness backup and checkpoint retention. It selects `id, status, finished_at`, an incomplete-recovery flag and the issue ID for one company, agent and native session. **Subsystem affected** Server native runtime (retention, recovery, resume and archive reads) and the database schema. **Current behavior** `heartbeat_runs` has no index on `native_session_id`. PostgreSQL reads the whole run history of the company (or of the agent through `heartbeat_runs_company_agent_started_idx`) and filters by session. The cost grows with the run history, not with the size of the session. **Proposed behavior** The index finds the few runs of one session directly. Every caller already filters by `company_id` and `native_session_id`, so one index serves the agent-scoped and the issue-scoped reads. **Reason and benefit** This statement family used about 1,500 s of database time in one day on a busy hosted database. The local proof below shows 12.3 ms to 0.04 ms per call. ## What Changed - `packages/db`: add `heartbeat_runs_company_native_session_idx` on `(company_id, native_session_id) WHERE native_session_id IS NOT NULL`. Migration `0457_native_session_run_index` builds it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration is registered as a top-level-statement migration, the same way as 0454–0456. - Add a regression test in `native-harness-backup-expiration.test.ts`. It captures the real retention lookup and proves that its plan can use the index. It also pins the company and session scope: a live run of another company with the same session ID does not protect a checkpoint. A live run of the same agent in another session does not protect it. A live run of the same session does protect it. - Add `scripts/benchmark-native-session-run-index.ts`. It runs the real checkpoint retention pass (dry run), captures the exact SQL, and replays it with and without the index. It also times the migration through the normal migration runner. - `doc/DATABASE.md`: describe the index and the benchmark command. ## Verification Local SQL measurements, synthetic data. PostgreSQL 18.1 (embedded test server). Fixture: 100,000 runs (50,000 of the measured agent, 50,000 of a second agent), 4 runs per session, 4 KiB incompressible `runner_profile_json` per run, 638 MB table. Order: no index, index, no index, index. 15 `EXPLAIN (ANALYZE, BUFFERS)` samples after 3 warmups. | Pass | Plan | Median execution | Min / max | Shared buffers | | --- | --- | --- | --- | --- | | No index (1) | Seq Scan | 12.30 ms | 10.33 / 35.27 ms | 3,607 | | Index (2) | Index Scan using `heartbeat_runs_company_native_session_idx` | 0.036 ms | 0.021 / 0.115 ms | 38 | | No index (3) | Seq Scan | 12.38 ms | 10.24 / 26.14 ms | 3,607 | | Index (4) | Index Scan using `heartbeat_runs_company_native_session_idx` | 0.043 ms | 0.022 / 0.098 ms | 38 | - Correctness: the service SQL is the same in every pass. The returned rows (all columns) and the retention result are equal in every pass. The script fails on any difference. - Migration: the first run through the migration runner took 525 ms on the 638 MB fixture. The rerun took 273 ms and skipped the existing index. A direct concurrent build took 145–221 ms. Index size: 1.9 MB. - `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-native-session-run-index.ts` - `npx vitest run src/__tests__/native-harness-backup-expiration.test.ts` (server): 108 passed. - `pnpm --filter @paperclipai/db check:migrations`: passed. Five snapshots remain. - `pnpm verify` exits 2 (a new script under `scripts/` is a global CI input). Required CI supplies the broad coverage. ## Risks - Low risk. The migration adds one index concurrently and does not lock writes. The SQL of every caller is unchanged. - The index adds a small write cost to `heartbeat_runs` inserts and to updates of `native_session_id`. Rows with a null session are not indexed. - Persisted run state: no saved run, session or checkpoint shape changes. State saved by the previous release carries forward unchanged. - Hosted latency is not measured yet. The numbers above are local SQL measurements. ## 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> |
||
|
|
79e28dc0d0 |
fix(runner): record pending calls at sidecar shutdown (#20272)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The ACPX sidecar carries task tools between a provider and its controller. > - A closed controller pipe rejects all pending tools with `sidecar stdin closed`. > - Current logs do not record the first shutdown trigger or the pending calls at that point. > - The sampled runs record a server SIGTERM before the 300-second tool idle timeouts and later pipe failures. The exact pipe-close cause remains unresolved. > - This pull request adds one bounded shutdown record before cleanup removes the evidence. > - Operators can use it to compare controller shutdown with pending tool calls. ## Linked Issues or Issue Description **What happened?** Several task document reads failed together with `sidecar stdin closed`. In two sampled staging traces, earlier task tools reached a 300-second idle timeout. The sidecar source maps the closure error to pending-call rejection during shutdown. It does not record the trigger or the pending call population. Pattern: [Task reads lose their tool connection](https://www.braintrust.dev/app/Paperclip/p/Paperclip%20Staging/patterns/5544e6f3-9e33-4661-95c3-5849c9a8f2b6). **Expected behavior** Record the local shutdown reason and a bounded snapshot of pending calls before cleanup removes them. Keep tool cancellation and authorization unchanged. **Steps to reproduce** 1. Start the sidecar with pending task calls. 2. Close controller stdin, or send SIGTERM or SIGINT. 3. Inspect the sidecar event stream and stderr. **Paperclip version or commit** Baseline: `33a23e024697552a4a639e49957a3443c75a5bd5`. ### Hypothesis and thesis for a fix The provisional hypothesis is that controller teardown causes the batch failures after an earlier stall. The source proves how EOF rejects pending tools. The traces establish ordering. They do not prove why the controller closed stdin or why the earlier calls stalled. Run logs now confirm an earlier server shutdown in both examples. One later recovery could not find the provider; the other later ended at a durable receipt limit. The first failed settlement and exact pipe-close trigger remain unresolved. The later example already used a provider revision containing #19365 and #19290, so those earlier fixes do not alone explain all occurrences. This is a **diagnostic-only change**. The sidecar owns the local shutdown signal and its pending waiters. Record that state once at the first shutdown request. Do not add retries or change deadlines without evidence. This supports each provider that uses this sidecar. It preserves company/task/run authority, approval checks, tool admission and cleanup ownership. It does not resolve or claim recovery of the original read stalls. ## What Changed - Emit `sidecar_shutdown_requested` through the existing diagnostic stream and stderr before shutdown can remove waiters. - Record pending tool, input, permission and completion-normalization counts. Include at most 16 oldest tool calls with elapsed time and a SHA-256 call ID. Exclude arguments, results, operation names and raw call IDs. - Add real sidecar process tests for EOF and both shutdown signals. Check successful resolution, cancelled calls, pending-call rejection, provider cleanup and sidecar exit with code 0 and no signal before fallback cleanup. - Document the record and its limits. Saved session formats and previous-release behavior stay unchanged. ## Verification - Candidate: `85a929eef87d5f775bbed9f2c98409432c86c702`. The baseline replay uses an isolated detached checkout at the baseline SHA with the exact candidate regression file. - Baseline red: the three process regressions failed because the expected shutdown record was absent. The sidecar initialized, handled calls and shut down before each assertion. - Unchanged process regressions pass with the implementation. The focused sidecar and snapshot suites passed: 117 tests, one unrelated macOS candidate-admission test skipped on Linux x64. - `cd packages/paperclip-runner && pnpm exec vitest run src/cli/acpx-runtime-sidecar.test.ts src/cli/acpx-sidecar-shutdown.test.ts` - `cd packages/paperclip-runner && node --test test/acpx-sidecar-contract.test.mjs`: 16 passed. - `cd packages/paperclip-runner && pnpm exec tsc -p tsconfig.json --noEmit`: passed after building the local eval-kernel dependency. - `git diff --check`: passed. - These tests use the real sidecar pipe and process boundary with a controlled provider host. Independent outcome and cleanup files prove rejection and process cleanup. They do not exercise a live authenticated Claude/staging run or prove deployed recovery. - The local preflight could not build Rust because this host has no Cargo. CI must supply the applicable Rust/build checks. At head `85a929eef87d5f775bbed9f2c98409432c86c702`, all 61 check runs are complete: 58 passed and three conditional jobs skipped. The legacy Snyk status passed. Greptile is 5/5 with both threads resolved. [CI result](https://github.com/paperclipai/paperclip/actions/runs/42630335496). ## Risks - The controller can stop reading before it receives the diagnostic frame. Stderr is an additional path, not a delivery guarantee. - EOF records a local pipe closure. It cannot identify the upstream controller cause by itself. - Hashed IDs require offline hashing of the existing tool-call journal for correlation. At most 16 samples are retained; counts include the full pending population. - No persistent state, retry, cancellation or authorization contract changes. ## 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> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
bf0220d34a |
perf(workspaces): migrate with one archive and no readback (#20554)
## Thinking Path > - Paperclip coordinates agents and keeps their workspace files. > - A legacy workspace must move from a local volume to object storage. > - The importer creates an archive, downloads and restores it, and captures the source again. > - Parent imports also download and recapture children that already have committed snapshots. > - This repeated work makes the one-time migration slow. > - This change copies each source file once and trusts the upload acknowledgement. > - Existing source locks and atomic publication still own the migration. ## Linked Issues or Issue Description **What existing behavior does this improve?** The one-time migration of legacy local and Git workspaces to durable snapshots. **Current behavior** Each workspace is captured twice and restored as a test during migration. Parent imports also read back already imported child snapshots. Capture hashes source files before writing them and scans them again afterward. **Proposed behavior** Build one archive from one source-file read, encrypt it, upload it, and publish its reference. Trust the upload acknowledgement. Reuse journaled uploads after an interrupted process. Retain committed child snapshots by database reference. **Reason and benefit** Remove repeated source reads, temporary copies, complete downloads, and recursive child verification. The tests assert one capture, one upload, and zero migration downloads. **Breaking changes** The archive format stays compatible. File hashes required by current readers are computed inline during the copy. There is no preliminary file-hashing pass. Existing readers can cold-restore the result. Related work: #19817 and #20447. ## What Changed - Add single-pass capture that fills fixed-length checksum fields in the existing archive header after copying. - Remove migration readback, test extraction, fresh-source recapture, and Git integrity scans. - Compute the encrypted archive checksum while writing it. - Reuse acknowledged and journaled uploads without rebuilding or downloading them. - Retain already imported children through their database bindings. Remove the recursive payload verification cache. - Keep company checks, source and writer locks, recovery bindings, journal identities, cancellation, and atomic publication. - Bind archived repository intent paths to the operation without reconstructing an archive to check them. - Update the migration contract and the affected tests. ## Verification - Node 24.22.2 and pnpm 9.15.4 on macOS arm64. - At |
||
|
DottaandClaude
|
94cfb84150 |
fix(scripts): build the plugin SDK in the agent worktree preflight (#20473)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Contributors and agents prepare each new git worktree with `node
scripts/agent-preflight.mjs`
> - The server imports `@paperclipai/plugin-sdk` through its built
`dist`. The preflight did not build that `dist`.
> - On a fresh worktree, the server and the restart-chaos harness stop
at startup with `ERR_MODULE_NOT_FOUND` for
`server/node_modules/@paperclipai/plugin-sdk/dist/index.js`
> - The SDK exports `dist` on purpose: plugin workers load it under
plain Node. CI, `paperclipai run` and the typecheck scripts already
build it with `scripts/ensure-plugin-build-deps.mjs`.
> - This pull request makes the preflight run the same builder as a step
> - The benefit is that one preflight command gives a worktree that can
start the server and the harness
## Linked Issues or Issue Description
**What happened?**
After `node scripts/agent-preflight.mjs` on a fresh worktree, `node
cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts ...` stops
at startup. The error is `ERR_MODULE_NOT_FOUND` for
`server/node_modules/@paperclipai/plugin-sdk/dist/index.js`, imported
from `server/src/services/reusable-sandbox-resume-error.ts`. The
workaround is `pnpm --filter @paperclipai/plugin-sdk build`.
**Expected behavior**
The preflight prepares everything that the server needs to start. The
server and the harness import their modules without a manual build.
**Steps to reproduce**
1. Make a new worktree from `origin/master`.
2. Run `node scripts/agent-preflight.mjs`.
3. See that `packages/plugins/sdk/dist` does not exist.
4. From `server/`, import
`./src/services/reusable-sandbox-resume-error.ts` with tsx. The import
fails with `ERR_MODULE_NOT_FOUND`.
**Paperclip version or commit**
`origin/master` at
|
||
|
|
f4337bb5fb |
test(evals): land the chat catalog eval harness on master (#19614)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The chat-feature evals measure whether agents complete real chat requests through the product > - The chat catalog eval harness and its repairs live on the `chat-eval/master-candidate` branch, not on master > - Today's work made 139 of 242 chat evals pass in verified single-case live runs. Each product fix landed on master as its own pull request > - The eval-side changes (setups, graders, fault seams, evidence capture) still exist only on the candidate > - This pull request lands the harness on master. Master stays the source of truth for product behavior > - The benefit is one tree: evals run against master commits, and product fixes reach the evals as soon as they merge ## Linked Issues or Issue Description Refs #19441, Refs #19599, Refs #20270, Refs #20334, Refs #20342, Refs #20344, Refs #20354, Refs #20360, Refs #20373, Refs #20382, Refs #20409 **What existing behavior does this improve?** The chat catalog eval harness and the product it evaluates live on a long-running branch outside master. Eval fixes do not reach master, so master-based eval runs use old setups and graders. **Subsystem affected** `tests/runner-e2e` chat catalog harness, plus a small set of product changes that exist only on the candidate. **Proposed change** Land the harness on master. Every product fix from this eval work already merged to master as its own pull request. Conflicts keep master's product behavior. ## What Changed - Most of the diff (808 of 851 files) is under `tests/runner-e2e/`: chat-catalog domains, scenarios, descriptors, the TestOps manifest, private evidence tooling, and their tests. - Graders now check saved state, setup literals and rendered or clicked links. Many keyword and regex checks on free-form replies are removed. Each changed grader has a bumped `oracleVersion` and calibration tests (a valid reply, a reworded valid reply, and broken variants that must fail). - The shared link helper `hasLink` no longer treats a list link (for example `/COM/issues?view=mine`) as a link to every task without a URL key. - Setup fixes build real state through supported product routes: routine history, agent grants through the board grant route, budget holds with a real cap, and a storage-layer archive fault for `inbox-search.archive-partial-failure`. - Browser boundary journeys (sign-in, chat-disabled, onboarding) now write the standard pass evidence files. - Retargeted cases, approved by the maintainer: `agent-chat.chat-disabled` (Conference Room gate), `agents.restore-instructions` (no saved instruction history), `access.manage-membership` (member permission edits stay board-only). - The private evidence trusted files (`manual-private-handoff.ts`, `manual-private-retention.ts`, `manual-private-report-assets.ts`) are byte-identical to master. - Product files outside `tests/` (43 files, +1,328 −49) carry candidate-only changes that master does not yet have. Most are tests. The main code changes are in `request-human-input.ts`, `heartbeat-run-summary.ts`, `native-completion-feedback.ts`, `native-run-finalizer.ts`, `openapi.ts`, `secrets.ts` and `IssueChatLongThreadPerf.tsx`. ## Verification - `pnpm vitest run chat-catalog-domains chat-catalog-contract.test.ts chat-catalog-consistency.test.ts unit-launcher.test.mjs --testTimeout=180000` in `tests/runner-e2e`: 269 files pass, 3,550 tests pass. - `node --test chat-full-catalog-testops-manifest.test.mjs`: 10 pass. - `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`: no change. - `tsc` for `server` and `tests/runner-e2e`: clean. - Live single-case verification: 139 of 242 cases passed on paid runs, and a person read the saved evidence for each. These are single-case runs, not one full-cohort run. ## Risks - Large diff. Most of it is test-only, but the 43 non-test files change product code paths. Review those files first. - Some graders are now less strict on reply wording. A person must read replies for meaning on those cases. - 42 cases need disposable outside accounts (Notion, Slack, GitHub, Daytona and others) and stay blocked. `access.invite-teammate` stays failing until local agent isolation lands, because the runner's durable state keeps a live invite token. - The paid workflow compares the trusted private-evidence files with master. They are unchanged, so this risk is low. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution, in Claude Code. Subagents used the same model. ## 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> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
15a734d068 |
fix(heartbeat): show a storage outage wait as live status, then record it once (#20460)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In cloud mode a run reads and writes file storage on its critical path: it starts its run log (`s3_segments`), restores its saved workspace, and saves it again > - Since #20299 a run waits out a storage outage there for up to 5 minutes instead of failing > - That wait was silent. The board showed "running" with no sign of life, so a storage wait looked like a dead run (restart-chaos R5 `silent_dead_run`) > - The first version of this PR wrote a run event on every wait. That writes the run row (`next_event_seq`, `updated_at`) while the run-log claim is still in progress, and it covered only the run-log begin > - This PR shows the wait as live run status instead, writes nothing during the wait, and records one lifecycle event when the wait ends. It covers every critical-path wait in the run's storage scope > - The benefit is that a storage outage shows on the board as waiting, the run's history keeps one honest line about it, and the claim path is left alone ## Linked Issues or Issue Description Refs #20299 (wait out a store outage on a run's critical path). **What happened?** - `w-20261110T043533-s11936` (Daytona R1, ST1 194 s, K2): run `d49f58d8` waited 186 s in `heartbeat.prepare.run_output` with no event. The grader failed it as `R5:silent_dead_run`. - `w-20261110T105903-s11936`: the restore download (`storage.snapshot.download`) waited 182 s with no sign of life. **Expected:** the board shows the run waiting for file storage, and nothing about the notice can disturb run-log ownership. **On the failed `run_output` seen on builds of the first version** (`w-20261110T101517-s11936` t01 run `9d39550a`, `w-20261110T081647-s11936` t02 run `9da10424`): the evidence points to an expired restart lease, not to the notice's row writes. Neither the writer-authority check (`run-log-authority.ts`) nor the journal claim compares `next_event_seq` or `updated_at`. A remote `bootstrap_incomplete` restart claim gets a 60 s lease (`REMOTE_CONTROLLER_LEASE_MS`), and preparation renews it only for `reattach_remote_runner` claims. In t01 the claim was at 10:19:55, the outage ended at 10:22:18.15, and `run_output` failed at 10:22:18.17, on the first authority check after the store came back. That fix ships separately as its own PR. ## What Changed - `storage/storage-unavailable.ts`: - The run's storage scope can carry a wait observer (`reportStorageOutageWaitsTo`). - `retryThroughStorageOutage` tells it about each backoff (`waiting`). The operation that owns the wait reports one `ended` (`recovered` true or false), so nested waits do not double-report. - `storageOutageWaitFor(purpose, work)` names the step. - An observer that throws never changes the wait. `endStorageOutageWaitOn` keeps the observer. - `heartbeat/storage-wait-notice.ts` (new): - Each wait sets live runtime status (phase `waiting_for_storage`, "Waiting for file storage before starting the run log (45s so far)") and publishes `heartbeat.run.progress`. Nothing goes to the database. - When the wait ends, it writes one `lifecycle` warn event: "Waited Ns for file storage before …", with payload `reason: "storage_outage_wait"`, `purpose`, `waitedMs` and `recovered`. - `heartbeat.ts`: installs the notice next to `endStorageOutageWaitOn` (one line). - Purposes: - run-log begin: "starting the run log"; - `loadWorkspaceSnapshotArchive`: "restoring the workspace"; - `uploadWorkspaceSnapshot`: "saving the workspace". - `packages/shared`: adds `waiting_for_storage` to `HeartbeatRunStatusPhase`. - `tests/restart-chaos/live-status-evidence.ts` (new): R5 also counts the board's live run status (`/api/issues/:id/live-runs` `currentStatusUpdatedAt` / `lastEventAt`) as evidence that a running run is alive. Wiring it into `observer.ts` takes one import and one changed line. - `doc/run-log-events.md`: documents the live status and the one event. - Removed: the first version's per-wait run event. ## Verification - `cd server && npx vitest run src/services/heartbeat/run-output.test.ts src/__tests__/storage-outage-wait.test.ts src/services/remote-workspace-snapshot-storage.test.ts src/services/object-byte-journal.test.ts src/__tests__/storage-s3-credential-rotation.test.ts`: 103 pass. - Red/green: with the observer disabled (master behaviour), these new tests fail. With this change they pass: - run-output "shows the wait as live status and writes nothing to the run while its log claim waits": no run-row write and no event during the claim's waits; the live status is `waiting_for_storage`; the claim's writer-authority check still passes; one lifecycle event afterwards; - the helper's observer tests (one end per wait; gave-up reporting; a throwing observer; the Stop signal keeps the observer); - snapshot storage "tells the run it is waiting for storage while a save or a restore waits out an outage". - `PAPERCLIP_IN_WORKTREE=false npx vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/live-status-evidence.test.ts tests/restart-chaos/observer.test.ts tests/restart-chaos/triggers.test.ts`: 65 pass. `node scripts/typecheck-test-projects.mjs tests/restart-chaos`: pass. `cd server && npx tsc --noEmit`: pass. - Live restart-chaos waves: results in a comment below. ## Risks - Low. - The retry policy, its budget and Stop handling are unchanged. - During a wait, the only new effect is an in-memory status and a live event. - Afterwards, one event per wait, written without blocking the run. - Runtime status is per server process. A board on another instance sees the waiting line only after the closing event. - Shared journal work (`runSharedStorageWork`) keeps its own scope without the observer. Its waits surface through the enclosing run-scoped wait, as before. - Previous-release state: this persists no new run or session state. A run saved by the previous release resumes unchanged. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
fea4530a30 |
fix(recovery): retry a preparation killed by a restart in the uncharged lane (#20534)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, an interrupted run must continue on its own. A user must not have to press Retry because a deploy or a crash happened > - A preparation that a restart stops before the provider starts spent no provider work. It is retried in the uncharged `interrupted_preparation` lane (12 retries), not the charged `transient_failure` lane (2 retries) > - #20496 made that work for a graceful drain (SIGTERM). A hard kill (SIGKILL) during workspace preparation still landed in the charged lane > - Two killed preparations in one chain then spent the failure budget, and the next interruption could not continue: the task needed a manual Retry > - This pull request lets restart recovery hand a killed preparation to the uncharged lane once the killed controller is proven gone > - The benefit is that a task survives any number of restarts during workspace preparation (up to the crash-loop cap) without a Retry click ## Linked Issues or Issue Description **Subsystem affected** Heartbeat restart recovery: the process-loss reaper in `server/src/services/heartbeat/recovery.ts`. **Problem or motivation** The restart-chaos harness on Daytona with `--storage s3 --storage-trigger snapshot-download` (seed 603, task t02, a codex C1 follow-up) kills the server while a workspace snapshot downloads. On builds that contain #20496, two replays still ended in `R3:retry_needed`. The run chain in replay `w-20261110T061107-s603-replay-t02-1794309789008`: 1. K1 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 1/2 … transient_failure". 2. K2 drain during preparation: `interrupted_preparation` 1 (the #20496 path, correct). 3. K4 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 2/2 … transient_failure". 4. K2 drain during preparation: the failure budget was spent (`failureRetries` 2), so the hold could not continue. The harness had to click Retry. **Root cause** The reaper does record the no-provider-work certificate (`executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`) for a preparing run from another boot. But before it fails the run, `revokeExpiredLegacyControllerWithState` swaps in a random boot id with a 60 s lease, a tombstone that no controller holds. `scheduleBoundedRetryForRun` then asks `isInterruptedNativePreparation`, whose `preparerMayStillUnwind` check sees that live lease and says the old preparer may still be unwinding. The lane falls back to `transient_failure` and charges the failure budget. Every killed preparation is charged, whether or not its controller is provably gone. **Proposed solution** When the reaper has already proven the old controller's recorded process gone (`legacyControllerGone`, the shared owner prover that #20496 also uses) and the revoked state shows the run was still preparing with no process recorded (`restartInterruptedBeforeDispatch`), expire that tombstone lease in the same `process_lost` write. The lane decision then sees an expired lease and applies its existing checks (no coordinator, result or execution events; leases released with stop receipts; workspace repair when needed). Any evidence of provider work, or a controller that cannot be proven gone, keeps today's charged behaviour. **Alternatives considered** - Revoking with an already expired lease: the 60 s revocation lease fences other servers while this pass fails the run, so the expiry belongs in the terminal write. - Teaching `preparerMayStillUnwind` to ignore unstamped leases: that would also skip the wait for a lease of a live controller saved by an older release. ## What Changed - `server/src/services/heartbeat/recovery.ts`: the `process_lost` write sets `controllerLeaseExpiresAt` to now when `restartInterruptedBeforeDispatch && legacyControllerGone`. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: - New: a chain whose preparation is killed twice (controllers proven gone) and then drained. Each kill schedules `interrupted_preparation` (attempts 1 and 2, `failureRetries` 0), and after the drain the restart settlement schedules `interrupted_preparation` 3 without a Retry. - New old-saved/new-resumed case: a preparing run saved by an older release without a controller process stamp keeps the charged `transient_failure` lane. - `doc/execution-semantics.md`: one additive sentence saying the lane also covers a hard kill once the killed controller is proven gone. ## Verification - Red/green: `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "killed twice"` fails on master (the first kill schedules `transient_failure`) and passes with the fix. - Related suites pass: `heartbeat-process-recovery`, `interrupted-preparation-hold-settle`, `execution-recovery-dead-controller`, `legacy-controller-lease` and `ownership-reconciliation-report` (5 files, 521 tests). - Live (Daytona, `--storage s3 --storage-trigger snapshot-download`, replay of `w-20261110T061107-s603/t02` with K1/K2/K4/K2, image `paperclip-daytona-runner@sha256:05a4a092…` whose runner content matches this branch): running; the result will be posted as a comment. ## Risks - Low. The change applies only when the shared owner prover returns `gone` for the exact recorded controller and the run was still preparing with no process recorded. The lease's boot id stays the random revocation id, so the old controller can still never renew it. Old rows without a controller stamp keep today's behaviour. - Previous-release story (upgrade-safe-run-state): no persisted shape changes. A run saved mid-preparation by an older release has no `controllerProcess` stamp, cannot be proven gone, and keeps the charged lane (covered by the new old-saved test). Retries an older release already scheduled in the charged lane keep their counters. ## 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
|
bf45700ca3 |
fix(native): make Retry work after a turn start cut off once Codex recorded the turn (#20516)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run sends `turn/start` through its runner. A control-plane restart can kill the runner and Codex while that request is in flight > - #20507 lets the runner resume by itself when Codex's thread history proves the turn was never created. When Codex did record the turn, the turn may have done work, so the runner still fails closed and nothing replays it > - That branch must still end in a Retry that works. The next run on the same conversation is refused at the runner-state fence before it asks for a runner. #20507 let Retry continue after that run by matching the refusal's error text > - Driving that branch end to end with the real runner exposed a second dead end. When the owner's runner dies, its provider-session close cannot prove suspension, so this server process can keep an operator-required cleanup quarantine for the task. Retry was then refused outright ("Send a new message") > - This pull request replaces the text match with a server-authored run-log event, lets Retry take the same verified-stop path as a new message, and adds a real-runner test of the whole branch: the cut-off owner fails closed, the follow-up is refused with structured proof, Retry is admitted, and the fresh Retry run completes > - The benefit is that this restart window cannot strand a task, and the Retry gate no longer depends on error wording ## Linked Issues or Issue Description **What happened?** Follow-up to #20507 (restart-chaos wave `w-20261110T033426-s108`, t03). #20507 left one item open: prove that a person's Retry produces a working run when the restart cut off `turn.start` after Codex recorded the turn. It also matched the fence refusal by its error text. **Expected behavior** After that cut-off, the owner stays failed closed and the turn is never started again by itself. A person's Retry starts a new run that completes. It never fails with `quarantine_owner_unverified` and never returns 202 with no run. **Steps to reproduce** 1. Run `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-cut-turn-start-retry.integration.test.ts`. 2. Without the new event, the test fails: the refused follow-up has no proof that it never launched, so Retry after it would wait on `process_identity_missing`. ## What Changed - `native-session-executor.ts`: when the runner-state fence refuses a run before launch (`prior_owner_*` or `legacy_prior_owner_*`), the executor records the server-authored event `native.prior_owner_fence_refused` with the closed refusal `reason`, then fails the run as before. A failed write leaves the run held (the safe side). - `native-local-process-stop.ts`: `nativeRunNeverLaunched` accepts a failed run that has this event (and still no launch, identity, `adapter.invoke` or runner-sourced event). It no longer reads the error text. - `explicit-native-continuation.ts`: when the task's in-process cleanup quarantine is operator-required, a Retry no longer returns `cleanup_quarantined` at once. Like a new message, it must first pass `verifyStoppedNativeSessionForContinuation`, which releases the quarantine and records `native.stopped_conversation_verified`. A native Retry always starts a fresh conversation (`forceFreshSession`), so it never resumes the interrupted turn. If the proof fails, Retry still returns `cleanup_quarantined`. - `verifyStoppedNativeSessionForContinuation` (`native-session-executor.ts`), three narrow additions: - a run with `native.prior_owner_fence_refused` may stand in for the saved state's owner, like a run refused by the cleanup quarantine (both were refused before launch); - when the session root is gone, it reads the fence's single `identity_indeterminate` quarantined copy of the owner's state (several copies stay ambiguous); - a provider startup attempt in phase `initialization_failed` whose direct child exited and whose process and process group are gone no longer blocks the proof. A same-run restore that fails closed leaves exactly this record. Those PIDs are also re-checked before the quarantine is released. - New real-runnerd integration test `native-cut-turn-start-retry.integration.test.ts` (real runnerd and fake Codex with `--hang-turn-start after`): 1. The owner run's `turn/start` hangs after Codex recorded the turn; the runner and Codex are killed. The owner's provider state keeps the ambiguous start, and the thread history keeps the turn. 2. The same-run resume restores the runner and fails closed with "cannot safely recover an ambiguous Codex turn start". Codex saw exactly one `turn/start`. 3. A follow-up on the same conversation is refused with `prior_owner_*`, never `quarantine_owner_unverified`. It records `native.prior_owner_fence_refused`, launches nothing, and `nativeRunNeverLaunched` is true for it. 4. The owner's operator-required in-process cleanup quarantine is pinned (whether the real close lands there is timing-dependent; about half of the local runs did). 5. Retry of the refused follow-up passes `admitExplicitNativeContinuation` (not a dry run) with no block reason, releases the quarantine and records `native.stopped_conversation_verified`. 6. The Retry run, prepared with a fresh conversation as task Retry does (`forceFreshSession`), uses a different runner-state root and completes with exit code 0. Codex saw one new `thread/start` and one new `turn/start`; the cut-off turn was never started again. - `explicit-native-continuation.test.ts`: the fence rows now insert the event. A new row shows that the refusal text alone keeps the run held. The "worker loss (retry)" case now expects a verified Retry to be admitted like a new message. - `native-recovered-owner-followup.test.ts`: a fixture event moves to a free sequence number, because the refused follow-up now writes its own event. - Docs: `doc/run-log-events.md` (new event) and `packages/paperclip-runner/docs/durable-recovery.md`. ## Verification - Red/green: - Without the event write, the integration test fails (no fence event, so the follow-up is not proven unlaunched). - Without the gate and proof changes, it fails with `expected [ 'cleanup_quarantined' ] to deeply equal []`: Retry is refused while the owner's quarantine is held. - With all changes, it passes. - `vitest run` of `native-recovered-owner-followup`, `explicit-native-continuation` (427 tests), `quarantined-task-retry`, `issue-confirm-run-stopped-routes`, `native-finalization-recovery`, `task-interruption-query`, `execution-projection`, `chat-run-publications`, `closed-native-receipt-accounting`, `agent-live-run-routes` and the new integration test pass. - Server `tsc --noEmit` passes. - Runner code and the pack `contractDigest` do not change. ## Risks - Low-medium risk. The new event is written only on the fence refusal path, which already fails the run. It is local run-log data (no Telemetry or OpenTelemetry). - Retry after an operator-required cleanup quarantine is a behaviour change: it was always refused, and now it is admitted only after the same exact stop proof a new message needs. That proof checks the owner's runner stop record, every recorded provider and startup PID and process group, and the unchanged state fingerprint before it releases the quarantine. - A run refused by a build between #20507 and this change has only the error text. It stays held on `process_identity_missing` until a person sends a new message or the board confirms the stop. #20507 has not shipped in a release yet. - The test models the restart in-process: it kills the runner and Codex and calls the executor again. It does not drive the HTTP Retry route; the route's own behaviour (fresh conversation, admission gate) is covered by existing tests and by `nativeRunNeverLaunched` here. - Open decision, unchanged: a turn Codex recorded before the restart is never replayed or adopted. Continuing is a person's decision. > 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`), via Claude Code with tool use (shell, file edits, 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 Refs #20507 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
c0593b69f1 |
fix(accounting): close a remote-Runner turn completed across a session restore as a lower bound (#20524)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs settle their charge once the final usage receipt is complete. > - #20517 fixed one case for the local Runner. If a restart stops the provider before a turn's final usage report, and the restored Runner then reports that turn completed, the turn now closes as a lower bound instead of waiting 10 minutes. > - Remote Runners deliver usage through a separate producer-owned receiver. That receiver dropped `session.resumed`, so the same turn there still waited for the 10-minute closure timeout. > - This pull request applies the #20517 rule to that receiver. > - The benefit is that remote ACPX and OpenCode runs settle promptly with an honest lower-bound label after a mid-turn restore. ## Linked Issues or Issue Description Refs #20517. **What happened?** After #20517, `session.resumed` reached turn accounting only through the local Runner's fallback recorder. The remote producer receiver (`native-usage-delivery.ts`) and the session-authority filter (`isNativeUsageAccountingEvent`) both dropped it. A remote ACPX or OpenCode turn that was open across a session restore, and then reported terminal without a complete report, left the receipt incomplete until the closure timeout. **Expected behavior** Same rule as the local Runner. Such a turn closes as a lower bound right away. A complete report that arrives after the restore is still used, and a turn that starts after the restore stays pending until its own report is complete. **Steps to reproduce** `native-usage-delivery.integration.test.ts > a turn restored mid-turn on a remote Runner > closes the turn as a lower bound, also after the controller restarts between restore and terminal` fails on master: the receipt stays `accountingReceiptReady: false`. ## What Changed - **Receiver (`createNativeUsageReceiver`).** It now accepts `session.resumed` for turn-reporting providers (ACPX, OpenCode) while a turn is open. It journals the restore as a receipt like any other producer event, so turn accounting marks the open turn restored. A replacement controller rebuilds the same state from that journal. - **Restores never fail delivery.** A restore from another run or source, or one with no open turn, is ignored, not rejected. An unacknowledged lifecycle event would otherwise replay without end. - **Session-authority filter.** `acceptAccountingEvent` now uses a new `isNativeUsageDeliveryEvent`: the usage events plus `session.resumed`. A restore for another run is ignored there too. - **Recovery routing is unchanged.** `acceptRecoveryAccountingEvent` and `onRecoveryAccountingEvent` keep the usage-only `isNativeUsageAccountingEvent`, so a source run's restore never requires a proven recovery source. - **Turn accounting** gets a `hasOpenTurn()` helper. - **Docs:** `doc/SPEC-implementation.md` now covers both Runners. ## Verification - `native-usage-delivery.integration.test.ts`, describe `a turn restored mid-turn on a remote Runner` (real PostgreSQL): | Test | On master | Here | |---|---|---| | `closes the turn as a lower bound, also after the controller restarts between restore and terminal`: the restore is journaled by one receiver and the terminal arrives at a rebuilt one; replaying everything again is idempotent | fails | passes | | `keeps the turn's own final report when it arrives after the restore` (guard) | passes | passes | | `keeps a turn started after the restore pending, and journals nothing for a restore with no open turn` (guard) | passes | passes | - `native-session-executor.test.ts > binds retained warm transport usage to the successor run authority` covers routing through the session-authority filter: - a current-run restore reaches the successor's capture; - a draining-run restore reaches the drained capture; - an unrelated-run restore resolves with no capture; - recovery routing ignores a source restore. It fails when the filter is reverted to `isNativeUsageAccountingEvent`. - Related suites pass: `native-usage-delivery.integration` and `native-usage-accounting` (46 tests), and the usage, accounting, receipt and authority cases in `native-session-executor.test.ts` (224 tests). Server typecheck passes. - **The live remote path is unproven.** No Daytona wave ran because the Daytona Runner image has not been rebuilt yet. The local Runner counterpart (#20517) passed restart-chaos L1, seed 20386, 4/4 (`w-20261110T104332-s20386`). ## Risks - **Upgrade safety.** Usage state saved by current master has no journaled restore and rebuilds as before. A receiver that predates this change replays a journaled restore as an ignored event, which is the behaviour on master. A restore that an older controller already acknowledged is not replayed, so that run waits for the closure timeout as it does today. - **One extra receipt per mid-turn restore**, only for ACPX and OpenCode and only while a turn is open. Nothing is added before `turn.start`: a restore with no open turn returns immediately. - **Codex and other providers are unaffected.** They do not use turn accounting. - No schema or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution 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 - [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
|
37808f2a4e |
fix(accounting): close a local-Runner turn completed across a session restore as a lower bound (#20517)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs record usage receipts, and accounting settles a run's charge once its final receipt is complete. > - ACPX Claude reports provisional zero usage until a turn ends, then one final report. > - A restart can stop the provider after the agent's finish call and before that final report. The restored Runner then resumes the session and reports the same turn completed with no complete report. > - The receipt stayed incomplete, so the accepted run waited for the 10-minute closure timeout and then settled as unpriced. Its spend was missing from company totals in the meantime. > - This pull request closes such a turn as a lower bound, the same way a turn the Runner fails as lost on restore is already closed. > - The benefit is that an accepted run settles right away with its observed usage and an honest lower-bound label. ## Linked Issues or Issue Description Refs #20484 (restart-chaos L1 contention follow-up). **What happened?** After #20484 fixed the database contention, restart-chaos L1 (`--seed 20386 --db-pressure 4`, codex and claude, K1) reached 4/4 tasks done. Each task still failed the T-oracle `company_spend_settled` with `pending_runs=1`. A replay on master with `--keep` (`w-20261110T100747-s20386`) reproduced it, and I read the kept database: - The pending run (`71f71d10`, ACPX Claude) succeeded and had its result accepted. Its usage had `accountingReceiptReady: false`, zero tokens, `costStatus: unpriced`, and no cost event. - Its only turn started before K1, and every usage report in it was provisional (`runDeltaComplete: false`). K1 struck right after the agent's `paperclip_finish` call (source seq 118), before the turn's final report. - On restart, recovery verified the local process stopped and resumed the session (`session.resumed`, seq 119). The restored Runner then reported the same turn `turn.completed` (seq 124) with no usage report. - The other Claude run in that wave lost its turn as `provider_turn_lost_on_restore`, ran a new turn live with full usage, and settled at once. **Expected behavior** An accepted run whose open turn's final report was lost with the provider process settles promptly. It records its observed usage as a lower bound, like a turn lost on restore. A complete report that does arrive after the restore is still used. **Steps to reproduce** `server/src/services/native-runtime/native-usage-accounting.test.ts > closes a turn that completed across a session restore without its final report, as a lower bound` uses the event sequence from the kept database. It fails on master because the snapshot stays incomplete. ## What Changed - `createNativeTurnAccounting` observes `session.resumed` and marks every open turn as restored. A terminal event for a restored turn without a complete report marks it lost. The snapshot then closes it as a lower bound (`accountingUsageLost`), through the existing lost-on-restore path. A new `turn.started` clears the mark. - A lost terminal turn keeps its identity in the snapshot, so the executor's terminal-turn match certifies the final checkpoint as complete. This came from review. - The native session executor includes `session.resumed` when it rebuilds turn state from the run's event log after a controller restart. - `doc/SPEC-implementation.md` documents the case next to the existing lost-on-restore rule. ## Verification - Red on master, green here: `native-usage-accounting.test.ts > closes a turn that completed across a session restore without its final report, as a lower bound`. It also checks that replaying the history twice gives the same snapshot. - Red on the first revision, green after the review fix: `native-session-executor.test.ts > certifies a turn completed across a session restore as a lower bound (history replay: false|true)`. It drives the restored sequence through executor completion, both live and from the event history. - New guards that pass before and after: - `keeps a restored turn's own final report when it arrives after the restore`: a complete report after resume is used, not marked lost. - `keeps an incomplete turn pending when the session resumed before it started`. - The existing `keeps an incomplete provider report pending instead of trusting placeholder zeroes` still passes. - Related suites pass: `native-usage-accounting`, `native-usage-delivery.integration`, `closed-native-receipt-accounting` (107 tests), and the usage, accounting and receipt cases in `native-session-executor.test.ts` (151 tests). Server typecheck passes. - Live, restart-chaos L1, seed 20386, `--db-pressure 4`, codex and claude, K1, head `a6e521f06c`, clean worktree: `w-20261110T104332-s20386` passed 4/4. R1–R6 and the T-oracle passed for every task. `company_spend_settled` reported `pending_runs=0` (summary 85.30823 = cost events 85.30823), with zero serialization failures under about 397k pressure updates. On master the same seed failed every task on `company_spend_settled pending_runs=1` (`w-20261110T100747-s20386`, among others). ## Risks - A restored turn without a complete report now settles at its observed usage, flagged as a lower bound, instead of after 10 minutes as "closure timed out". In both cases the final report never arrived. The money recorded is the same observed usage; only the timing and the provenance label differ. - Scope: local Runner only. The executor fallback-recorder path sees `session.resumed`. The producer-owned delivery path for remote runners (`native-usage-delivery.ts`, `isNativeUsageAccountingEvent`) still drops it, so a remote ACPX/OpenCode turn restored mid-turn still waits for the closure timeout. That is a follow-up. - No schema or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution 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 - [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
|
dc9971bdb8 |
fix(native): keep another server's orphan sweep off a lease its owner took back (#20508)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an executor lease on its `native_run_finalizations` row. #20474 lets the same owner take back its own lease when the lease lapsed only because the host slept. > - That take-back is safe only if every other actor that acts on a lapsed lease records its action under the coordinator row lock first. The plan `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md` lists the actors that do not. Gap 2 is another server's orphan sweep. > - The sweep reads the lease as lapsed. Later it writes the ownership-unverified marker on the run, without the coordinator lock and without a new check of the lease. If the owner wakes and takes its lease back between the read and the write, a healthy run gets the marker. > - The board then shows "automatic recovery is blocked" on a run that works. After the next host sleep, the owner cannot take its lease back, so the turn is interrupted and the user must click Retry. The owner's rule is that a task interrupted by sleep must resume by itself. > - This pull request makes the sweep write its marker under the coordinator row lock, and only while no executor lease on the run is live. > - The benefit is that exactly one side wins: the sweep marks a run whose owner did not come back, and it leaves alone a run whose owner took its lease back. ## Linked Issues or Issue Description Refs #20474 **What happened?** Two servers share one database. Server A's host sleeps, so the executor lease of its native run lapses. Server B's orphan sweep reads the lapsed lease. Server A wakes and takes its lease back (#20474). Server B then writes `native_execution_ownership_unverified` on the run that A still owns. **What did you expect?** Server B writes the marker only if the lease is still lapsed when it writes. One side wins. **Steps to reproduce** `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` holds the owner's take-back open, runs the real `heartbeatService(db).reapOrphanedRuns()` until its write waits on a lock, then commits the take-back. Before this change the run ends with `errorCode: native_execution_ownership_unverified` and the owner's next take-back fails with `native_session_lease_lost`. ## What Changed - `setRunStatusFromLive` (`server/src/services/heartbeat/run-lifecycle.ts`) has a new option `onlyWhileNativeLeaseLapsed`. It locks the coordinator row first. This is the same order that `lockNativeExecutionLeaseForReacquire` uses. The UPDATE then requires that no executor lease on the run is live. - `markNativeOwnershipUnverified` (`server/src/services/heartbeat/recovery.ts`) takes `leaseLapsed`. The orphan sweep in `recoverActiveExecutions` sets it when it read the lease as lapsed or unset. Other callers do not change. They keep the same call shape. - A new regression test, red before the fix: `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts`. It also checks that the sweep still marks a lapsed run whose owner did not come back, and that the late owner stays fenced out. - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: gap 2 is marked closed without the epoch design. ## Verification - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` — red on `origin/master` (the run keeps the marker), green with this change. - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts src/services/heartbeat/recovery.test.ts src/services/native-runtime/native-execution-lease-reacquire.test.ts src/__tests__/heartbeat-status-preserving-telemetry.test.ts` — 129 passed. - `cd server && pnpm exec tsc --noEmit -p tsconfig.json` — clean. - Restart-chaos time-fault wave on this branch: see the wave comment below. ## Risks - Low risk. Only the orphan sweep's marker write changes, and only for a run whose lease it read as lapsed. The sweep now takes the coordinator row lock before the run row. The owner's take-back uses the same order, so the two cannot deadlock. - If the owner took the lease back, the sweep now writes nothing and appends no event. The next sweep sees a live lease and defers, as before. - The other gaps in the plan (workspace finalization, shared-workspace holder detection, board copyback confirmation) do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
0b20ea6fe9 |
fix(accounting): keep budget admission and usage handoff alive under run-row updates (#20484)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Before a run starts provider work, Paperclip reserves its budget and binds a usage recorder. Both write the run's `heartbeat_runs` row. > - Run-log appends, output progress and lifecycle events update that same row while the run prepares and streams. > - #20385 fixed this race for the standalone usage-receipt write only. Budget admission and the recorder handoff still take a repeatable-read snapshot before they lock the run row. > - Each concurrent row update makes that snapshot stale. Four retries run out with SQLSTATE `40001`, and the run ends `adapter_failed` with raw SQL as its error. > - This pull request applies the #20385 pattern (READ COMMITTED under the existing locks) to those writers and to the final dispatch gate. It also gives exhausted retries a bounded cause. > - The benefit is that busy runs start and account normally, and a remaining failure names its cause. ## Linked Issues or Issue Description Refs #20385. Builds on #20483 (bounded failure cause and `withoutSqlStatementText`). **What happened?** The restart-chaos L1 workload with `--db-pressure 4` (harness writers that update each running run row, one row per statement, like a fast stream) failed on master. In one wave, 0 of 4 tasks passed. Every first run failed in `reserveRunBudget` before spawn. In an earlier wave, a run failed in `createRunUsageRecorder`. The stack in both cases is `could not serialize access due to concurrent update` (`40001`) on the run-row `FOR UPDATE`. The board `runError` showed the full `select ... from heartbeat_runs ... for update` statement. The server log line `heartbeat execution failed` dropped the database cause. Realism: the harness writers are synthetic, but the product has the same writers on the same row. `appendHeartbeatRunEvent` increments `next_event_seq`, output progress updates `last_output_*`, and lifecycle events land while a run prepares. On a hosted database the window between snapshot and row lock is wider than locally, so a busy run reaches the same race without the harness. **Expected behavior** Budget admission and the recorder handoff commit while other writers update the run row. Budget invariants stay intact: one reservation per run, a reached hard stop still refuses work, and committed holds count against later admissions. Usage receipts are journaled once. If retries still run out, the run shows a bounded cause without SQL, and the server log records the SQLSTATE. **Steps to reproduce** Run `server/src/services/run-accounting-contention.test.ts` against the parent commit. The deterministic cases commit a real `appendHeartbeatRunEvent` after the writer's transaction checks the company and before it locks the run. Six of the eight cases fail with `40001`. ## What Changed - `reserveRunBudget` runs at READ COMMITTED. All mutable reads follow the shared admission fences, the agent `FOR SHARE` lock and the run-row `FOR UPDATE`. Exposure stays one SQL statement (hold-to-ledger atomic). The `budget_reservations_run_idx` unique index and the run lock still prevent a second hold. - The final dispatch gate's accounting phase (`dispatchResolvedInteractionIfCurrent` through `withIssueThenRunLocks` in `run-dispatch/adapters/postgres.ts`) runs at READ COMMITTED. Its ownership phase already read the same staleness facts at READ COMMITTED under the same issue and run locks. Retry promotion and stale-run cancellation keep their repeatable-read snapshot. The first L1 proof wave found this writer exhausting `40001` once the other three were fixed. - `createRunUsageRecorder` runs its receipt-source handoff and its capture-failure fence at READ COMMITTED. Both lock the run row first, the same as `persistUsageReceipt` after #20385. - `withOperationAccountingTransaction` raises `AccountingContentionError` when retries run out. It has a bounded message, `code` keeps the SQLSTATE (`40001`/`40P01`), `cause` keeps the database error, and a warning logs both. - `failRun` shows that bounded message. The code stays `adapter_failed`, so chat and recovery retry paths still apply. #20483's `failureCause` records `40001`. All other SQL text handling comes from #20483 (`withoutSqlStatementText`), unchanged here. - `doc/DATABASE.md` documents the isolation choice and the failure contract. ### Inventory of REPEATABLE READ / SERIALIZABLE transactions that write run-scoped rows | Writer | Status | Notes | | --- | --- | --- | | `persistUsageReceipt` (`usage-receipts.ts`) | Fixed in #20385 | READ COMMITTED under the run lock. | | `reserveRunBudget` (`budget-reservations.ts`) | Fixed here | Seen in waves (`budget-reservations.ts:90`). | | `createRunUsageRecorder` handoff (`usage-receipts.ts`) | Fixed here | Seen in waves (`usage-receipts.ts:572`). | | `createRunUsageRecorder` capture-failure fence (`persistFailure`) | Fixed here | Same shape: run lock first, runs mid-stream. | | Native finalizer `claimCoordinator` (`native-run-finalizer.ts`) | Safe | A default READ COMMITTED transaction with `FOR UPDATE` on the coordinator row. It cannot raise `40001`. The wave errors there were `native_finalization_missing`, a symptom of the failed reservation (no coordinator existed yet). | | Final dispatch gate, accounting phase (`dispatchResolvedInteractionIfCurrent`) | Fixed here | Seen in proof wave `w-20261110T075857-s20386` (runs `140ddb10`, `c028c6c4`, `fea0ee5a`). Its ownership phase already used READ COMMITTED under the same locks with the same staleness reads. | | Retry promotion and stale-run cancellation (`promoteOrCancelDueRetry`, `cancelStaleQueuedRun`, same lock helper) | Follow-up | Kept at repeatable read: `loadGateFacts` combines wakes, comments, dependencies, tree holds and repair state in one decision. They act on queued or scheduled runs, not streaming ones. Not seen failing in waves. If retries run out, they now fail with the bounded cause. | | Run settlement `accountRunCost` (`run-cost-accounting.ts`) | Follow-up | Seen with `40001` in a wave log. Leaves the run pending; `reconcileRunCosts` retries it durably. Multi-aggregate (coordinator, receipts, images, closure). | | Tool cost events (`paperclip-runner-tool-authority.ts`, `costs.ts`) | Safe | Insert `cost_events` only; the run row is referenced, not locked or updated. | | Budget enforcement and evaluation (`budgets.ts`), billing reconciliation, accounting-integrity repair | Safe | Lock policy, invoice or company rows, not a streaming run row. Run reads are plain selects. | | Browser use, decisions, prompt-injection, fast responses | Safe | Write their own invocation/reservation rows, not `heartbeat_runs`. | | Read-only RR snapshots (`withAccountingReadSnapshot`, idle-sleep, tool gateway, customer success, `routes/decisions.ts`) | Safe | Read only, or no run-scoped writes. | ## Verification - Red on the parent commit, green on this branch: `server/src/services/run-accounting-contention.test.ts` (real embedded PostgreSQL). - `reserveRunBudget > reserves once when run events commit between its company check and its run lock` (red: `40001`) - `reserveRunBudget > reserves every run while back-to-back writers update the run row` (red: `40001`) - `reserveRunBudget > counts every committed hold, so successive admissions never exceed the budget` (red) - `reserveRunBudget > still refuses a run once the hard stop is reached` (red: `40001` instead of `409`) - `createRunUsageRecorder > binds its receipt source when run events commit between its company check and its run lock` (red: `40001`; green asserts the spooled receipt is journaled exactly once) - `createRunUsageRecorder > binds while back-to-back writers update the run row` (red: `40001`) - `reserveRunBudget > never creates a second reservation for one run when two dispatches race` (invariant; passes before and after) - `final dispatch gate > admits and hands off a reserved run when run events commit between its company check and its run lock` (red: `AccountingContentionError` after 4 × `40001`) - `createRunUsageRecorder > persists its capture-failure fence once when run events commit between its company check and its run lock` - `final dispatch gate > admits while back-to-back writers update the run row` (red: `40001`) - `exhausted retries > fail with a bounded, SQL-free cause that keeps the SQLSTATE` (new contract) - `run-completion.test.ts > saves a bounded cause and keeps the retryable generic code when accounting retries are exhausted` (saved code, message and `failureCause`, and log fields, through `failRun`) - Related suites pass: `usage-receipts-read`, `admission-independent-of-cleanup`, `budget-live-attempts`, `idle-accounting-work`, `budget-enforcement-scoping`, `closed-native-receipt-accounting`, `budget-evaluation-work`, `run-dispatch/adapters/postgres`, `heartbeat/run-completion` (all 126 run-completion cases pass; its `afterAll` embedded-PostgreSQL teardown exceeded 30 s on this loaded host, the known local flake). - `pnpm --filter @paperclipai/server typecheck` passes. - Latency of `reserveRunBudget` on the normal path (no contention, embedded PostgreSQL, 350 measured reservations per run, 3 runs each): before p50 14.7 / 15.0 / 18.6 ms, p95 25.6 / 31.4 / 39.4 ms; after p50 14.2 / 14.0 / 16.6 ms, p95 29.7 / 24.0 / 29.3 ms. No added latency: the statements are the same, only the isolation level changes. Under contention it removes the retry backoff. - Live restart-chaos L1 waves (`--seed 20386 --db-pressure 4`, codex and claude, K1): - Master before this change: `w-20261110T050818-s20386`, 0/4. Every first run failed in `reserveRunBudget` with `40001`. - Head `90530789b0` (first three writers fixed): `w-20261110T075857-s20386`, 2/4. Both codex tasks passed. Both claude failures (t02, t03) trace to the dispatch gate, now fixed. Those runs already showed the bounded cause, with no SQL on the board. The failure was logged with `sqlState` 40001. No `reserveRunBudget` or recorder `40001` remained. - Head `d5ed21926f` (dispatch gate fixed): `w-20261110T090531-s20386`. All 4 tasks reached done with no retry and R1–R6 passed. There were no failed runs and zero `40001` in the server log, under 310k pressure updates. Each task still failed the T-oracle `company_spend_settled` with `pending_runs=1`. That pending run is t02's continuation run `601ae80a`, cancelled before provider work as `issue_terminal_status`, whose accounting debt was not yet settled when graded about 15 s later. It is not a contention failure. Caveat: I edited the worktree during this wave (`worktree_changed_mid_wave`), so a clean wave on the final head follows. - Final head `9f1c480717`, clean worktree: `w-20261110T094417-s20386`. All 4 tasks reached done with no retry, and R1–R6 passed for each. There were no failed runs and zero serialization failures in the server log, under about 523k pressure updates. Each task still fails only the T-oracle `company_spend_settled` (`pending_runs=1`). It is the same cancelled pre-provider continuation run (`issue_terminal_status`), whose accounting debt was still unsettled at grading. That is a separate settlement-timing follow-up, not database contention. Master was 0/4 on the same seed. ## Risks - READ COMMITTED gives each statement its own snapshot. Admission reads policy, scope and exposure in separate statements. The shared company/project fences and the agent share lock exclude concurrent pause and policy edits, and exposure stays one statement, so each read is at least as fresh as the old snapshot. - Concurrent admissions of different runs stay best-effort, as `doc/SPEC-implementation.md` already states ("concurrent starts can exceed the same observed balance"). This change does not widen that window: each statement snapshot is newer than the old transaction snapshot. It does not add a company mutex. - A run that fails after exhausted retries now records `accounting_contention` instead of `adapter_failed`. Retry rules that key on `adapter_failed` spawn-like messages did not match this failure before, so no automatic retry changes. - No schema, migration or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution 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 - [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
|
1402e92d1f |
fix(native): take back an executor lease that lapsed only because the host slept (#20474)
## 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 lease lasts 20 minutes, and the owner renews it every 5 minutes. > - When a laptop sleeps for longer than the lease, the wall clock and the database clock move past `lease_expires_at`. The server's monotonic timers barely move. > - After the wake, the next renewal finds its own lease expired. It interrupts a healthy turn with `native_session_lease_lost`, and the user must click Retry. Nobody else took the lease. > - Several background consumers act on an expired lease without changing its owner. So expiry alone cannot simply be ignored for the owner. > - This pull request makes takeover explicit. The same owner can take back its own lapsed lease only when no other actor has recorded anything on the row or the run. > - The benefit is that a run interrupted only by host sleep continues with no Retry. A real takeover still fences the late owner out. ## Linked Issues or Issue Description **What happened?** The restart-chaos time fault TM2 stops the server and runner tree and ages every database time by one hour, as a stand-in for a laptop that slept. After the thaw, the next 5-minute renewal of the native execution lease found the lease expired. It interrupted the turn (`turn.interrupted` at 300 s, `native_session_lease_lost`). The failure settlement was then refused with the same code, and the UI showed "Couldn't finish this attempt" with Retry. The owner, attempt, boot and pid were unchanged. Evidence: wave `w-20261110T051535-s304` t01 (TM2, 1 h) failed R3. t02 in the same wave (TM1, a 113 s server-only freeze with no clock jump) passed, so the failure follows the clock jump and not the pause. **Is this a harness artifact?** No. TM2 ages the stored rows, and a real sleep moves the clock forward. Both produce the same comparison: a stored `lease_expires_at` is earlier than `clock_timestamp()` after the wake. The server's renewal timer and local deadline use the monotonic clock. On macOS and Linux that clock does not count suspended time, so they do not notice the sleep in either case. The first database check after the wake is the renewal predicate `lease_expires_at > clock_timestamp()`, and it fails in both cases. **Expected behavior** A run whose lease lapsed only because time jumped, and that no other actor claimed, keeps running and finishes without Retry. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex --workloads B5 --faults TM2 --time-age-hours 1 --concurrency 1 --tasks 2 --seed 304` with the harness from #20458. Refs #20458, #20500 ## What Changed - `native-execution-lease.ts`: add `nativeExecutionLeaseHeldCondition` and `lockNativeExecutionLeaseForReacquire`. A lease is held when it is live. For a local run, it is also held when it lapsed and is untouched. "Untouched" means all of these are true: - the claimed generation matches; - the phase is `observed` and no result is accepted; - `recovery_state`, `recovery_request_id`, `failure_code` and `next_attempt_at` are null; - `controller_host_hash` is not the board's attested-stopped stamp; - the run is still `running`, without the ownership-unverified error code and without a `nativeStartupCleanup` marker. - The caller's WHERE already matches the owner token, attempt and controller boot/pid/start. - The lock helper locks the coordinator row. If the lease lapsed, it also takes a share lock on the run row. The predicate then reads the latest committed run, and an in-flight run write cannot slip past it. - Renewal (`startNativeSessionExecutionLeaseRenewal` / `renewNativeSessionExecutionLease`) takes back a lapsed lease when the executor passes its claimed generation. Remote targets never do. A deadlock (40P01) on that rare path retries at the next tick, as a statement timeout already does. - `native-session-executor.ts`: the executor passes its generation to renewal. Failure settlement and the final release use the same lock and condition. After a result is accepted, a lapsed lease can only be relinquished (set to null), never renewed, because workspace finalization may act on it. - New `native-execution-lease-reacquire.test.ts` (embedded Postgres, 24 cases). - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: step 2, which adds explicit claim tokens for the consumers this PR does not yet move. Consumer inventory (readers of `native_run_finalizations.lease_expires_at` that can act on a lapsed lease): - Restart recovery and takeover (`claimNativeRestartRecoveries`): claims first. It writes `recovery_state`, `lease_owner` or `controller_*` under the coordinator lock. - Finalizer and cleanup claims: claim first (`lease_owner`). - Failed-run owner release, environment settlement, observed-workspace preservation, retained-session cleanup: require a terminal run status, `committed`, or a recovery state. The rule refuses all of these. - Finalization reconciler, live recovery: require `retryable_failure`, a field the rule checks. - Board controller-stopped confirmation: writes `controller_host_hash`, which the rule checks. - Startup cleanup promotion: writes the run's `nativeStartupCleanup` marker, which the rule checks. It also applies only to runner-task leases. - Another server's orphan sweep: writes the run's ownership-unverified code. The rule checks it when the marker lands first. The reverse order is deferred (see Risks). - Workspace finalization copyback: acts on a lapsed post-result lease without a coordinator write. Post-result leases are therefore never renewed (deferred to step 2). - Accepted-result and token lease settlement, checkpoints, cancellation delivery, admission and the other readers: act on live leases only, or are read-only. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-execution-lease-reacquire.test.ts` passes (24/24) on this branch. With the source files from `origin/master`, 5 fail, including "lets the same owner take back a lease that lapsed untouched" and "renews from the shared timer after the sleep instead of interrupting the turn". - (a) The owner takes back its lapsed lease when no other actor claimed it. Also covered: the shared 5-minute timer, rows saved by the previous release, and the post-result relinquish. - (b) 14 cases where a consumer claimed first fence the owner out and leave the row unchanged. A release over a recorded recovery claim is refused. A different boot (a deploy mid-run), a different generation, a missing generation and a remote target are refused. - (c) Races have exactly one winner: a claimant and the owner lock the coordinator in each order, an in-flight run status write races the owner, and 8 simultaneous starts each have one winner. - Existing suites pass: `native-execution-lease.test.ts`, `native-session-executor.test.ts`, `native-runner-restart-recovery`, `native-runner-restart-lease-renewal`, `native-runner-restart-preparation` and `native-runner-restart-checkpoint-cost` integration tests. - Live proof: `B5 × TM2 --time-age-hours 1 --tasks 2 --seed 304` with `--server-from` this branch. Wave `w-20261110T083444-s304` passed 2/2, with no Retry and no `native_session_lease_lost`. - Both runs ran past the 300 s renewal after the one-hour aging, at the exact point where `origin/master` fails. t01 ran 310 s and t02 ran 319 s. Each was a single run that succeeded. - The harness used the aging exclusion from #20500 (merged): process start identity is not aged. Without that exclusion, TM2 also moves `controller_process_started_at`. The live owner then fails its identity check, which a real sleep never causes. An earlier wave on the unpatched harness, `w-20261110T072106-s304`, failed 0/2 for that reason only. ## Risks - Previous release: no persisted shape changes and no migration. Rows saved by the previous release have the same columns, and a null `controller_host_hash` is accepted. A deploy that lands mid-run cannot take back the old server's lease, because the boot and pid differ. - On the normal path, a live lease takes the same single coordinator lock as before. The run share lock is taken only after a lapse, and nothing is added before `turn.start`. - Deferred to step 2 (see the plan): - The orphan sweep of another server process can read the row before the owner takes the lease back and write after. That run would carry an ownership-unverified error while it is live. - Shared-workspace holder detection reads a silent run with lapsed leases as gone. - Post-result leases are not renewed. - Remote leases are unchanged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used in Claude Code with tool use (shell, file editing, sub-agent code audit). ## 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 - [ ] 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
|
ca472d543a |
fix(runner): settle a saved governed wait whose shutdown drain left sequence gaps (#20494)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native agent run can ask the board a question and park on that saved (governed) wait > - After the question is saved, the runner interrupts the provider and drains its usage and terminal for a bounded time > - The drain keeps only usage, the terminal and cancellation facts of the parked turn. It discards every other numbered provider event > - If the control plane restarts before the drain settles, recovery replays the saved journal. That replay required strictly contiguous `sourceSeq`, so the discarded events made a valid saved wait look like the wrong conversation > - This pull request accepts a gap after the saved trigger only when every event after the gap is a drain-retained fact of the same turn. Identity checks do not change > - The benefit is that the asking run settles its saved wait after a restart instead of recording a failure ## Linked Issues or Issue Description **What happened?** Restart-chaos wave `w-20261110T063253-s208`, tasks t02 (Codex Q1, PV2: provider terminal dropped) and t03 (Codex Q1, PV1: provider terminal held), each with K1 (`kill -9` of the control plane about 3-4 s after the fault). The question, the board answer and the final brief were all correct. But the asking run was recorded `failed: native_session_interrupted`, and the grader failed `T:asking_run_settled`. **Cause** The run log of the asking run shows `failureDetail.message: native_governed_wait_replay_mismatch`. It is thrown at `packages/paperclip-runner/src/native-session-runtime.ts:2344` (master). The persisted runner events of the asking run have `sourceSeq` 33..40 and then 43 (t02), and 41, 42 and then 45 (t03). The two missing numbers are events that the accounting drain (`consumeTurn` → `settleDurableResult`, around line 1206 on master) read and discarded, because they are not usage, a terminal or a cancellation fact. Runs without a restart have the same gap (for example `w-20261110T032350-s201/t01`: 34, then 37). Those runs settle without replay, so the gap was not seen before. With a held or dropped terminal and a kill before the shutdown settlement receipt, recovery must replay. Its strict contiguity check then fails on a gap that the runner made itself. **Expected behavior** The question was saved before the provider was interrupted. Recovery must settle that same saved wait, as #20336 does after verified shutdown. Recovery must not start a new model turn. **Related** Refs #20336 (settle saved handoffs after verified shutdown). Not related in cause or code: #20477, #20482, #20483. ## What Changed - `isGovernedDrainRetainedEvent` (new, in `native-session-runtime.ts`) names the facts that the governed-wait drain persists: usage, the turn terminal, the interrupt acknowledgement, cancelled or expired runtime requests, and a failed or cancelled `run.terminal`, all for the parked turn. The drain now uses it in place of the same inline condition. Drain behavior does not change. - Governed-wait recovery replay accepts a `sourceSeq` gap only when: - the trigger is the first replayed event (a gap before it still fails), - sequence numbers strictly increase, - every event after the first gap is a drain-retained fact of the trigger's turn. - The run, normalized session and source instance identity checks do not change. Any mismatch still fails with `native_governed_wait_replay_mismatch`. - `doc/execution-semantics.md`: the governed-wait text said that shutdown keeps a contiguous journal. It now describes the gaps and the replay rule. Note for the sibling Claude/ACPX `turn.cancel` work: this change touches `settleDurableResult` only in the drain filter line (same condition, shared helper). It does not change cancel, the drain timeout or `settleCancelledTurn`. ## Verification - Red/green: `packages/paperclip-runner/src/native-session-runtime.test.ts` - `settles a saved governed wait whose drain left sequence gaps before a restart (terminal-after-restart)` - `settles a saved governed wait whose drain left sequence gaps before a restart (late-terminal-saved)` The first run uses the real drain. It discards two numbered events, so the history is trigger, acknowledgement, then usage with a gap. The stream then ends without a terminal, as with PV1/PV2 plus K1. On master, recovery rejects with `native_governed_wait_replay_mismatch` (the live error). With this change, recovery settles the saved wait as yielded. It does not call `resolveGovernedWait` again. It starts no second turn. The terminal is appended once. When the terminal arrived late and is already saved, recovery does not cancel the provider again. A further recovery of the settled checkpoint does not submit or settle again. - Fail closed: `keeps rejecting an unproven governed-wait replay with a sequence gap: …` covers a gap before the trigger, a foreign run, a foreign session, a foreign source instance, usage of another turn, a non-accounting event after a gap, and an accounting event followed by a non-accounting event. - `cd packages/paperclip-runner && npx vitest run src/native-session-runtime.test.ts`: 261 passed. `npx tsc --noEmit -p .`: clean. - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t governed`: 12 passed. - `pnpm verify` returned exit 2 (test file outside resolved graphs). Required CI supplies the broad coverage. - Live restart-chaos wave: see the comment below with the wave id and result. ## Risks - Low. The rule loosens only the sequence check, and only after the saved trigger, and only for facts that the drain itself retains and that carry no work authority. Identity checks stay strict. A non-accounting event after a gap still fails closed. - No change to the normal start path or turn startup (fast-turn rule). - No schema, protocol or env change. The pack `contractDigest` and provider `commandDigest` do not change. No Daytona image rebuild is necessary. ## Model Used - Anthropic Claude, model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
DottaandClaude
|
1e13c58a4e |
fix(heartbeat): name the cause of generic adapter failures and keep SQL text off the board (#20483)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Heartbeat run settlement records why a run failed, and the board shows that record. > - Runs that fail without a named code settle as `adapter_failed` (or `setup_failed`) with no cause. > - A user, an operator, and the restart chaos harness cannot tell what went wrong from such a run. > - Some run errors also show raw SQL statement text to users. > - This pull request records a bounded `failureCause` code for generic failures, shows it on the board, and keeps SQL text out of run errors. > - The benefit is that a generic failure now names its cause, for example "adapter failed (40001)", without leaking messages, SQL, or secrets. ## Linked Issues or Issue Description Refs #20385 (the nested-cause extraction this change reuses). Related: #20421 (restart chaos universal checks U1-U4). **What happened?** The restart chaos audit of 79 saved waves found 30 failed runs with `errorCode: "adapter_failed"` and a null or generic error. Every U3 ("diagnosable") warning was `adapter_failed`. Nobody could tell what failed. Some board run errors also showed raw SQL statement text from wrapped database errors (Drizzle `Failed query: ... params: ...`). **Expected behavior** A run that fails with a generic code names a short, safe cause. The board shows the cause next to the stop reason. Run error text never shows SQL statements or their parameters. **Steps to reproduce** Make an adapter throw a wrapped database error (an `Error` whose nested `cause` has `code: "40001"` and whose message has a Drizzle `Failed query:` statement). Before this change, the run settles as `adapter_failed` with no cause, and `heartbeat_runs.error` contains the SQL statement and parameters. The new tests in `server/src/services/heartbeat/run-completion.test.ts` reproduce this. ## What Changed - Runner: move the #20385 event-commit cause extraction into `safeErrorCauseCode` (`packages/paperclip-runner/src/control-plane/safe-error-cause.ts`) and export it. The Runner's own diagnostics use the same function, unchanged in behavior. - Shared: add the `failureCause` contract (`packages/shared/src/run-failure-cause.ts`): one `[A-Za-z0-9_.:-]` token of at most 80 characters, `normalizeRunFailureCause`, `readRunFailureCause`, and `withoutSqlStatementText`. - Server: add `server/src/services/heartbeat/run-failure-cause.ts`. Settlement records `resultJson.failureCause` only when the code is generic (`adapter_failed`, `setup_failed`). `errorCode` stays the same. - Thrown errors (`failRun`, `failRunSetup`): `provider_http_<status>`, else `provider_<category>`, else the first safe code in the error and up to three nested causes (`40001`, `ECONNRESET`), else the error class name, else `unknown_error`. - Adapter results without a code (`completeRun`): the error family, `signal_<name>`, `process_cancellation_failed`, `exit_code_<n>`, `adapter_reported_error`, or `unknown_error`. - The failure log lines (`heartbeat execution failed`, `heartbeat execution setup failed`, and a new `heartbeat run failed` warning for generic adapter results) include `failureCause` and `errorCode`. - Server: replace SQL statement text in run error text with "Database query failed (<cause>)" at all three settlement paths, including the cancelled branch. The execution blocker's `runError`, the attention inbox (failed-run excerpt and metadata), and continuation summaries apply the same replacement on read, so older rows also stop showing SQL there. - Server: Retry admission and failed-preparation replay accept only a closed set of `resultJson` keys. `failureCause` is added to each of those lists (`task-retry.ts`, `chat-channels/failed-run-retry.ts`, `cancelled-native-startup.ts`, `fresh-workspace-continuation.ts`). It is a diagnostic code with no execution or replay authority, so a failed-preparation run stays retryable. - Server: `runsForIssue` returns a validated top-level `failureCause`. It is null for older rows, non-string JSON values, and malformed values. - UI: `RunForIssue.failureCause`; `IssueRunLedger` shows "adapter failed (40001)". Older runs keep "adapter failed". The Storybook run-ledger fixture has a failed run with a cause. - Restart chaos harness: run samples keep `failureCause`. U3 treats a generic code with a specific `failureCause` as diagnosable. A bare `Error`, `unknown_error`, or the no-code fallback `adapter_reported_error` still counts as vague. U3 stays a warning. - Docs: `doc/run-log-events.md` and the harness README describe the field and the SQL replacement. ## Verification - Red before the fix: the four new `run-completion.test.ts` cases failed against `origin/master`'s `run-completion.ts` (no `failureCause`, SQL text in `error`). - Green: - `server/src/services/heartbeat/run-completion.test.ts`: all tests passed (real embedded PostgreSQL). New cases cover a nested `40001` three levels deep for execution and setup failures, SQL and a secret parameter absent from `error` and `resultJson`, the bound (an 81-character code and a SQL "code" fall back to the class name), `exit_code_137` for a code-less adapter result, a cancelled result with SQL text, no cause for a named code, and the log line. On this loaded host the suite's `afterAll` database teardown hit its 30 s hook limit after every test passed (the known local teardown limit, also seen in #20385). CI runs the suite. - `server/src/services/heartbeat/run-failure-cause.test.ts` (4), `server/src/__tests__/activity-service.test.ts` (recorded, legacy, malformed, overlong, boolean, and number cause projections), `server/src/__tests__/issue-continuation-summary.test.ts` (legacy SQL row). - `packages/shared/src/run-failure-cause.test.ts` (6), `ui/src/components/IssueRunLedger.test.tsx` (21), restart chaos `universal.test.ts` and `observer.test.ts`. - `packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts`: 207 passed (the stall-diagnostic cases still read `40001` and `ECONNRESET`). - Typecheck: shared, runner, server (all layers), UI, restart chaos harness. `pnpm check:token-gates`: all gates clean. - `pnpm verify` returned exit 2 (global doc input changed); required CI supplies broad coverage. - CI on `6295f8d` found that the new key made failed-preparation runs ineligible for Retry (`chat-channels.integration.test.ts` saved board-message Retry got 409; `workspace-recovery-preparation-dispatch.test.ts` dispatched 0 Retries). Both reproduced locally and pass after the allowlist fix, with the other 459 Retry cases in `chat-channels.integration.test.ts` and all 25 in `workspace-recovery-preparation-dispatch.test.ts`. - No chaos waves were run. Storybook was not opened in a browser; the fixture typechecks and the label is covered by the UI test. ## Risks - Low risk. `errorCode` values do not change, so retry, recovery, and comparison logic is unaffected. - No schema change and no migration. `failureCause` is a new optional `resultJson` key. Rows saved by the previous release have no key and render as before; a new row read by an old server is ignored. - The SQL replacement only acts on a Drizzle `Failed query:` marker, or on `$n` placeholder text with a statement shape. Ordinary error text does not change. When it acts, it also drops the text after the statement. - Code that checks `resultJson` against a closed key list must allow `failureCause`. I found and updated the four lists on the Retry path; a list I missed would refuse a Retry for a run with a generic failure. - This is the run log / instance database path. It adds no Telemetry event. - The server failure log line still logs the raw error message (unchanged behavior); only the board-facing text is changed. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file editing) and code execution for 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 - [ ] 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> |
||
|
Devin FoleyandPaperclip
|
53c9be8d41 |
Reuse identical captured workspace archives (#20476)
Reuse the current committed snapshot when a fresh capture matches its manifest and plaintext archive hash and size. Preserve the original encryption binding and verify each new run receipt for recovery, concurrent replay and storage request statistics. Verified with 338 focused save and recovery tests, server typecheck and build, all final-head CI checks, and Greptile 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Nicky Leach
|
8809f44b5e | Connect Codex and Claude login to agent computers (#20417) | ||
|
Devin FoleyandPaperclip
|
549ca46dae |
fix(runner): retain Claude GitHub broker settings through the Rust sidecar (#20463)
## Thinking Path > - Paperclip lets people manage AI agents for work. > - Shared agents use the instructing person's GitHub identity for managed commands. > - Native Claude starts through the Rust daemon and a TypeScript ACPX sidecar. > - The Rust launch filter kept the command launchers but removed their broker transport. > - Shell commands then reported `GitHub capability_missing`, although connector tools worked. > - This pull request retains that transport for managed Claude launches. > - Each command can acquire its current identity through the existing broker checks. ## Linked Issues or Issue Description **What happened?** Native Claude could use GitHub connector tools but could not authenticate `git` or `gh`. The Rust daemon removed the broker URL and session/bridge capabilities before starting the ACPX sidecar. Reconnecting the account did not repair this process environment. **Expected behavior** Managed shell commands use the current authorized GitHub identity. The provider receives broker capabilities, while raw GitHub tokens remain confined to individual commands. **Steps to reproduce** 1. Start native Claude through the Rust Runner with managed GitHub launchers. 2. Run `gh api user` and a private repository ref lookup through Bash. 3. Observe `capability_missing` and no request to the broker. **Paperclip version or commit** Reproduced from source at `2c2186706ef4bb352b662f9d8faebefe47fb52ac`. The repair is based on `a5e01634f69f18dfe10a0bf709df02d9e6a2135b`. **Deployment mode** Native Claude through ACPX. Local acceptance used macOS arm64 and Node 25.6.1. The reported failure also affected a Linux sandbox. Related: #18212 preserved launcher paths. #18850 repaired the TypeScript filter. This change repairs the preceding Rust filter. The duplicate search found no open repair for this boundary. ROADMAP.md already lists managed personal GitHub identities. ## What Changed - Retain the broker URL and both broker capabilities when Claude has a nonempty managed launcher directory. - Keep raw GitHub tokens, unrelated secrets, empty launcher configurations, and sibling ACPX providers outside that exception. - Exercise the regression through the public sidecar initialize protocol in the existing Rust integration suite. Document the restart requirement. The Rust sidecar transport owns this filter. Credential resolution, company access, approval, and run identity policies keep their existing owners. No schema or API changes are needed. ## Verification - Red/green: the subprocess regression failed on the missing `PAPERCLIP_GITHUB_BROKER_URL` before the fix, then passed with all three broker settings and the negative controls. - Passed before moving the regression: 615 Rust core tests, with 3 existing ignored tests. After moving it into the integration suite, all 20 sidecar integration tests and the 9 remaining sidecar unit controls pass. The moved regression was also run against the unfixed source and failed on the missing broker URL. - Passed: 85 TypeScript environment, launcher, sandbox, and configured-environment controls. These include operation identity changes, revocation, recovery, and exclusion of capabilities from persisted environment data. - Passed: Runner Rust typecheck and format check, plus the Runner build through `node scripts/agent-preflight.mjs --skip-rust-debug`. - Live before/after: real `claude-opus-5-5` through the actual Rust daemon and built ACPX sidecar reproduced the failure with the old binary. Neither shell command reached the broker. With the fixed binary, `gh` identified the connected account and `git` read private repository refs. A second turn retained a workspace file and authenticated again. After the controlled broker denied access, the next command failed without reusing the prior token. No GitHub writes were made. - Live release cutover: saved a session with the unfixed daemon, suspended it, and restored it with the repaired daemon on `aa9f5b30614c011ebc3fe4509e11173b766c1fea`. The same session and workspace marker survived. `gh` authenticated after restoration, then failed when the controlled broker revoked access. - Live acceptance used an Anthropic API credential and a loopback broker with an authorized operator GitHub credential. This isolates the repaired transport; it does not qualify the hosted subscription credential or production grant resolver. - The first PR review gate did not recognize the in-source Rust test. The regression now lives in the existing integration test file, and the same test-coverage gate passes locally. - `pnpm verify` returned its documented exit 2 and required broad CI coverage. The final head passed the full CI typecheck, build, and test lanes. - Greptile reviewed `64cca69b76073cc3185b28334465dee48fbadb29` at 5/5 with no actionable findings or open review threads. Its check passed on that head. The same head is mergeable with no conflicts. - Final head `64cca69b76073cc3185b28334465dee48fbadb29` passed [full PR CI](https://github.com/paperclipai/paperclip/actions/runs/42761401396), [Windows Rust](https://github.com/paperclipai/paperclip/actions/runs/42761401117), and [isolated Runner compilation](https://github.com/paperclipai/paperclip/actions/runs/42761400973). Typecheck, build, all test shards, Runner checks, browser checks, and aggregate verification passed. No checks are pending or failed; five conditional jobs were skipped. ## Risks - The Claude sidecar now receives the intended broker capabilities. The existing broker must continue to enforce active-run identity and current connection access. Raw GitHub tokens remain excluded from the provider launch environment. - Previous-release checkpoint and profile formats carry forward unchanged. A running provider must retire and restart normally to inherit the repaired environment. No profile reset or data migration is required. - Hosted recovery requires delivery of the updated Runner binary. This PR does not deploy or retry existing tasks. Live acceptance has not exercised Linux sandbox confinement or a GitHub write. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with xhigh reasoning. Used repository inspection, code editing, and shell execution. The context window size is not exposed. Anthropic `claude-opus-5-5` was exercised as the acceptance-test provider. ## 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
|
120017e39d |
fix(workspaces): read saved workspace files in place instead of extracting the whole archive (#20442)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a cloud task finishes, its workspace is saved as an encrypted
snapshot and its sandbox is released. The board's Files view then reads
that saved copy
> - Saved Files access downloaded, decrypted and extracted the whole
archive for every request, and refused archives over 512 MiB with
`too_large`
> - A finished task in a Paperclip-sized repository saves a snapshot of
about 730 MiB, so its files could not be listed or opened at all
> - The archive format already records each file's size and SHA-256, and
the snapshot records the manifest digest, so single files can be read
and verified without the rest of the archive
> - This pull request reads the saved archive in place, one verified
file at a time
> - The benefit is that users can open any file of a large finished
workspace quickly, with bounded memory and no whole-archive transfer
## Linked Issues or Issue Description
**What happened?**
In restart-chaos waves on Daytona with a real Paperclip clone, the
committed workspace snapshots were 763,944,535 bytes. After the sandbox
was released, every saved Files read returned 422 `too_large` (506 reads
in total), because `createSavedWorkspaceLoader` refuses archives over
`maxArchiveBytes` (512 MiB). The board Files view uses the same loader,
so it fails the same way.
**Expected behavior**
A user can list and open any single file of a saved workspace of
Paperclip-repository size without the server extracting or buffering the
whole archive. Memory and time stay bounded per request, and integrity
is still verified.
**Steps to reproduce**
1. Run a cloud task whose workspace is a large repository, so the final
snapshot exceeds 512 MiB.
2. Let the task finish and its sandbox be released.
3. Open the task's Files view, or call `GET
/api/issues/:id/file-resources/content?workspace=execution&path=…`: it
returns 422 `too_large`.
**Paperclip version or commit**
`master` at
|
||
|
DottaandPaperclip
|
0934b834c4 |
perf(workspaces): release retained cohort parent scratch early (#20447)
## Thinking Path > - Paperclip manages agent work and preserves its workspace files. > - Online migration captures local files and publishes remote workspace authority. > - Each capture verifies encrypted readback, restores the archive and compares the current source. > - A cohort kept every completed parent payload until the whole cohort published. > - A larger cohort could retain many complete copies while later members still worked. > - This pull request releases only finished parent payloads before a capture worker starts another member. > - Nested-child proof, final fresh checks and atomic publication keep their existing owners. ## Linked Issues or Issue Description Refs: #20418, #20437, #20441. **What happened?** A cohort retained each member's capture archive, verified archive, comparison archive and restored tree until atomic publication. The capture worker limit did not limit that retained parent data. **Expected behavior** Release completed parent payloads once their consumers finish. Keep files that a later publication check still needs. **Steps to reproduce** Run the added held-peer regression against the previous owner. It completes one parent, holds a later member and finds all four completed parent paths still present. ## What Changed - Release three named parent archives and the restored tree after a successful online cohort capture, before that worker takes another member. - Keep parent scratch roots and all nested-child verification areas. Keep every final fresh child and publication check. - Retain payloads for archived repository intents. Use the existing writable-tree cleanup for read-only restored directories. - Add real filesystem regressions for early release, retained child proof, late child mutation and the final archived-intent path check. Update the source-fence guide. This branch incorporates the final #20441 owner and both distinct saved-relative-path regressions. Its scratch-specific production change is nine added lines and one removed line. Single imports and saved journal/archive formats stay unchanged. ## Verification - Canonical Node24 preflight passed. The Runner build came from the content cache. After the current-master merge, the preflight checked 1,674 inputs and 1,817 outputs and found unchanged build inputs. Plugin SDK prerequisites passed. - Genuine RED on the unchanged #20441 owner: one held-peer case failed at four retained payload paths; the archived-intent control passed. The run took21.14s, with221 unrelated cases intentionally unselected. The first sandbox-restricted setup attempt hit DNS errors and was stopped; the normal network-qualified preflight then passed. - Initial treatment: four selected PostgreSQL/filesystem cases passed in38.61s, with219 intentionally unselected. The held peer observes released parent payloads and retained child areas. A later child mutation still refuses the whole cohort and publishes no authority or activity. - The archived-intent control observes all parent payloads at the actual final saved-relative-path check. The changed path refuses publication and keeps task/repository state intact. This exercises its actual singleton owner; archived-only sources do not enter the online cohort API. - After a normal merge of current master and the final #20441 test, the five affected cases and server TypeScript passed. No full local matrix was repeated. Initial CI failed the inherited recovered-upload assertion because the new retention test leaked its archive and filesystem spies. That case passed alone but failed in the adjacent three-case sequence (two pass, one fail); scoped `finally` cleanup made the same sequence pass three of three in17.15s, with191 unrelated cases intentionally unselected. No upstream assertion or production code changed. Server TypeScript passed again. All required [CI](https://github.com/paperclipai/paperclip/actions/runs/42746586511) passed on final normal-merge head `604c0f67bf7282ecd3f6e5e5da1c9d8637ef70cc` (56 success, four skipped and one neutral; no pending or failing checks). The previously failing server4/12 passed in10m23s. Fresh Greptile is5/5 on that exact head with no open threads. The merge resolves the duplicate #20441 squash insertion; both tested owner and test files are byte-for-byte identical to repair head `b5c8eed82b6a676985e1a8af785e5df2ae3be2ce`. The prior head received [Greptile5/5](https://github.com/paperclipai/paperclip/pull/20447#issuecomment-6757435885); its archived-cohort coverage warning was [answered using the actual public admission boundary](https://github.com/paperclipai/paperclip/pull/20447#discussion_r4520641017). - `git diff --check` passed. No hosted cleanup, migration, provider action or deployment was performed. ```sh node scripts/agent-preflight.mjs --skip-rust-debug pnpm --filter @paperclipai/plugin-sdk ensure-build-deps node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts server/src/services/remote-workspace-migration.test.ts -t 'releases completed parent payload|retains archived intent payload|shares the merged child verifier|rechecks the current archived repository intent path at publication' pnpm --filter @paperclipai/server exec tsc --noEmit ``` ## Risks - This reduces retained parent data. It does not establish a total disk bound. Active capture/encryption/readback files and retained child areas still need measured capacity. - Cleanup failure uses the existing worker abort and peer-join path before publication. Original source files, uploaded objects and journals remain intact. - Archived repository intents still need their restored payload at publication. Those payloads stay retained. No capture limit, budget or integrity guard changes. - Saved operations and archive identities carry forward unchanged. Resume still uses the existing full source and authenticated object checks. ## Model Used OpenAI GPT-6 (Codex), with 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
|
1b52f359dd |
test(restart-chaos): add config fault waves (CF1-CF3) and the CF4 design (#20446)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness (`tests/restart-chaos/`) starts an isolated control plane, runs real agent tasks and restarts the server during them > - Every wave starts from a new instance with valid default settings and one valid provider > - On 2026-11-09 production failed on state that a new instance never has: a setup command that named a renamed script (#20269), and an account default that did not exist (#20361) > - This pull request adds a "config fault" family (CF1–CF3): the harness breaks one saved reference on a task's own agent or project, restarts the server while the task is parked, repairs the setting and grades the result > - The benefit is that the harness now catches "stale saved setting" failures, including a run that retries forever or a task that stays Queued with no explanation ## Linked Issues or Issue Description Refs #20269 Refs #20361 **What existing behavior does this improve?** The restart chaos harness only tests new instances with valid settings. It cannot see failures that come from saved settings that point at a missing object. **Subsystem affected** `tests/restart-chaos/` (test harness only). No product code changes. **Current behavior** No wave breaks a saved setting. The CF2 failure (#20361) and the CF1 failure (#20269) were found in production, not by the harness. **Proposed behavior** `config-wave.ts` runs CF1–CF3. The oracle is not "the task completes". A task passes when all of these are true: - it completes, or it rests on a message that names the broken setting on the task page within a bound; - no run retries unchanged configuration forever; - no task stays Queued with no owner; - the rest survives a K1 or K2 restart; - after repair and the offered action (connection card, Retry, or agent setup Retry), the task completes. ## What Changed - `tests/restart-chaos/config-faults.ts` (new, pure): the catalog of broken references, the seeded plan, `parkState`, the task-level message check and `gradeConfigTask`. CF1 is a missing setup script. CF2 is a missing personal OpenRouter default. CF3 is a seeded walk over a deleted secret, a missing workspace path, a missing base ref, an unknown model, plus CF1 and CF2. `CONFIG_REFERENCES_NOT_BROKEN` lists the settings CF3 cannot break, and why. - `tests/restart-chaos/config-wave.ts` (new): the wave runner. Each task gets its own agent, credential secret and project. The runner breaks the setting through board routes, launches an A1 brief task, polls, fires K1 or K2 while the task is parked, repairs the setting, uses the offered action and grades the task. It reuses the existing supervisor, seed and A1 oracle. Waves are limited to 4 tasks and a concurrency of 2. The instance is deleted at the end. - `tests/restart-chaos/config-faults.test.ts` (new): unit tests for the plan, the arguments, `parkState` and the grader. These include the CF2 bug signature and the CF1 "message only on the run page" signature. - `tests/restart-chaos/README.md`: a new "Config faults" section and a row in the pieces table. - `doc/plans/2026-11-09-chaos-config-and-aged-state-faults.md`: the CF4 design (an instance aged by an older release, built on the existing K14 `--upgrade-from` machinery). CF4 is not built in this pull request. - There are no edits to `main.ts`, `types.ts`, `workloads.ts`, `invariants.ts` or `grader.ts`. Other agents work on those files in parallel. ## Verification - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts`: 24 files, 291 tests pass (before the last unit test was added). The config-fault suite has 13 tests, and all pass. - `node scripts/typecheck-test-projects.mjs tests/restart-chaos`: passes. - Live waves (local, isolated instance, one wave at a time): - **CF2 red before #20361** (base `8004ad069e`, before #20361 merged): - Waves `w-20261110T000602-s201-cf` and `w-20261110T004818-s202-cf` fail with `CF2/ai_default:bounded_rest+actionable_message+lifecycle_retrying_unchanged_config+queued_without_live_owner`. - The user sees `Queued`, a `resume` user action, and "Execution configuration check failed. Paperclip could not complete this step. It will retry automatically." There is no card. - A debug run with logging added to the lifecycle failure log (not committed) showed that the underlying error is `ai_connection_default_missing`. Verification retried it every 60 s, more than 20 times. - After the harness connects an account that is installed for the agent, the task completes 35 s later on its own. - The CF2 run after #20361, and the CF1 and CF3 waves on current master, are added below as they finish. ## Risks - Low risk. The change is test harness code and docs only. - A wave costs provider tokens only when the repaired task runs (one A1 brief). - The CF2 repair creates a real OpenRouter account in the isolated instance. The server validates the key live. - The oracle reads the task page through the API (interruption, execution status, cards, agent setup status and notices). It does not render the board UI. A message that appears only in the run's own error text is graded `message_only_in_run_detail`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, subagents). 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 - [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
|
0991d9bda1 |
fix(workspaces): protect migration sources and add guarded recovery service (#20443)
## Thinking Path > - Paperclip manages agents and their task workspaces. > - Online migration saves files before it publishes remote workspace authority. > - A saved migration journal can survive the importing process. > - Git cache expiration did not count that journal as a consumer. > - Expiration could remove a source and change its fingerprint while its import was incomplete. > - This PR protects unfinished imports and exposes a guarded internal service transition for an already affected cohort. > - New imports can preserve the current sources while all old recovery data stays retained. ## Linked Issues or Issue Description **What happened?** Git cache expiration could archive and remove a workspace after an import process ended but before its saved cohort finished. The saved source fingerprint then differed from the current archived workspace. Exact cohort resume correctly refused that changed source. Reopening the workspace cannot recreate the old metadata or fingerprint. **Expected behavior** An unfinished migration remains a durable consumer after process loss. If an older server already expired some sources, one canonical transition can fail the exact unpublished cohort without losing its snapshots, objects, source history, or recovery obligations. New attempts use the current verified sources. **Steps to reproduce** 1. Start a cohort import of eight Git worktrees with local changes. 2. Interrupt it after four snapshot uploads. 3. Let an older server expire two idle source worktrees. 4. Resume the saved cohort. It refuses the changed archived sources. Related: #20429 preserves exact cohort identity when independent companion imports change only the scope proof. #20424 preserves prepared recovery state for archived sources. This PR handles completed source expiration, not a scope-only change. Public searches found related expiration work and no matching compensation owner. ## What Changed - Count saved migration markers and unfinished migration journals as Git expiration consumers. Keep failed migration retention holds as consumers too. - Expose `supersedeExpiredRemoteWorkspaceMigrationCohort` as an internal service API for a privileged operator with a real database handle and reviewed company/cohort inputs. It is called directly by the PostgreSQL/Git test and reviewed one-time administrative qualification. It adds no CLI mode or HTTP route. The existing CLI still refuses a damaged cohort; this PR does not claim automatic CLI recovery. - Require an exact unpublished cohort of up to eight members. It requires completed expiration and an unexpired restore recipe for each missing member. The other members must retain their saved source fingerprints. - Recheck current source and recovery state under source, lifecycle, finalizer, and row locks. One transaction fails all old journals, writes audit records, and removes only their exact matched active markers. - Keep old cohort proofs, operations, snapshots, and object references. Genuine failure receipts use the existing failed-save retention hold. This transition does not report migration success. - Reuse the existing bounded source-lock queue when `sourceLockWaitMs` is set. Keep fast refusal by default. Cancellation targets only this waiter, and current checks still run after admission. - Record predecessor IDs on new attempts and reserve a snapshot generation above the existing ledger maximum. New success does not retire the old hold. - Document the owning transition and retention rules. ## Verification - The two new journal-consumer cases failed under the old expiration predicate and passed under the new predicate. - The full new recovery case passed in 21.12 seconds with real PostgreSQL locks, Git repositories, encrypted local object storage, and archive restores. It covers four retained uploads, two real expirations, unchanged old resume refusal, authority change refusal, transaction rollback, a held native finalizer, idempotence, eight new imports, dirty/untracked file restores, and a retention pass 30 days later with no object deletion. - Server TypeScript and module-boundary checks passed. - On `7cec8c915acfad2d08874eaf26abaa852f560219`, the queued-admission regression failed before forwarding the option (24.07 seconds), then passed (22.25 seconds). It verifies a real held project lock, cancellation that preserves the holder and all journals, release into full compensation, retention, and eight restored fresh imports. Current server TypeScript and module/diff checks passed. The only production follow-up changes are the optional queue input and its existing-owner call. - The earlier `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c` CI run had one Runner support failure: the unchanged credential-free Python/Pi calibration exceeded its outer 25-second subprocess deadline. That job passed 6,926 other tests and skipped four. The timeout is retained as a nonpass. The changed logical head requires fresh CI and review; no test deadline was raised. - The affected cohort, Git expiration, and eligibility suites passed all 148 cases in 228.41 seconds on `aa589f3f447e36d0277c32f8b44cbe5e84a673be`. Merged current master `a9d0946e64`; the only conflict retained both independent added test blocks. The complete merged cohort suite passed all 30 cases in 70.00 seconds on `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c`, including upstream larger-cohort controls. Final-head server TypeScript, incremental preflight, module-boundary, and diff checks passed. Unchanged expiration/eligibility evidence remains valid. Fresh `809ab704e619812d8f1bf38cf5de9c65893861cc` CI and review are pending. - On `809ab704e619812d8f1bf38cf5de9c65893861cc`, the complete recovery fixture passed in 21.38 seconds after adding one valid archived recipe with no restore point. It refuses with `migration_supersession_expiration_unverified` and keeps all workspaces, journals, and snapshots unchanged. The same case then verifies queue cancellation/release, full compensation, retained history, fresh imports, and restored bytes. Server TypeScript passed. This follow-up changes only the test and service-scope documentation; product bytes remain identical to qualified `7cec8c915acfad2d08874eaf26abaa852f560219`. - Earlier fixture attempts did not pass. They exposed the selection of a held source and an incorrect expected refusal string. The fixture now selects eligible sources and asserts the canonical authority refusal. Production guards were not relaxed. - Live qualification of the exact owning product bytes completed in 230.419 seconds. All eight old operations became failed with auditable supersession lineage. All eight old snapshot rows remained byte-for-byte identical, and their object references remain retained. The existing source-lock queue waited 1.151 seconds for a normal finalizer handoff. The three complete source/recovery passes took about 218 seconds including their transaction, which explains why the earlier 180-second total budget was insufficient for that measured workload. No source proof or publication guard was skipped. Compensation made no provider or object-storage calls. - Earlier live qualifications remain nonpasses: the 180-second attempt did not finish; its instrumented successor completed source checks and refused a busy source lock before mutation. The successful bounded-queue attempt retained those failure records. Fresh archived/online imports are a separate subsequent step and are not claimed complete here. - The local checks do not claim browser acceptance, app deployment, fleet enrollment, or a completed zero-downtime cutover. ## Risks - This owner mutates migration bookkeeping. It preserves all-or-nothing row comparisons and current source/recovery locks. A changed journal, authority, writer, finalizer, expired recipe, or partial transaction refuses the transition. - Historical snapshots and objects remain pinned after new success. This intentionally retains storage. Release requires a separate reviewed action. - An older importer cannot use the successor generation contract. Use the containing canonical owner for recovery. Normal app delivery must also carry the expiration guard; a private maintenance graph does not patch a running server's cleanup worker. - The migration owner crosses 1,000 lines. The bounded compensation transition remains beside its private source inspection, fingerprint, fence, reservation, and publication contracts. Moving it now would require exporting or duplicating those safety owners. No unrelated refactor or new framework is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source editing, Git, and local execution tools. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [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> |
||
|
DottaandPaperclip
|
b6f4008f9b |
docs: explain individual agent interaction permissions (#20422)
Document all eleven interaction controls, sparse defaults, CLI administration, ownership rules, pending-delivery behavior, and access boundaries. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Dotta
|
f73ff4ace9 |
fix(native): reprove stale filesystem preservation receipts (#20438)
## Thinking Path > - Paperclip manages agent work and preserves saved native sessions across releases. > - Filesystem independence receipts bind database facts to preserved controller and skill evidence. > - A workspace migration can add run metadata without changing those preserved bytes. > - That change makes the saved facts digest stale. Full native preservation then refuses the existing receipt, even after fresh source checks. > - WEB preservation already supports a full current proof and guarded receipt refresh. > - This pull request applies that pattern to full native receipts, reuses the authenticated archive, and keeps the existing ownership and destruction checks. > - The benefit is a current receipt without a new archive or provider replay. ## Linked Issues or Issue Description Refs: #20322. **What happened?** Adding the canonical `legacyWorkspaceRecoveryMigration` marker to a settled native run makes its saved receipt stale. The full native preservation owner checks current evidence but then refuses the old facts digest. **Expected behavior** The owner can refresh the proof metadata after it proves all current facts and preserved bindings again. Changed evidence or authority must leave the prior receipt intact. **Steps to reproduce** Use the added PostgreSQL regression. It saves a valid receipt for a succeeded source with 453 checkpoint parts, adds the producer-shaped migration marker, and calls preservation again. The unchanged owner fails with `native_harness_filesystem_independence_unverified`. ## What Changed - Reuse the prior controller archive through the existing authenticated readback path. - Require the old receipt to match every current immutable binding before a facts refresh. Reconstruct its exact digest from unchanged bindings and the prior proof fields. - Recheck current database facts, assets, controller and local files under the existing finalizer, selection, task and run fences. - Publish with an exact prior-receipt comparison. Keep a valid concurrent winner and refuse changed receipts or new owners. - Audit the prior receipt digest and four proof fields. Add focused source, integrity, race and DB-only validation tests, and update the database guide. ## Verification - Local Node24 canonical preflight and plugin SDK prerequisites passed. The Runner build came from the content cache. After a normal merge of fresh master, preflight confirmed unchanged build inputs. - Genuine RED on `3040448da5`: the saved receipt is valid before the migration marker and stale afterward. Reproof fails at the old receipt guard (one failed, 102 intentionally not selected). Both retained RED attempts reached that same guard. - GREEN on final local merge `00f4b5d678083106afc88fb2f7e0588961b46d6d`: 19 PostgreSQL controls passed, 91 intentionally not selected (17.75s). The wrapper completed cleanup. - The positive preserves the same archive, controller bytes, assets, local proof and all 453 checkpoint parts. It performs authenticated object reads and zero archive PUTs. Runs, results, finalizers and leases remain unchanged except for the receipt. The audit reconstructs the exact prior receipt. - DB-only validation passes with the original home, state directory and volume absent, while storage reads throw. - Refusal and race controls cover changed asset bindings, raw controller bytes, missing destruction evidence, unknown receipt fields, a valid concurrent winner, changed receipts and a late active owner. Existing WEB refresh, physical finalization-lock and saved-binding controls also pass. - Server TypeScript passed, including the adjacent integration test. Fresh Greptile is 5/5 on exact head `00f4b5d678083106afc88fb2f7e0588961b46d6d`, with no review threads ([review](https://github.com/paperclipai/paperclip/pull/20438#issuecomment-6756548830)). Final-head CI is terminal: 56 success, 4 skipped and 1 neutral. The aggregate `ci / verify` passed ([run](https://github.com/paperclipai/paperclip/actions/runs/42737704110)). - The first Runner-support job had one unrelated process-inspection nonpass: `server-stop.test.ts` expected a successful wrapper close, but its bounded process inspector returned no table. Its cause remains unknown. The unchanged test, process helpers, workflow and dependency inputs match the passed master control. One authorized same-head job retry passed all 328 files / 6,927 tests (4 skipped); the exact previously failed case passed in 665ms ([retry](https://github.com/paperclipai/paperclip/actions/runs/42737704110/job/129576420893)). No timeout, assertion or source change was made for the retry. ```sh node scripts/agent-preflight.mjs --skip-rust-debug pnpm --filter @paperclipai/plugin-sdk ensure-build-deps 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-harness-filesystem-independence.integration.test.ts -t "reproves stale native facts|retains stale native proof|guards stale native reproof|invalidates a published receipt after changed|actual physical-finalization lock|refreshes stale WEB facts|refuses a WEB facts refresh|retains or refuses the existing receipt when an earlier capture finishes late" pnpm --filter @paperclipai/server exec tsc --noEmit ``` ## Risks - Old receipt schemas and immutable bindings remain required. Full fresh proof carries compatible saved receipts forward; unknown fields or changed evidence refuse refresh and preserve the old receipt. - Full native preservation still requires canonical destroyed-provider receipts. The change retains the existing source capability, cancellation, finalizer and publication checks. - Only proof metadata changes. No provider operation, archive replacement, hosted recovery or deployment was performed for this PR. Live qualification remains with the operator after delivery. ## Model Used OpenAI GPT-6 (Codex), with 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 |
||
|
DottaandPaperclip
|
07f6e8ff75 |
perf(workspaces): group same-root cohorts with bounded workers (#20437)
## Thinking Path > - Paperclip preserves agent work when its control-plane server is replaced. > - Online migration imports local workspaces into authenticated snapshots. > - Several workspace records can point to one physical directory. > - Shared child verification avoids repeated work inside one held cohort fence. > - The eight-member limit splits 30 identical-root aliases into four cohorts and repeats the child passes. > - This pull request admits up to 32 members only for one canonical physical root, while keeping each work stage bounded to eight peers. > - The benefit is fewer repeated child checks while capture and publication guards stay in place. ## Linked Issues or Issue Description Refs: #19873, #19882, #20418, #20424, #20429. Related PR search found shared verification and restart fixes, but no larger same-root cohort owner. The shared verifier and archived-source changes are merged into this branch from current master. **Current behavior** Thirty fresh aliases of one physical root require cohorts of 8, 8, 8 and 6. Each cohort performs an initial and final local child-validation pass. The same children are checked eight times in total. **Proposed behavior** Admit one cohort with nine to 32 members only when every canonical resolved physical root is exactly equal. Keep the existing complete source-scope equality and admission guards. Keep mixed-leaf cohorts limited to eight members. Bound source reads, reservations, recovery checks, final readers, replay and cleanup to eight joined peers. Capture concurrency stays at most eight. **Reason and benefit** For N distinct canonical covered children, the 30-alias case changes 8N local child checks to 2N, a 75% reduction in repeated child validation. If N is 68, that is 544 checks versus 136. Thirty parent captures, uploads and restores remain required. No total wall-clock improvement is claimed. **Breaking changes** Saved cohort fields and exact membership remain unchanged. Older tools that enforce the eight-member bound refuse larger cohorts. Use the newer tool to resume them; never split or regroup their saved membership. ## What Changed - Raise the canonical service, saved-cohort and maintenance-request member bounds to 32. Keep the capture-worker bound at eight and single-workspace modes at one. - Require one exact canonical physical root for more than eight members. Keep the existing full source-scope comparison, leaf rule and P/I/L/F guards. - Use one private stage mapper with batches of at most eight. Join every started peer before an error can leave the stage. Stop future batches after a failure or cancellation. - Preserve the existing cancellation and pending-snapshot ordering for stages of at most eight members. Check cancellation around extended batches. - Keep all saved cohort, member, operation and snapshot identities, proof refresh and atomic publication checks. - Bound cleanup without skipping later scratch cleanup after an earlier cleanup error. - Extend the existing PostgreSQL fixture with ten real aliases, partial-upload resume, cold restoration, replay and mixed-root refusal. Extend the request boundary test. - Document old-tool refusal, work counts and the retained-scratch capacity bound. ## Verification - Canonical Node24 worktree preflight and Plugin SDK prerequisites passed. After merging current master, preflight passed again with unchanged cached build inputs. - Genuine RED on the old bound: both real ten-alias import/resume cases and the 32-member request reject the eight-member limit (3 failed; 47 not selected; 18.23 seconds). - First focused GREEN: 4 passed in 24.20 seconds. The fresh ten-alias case performs exactly four local child captures for two children: one initial and one final pass. Actual reader, transaction and upload maxima are eight. All ten archives cold-restore after the original source is parked; replay retains the same cohort. - The partial-upload case keeps all ten saved operation IDs and the existing snapshot IDs. More than eight members across different canonical leaf roots refuse before any journal or storage work. The request parser accepts 32 members and eight workers, and refuses 33 members or nine workers. - The first expanded corpus had 55 passes and one real compatibility failure: a new eager cancellation check skipped the old pending snapshot boundary. The mapper now retains existing cancellation ordering for at most eight peers. That failure remains recorded as a nonpass. - Final sources include current master at `18697b8d21`, with the verifier, archived-source and UI repairs. The selected four-file corpus passes 47 cases in 78.88 seconds, with 191 intentionally not selected. The full maintenance suite separately passes 23 cases in 10.21 seconds. Nine cases overlap; the final union is 61 distinct passing checks. - Final coverage includes all 27 cohort cases, all 23 maintenance cases, all six actual old-format compatibility cases and five archived prepared-source controls. Existing source mutation, finalizer contention, actual fence loss, cancellation peer joins, journal compare-and-swap and outer-commit rollback controls pass. - Final server TypeScript, module boundaries and whitespace checks pass. Only the resource-risk documentation changed after that test run; runtime and fixture bytes stayed unchanged. - Commands: `pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts server/src/maintenance/online-workspace-migration.test.ts server/src/__tests__/online-workspace-migration-compatibility.test.ts`; archived controls use the existing `remote-workspace-migration.test.ts` cases named `unselected archived recipe` and `archived prepared source`; `pnpm --filter @paperclipai/server exec tsc --noEmit`; `node scripts/check-module-boundaries.mjs`. - Initial source review was 4/5 and requested a control for the extended reader-batch stop boundary. The test-only follow-up `ca80ae9cc8791a72dab632090488093c5d0916f3` adds actual ten-member failure and cancellation cases. They hold a first-batch peer, verify that the source fence stays held, and prove members nine and ten never start preflight. Both pass in 10.87 seconds with no new journal, snapshot or upload effects. Runtime source hashes are unchanged; the local union is now 63 distinct checks. - The first stop-test attempt observed the two controlled readers before all SQL-backed peers had entered the inspection callback. Its assertion was too early. The corrected test checks the held first-batch readers at that point, then requires all eight to join before completion. Both cases pass; the earlier attempt remains a nonpass. - Final head `ca80ae9cc8791a72dab632090488093c5d0916f3`: [required CI](https://github.com/paperclipai/paperclip/actions/runs/42737676664) and [aggregate verify](https://github.com/paperclipai/paperclip/actions/runs/42737676664/job/129575813765) passed. Full rollup: 56 successes, four intentional skips and one advisory neutral result. Classic Snyk passed. [Greptile](https://github.com/paperclipai/paperclip/pull/20437#issuecomment-6756419093) and [Ponytail](https://github.com/paperclipai/paperclip/pull/20437#issuecomment-6756388693) both reviewed that exact head at 5/5. The one review finding is addressed and resolved; no open findings remain. Older partial CI and the initial 4/5 review remain nonfinal evidence. Hosted acceptance and measured wall-clock savings remain pending. ## Risks - Completed parent captures retain their scratch until the atomic publication completes. A 32-member cohort can retain up to four times the parent scratch of an eight-member cohort. Check actual parent capture, verification and restore sizes against current target filesystem capacity before selecting a larger cohort. Keep the existing resource guards. - Larger member arrays must not raise execution fanout. Every reader/reservation/final stage is bounded to eight and joins current peers before failure. Capture workers remain at most eight; child-verification workers remain two. - More than eight members require the exact same canonical physical root and complete source scope. A different root, new writer, changed source, recovery owner or journal still refuses through the existing owners. - Shared verification is limited to the held invocation fence. The separate fresh final pass remains required after all captures join. Different child sets and restart attempts do not share a local-byte proof. - The old saved membership and archive identities remain authoritative. Older tools fail closed on the larger bound. No schema, archive or provider format changes are included. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The runtime does not expose a more specific deployment identifier 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
232480d18f |
perf(runner): write preparation span rows to the run log in the background (#20426)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a native run starts its provider, the executor copies the preparation spans (queue, prepare phases, environment) into the run trace and the run log > - It records about 25 spans and waits for one run-log insert per span, one after another > - On a hosted test deployment, that step (`native.controller.span_persistence`, added in #20396) took a median of 0.28 s, and every task waited for it before the Runner started > - The run trace already has a way to write diagnostics without blocking the caller: `defer`, which `finish` drains before the root span closes > - This pull request writes those run-log rows through that path, in order, while the spans themselves are still recorded at once > - The benefit is about 0.25 s less before the provider starts, on every native run, cold or warm ## Linked Issues or Issue Description **What existing behavior does this improve?** The time between the end of heartbeat preparation and `native.session.execute` for native Runner tasks. **Subsystem affected** Server: native runtime tracing (`server/src/services/native-runtime/native-run-trace.ts`) and the native session executor. **Current behavior** `executeNativeSession` calls `trace.record` for each preparation span and awaits it. `trace.end` awaits `emitCompletedSpanEvent`, which inserts one `run.performance.span` row through `onEvent`. On a hosted test deployment, measured with the #20396 spans across 10 runs, `native.controller.span_persistence` took a median of 0.28 s: - 0.30 to 0.36 s for runs right after a computer wake - 0.17 to 0.25 s for runs on an awake computer It is the second largest step between preparation and the provider start (the median of that whole gap is 1.0 s). **Proposed behavior** The spans are still started and ended at once, with the same parents. The speed-budget bookkeeping still runs at once too. Only the run-log row is written later: one row at a time, in the order the spans ended, before the trace finishes. **Reason and benefit** The rows are diagnostics, and they already carry their own start time (`wallStartedAtMs`, `startOffsetMs`). Readers do not depend on the row order: the only reader in the repository, `summarizeStartupReuse`, looks for `runner.process.launch`, which this change does not defer. The provider can start without waiting for these inserts. **Breaking changes** None. ## What Changed - `native-run-trace.ts`: `end` and `record` take an optional `deferRunLog`. With it, `end` queues the run-log row on a per-trace chain registered with `defer`, so rows are written one at a time in order and `finish` waits for them. A row requested after the trace finished is dropped, as for other deferred diagnostics. - `native-session-executor.ts`: the preparation spans and the `environment.startup` scope use `deferRunLog: true`. - `doc/run-log-events.md`: note that native preparation span rows are written late, in order. - `native-run-trace.test.ts`: two tests. - Spans are recorded under their parent while every row write is blocked, and the rows are written in order before `task.provider_session.measured`. - A deferred row requested after `finish` is not written. ## Verification - `npx vitest run src/services/native-runtime/native-run-trace.test.ts src/services/native-runtime/native-run-trace-context.test.ts` (34 passed). Both new tests fail without the source change: the first times out waiting on the blocked write, and the second sees the row written. - `npx vitest run src/__tests__/heartbeat-preparation-gap-spans.test.ts src/services/native-runtime/native-session-executor.test.ts` (1321 passed) - `pnpm --filter @paperclipai/server typecheck` ## Risks - Low risk. The change is diagnostic only: OpenTelemetry spans and the speed-budget checks are unchanged. - The preparation rows can now land in the run log after the first Runner events. Readers already use the payload times, not the row order. - If the server process dies between the provider start and `finish`, the preparation rows of that run can be missing from the run log. The OpenTelemetry spans are not affected. - The deferred rows are written while the run continues, as budget-breach rows already are, so the sink sees concurrent writes from one run. This was already the case for deferred diagnostics. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs, Honeycomb queries) and extended thinking. ## 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
|
18697b8d21 |
perf(workspaces): share cohort child-source verification (#20418)
## Thinking Path > - Paperclip must preserve agent workspace bytes during online migration. > - A cohort can contain several aliases of the same physical source. > - The importer verifies imported children before capture and again before publication. > - The immutable-object cache avoids repeated downloads but still recaptures local child bytes for every alias. > - This pull request shares the initial proof within the held cohort fence and checks fresh bytes once more after every capture joins. > - The benefit is less repeated local archive work while child mutations still block the whole cohort. > - Required browser CI twice reproduced an independent stale composer-mode bug. A focused regression proves that refreshed persisted task mode was ignored after mount. The state repair follows task data when there is no local override. Explicit local mode choices retain their own generation so a completed send cannot clear the next draft’s choice. ## Linked Issues or Issue Description Refs: #20311, #20387, #20401, #20404. **Current behavior** Two aliases recapture the same imported child's local bytes four times, even with one immutable object read. A larger alias cohort repeats this multi-GB work for each member. **Proposed behavior** Share the successful initial verification for the same physical root, Git roots and exact recorded child set or discovery mode. After all captures join, run a separate fresh final verification for each root and recorded child set. Keep every member's database and publication checks. **Reason and benefit** The local PostgreSQL fixture proves that two aliases now perform two child recaptures instead of four. Hosted timing improvement has not been measured. ## What Changed - Add invocation-local initial and final proof maps to the existing cohort child verifier. - Retain the existing canonical source-owner check before and after shared results. - Defer cohort final nested checks until all captures finish, and join every final reader before publication or fence release. - Keep singleton verification, saved journal schemas, snapshot IDs, archive bytes and per-member publication checks unchanged. - Extend the existing real PostgreSQL alias fixture with a child mutation during the last parent upload. - Repair the required CI planning-mode race with a nullable local override: absent override follows task data; a user selection belongs to the draft. A successful send or confirmed resend/receipt clears it only if no newer choice exists. Failed saves preserve the mode and restored message. Queued-edit backup retains whether the mode was explicit. Leave the browser assertion and timeouts unchanged. ## Verification - Canonical Node24 worktree preflight and plugin SDK prerequisites passed. Server TypeScript passed. - Genuine unchanged-source PostgreSQL RED: two aliases performed four local child recaptures instead of two (one failed, 22 intentionally not selected). GREEN: five source-sharing, last-upload child mutation, capture/cancellation join and actual outer-commit rollback controls passed (18 intentionally not selected; 66.53s). The actual fence-loss control also passed (one passed, 22 intentionally not selected; 38.13s). Both wrappers completed cleanup. - Migration source and tree bytes remain identical to locally qualified `466837cfe997924c3794fd5d0e96134cab1cd04c`; later commits only repair the composer and its tests. - Required planning browser CI originally failed, then failed its one unchanged-head retry, because the planning chip remained absent after opening-question dismissal. Those failures remain recorded ([original](https://github.com/paperclipai/paperclip/actions/runs/42723622165/job/129523785985), [retry](https://github.com/paperclipai/paperclip/actions/runs/42723622165/job/129534415590)). The final-head identical case passes in 30.5s, with all 34 shard cases passing ([final CI](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129550549775)). Assertions and timeouts were not relaxed. - Composer regressions reproduced stale loaded mode, optimistic-save rollback, overwritten next-draft choices, and stale mode after confirmed resend/receipt. All 145 composer tests now pass (2.88s). Canonical UI TypeScript and token gates pass. The original planning browser case also passed locally (15.5s; 26.9s including setup). - The first final-head serialized-suite attempt was automatically cancelled at its unchanged 20-minute job limit. Grouped Turbo output did not expose an individual failing assertion; cause remains unknown ([retained attempt](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129550550373)). One same-head diagnostic retry was a cache miss and executed all 419 heartbeat recovery tests successfully in 376.94s on the identical CI checkout ([retry](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129561196792)). It is not classified as infrastructure or a known flake. - Final exact head `ba1f85f18085dd0e84c594ed0cb68d5bf817c93c`: all 61 checks terminal (56 success, four skipped, one neutral); required CI is green ([workflow](https://github.com/paperclipai/paperclip/actions/runs/42731309796)). Fresh Greptile is 5/5 with every review thread resolved ([review](https://github.com/paperclipai/paperclip/pull/20418#issuecomment-6754705323)). Focused commands: ```sh node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts -t "shares the merged child verifier|joins active capture peers|keeps every authority and activity unpublished" pnpm exec vitest run ui/src/components/task-chat/TaskChatComposer.test.tsx pnpm --filter @paperclipai/server exec tsc --noEmit pnpm --filter @paperclipai/ui typecheck pnpm check:token-gates ``` ## Risks - Reuse is limited to one invocation under the same complete source and finalizer fences. Different physical roots, Git roots, recorded child sets and discovery mode have different keys. No local-byte proof is stored durably or reused after restart. - Initial and final maps are separate. A child change during any parent upload must fail the fresh final verification before the atomic publication transaction starts. - Final readers all finish before an error can release the source fence. Per-member database child rows, source fingerprints, journals, keys and publication locks retain their existing checks. - This reduces repeated verification for aliases. It does not change capture concurrency, upload policy or immutable checkpoint validation. Hosted speedup remains unverified. - The CI repair must distinguish task data from explicit draft intent. Only a successful send with the same choice generation clears its override; a later draft selection and failed-save rollback retain it. Regression tests cover these transitions. It adds no styling, request, or persistence changes. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model identifier 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> |
||
|
DottaandPaperclip
|
3040448da5 |
fix(workspaces): refresh saved cohort source proofs on resume (#20429)
## Thinking Path > - Paperclip preserves agent work when a control-plane server is replaced. > - Online workspace imports save exact cohort, operation and snapshot identities for restart. > - A cohort proof also includes the complete current companion source scope. > - An independent companion import removes that companion from the local scope. > - The saved cohort then fails resume even when every saved member is unchanged. > - This pull request rechecks the current scope under fresh locks and refreshes its proof for all members in one transaction. > - The benefit is restartable online migration while independent work progresses. ## Linked Issues or Issue Description Refs: #19873, #20394, #20418. Related PR search found the original cohort owner, recovery-proof refresh and separate child-verification reuse. This fix addresses cohort scope freshness. **What happened?** An interrupted cohort rejects resume with `workspace_cohort_membership_changed` after a nonmember companion is migrated. The saved source proof includes companion workspace and task IDs. The exact saved members and their fingerprints can remain unchanged. **Expected behavior** The same saved cohort must resume after fresh admission. It must keep all member, operation and snapshot IDs. Changed members, active owners, changed journals and lost source locks must still refuse publication. **Steps to reproduce** 1. Start a two-member cohort and interrupt the second upload after the first archive is saved. 2. Import an archived nonmember companion through the existing canonical migration owner. 3. Resume the same two-member cohort. The original implementation refuses before it acquires the source fence. **Paperclip version or commit** Reproduced on master `3a5572c9e5`. **Deployment mode** Online administrative workspace migration with PostgreSQL and authenticated snapshot storage. ## What Changed - Admit a changed aggregate scope proof only when every saved member has a running journal and its current fingerprint matches both saved fingerprints. - Keep partial reservations on the original strict aggregate-proof path. Keep exact cohort and operation membership checks. - Run the existing source, lifecycle, recovery and finalizer admission before refresh. Lock member rows, inspect the current scope again, and compare every journal in one transaction. - Change only `cohortMigration.sourceProof`. Preserve all other metadata, identities, archive and recovery records. Abort the complete refresh on cancellation, a changed journal or a lost fence. - Extend the existing real PostgreSQL fixture with partial upload, canonical companion migration, refusal controls, transaction rollback, cold archive restore and replay. - Document the proof lifetime and resume boundary. ## Verification - Canonical Node24 worktree preflight passed. Plugin SDK build prerequisites passed. - Genuine RED on unchanged product: the real partial-upload and companion-import sequence fails with `workspace_cohort_membership_changed` (1 failed; 22 intentionally not selected; 10.44 seconds). - The identical positive passes after the fix (1 passed; 22 intentionally not selected; 9.87 seconds). - The complete focused cohort file passes: 23 tests in 33.33 seconds. Existing controls include changed member fingerprints, foreign and completed journals, active operations, peer cancellation, actual source-fence connection loss and outer-commit failure. - The new sequence also checks a held source lock, a held finalizer lock, cancellation before refresh, and an injected second-member update failure. The failed update rolls back every proof change. Both saved archives restore tracked and untracked work, and replay uses the same cohort. - After one final read-only preflight assertion, that complete new sequence passes again in 14.52 seconds with 22 existing cases intentionally not selected. Server TypeScript passes on the final files. Module boundaries and `git diff --check` pass. - Commands: `pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts`; final case selection: `-t 'same uploaded cohort'`; `pnpm --filter @paperclipai/server exec tsc --noEmit`; `node scripts/check-module-boundaries.mjs`. - Initial local setup attempts remain nonpasses: sandbox PostgreSQL could not bind localhost, then the new archived companion fixture used a noncanonical repository folder. The corrected fixture uses the existing managed repository path. Neither setup failure is counted as the product RED. - Fresh [Greptile review](https://github.com/paperclipai/paperclip/pull/20429#issuecomment-6755862789) is 5/5 on exact head `6c640c4667b136e5416c1375f491729cf4bb9b49`, with zero inline threads. Required final-head CI is running. Hosted acceptance is not claimed by this PR. ## Risks - The old aggregate proof contains only a hash. Refresh therefore requires durable fingerprints for every saved member and repeats all current admission guards. A partially reserved cohort cannot use this path. - A stale proof must never hide a changed member or a new writer. Current source inspection, P/I/L/F locks, locked member rows, complete journal compare-and-swap and the existing capture and publication checks remain authoritative. - The all-member update must stay atomic. A database fault or cancellation rolls it back. The original cohort, operation and snapshot IDs remain fixed. - The change adds one bounded transaction only when a fully saved cohort's scope proof changes. It adds no recurring work, schema, provider, archive format, concurrency or deployment changes. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The runtime does not expose a more specific deployment identifier 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 - [ ] 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
|
347d85fef3 |
perf(chat): index GitHub review lookups and share review-wait reads (#20400)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub chat endpoints let agents review pull requests. The server reads review state when it lists issues and when it updates GitHub checks. > - On a hosted instance, two GitHub review queries used the most total database time. One query ran about 660,000 times at about 33 ms. The other query ran about 10,000 times at about 1 s. > - The first query finds a source comment's chat link. No index covers `chat_message_links.comment_id`, so each call reads the full table. Issue lists also read the same review waits two times per request. > - The second query joins deliveries, links and runs on a JSON key. The planner walks the agent's run history in creation order and reads each run's context. > - This pull request adds four concurrent indexes, splits the run lookup into two small reads, and shares review-wait reads in one issue-list request. > - The benefit is less background database load, so request reads wait less. The results stay the same. ## Linked Issues or Issue Description **What existing behavior does this improve?** GitHub review state reads. These are the inherited review-wait lookup by source comment, and the review-check lookup of the run that a delivery's comment woke. **Subsystem affected** Server chat channels (GitHub review waits and checks), issue list attention maps, and the database schema. **Current behavior** - `readInheritedGitHubReviewWait` filters `chat_message_links` by `company_id`, `comment_id` and `direction`. No index has `comment_id`, so PostgreSQL reads the full table on each call. The run-origin check does the same lookup first. - An issue list calls the external review-wait read from the review attention map and again from the blocked-inbox attention map. The two maps read the same in-review issues. - `githubReviewCheckState` joins deliveries, links and runs with `context_snapshot->>'wakeCommentId' = comment_id::text`, sorts by `created_at desc` and takes one row. The planner reads the agent's runs in creation order and reads each run context. If no run matches yet, it reads all of them. The delivery filter on `normalized_event` and the link join on `delivery_id` also have no index. **Proposed behavior** The same rows come back from small index reads. Each issue's review wait is read one time per issue-list request. **Reason and benefit** These two statement families used about 33,000 s of database time in one cumulative statistics window. The database had about 50 concurrent active queries, and request reads waited behind this background work. **Breaking changes** None. The migration adds indexes only. ## What Changed - Migration `0456_chat_review_lookup_indexes` builds four indexes with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. It runs through the existing top-level statement path, which repairs an invalid index on retry: - `chat_message_links_company_comment_idx` on `(company_id, comment_id)` where `comment_id is not null`. - `chat_message_links_delivery_idx` on `(delivery_id)` where `delivery_id is not null`. - `chat_deliveries_github_automatic_delivery_idx` on `(endpoint_id, normalized_event->'githubAutomatic'->'context'->>'deliveryId')` where that value is not null. - `heartbeat_runs_company_wake_comment_idx` on `(company_id, context_snapshot->>'wakeCommentId')`. This index is not partial. The planner does not use expression statistics from a partial index. Without these statistics, it estimates many matches and keeps the creation-order walk. - `githubReviewCheckState` reads the delivery's comment IDs first. If there is a comment, it reads the newest same-agent run with one of these `wakeCommentId` values. If there is no comment, it skips the run read. The comparison stays text equality on `comment_id::text`. - `createExternalReviewWaitReader` shares one read per issue between the review and blocked-inbox attention maps of one issue-list request. The reader exists only for that request. - `doc/DATABASE.md` describes the indexes and the new read order. - `scripts/benchmark-review-lookup-indexes.ts` compares the pre-fix and new code with and without the indexes. `scripts/benchmark-review-lookup-index-migration.ts` times the migration on a large synthetic database. ## Verification Local SQL measurements on a synthetic fixture. They are not production timings. Environment: embedded PostgreSQL 18.1, baseline revision `02bfe71a6b`. Fixture: 50,000 links, 5,000 deliveries with about 2 KiB events, and 4,000 runs with about 12 KiB incompressible contexts. The wake run is in the middle of the run history. Each value is the median server execution time from `EXPLAIN ANALYZE` (2 warmups, 9 samples). The order was: no indexes, indexes, then no indexes again. | Caller and statement | Before (no indexes, two passes) | After | Shared buffers before → after | | --- | --- | --- | --- | | Inherited review wait, link by source comment (same SQL) | 2.72 / 2.36 ms | 0.076 ms | 929 → 12 | | Review check, wake run found | 31.9 / 21.9 ms | 0.29 ms (two statements) | 25,776 → 62 | | Review check, no wake run yet | 71.0 / 47.5 ms | 0.06 ms (two statements) | 34,861 → 12 | - Indexes alone do not fix the review check. The old SQL with the new indexes took 8.6 ms (run found) and 29.7 ms (no run), because it still walked the run history. The new SQL without the new indexes was neutral (23.5 and 35.1 ms). - Service wall times on this shared machine were noisy, so the table reports server execution time. - Correctness: the benchmark fails unless the inherited statement returns the same positional rows in each index state. The check service result and the final wake-run rows of the new code must also match the pre-fix module for both cases. - Migration timing, normal migration runner, synthetic data: 200,000 runs (1.8 GB with 8 KiB incompressible contexts), 500,000 links and 100,000 deliveries. First run 4.2 s; rerun 0.38 s. Separate builds: links by comment 1.5 s, run wake comment 1.1 s, deliveries 0.7 s, links by delivery 0.3 s. All indexes were valid. The run index was 8.3 MB. - New tests: - `server/src/__tests__/github-review-check-state.test.ts` checks that the newest same-agent run is selected. It also checks that other agents, other companies, uppercase IDs, non-string values, list-only `wakeCommentIds`, comment-less links and unknown deliveries do not match. It checks that the generated statements can use the new indexes. - `server/src/services/native-runtime/external-review-wait.test.ts` checks that the reader reads each issue one time, keeps concurrent callers correct, and rejects all waiting callers when a read fails. - Red/green: with the pre-fix `review-check-state.ts`, the behavior test passes and the index test fails. With this change, both pass. - Focused runs pass: `issue-blocker-attention`, `heartbeat-issue-liveness-escalation`, review-wait, review-check-state, external-review-wait, review-repair (106 tests), and `external-chat-wait.integration` (285 tests). `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-indexes.ts --baseline-ref=02bfe71a6b pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-index-migration.ts ``` ## Risks - The migration builds four indexes concurrently. It does not block writes. On a large database, the run index reads every run context one time. The local run took 1.1 s for 1.8 GB from a warm cache. Cold storage on smaller compute can be slower. - The run index adds one entry for each run insert and for each run update that is not a heap-only update. Many runs have no `wakeCommentId`, so these entries hold a null key. - The review check now sends two statements instead of one when a delivery has a comment. Each statement reads only a few index entries. - When two matching runs have the same `created_at`, either can be selected. The previous query had the same tie behavior. - This change does not change the number of in-review issues that one issue list reads. The blocked-inbox map still reads all in-review issues of the company. A smaller scope needs a separate change to the liveness classifier. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) by Anthropic, in 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> |
||
|
DottaandClaude Opus 5.5
|
0116e45958 |
perf(db): index pending budget Stops instead of scanning all run results (#20391)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget enforcement stops agent runs when a budget hard limit is reached. The stop intent is saved in each run's `result_json` until run control delivers it. > - The budget worker, run control, and the idle-sleep check find these pending stops with a predicate on `result_json`. No index supports that predicate. > - Each check therefore reads every heartbeat run and its large result JSON from TOAST. On a hosted database, this statement family was among the most expensive: about 8,000 calls at about 1 s each, about 9,800 s in total. > - Pending stops are rare. The predicate is constant, so a partial index can contain only those rows. > - This pull request adds that partial index. The SQL that the server sends does not change. > - The benefit is that each check reads a few index entries instead of the full table. The database has more capacity for request work. ## Linked Issues or Issue Description **What existing behavior does this improve?** The check for committed budget Stops that are not delivered yet. The budget worker runs it each time it looks for work (`hasPendingBudgetEnforcement`), and again for each delivery pass (`listPendingBudgetCancellationScopes`). Run control and the idle-sleep check use the same predicate. **Subsystem affected** Budget enforcement in the server (`server/src/services/heartbeat/budget-cancellation.ts`, `server/src/services/budgets.ts`, `server/src/services/heartbeat/run-control.ts`, `server/src/services/idle-sleep-safety.ts`) and the `heartbeat_runs` schema in `packages/db`. **Current behavior** The query groups all rows of `heartbeat_runs` and reads `result_json->'startupCancellation'->'budgetEnforcement'` for each row. A limit of 1 does not help, because the group and sort occur before the limit. Cost grows with the run history, not with the number of pending stops. **Proposed behavior** A partial index, `heartbeat_runs_pending_budget_cancellation_idx`, contains only runs with a pending budget Stop. The planner uses it for all four callers. The results are the same. **Reason and benefit** Local SQL measurements show a 300x to 1,000x decrease in time and buffer reads (see Verification). **Breaking changes** None. ## What Changed - Added the partial index `heartbeat_runs_pending_budget_cancellation_idx` on `heartbeat_runs (company_id)`. Its predicate is identical to `pendingBudgetRunCancellationPredicate`. - Added migration `0455_pending_budget_cancellation_index.sql`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`, and it is in the list of migrations that run outside a transaction, as `0436` is. - Moved the scoped run-control read and the idle-sleep check into exported builders in `budget-cancellation.ts`, next to the scope list query. The SQL does not change. The callers and the test now share the same statements. - Added a real PostgreSQL regression test. It checks that delivered, malformed, and missing intents are not pending. It also EXPLAINs the statements that the callers send (scope list with limit 1 and 100, scoped read, `EXISTS` check) and requires the new index in each plan. If a person changes a query shape, the server predicate, or the index predicate, the test fails. - Added `scripts/benchmark-budget-cancellation-scan.ts`. It captures the SQL that the real service sends, then compares the plans, timings, and complete rows with and without the index. - Updated `doc/DATABASE.md`. ## Verification Red/green regression test: - Before the migration, the new test failed: the plan was `Seq Scan` with the predicate as a filter. - After the migration: `npx vitest run src/services/heartbeat/budget-cancellation.test.ts` passes (17 tests). - Also passed: `src/__tests__/budgets-service.test.ts`, `src/__tests__/idle-sleep-diagnostics-postgres.test.ts`, `src/services/budget-resume-continuation.test.ts`. - `pnpm --filter @paperclipai/db check:migrations`, and typecheck of `@paperclipai/db` and `@paperclipai/server` pass. Performance and result equality (local SQL measurements only): ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-budget-cancellation-scan.ts --runs=20000 --payload-kib=16 [--reverse] ``` - Fixture: embedded PostgreSQL 18.1. 20,241 runs in 2 companies, with incompressible result JSON of about 16 KiB (table 379 MB). 40 pending stops in 6 scopes, 200 delivered or malformed decoys, and 1 cross-company intent. - The SQL is the same in both treatments. The script toggles only the new index. 2 warmups and 7 samples per query. Rows are compared outside the timed part. Ordered queries must match in order. The scoped run-control read has no `ORDER BY`, so it is compared as a multiset. | Query (caller) | Baseline median | With index median | Shared buffers | | --- | --- | --- | --- | | `listPendingBudgetCancellationScopes(db, 1)` (worker `hasPending`) | 688 ms | 2.1 ms | 243,500 → 1,158 | | `listPendingBudgetCancellationScopes(db)` (delivery) | 799 ms | 1.4 ms | 243,500 → 1,158 | | Scoped pending runs (run control) | 357 ms | 1.1 ms | 121,835 → 322 | | `EXISTS` pending (idle-sleep check) | 440 ms | 0.37 ms | 240,560 → 2 | - Reverse order (index first) gave the same result: 524 ms → 1.6 ms, 464 ms → 1.4 ms, 306 ms → 1.0 ms, 306 ms → 0.42 ms. - Plans change from `Seq Scan` (or a broad bitmap scan) to `Index Scan` or `Index Only Scan` on the new index. - All result rows are equal in every run. Migration duration: - 20,241 runs (379 MB): index build 2.0 s to 3.9 s. - 200,241 runs with 8 KiB result JSON (2.1 GB): index build 5.6 s. With this table size, the four queries change from 1,148 / 879 / 514 / 4,392 ms to 1.1 / 1.2 / 0.85 / 8.3 ms. - The build is concurrent, so it does not block run writes. ## Risks - Migration: the build reads every run's result JSON one time. It is concurrent and resumable (`IF NOT EXISTS`). The executor repairs an invalid concurrent index on retry. - Write cost: run writes evaluate the index predicate. The index contains very few rows, so the extra cost is small. - Predicate drift: if a later change edits only one of the two predicates, the planner cannot use the index. The new regression test detects this. - The hosted timings are not measured yet. The local numbers come from a synthetic fixture. ## 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> |
||
|
DottaandClaude Opus 5.5
|
ab80fea62f |
perf(recovery): probe run bindings per comment in the lost-wake check (#20412)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The ownership reconciler finds user comments whose wake a restart lost, and it saves a wake for them (plan invariant 4) > - Its lost-wake check matched a task's runs with one OR across the legacy context ID and the native issue column > - With that OR, PostgreSQL can join runs only by company. It then tests each recent comment against every run of its company and reads each run's context > - On a hosted staging database, this statement used about 23 s per call on average (384 calls, about 8,900 s in total). One call ran for 35 s. Other requests waited behind it > - This pull request reads the check in three stages and probes the two run bindings separately with existing indexes > - The benefit is the same result with bounded, index-driven work for each recent comment ## Linked Issues or Issue Description **What happened?** The reconciler's user-comment lost-wake check was one of the most expensive statements on a hosted staging database. Query statistics showed 384 calls with a mean of about 23 s. One active call was seen at 35 s. The database compute was busy, and short authentication reads waited behind long statements. The statement matched runs with `(context_snapshot->>'issueId' = issue OR native_issue_id = issue)` in three subqueries. The OR gives PostgreSQL no join key except `company_id`. A local plan on synthetic data showed the cost: a hash join of 273 comments against all 38,002 runs, which took 1.0 s of a 1.3 s statement. The work grows with recent comments multiplied by the company's run count, and each test reads a run context. **Expected behavior** Each recent comment uses a small number of indexed probes. The check returns the same comments in the same order, with the same limit and grace logic. **Steps to reproduce** 1. Seed a company with a deep run history and a few hundred recent user comments. 2. Let some recent comments have no wake (for example, after a restart lost the wake of a steered comment). 3. Run the check. PostgreSQL can pick a hash join on `company_id` with the OR as a join filter. With nested loops off, the old statement takes seconds. The local benchmark below shows this. **Paperclip version or commit** Baseline: `02bfe71a6b`. **Deployment mode** Server with PostgreSQL. Local proof uses disposable PostgreSQL 18.1. Production and staging use hosted PostgreSQL. Related: the earlier change that split native and legacy bindings for other ownership reads (see the "Execution recovery query performance" section in `doc/DATABASE.md`). A search found no open pull request for this check. ## What Changed - `checkUserCommentsWithoutWake` reads in three materialized stages: 1. Recent user comments on assigned, non-conversation tasks that no wake request carries. This stage keeps the same issue, status and agent-closed rules. 2. Of those, comments with no live or later run and with at least one earlier run. 3. Of those, comments that no live run acknowledged in an item event (the steer check). Only this stage reads event payloads. - Each run lookup is two probes: one on the legacy context ID and one on the native issue column. Both use existing indexes. `exists (A or B)` becomes `exists (A) or exists (B)`, which is logically the same. - Each probe ends with `offset 0`. This keeps it a per-row index lookup. Without it, PostgreSQL can build a hashed subplan over all runs, or a hash join over all item events of the company. The repository already uses this fence in `execution-blocker.ts`. - The final `order by c.created_at limit N`, the projection, the grace window and the reported reason do not change. - Add `scripts/benchmark-comment-wake-check.ts`. It loads the check from an explicit baseline commit and from the working tree, and it compares the complete service result and the complete positional SQL rows before it records time. - Add a regression test for each run binding on its own: native only, legacy only, live run, later run, steered run, an acknowledgement before the comment, a log-only mention, a run that ended before the comment, and a run in another company. - Document the check and the benchmark command in `doc/DATABASE.md`. - No schema change, index, migration, or new startup work. ## Verification - `pnpm exec vitest run server/src/__tests__/ownership-reconciliation-report.test.ts --maxWorkers=1`: 34 passed. This includes the existing lost-wake, restart, steered and agent-closed tests and the new binding test. - `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-comment-wake-check.ts --baseline-ref=02bfe71a6b --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 so contexts and payloads do not compress well. There are two companies per fixture. Two warmup rounds, then 7 timed samples for each arm, in alternating order. Each sample runs the real check function. Before any timing, each fixture asserts that the complete check result and every positional SQL row are equal for baseline and treatment, and that the fixture reports at least one owed comment. Every timed sample also asserts the same result. | Fixture | Runs / recent comments / owed | Baseline median | Treatment median | Speedup | Hash-join plan, baseline → treatment | | --- | --- | --- | --- | --- | --- | | small | 2,541 / 50 / 6 | 8.5 ms | 2.6 ms | 3.2x | 19 ms → 3 ms | | representative | 75,502 / 750 / 75 | 26.4 ms | 23.3 ms | 1.1x | 2,262 ms → 17 ms | | lost steer wakes | 38,002 / 750 / 75 | 454.7 ms | 66.9 ms | 6.8x | 2,717 ms → 68 ms | | deep history | 150,750 / 1,125 / 114 | 26.9 ms | 18.8 ms | 1.4x | 4,006 ms → 34 ms | | busy hour | 128,750 / 5,625 / 200 (limit) | 79.2 ms | 47.4 ms | 1.7x | 20,047 ms → 75 ms | | long steered runs (stress) | 19,100 / 600 / 60 | 626.4 ms | 457.4 ms | 1.4x | 5,871 ms → 502 ms | - The table above used baseline `02bfe71a6b`, the branch base at that time. After the rebase, a repeat run with baseline `8004ad069e` (the new merge base, with the helper guard fixed to run from the repository root) gave the same picture on a busier machine: small 15.4 → 9.1 ms, lost steer wakes 1,733 → 139 ms, busy hour 103 → 65 ms, all with equal results. - The "lost steer wakes" fixture shows the slow plan with default planner settings. PostgreSQL hashed all runs by company and tested 273 comments against 38,002 runs. - The last column runs each statement once with `enable_nestloop = off`. This makes the company-only hash join that the old statement allows visible, and it shows that the new statement has no such plan. The result is checked as equal in this mode too. Staging's 23 s mean matches this growth: recent comments multiplied by the company's run count. - Shared buffers fell in every fixture except "small" with default settings (for example, busy hour 176,416 → 89,803). - Neutral path: in the "long steered runs" stress fixture, every steered comment lost its wake. These comments correctly reach the steer check. That check must read the payload of each item event after the comment in the run that was live, so it is the remaining cost. In normal operation a comment sent to a live run has a deferred wake that carries it, so stage one removes it. ## Risks - Low risk. The predicate is the same conjunction of conditions. Only the evaluation order and the probe shape change, and the benchmark asserts complete-row equality on six fixtures and in both planner modes. - Rows with the same `created_at` have no defined order, as before. The benchmark uses distinct times so positional comparison is meaningful. - `offset 0` stops PostgreSQL from turning these probes into joins. If a task someday has a very large number of runs, each probe still uses the company and issue indexes. - Upgrade safety: this change reads saved run, wake and event state only. It changes no persisted shape, run identity, or saved session, so state saved by the previous release carries forward unchanged. A cutover test is not applicable. - Not verified on the hosted database. The local proof uses synthetic data, and I did not capture a hosted plan. Confirm with a fresh query statistics window after deployment. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file editing, 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 - [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
|
b2a7d322ed |
test: preserve runner commit stall acknowledgement invariants (#20383)
Assert that stalled event commits do not advance acknowledgement or committed state. Clarify the limits of historical failure attribution. Preserve the production implementation already merged in #20385. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
b09f17fbfd |
fix(agents): repair queued setup through task connection cards (#20361)
## Thinking Path > - Paperclip manages AI agents doing work for people. > - Agent verification must finish before an accepted task can start. > - A missing AI account default can prevent verification before a provider starts. > - Lifecycle policy treated that owned configuration refusal as a temporary failure and retried each minute. > - The accepted turn remained Queued without explaining the prerequisite. > - Park account configuration failures and reuse the task's existing inline connection card. > - Successful repair rechecks verification and releases the original queued turn under its original authority. ## Linked Issues or Issue Description **What happened?** An OpenCode agent selected the responsible user's OpenRouter default, but that default did not exist. Verification retried unchanged configuration indefinitely and its accepted task stayed Queued. **Expected behavior:** Preserve accepted work, explain the setup prerequisite, and let the appropriate person repair it using the existing task connection UI. Completion must verify the agent before starting the same queued turn. Genuine pause, approval and access gates remain effective. **Reproduction:** Create an OpenCode Runner agent with a responsible-user OpenRouter binding and no personal default; assign a task while verification is pending. The base retries verification and shows an unexplained queue. Related: #18702 handles pending task connection recovery after account repair. This PR handles the earlier agent-verification boundary. #20330 addresses a separate Codex governed-wait timeout and is already merged. ## What Changed - Share owned account-selection failure evidence through a pure library contract. Lifecycle policy parks those configuration failures with fixed, redacted guidance; temporary infrastructure errors retain their retries. - Batch setup-state reads for unstarted queued turns. Distinguish manual repair from automatic setup retries and preserve Stop, started/history state and lifecycle holds. - Reuse the existing `connection_intent` card, account selection and completion APIs. Its controller-only `agentSetup` marker accepts only never-started legacy turns; runtime tokens still require a running run. - Address the card to the lifecycle verifier's account owner, preserving the task's execution principal and private-task visibility. Attach missing cards on admission, verification, startup and full recovery, including retained retries and continuations. Repeated delivery and publication remain bounded. A new queued turn supersedes the setup card of a stopped turn; generic connection cards retain cross-run reuse. - Recheck lifecycle verification after usable-account completion, then release the existing turn through the normal readiness path. Preserve decline, Stop, reassignment and changed setup-owner gates. Fence an in-flight verification failure from overwriting a repair; add no provider wake or recurring worker. - Park owned account refusals during resume as well as initial verification. Bind setup cards to the lifecycle operation, settle obsolete deliveries, and keep explicit Retry guidance accurate. - Reuse the router’s existing lifecycle pin for setup repair without rerouting the task, advancing the pool cursor, or replacing its saved binding. Legacy direct-account cards remain readable. - Hide the incorrect Resume banner during setup, retaining genuine manual/budget pause controls. Document the execution and retry contract. ## Verification - Red: eight original behavior assertions failed on the base, including the real selector's retry decision and the real batched queue projection. Six owned selection cases failed classification. Temporary-error presentation, different task/verifier users, retained retry/continuation recovery and the setup Resume banner also have failing-before/passing-after regressions. - Green: 430 affected backend, lifecycle, shared-contract and UI tests passed before the final review corrections. Subsequent focused runs cover one inline card, real account selection/completion, duplicate delivery/publication, verification before readiness, unchanged run authority, private tasks, no extra provider wake, decline/Stop/reassignment, company isolation, invalid runtime tokens, transient retries and redacted diagnostics. - Final setup/card and generic connection-service suites: 49 tests passed. The stopped-turn case failed before the correction, then passed with real account completion, the stopped source remaining cancelled, and the replacement source remaining queued. - After rebase, all 129 focused assertions passed. The local queue suite’s 88 execution assertions passed repeatedly, but its database-fixture teardown exceeded both 30-second and 120-second hooks. An isolated queue case completed, and the setup/connection suites fully passed. The original queue test and cleanup remain unchanged; broad CI supplies the full-suite result. - Server/UI typechecks, module boundaries, token gates and diff checks passed. `pnpm verify --explain` requested broad CI because the execution-contract document changed; no redundant full local suite was substituted. - Real source acceptance: macOS arm64, Node 26.4.0, isolated embedded PostgreSQL and local board identity. Created an OpenCode/OpenRouter agent and task through the API with no default. Admission retained one unstarted turn and produced the existing inline Connect OpenRouter card. The browser opened its credential form and closed it without entering credentials. The same card/run persisted after server restart and reload, with the setup label and no incorrect Resume. The browser also reproduced Stop followed by a new queued turn retaining the stale card on the prior head. On `12f609c98372fd1e50644028db75cfcc8deb210d`, startup superseded the stale card and retained the same replacement turn, with the original turn still cancelled and no provider execution. - The earlier CI case-search failure also reproduces on exact base `0e3fdebea8257bdff11573081c76b306f12db230` without this PR. Its repair is now in master via #20371; this branch is rebased onto it and contains no API-search changes. - Staging's actual Runtime test returned “Connect an account and choose your personal default” before provider startup. The original turn later started independently on deployed code, then hit a separate workspace-export failure. Automatic file recovery subsequently marked that run successful; its task remains Blocked and the trial’s useful output is unverified. This PR is not deployed; provider authentication through its card and downstream publication remain unverified. No account, credential, default, permission or binding was changed during diagnosis. - Previous revision review: Greptile 5/5 on `12f609c98372fd1e50644028db75cfcc8deb210d`, with all four inline findings addressed and resolved. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42713494508) on that revision: 59 successful checks, one neutral check and three intentional skips, with no pending or failed checks. - Latest revision `976bcae993805a8a2222395bbff9394f6791b424`: resume, router and Retry-copy regressions failed before repair. Green: 92 lifecycle/setup/shared tests, 83 setup/generic-connection/router tests, and 137 UI/shared tests. Server/UI typechecks, module boundaries and token gates passed. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42720731137) on this exact revision: 59 successful checks and three intentional skips, with no pending or failed checks. Greptile reviewed this head at 5/5; all six review findings are addressed and resolved. Real Stop/retry acceptance retained the same replacement turn and current-operation card through two actual server restarts, with no provider execution. ## Risks - Known account configuration failures now require repair or explicit Retry. Temporary failures keep automatic retries; accepted work and authority remain intact. - The projection adds one company-scoped batch read only for unstarted queued turns. Card attachment examines at most 100 active candidates and adds no timer, worker, schema or migration. - The optional stored setup-card marker requires an aware reader during rollback. No run/session/checkpoint identity format changes. Setup presentation grants no access and cannot bypass verification or other gates. ## Model Used - OpenAI GPT-6 (Codex), with reasoning, tool use, code editing and test execution. The exact serving model identifier 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 Opus 5.5
|
c985541a57 |
perf(db): index retry-exhaustion receipts for the attention scan (#20386)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The attention feed shows runs that failed after automatic retries stopped > - To find these runs, the feed reads every `heartbeat_run_events` row of the company and filters the rows by a message prefix > - Run output makes this table very large, so each feed request does a large scan > - On a hosted test database, this one statement used the most total database time: 923 calls at a mean of 5 s (4,709 s in total) > - Request authentication waits behind this background work, so all users feel the load > - This pull request adds a small partial index that holds only the retry-exhaustion receipts > - The benefit is that the same query reads a few index entries instead of all company events ## Linked Issues or Issue Description **What existing behavior does this improve?** The attention feed query that finds retry-exhausted runs (`listAttentionExhaustedRuns`). **Subsystem affected** Server attention feed and the `heartbeat_run_events` table schema. **Current behavior** The query reads all company rows in `heartbeat_run_events`, filters them by `event_type = 'lifecycle'` and the `Bounded retry exhausted` message prefix, and then joins the few matches to failed or timed-out runs. The cost grows with all run output. It does not grow with the number of exhausted runs. **Proposed behavior** A partial index holds only the matching receipts. The query plan reads the index and joins a few rows. The query text and the result do not change. **Reason and benefit** The scan was the most expensive statement by total time on a loaded hosted database. A local reproduction shows a 54x faster query and about 116x fewer buffer reads. **Breaking changes** None. The API, the feed rows, and their order do not change. ## What Changed - Add the partial index `heartbeat_run_events_retry_exhausted_idx` on `(company_id, run_id, id DESC)`. The index covers only rows where `event_type = 'lifecycle'` and `message like 'Bounded retry exhausted%'`. - Add migration `0454`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration runner runs it as a top-level statement, the same as migrations 0432, 0436 and 0450. - Add a regression test. It pins the row order (newest run first, then the newest receipt on a tie) and the latest receipt per run. It also checks that the query plan can use the new index. If the query predicate changes so that the index no longer applies, the test fails. - Add `scripts/benchmark-exhausted-run-attention.ts`. It measures the same query with and without the index, and the time to build the index. - Document the index in `doc/DATABASE.md`. ## Verification Cause and thesis. The scan cost comes from reading all company events, not from the few exhausted runs. Thesis: an index that holds only the receipts makes the query cost depend on the number of receipts, and keeps the result identical. The benchmark below confirms the thesis. Rejected alternative without a migration. I first tried a LATERAL lookup that starts from the failed and timed-out runs. On the same fixture it was about 2x slower (median 379 ms against 194 ms). It reads the full event history of each failed run with random I/O, and that costs more than one sequential scan. Local SQL benchmark (the SQL that the service generates; same SQL with and without the index): ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-exhausted-run-attention.ts --runs=30000 --events-per-run=100 --samples=9 ``` Fixture (synthetic): one company with 30,000 runs (3,819 failed or timed out) and 3.0 M events, plus a decoy company with 0.5 M events. The table is 1.4 GB. About 5% of events are `lifecycle` events. Events of 20 concurrent runs are interleaved. 1 in 23 runs has a receipt, some runs have two receipts, and output decoys contain the prefix. Two agents are terminated. Embedded PostgreSQL 18.1 on an Apple laptop. The hosted database is Neon with different compute, so the absolute times are local SQL measurements only. The script runs two cycles (without index, then with index; then with index, then without index) to expose cache bias. Each measurement has 2 warmups and 9 samples. | Query | Without index | With index | Samples and spread | Output check | | --- | --- | --- | --- | --- | | `listAttentionExhaustedRuns` | median 157 ms (96–1,271 ms) | median 3.3 ms (2.2–5.7 ms) | 18 each | 133 rows, positional rows equal in all 4 measurements | | Buffers (EXPLAIN ANALYZE) | 127,743 blocks | 5,580 blocks | | plan uses the new index (asserted) | Speedup: 48x (median), on head `ae6037b2a0` (the same SQL and index as the final head). The machine was under heavy load from other work, so the spread without the index is wide. An earlier run on the first head gave 248 ms against 4.6 ms (54x). Index build (`CREATE INDEX CONCURRENTLY` from the migration file): 1.7 s and 2.5 s for the 1.4 GB table. Index size: 152 KiB. Review fix: the query now writes `event_type = 'lifecycle'` as a SQL literal, not a bound parameter. With prepared statements, PostgreSQL can reuse a generic plan. A generic plan cannot prove that a bound parameter matches the index predicate, so it would not use the index. Tests: - `cd server && npx vitest run src/__tests__/attention-service.test.ts`: 44 passed. The new test checks a fresh plan and a forced generic plan (`plan_cache_mode = force_generic_plan` with `PREPARE`). It failed with the bound `event_type` parameter and with a changed message prefix (confirmed red), and it passes with the fix. Locally, the file's `afterAll` database cleanup hook timed out on a heavily loaded machine (load average about 28). The timeout also occurs when the new test is excluded. - `pnpm --filter @paperclipai/db check:migrations`: passed. - `pnpm --filter @paperclipai/db typecheck` and `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm verify`: see the CI results for this head. ## Risks - Migration time. A concurrent index build reads the full `heartbeat_run_events` table and does not block writes. Locally, the build took 1.2–2.5 s for 1.4 GB. The time grows linearly with the table size. I did not time the build on a production-size clone. On a database with a 5 s full company scan, I estimate the build at tens of seconds, which is in the 1–2 minute budget. This estimate is not proven. - If the build stops, PostgreSQL can leave an invalid index. The migration runner repairs invalid concurrent indexes when it runs again, and `IF NOT EXISTS` makes a rerun safe. - The index predicate must stay textually equal to the query predicate. If someone changes the message prefix or binds a predicate value as a parameter, the planner cannot use the index. The new test catches both cases, for fresh and generic plans. - Each new receipt writes one more small index entry. Receipts are rare, so the write cost is very small. - The hosted improvement is not verified until deployment. After deployment, compare a new query-statistics window for this statement. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). Standard context window and reasoning mode. ## 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
|
c57cd1bdb9 |
perf(server): read plugin lifecycle inbox heads in one pass (#20405)
## 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. > - The inbox page query checked each unacknowledged event against all older events of the same resource. Each check walked the acknowledged history of that resource. > - Agent runtime streams get a new event for each configuration change, so one resource can have thousands of acknowledged events. The cost grew as pending events times acknowledged history. > - On a hosted instance, three copies of this read ran for 601 seconds each, and one earlier copy ran for 19 minutes. The delivery worker runs it again after each acknowledgment. > - This pull request selects the head of each resource in one pass and keeps the same result. > - The benefit is that the page read is linear in the company journal size. In local tests it is 250x to 660x faster, and the result rows are equal. ## Linked Issues or Issue Description **What happened?** The plugin lifecycle inbox page query (`select "id", "company_id", "resource_type", "resource_id", "action", "created_at" from "resource_lifecycle_events" where ...`) ran for 601 seconds on a hosted database. Three copies were active at the same time. **Expected behavior** A lifecycle page read must stay fast when a resource has a long acknowledged history and a pending backlog. **Steps to reproduce** 1. Give a company 40 agents with 1,500 `agent_runtime` events each. 2. Acknowledge the first 1,200 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 10 seconds for 60,000 events of that company. At 200,000 events it takes about 6 minutes. **Paperclip version or commit** `02bfe71a6b` (master). **Deployment mode** Hosted (PostgreSQL). Local repro uses embedded PostgreSQL 18. ## What Changed - `server/src/services/plugin-lifecycle-inbox.ts`: `list` now selects one head per resource with `DISTINCT ON (resource_type, resource_id)`. The head is the unacknowledged creation, else the oldest unacknowledged event. Then the query applies `afterId`, id order and the limit. This is the same set that the old `firstForResource` predicate admits. `acknowledge` keeps the old predicate for its single-event check. - `plugin-lifecycle-inbox.ts`: new `hasPending(companyId, types)`. It returns whether any unacknowledged event exists. Each resource with an unacknowledged event has a head, so this gives the same answer as `list(companyId, 1).length > 0`. - `server/src/services/plugin-lifecycle-delivery.ts`: the idle-sleep journal check uses `hasPending`. This keeps its early exit. A one-row `list` page would read the full journal. - `server/src/__tests__/resource-lifecycle-events.test.ts`: new database test for heads across acknowledged history, a creation recorded after transitions, acknowledged gaps, shared ids across resource types, per-plugin progress, another company, `afterId` pages, limits, `hasPending`, and the next head after an acknowledgment. The test also passes on the old query, so it locks the contract. - `scripts/benchmark-plugin-lifecycle-inbox.ts`: new benchmark. It loads the baseline service from git, runs both on the same fixture, alternates order, asserts equal rows, and prints plans. - `doc/DATABASE.md`: describes the read cost and the benchmark command. - No schema change, index or migration. ## Verification Thesis: the old read cost is (unacknowledged candidates) x (acknowledged older events per resource), because each candidate walks the acknowledged prefix of its resource. One pass that keeps the per-resource head removes that product and returns equal rows. Baseline plan at 220,080 events: 40,020 candidates each walked about 3,999 older rows, which gave 160,039,980 acknowledgment probes and 562,140,833 buffer hits. The treatment plan reads the journal once with a hash anti join and sorts the 40,020 unacknowledged rows (3,925 buffer hits). Local SQL timings (embedded PostgreSQL 18, Apple Silicon; event counts include a 20,000-event second company, synthetic data, warm cache, one warmup, order alternates): | Fixture (events / acknowledged) | Case | Before median | After median | Samples (after) | Speedup | | --- | --- | --- | --- | --- | --- | | 40,060 / 16,040 | page of 100, all types | 3,263 ms | 13.1 ms | 3 (12.0-14.1) | 250x | | 80,080 / 48,060 | page of 100, all types | 10,137 ms | 23.9 ms | 5 (22.7-32.6) | 424x | | 220,080 / 160,060 | page of 100, all types | 355,718 ms | 538 ms | 3 (72-785) | 662x | | 1,020,080 / 960,060 | page of 100, all types | not run (est. > 30 min) | 1,682 ms | 7 (1,203-2,384) | - | | 80,080 / 48,060 | page of 100, agent and project only | 1.5 ms | 1.6 ms | 5 | neutral | | 80,080 / 48,060 | idle-sleep check | 14.5 ms | 13.8 ms | 5 | neutral | | 1,020,080 / 960,060 | idle-sleep check | not run | 406 ms | 7 | - | - Result rows are equal (`assert.deepEqual` on all mapped rows, in order) for each case where both versions ran. - The agent runtime revision read in `plugin-agent-runtime-state.ts` and `agent-file-store.ts` (`select id ... order by id desc limit 1`) uses a backward index-only scan and reads 5 buffers at 1M events. It is not changed. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=02bfe71a6b --agents=40 --per-agent=1500 --acked=1200 --samples=5 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=02bfe71a6b --agents=40 --per-agent=25000 --acked=24000 --samples=7 --treatment-only cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/agent-runtime-hooks.test.ts ``` - `resource-lifecycle-events.test.ts`: 28 passed. `plugin-lifecycle-delivery.test.ts`: 40 passed (includes idle-sleep "none" and "present"). `agent-runtime-hooks.test.ts`: 22 passed. - `tsc --noEmit -p server`: pass. ## Risks - The page read is still linear in the company journal size, because acknowledgments are per plugin and the journal has no retention yet. At 1M events a page takes about 1.7 s locally. Each acknowledgment wakes another full pass. A per-plugin cursor or retention can remove this later. - The planner can choose a different plan on the hosted database. The local plans use a hash anti join and one sort. The hosted improvement is not yet measured. - `acknowledge` still checks one event against older events of its resource. That check is linear in that resource's history per call, as before. - Low behavior risk: the new test passes on both the old and new queries. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). Extended reasoning on. ## 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
|
199670ba99 |
fix(accounting): prevent usage receipt stalls during streamed run updates (#20385)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner commits usage receipts before it acknowledges retained events. > - Usage receipts and streamed run logs update the same run row. > - A repeatable-read snapshot can become stale before the receipt writer locks that row. > - Continuous log writes can exhaust all accounting retries and stall the retained event. > - This pull request uses the existing run lock with READ COMMITTED for standalone receipt writes. > - Other accounting operations keep their consistent snapshot. Stall diagnostics retain safe nested error codes. ## Linked Issues or Issue Description Related: #20235. **What happened?** A hosted native run stopped on a retained usage event. Its diagnostic reported `Error at source seq N`. The saved outbox and accounting records locate the failing delivery boundary. Traces show repeated accounting transactions while streamed events still updated the run row. The historical inner exception is unavailable. A controlled PostgreSQL reproduction proves a matching defect: a run-log append after company validation makes the receipt writer's repeatable-read snapshot stale. Repeated appends exhaust all four retries with SQLSTATE `40001`. This is a demonstrated failure mechanism; attribution of the historical incident remains an inference. **Expected behavior** Commit the receipt under its existing run-row lock despite concurrent run-log writes. Observe concurrent finalization and preserve late-receipt debt. Keep duplicate receipts idempotent and reject conflicting or cross-company receipts. **Steps to reproduce** Run `server/src/services/usage-receipts-read.test.ts`. The concurrency regression uses real PostgreSQL and the production run-log append function. It commits a log append after company validation and before the receipt locks the run. Before the repair, the receipt rejects with a wrapped `40001` after four attempts. After the repair, it commits once. The finalization variant verifies that a newly settled run receives late-receipt debt without replacing settled state. ## What Changed - Allow aggregate-lock-protected accounting writers to select READ COMMITTED. Keep repeatable read as the default. - Use READ COMMITTED only for standalone usage receipt writes. Keep the existing run lock, immutable receipt journal, replay checks and transactional budget-work publication. - Add PostgreSQL regressions for concurrent log appends, concurrent finalization, duplicate delivery and the unchanged default snapshot. - Read the outer event-commit error and at most three nested causes for an existing safe error code. Exclude exception messages and SQL. - Document receipt isolation and bounded stall diagnostics. ## Verification - Red: the production receipt writer exhausted four retries with PostgreSQL `40001` under controlled concurrent run-log appends. - Red: wrapped database and transport stall diagnostics returned `Error` instead of `40001` and `ECONNRESET`. - Green: Runner control-plane tests passed all 207 cases. All 68 server assertions passed across receipt reads, native usage delivery and cost accounting. Cost-accounting teardown exceeded its 30-second limit twice on this loaded host, after all 28 assertions passed. The workspace project retained its configured 30-second hook limit despite the CLI override. This is a local suite cleanup failure, not a passing suite; the same revision passed the complete server suites in CI. - Server layered TypeScript checking passed, including source and test layers. Runner TypeScript checking passed. Server checks cover `ccfe3bdeb1ab88537a03fd6af9044f66863939a5`; the unchanged Runner checks cover its parent. - Local environment: macOS arm64, Node 26.4.0, disposable embedded PostgreSQL. Native usage-delivery tests use the real Rust receiver. - CI at `ccfe3bdeb1ab88537a03fd6af9044f66863939a5`: [build](https://github.com/paperclipai/paperclip/actions/runs/42714760702/job/129493568428), [typecheck](https://github.com/paperclipai/paperclip/actions/runs/42714760702/job/129493568367), native Runner integration, browser shards and all nine serialized server groups passed. All 12 general server shards passed in the [full run](https://github.com/paperclipai/paperclip/actions/runs/42714760702). Final rollup: 59 successful checks/statuses, three intentionally skipped checks, no pending or failed checks. - [Greptile reviewed this revision at 5/5 with no actionable findings](https://github.com/paperclipai/paperclip/pull/20385#issuecomment-6753163397). - Hosted evidence came from the task, run, saved sandbox outbox, accounting receipts and traces. The run started fresh and did not cross a deployment. No original agent turn was replayed. - The hosted repair has not been deployed. The original task has not been resumed. A fixed-revision remote-provider user journey remains unverified. ## Risks - READ COMMITTED changes snapshot timing for standalone receipt writes. The run-row lock protects all mutable receipt/accounting reads and serializes competing receipts and finalization. Other accounting operations retain repeatable read. - Receipt identity, replay conflict checks, company scoping, budget publication, retry bounds and acknowledgement rules remain intact. - No persisted format, provider identity or database schema changes. Existing saved state needs no migration. - Diagnostics inspect at most four objects and retain only the existing identifier format, with an 80-character code limit. - The reproduced race matches the incident boundary and timing, but the original inner exception was not recorded. Rollout and live acceptance remain necessary before incident resolution. ## Model Used - OpenAI Codex, GPT-6 family, with code and shell tools. The exact model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
bf818280f2 |
perf(workspaces): reserve cohort members concurrently (#20401)
## Thinking Path > - Paperclip controls agent work and preserves its workspaces. > - The online importer moves up to eight shared-source leaves under one source fence. > - Each member must have a durable reservation before capture starts. > - Serial reservations repeat the same validation cost before any archive work. > - This pull request reserves members in parallel through their existing independent transactions. > - Every reservation finishes before an error can release the fence or start capture. ## Linked Issues or Issue Description Refs: #19873, #20387, #20394. **Current behavior** A measured six-member cohort spent about 110 seconds in serial reservations. Each reservation keeps its existing source, task, recovery and journal checks. **Proposed behavior** Start the bounded member reservations together. Join all outcomes, retain member order, and refuse capture if any member fails. Keep the exact partial-journal resume contract. **Reason and benefit** This removes serial metadata work before capture. Hosted speedup has not been measured for this change. ## What Changed - Replace only the cohort reservation caller loop with `Promise.allSettled`. - Preserve the existing per-member transaction, preassigned operation ID, source fingerprint and all source fences. - Extend the real PostgreSQL failure fixture to require overlapping reservations and complete peer closure before fence release. - Document the unchanged partial-journal and atomic publication contract. ## Verification - Canonical cached worktree preflight and plugin SDK prerequisites passed on Node 24.22.2. - The unchanged production baseline failed the new reservation overlap assertion: one reader entered instead of two. The canonical PostgreSQL wrapper cleaned up. The other 21 tests were intentionally not selected. - The first baseline command failed before PostgreSQL because root-level `tsx` resolution was absent. The corrected command uses the checkout's explicit `cli/node_modules/tsx/dist/loader.mjs`. - The four focused PostgreSQL controls passed before the final rebase: overlap/join failure, exact partial-journal resume, parallel capture with both cold restores, and atomic outer-commit rollback. Eighteen tests were intentionally not selected. Server TypeScript passed. - The final branch is based on merged #20394 at `302ada90dfda0e8fbcd8a514d506d310c9843bcb`. The two directly affected PostgreSQL reservation/resume controls passed again on final head `06d436e45705da3b616a2b91725960f6e0ea52c0` (2 passed, 20 intentionally not selected; 100.56 seconds). Server TypeScript passed again. The canonical wrapper completed cleanup. - The separate Ponytail review remains 4/5. Its nonblocking fan-out warning was resolved as the documented phase contract; no source change was required. Optional multi-error aggregation is outside this change. - Fresh Greptile review is 5/5 on `06d436e45705da3b616a2b91725960f6e0ea52c0`, with no actionable findings or unresolved threads: https://github.com/paperclipai/paperclip/pull/20401#issuecomment-6754025480. - Exact-head CI is complete: 56 success, 4 skipped and 1 neutral, with no failed or pending checks. Aggregate `ci / verify` passed: https://github.com/paperclipai/paperclip/actions/runs/42718439355. - Reproduce the final focused cases on Node 24: `node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts -t "joins concurrent reservation validation readers|resumes the exact membership after reservation stops halfway"`. Run `pnpm --filter @paperclipai/server exec tsc --noEmit` for TypeScript. ## Risks - API phase contract: `concurrency` limits capture workers. Preflight, reservation and post-finalizer validation fan out over the exact selected member set, with the canonical maximum of eight. The original DB pool bounds connections. `concurrency: 1` does not promise one metadata query or transaction at a time. - Up to eight reservations can use independent pool connections at once. Members that pass can leave durable journals when a peer fails. The existing exact-member resume path retains those journals; this change does not retry automatically. - Capture and publication remain blocked until every reservation settles. Publication stays serial in one transaction. Saved journal schemas, singleton behavior and recovery proof checks do not change. - This does not change concurrency inside the publication transaction. One PostgreSQL connection serializes that SQL, and changing its row-lock order has no measured benefit here. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model identifier 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
d95fdd8d79 |
feat(observability): span the unnamed gaps in run preparation (#20396)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a task reaches the agent, the server prepares the run: it reads the task, the workspace and the configuration, and then hands the run to the native controller > - With an OTLP endpoint, each preparation step emits a `heartbeat.prepare.<phase>` or `native.controller.<phase>` span > - On a hosted test deployment, 0.3 to 0.5 s of each `task.prepare` sits between those spans, and about 0.5 s more sits between `execution.dispatch_admission` and `native.coordinator.claim` > - Nobody can say which step is slow in that time, so latency work in this area is guesswork > - This pull request adds spans around the existing awaited work in those gaps, with the existing timing helpers and naming conventions > - The benefit is that a trace of a slow preparation now names the step that used the time ## Linked Issues or Issue Description **What existing behavior does this improve?** The OpenTelemetry trace of task preparation. Today it has unnamed gaps between the named preparation spans. **Subsystem affected** Server: heartbeat run preparation and the native controller (`server/src/services/heartbeat.ts`, `server/src/services/native-runtime/native-session-executor.ts`). **Current behavior** On a hosted test deployment (8 runs per group), `task.prepare` takes 2.6 to 3.4 s. Between `heartbeat.prepare.runtime_state` and `heartbeat.prepare.run_identity`, about 0.25 s has no span. Between `execution.dispatch_admission` and `native.coordinator.claim`, about 0.5 s has no span, and in one pair of runs it was 1.4 s. Nobody can tell which step used that time. **Proposed behavior** Spans name the existing awaited reads and writes in those gaps. The change adds no reads and records no content. The operator endpoint gate and the no-operation behaviour stay. **Reason and benefit** Latency work in this area needs a measurement. With these spans, a trace of a slow preparation names the step that used the time, so a fix can be confirmed or rejected after deployment. **Breaking changes** None. The change adds span names to the closed list. It changes no API, schema or behaviour. **Additional context** The server uses postgres.js, which the auto-instrumentation does not cover, so database spans do not fill these gaps. ## What Changed - `server/src/services/execution-preparation-timing.ts`: add the new closed phase names. - `server/src/services/heartbeat.ts`: add `heartbeat.prepare.*` spans for `chat_retry_admission`, `workspace_selection`, `dependency_readiness`, `issue_checkout`, `instance_settings`, `workspace_context`, `project_context`, `plan_wake_routing`, `routine_environment`, `responsible_user` and `execution_start`. The re-reads of the issue after a checkout use the existing `issue_context` name. - `server/src/services/native-runtime/native-session-executor.ts`: add `native.controller.*` spans for `attachment_staging`, `warm_restore`, `span_persistence` (persistence of the preparation spans to the run log) and `state_migration` before the coordinator claim. - `doc/observability.md`: list the new spans. - `server/src/__tests__/heartbeat-preparation-gap-spans.test.ts`: run a real heartbeat through native preparation with an in-memory span exporter. Check that each new span is emitted and that the new preparation spans are children of `task.execution_attempt`. ## Verification - `pnpm --filter @paperclipai/server typecheck` - `npx vitest run src/__tests__/heartbeat-preparation-gap-spans.test.ts` (fails without the source change, passes with it) - `npx vitest run src/services/heartbeat/preparation-timing-context.test.ts src/services/execution-preparation-timing.test.ts src/services/native-runtime/native-session-executor.test.ts src/__tests__/heartbeat-context-compaction-execution.test.ts` ## Risks - Low risk. The change only wraps existing awaited calls. Values and errors pass through the timing helper unchanged, and a checkout conflict still reaches the same `catch`. - Phases of 50 ms or more inside `heartbeat.prepare.*` are also written to the run log, as for the existing phases. This adds a few run-log rows on slow runs. - Without an OTLP endpoint, the helpers keep their current no-operation behaviour. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs) and extended thinking. ## 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> |
||
|
Nicky LeachandPaperclip
|
1838de8c6c |
Pause agents automatically when moving to a plugin environment (#20375)
## Thinking Path > - Paperclip manages AI agents and their execution environments. > - Core owns agent lifecycle state and controls when work can run. > - A plugin environment can need a computer that belongs to the agent. > - Moving a ready agent to that environment currently requires manual pause and resume actions. > - Saving the environment is sufficient intent to perform those routine steps. > - This change records the pause with the environment update and uses the existing lifecycle worker. > - Work can resume only after execution stops, the plugin starts its resources, and verification passes. ## Linked Issues or Issue Description Related: #19173 introduced environment-scoped plugin selection and the manual-pause requirement. I searched for related pull requests and found no duplicate fix. No separate issue is linked. **What happened?** Saving a ready agent with an environment whose lifecycle plugin is not selected returns a conflict. The error tells the user to pause the agent, change its environment, and resume it. **Expected behavior** Saving the environment change starts the required pause, resource setup, and verification automatically. Existing manual, budget, and company holds still block resume. **Steps to reproduce** 1. Use an agent in `ready` on an environment without an agent-owned plugin. 2. Select an agent-owned plugin environment in the agent configuration. 3. Save without first pausing the agent. Before this fix, the save fails. With this fix, the agent moves through `pausing`, `resuming`, and `verifying` before `ready`. ## What Changed - Save the new environment and automatic pause in the same transaction. Keep the existing plugin selection during stop. - Use the existing completion of a pause without holds to resume automatically. Set the saved resume target to `verifying` so the new environment must pass its test. - Preserve independent pause holds, lifecycle version checks, notifications, resource status events, and worker recovery. - Add migration tests for idle and running agents, real process-stop proof across a worker restart, added holds, failed verification, and route authorization. - Update the lifecycle and development docs. Keep the feature maps unchanged, as required for a small behavior fix. ## Verification - Current revision: `300f57539bce9b198406c3793ab3c2de46492659`. - Lifecycle and case API discovery suites passed after updating the branch to master. The final environment-plugin suite passed all 22 tests, including four real-route authorization cases for users and agents. Coverage includes real process-stop proof across worker replacement, independent holds, and failed migration verification. - Server typecheck: passed on the current base. All workspace typechecks and `pnpm build` passed before the base update; fresh CI covers the updated base. - `node scripts/check-module-boundaries.mjs`, `node scripts/check-maps.mjs`, and `git diff --check`: passed. - Reviewed the diff for secrets, personal data, private addresses, and run artifacts before each push. - [Current CI](https://github.com/paperclipai/paperclip/actions/runs/42714733533): passed on `300f57539bce9b198406c3793ab3c2de46492659` (52 successful jobs; the optional test-selection optimization was skipped). Standard Greptile: 5/5 on the current revision, with no unresolved findings. The security scanner accepted the documented configuration-authority policy. - Earlier CI failed the `case-history-api-discovery` assertion for the query `case for task`. It also failed locally. The branch now includes master's fix #20382, and that test passes. The migration implementation did not change. The superseded CI attempt was cancelled after recording the failure. - Full local `pnpm test:run`: incomplete; stopped after two passing chat partitions. The focused suites passed, and required CI supplies broad coverage, as specified in the repository instructions. - Local target: isolated source checkout on macOS ARM64, Node 26, disposable PostgreSQL, fixture lifecycle plugin. Initial database startup failed because the bundled macOS PostgreSQL package lacked dylib aliases. Temporary dependency aliases repaired startup; the focused database tests then passed. The installed Rust toolchain also needed to be added to PATH. - The pending-stop test retains a live process ID in the cancelled run. The real host stop check blocks plugin setup. After stop acknowledgment and worker replacement, the saved migration reaches readiness. ## Risks - Saving this environment change can stop current execution. The system performs that pause as part of the requested migration. - A failed stop, plugin call, or verification keeps the agent blocked and uses existing retry behavior. - Editing a shared environment or the instance default still uses its existing conflict checks. This change covers the individual agent environment update. - Configuration permission authorizes the temporary system pause needed for the resource move. Separate manual-pause authority is not required for this configuration operation. Real route tests cover authorized and denied users and agents. - No schema migration or deployment is included. A fixture plugin does not prove live computer provisioning or model execution. ## Model Used OpenAI Codex, based on GPT-6, with code execution and tool use. The exact runtime model ID and context limit 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> |
||
|
Nicky LeachandPaperclip
|
2550f399d9 |
fix(lifecycle): align stop proof for pre-dispatch resource waits (#20384)
## Thinking Path > - Paperclip manages AI agents and controls when their work can run. > - An agent pause waits until every earlier run has stopped. > - Recovery already treats verified workspace and AI-capacity waits as pre-dispatch outcomes. > - The stop proof did not accept those same receipts. Historical waits could keep an agent pausing forever. > - This pull request shares the resource-wait classification and the existing execution-event check. > - The benefit is consistent stop evidence without a new lifecycle command, database backfill, or host-boot assumption. ## Linked Issues or Issue Description Refs #18147 and #17718. This fixes the verified resource-wait case of #18147. It does not certify ambiguous historical runs that have no positive non-execution evidence. An agent can have an old cancelled legacy run with `errorCode: workspace_busy`, `executionStage: preparing`, and `executionRecovery: { kind: workspace_wait, providerWorkStarted: false }`. No process, native coordinator, or retained resource exists. Recovery recognizes this as a resource wait, but pause previously required a stop acknowledgment from an executor that never started. ### Credit and related work Thanks to @br1an-c0des (#17692) and @crownlayerapp (#15383) for the focused preparation and shared-admission analysis, @lucasweismann-ai (#18072) for the durable dispatch-evidence and race analysis, and @Pdesengrini (#18905) for the cancellation-receipt and process-identity guard cases. We reviewed these alternatives before implementing this smaller change. No contributor commits were cherry-picked. The design uses receipts already written by core. It does not adopt a reboot-time bypass, a new adapter-entry journal, or a pause-abort route. #17236 and #20037 address the separate cost of history reads. ## What Changed - Add one shared classifier for matching workspace and AI-capacity wait receipts. Use it in legacy recovery and cancellation stop proof. - Reuse the existing dispatch-event predicate instead of maintaining another event list. - Retain checks for native execution references, unresolved workspace obligations, controller leases, and resource cleanup. A live controller or cleanup still in flight cannot pass the proof. - Add a focused PostgreSQL suite. It drives the real pause worker with historical wait records and checks Retry eligibility against the same evidence. - Document the contract. No schema, API, UI, dependency, or scheduling change is required. ## Verification Source checkout on macOS arm64, Node 26.12.0, isolated local PostgreSQL 18, synthetic data only. ```sh node node_modules/vitest/vitest.mjs run --root server \ src/modules/agent-lifecycle/__tests__/resource-wait-stop.test.ts \ src/services/legacy-execution-recovery.test.ts ``` - 61 tests passed across these two files. - All eight positive regressions failed on unchanged master. The fix restores them. - After rebasing onto current master, the 61 focused tests passed again. Another 53 selected tests passed in `explicit-native-continuation.test.ts` with `-t 'pre-dispatch review wait|unclaimed workspace retry|historical native preparation'`. The other 361 cases in that file were not run. - Guard cases cover wrong/missing receipts, dispatch events, process identity, native ownership, live controller leases, workspace recovery, cleanup in flight, unreceipted remote resources, and an unknown historical HTTP dispatch. - A remote lease passes only after its exact termination receipt is stored. - Secret/PII diff scan and `git diff --check` passed. - Standard Greptile reviewed commit `201a61d2bb63290ca9b9b9d524e6c6715a872310`: [5/5 with no actionable findings](https://github.com/paperclipai/paperclip/pull/20384#issuecomment-6753165178). No review threads are unresolved. - Local testing is limited to affected cases. GitHub CI owns broad typecheck, build, and test coverage. Verification setup failures remain accounted for: worktree preflight could not complete because Cargo is unavailable; the first bundled PostgreSQL run failed because its macOS binary lacks a shared library. The focused tests passed on a separate, temporary local PostgreSQL 18 cluster. An early local server typecheck could not resolve the unbuilt Runner package; it is not claimed as a pass. ## Risks - Stop proof now accepts existing positive resource-wait receipts. Missing PIDs, terminal status, elapsed time, and a different boot ID remain insufficient. - The receipt must match its error code. The runtime must remain unresolved, with a preparation or settled stage and no conflicting native or workspace references. - The shared cancellation proof is also used by explicit continuation and Retry. Those paths keep their existing user-intent, ownership, pause, budget, and authorization gates. A stop proof does not itself start another run. - Runs that lack usable evidence remain held. This PR does not claim to repair every run shape reported on #18147. - No migration or live data edit is needed. No hosted deployment is included. ## Model Used OpenAI GPT-6 through Codex, with repository inspection, code execution, PostgreSQL regression tests, and GitHub tools. The exact serving revision, context window, and internal thinking configuration 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 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 - [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
|
9c634e42e4 |
fix(workspace): bound queued source-lock admission for imports (#20370)
## Thinking Path > - Paperclip manages AI agents and their work. > - Existing local workspaces need durable imports before a volume-free deployment. > - Imports must acquire the same source locks as workspace writers. > - Try-only admission can repeatedly lose to background maintenance, even when each writer finishes. > - This pull request adds an optional bounded wait to the canonical migration APIs. > - Operators can join the source-lock queue while all existing preservation checks remain active. ## Linked Issues or Issue Description **What happened?** An online import can repeatedly return `workspace_source_busy` while background workspace maintenance reacquires its project source lock. The import does not join the PostgreSQL lock queue. **Expected behavior** An administrative import can wait for a finite writer to finish. One wait budget covers the complete source scope. Timeout or cancellation must release acquired locks before the import reserves a journal. **Steps to reproduce** 1. Hold the source lock for an eligible local workspace. 2. Start an online import while the writer holds that lock. 3. Release the writer. Before this change, the import has already refused admission. With `sourceLockWaitMs`, the import acquires the source scope and commits normally. **Paperclip version or commit** Reproduced on master `0e3fdebea8257bdff11573081c76b306f12db230`. **Deployment mode** External PostgreSQL with an online workspace import. Related preservation work: https://github.com/paperclipai/paperclip/pull/20311. ## What Changed - Add optional `sourceLockWaitMs` to singleton and cohort migration APIs. Omitted options keep current admission behavior. - Queue the sorted source keys with one decreasing wait budget. Abort cancels only the dedicated backend identified by PID and backend start time. - Use an independent source transaction for queued singleton imports. Keep lifecycle checks, fresh source checks, journal ownership, and publication after commit. - Document the canonical API option. Add real PostgreSQL importer regressions for release, timeout, abort, and partial-key cleanup. - Correct the inherited task-case lookup summary so the existing search-discovery regression passes without changing its assertion or search algorithm. ## Verification - Baseline RED: the new singleton regression fails on the unchanged product because the import returns busy without queueing. - `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'queued|bounds the whole source queue'`: 20 passed, including five new cases. - `pnpm exec vitest run server/src/services/isolated-remote-workspace-source-lock.test.ts`: 4 passed. - Server layer typecheck, `node scripts/check-module-boundaries.mjs`, and `git diff --check` passed. - Tests used Node 24.22.2 and the existing embedded PostgreSQL fixture. Hosted rollout verification remains pending. - CI build, typecheck, browser suites, all serialized suites, and the other general shards passed. The final server shard failed only the inherited `case-history-api-discovery.test.ts:14` search-ranking assertion. Its unchanged local regression reproduces on the base and passes 3/3 with a separate one-line route-summary correction. The shared one-line summary correction is included in follow-up commit `8163ffa9d4728134676b84c8c242a5bf62c802b5`; the unchanged case-discovery suite passes 3/3 locally. The original CI attempt remains failed: https://github.com/paperclipai/paperclip/actions/runs/42704666536/job/129458605783. ## Risks Queued admission can hold earlier source keys while it waits for a later key. The caller controls the finite budget. The abort path opens a short-lived dedicated cancellation connection. If cancellation cannot connect, PostgreSQL's remaining lock timeout still bounds the wait. This option must be called at the root of administrative work; an external scope that already holds the same source locks can cause self-contention. The existing pre-held journal-resume path must keep its omitted-option behavior. ## Model Used OpenAI Codex, based on GPT-6, with tool use and code execution. The exact deployment identifier 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 - [ ] 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> |