PaperClipAI

0 Fork 0

mirror of https://github.com/paperclipai/paperclip.git synced 2026-11-13 15:38:22 +01:00

Author	SHA1	Message	Date
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>	2026-11-10 11:00:31 -06:00
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>	2026-11-10 10:44:51 -06:00
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>	2026-11-10 10:04:22 -06:00
DottaandPaperclip	ce1ee9e89f	fix: stop prompts from encouraging subtasks (#20362 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents read runtime prompts, tool help, skills, and role instructions. > - Several of these texts tell agents to create child tasks as a normal next step. > - One accepted-plan prompt even forbids work on the current task. > - Task parentage is useful, but it does not need a prompt that encourages task creation. > - This pull request removes those prompts and keeps the task APIs available. ## Linked Issues or Issue Description Refs #10642. This change adjusts prompt wording; it does not add delegation-cycle guards. Current behavior Prompts tell agents to create children after plan approval, use children for long work, and create a task just to pass context. Onboarding requires a child even for a single request. Proposed behavior Continue work on the current task. Keep explicit assignments, optional task creation, parentage, dependencies, and approval gates. Use the separate-task recipes when the user requests that structure or when continuing existing work. Reason and benefit Reduce needless task trees without removing subtask support. Keep the patch limited to wording, existing test expectations, and generated catalog data. ## What Changed - Align accepted-plan prompts and remove direct child-creation advice from runtime and error text. - Make task tool descriptions neutral. Keep their fields, defaults, and handlers. - Update the main skill, onboarding, CEO instructions, company templates, specialist workflows, and matching guides. - Keep historical release snapshots intact. Refresh the explicitly seeded schema fixture's catalog digest; the server still verifies historical release snapshot hashes. - Refresh the affected generated catalogs, capability inventory, semantic contracts, and fixture manifest. ## Verification - Final tested head: `c6e304d0c105cdbfbe0b2ab0a233307fcd91a04b`. - All 66 check runs are complete: 60 pass, one is neutral, and five are skipped by CI conditions. Snyk also passes. No checks fail or remain pending. - [PR CI](https://github.com/paperclipai/paperclip/actions/runs/42846226554) passes typecheck, build, server/general tests, all eight E2E shards, Runner support, Runner Rust, and the aggregate verify gate. [Windows](https://github.com/paperclipai/paperclip/actions/runs/42846226546) and [Docker Runner](https://github.com/paperclipai/paperclip/actions/runs/42846226108) also pass. - [Greptile](https://github.com/paperclipai/paperclip/pull/20362#issuecomment-6751419835) and [Paperclip review](https://github.com/paperclipai/paperclip/pull/20362#issuecomment-6751454010) give this exact head 5/5. All six inline threads are resolved. Neither current summary has findings. - Merged master `bf0220d34a156c81853755c3d1365ddc527037de`. Resolved three generated-file conflicts with the canonical generators. - Local verification passes: 473 focused prompt, continuation, UI, catalog, runtime and native-execution tests; Runner TypeScript build; generated metadata; UI token gates; and diff checks. - The prior head `c22018cb0f` failed three SDK evidence tests because two source hashes were stale. Reproduced all three failures locally. The existing generator refreshed only the hashes for the denial-copy file and its test. All 53 affected evidence/source-pin tests then passed, including private-value rejection. No scanner logic, assertions, or runtime dependency pins changed. Current CI confirms the support suite passes. - A prior commit failed the Rust process reaping test. Its cause remains unproven. No Rust code or assertions changed. Later Rust runs pass; this does not establish the cause of the earlier failure. - Local preflight built shared/plugin SDK packages, then stopped because `cargo` is absent. Current-head CI supplies Rust and full build coverage. `pnpm verify --explain` selects broad required CI; full local suites were not repeated. ## Risks - Wording changes guide the model; they do not enforce a ban on subtasks. - Existing installed skill versions and saved custom instructions need their normal refresh process. This PR does not migrate company configuration. - Existing approved task assignments and approval gates remain in effect. ## Model Used - OpenAI GPT-6, through Codex, with reasoning, tool use, and code execution. The exact routed model ID and context-window size were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-11-10 09:58:54 -06:00
Dotta Paperclip Claude	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>	2026-11-10 09:57:01 -06:00
Dotta Paperclip Claude	faae6b919f	fix(runner): explain task scope on missing tool inputs (#19774 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents read saved task documents through native tools. > - Active-task tools use the task bound to the run. Other task tools require a task UUID. > - Agents sometimes call the second set with only a document key. > - The validator rejects the call, but gives no instruction about the active-task tool. > - This pull request adds scope instructions to the tool schemas and returns them for missing required fields. > - Agents can correct a rejected call. Task selection and permission checks stay in place. ## Linked Issues or Issue Description What happened? Two observed document reads sent only `{"key":"sentry-case"}` to `read_task_document`. Each returned `Invalid tool input: / must have required property 'taskId'`. Each later retry with a task UUID returned without an error. Pattern: [Task reads fail when agents omit taskId](https://www.braintrust.dev/app/Paperclip/p/Paperclip%20Staging/patterns/703d8c29-6a40-4aa2-b2c6-e2c5c36b5657). Related PR: #18824 changed document key validation. It does not address task scope. Expected behavior The tool contract must state which task it reads. A missing-field error must include the schema owner's recovery instructions. A retry must still pass validation and task permission checks. Steps to reproduce 1. Call `read_task_document` through the MCP bridge with only a document key. 2. Inspect the error sent to the caller. 3. Retry with an authorized task UUID. Confirm the returned document belongs to that task. ### Hypothesis and thesis for a fix Hypothesis: generic tool descriptions contribute to confusion between active-task and explicit-task tools. An agent can also ignore a correct schema. These tests do not measure the rate of model mistakes. Thesis: Paperclip owns task scope instructions. Its five explicit-task tools now describe the required UUID and the active-task alternative. The two MCP transports return direct missing-property descriptions from the admitted schema. The transports do not infer task IDs or apply Paperclip task policy. Proof: the same HTTP MCP regression fails on baseline `f673bd3cb5b5b65ec51781e37355e168c3fe8ae4` and passes with the fix. It still rejects before handler dispatch. An explicit retry reaches the requested task. An isolated real API and PostgreSQL test checks the saved revision, active-task selection, malformed UUIDs, and cross-company denial through both transports. ## What Changed - Add UUID and active-task guidance to `get_task`, `comment_on_task`, and the three explicit-task document tools. - Return bounded, distinct descriptions for up to three direct missing required fields. Keep ordinary validation errors unchanged. - Share this formatter between the Runner and OpenCode MCP bridges. - Add transport regressions, real route and database checks, and tool documentation. ## Verification - Baseline red: both transport regressions return only the required-property error and fail the recovery instruction assertion. - Candidate green: 58 Runner and OpenCode transport and error formatter tests pass on Linux with Node 24.22.2. - Final-head server verification passed all 115 tests: 113 document-key checks and two real API integration cases. - The two local integration cases use real MCP HTTP, agent authority, API routes, and disposable PostgreSQL. They read a selected other task and its saved revision, read the active task separately, reject missing IDs for all five tools, reject malformed UUIDs, and return 404 for a foreign company task. - Runner TypeScript typecheck and build pass. - The full server typecheck passed on head `03a37e9e50896416cfd45be33bf6dbe3e13169df` with `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 node scripts/typecheck-layers.mjs`. The first run ended without a compiler diagnostic. Final-head CI passes and Greptile is 5/5. - Full preflight stops because cargo is absent. Rust checks were not run locally. - One existing delayed-body fetch test failed with a Node 25 fetch runtime error. All transport tests pass on Node 24. The first integration attempt used the wrong fixture return shape; the test was corrected before the passing run. - CI follow-up: the Runner test program reported four TS18046 errors because `Response.json()` returns `unknown` under CI typings. Both tests now declare the response type. `pnpm exec tsc -p tsconfig.test.json --noEmit` passes, and both transport suites pass again (56 tests). Final head `ffccebca19fe9e21a097829adc020679ffa7d86c` passes CI and Greptile 5/5. - Review follow-up: the foreign task now has a saved document with a distinct body. Both integration cases passed again after this test-only change. The read must return 404 and must not return that body. Current-head CI passes and Greptile is 5/5. - Final CI: https://github.com/paperclipai/paperclip/actions/runs/42352822739 . All required jobs passed for `ffccebca19fe9e21a097829adc020679ffa7d86c`. Greptile is 5/5 with no unresolved review thread. - Real local API behavior is verified. External model behavior and deployed recovery are not verified. No merge or deployment is included. ## Risks - Missing-field errors now contain schema descriptions. The text is limited to direct properties in the admitted schema, at most three distinct descriptions and 1,000 characters each. - Nested schemas and references do not get recovery text. Their validation errors remain intact. - Clearer instructions can help callers recover. They do not guarantee that a model will choose the correct tool. ## Model Used - OpenAI GPT-6 via Codex, with tool use and code execution. The exact deployment model ID and context window are not exposed to 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> Co-authored-by: Claude <noreply@anthropic.com>	2026-11-10 09:07:11 -06:00
Dotta Claude Opus 5.5 Paperclip	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>	2026-11-10 08:02:11 -06:00
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>	2026-11-10 07:42:43 -06:00
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>	2026-11-10 06:16:42 -06:00
DottaandClaude	16c30ac32b	fix(runner): relaunch a provider whose startup only timed out without a reply (#20521 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs execute through `paperclip-runnerd`. After a restart, a new runner relaunches the provider (for example the Codex app server). A durable startup fence stops a second runner from launching a provider while an unadmitted one may still run. > - #19784 lets a new runner retire a fence whose recorded provider process and group have provably exited. That rule covers a launch that a kill cut off. It deliberately keeps a failed initialization closed, because that can be a real provider failure. > - A relaunch whose reply never came within the runner's 30 s request deadline is saved as the same failed initialization. A stalled or frozen host causes this. The restart-chaos fault TM2 SIGSTOPs the runner and Codex, and their monotonic clocks keep running while they are stopped. > - Every later runner then refused to restore with "provider startup ownership remains unadmitted", and the user had to click Retry. The owner's rule is that a run interrupted by a restart or sleep must resume by itself. > - This pull request records whether initialization failed only because the provider gave no reply, and lets a later runner retire that fence once the provider is provably gone. > - The benefit is that a stall during a provider relaunch continues automatically, while a real provider rejection still fails as before. ## Linked Issues or Issue Description Refs #19784 What happened? Restart-chaos wave `w-20261110T091152-s426`, task t03 (Codex, B5), run `c0bbce18-836b-43af-8195-92ebe3e21cca`. A TM3 restart relaunched the Codex provider. A TM2 freeze stopped the tree 1 s later for 45 s, and a TM3 kill followed the thaw. The next three runners each failed `runner.drain` with "provider startup ownership remains unadmitted". Recovery stopped with `native_session_retry_exhausted`, and the task needed Retry. What did you expect? The new runner relaunches the provider and the run continues. Steps to reproduce The new test `restart_relaunches_a_provider_whose_relaunch_timed_out_without_a_reply` (runner-core `tests/codex_provider.rs`). The fake Codex stalls its first `thread/resume` reply past the 30 s deadline. On `origin/master`, the saved attempt is `initialization_failed` / `thread_open`, and the next runner fails with "provider startup ownership remains unadmitted". ## What Changed Design: - The deadline that passes is the runner's own 30 s request deadline. When a startup request (`initialize`, `thread/start`/`thread/resume`, `thread/read`) reaches that deadline with the provider still running and silent, the runner sets `noReply: true` on the durable `initialization_failed` startup record. - Not `noReply`: a provider error response, a provider that exited before it replied, or closed stdout. - `retire_orphaned_startup_attempt` retires an `initialization_failed` fence only when `noReply` is set and the recorded process and its process group are both gone (`ESRCH`). This is the same proof as `spawned` in #19784. A rejection, an exit, a live process or group, and spawn or admission failures stay fenced. - Validation accepts `noReply` only on `initialization_failed` at the `initialize`, `thread_open` or `thread_read` stage. - Bounded: a runner restores once (`restore_checked` / `restore_error`), so the loop is one relaunch per recovery attempt. Recovery's existing resume budget (`native_session_retry_exhausted`, seen after three attempts in the wave) stops a provider that never replies. - Upgrade: `noReply` is `#[serde(default, skip_serializing_if = "Not::not")]`. A new runner reads state saved by master as `noReply: false`, so that state keeps today's behaviour and stays fenced. - Rollback: an old runner (`deny_unknown_fields`) rejects state that holds a `noReply` fence and fails closed. That run needs Retry, which is today's outcome for the same fence. The field is written only while a timed-out fence is present, and admission clears it, so all other state is unchanged for old runners. - Normal path: nothing is added before `turn.start`. The cost is one in-memory flag per request; no extra I/O. Files: - `runner-core/src/codex_provider.rs`: `last_request_got_no_reply` and the `no_reply` field on `ProviderStartupObservation::Failed`. - `runner-core/src/provider_backend.rs`: the `noReply` field, its validation, and the retirement rule, with unit tests: - a no-reply fence is retired at `initialize`, `thread_open` and `thread_read`; - it is kept while the provider may still run; - a rejection or a state saved without the field stays fenced. - `runner-core/src/bin/fake-codex-app-server.rs`: the test-only switch `--stall-first-thread-resume-ms`. - `runner-core/tests/codex_provider.rs`: the end-to-end red/green test. - `tests/restart-chaos/README.md` and `time-faults.ts`: correct the TM2 claim. A stopped process's monotonic clock keeps running, so TM2 is also a long process stall. ## Verification - `cargo test -p paperclip-runner-core --test codex_provider restart_relaunches_a_provider_whose_relaunch_timed_out` is red on `origin/master` (the next runner refuses restore) and green here, in about 31 s. - `cargo test -p paperclip-runner-core`: 1080 passed, 0 failed. This includes `failed_autonomous_restore_and_rollover_keep_their_exact_startup_origin`, where a real "no rollout found" rejection still never restarts. - `cargo fmt -p paperclip-runner-core -- --check` is clean. - A restart-chaos proof wave on seed 426 will follow in a comment. ## Risks - A provider that hangs for good is relaunched once per recovery attempt until the existing resume budget stops it. Before, it failed on the first attempt. This costs at most a few extra 30 s attempts, and they are bounded. - Rollback to an older runner while a `noReply` fence exists fails closed for that one run, which needs Retry, as it does today. - No database, server or API change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, cargo and 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>	2026-11-10 05:23:30 -06:00
DottaandClaude	289b402c26	fix(runner): settle a saved governed wait whose parked turn was lost on restore (#20519 ) ## 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 > - PR #20494 made a saved wait settle after a control-plane restart for Codex, which reports the parked turn as interrupted > - Claude runs through ACPX. After a kill -9 restart, Runnerd restores the conversation and reports the parked turn as `turn.failed` with `provider_turn_lost_on_restore` > - The governed-wait drain and the saved-replay check rejected every failed terminal as unproven, so the saved question failed and the task needed a human Retry > - Runnerd emits `provider_turn_lost_on_restore` only after it proves that the old provider lifetime ended. That is the same proof as an interrupted terminal > - This pull request accepts that recoverable restart loss as proof for a saved governed wait, and only that failure > - The benefit is that a Claude question survives a control-plane kill the same way a Codex question does ## Linked Issues or Issue Description Refs #20494. Related: #20498 (Claude lost cancel reply; this change does not touch that code). What happened? Restart chaos wave w-20261110T094930-s214 (Claude, Q1, PV1 and PV2 with `--provider-fault-restart K1`) failed 2 of 2 Q1 tasks with `R3:retry_needed`. The asking run ended `failed` with `native_provider_terminal_failed`. Its events show the question saved, the K1, recovery with `native_governed_wait_settlement_pending`, and then `turn.failed` with `provider_turn_lost_on_restore` for the parked turn. The thrown `providerCode` was `governed_wait_provider_terminal_unproven`. What did you expect? The saved question settles after the restart. The asking run ends `succeeded`. The board answer wakes the agent without a Retry. Steps to reproduce Run the harness with `--adapters claude --workloads Q1 --faults PV1,PV2 --provider-fault-restart K1 --provider-fault-delays 3 --provider-fault-restart-after-seconds 1 --concurrency 1 --tasks 4`. The new unit-test variants `lost-on-restore-after-restart` and `lost-on-restore-saved` failed with the same `providerCode` before this change. ## What Changed - `native-session-runtime.ts`: add `governedWaitTerminalProven(event)`. A failed terminal proves the stop only when `isNativeRestartInterruption(error)` is true (code `provider_turn_lost_on_restore`, `recoverable: true`). Other terminals keep the old rule (`providerTerminalObserved !== false`). - The governed-wait accounting drain and the saved-replay check both use that helper. - Tests: two new variants of the #20494 test (a lost terminal after the restart, and a lost terminal already saved before the next recovery). Two new rejection cases: a non-restart failed terminal and a non-recoverable restart loss still fail with `governed_wait_provider_terminal_unproven`. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/native-session-runtime`: 265 tests pass. The two new settle variants failed before the fix. - `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit -p .` passes. - A live restart chaos wave on this branch (Claude Q1, PV1/PV2 + K1) is linked in a comment below when it finishes. ## Risks - Low risk. The change accepts one more terminal as proof for a saved governed wait. That terminal comes only from Runnerd after it proves the old provider lifetime ended, and the code already trusts it to continue a restart-lost turn (`nativeRestartInterruptedTurnId`). - A failed terminal with any other code, or a non-recoverable restart loss, is still rejected. ## Model Used - Claude (Anthropic), model ID `claude-opus-5-5`, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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>	2026-11-10 05:13:21 -06:00
DottaandClaude	8b3348c6ba	fix(runner): classify Claude's rejected selected model as an unavailable model (#20513 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runner agents on the Claude (ACPX) provider run with the model the user saves on the agent > - When the provider cannot run that model, Paperclip should classify the run as `native_provider_model_rejected`, ask the user to choose a model, and let Retry finish the task > - Claude Code accepts an unknown model at session start and reports it only on the first turn, as an AIR `request` failure with a fixed title. The runner reported it as an unknown provider failure > - So the run failed as `native_provider_terminal_failed`, and the task stopped with `provider_effect_inventory_unavailable` ("Automatic recovery stopped. Recorded work is preserved…") without naming the model > - This pull request classifies that Claude title as an unavailable model, the category the native runtime already turns into `native_provider_model_rejected` > - The benefit is that an unknown Claude model asks for a model choice and names it, as Codex does after #20488 ## Linked Issues or Issue Description What happened? Config wave `w-20261110T094430-s207-cf` on `master` (`82abb3e`), `--adapters claude`: CF3 `model_id` failed both times (t01 K1, t04 K2) as `CF3/model_id:actionable_message`. The task showed `Stopped`, cause `provider_effect_inventory_unavailable`, "Automatic recovery stopped. Recorded work is preserved; actions with unverified outcomes will not be repeated." The run was `native_provider_terminal_failed`, "ACP agent reported a terminal request failure." Root cause, from a direct ACP probe of the bundled `@agentclientprotocol/claude-agent-acp` 0.73.0 with the AIR `sessionFailure` capability and the model in `_meta.claudeCode.options.model` (as ACPX passes it): `session/new` lists `chaos-missing-model-abc12345` among the model options, so admission's model check passes. `session/prompt` then returns `stopReason: "end_turn"` with `_meta.jetbrains.air.sessionFailure` = `{"category":"request","severity":"error","title":"There's an issue with the selected model (chaos-missing-model-abc12345). It may not exist or you may not have access to it. Run --model to pick a different model.","actions":[]}`. ACPX turns that into "ACP agent reported a terminal request failure.", and `acpxProviderFailure` classified only AIR `limit` titles, so the provider failure stayed `unknown`. (Selecting the same model later through `session/set_config_option` instead returns JSON-RPC `-32603` "Internal error", `data.details` "Invalid value for config option model: <id>". The runner does not take that path for Claude, so this PR does not change it.) Expected behavior The run is `native_provider_model_rejected`, the task shows `choose_model` with the rejected model, no recovery incident opens, and Retry finishes the task after the model is repaired. Steps to reproduce `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/config-wave.ts --faults CF3 --adapters claude --references model_id --tasks 2 --concurrency 2 --restarts K1,K2 --seed 207` on `master`. Paperclip version or commit `master` at `82abb3e`. ## What Changed - `packages/paperclip-runner/src/drivers/acpx/provider-failure.ts`: for the `claude` agent, an AIR `request` session failure whose title is Claude Code's fixed "There's an issue with the selected model (…). It may not exist or you may not have access to it." is classified `model_unavailable` (`requires_action`). The native runtime already marks such a failed turn `native_provider_model_rejected`, and the server already projects that as `choose_model` with `rejectedModel`. - Other AIR categories, other agents and other request titles stay `unknown`. - `server/src/services/native-runtime/native-session-executor.ts`: a `native_provider_model_rejected` failure after the provider session opened now gets a choose-model next action ("The provider rejected the selected model. Choose a supported model for this agent, then retry the task. Automatic retries are stopped."). Before, only a rejection before the session opened got that action; the Claude case fell through to "Inspect the original provider failure and explicitly resolve the ambiguous session state; do not open a replacement provider session." Recovery mode, blocking and retry behavior are unchanged; only the action text changes. ## Verification - Red before, green after: `src/drivers/provider-failure.test.ts` — "classifies Claude's rejection of the selected model as an unavailable model", with the exact title captured from `claude-agent-acp` 0.73.0, through both `acpxTerminalResult` and the thrown form, plus negative cases. - Red before, green after: `native-startup-retry.integration.test.ts` — "asks for a model choice when the provider rejects the model on an opened session's turn (Claude ACPX)": a durable `turn.started` and the `NativeProviderTerminalFailure` the runtime throws; the coordinator and the board recovery action name the model choice, not ambiguous state. - Live: on `master` the wave failed (see above). On this branch: `w-20261110T103809-s208-cf` on head `3881650` (CF3 `model_id`, claude, K1 and K2): 2/2 pass. Each task rests as `waiting_for_user` / `choose_model` with `rejectedModel: "chaos-missing-model-…"`, cause `native_provider_model_rejected`, next action "The provider rejected the selected model. Choose a supported model for this agent, then retry the task. Automatic retries are stopped.", rests again after the restart, and is done after the model is restored and Retry. An earlier wave on `e552b24` (`w-20261110T100526-s207-cf`, 2/2 pass) still showed the ambiguous-state text that the second commit replaces. ## Risks - Low risk. The match is limited to the Claude agent, the AIR `request` category and Claude Code's fixed title. If Claude Code changes the text, the run falls back to today's generic failure. - No profile, digest or pack contract change (`rules/acpx-profile-identity.mdc`): ordinary Runner source. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs, a direct ACP probe of `claude-agent-acp`). ## 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>	2026-11-10 04:41:02 -06:00
DottaandClaude	a09093a5c5	fix(native): resume a run whose turn.start a restart cut off before Codex created the turn (#20507 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run sends `turn.start` to its runner, and the runner sends `turn/start` to Codex. The runner marks the start "ambiguous" before it sends the request > - A control-plane restart can kill the runner and Codex while that request is in flight. After the restart, neither journal says whether Codex created the turn, and the resumed thread has no active turn > - The restored runner then failed closed on every same-run resume with "cannot safely recover an ambiguous Codex turn start without an active replacement turn". The answer never reached the agent > - The next run on the task was refused at the runner-state fence (`prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(turn.start.failed)`) before it asked for a runner. Retry after that run then waited forever on `process_identity_missing` for a process that never existed > - This pull request lets the runner prove from Codex's own thread history that the start never created a turn, and then resume and start the turn once. It also lets Retry continue after a run that the runner-state fence refused before launch > - The benefit is that a turn start cut off by a restart continues by itself when Codex never saw it, and a person's Retry is no longer a dead end ## Linked Issues or Issue Description What happened? Local restart-chaos wave `w-20261110T033426-s108`. Command: `--env local --adapters codex --workloads Q1,Q2 --faults K1 --triggers answered --tasks 4 --concurrency 2 --seed 108`. Task t03 (Q2): the answer run `92a57f50` resumed after the first restart. A second K1 restart landed 0.25 s after `session.resumed`, while its `turn.start` was in flight. Each same-run resume then failed with `PRP command session.snapshot failed: cannot safely recover an ambiguous Codex turn start without an active replacement turn`. When the prerequisite finished, the next run `0e6aa9d1` failed before launch with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(turn.start.failed)`. The accepted Retry (202) never created a run: the task stayed on `retryWait.reason=process_identity_missing` until the wave timed out (`R3:retry_no_new_run`). Expected behavior If Codex never created the turn, the run resumes and starts it once by itself. If that cannot be proven, a person's Retry starts a new run instead of waiting forever. Steps to reproduce 1. Run the wave command above on master. 2. Or run `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-cut-turn-start.test.ts` on master (case "before" fails with the wave error). ## What Changed - Runner (`codex_provider.rs`, `provider_backend.rs`): when the restored provider state has an ambiguous turn start, no active turn before the loss, and the resumed thread has no active turn, the runner reads the newest turn of the thread (`thread/turns/list`, limit 1, metadata only, the same request `read_thread` already sends). If there is no turn, or the newest turn is one Paperclip already settled, Codex never created the turn. The runner clears the ambiguity, emits a `harness.diagnostic` with code `ambiguous_turn_start_not_created`, and restores an idle session. The old `turn.start` stays `execution_indeterminate` and is never executed again. The same-run resume then starts the run's turn once, through the existing "recovered session without an active turn" path. - Any other history (a turn Paperclip never settled, an in-progress turn, an incomplete page, a non-Codex provider) keeps today's fail-closed error. The probe runs only on this recovery path. The normal turn path does no new work before `turn.start`. - Server (`native-local-process-stop.ts`): `nativeRunNeverLaunched` also accepts a failed native run whose error is the runner-state fence refusal `runner_state_identity_mismatch: prior_owner_` (or `legacy_prior_owner_`). The executor throws it only while it migrates another run's saved state, before it requests a launch. The existing evidence check still applies: any launch request, identity record, `adapter.invoke` or runner event keeps the run held. Other runner-state errors (for example `authority_indeterminate`) stay held. - Fake Codex (`fake-codex-app-server.rs`): keeps the thread's turn history in a `<state-file>.turns` sidecar, so `thread/turns/list` returns history like real Codex (newest first, honours `limit`). New `--hang-turn-start <file>` holds `turn/start` "before" or "after" the thread records the turn, to model a kill in flight. - Tests: - New real-runnerd test `runnerd-codex-transport-cut-turn-start.test.ts`. A real runner and fake Codex start a turn; both processes are killed while `turn/start` is in flight. "before": the resumed run completes, the journal holds the old `turn.start` (`execution_indeterminate`) and one new completed `turn.start` with the same task text, and Codex saw exactly one more `turn/start`. "after": recovery still fails closed and no new `turn/start` is sent. - Rust: `ambiguous_turn_start_that_never_created_a_turn_recovers_and_starts_once` and `ambiguous_turn_start_that_created_an_unsettled_turn_still_fails_closed`. Both rewrite saved provider state the way a master runner wrote it, so they also cover state saved before this change. - `explicit-native-continuation.test.ts`: five new rows for the runner-state fence. Admitted: refused before launch (with and without a coordinator). Held on `process_identity_missing`: refused after a launch request, refused after a runner event, and a run that failed its own `authority_indeterminate` check. - `docs/durable-recovery.md`: describes both behaviours. ## Verification - Red/green, real runnerd: on master (the staged master runnerd), `runnerd-codex-transport-cut-turn-start.test.ts` "before" fails with `PRP command runner.drain failed: ... cannot safely recover an ambiguous Codex turn start without an active replacement turn`, the wave error. With this change, both cases pass. - Red/green, Rust: without the `provider_backend.rs` change, `cargo test -p paperclip-runner-core --test codex_provider ambiguous_turn_start_that` fails the "never created" test. With it, both pass. The full `cargo test -p paperclip-runner-core` passes. - Red/green, server: without the `native-local-process-stop.ts` change, the two "refused at the prior-owner runner-state fence" rows fail with `process_identity_missing`. With it, all 14 rows pass. - Related suites pass: 11 Runner transport files, 419 tests (`runnerd-codex-transport`, `-attachment-recovery`, `-cold-restore`, `-warm-attachment`, `-release-resume`, `-cut-attach`, `-cut-turn-start`, `-settlement`, `-suspension`, `runnerd-restart-continuation`, `runnerd-saved-continuation`); server `explicit-native-continuation`, `quarantined-task-retry` and `issue-confirm-run-stopped-routes`, 3 files, 470 tests; full `cargo test -p paperclip-runner-core`. - `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit` and server `tsc --noEmit` pass. - Pack `contractDigest` is unchanged: `sha256:3164357d…` before and after (`scripts/contracts/contract-fingerprints.mjs`). - Live proof: the wave command above on this branch. Results follow in a comment. No duplicate provider work. The runner starts a new turn only when Codex's own history proves the cut-off start created nothing. The old command is never executed again (the runner refuses an `indeterminate` command). A recorded turn that Paperclip never settled keeps the run failed closed. ## Risks - Low-medium risk. The runner change runs only while it restores a session whose saved state has an ambiguous turn start and no active turn. Before this change, that run could never continue. - The proof relies on Codex recording a turn in the thread before it runs anything. If Codex accepted the request but died before it wrote the turn, the runner treats the start as not created and starts the turn once. Codex writes the user turn to the thread when it starts the turn, before it asks the model for output, so no model output or tool call is expected without a recorded turn. - If the thread's newest turn is not in the runner's settled set (for example after a provider identity epoch rollover forgot older identities), the runner keeps failing closed, as today. - Open decision (not changed here): when Codex did record the cut-off turn, it may have done work. This pull request keeps the run failed closed and does not replay or adopt that turn. Continuing is a person's decision through Retry or a new message. - The server change trusts the executor's own error text `runner_state_identity_mismatch: prior_owner_`, together with the absence of every launch and runner event. A run that requested a launch keeps every stop proof. - Runner release:* the pack `contractDigest` does not change. Local runners use the runnerd built with the app, so the fix applies with this release. - Not covered: the related `authority_indeterminate` variant in the same wave (t04, Q1) is a separate cause; its Retry already worked in that wave. > 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 #20477, #20482 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-11-10 03:49:22 -06:00
DottaandClaude	914e3ae14f	fix(runner): settle ACPX turns when the cancel reply is lost (#20498 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Claude agents run through the native Runner and the ACPX sidecar > - A saved question or a board Stop cancels the active turn with `turn.cancel` > - The Runner waited 30 s for the sidecar's reply to that cancel, and the Runner serves controller commands one at a time > - When that reply was lost, the controller's shutdown commands timed out first, so the run failed and the task needed a human Retry > - This pull request bounds that wait, keeps the transport open, and lets the provider's own turn terminal settle the turn > - The benefit is that a lost cancel reply on Claude settles the same way it already does on Codex (#20336) ## Linked Issues or Issue Description Refs #20336 (Codex equivalent: settle a saved handoff after verified shutdown). Related, no overlap: #20477, #20482, #20483. What happened? The restart-chaos harness dropped the ACPX sidecar's reply to the Runner's `turn.cancel` (fault PV2, target `interrupt_ack`). A Claude task that saved a question (Q1) failed with "The waiting state is saved; confirming shutdown of the previous execution is still pending". Its next attempt failed `native_session_cleanup_quarantined`. A Claude task stopped by the board (S1) ended `blocked`. Retry was accepted but started no run. Cause, in `packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs` (`interrupt_turn`, master line ~800): the cancel used the ordinary sidecar request with the 30 s bound from `acpx_provider_backend.rs:539`. While it waited: - `turn.stop` and `runner.suspend` stayed queued. The controller's 15 s governed drain plus its 10 s close grace ran out first (`runner did not durably suspend before checkpoint`, suspend command `pending`). - At 30 s, `fail_closed` killed the sidecar and dropped the provider's buffered turn terminal. The poll path then recorded `turn.failed` (`acpx_provider_transport_failed`) and `provider_exit_unconfirmed` (`acpx_provider_backend.rs:2716-2738`). `turn.stop` returned `already_settled`, and `runner.suspend` refused with `ACPX provider lifetime cleanup is not yet proven` (`acpx_provider_backend.rs:2613-2621`). Codex passes the same fault because its Runner does not block on the interrupt reply. Expected behavior A lost cancel reply must not fail the run. The saved question must settle as `succeeded`. A board Stop must end the run as cancelled with a proven suspension, so the next turn can start. Steps to reproduce `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters claude --workloads Q1,S1 --faults PV2 --provider-fault-message interrupt_ack --concurrency 1 --tasks 4` Failing wave on master: `w-20261110T060120-s207` (t04, t05). Deployment mode Native Runner with the Claude ACPX profile, local. Hosted sandboxes run the same Runner code. ## What Changed - `AcpxSidecarTransport::request_turn_cancel`: sends `turn.cancel` and waits only to a caller deadline. On a missing reply it keeps the transport usable and retains the request ID, so a late reply is skipped in both the response and the event-poll paths. A rejected reply and transport errors keep their existing fail-closed result. - `AcpxProviderSession::interrupt_turn`: waits at most 5 s (or the shorter configured request bound) for the cancel reply. On a missing reply it retires the runtime from warm reuse and returns. The provider's turn terminal still arrives through polling, and `turn.stop` still reaps a provider that never stops. - `AcpxProviderSession::deliver_tool_result`: a board Stop can land while a semantic tool runs. The session's own `turn.cancel` then retires the call in the sidecar before its result arrives, and the turn terminal may still be buffered. The sidecar rejects the result as stale (`ACPX_TOOL_CALL_STALE`). When this session sent `turn.cancel` for that turn, the Runner now keeps the durable result, like a result for an already closed turn. Every other rejection still fails closed. (`AcpxSidecarTransport::request_tool_resolve` reports whether a rejection was the stale-call case.) - A repeated interruption while a cancel is unanswered sends nothing and returns at once (review finding). - `fake-acpx-sidecar`: new `--cancel-reply lost\|late` flag. After that cancel, `tool.resolve` is rejected as stale. The provider ends the turn, and the reply is dropped or written just before the next request. - Three regression tests (below) and a short section in `packages/paperclip-runner/docs/durable-recovery.md`. No TypeScript, server, schema or protocol change. The normal start path is unchanged; nothing waits before turn start. ## Verification Red/green (red runs used the master versions of `acpx_provider_session.rs` and `acpx_sidecar_transport.rs`): - `cargo test -p paperclip-runner-core --test acpx_warm_attachment a_lost_cancellation_reply_still_settles_the_turn_and_suspends_the_runner` - Red: after 30.4 s, `ACPX turn interrupt failed: ACPX sidecar request timed out at turn.cancel` (the live error). - Green: `turn.interrupt` returns in about 5 s, then `turn.interrupted` and a `cancelled` `run.terminal` (no `turn.failed`), `turn.stop`, and `runner.suspend` completes. - `cargo test -p paperclip-runner-core --test acpx_provider_turns lost_cancellation_reply_keeps_the_turn_terminal_and_the_transport` - Red: `interrupt_turn` fails at the request bound. - Green, for a lost reply and for a late reply: the terminal is polled, warm reuse is refused, and the next request (`session.close`) succeeds after the late reply arrives. A late reply causes no second settle and no second prompt. - `cargo test -p paperclip-runner-core --test acpx_provider_turns a_tool_result_for_a_call_retired_by_our_cancellation_does_not_fail_the_transport` - Red (guard disabled): `ACPX sidecar command tool.resolve was rejected (retryable=false, classification=provider_tool_call_retired)`, the same error as live wave task t03 below. - Green: the result is kept, the pending tool clears, and the turn terminal is polled after it. - `cargo test -p paperclip-runner-core` (whole crate): all pass. Existing `turns-tool-resolution-rejected` coverage still fails closed when no cancel was sent. `cargo fmt --check` passes. - Pack contract digest: unchanged (`sha256:3164357d…` before and after, from `scripts/contracts/contract-fingerprints.mjs`). - Live wave 1, `w-20261110T072706-s211` (commit `d91bff1e9f`, the first fix only), `--adapters claude --workloads Q1,S1 --faults PV2 --provider-fault-message interrupt_ack --tasks 4`: 3 of 4 pass. Both Q1 tasks pass (the asking run `succeeded`, no failed run, answer in the brief). One S1 passes. S1 task t03 failed: Stop landed during a `write_task_document` call and hit the stale tool-result error above. The second commit fixes that case. - Live wave 2, `w-20261110T082702-s212` on head `76dd57515d`, same flags: every task passes R1–R6 and U1. Both Q1 tasks pass with the fault applied (asking run `succeeded`, no failed run). S1 t01 passes with the fault applied (Stop took effect, task `done`, no Retry). S1 t03 fails only `provider_fault_applied` (`applied=0`): the agent finished before the Stop window, so nothing was injected and the task proves nothing either way. - Live wave 3, `w-20261110T091057-s213` on head `76dd57515d`, same flags: the same result. t01 (S1), t02 and t04 (Q1) pass with the fault applied; t01's Stop took effect (`status=cancelled`), task `done`, no Retry. t03 again fails only `provider_fault_applied` (`applied=0`, the run finished before the Stop window); all its R1–R6 and U checks pass. - Across waves 2 and 3 on the final head: 6 of 6 tasks with the fault applied pass (4 Q1, 2 S1). No product rule failed. The two misses are harness timing (no injection), not product failures. ## Risks - A healthy cancel reply that takes longer than 5 s now retires the warm runtime. The next turn starts a fresh provider process for that session. The observed healthy reply time is about 1 s. - If the provider never stops and the reply is lost, the turn stays active until `turn.stop` terminates the provider and proves the lifetime fence. If that proof fails, the existing settlement-pending and quarantine states are unchanged; this PR adds no success path without proof. - Runner binary change: hosted Daytona images need a normal image rebuild to pick up the fix. The pack `contractDigest` does not change, so existing image packs stay reusable. ## Model Used - Anthropic Claude, model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edit, 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 <noreply@anthropic.com>	2026-11-10 03:17:14 -06:00
DottaandClaude	82abb3ea04	fix(native): classify a Codex API-key unknown model as a model rejection (#20488 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runner agents run on a provider (here Codex) with a model the user saves on the agent > - When the provider rejects that model, Paperclip should classify the run as `native_provider_model_rejected`, ask the user to choose a model, and let Retry finish the task > - Paperclip recognized only the ChatGPT-account rejection (a 400 JSON envelope). With API-key auth, Codex returns a different terminal error for an unknown model > - So the run failed as a generic `adapter_failed` ("Native session failed"), and recovery opened a reconciliation incident ("Recovery needed") for a turn that never did any work > - This pull request recognizes the real API-key shape, bound to the configured model, and names the rejected model on the task > - The benefit is that the task asks for a model choice instead of a reconciliation, and Retry completes the task after the user picks a valid model ## Linked Issues or Issue Description What happened? The restart-chaos config fault CF3 `model_id` saves a Codex agent with the model `chaos-missing-model-<nonce>`. The run stays "running" for about 60 seconds, then fails with `errorCode: adapter_failed`, "Native session failed". The task shows "Recovery needed", cause `native_continuation_requires_reconciliation`. The grader fails as `CF3/model_id:actionable_message` (waves `w-20261110T061914-s206-cf` t01 and `w-20261110T053831-s205-cf` t02). Root cause: the wave runs Codex with an OpenAI API key. I drove the bundled `codex app-server` (0.160.0) directly with an unknown model: - With API-key auth, Codex retries the Responses API 404 ten times (about 59 s), then fails the turn with `{"message":"unexpected status 404 Not Found: The model \`<id>\` does not exist or you do not have access to it., url: https://api.openai.com/v1/responses, cf-ray: …, request id: req_…","codexErrorInfo":{"httpConnectionFailed":{"httpStatusCode":404}}}`. - With ChatGPT auth, the turn fails in about 3 s with `message` = `{"type":"error","status":400,"error":{"type":"invalid_request_error","message":"The '<id>' model is not supported when using Codex with a ChatGPT account.","code":"model_not_found"}}`. Paperclip already classified this one. `createNativeProviderFailureObservation` parsed only the JSON envelope, and the runner's provider-failure category for an HTTP 404 is `unknown`. The terminal therefore stayed unclassified. Expected behavior The run is `native_provider_model_rejected`, the task shows `userAction: "choose_model"` and names the model, no reconciliation incident opens, and Retry completes the task after the user picks a valid model. Steps to reproduce `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/config-wave.ts --faults CF3 --adapters codex --references model_id --tasks 1 --concurrency 1 --restarts K2 --seed 206` on `master` (with `OPENAI_API_KEY` set). Paperclip version or commit `master` at `5ab98b0`. ## What Changed - `server/src/services/native-runtime/native-provider-failure.ts`: the Codex observation recognizes the API-key terminal shape (`codexErrorInfo` `httpConnectionFailed` or `responseStreamDisconnected` with `httpStatusCode: 404`, and the message prefix for the configured model only). It classifies it as `native_provider_model_rejected` with a new closed diagnostic `NATIVE_MODEL_NOT_FOUND_DIAGNOSTIC` (`codex`, `model_not_found`, `404`, `api_key`). No provider text or model name is stored in the diagnostic. A recoverable turn, another model's 404, or another status is not classified. - `readNativeModelRejectionDiagnostic` accepts both closed diagnostics, so run failure diagnostics show it. - `TaskInterruptionPresentation.rejectedModel` (shared type): with `userAction: "choose_model"`, the task query carries the model id from the run's native execution input. Only a `native_provider_model_rejected` run discloses it. - `TaskChatThread` names the model in the "Model unavailable" marker: `The provider rejected the model "<id>".` - The restart-chaos config wave reads `interruption.rejectedModel` as task-level text. For `choose_model`, its repair step uses the Retry that the "Model unavailable" marker offers (`POST /agents/{id}/wakeup` with `reason: "retry_failed_run"`, the same request as `agentsApi.retryFailedRun`), because a model rejection does not offer the task-level Retry. ## Verification - Red before, green after (the classification line disabled makes both fail): - `native-provider-failure.test.ts` — "classifies the API-key 404 for the configured model as a model rejection (codex 0.160.0 shape)", with the exact terminal error captured from `codex app-server` 0.160.0, plus negative cases. - `native-provider-failure.integration.test.ts` — new `api-key` scenario through `executePaperclipNativeSession`, the persisted evidence reader and `finalizeNativeRun`: the run ends `native_provider_model_rejected`, the issue is `blocked` with that decision, and no issue recovery action is created. - `task-interruption-query.test.ts` — the real task query carries `rejectedModel` only for a model rejection. `TaskChatThread.test.tsx` — the marker names the model. - `pnpm --filter @paperclipai/shared typecheck`, `pnpm --filter @paperclipai/server typecheck`, `pnpm --filter @paperclipai/ui typecheck` pass. - Live proof: config wave `w-20261110T090801-s206-cf` (CF3 `model_id`, codex with an OpenAI API key, K2, seed 206) on commit `05e8938`: 1/1 pass. The run ends `native_provider_model_rejected`; the task rests as `waiting_for_user` / `choose_model` with `rejectedModel: "chaos-missing-model-472e1851"` and execution cause `native_provider_model_rejected` (no reconciliation incident), rests again after the K2 restart, and finishes 27 s after the model is restored and the marker's Retry is used. On `master` the same wave failed as `actionable_message` with "Recovery needed" / `native_continuation_requires_reconciliation` (`w-20261110T061914-s206-cf`). ## Risks - Low risk. The match is bound to the configured model, a 404 status and a non-recoverable turn, so other provider failures keep their current behavior. - The match depends on the Codex terminal text. If a later Codex release changes it, the run falls back to the old generic failure, not to a wrong classification. - Claude (ACPX) unknown models are not changed here. Claude Code reports an unknown model as a synthetic assistant message ("There's an issue with the selected model …") rather than a provider error; this PR does not change or live-verify that path. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs, a direct `codex app-server` probe). ## 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>	2026-11-10 03:11:10 -06:00
DottaandClaude	a14ae9b5c9	fix(workspaces): name a failed workspace setup command on the task page (#20478 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before an agent starts, Paperclip prepares the task workspace and runs the project's configured setup commands (for example the workspace provision command) > - When that command fails, the run fails as `setup_failed`, and the task stops with a Retry > - The task page shows only "Failed" and a generic recovery notice. The real error (the command, its exit status and output) is visible only on the run page > - The user cannot see from the task which setting is broken or what to change > - This pull request records a bounded, named cause for a failed setup command and shows it on the task page > - The benefit is that the task names the broken setting and what to change, and classification, retry and recovery stay the same ## Linked Issues or Issue Description What happened? The restart-chaos config fault wave CF1 sets a project `workspaceStrategy.provisionCommand` to `bash ./scripts/provision-worktree.sh`, a script that does not exist. The run fails as `setup_failed` with `Execution workspace provision command "bash ./scripts/provision-worktree.sh" failed: bash: ./scripts/provision-worktree.sh: No such file or directory`. The task page shows "Failed" and "Paperclip automatically retried continuation … still has no live execution path". The grader fails the task as `CF1/provision_command:message_only_in_run_detail`. Expected behavior The task page names the failed setup step, the configured command, its exit status and a short tail of its output, and says what to change. Retry behavior stays the same. Steps to reproduce `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/config-wave.ts --faults CF1 --adapters codex --tasks 2 --concurrency 2 --restarts K1,K2 --seed 204` on `master`. Paperclip version or commit `master` at `5ab98b0` (wave `w-20261110T043929-s204-cf`, tasks t01 and t03). ## What Changed - `WorkspaceCommandExitError` now carries the setup step (`workspace_provision`, `workspace_seed`, `workspace_runtime_provision`), the configured command and the command output. - New `server/src/services/workspace-setup-command-failure.ts`: it builds a bounded, redacted `setupCommandFailure` record (step, command up to 200 characters, exit code, last 300 characters of output). It also recognizes the older message format, so runs recorded before this change still get a name. - `run-completion.ts` adds `setupCommandFailure` to the setup failure `resultJson`. The error code (`setup_failed`) and all retry and recovery fields stay the same. - `TaskInterruptionPresentation.setupFailure` (shared type) is set for `setup_failed` runs. It goes through the same per-run secret redaction and user-name redaction as `workspaceFailure`. - `ExecutionBlockerNotice` shows "Couldn’t set up the task workspace. <command> failed with exit code N: <output>. Fix the command in the project’s workspace settings, then retry." A scheduled transient retry keeps its countdown and only adds the name. - The restart-chaos config wave reads `interruption.setupFailure` as a task-level text for its actionable-message check. ## Verification - Red before, green after (the tests fail with the source changes reverted): - `server/src/__tests__/workspace-runtime.test.ts` — "names a failed project provision command with its exit status and output (CF1)" runs a real git worktree with a missing provision script (exit 127). - `server/src/services/task-interruption-presentation.test.ts` — "setup command failure presentation (CF1)": names the command, keeps `state`/`canRetry`/`userAction` identical, recognizes the older message, keeps a scheduled retry, bounds output and strips credentials from the command. - `ui/src/components/ExecutionBlockerNotice.test.tsx` and `tests/restart-chaos/config-faults.test.ts` cover the notice text and the harness check. - `pnpm --filter @paperclipai/shared typecheck`, `pnpm --filter @paperclipai/server typecheck`, `pnpm --filter @paperclipai/ui typecheck` pass. - Live proof: config wave `w-20261110T083227-s204-cf` (CF1, codex, 2 tasks, K1 and K2, seed 204) on commit `ae9a4b7`: 2/2 pass. Both tasks show `The project's workspace provision command "bash ./scripts/provision-worktree.sh" failed with exit code 127: bash: ./scripts/provision-worktree.sh: No such file or directory` on the task, rest again after the restart, and finish after the repair and Retry. On `master` the same wave failed both tasks as `message_only_in_run_detail` (`w-20261110T043929-s204-cf`). ## Risks - Low risk. The change adds a presentation field. It does not change the error code, retry, recovery or blocking behavior. - The command and output are user-configured text. They are bounded, passed through `redactSensitiveText`, the run's secret registry and user-name redaction. If redaction fails, the field is omitted, like `workspaceFailure`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (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>	2026-11-10 02:35:18 -06:00
DottaandClaude	3e572475c8	fix(native): re-attach an answer run whose run.attach a restart cut off (#20477 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a person answers an agent's saved question, Paperclip starts an answer run. For a per-turn session, that run starts a new runner and sends `run.attach` to bind the saved conversation to the run > - The runner journals each command before its effect. A restart that kills it inside that window leaves `run.attach` with an unknown outcome. The restored runner settles it as `execution_indeterminate` and never executes it again > - The controller's same-run resume waited on that old command, so every resume failed with "PRP command run.attach failed". The answer never reached the agent. A Retry or a stall followed > - A kill during the attachment's provider launch can also leave a startup fence that no later runner retires, so every later command fails with "provider startup ownership remains unadmitted" > - This pull request attaches again with a new command when nothing after the cut-off attachment started provider work. It also lets the runner retire that orphaned startup fence > - The benefit is that an answer given just before a restart continues on its own ## Linked Issues or Issue Description What happened? Local restart-chaos journey wave `w-20261110T054406-s108` on master `2c2186706e`. Command: `--adapters codex --workloads Q1,Q2 --faults K1 --triggers answered --tasks 4 --concurrency 2 --seed 108`. Result: 1 of 4 passed. - t02 (Q1, K1 2.7 s after the answer). Answer run `0b357a1f` resumed after the restart. Each resume failed with `native_session_recovery_failed: PRP command run.attach failed: execution_indeterminate`. Then "Automatic recovery stopped", and the task needed a Retry. - t01 (Q2, K1 1.1 s after the answer). Answer run `82c60af9` failed the same way on three same-run resumes. Its provider state kept `lifecycle: "prepared"` with a `spawned` startup attempt for `run.attach`, and that provider process was gone. Each later drain and suspend failed with "provider startup ownership remains unadmitted". The successor run then refused with `retirement_refused=journal_commands_unsettled(run.attach.failed)`. The accepted Retry created no run (`retry_no_new_run`). The earlier wave `w-20261110T033426-s108` hit the same window at different points. It showed `authority_indeterminate` (Q1) and an ambiguous `turn.start` stall (Q2). See Risks for what this pull request does not cover. Expected behavior An answer run that a restart interrupts before it starts provider work resumes by itself. It does not need a Retry. Steps to reproduce 1. Run the wave command above on master. 2. Or run `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-cut-attach.test.ts` on master. Both cases fail with the error from the wave. ## What Changed - `runnerd-codex-transport.ts`: the same-run resume waits on the recovered `run.attach` as before. If that wait fails, the new `cutOffRunAttachmentPayload` checks the journal. It queues one new `run.attach` with the same payload, and waits on it, only when all of these are true: - the latest `run.attach` has an unknown outcome (`indeterminate`, or `failed` with `execution_indeterminate`); - it is a cold attachment (no `paperclipNextAuthority`), with no warm transition and no staged attachment; - every later command is recovery maintenance, as the control plane's shared `isRunnerRecoveryMaintenanceCommand` defines it (`session.snapshot`, `session.goal.get`, `runner.drain`, `runner.suspend`, `runner.shutdown`, `turn.stop`); - no committed event shows provider work (`turn.accepted`, `turn.started`, `item.started`, `tool.execution.started`, `semantic_tool.input`, `runtime_request.created` or `mcp_app.tool_input`). Any other failure is thrown unchanged. - `provider_backend.rs` (Runner): `retire_orphaned_startup_attempt` also treats a `prepared` state with a saved thread and a nonzero process generation as restorable. `attach_run` already documents this state as the settled checkpoint that `run.attach` restores. The other fence rules do not change. The fence is retired only before the spawn, or when the recorded process and its group have exited. - Tests: - New real-runnerd test `runnerd-codex-transport-cut-attach.test.ts`. A real runner and the fake Codex server run an asking run, then a cold answer-run attachment. The test kills both processes and rewinds both journals to "`run.attach` journaled, outcome not recorded", then resumes. One case has a startup fence and one does not. - A unit test for `cutOffRunAttachmentPayload`. It covers each maintenance command, and the cases where it must not re-attach (a definite answer, a pending command, a warm or staged attachment, any work command, a turn event). - The Rust orphaned-startup test also covers `prepared`. A new test shows that a `prepared` state with no saved thread keeps its fence. - `docs/durable-recovery.md`: describes both behaviors. ## Verification - Red/green, real runnerd: - On master, `runnerd-codex-transport-cut-attach.test.ts` fails both cases with `PRP command run.attach failed: ... execution_indeterminate`, the error from the wave. - With only the transport change, the case without a fence passes. The fence case fails with `provider startup ownership remains unadmitted`, the t01 error. - With both changes, both cases pass. - Red/green, Rust: `cargo test -p paperclip-runner-core --lib orphaned` fails `restart_retires_a_spawned_startup_whose_provider_process_exited` for `prepared` without the runner change. With it, all 10 tests pass. - Related transport suites, 8 files and 387 tests, pass: `runnerd-codex-transport`, `-attachment-recovery`, `-cold-restore`, `-warm-attachment`, `-release-resume`, `-cut-attach`, `runnerd-restart-continuation` and `runnerd-saved-continuation`. `tsc --noEmit` for the Runner package passes. - Live proof: the same wave command on this branch. Results follow in a comment. No duplicate provider work. The new attachment is sent only when the journal proves that no provider work followed the cut-off one. No command after it can start or steer work: only the shared recovery-maintenance commands are allowed, and none of them starts a turn. No turn, item, tool-input or runtime-request event is committed. `run.attach` itself starts no turn. In the Codex and ACPX runners, `attach_run` refuses to attach unless the session is settled. That means no active or ambiguous turn, no pending tool call and no live active goal. The old command is never executed again, because the runner refuses an `indeterminate` command. The new attachment is a new command with a new ID. ## Risks - Low-medium risk. The new path runs only after a same-run resume has already failed on an unknown-outcome attachment. Before this change, that run could not continue. - If an attachment's partial effect had changed provider state, the runner's own settled-session check refuses the new attachment. The run then fails as it does today. - Runner release: the Runner change does not change the pack `contractDigest`. It is `sha256:3164357d…` before and after (`scripts/contracts/contract-fingerprints.mjs`). Local runners use the runnerd built with the app, so the fix takes effect with this release. A remote image pack (for example Daytona) that was built before this change keeps the old fence rule until a new image or pack is built and qualified. No release or pin bump is part of this pull request. - Mixed versions during an upgrade: - New server with an old runner: the server re-attaches. A run without a startup fence continues. With a fence, the old runner refuses the new attachment, the same failure as today. - Old server with a new runner: the runner retires the fence, but the old server still fails on the unknown-outcome attachment, as today. - Neither mix adds a new failure. Saved state from older builds needs no migration. - Not covered here, and still open: - A `turn.start` with an unknown outcome where the resumed thread has no active turn (wave `w-20261110T033426-s108` t03). The runner deliberately fails closed for that case. - A warm-runner attachment that may already have been delivered (`authority_indeterminate`). - A Retry that waits on `process_identity_missing` for a run that failed before any process started. A separate pull request will cover it. ## 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 #20146, #19784, #20148 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-11-10 02:20:14 -06:00
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>	2026-11-10 02:19:21 -06:00
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>	2026-11-10 02:14:53 -06:00
Nicky Leach	8809f44b5e	Connect Codex and Claude login to agent computers (#20417 )	2026-11-09 22:12:49 -08:00
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>	2026-11-09 21:42:46 -08:00
DottaandPaperclip	5bad074baa	feat: add individual agent interaction permissions (#20411 ) Add eleven sparse incoming and outgoing restrictions, human CLI/API administration, shared mutation enforcement, and pending-delivery checks. Preserve existing defaults, agent settings, and existing authorization boundaries. Co-Authored-By: Paperclip <noreply@paperclip.ing>	2026-11-09 20:34:04 -06:00
Nicky LeachandClaude Opus 5.5	ff4ba7d319	perf(runner): run launch artifacts in place from a trusted read-only root (#20428 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Rust Runner (`paperclip-runnerd`) starts provider sidecars and proxies from qualified launch artifacts: a Node runtime and the scripts it runs > - Since #12699, Runner copies each artifact into a sealed memfd and checks its SHA-256 on every verification. This defends against a path swap or an in-place rewrite between the check and the exec > - On a hosted agent computer (4 vCPU), the copy and digest of the 127 MB Node takes about 0.26–0.30 s warm and about 1.2 s right after a wake. This is on the path from `session.open` to the sidecar spawn, and again on every warm `run.attach`. Each Runner also holds about 125 MiB of shmem for the copy > - On that computer, the artifacts live on the Runner ROM: a read-only erofs image that the deploy pins by digest. The threat that the memfd defends against does not exist there. On local hosts the artifacts are writable, so the memfd must stay > - This pull request adds an explicit, path-scoped, default-off `--trusted-launch-root <dir>`. Runner runs an artifact in place only when its open descriptor proves that it is on an immutable mount under that root. Everything else keeps the sealed, hashed snapshot > - The benefit is a shorter `session.open` and `run.attach` on agent computers, and about 125 MiB less shmem per Runner, with no change where the flag is absent ## Linked Issues or Issue Description What existing behavior does this improve? Launch-artifact verification in `paperclip-runnerd` (`qualified_launch.rs`). Its callers are the ACPX `verified_transport` (every `session_config`, including the park rebind) and the OpenCode and Cursor Cloud proxy launches. Subsystem affected Rust Runner (`packages/paperclip-runner/runner/crates/runner-core`). Current behavior Every verification opens the artifact, copies all bytes into a sealed memfd, and hashes them. Node is 127 MB. The cost is described in the Thinking Path. Proposed behavior When the host passes `--trusted-launch-root /opt/paperclip/runner` and the root passes its checks, Runner holds each admitted artifact open in place and executes it through `/proc/self/fd/N`. It does not copy or re-hash it. Without the flag, nothing changes. Reason and benefit See the Thinking Path. Refs #12699 (introduced the sealed memfd). Refs #20423 (overlap, see below). ## What Changed - `qualified_launch.rs`: new `TrustedLaunchRoot` and `set_trusted_launch_root` (process-global, set once at startup, like `--task-memory-cgroup`). - Root checks at startup. The path must be absolute, already canonical (no symlink or `..`), a directory, and on a read-only, exec-permitted erofs or squashfs mount. If a check fails, runnerd logs a warning and keeps today's behavior. - Per-artifact checks, on the open descriptor. After today's checks (regular file, not a symlink, not group- or world-writable, open-time identity), the descriptor must be a regular file on the root's device. Its kernel path (`readlink /proc/self/fd/N`) must lie under the root by path components, so `/opt/paperclip/runner2` does not match. Its mount must pass `fstatvfs` (`ST_RDONLY` set, `ST_NOEXEC` clear) and `fstatfs` (erofs or squashfs magic). If any check fails, Runner logs the reason and uses the sealed snapshot. - Admitted artifacts become `VerifiedProcessArtifact::in_place_on_trusted_root` (crate-private). The held file goes through the existing `inherited_artifact` path, so the exec is `/proc/self/fd/N` and `PAPERCLIP_VERIFIED_RUNTIME_EXECUTABLE` still names a descriptor that nested launches can reopen. - New `verify_assigned_executable_launch_artifact` never uses the root. `stdio_mcp.rs` now calls it, so host-assigned stdio MCP binaries always keep the sealed snapshot. - `paperclip-runnerd.rs`: parse `--trusted-launch-root <dir>`. Older runnerd builds ignore unknown flags, so a host can pass it before every Runner understands it. - The trusted path is Linux-only. On macOS and Windows, `TrustedLaunchRoot::open` always refuses. - `tests/runner-e2e/SECURITY.md`: documents how the Kimi and Pi provider-death fixtures behave under a trusted root. ### Decision: the trusted path drops the digest The trusted path does not hash the artifact, not even once per process: - The ROM image is pinned by digest at deploy time. - On the agent computer, the Launcher computes the expected `--acpx-sidecar-command-sha256` and `--acpx-sidecar-script-sha256` from the same ROM files immediately before it starts Runner. A re-hash of an immutable file can only repeat that result. - One hash per process would not help `session.open`, because each task starts a new Runner process. - The mount check is the guard: erofs and squashfs have no write path, so the bytes cannot change while the descriptor is open. ### Fixtures that assert the sealed memfd - Untrusted-mode assertions are unchanged. The fixtures do not pass the flag. - Kimi (`kimi-provider-fault.py` `checked_parent_descriptor`): with a trusted root, the parent descriptor is the ROM's Node file (one link). The existing linked-descriptor branch handles it: same inode as `/proc/<pid>/exe`, and the complete bytes hashed against `nodeSha256`. That branch never accepts the file as a memfd. The existing test `test_linked_descriptor_keeps_regular_single_link_checks_without_memfd_claim` covers it. - Pi provider-death admission binds `/proc/self/fd/3\|7` to the native distribution snapshot's Node. The TypeScript ACPX closure copies that file whether or not a trusted root is set, so the flag does not affect it. ### Overlap with #20423 #20423 (draft) moves the same copy and hash to Runner boot and reuses the result. Where this flag is set (Cloud agent computers), there is nothing left to precompute, so #20423 is mostly redundant there. #20423 still helps hosts without a trusted root (local and sandbox Runners). This PR does not change #20423. ## Verification All runs are on Linux (aarch64, Rust 1.97.1, in a container): - `cargo fmt --manifest-path runner/Cargo.toml --all -- --check` - `cargo check --manifest-path runner/Cargo.toml --locked --workspace` - `cargo test -p paperclip-runner-core --lib -- qualified_launch trusted_root commonjs`: 12 passed. The new tests are: - `an_artifact_under_an_immutable_trusted_root_runs_in_place_without_a_digest`: trusted-root hit, with no memfd, the same inode as the source, and no re-hash. - `an_artifact_outside_the_trusted_root_keeps_the_sealed_snapshot`: a path outside the root is sealed, and a wrong digest still fails. - `a_sibling_with_the_root_as_a_name_prefix_is_outside_the_root`: `rom2/` does not match `rom`. - `a_root_that_is_not_read_only_falls_back_to_the_sealed_snapshot`: the real probe refuses a writable root at startup. A per-artifact mount failure falls back, and a wrong digest still fails. - `the_real_mount_probe_rejects_a_writable_filesystem` - `symlink_and_parent_escapes_from_the_root_fall_back`: a directory symlink out of the root, a `..` path out of the root, and a final-component symlink (still refused). - `a_root_must_be_absolute_canonical_and_a_directory` - `without_a_trusted_root_launch_artifacts_are_sealed_and_hashed`: flag absent, behavior unchanged. - `host_assigned_executables_ignore_the_trusted_root` - `trusted_root_artifacts_execute_the_held_file_and_reopen_it_for_nested_launches`: Runner execs from the held file, and a child re-execs `$PAPERCLIP_VERIFIED_RUNTIME_EXECUTABLE`. Both report the source file as `/proc/self/exe`. - `cargo test -p paperclip-runner-core --test stdio_mcp --test process_supervisor`: 4 and 8 passed. - Ad hoc, not committed: a privileged container with a real loop-mounted squashfs. The real `fstatvfs`/`fstatfs` probe admitted a `ro` mount and refused a `ro,noexec` mount. The unit tests replace only that mount probe, because CI cannot mount filesystems. - Not verified: a run on an agent computer with the real erofs ROM, and the latency and memory savings there. The Cloud Launcher change that passes the flag is a separate follow-up. ## Risks - This change needs review from a security owner before merge. It removes the sealed copy and the per-launch digest for admitted artifacts. That relies on the host's assertion that the root is a digest-pinned immutable image, plus the descriptor checks above. - Misconfiguration fails closed. A wrong, writable, `noexec`, non-canonical or missing root only loses the speedup, and runnerd logs a warning. There is no global "trust everything" switch, and Runner never selects a root from the filesystem it finds. - Residual trust. On an admitted file, Runner no longer detects a ROM image that differs from what the launch profile expected. On the agent computer, the Launcher derives the expected digests from the same ROM. Another host that passes the flag must pin its image the same way. An attacker with root in the guest (who could write the backing block device or remount) is out of scope; such an attacker can already control runnerd. - Allowlist. Only erofs and squashfs are accepted, because they cannot be written through any mount. A read-only bind mount of a writable filesystem is refused. A ROM that changes to another filesystem type will silently fall back to the sealed path (slower, still safe). - No migration. Behavior is unchanged when the flag is absent. Older runnerd builds ignore the flag. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, container builds). Extended reasoning was 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>	2026-11-09 17:46:33 -08:00
Nicky LeachandPaperclip	3a5572c9e5	Extend login PTY contracts for existing computers (#20410 ) ## Thinking Path > - Paperclip manages agent execution across environments. > - Environment providers own execution resources. > - Codex and Claude already share the login PTY transport. > - A login on an existing computer needs its own allocation and deadline. > - Input retries must retain their identity. > - Codex also needs a fixed credential-file read; Claude already returns its token through PTY output. > - This change extends the shared contract and keeps both existing provider parsers. ## Linked Issues or Issue Description Problem The existing login contract assumes a temporary sandbox. It cannot describe a login allocation on an existing computer or export its fixed credential result without generic execution. Proposed solution Add shared allocation purpose, command identity, PTY deadline, and input identity fields for both Codex and Claude. Add one optional credential-result hook for Codex's auth.json file. Alternatives considered A separate Claude login implementation would duplicate the current setup-token parser and owner-bound credential writer. Claude continues to use that existing path. The new artifact hook is Codex-specific because Claude's setup-token result comes through the existing shared PTY output stream. ## What Changed - Export the login contract types from the main SDK entry point. - Forward login purpose and command through plugin allocation and retain them in lease metadata. - Forward the PTY deadline and give each input write a stable identity. - End a login route when the worker cannot confirm input delivery. - Preserve the opening company scope for input, stop, and close callbacks. - Advertise and dispatch the optional fixed Codex credential-result hook. The allocation, deadline, input, output, and cleanup contracts serve both providers. This PR does not enable either provider on an existing computer. The dependent consumer PR connects admission, services, credential completion, and UI for both providers. ## Verification - Close-ordering regression: all 91 host worker-manager tests passed, including concurrent close callers waiting behind a held plugin acknowledgement after input failure. Server TypeScript check passed. All current CI gates, ordinary review, and the selected follow-up review pass on `a39fb59bd1`. - Shared and SDK TypeScript builds passed. - SDK worker dispatch: 31 tests passed. - Host worker-manager: 90 tests passed, including a nested worker-to-host scope guard. - Environment runtime with embedded PostgreSQL: 197 tests passed. - Added absent-hook, input-failure, and allocation-forwarding checks. - A running instance is not required to validate this SDK-only increment. Real provider acceptance follows after the dependent hooks are connected. ## Risks The hook and fields are optional. Existing sandbox providers keep their current path. Failed input delivery now ends its route instead of waiting for the deadline. There is no database migration. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The exact runtime model identifier and context size are not exposed to this agent. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [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>	2026-11-09 16:50:58 -08:00
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>	2026-11-09 18:47:34 -06:00
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>	2026-11-09 18:14:52 -06:00
DottaandClaude Opus 5.5	60a60a0cd1	fix(opencode): attribute unreadable tool-result images to the tool (#20399 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Paperclip runner drives OpenCode agents. It sends their OpenRouter and Paperclip-connection model calls through a local provider proxy. > - An agent often takes a screenshot of its own work and reads it to verify the result. > - OpenCode 1.18.34 cannot put images in OpenAI-compatible tool results. It moves them into a synthetic user message ("Attached media from tool result:"). For a text-only model, it replaces the image with "ERROR: Cannot read image (this model does not support image input). Inform the user." > - The model reads that text as a message from the person. In a chat eval, a DeepSeek worker redid its verification for about 5 minutes, registered its deliverable twice, and ran out of the 15-minute attempt clock. > - This pull request moves that notice into the tool result that produced the image, worded as a runner note, and removes the synthetic user message. > - The benefit is that a text-only agent sees a clear tool outcome and continues its work. It does not mistake the notice for the person. ## Linked Issues or Issue Description What happened? An OpenCode agent on `openrouter/deepseek/deepseek-v4.1-flash-1017` read a PNG screenshot that it made. The next model request contained a user-role message with the text "Attached media from tool result: ERROR: Cannot read image (this model does not support image input). Inform the user." The model reasoned "The previous input from the user was ..." and repeated finished work. Steps to reproduce 1. Run OpenCode 1.18.34 with an `@ai-sdk/openai-compatible` provider and a model that has no image input. 2. Have the model call the `read` tool on a PNG file. 3. Inspect the next chat-completions request. The last message is the synthetic user message above. Expected behavior The model must get the notice as part of the tool result, not as a message from the person. Environment Paperclip runner, OpenCode 1.18.34 (qualified pin), OpenRouter and Paperclip-connection model routes. ## What Changed - Add `packages/paperclip-runner/src/drivers/opencode/tool-media-attribution.ts`. It finds the OpenCode synthetic attachment message only when it directly follows tool results. It moves each "Cannot read ..." or "empty or corrupted" notice into the last tool result of that step as a runner note. It keeps readable media in the synthetic message. It removes the message when no media remains. - The OpenCode provider proxy applies this rewrite before it forwards a request. It re-serializes the body only when something changed. - Messages written by a person are never changed. A person who attaches an image to a text-only model still gets "Inform the user", which is correct for that case. - Add unit tests for the rewrite and an integration test through the real provider proxy. ## Verification - Red: the new proxy test failed before the change (`expected ... to have a length of 4 but got 5`). - Green: `npx vitest run src/drivers/opencode/opencode-server-driver.test.ts src/drivers/opencode/tool-media-attribution.test.ts` (82 tests pass). `tsc --noEmit -p packages/paperclip-runner` is clean. - Real binary: I ran the real OpenCode 1.18.34 binary with a mock OpenAI-compatible server and had the model read a PNG. The captured request has exactly the message shape that the test fixture uses. - `pnpm verify` returned exit 2 (test files outside the resolved graphs). Required CI supplies broad coverage. - A paid live run of `chat-features.runner-opencode.local.task-coordination.delegate-with-context` will follow in a PR comment. ## Risks - Low risk. The rewrite matches the exact OpenCode 1.18.34 strings. If a future OpenCode version changes them, the rewrite does nothing and the old behavior returns. It does not fail the request. - The rewrite is deterministic, so prompt-cache prefixes stay stable across requests. - No persisted state changes. OpenCode history stays as it is. Sessions saved before this change also get the rewrite when they resume. - Providers that do not use the runner provider proxy are not covered by this change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-09 18:11:50 -06:00
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>	2026-11-09 18:04:00 -06:00
DottaandClaude Opus 5.5	50df9a19b9	perf(inbox): read only the execution workspaces Mine rows reference (#20388 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Mine inbox is the first page a board user opens to see the tasks that need them > - The page loaded the company's full execution-workspace summary list and kept task rows hidden until that list arrived > - On a company with thousands of workspaces, that read checks each workspace row against the privacy predicate. It was the slowest request on the page > - The inbox needs workspace names only for the workspaces its rows reference, and only when the view shows workspace data > - This pull request adds an `ids` filter to the summary read, makes the inbox request only referenced workspaces, and stops rows from waiting for workspace data when the view does not show it > - The benefit is that Mine rows appear when the task data arrives, and the workspace read becomes a small bounded query ## Linked Issues or Issue Description What existing behavior does this improve? The Mine inbox (`/:companyPrefix/issues?view=mine`) shows its task rows late on companies with many execution workspaces. Subsystem affected Board UI inbox and the execution-workspace summary API. Current behavior The page calls `GET /api/companies/:id/execution-workspaces?summary=true` with no filter. On a large company this returned 8,758 rows (2.5 MB) and took 12.3 s on the server. The reveal gate included this read, so rows appeared at 13.4 s although `inbox/mine` arrived at 1.6 s. Proposed behavior The inbox requests summaries only for the workspaces its rows reference. Rows appear when the Mine task data arrives, unless the view shows workspace data. Reason and benefit The workspace read becomes a small bounded query, and it leaves the critical path for the default Mine view. Breaking changes None. The `ids` parameter is optional and additive. ## What Changed - `packages/shared`: add `executionWorkspaceSummaryIdsQuerySchema` (comma-separated UUIDs, 1–100 distinct values) and `EXECUTION_WORKSPACE_SUMMARY_IDS_MAX`. - `server`: `GET /companies/:companyId/execution-workspaces` accepts `ids`. Bad input returns 422. The service adds `inArray(execution_workspaces.id, ids)`. The company scope and the actor read condition still apply, so privacy does not change. - `ui/src/api/execution-workspaces.ts`: `listSummaries` accepts `ids`. New `listSummariesByIds` reads ids in chunks of 100 and concatenates the results. - `ui/src/pages/Inbox.tsx`: the workspace query is keyed on the sorted distinct `executionWorkspaceId`s of the rows the inbox holds (Mine, touched, context and remote search issues). It uses `keepPreviousData` and runs only when isolated workspaces are on and at least one id exists. - Reveal gate: rows wait for the workspace read only when workspace data changes what the rows show. This is workspace grouping, a visible Workspace column, or an active search (workspace names are searchable). Otherwise rows reveal without it. - Tests: route ids pass-through and 422 cases (non-uuid, empty, separators only, more than 100); service test for the id filter with company scope and the read condition; UI api chunking test; Inbox tests for "rows reveal while the workspace read is pending when no workspace column or grouping is active", rows waiting when the Workspace column is visible, and "shows the resolved isolated workspace name" with the id-filtered call. Follow-ups (not changed here, they still read the unfiltered list): - `ui/src/pages/LegacyInbox.tsx:772` - `ui/src/components/IssuesList.tsx:985` - `ui/src/components/LegacyIssuesList.tsx:888` - `ui/src/components/NewIssueDialog.tsx:570` (filtered by project and reuse, but not by ids) ## Verification Focused checks on this revision: - `cd server && npx vitest run src/__tests__/execution-workspaces-routes.test.ts src/__tests__/execution-workspaces-service.test.ts -t "summar\|ids"`: 14 passed (embedded Postgres ran the service test). - `cd ui && npx vitest run src/api/execution-workspaces.test.ts src/pages/Inbox.test.tsx`: all passed. - `pnpm --filter @paperclipai/server --filter @paperclipai/ui --filter @paperclipai/shared typecheck`: passed. - `pnpm check:token-gates`: clean. - `pnpm verify` selected 2,481 Vitest files (the shared validator change reaches most suites). I stopped it at a 25-minute limit on a heavily loaded shared host. Before the limit, three server tests outside this change failed: `agent-commentary-skills` (skill mount), `agent-config-read-access` (24.8 s timeout) and `agent-directory-working-copies`. Required CI supplies the full coverage. - Review fix `fc3f82dae0`: when workspace data is visible, the reveal gate also waits while `keepPreviousData` supplies the previous id set's summaries. A new test switches Mine → Recent to a new workspace and checks the Recent row stays hidden until its workspace loads. It fails without the change. - Follow-up commits `bf34f84823` and `8fc743b5ca` add a 100-row Mine test with a counted workspace id scan. It fails with the old `= []` defaults and passes with the stable `NO_ISSUES` default. Performance (local, synthetic fixture): - Instance: isolated worktree instance, `authenticated` mode, signed in as a non-admin company member so the real workspace privacy predicate runs. Production UI bundle (`PAPERCLIP_UI_DEV_MIDDLEWARE=false`). Headless Chromium 1440×900, a new browser context per run (cold HTTP cache and cold query cache). macOS, Apple Silicon, with other local sessions running. - Fixture: 8,750 execution workspaces (each with a source task and a privacy-source key), 38,750 tasks, 150 Mine tasks; the Mine snapshot returns 100 rows that reference 100 workspaces. - Finish event: the first `[data-slot="task-row"]` is in the DOM. \| Measurement \| Before (`02bfe71a6b`) \| After \| \| --- \| --- \| --- \| \| Cold time to rows, median of 7 \| 1,209 ms (1,046–1,312) \| 702 ms (567–1,209) \| \| Workspace summary read, server wait (time to first byte), median of 7 \| 496 ms unfiltered, 2.0 MB \| 18 ms with 100 ids, 22 KB \| The difference of medians is 507 ms. This is close to the unfiltered read's wait on this fixture (about 550–590 ms in the browser runs). The runs were sequential batches, not alternating pairs. Limits: this fixture does not reproduce the 12.3 s server time seen on the development instance. Here the unfiltered read cost 0.5–1.7 s and grew with the company's task count. The browser gain on the original instance should be close to that instance's unfiltered read time, but I did not measure it there. ## Risks - Low risk. The `ids` filter only narrows the existing query. It does not change the read condition or the company scope. - A row whose workspace is not readable by the actor shows no workspace name, as before. - When the Workspace column or grouping is visible, rows still wait for the workspace read. That read is now small. - When the id set changes, the query refetches all chunks. Before a view first reveals, rows wait for the new summaries if workspace data is visible. After that, the previous names stay visible while a refresh loads. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edit, headless browser). 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>	2026-11-09 18:02:55 -06:00
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>	2026-11-09 17:49:34 -06:00
DottaandClaude Opus 5.5	7e93ccb4b3	perf: fetch recent task summaries in one request (#20389 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board UI sidebar shows the person's recent tasks on every page, including the Mine inbox > - The sidebar refreshes those rows through `GET /companies/:id/issues/summaries`, which accepted at most five IDs > - `useRecentTasks` sent the chunks one after another, so a 25-task history cost five sequential requests > - On staging each request took 3–6 s, so the recent-task rows settled about 20 s after load and every request competed with the inbox rows > - This pull request raises the per-request cap to 50 and fetches any remaining chunks in parallel > - The benefit is one summaries request for a typical history instead of a serial chain ## Linked Issues or Issue Description What existing behavior does this improve? The refresh of the sidebar's recent tasks on page load. Subsystem affected Board UI (`ui/src/hooks/useRecentTasks.ts`) and the `/issues/summaries` route bound (`packages/shared`, `server/src/routes/issues.ts`). Current behavior A history of N recent tasks sends ceil(N/5) summaries requests in series. Measured on staging on 2026-11-09 for a cold Mine load: five requests of five IDs, started at 1.0 s, 4.3 s, 8.1 s, 12.3 s and 18.4 s, each taking 3.3–6.1 s. Proposed behavior Up to 50 recent tasks are one request. Longer histories send their chunks at the same time. Reason and benefit The staging server is saturated, so each extra serial round trip delays the sidebar and the inbox rows. One request removes four round trips from a typical load. ## What Changed - `ISSUE_SUMMARIES_MAX_IDS` is now 50 (was 5). The shared validator, the route's 400 message, the service guard and the OpenAPI description use the new bound. - `useRecentTasks` builds all chunks first and fetches them at most three at a time (`RECENT_TASK_SUMMARY_CONCURRENCY`). The recent list has no size cap, so this keeps a long history from becoming a burst. The cache key, omission handling and pruning are unchanged. A failed chunk still fails the batch, so nothing is pruned on a partial failure. - Tests: the route test accepts exactly `MAX` IDs and rejects `MAX + 1`; a hook test checks that at most three chunks are in flight; the API test uses the constant; `useRecentTasks` tests cover a 25-task history in one request and a 103-task history with three chunks in flight together, with and without a failed chunk. - New browser check `tests/perf/collections/recent-task-summaries.spec.ts`: a Mine load with a 12-task history sends summaries requests that each carry all 12 IDs, and the server answers 200 with all 12 tasks. ## Verification - `pnpm verify` (selected Vitest files) passes. - `server`: `npx vitest run src/__tests__/issue-summaries-routes.test.ts src/__tests__/issue-display-status.test.ts` passes (embedded PostgreSQL). - `ui`: `npx vitest run src/hooks/useRecentTasks.test.tsx src/api/issue-summaries.test.ts src/components/task-chat/TaskChatBlockerLinks.test.tsx src/components/CoordinatorSidebar.test.tsx src/components/SidebarRecentTasks.test.tsx` passes. - Browser check against a disposable local instance: `PAPERCLIP_COLLECTION_PERF_BASE_URL=http://127.0.0.1:<port> pnpm exec playwright test --config tests/perf/collections/playwright.config.ts recent-task-summaries.spec.ts`. It passes on this branch. With `master`'s hook and constant it fails: three requests of 5, 5 and 2 IDs. ## Risks - Low risk. The route still validates UUIDs, deduplicates, and applies the same read authorization per row. Blocker attention is computed with set-based queries, so 50 IDs is the same number of queries as 5. - During a rolling deploy, a new UI against an old server would get a 400 for a history above five tasks. The server serves the UI bundle, so both change together on a normal deploy. The sidebar keeps its stored snapshot when the refresh fails. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, browser). 1M-token context window. Extended thinking was off. ## 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>	2026-11-09 17:42:32 -06:00
Nicky LeachandClaude Opus 5.5	5858cef8f9	perf(runner): poll the provider right after an ACK releases it (#20374 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Runner (`paperclip-runnerd`) sends provider events to the controller over one authenticated socket > - Its main loop reads one control frame, then polls the provider only after a read finds no frame > - After the controller ACKs the last sent event, the next read waits the full 250 ms `RUNTIME_READ_TIMEOUT` before the Runner polls the provider again > - A Codex turn start needs one or two such rounds before the Runner reads `turn/started`, so every Codex turn waits 250 ms or 500 ms for no reason > - This pull request reads only the frames that are already queued after a consumed control frame, and then polls at once, when this connection produced the unacknowledged output > - The benefit is 250 ms less time to `turn/started` for each ACK round, and faster event delivery after each ACK ## Linked Issues or Issue Description This is an enhancement. The fields follow `.github/ISSUE_TEMPLATE/enhancement.yml`. What existing behavior does this improve? Codex turns wait for the Runner read timeout before the Runner sends `turn/started`. Subsystem affected The Runner (`paperclip-runnerd`) durable loop in `packages/paperclip-runner/runner/crates/runner-core/src/durable/`, for the local Codex provider. Current behavior On a hosted test deployment (48 h, 114 Codex turns), `runner.transport.started_event` for Codex has two modes: \| Codex turn \| n \| `started_event` p50 \| \| --- \| --- \| --- \| \| no provider cold start in the trace \| 57 \| 468 ms (all < 700 ms) \| \| provider cold start in the trace \| 57 \| 918 ms (51 of 57 ≥ 700 ms) \| For Claude (ACPX) turns, the p50 is 90 ms. ACPX returns `turn.started` with the command result. Codex reads `turn/started` from the provider in a later poll. Proposed behavior The Runner polls the provider as soon as the controller ACK releases the poll gate. The time to `turn/started` then follows the controller round trip, not the read timeout. Reason and benefit - Real Codex (`codex app-server` 0.157, local) sends `turn/started` 2–16 ms after its `turn/start` response. The provider is not the cause. - `runner.transport.started_event` starts when the `turn.start` command result arrives. It ends when the controller commits `turn.started`. In this time the Runner sends `turn.accepted`, waits for its ACK, waits 250 ms, polls, and sends `turn.started`. - On the first turn after a provider launch, the first poll returns only the launch diagnostic that the executor retained. A second ACK round and a second 250 ms wait come before the poll that reads `turn/started`. This is the difference between the two modes above (about 450 ms: 250 ms wait and one event round). - So 250 ms (warm turn) or 500 ms (first turn) of each Codex `started_event` is the read timeout. The rest is one or two controller event rounds (about 200 ms each on that deployment), less than 20 ms of Codex time, and the 10 ms pump interval of the TypeScript transport. - The expected benefit is 250 ms less per warm Codex turn start and 500 ms less for the first turn after a provider launch, plus faster Codex event delivery after each ACK. Breaking changes None. No protocol, schema, or controller change. ## What Changed - `AuthenticatedTransport::receive_queued_json` reads with a 1 ms timeout and then restores `RUNTIME_READ_TIMEOUT`. - New `CommandExecutor::provider_poll_is_live` hook (default `false`). The local Codex executor returns `true` only when it owns a running provider with no open startup attempt, so a poll cannot restore or launch a provider. The native executor forwards it for the local Codex facade only. - In the steady-state loop, after a consumed control frame, the Runner uses `receive_queued_json` when it has sent output that this connection produced (a command or a provider poll) and the executor reports a live provider. If no frame is queued, it polls the provider at once. - When a provider poll returns no output, or when no frame was consumed, the Runner uses the 250 ms read as before. A 1 ms read follows only a consumed frame, so an idle loop does not spin. - The replay boundary is the highest source sequence before the welcome's pending commands run. A fresh command delivered in the welcome counts as this connection's output. Replayed commands only resend their stored results. - Replayed output from an earlier connection or process, ACPX, and the other providers keep the 250 ms read. The controller can still send its next command before a poll restores a provider. (Without the live-provider gate, six ACPX source-recovery tests in `runnerd-acpx-source-reconciliation.test.ts` failed: the fast poll restored the ACPX sidecar before the next recovery command.) - Control frames that are already queued behind the ACK are still read and executed before the provider poll. The ACK gate in `poll_executor_events_after_controller_ack` is unchanged. ## Verification - `cargo test -p paperclip-runner-core --lib durable::` (121 passed). Five new tests in `src/durable/tests/ack_release_poll.rs`, included by the transport test module like `hosted_retention.rs`: - `acknowledged_output_polls_the_provider_without_waiting_out_the_read_timeout`: the provider poll follows the ACK read in less than `RUNTIME_READ_TIMEOUT`. Without the change it fails (the poll waited 280–356 ms). - `control_queued_behind_an_ack_runs_before_the_next_provider_poll`: a command sent in the same write as the ACK executes before the provider poll. - `welcome_command_output_polls_the_provider_without_waiting_out_the_read_timeout`: the output of a fresh command in the welcome also releases the poll at once. With the replay boundary taken after the welcome commands, it fails. - `replayed_output_ack_keeps_the_read_timeout_before_the_provider_poll`: after a reconnect, a command sent 50 ms after the ACK of replayed output executes before the provider poll. Without the replay condition it fails. - `restorable_provider_ack_keeps_the_read_timeout_before_the_provider_poll`: when the executor does not report a live provider, a command sent 50 ms after the ACK executes before the provider poll. - `npx vitest run src/live/runnerd-` in `packages/paperclip-runner` with the new debug `paperclip-runnerd`: run with `--no-file-parallelism`. Before the welcome-boundary fix: 576 passed, 3 skipped. After it: 575 passed, 3 skipped, and 1 failure once (`stops a lost runnerd's provider and escaped tool job from its lineage journal (journal=true)`, a SIGKILL timing test) while `cargo test` ran at the same time. `runnerd-codex-transport.test.ts` then passed 293 of 293 three times with this build, and three times with the base build. - Local before and after measurement with the real Runner, the TypeScript transport, and the fake Codex app-server (8 turns, 2 runs each, debug build): \| span \| before p50 \| after p50 \| \| --- \| --- \| --- \| \| `runner.transport.started_event` \| 377 ms (first turn 682–695 ms) \| 119–122 ms (first turn 117–119 ms) \| \| `runner.transport.turn_start` \| 462–471 ms (first turn 775–780 ms) \| 204–212 ms (first turn 247–251 ms) \| - The saving on the hosted deployment is not measured. From the analysis above, the expected saving is about 250 ms for a warm Codex turn and about 500 ms for the first turn after a provider launch. ## Risks - Only local Codex gets the fast path. ACPX streaming keeps the old 250 ms wait after each ACK. A later change can opt ACPX in when its poll can prove that it does not restore the sidecar. - More provider polls during Codex streaming: a poll can now follow each ACK at once instead of after 250 ms. Batches can become smaller, so the provider state save for each batch can happen more often. Each event already has its own durable save. - A command that the controller sends more than 1 ms after its ACK can now arrive after a provider poll. This was already possible after 250 ms. The change applies only when this connection produced the output, so the provider is already active. - If the Runner cannot restore the 250 ms read timeout, it treats the socket as failed and reconnects. It does not continue with a 1 ms timeout. - No protocol, schema, or controller 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 (`claude-opus-5-5`) in Claude Code, with tool use (shell, code search, Honeycomb queries) and extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] 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>	2026-11-09 15:38:58 -08:00
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>	2026-11-09 17:37:20 -06:00
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>	2026-11-09 17:35:13 -06:00
DottaandClaude Opus 5.5	09241442ea	fix(server): let a native reviewer approve its review stage (#20373 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution policies can require an independent review stage before a task is done > - A native (Runner) agent that is the active reviewer had no way to record its decision > - `PATCH status: "done"` is refused for a native run that owns the task (#18411), and `paperclip_finish` accepts the report but leaves the stage pending > - The reviewer then searches the API until a deadline, and the task stays in review with nobody able to decide it > - This pull request records the final-stage approval on `PATCH` without releasing the run's lock, and lets the run's own finish report set the final status > - The benefit is that an agent reviewer can actually approve its review stage, in one short turn ## Linked Issues or Issue Description Refs #18411 (added the native status guard). Related: #20264 (explicit stage-decision operation, still open; this change is narrower and compatible with it). What happened? A native reviewer agent was woken as the active `review` participant. It read the deliverable and wrote its review in about 100 seconds. It then called `paperclip_finish` with `done`. The tool answered "Completion report accepted; the task's configured review stage is still pending". It then tried `PATCH /api/issues/{id}` with `status: "done"` and a comment. The route refused it with `native_run_status_reported_by_finish`. The agent searched the API for 2 more minutes and found no other path. The stage stayed pending. When the run ends with `done`, the status arbiter sees the pending stage and keeps the task in review (`governed_gate_pending`), so the stage can never resolve. Expected behavior The active review/approval participant can approve its stage from a native run. The task reaches `done` after the approval. The implementer still cannot approve. A run that is not the active participant still cannot change the task status. Steps to reproduce 1. Create a task with an execution policy that has one `review` stage with a native agent participant. 2. Move the task to `in_review` so the stage is pending and the reviewer agent wakes. 3. In the reviewer run, call `PATCH /api/issues/{id}` with `{ "status": "done", "comment": "Approved: ..." }`. 4. Observe `409 native_run_status_reported_by_finish`. Call `paperclip_finish` with `done` and observe that the stage stays pending. ## What Changed - `PATCH /api/issues/:id`: when the requester is the native run that holds the task lock and is the active participant, a `status: "done"` request on the final stage now records the approval decision (`executionState` completed, decision row, activity). The route removes `status` from the write, so the task stays `in_review` and the run keeps its execution lock. The run's `paperclip_finish` report then sets `done` through the normal status arbiter. - An intermediate-stage approval (which would hand the task to the next participant) and every other status change from such a run keep the existing `native_run_status_reported_by_finish` refusal. - `paperclip_finish` from the active participant (`done` or `needs_review`) while its stage is still pending is now rejected with the exact decision path, instead of being accepted with "stage still pending". - The native finalizer now treats the final-stage approval that the run itself recorded (read from the `issue_execution_decisions` row, bound to this run and agent) as the completion authority for its clean `done` report. Before this, a reviewer that cited free-text evidence had its report graded as incomplete, and the task was reopened as `in_progress` with a new wake for the reviewer. Failed verification, blocking remaining work, attention requests, other dispositions, and a still-pending stage keep the old outcome. - The reviewer/approver wake text names the decision path (`PATCH` with `status` and a required decision comment) and says a completion report does not record the decision. A Runner reviewer that needs changes is told to report them with `paperclip_block`, because a changes request from the run that owns the task is a reassignment that stops that run. - The locked review-policy recheck treats the deferred approval as a verdict, so a policy changed to `human_only` before the locked write still refuses it. ## Verification - `pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/issue-execution-policy-routes.test.ts server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/issue-execution-policy.test.ts packages/adapter-utils/src/server-utils.test.ts`: 5 files, 595 tests pass. - `pnpm exec vitest run server/src/services/native-runtime/native-completion-feedback.test.ts`: 29 tests pass (embedded PostgreSQL), including the decision-row binding for `executionStageApprovedByRun`. - `pnpm exec vitest run server/src/services/native-runtime/status-arbiter.test.ts server/src/services/native-runtime/native-run-finalizer-telemetry.test.ts server/src/services/native-runtime/native-stale-owner-completion.test.ts`: 72 tests pass. - New route tests fail without the route change (409 instead of 200/422) and pass with it. The new feedback test fails without the feedback change ("Completion report accepted" instead of a rejection). - `npx tsc --noEmit -p server` and `-p packages/adapter-utils`: pass. - A route test changes the review policy to `human_only` between the first check and the locked write; the deferred approval is refused (403). - Live full-stack chat eval `task-coordination.independent-review` (Runner, OpenCode) with this change: passed in GitHub Actions run 42705964209 (eval branch with the first two commits cherry-picked). The reviewer run recorded its approval 24 s after its review brief was released, reported `done` 14 s later, and the finalizer set the task to `done` with reason `execution_stage_approved`; total reviewer run 73 s. The earlier run 42702224713 exposed the finalizer evidence issue that the second commit fixes. ## Risks - Low to medium. The change is limited to one request shape: a native run, holding the lock, that is the active participant of a pending final stage, sending `status: "done"`. All other requests keep the old guard. - Between the approval and the finish report, the task shows `in_review` with a completed `executionState`. If the run fails before it reports, the existing native recovery owns the task, as for any other run. - Previous-release story: no persisted shape changes. A reviewer run that started before the deploy could not have recorded a deferred approval (the old route refused it), so the new finalizer input is false for it and the old outcome applies. - Intermediate-stage approvals and changes requests from native runs are still refused by the route. A Runner reviewer reports required changes with `paperclip_block`, which keeps the stage pending for a person. A self-service changes request needs the deferred handoff semantics in #20264. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-09 17:11:42 -06:00
DottaandPaperclip	facd409073	perf(workspaces): index legacy workspace companion lookups (#20372 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration must retain prepared sources and recovery holds. > - Companion discovery reads saved workspace references from run profiles. > - The selector can scan large saved profiles even when it returns few rows. > - This pull request adds one company and workspace expression index. > - Companion discovery uses the same expression as existing migration lookups. > - The result preserves each source check and reduces history scan work. ## Linked Issues or Issue Description What happened? The companion selector used a JSON path expression with no matching index. A read-only plan on about 22,000 run rows showed a sequential scan over a table with about 1.3 GB of table and TOAST storage. A repeated inspection reached its statement timeout. Existing migration lookups use a different JSON operator form, so one expression index must match both callers. Expected behavior Use a company-scoped indexed lookup. Keep prepared and finalized references, the exact text comparison, native issue references, and run order. Keep all source and recovery validation. Steps to reproduce 1. Retain many run profiles with saved workspace references and wide JSON fields. 2. Inspect a workspace with few matching companion runs. 3. Explain the company and saved-workspace selector. Without the new index, it can scan run history and read unrelated JSON values. Paperclip version or commit Baseline: `0e3fdebea8257bdff11573081c76b306f12db230`. Deployment mode Server with PostgreSQL. The local proof uses disposable PostgreSQL 18.1. The observed server plan used PostgreSQL 17.11. Related: [batched companion discovery in #19735](https://github.com/paperclipai/paperclip/pull/19735). The existing batching remains in place. The duplicate search found no open workspace index change. This branch also carries the isolated API route-summary correction shared with [#20370](https://github.com/paperclipai/paperclip/pull/20370). ## What Changed - Add one concurrent index on company ID and the saved native workspace ID expression. The index has no state, runtime, or status predicate. - Use nested JSON operators in companion discovery. Existing migration lookups already use this expression. Keep both accepted states and the same ordering. - Register the migration on the existing top-level migration path. Retain its advisory lock, interrupted index repair, and completion journal behavior. - Add a real PostgreSQL migration and ordered-result test. Update the existing companion query counter for the equivalent SQL form. - Generate the schema snapshot and retain the five newest snapshots. - Describe the task case-list route as “Find the cases for a task.” This keeps the route on the first discovery search page. Search logic and test assertions are unchanged. ## Verification - `pnpm --filter @paperclipai/db exec vitest run src/heartbeat-workspace-companion-index.test.ts --maxWorkers=1`: 1 passed. It builds the concurrent index through the canonical migrator, checks index validity, preserves prepared/finalized and cross-issue/null-issue results, excludes foreign and malformed references, and checks one journal entry after a second migration call. - `pnpm --filter @paperclipai/server exec vitest run src/services/remote-workspace-online-source.test.ts -t 'discovers all Git companions' --maxWorkers=1`: 1 passed; 86 not selected. This uses the real source owner and retains finalized recovery holds, company isolation, and a fresh obligation between inspections. - `pnpm --filter @paperclipai/db typecheck`, migration numbering/safety checks, and `git diff --check`: passed. - One small local SQL comparison used the captured baseline selector, the changed selector, 3,012 synthetic rows, about 37.8 MB of table storage, and the new expression index. Ordered results were identical. One client timing changed from 8.70 ms to 0.86 ms. This is one local warm-cache comparison, not a server latency claim. Hosted improvement is unverified. - Initial setup attempts failed before the affected test ran: registry access, local tsx IPC, and a missing SDK build. The canonical Node 24.22.2/pnpm 9.15.4 setup and SDK prerequisite resolved these setup failures. The local SQL timing client used Node 26.4.0; the focused application tests used Node 24.22.2. - [The first CI run](https://github.com/paperclipai/paperclip/actions/runs/42705129724/job/129460702073) failed the inherited case-history API first-page search assertion. The same failure was reproduced on the unchanged base: the route ranked sixth for “case for task.” The isolated route-summary correction from #20370 passed the same three assertions. The search algorithm and assertions were not changed. - No public command changes require a documentation update. The schema and migration record the added index. No hosted DDL or deployment was performed for this PR. - Final head `c50101dabf46ade42b545d2051d724d93f413ccd`: [full CI](https://github.com/paperclipai/paperclip/actions/runs/42707623920) passed with 60 successful checks and three configured skips. [Fresh Greptile review](https://github.com/paperclipai/paperclip/pull/20372#issuecomment-6752305627) is 5/5 on this head with no open threads. ## Risks - The index adds storage and write cost when a run profile changes. - A concurrent build reads the table and can wait for old transactions. The existing migration path runs it outside a transaction and can repair an interrupted invalid index. - Existing nested-operator lookups can use the index. The companion selector needs this code change to use it. Other JSON path queries are unchanged. - The small local result does not establish end-to-end import speed or PostgreSQL 17 hosted latency. Fresh deployed measurement remains necessary. ## Model Used OpenAI Codex, GPT-6 family. The exact runtime model ID and context window are not exposed in this session. Used reasoning, code editing, shell tools, and local PostgreSQL tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-11-09 16:17:13 -06:00
DottaandClaude Opus 5.5	1baac27888	feat(server): chat bootstrap reads for instant task opens (#20355 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People open tasks from the task list to see the latest messages and to reply to an agent > - Before the task chat shows its first frame, it waits for about ten separate reads: the issue, recent comments, comment input facts, recent activity, recent runs, interactions, attachments, live runs and the active run > - On a busy instance each read waits for the database, and the slowest read decides when the chat appears. A cold open takes 1.5–4 s on staging > - This pull request adds one read that returns all of that data for up to ten tasks, in the same JSON shapes as the existing endpoints, plus a single-task form > - The benefit is that the task list can preload its top tasks, and a cold open needs one round trip instead of about ten. The UI change is in the stacked pull request ## Linked Issues or Issue Description Problem or motivation A task open waits for about ten reads. With the database under load, these reads queue, and a cold open takes seconds. The list cannot preload a task cheaply, because each task costs about ten requests. Proposed solution `GET /api/companies/:companyId/issues/chat-bootstrap?ids=` returns the first-frame data for up to 10 tasks. `GET /api/issues/:ref/chat-bootstrap` returns it for one task. Each field equals the matching single endpoint's JSON exactly. Alternatives considered - Prefetch the existing endpoints for each list row. For 30 rows this sends about 210 requests to a database that is already the bottleneck. - A new projection with different shapes. This would duplicate client merge logic, and freshness rules could drift. Roadmap alignment This is part of the task-loading performance plan in #20276. It follows #20277, #20284 and #20286. ## What Changed - `packages/shared`: - `ISSUE_CHAT_BOOTSTRAP_MAX_IDS` (10). - `issueChatBootstrapQuerySchema`: UUIDs or identifiers, normalized, deduplicated and strict. - The `IssueChatBootstrap` and `IssueChatBootstrapItem` types. - New `server/src/services/issue-chat-bootstrap.ts`: - Resolves the refs inside the company and applies the `issue:read` decision to each task. - Reads the tasks with a concurrency of 2, and returns them in request order. - `server/src/routes/issues.ts`: - The core issue read and the recent-comments read are now shared helpers, so the bootstrap and the single endpoints use the same code. The core view keeps its Server-Timing header. - Adds the batch route and the single-task route. - Excludes the single-task route from the issue ETag middleware, so a fetch without validators always gets a 200 with `Cache-Control: private, no-store`. - New `server/src/services/issue-run-presence.ts`: - The live-runs and active-run payload builders, moved from `agents.ts` without change. - The existing routes call them, and their behavior is unchanged. - `server/src/routes/openapi.ts`: entries for both routes. ## Verification - `cd server && npx vitest run src/__tests__/issue-chat-bootstrap-routes.test.ts` passes 17 tests. They run on embedded Postgres with privacy enforcement on. They cover: - Each field deep-equals its single endpoint's JSON, for an ordinary task and for a conversation task. - The single-task route equals the batch item, by UUID and by identifier. It is never answered with a 304. - Items keep the request order, with UUIDs and identifiers mixed. - Private, other-company, missing and unknown tasks are omitted, and none of their data appears. The single-task route returns 404 for them. - Malformed, extra or repeated parameters return 400. More than 10 ids also return 400. - A task-bridge key gets 403. - A 10-task request writes nothing: no inserts, updates or deletes, and no activity-log or comment changes. - The issue, agent, activity, live-run, cold-open, summaries, ETag and OpenAPI route suites pass: 56 files, 1,771 tests. - `cd server && pnpm typecheck` passes. - Query cost, local embedded Postgres, one 10-task request: about 49 SQL statements per task, 55–130 ms in total. A test fails if it goes above 60 per task. ## Risks - Database load: the stacked UI preloads up to 30 tasks, about 1,500 statements at idle time. This is the same work as opening those tasks, but it is done before the person clicks. The work runs in sequential requests of 10, with 2 tasks at a time. Most of the cost is existing run decoration. Live runs and the active run decorate the same running run, so that work runs twice. Sharing it is a follow-up. - Stricter access: live runs and the active run now apply the `issue:read` policy inside this endpoint. The existing routes keep their company-only check. - Hidden tasks are returned, as `GET /issues/:id` returns them. - No migration and no persisted-state change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used in Claude Code with tool use (shell, browser automation, file editing, subagents). ## 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 Opus 5.5 <noreply@anthropic.com>	2026-11-09 15:05:41 -06:00
DottaandPaperclip	c67909cb0f	fix: keep tool approvals valid through OAuth refresh and delayed review (#20340 ) ## Thinking Path > - Paperclip lets people manage agent work and approve app actions. > - An approval must bind the action that the person reviewed. > - OAuth access tokens can expire while that person is away. > - The old target snapshot treated each token refresh as a new authorization. It also imposed a one-hour review deadline. > - This pull request separates token renewal from authorization identity and keeps the human decision durable. > - Current access, the reviewed arguments, meaningful target changes, task closure, and execution receipts still control dispatch. > - People can approve later without repeating a decision after routine connection maintenance. ## Linked Issues or Issue Description What happened? A pending app action returned `approved_tool_target_changed` after its OAuth tokens refreshed. The tool and connection settings were unchanged. The same card also expired after one hour. A sign-in problem could discard an accepted decision before provider dispatch. Expected behavior The original card remains reviewable after routine OAuth refresh and a server restart. A later approval runs the exact reviewed action once. A connection repair preserves the decision while Paperclip verifies the same authorization. Meaningful changes require a new review. Steps to reproduce 1. Connect an OAuth MCP app and configure an action to require approval. 2. Request the action, then refresh its access and refresh tokens before approving. 3. Approve the original card. Before this change, it returns HTTP 409 even when the authorization is unchanged. 4. Wait more than one hour. Before this change, the card expires without a task-specific deadline. Paperclip version or commit Reproduced from master at `76254ead2229`. The final branch is rebased onto current master. ## What Changed - Add versioned approval target comparison. New snapshots bind credential identity epochs, consent, permissions, ownership, connection configuration, tool contracts, and credential references. Authenticated OAuth refresh preserves identity. Manual credential replacement does not. - Remove the implicit one-hour deadline from ordinary app action reviews. Keep explicit deadlines and terminal-task expiry. Synchronize the nullable interaction deadline in shared contracts and UI. - Keep accepted, undispatched actions when sign-in is required. Show a saved-approval repair state. Resume on committed connection changes after target and access revalidation, without an idle retry loop. - Preserve the existing execution claim, result receipt, and uncertain-outcome no-replay rules. - Explain meaningful target changes without inviting the person to retry the stale card. - Add a production task-thread Storybook example and document compatibility and recovery behavior. ## Verification - Red: production-refresher invalid-grant handling and formal-approval withdrawal during a sign-in wait also reproduced as failing regressions before their fixes. - Red: before implementation, the local OAuth HTTP fixture failed for both public and confidential clients with `approved_tool_target_changed` after token refresh. - Green: the extended fixture passes for public and confidential clients with personal and shared grants. It covers actual local HTTP token exchanges, multiple refreshes, a rebuilt gateway, an ended originating run, approval after 25 hours, duplicate approval, preserved sharing arguments, sign-in waiting, and event-driven repair. The gateways use the production OAuth refresher; an invalid refresh preserves the accepted decision before provider dispatch. Omitted-scope responses cover equivalent scope-reporting changes. The provider is an isolated local fixture, not a deployed Notebook service. - Negative controls refuse changed consent, scopes, tool definition, destination, and manual credential replacement before provider execution. - Existing gateway service, connected-tool acceptance, secret owner, secret service, unsigned-review queue, and connection-health suites passed. Explicit-deadline fixtures still prove expiry behavior, including deadlines on accepted sign-in waits. A withdrawn formal approval cancels a waiting action before connection recovery. - Approval card, task-chat, error-copy and target-comparison tests passed. Server and UI typechecks passed. Token gates and Storybook catalog checks passed. - Inspected the new saved-approval repair card in local Storybook using the production task thread. It shows a reconnect link and no second approval button or false running state. - `pnpm verify --explain` returned exit 2 and requested broad CI coverage. [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507) passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build, typecheck/release registry, general and serialized tests, and all eight browser shards. - Greptile reviewed the same final head at 5/5. Both findings from the first review are fixed and their threads resolved; no open follow-ups remain. ## Risks - No database migration is needed. The new comparison reuses the vault identity epoch. Only authenticated OAuth refresh code can preserve that epoch; HTTP credential-write schemas cannot select that behavior. - Earlier signed requests keep their original exact-version checks and saved deadlines. Already-expired cards are not revived. A historical card whose credentials already changed needs a fresh review. - A new consent, account, permission set, destination, or manually replaced credential must not inherit the old authorization. Reconnection continues automatically only when revalidation proves the same saved authorization. - The interaction enrichment now permits a null deadline. Existing string deadlines remain valid. The web client and shared validator change together. - This PR does not deploy to staging or perform a live Notebook publication. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## 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>	2026-11-09 13:46:28 -06:00
DottaandPaperclip	0e325e8d52	fix(runner): settle saved handoffs after verified shutdown (#20336 ) ## Thinking Path > - Paperclip manages AI agents and their work. > - An agent can save a question or approval handoff before its provider stops. > - A missing provider terminal notification does not prove that the saved work failed. > - Runner already verifies provider exit, event drain, tool settlement and suspension. > - This change uses that proof to settle the same saved handoff. > - Unproven shutdown holds continuation and explains that the work is saved. > - Recovery must never repeat the completed work or allow overlapping execution. ## Linked Issues or Issue Description Refs #20330. That PR extends the Codex terminal drain window. This change handles the separate case where no terminal arrives but the owned shutdown can prove stopped execution. What happened? A native run saved a human handoff, accepted cancellation, and later stopped its provider. The missing terminal notification still made the attempt fail and show “Couldn’t finish this attempt.” Expected behavior Keep the saved handoff. Accept its yielded result after either an authoritative provider terminal or verified shutdown of the exact execution. If shutdown remains uncertain, retain the handoff and explain the hold without offering a duplicate Retry. Steps to reproduce Save a governed wait, acknowledge interruption, and omit the provider terminal. The new regression uses the real Runner binary and a controlled Codex process. The old runtime rejects the handoff instead of asking the existing shutdown owner to settle it. Deployment mode Native Runner with Codex. Local verification used macOS arm64 and Node 26.4.0. Hosted sandbox recovery has not been qualified. ## What Changed - Add an optional driver shutdown-settlement operation. Runner returns proof only after its existing stop, drain, tool settlement and suspension barrier passes. - Bind the receipt to the run, normalized session and turn. Persist it with the saved wait and validate it during recovery. - Complete the same yielded handoff after verified shutdown. Retire the stopped session from warm reuse. Do not invent a provider terminal or complete usage. - Retain a named shutdown-pending condition under the existing bounded same-run recovery policy. Preserve actual provider failures and integrity failures. - Show saved-work shutdown guidance in task and inbox presentation. Keep genuine approvals authoritative and disable duplicate Retry. - Document the completion and restart contract. The checkpoint field is additive; older checkpoints retain the terminal-or-shutdown path. ## Verification - Red: the new runtime regression failed against the original implementation because verified shutdown was never requested. Server presentation and error-classification regressions also failed before the fix. - Red: both inbox shutdown states showed generic text before the inbox change. - Green: Runner runtime, shutdown and harness-backend suites pass 301 tests. Tests cover missing terminals, failed stop proof, wrong identity, cancellation, late proof, genuine provider failure, and restart after receipt persistence. Recovery completes once without another model turn. - Green: the native executor and task-presentation suites pass 1,371 tests. Execution projection, inbox and blocker presentation pass 125 tests. Two test-authoring mistakes were corrected: an integrity-error text expectation and an assertion for a nonexistent projection field. Their initial failures remain in local logs. - Runner TypeScript and full server layered type checks pass. Token gates and diff whitespace checks pass. - Live provider acceptance: real Runner and Codex with gpt-6.1-sol, one disposable local workspace, one tool call, and one deliberately dropped terminal notification. The saved file, bound settlement receipt, one accepted yielded result, one provider turn, confirmed provider exit and suspended Runner all passed. The tool handler and persistence port were controlled. This does not claim a full server or hosted journey. - The full browser question-and-answer cell stopped before execution because OPENAI_API_KEY was absent. No model work ran in that attempt. Hosted sandbox recovery and the full browser continuation journey remain unverified. - `pnpm verify` returned exit 2 because the execution-spec change requires broad CI. Required CI will supply that coverage; a full local repository suite was not repeated. - Review correction in `8553da244e`: a failed trailing-event write could hide a provider failure. A new regression reproduced red, then passed after all ordinary drain errors retained the named pending condition. No settlement receipt or result can be accepted after that error. The live missing-terminal check passed again on the corrected runtime. - The real PostgreSQL control-plane regression and three existing governed-wait controls pass. The new case persists one accepted yielded result, a bound shutdown receipt, and the original pending human question without a provider terminal. - First CI revision `7c2cb9b1`: test-program typecheck found a missing `PrpEvent` annotation in the new fixture. The annotation is fixed and the Runner test-program typecheck passes. Runner support also timed out in the unchanged `registered roots remain protected even when equal to genuine public static bytes` case after 30 seconds. One unchanged local diagnostic passed in 3.69 seconds overall. The hosted timeout cause remains unknown; no timeout, fixture, or assertion was weakened. [Original failed support job](https://github.com/paperclipai/paperclip/actions/runs/42670851821/job/129346900133). - Rebased onto the merged #20330 and current recovery changes. The combined branch passes 1,402 executor/presentation tests. Both prompt and 18-second provider terminals still pass. A warm session with no terminal passes after the full 30-second drain and verified stop/suspension. Its initial overly broad assertion rejected a nonterminal turn event; the corrected assertion rejects every terminal type. The rebased CI gate was cancelled before any runner or step started; its cause is unconfirmed and it supplies no validation credit. Final revision `b49ec5a144` starts fresh CI and review. - Final-head CI attempt 1 failed before creating jobs. GitHub reports an internal server error (correlation `2c576b07-b67f-4179-a69f-74f7d99cd926`), followed by empty runner-selection outputs for the aggregate jobs. One unchanged-commit retry was requested; the initial admission failure remains recorded. [CI run](https://github.com/paperclipai/paperclip/actions/runs/42673159868). - Current blocker: final head `b49ec5a144e94e41e96e2508a79bd9aec91fe45a` has a fresh 5/5 Greptile review and zero unresolved threads, but required CI is not complete. GitHub reports an active [Actions and Checks API incident](https://www.githubstatus.com/incidents/tp8cghg7tlc4). Attempt 2 ran only part of the full selected graph. Build, canary, Rust, sandbox providers, admitted browser shards and other completed test jobs passed. The E2E aggregate correctly failed because `policy` and `e2e_shards` had `abandoned` results. Server shard 5/12 received a runner shutdown signal at 18:36:55 UTC and its test step was cancelled; its job still reports in progress. The shutdown cause is not established. The separate review workflow had the same pre-execution internal-error class and passed one bounded retry. No further retry was submitted during the incident. Resume with the missing/cancelled coverage after GitHub recovers; do not count the absent typecheck, Runner or support jobs as passes. [Failed coverage gate](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129354318236), [interrupted server shard](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129350854174). ## Risks - A missing terminal can use one additional bounded shutdown interval. Unknown shutdown still blocks continuation. - Provider terminal and usage accounting keep their separate meaning. The receipt proves stopped execution and settled tool outcomes, not model success. - Recovery rejects a foreign receipt, a later turn, an active turn with a settled receipt, and an actual failed provider terminal. - There is no database migration. Older checkpoint shapes remain readable. Drivers without this optional proof retain a named pending state. - This PR does not deploy the fix or repair an existing hosted task. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and local code execution. The exact serving model ID and context window are not exposed in this session. Live test inference used gpt-6.1-sol. ## 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>	2026-11-09 13:46:16 -06:00
Nicky LeachandPaperclip	79bf733012	fix(runner): share the bounded ACPX cleanup proof (#20332 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Runner keeps a lifetime fence until the original ACPX provider stops. > - A recovered Runner must prove that fence is free before it clears saved cleanup state. > - Suspension and other recovery paths already wait for transient listener holders. Recovered shutdown used one immediate probe. > - That difference made parallel Rust tests fail after they dropped their original listeners. > - This pull request uses the existing bounded proof for recovered shutdown and removes the repeated test retry. > - The benefit is one cleanup rule, with the same fail-closed proof and no new work on normal task dispatch. ## Linked Issues or Issue Description Fixes: #20210 Thanks to @genius-0963 for the test-only fix proposed in #20321. That related alternative adds a test-only shutdown retry. This change applies the existing bounded cleanup proof in the shared shutdown path and removes the same retry pattern from native-provider integration tests. It also tests a single shutdown call while the original quorum is still held. ## What Changed - Use `await_provider_lifetime_fence` for a recovered executor with no live session and unconfirmed provider exit. Delete the unused one-shot helper. - Keep the acquired quorum held through the existing durable state write. A failed proof leaves `providerExitUnconfirmed` set. - Give the shared timeout message wording that applies to shutdown and suspension. - Extend the recovery test with a worker-start signal. Hold the original listeners until the single shutdown call is waiting, then release them and assert successful cleanup without a provider session. - Use the same bounded proof in the retired-quorum fixture test. Remove the duplicate retry loop from the native-provider test helper. - Document the bounded recovery cleanup wait. ## Verification - macOS arm64, Rust `1.97.1`, source base `3a2f71db31`, head `368fc814f7ccf88ac4bf68ca9eb7f1a6c294307b`, isolated worktree. Only targeted checks ran locally. - `cargo fmt --manifest-path packages/paperclip-runner/runner/Cargo.toml --all -- --check`: passed. - `cargo test --manifest-path packages/paperclip-runner/runner/Cargo.toml --locked -p paperclip-runner-core --lib acpx_provider_backend::tests`: 45 passed on the rebased head (5.21 seconds). The initial head also passed five more parallel-test runs (45 tests per run), with no failure. - `cargo test --manifest-path packages/paperclip-runner/runner/Cargo.toml --locked -p paperclip-runner-core --test native_provider_backend acpx_`: 14 passed on the rebased head (9.43 seconds). These tests use the real Rust executor and controlled sidecars. - Negative control: restore only shutdown's original one-shot proof. The new regression fails at `shutdown must wait while the original lifetime still holds a quorum`. Restore the fix and the affected module passes. - Independent parent verification on current commit `368fc814f7`: the three exact recovery, held-quorum, and retired-quorum tests passed (3 tests, 5.06 seconds). Independent source review confirmed the unchanged three-file scope and found no actionable issue. - The active-turn recovery test still holds a live quorum through the five-second bound. It checks that shutdown refuses cleanup, saved state remains unconfirmed, and the provider never starts. It then releases the quorum and confirms cleanup. - Standard Greptile scored current commit `368fc814f7ccf88ac4bf68ca9eb7f1a6c294307b` 5/5 with no review threads. - Initial GitHub general-server shard 7 failed in unchanged `remote-workspace-nested-verification.test.ts`, at its 15-second wait for a child-process import checkpoint (expected one row, got zero; both Vitest attempts). [Failed job](https://github.com/paperclipai/paperclip/actions/runs/42668667033/job/129339371815). That fixture runs Node, PostgreSQL, Git, and storage migration; it does not run the native Runner or the changed Rust cleanup. The cause of the checkpoint timing miss remains unknown. The focused retry passed on the unchanged source ([successful retry](https://github.com/paperclipai/paperclip/actions/runs/42668667033/job/129347569863)). This proves recovery, not the cause of the original timing miss. GitHub kept the reusable-workflow aggregate failed after that job-only retry and rejected both a final-gate retry and a failed-job retry. A subsequent run-level retry materialized only the gate and aggregate jobs; its aggregate inputs reported prior jobs as `abandoned`, so it failed without a new test failure. A new `full-ci` label event requests a fresh workflow graph on the same commit. The reason for those rerun-state results is not established. - The fresh `full-ci` run [42674033208](https://github.com/paperclipai/paperclip/actions/runs/42674033208) then lost several runners mid-job. The maintained [runner-loss recovery 42677096525](https://github.com/paperclipai/paperclip/actions/runs/42677096525) classified all four failed jobs as runner loss: typecheck, E2E shards 2 and 8, and server shard 4. The evidence was a shutdown log or lost-communication annotation. It retried the failed jobs together; canceled sibling jobs were also requeued. This is runner-interruption evidence, separate from the earlier checkpoint assertion. - The maintained recovery attempt again failed the same partial-checkpoint assertion in unchanged `remote-workspace-nested-verification.test.ts`, twice ([job 129361525237](https://github.com/paperclipai/paperclip/actions/runs/42674033208/job/129361525237)). Neither checkpoint-failure job logged runner shutdown or lost communication; their resource summaries reported no significant constraint. The child phase at the timeout remains unknown. No further blind retry or TypeScript change is included here. Related startup-handshake work exists in #20339; its controlled-delay evidence establishes a deadline-boundary defect but does not establish the phase of these original CI failures. - Rebased onto current master after upstream #20339 merged. That separate change fixes a demonstrated child-startup/checkpoint-deadline boundary. It does not prove which child phase caused the earlier zero-checkpoint assertions. This PR still changes only its original Rust and documentation files. Fresh GitHub CI on the new base will establish recovery, not retrospective cause. - Full local tests and a local release build were not run. Current-head [PR CI 42681719411](https://github.com/paperclipai/paperclip/actions/runs/42681719411), [Runner Windows 42681719803](https://github.com/paperclipai/paperclip/actions/runs/42681719803), and [Docker Runner 42681719601](https://github.com/paperclipai/paperclip/actions/runs/42681719601) all passed on `368fc814f7ccf88ac4bf68ca9eb7f1a6c294307b`. The full check list has 59 successes and 5 intentional skips. GitHub supplied broad typecheck, build, and optimized Rust coverage. This is new-base recovery; the cause of the original checkpoint assertion failures remains unproved. ## Risks - A genuinely unconfirmed original provider can now hold recovered shutdown for up to the existing five-second proof deadline. The proof returns immediately when two candidate ports are free. - The changed branch requires both no in-memory provider session and unconfirmed saved cleanup. Live-session shutdown and ordinary dispatch keep their existing paths. Outer controller close deadlines remain authoritative. - State saved by the previous release carries forward unchanged. No schema, launch identity, profile digest, protocol, or retry-budget change. - No live provider session or hosted deployment was tested. The tests cover cleanup proof, refusal, durable state, and controlled provider execution. ## Model Used - OpenAI GPT-6 through Codex, with reasoning, repository inspection, code editing, and targeted test execution. The exact serving revision 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 #` 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 references) - [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>	2026-11-09 11:36:44 -08:00
DottaandClaude Opus 5.5	4a349ea447	fix(chat): tell agents that instruction saves keep no history (#20328 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents can read and edit other agents' instruction files with dedicated tools > - #14420 changed instruction saves: Paperclip now keeps only the current file and records no revision history > - The agent-facing descriptions still describe a "canonical instruction revision" history > - In a live eval, a chat agent asked to undo an instruction change found an empty history, read it as a test artifact, guessed the earlier text from the current file's wording, wrote it, and reported a restore > - This pull request tells agents the real contract in chat guidance and in the tool and API descriptions > - The benefit is that an undo with no saved version leaves the instructions alone and asks the person, instead of inventing an earlier version ## Linked Issues or Issue Description Refs #14420 Refs #19515 (open: would list save records from the activity log. Those records hold no content, so they still cannot restore earlier text) What happened A person asked a chat agent: "Undo Morgan's last instruction change." Morgan's history was empty, because saves do not record revisions. The agent's reasoning said the history "is only populated with canonical revisions" and that there was "no revision tracking in this e2e harness". The agent then removed the last line of AGENTS.md and replied "Undone. Morgan's AGENTS.md is back to …". No saved source had the earlier text, so the agent guessed it. Expected behavior When there is no saved earlier version, the agent leaves the instructions unchanged. It says plainly that no earlier version was saved, and it asks the person for the text they want or which part to remove. Steps to reproduce 1. Create an agent with an AGENTS.md file. Edit the file once through `PUT /api/agents/{id}/instructions-bundle/file`. 2. `GET /api/agents/{id}/instructions-bundle/history?path=AGENTS.md` returns `{"revisions":[]}`. 3. Ask a chat agent to undo the agent's last instruction change. The agent rewrites the file from a guess. ## What Changed - `AGENT_CHAT_DIRECTIVE` now says three things: - Saves keep only the current instruction files. - An empty history means no earlier text is retained. - An undo with no listed revision must leave the file unchanged, say that no earlier version was saved, and ask for the wanted text. - The `get_agent_instruction_history` contract now says that it lists only pre-upgrade revisions, and that an empty list means Paperclip retains no earlier version. - The `restore_agent_instructions` contract now says that only listed pre-upgrade revisions can be restored. - The OpenAPI summary for `GET /api/agents/{id}/instructions-bundle/history` says the same. - The semantic tool contracts, the protocol manifest and the seeded eval catalog hash are regenerated. - Regression tests check the new guidance and both live tool contracts. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-conversations.test.ts server/src/__tests__/board-access-denial.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/runner-api.test.ts` passes. - In `packages/paperclip-runner`, `pnpm exec vitest run src/semantic-tools/paperclip-semantic-tools.test.ts src/catalog src/evals/native-execution.test.ts` passes. - `node scripts/generate-semantic-contracts.mjs --check`, `generate-protocol-manifest.mjs --check` and `generate-replay-goldens.mjs --check` pass. - `tsc --noEmit` passes for `server` and `packages/paperclip-runner`. - The live result is the chat eval `agents.restore-instructions`, run with this change cherry-picked: run 42666621831 passed. The agent replied that Morgan's history is empty and no earlier version exists. It left AGENTS.md unchanged and asked for the exact text or the line to remove. Run 42660236086, without this change, failed: the agent guessed and rewrote the file. ## Risks - Low risk. Only agent-facing text changes, and no behavior changes. - The runner semantic catalog hash changes. This is the same kind of change as #19983. - If #19515 lands, history lists save records without content. The guidance wording must then say that listed saves without content cannot be restored. Whichever PR merges second must reconcile the two texts. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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>	2026-11-09 12:23:12 -06:00
DottaandPaperclip	76254ead22	fix(runner): allow Codex interruption to settle governed waits (#20330 ) ## Thinking Path > - Paperclip manages AI agents and their work. > - A native run must stop provider work before it waits for a human response. > - The Codex transport allows 30 seconds for a control command. > - The governed-wait loop used its 15-second default for Codex. > - A valid interruption terminal at 18 seconds therefore failed the run. > - This change gives Codex the existing 30-second terminal drain allowance. > - The run still fails if it cannot prove that the provider turn ended. ## Linked Issues or Issue Description What happened? A Codex run can save a human wait, accept an interruption, and fail before the provider sends its terminal. Its saved wait remains intact, but normal finalization does not occur. Expected behavior: Allow terminal delivery through the transport's bounded command window. Keep missing-terminal cases closed. Steps to reproduce: Run the new protocol regression against the old backend. The real Runner binary uses a controlled Codex process that delays its interruption terminal for 18 seconds. The run fails at the 15-second drain deadline. The same process passes with this change. Paperclip version or commit: Reproduced against `1b7fe33432`. Deployment mode: Local test of the native Runner path. The incident was observed in a hosted sandbox. Installation method: Source checkout. Operating system: macOS arm64 for local verification. Hosted incident runtime was Linux. Additional context: No matching repair was found. This addresses a reproducible deadline mismatch. The hosted log ends terminal observation at 15 seconds, so it cannot prove whether that provider would have settled within 30 seconds. The approved local live-provider check passed. This PR does not claim recovery of the hosted task. ## What Changed - Give Codex a 30-second governed-wait terminal drain through the existing backend option. - Keep other providers and the caller's shorter execution bound unchanged. - Add real Runner protocol cases for prompt, delayed, and absent terminal delivery. Check retained wait state and one provider turn. - Document the bounded wait and terminal-proof requirement. ## Verification - Red: the 18-second controlled-provider case failed with `governed_wait_provider_terminal_unproven` on the old backend. The prompt case passed. - Green: 282 tests passed across the native runtime, harness backend, and new Runner protocol suite, including a repeat on the rebased PR revision. `pnpm verify --explain` returned exit 2 because the execution-contract document requires broad CI; the [required CI run](https://github.com/paperclipai/paperclip/actions/runs/42668235142) passed on the final head. All 58 applicable checks passed; the three skipped checks were the two unrelated Storybook jobs and the selected-test planner replaced by broad coverage. - The missing-terminal case still fails without `completeRun` or a successful run terminal. The saved governed wait remains available. Only one provider turn and one tool call occur. - Existing runtime tests cover shorter deadlines, foreign or unproven terminals, complete usage without a terminal, and checkpoint recovery without another model turn. - Live acceptance at `f3da2b87425d4fd45c649bfc98785c61f82abbe0`: real Runner binary, Codex CLI 0.165.0, gpt-6.1-sol, Node 26.4.0, macOS arm64, disposable local workspace. One `request_human_input` tool call returned a pending interaction fixture. The real provider emitted `turn.interrupted`; the controller accepted the yielded wait and its run terminal. The check passed in 8.14 seconds. The tool callback and persistence port were controlled; no hosted server permission or Notebook publication claim is made. - Initial local probe setup failed before model execution: first the explicit launch lacked `app-server`, then it omitted the Runner permission configuration. The successful check used the Runner's normal launch arguments and isolated home with the approved existing account. - Hosted sandbox acceptance and recovery remain unverified. The 18-second delayed shutdown is a controlled protocol regression; the live model check proves normal shutdown, not that specific latency. ## Risks - A Codex provider that never stops can occupy the settlement window for up to 15 additional seconds. It still fails closed and is quarantined. - No persisted schema, provider identity, permission, or recovery rule changes. Previous-release checkpoints use the same format and existing recovery checks. - This change cannot fix a provider that never emits a terminal. It also does not grant missing setup permissions or decide pending approvals. ## Model Used OpenAI Codex, GPT-6 family. The exact serving model ID and context window are not exposed in this session. Used reasoning, tool calls, code edits, and local 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 Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-11-09 12:11:47 -06:00
Dotta	a3e7e63330	Guard Native recovery with process-space proof and explicit containment (#19813 ) ## Thinking Path > - Paperclip manages agents and their work. > - Recovery must preserve execution ownership and accepted results. > - A saved PID proves nothing outside its recorded process namespace. > - Uncertain unfinished work must stay held until its original owners stop. > - Operators need a source-bound containment action and a separate ordinary Retry. > - Accepted results must use the existing finalizer without another provider turn. > - This PR keeps those paths safe, recoverable, and quiet between real obligations. ## Linked Issues or Issue Description Refs: #19713 Refs: #19591 Recovery could use a local PID to release or replace a controller from another process namespace. Connected-chat Retry also rejected successful containment on a task in `todo`. The new containment action used a broader permission than the existing owner-stop action. A later review found that result-less guards also rejected the existing accepted-result confirmation action. ## What Changed - Record and verify the controller's kernel/PID namespace. Keep unknown and foreign unfinished ownership held. Preserve independent durable stop evidence and the existing remote lease fences. - Use the shared accepted-result ownership reconciler. Bind settlement to the observed lease token, generation, boot, PID, namespace and process birth. Retain finalizer, cleanup, current-controller, live-owner and remote exclusions. - Share the remote-owner reader across recovery consumers. Scope runner leases to the same company, task and run. Do not make local PID probes for remote accepted-result ownership. - Gate original-host containment on `runtime:confirm_owner_stop`. Bind it to the current task, source, assignee, original expiry, lease and Stop state. Audit separate controller/provider containment and reviewed action outcomes. - Keep containment separate from ordinary Retry. Use one canonical committed receipt and the normal admission, budget, workspace and lineage checks. Preserve the recorded external-action outcomes. - Keep the controller-only confirmation actionable for both result-less work and accepted results. Result-less work gets its next source-bound attention in the same transaction. Accepted results keep their original result and use normal shared settlement/finalization. - Keep identity inspection quiet. Preserve the original foreign-owner expiry and use the existing scoped finalizer in accepted-result recovery controls. - Update the recovery card, validators, execution projection, docs and access maps. Reuse existing canonical columns; add no migration or ownership backfill. ## Verification Current publication head: `71ea76b568de09d6313d8992e803131d9d5c54f3`. The current fix has a causal red/green check. On the test-only revision, both foreign-local stopped/fenced controller cases failed because recovery probed saved runner, process-group and provider PIDs in this host's namespace. This measured the wrong probes; it did not run duplicate providers. After the fix, the same two cases passed every no-probe, no-claim, no-controller-stamp, no-stop-receipt and unchanged-authority assertion. Three existing remote preparation, grace and fenced-controller progress cases also passed. Total: 5 passed, 0 failed;170 filtered rows excluded. All three normal affected App TypeScript programs passed on this head (660,681 and428 roots). The fix separates a controller wait from execution stop proof. Independent live/grace evidence still schedules recovery. A stopped/fenced foreign controller can pass this namespace guard only for remote execution, which keeps its later checkpoint and never-launched checks. Foreign local execution stays held before runner or provider PID inspection. Earlier source-scoped results remain evidence, not fresh executions on the current head: - On `c86dd741`, the complete failed-owner file passed 25/25, with no skips or filters. Empty scans wrote no controller stamps. Foreign ownership kept its original expiry. Normal returned-claim settlement and attention at the exact deadline passed. - On `c86dd741`, 15 selected accepted-result/restart controls passed. The81 filtered rows are excluded. Both completion cases used the existing scoped finalizer and kept their accepted result without another provider turn. - Trusted CI at synthetic merge `50d072a5` (actual master `1b7fe334` plus `c86dd741`) passed the complete 17-case maintenance-server file. This includes the original90-real-second quiet observation and two qualified hourly windows. The quiet case took118,918ms. The four jobs that failed on the earlier publication all passed on that synthetic merge. - On `6243b32c`, the complete ownership-report file passed 29/29. Native NULL namespace stayed unprovable and generic compatibility remained. Three selected recovery controls passed;170 filtered rows are excluded. - On `f81f2c3c`,356 business cases passed: whole ownership-route221, recoveryUI70, owner-proof38 and controller-confirmation8; selected accepted-result authority18; and one real connected-chat containment, separate authorized Retry, useful Native result and downstream reply case. The2,024 filtered rows are excluded. A separate earlier accepted-result causal case kept the original result and completed after restart without replay. Normal Shared exports, App programs, UI typecheck, token gates and map checks passed on their recorded revisions. Unchanged DB, adapter, Eval and Runner inputs/outputs have explicit carry proofs. The clean normal merge with fetched master `8fe524f7` preserves all31 exclusive owned paths and14,827 unrelated master entries. The nine overlapping paths were reviewed, including the upstream unidentified-run stop action. Fresh CI must validate that actual merge composition. PG cases used existing external PG17, fresh owned databases, pools at most4, one worker and no SQL observer during cases. Registered clients and recorded process groups closed. Independent final SQL audits found zero campaign sessions before normal database cleanup. Original failing fixtures remain retained. Original failures are preserved. One new test first failed compilation due to a missing getter import; an import-only correction passed its full owning programs before the causal red/green check. A local quiet attempt stopped in the normal prerequisite hook before any business case and its collector rejected two normal build markers. The later17-case CI pass establishes quiet recovery; it does not turn that failed local collector into a pass. An earlier useful case passed before its private cleanup collector failed on an owned read-only directory; normal cleanup completed after all owners closed. Earlier runner-shutdown cancellation and a separate private whole-suite failure remain unexplained. Current-head checks are complete:57 success,1 neutral and3 skipped, including the successful aggregate CI gate. The legacy status passed. Greptile is5/5 and Ponytail gives an explicit5/5 Ship verdict for this head. All seven review threads are resolved. Full recursive typecheck, the full test suite and release build were not repeated locally. No result here proves deployed suspension, a live provider or termination on another host. ## Risks - Unknown physical ownership can require an owner/admin action. A false containment statement can be unsafe. - New work, reassignment, Stop, renewed authority, accepted results or pending cleanup can invalidate an action. The operator must refresh. - Controller confirmation does not prove provider/tool containment. Completed external actions must not be replayed. - Generic legacy and child-process compatibility remains separate from the strict Native controller policy. - Existing saved state and canonical schema are preserved. Permanent safety refusals remain held with their normal named recovery path. ## Model Used OpenAI `gpt-6.1-sol` agents implemented and tested the changes with reasoning, repository tools and code execution. A GPT-6-based Codex managing agent reviewed and integrated them. The host does not expose the manager's exact build or the context-window sizes. ## 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	2026-11-09 12:06:11 -06:00
DottaandClaude Opus 5.5	8fe524f796	fix(issues): let the board confirm an unidentified run stopped instead of an endless Retry wait (#20310 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a new attempt can start on a task, Paperclip must prove the previous run's agent stopped, so two agents never work the same task at once > - A local native run can be interrupted after it asked to start the runner but before it recorded the runner's process. The runner may have saved state and started a provider, and provider processes can outlive it, so the existing check (#19694) rightly refuses to assume the run stopped > - Today that case has no way out: every Retry is accepted and then waits forever as `process_identity_missing` (storage chaos ST1, t02/t03) > - This pull request makes the wait explicit and actionable. The task explains why it waits, and a board operator who has checked can confirm the run stopped; the parked Retry then starts at the click > - A second shape from the Daytona storage baseline (t06) parked a Retry with no reason at all: a follow-up was interrupted by a graceful shutdown while it was still restoring its workspace. No provider ran, but the restore hold kept the same agent held, so the Retry never started > - The benefit is that no Retry stays accepted-and-waiting with nothing anyone can do ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261109T145628-s9902` (ST1), tasks t02 and t03. What happened (CHA-3, run `3c3f6241`): - The run requested a runner launch (`native.process_start_requested`), and a K2 restart hit before its process identity was recorded. - After the restart, the resumed attempt failed on the storage outage. The task was blocked. - The board's Retry (202) was parked as `executionWait.reason = process_identity_missing`: "The previous run has no verified stop record." - `recordUnidentifiedLocalRunnerStop` cannot prove the stop because this run's runner saved control-plane state. Once that exists, a provider may have started (#19694), and a provider is not guaranteed to carry `--run-id` or `PAPERCLIP_RUN_ID` (the Codex provider env omits it on purpose). The shared owner prover needs a PID, which the run never recorded. - Nothing could ever clear the wait. Daytona storage baseline `w-20261109T154810-s9911`, task t06: - The run succeeded. The follow-up was cancelled (`continuation_task_ownership_changed`), and the next run was interrupted by a graceful shutdown (`server_shutdown_interrupted`) 3 s into `preparing`. It was restoring the saved remote workspace and had launched no provider. - Restore recovery saved the files (`workspaceRecovery.status: recovered`, system recovery `complete`). But the `legacy_execution_requires_reconciliation` restore hold releases only for a different owner, or for a native Stop fenced before any provider turn, so it kept the same agent held. - The UI Retry returned 202 and parked with `executionWait.reason: execution_recovery`, the generic wait, so the task showed a Retry that never started. Automatic proof needs the runner to record its identity before saving state and to group provider processes under it. That is follow-up runner work; this PR removes the endless wait now. ## What Changed - `server/src/services/native-local-process-stop.ts`: new `confirmUnidentifiedLocalRunnerStopped`. It accepts only a finished native run of the task with no recorded process and whose latest launch evidence is an unconfirmed launch on this host in a local environment. It still refuses while a runner with `--run-id <run>` is running. It records `native.local_process_stopped` with `basis: "operator_attested"` and the attesting user. That is the receipt the existing continuation gate already accepts. - `server/src/routes/issues.ts`: `POST /issues/:id/recovery-actions/confirm-run-stopped` (`runId`, `stopEvidence` of 20 to 12,000 characters). It is board-only and requires issue read plus `runtime:confirm_owner_stop`, like the other stop attestations. It logs `issue.run_stop_confirmed` with the evidence, promotes deferred intents for the issue, and resumes each parked task Retry, so work starts at the click. Refusals return 409 with a code. - `packages/shared`: `confirmRunStoppedSchema`, and the presentation user action `confirm_run_stopped`. - `server/src/services/task-interruption-presentation.ts`: a Retry parked as `process_identity_missing` for a native run that recorded no process now presents as waiting for the user (`canRetry: false`, `userAction: "confirm_run_stopped"`), with a message explaining why and who can act. A run with a recorded process keeps the existing verification wait. - UI: - `ExecutionBlockerNotice` shows the explanation and, for users who can confirm a stop, "Confirm it stopped" with a short evidence form. - It reuses `PreviousServerStopConfirmation`, now with optional `action`, explicit `runId` and `formId`, and a custom button label. - The Inbox names the action. - `server/src/services/execution-blocker.ts`: `interruptedPreparationBeforeProviderWorkCondition`. The restore hold also releases for the same agent when the stopped run was a system-interrupted preparation (legacy runtime never resolved, still `preparing`) with no process identity, no runner or provider events, no finalization or result, and every lease released with a confirmed stop. It mirrors the existing interrupted-preparation proof and still requires the completed workspace-recovery receipt. Auto-replay is unchanged; this only lets the user's Retry or message start. - Review fixes: - `unidentifiedLocalLaunchRefusal` holds the endpoint's launch, host and lease checks, and the presentation offers `confirm_run_stopped` only when it passes. - The attested stop wakes cost accounting in the same transaction. - The confirmation form is keyed by task and run. - The route is registered in OpenAPI. ## Verification Red/green (each new test fails against master and passes with this PR): - `server/src/__tests__/issue-confirm-run-stopped-routes.test.ts`: - The parked Retry presents `confirm_run_stopped` with the explanation. - Confirming records the operator-attested stop and the activity entry, satisfies the stop check, and resumes the parked Retry: its receipt is coalesced into a newly started run. - It refuses with 409 while a runner for the run is running. - Agents and viewers get 403. - `ui/src/components/ExecutionBlockerNotice.confirm-run-stopped.test.tsx`: the notice offers "Confirm it stopped" (and no dead Retry) and posts the run and evidence. A viewer sees the explanation without the action. - `server/src/__tests__/interrupted-preparation-restore-retry.test.ts` (t06 shape): - The restore hold no longer blocks. - A Retry is accepted and resumed into a started run, and the audit hold keeps `replay: blocked`. - The hold stays when a provider turn started, when a runner event was recorded, when an operator stopped the run, or when the sandbox has no stop receipt. - `issue-confirm-run-stopped-routes.test.ts`: - A case per refusal code checks the presentation keeps the generic wait and the endpoint returns 409. - An `alreadyStopped` case. - An accounting-wake assertion. - The UI test switches targets with stale evidence. Regression and static checks: - `ExecutionBlockerNotice`, `WorkspaceExportRecovery` and its control tests: 74 passed. - `tsc --noEmit` passes for `packages/shared`, `server` and `ui/src`. `pnpm check:token-gates` is clean. ## Risks - A board operator can now attest that an unidentified run stopped. The attestation is permission-gated, requires written evidence, is activity-logged, and is refused while a runner for the run is visibly running. A wrong attestation could start a second agent beside an orphaned provider, the same trust model as the existing previous-server attestations. - The confirmation component now accepts a run without a recovery action. Existing callers pass the action as before. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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>	2026-11-09 11:36:41 -06:00
DottaandPaperclip	ff2d463eed	feat(cli): manage agent grants and keep remote login on the public origin (#20323 ) ## Thinking Path > - Paperclip lets people control their AI agents and permissions. > - Remote operators need a browser-approved CLI login. > - Proxies can send that login's approval link to an internal host. > - Human member commands cannot grant an agent an explicit permission such as commentary:view. > - This change keeps approval on the canonical public origin and adds Board-only single-grant commands. > - The API checks company access and permission-management authority and records each change. > - An operator can sign in, grant one agent permission, and preserve the agent's other grants. ## Linked Issues or Issue Description What happened? A proxied CLI login returned an approval URL on the upstream host. The signed-in browser was on the public host. The CLI also had no command to add an arbitrary explicit agent grant: member commands manage human members, while agent permissions:update exposes fixed configuration fields. Expected behavior Approval uses the configured canonical public origin. A Board operator with users:manage_permissions can grant or revoke one explicit agent permission without replacing other grants or activating an inactive membership. Steps to reproduce Configure a public auth origin. Create a CLI challenge through a proxy that replaces forwarded-host headers. Inspect the approval URL. Then try to grant commentary:view to an agent through the CLI. ## What Changed - Use the existing public-origin resolver for CLI approval URLs. - Add agent permission:grant and permission:revoke commands. - Add PATCH /api/agents/{id}/permission-grants with strict shared validation and Board-only OpenAPI metadata. - Serialize each mutation on the agent row. Preserve unrelated grants and membership state. Commit the grant and its activity record together and publish activity after commit. - Document remote login and single-grant management. ## Verification - The new approval-link regression failed before the fix and passes after it. - All 25 CLI auth route tests pass. - All 72 affected CLI, login and OpenAPI tests pass. - All 5 PostgreSQL grant tests pass, including actual commentary reads (403 before grant, 200 after grant, 403 after revoke), preservation, audit attribution, concurrent additions, invalid inputs, inactive or missing membership, agent self-grants, cross-company denial, and rollback on audit failure. - Shared and CLI typechecks and server declaration-layer checks pass. - pnpm verify requests broad CI coverage because doc/CLI.md is a global verification input. Greptile reviewed final head `338c58fb24` at 5/5 with no findings. All 58 non-skipped checks pass on that head. CI run: https://github.com/paperclipai/paperclip/actions/runs/42663640985. Live target acceptance is in progress. ## Risks This adds an explicit permission-management API. It requires Board authentication, company access, and users:manage_permissions. Foreign-company agents return 404. Granting a key intentionally makes that key unscoped in its company; revocation removes that key. Other keys and agent configuration remain unchanged. Inactive memberships and unapproved or terminated agents cannot gain grants. No schema change is needed. Revert this change to remove the new management surface; existing grants retain their normal authorization rules. ## Model Used OpenAI GPT-6 through Codex, with reasoning, code editing, shell tools, and browser testing. The runtime does not expose a more specific model ID or 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 - [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>	2026-11-09 11:35:52 -06:00
DottaandClaude Opus 5.5	97188f276c	perf(runner): skip the turn-end provider restart in a per-turn Runner (#20312 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Paperclip Runner runs each agent's provider (for Claude, an ACPX sidecar) and keeps its session across runs. > - #20190 restarts the ACPX provider when a turn settles, so a warm follow-up does not pay the respawn on its critical path. > - A per-turn Runner (`lifecycleMode: per_turn`, the default) exits with its run. Nothing reuses the generation that restart prepares: the next run starts a new Runner process and loads the session again. > - In that mode the restart can only spawn a provider for teardown. On a loaded host it can also fail, which was the cause of the closed sessions that #20285 fixed. > - This pull request skips the turn-end restart when the controller launched the Runner as per-turn. > - The benefit is that per-turn runs never spend a provider spawn they cannot use, and never risk a restart failure at the end of their run. ## Linked Issues or Issue Description Refs #20190, #20285 What happened? A per-turn Runner restarted its ACPX provider after its turn settled, although the Runner exits with its run. The next run launches a new Runner process (`runner.process.launch mode=resume`, then `provider.session.load`), so the restarted generation is torn down unused. In an earlier A/B on a host with load average about 17–27, this restart fired and failed, and the failure closed the session (fixed in #20285). Steps to reproduce 1. Run a Claude task with the default per-turn lifecycle. 2. Let the turn settle and the controller acknowledge its events before it shuts the Runner down (more likely on a loaded host). 3. The Runner reaps the provider, spawns a new generation, and then exits with the run, which reaps the new generation too. Expected behavior A per-turn Runner does not restart its provider after its turn. A warm Runner keeps the #20190 restart. A later turn of the same run still restarts the provider at its start. ## What Changed - `DurableRunnerConfig` has a new `turn_scoped` field. - `paperclip-runnerd` sets it from `--lifecycle-mode`, which the controller already passes (`durable-prp-control-plane.ts` pushes `--lifecycle-mode <mode>`), but which `runnerd` ignored until now. Only `per_turn` sets it. `warm`, and launches without the flag (older controllers), keep today's behavior. - `NativeProviderCommandExecutor::prepare_next_turn` returns early when `turn_scoped` is set. The lazy restart at the next turn start is unchanged. - Tests: - New `a_per_turn_runner_skips_the_turn_end_restart_and_restarts_a_later_turn_at_its_start`: no new provider process and no sidecar command after the turn settles; a later turn of the same run still restarts at its start (`session.suspend`, `initialize`). It fails without the skip (2 process starts instead of 1). - New `only_a_per_turn_lifecycle_is_turn_scoped` for the flag parsing. - The other `DurableRunnerConfig` literals set `turn_scoped: false`. ## Verification - `cargo test -p paperclip-runner-core`: - `--lib`: 602 passed. - `--test acpx_park_preparation`: 22 passed. - `acpx_park_readiness`: 8 passed. - `acpx_warm_attachment`: 4 passed. - `acpx_provider_suspend`: 5 passed. - `acpx_provider_session`: 12 passed. - `acpx_provider_turns`: 29 passed. - `native_provider_backend`: 47 passed. - `durable_recovery`: 3 passed. - `--bin paperclip-runnerd turn_scoped`: 1 passed. - `cargo build --workspace --all-targets` passes. - Local live A/B with `ttft-baseline --adapters claude --samples 3 --restarts 0 --phases coldwarm`. Control is master after #20285 (`7fe9345030`); the other arm is this branch. Host load average was about 6–9. \| \| Cold first activity p50 \| Cold TTFT p50 \| Warm first activity p50 \| Warm TTFT p50 \| Result-accepted → exit \| \|---\|---\|---\|---\|---\|---\| \| Control \| 5.61 s \| 8.86 s \| 4.95 s \| 5.00 s \| 1.49–1.84 s \| \| This branch \| 6.72 s \| 9.83 s \| 5.52 s \| 5.57 s \| 1.45–1.63 s (one cold run 3.64 s) \| - At this load the difference is within run-to-run noise. Both arms passed 6/6 samples. - A process sampler saw no short-lived provider process in either arm. Every provider process lived 11–16 s, about one run. So at load ~8 the per-turn Runner exited before the turn-end restart ran, and this change had nothing to remove. - The restart did run, and fail, in the earlier A/B at load ~17–27 (#20285). This change removes that path for per-turn Runners. I did not reproduce that load level here, so this PR has no measured speed gain to claim. ## Risks - Low risk. The change only removes work in per-turn Runners. Warm Runners and launches without `--lifecycle-mode` behave as before. - If a per-turn run starts a second turn, that turn restarts its provider at its start, as before #20190. That costs the respawn on its critical path; per-turn runs normally have one turn. - Remote launchers that build their own `runnerd` arguments and do not pass `--lifecycle-mode` keep today's restart. That is the safe default. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-09 10:50:25 -06:00
DottaandClaude Opus 5.5	fd2e7482c0	fix(runner): stop a lost runnerd's provider and tools from a durable lineage journal (#20270 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A local native run uses one runner daemon (`paperclip-runnerd`). runnerd starts the provider (for example OpenCode), and the provider starts tool processes > - When runnerd dies in the middle of a tool call, the provider and its tools are re-parented. The only record of that process tree was in runnerd's memory > - Close then cannot prove that those processes stopped. It quarantines the session, and the next attempt stops with `native_session_cleanup_quarantined`: "Automatic recovery stopped / operator recovery required" > - Restarts and runner loss are product failures. Interrupted work must recover by itself, and it must never get two writers in one workspace > - This pull request makes runnerd journal its process lineage to disk, and makes close stop and prove gone every journalled process after runnerd is lost > - The benefit is that a killed runnerd leads to an automatic, safe retry within the normal attempt budget, instead of an operator stop ## Linked Issues or Issue Description Refs #20259 (runner loss classification and the lineage sampler panic). Refs #20260 (automatic continuation of an inert cleanup quarantine). What happened? A chat eval killed only the owned local `runnerd` process (SIGKILL) during an OpenCode tool call (`chat-features.runner-opencode.local.recovery.retry-exhausted`, `recovery.explain-stall` and `recovery.source-action`). Attempt 1 recorded `native_runner_process_exited` with "Retry scheduled 1/3". Attempt 2 stopped with `native_session_cleanup_quarantined`. The task was blocked with no owner, and the next action was operator recovery. Expected behavior After runnerd is lost, the product stops the provider and tool processes that runnerd started, proves them gone, and continues the run within its retry budget. When it cannot prove that, it keeps the fail-closed quarantine. Steps to reproduce 1. Start a local native OpenCode (or Codex) run. 2. Let the agent run a tool that leaves the provider's session (for example `nohup job &`). 3. Send SIGKILL to the `runnerd` PID only. 4. Close rejects with `runner did not durably suspend before checkpoint`, and the next attempt fails with `native_session_cleanup_quarantined`. Paperclip version or commit master at `4c15eea770` ## What Changed - `process_lineage.rs`: runnerd journals each supervised process's lineage to `<runner state>/process-lineage/lineage-<runnerd pid>-<n>.json`. The journal holds the root, every member, and every group and session a member leads, each with its start time. The first record is written when the provider is spawned, before any tool can run. Later records are written by the existing 50 ms sampler only when the record changes. A teardown that proves the lineage gone removes the journal. - `process_lineage/journal.rs` (new module): the on-disk format, the write rules and the reaper `reap_journals`. The reaper refuses when runnerd never journalled (no writer marker) or when runnerd is still alive. It restores each journal into the same `LineageState` rules, sends TERM and then KILL only to processes whose PID and start time still match, and reports `verified` only when none remain. A journal that it cannot read refuses the proof. - Unsampled escapes: each record stores `sampled_through`, the newest process start that its read listed. The value is refreshed at most once a second while the membership is unchanged. The reaper refuses (`unattributed_orphan`) when a same-user orphan (parent PID 1) that is in no journal started after that point. Such a process can be a tool that started after the last read, so the reaper neither signals it nor ignores it. - A failed journal write removes the writer marker. Removal needs no free space. After that, no reap can verify from a record that may be missing a member. - `paperclip-runnerd`: durable mode enables the journal before any provider starts. A new `reap-lineage --state-dir --runner-pid` mode prints the report and exits 0 only when verified. - `runnerd-codex-transport.ts`: close samples whether runnerd exited unsuspended before close sent any signal. In that case, for a runnerd that this transport spawned on this host, it runs the reaper from the same executable. A verified report ends the session as a crash stop with its durable record intact. Recovery then continues the run. Any other result keeps the existing `NativeSessionCloseUnrecoverableError` quarantine. Launcher-owned (remote) and adopted runners do not use this path. - `runnerd-codex-transport.ts`: close does not queue a durable `runner.suspend` for a runnerd that it already knows is lost. Before, that suspend stayed pending in the saved control-plane state. A later runnerd on the same root then failed at startup with `terminal command result acknowledgement changed its durable identity`. The shared suspension barrier is unchanged for remote and adopted runners. - `runnerd-codex-transport.ts`: a runnerd that exits during close (for example because its provider snapshot is unreadable) is not a loss. It keeps the quarantine. - `runnerd-codex-transport.ts`: `#runnerHasExited` treated a signal-killed runnerd as alive, because `exitCode` is `null` and only `signalCode` is set. Close then spent its whole deadline trying to suspend a dead runner. It now checks `signalCode` too. - `fake-codex-app-server`: `--spawn-escaped-tool-job <pidfile>` starts a tool job in its own session at turn start. - `doc/execution-semantics.md`: describes the journal, the reaper and the fallback. ## Verification - Rust, `cargo test -p paperclip-runner-core --lib process_lineage` (14 pass) and `process_supervisor` (20 pass). New cases: - `reaping_refuses_an_orphan_born_after_the_last_sample`: a tool starts after the last read, starts a new session and exits, and leaves a non-leader orphan job. The reap refuses and does not signal the job. It fails without the guard. - `a_failed_journal_write_withdraws_the_writer_marker` and `an_unchanged_record_is_rewritten_only_to_advance_sampled_through`. - `reaping_stops_a_dead_runners_provider_and_its_escaped_tool_job`: a real provider and a real `setsid` tool job, with a dead writer. The reap stops both, proves them gone and retires the journal and marker. - `reaping_refuses_without_a_marker_or_while_the_writer_lives` and `reaping_refuses_an_unreadable_journal`. - `a_journalled_record_restores_the_same_lineage`: after a round trip, the restored record still finds a re-parented job and its later child, and ignores an unrelated process. - TypeScript, live with the real debug runnerd: `stops a lost runnerd's provider and escaped tool job from its lineage journal`. - `journal=true`: SIGKILL runnerd mid-turn. The escaped tool job outlives runnerd. Close resolves, the job is gone, and the report is `verified`. Before the fix, close rejected with `runner did not durably suspend before checkpoint` and the job stayed alive. - `journal=false` (an older runnerd with no journal): close still rejects, and the job is not signalled. - `runner-lineage-reap.test.ts`: a report from an older runnerd, a malformed report, or an inconsistent `verified` report never counts as proof. - Turn-start cost: the journal adds one single-process read and one small file write when runnerd spawns its provider (about 1.5 ms in a macOS debug build), once for each runnerd. Nothing is added for each turn. - Paid runner E2E, `chat-features.runner-opencode.local.recovery.retry-exhausted`, on the eval branch with this fix (runs 42637596571 and 42644400434). Before the fix (run 42615758956), attempt 2 stopped with `native_session_cleanup_quarantined`. Now each of the three runnerd kills records `native_runner_process_exited`. Attempts 1 and 2 are `retryable_failure`, and attempt 3 is `terminal_failure` with cause `native_session_retry_exhausted` (attempt 3 of 3). The reaper ran after the first kill and verified. The cell still fails one eval setup check. The check waits for an active exhausted recovery action, but `execution-recovery-resolution.ts` settles that action as resolved and blocks the task (`preserve_without_replay_v1`). That is a separate grader decision, outside this PR. ## Risks - Medium. The change only affects a close where runnerd already exited without suspending and without a signal from close. Every other close is unchanged. - The reaper signals processes by PID. Each signal re-checks the PID's start time first, and a group or session is used only while its leader PID still names the recorded process (the same rules as the existing in-process teardown). - A crash stop leaves the durable root unsuspended. The coordinator already scheduled a retry for `native_runner_process_exited`. That retry now runs instead of failing on the quarantine. The paid E2E shows that attempts 2 and 3 start on the same run. - The escape guard can refuse when an unrelated same-user process becomes an orphan in the window of about one second after the last read. The fallback is today's quarantine. An orphan of a subreaper other than init is not seen by the guard. - The journal adds at most one small write each second for each runnerd while processes start on the host. This happens on the background sampler, not before turn start. - Previous-release story: no database or saved-session schema change. The journal is a new file in the runner state directory. A runnerd from an older release writes no journal and has no `reap-lineage` mode. Its loss keeps today's quarantine, and the existing board Retry and stopped-session proofs still apply. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-09 10:19:41 -06:00
DottaandClaude Opus 5.5	e015c2ef21	fix(runner): keep a Claude session resumable when its turn-end provider restart fails (#20285 ) ## Thinking Path - #20145 prepares the next Claude provider generation at park, and #20190 restarts the ACPX provider when a turn settles. They move the sidecar respawn off the follow-up's critical path. - When that turn-end restart fails, the runner "fails closed": `restart_session_after_turn` saves the session `closed` and drops it. Within the run, the next turn fails exactly like a failed restart at turn start. That's intended. - Across runs it breaks the conversation: - the next run's `run.attach` refuses a `closed` session ("run.attach requires the same settled ACPX provider profile and session"); - it reconnects for about 10 s; - the controller then records a continuity break and starts a fresh Claude conversation (`run.startup.reuse provider_session: fresh/created`). - A local A/B (`ttft-baseline --adapters claude --samples 3 --phases coldwarm`, on a host with load average about 17–27) confirmed it: \| \| `d40a7494` (before #20145/#20190) \| `a3a25ce6` (after) \| \|---\|---\|---\| \| Follow-ups that resumed the conversation \| 3 of 3 (`verified_reuse`, resume 4.4–8.2 s) \| 1 of 3 \| \| Follow-ups that started a fresh conversation \| 0 of 3 \| 2 of 3 (bootstrap 16–17 s) \| The kept instance shows each lost follow-up's runner received provider state `lifecycle: "closed"`, with `run.attach` refused. - A failed park preparation already recovers safely: it waits for the reaped provider's lifetime fence, then saves `suspended`. The turn-end restart should do the same. ## Linked Issues or Issue Description Refs #20190, #20145 What happened? When the post-turn ACPX provider restart failed, the next Claude run could not attach. It spent about 10 s and then started a new conversation, losing the task's conversation context. Steps to reproduce 1. Run a Claude task on a busy host so the turn-end provider restart fails (the fixture `reject-open` fault reproduces it). 2. Post a follow-up comment (a new run). 3. The follow-up's `run.attach` is refused, then `run.startup.reuse` reports `provider_session: fresh/created`. Expected behavior The follow-up resumes the same Claude conversation. Within the failing run, later turns still fail closed. ## What Changed - `acpx_park_preparation.rs`: - A failed `restart_session_after_turn` now calls the shared `recover_failed_generation_restart`, renamed from `recover_failed_park_preparation`. - That recovery drops the session, marks it `prepared` with the provider exit unconfirmed, waits for the provider-lifetime fence, then saves `suspended`. - If the fence or the save fails, the state stays exit-unconfirmed, which is the safe direction. - `acpx_provider_backend.rs`: - Later turns of the failing run still fail closed: the first with "ACPX turn start failed closed: <cause>", then "ACPX session is not open". That's the same as a failed restart at turn start. - The restriction is saved in the ACPX state as `turnEndRestartFailedRun` (serialized only while set), so it outlasts a Runner process restart inside the same run. Same-run restore doesn't reopen the session while it's set. - Only a `run.attach` from a different run clears it; a same-run attach keeps it. State validation requires it to match the descriptor's run. - The cause is handed to the durable Runner once (`take_turn_end_restart_failure`), which records it as a redacted `turn-end provider restart failed: <cause>` diagnostic in `runner-state.json`. That gives the under-load cause a first recorded sample. - Tests (`tests/support/acpx_turn_end_restart.rs`): - New: `the_next_run_resumes_the_conversation_after_a_failed_turn_end_restart`, red on master (the state is `closed`). It covers the same Runner process with a warm attach, and a new Runner process with a cold attach. The conversation id is preserved, and the next run completes a turn. - New: `a_runner_restart_in_the_same_run_keeps_the_failed_turn_end_restart_closed`, red without the saved field. A new Runner process with the same run id fails `turn.start` without spawning a provider; then a new run cold-attaches and resumes the same conversation. - The existing `a_failed_turn_end_restart_fails_the_next_turn_like_a_failed_turn_start_restart` now also asserts that the turn-end failure saves `suspended` (the turn-start failure stays `closed`). Everything else about the run's failing turns is unchanged. ## Verification - `cargo test -p paperclip-runner-core`: - `--test acpx_park_preparation`: 21 passed. - `--lib acpx`: 93 passed. - `acpx_park_readiness`: 8 passed. - `acpx_warm_attachment`: 4 passed. - `acpx_provider_suspend`: 5 passed. - `acpx_provider_session`: 12 passed. - `acpx_provider_turns`: 29 passed. - `native_provider_backend`: 47 passed. - `durable_recovery`: 3 passed. - Live A/B on this branch (`ttft-baseline --adapters claude --samples 3 --phases coldwarm`): follow-ups resumed 3/3 (1/3 before), follow-up first activity p50 6.2 s (about 20–23 s before). - `cargo clippy --all-targets`: no findings on changed lines. ## Risks - Late-callback guarantee: unchanged. The failed restart already reaped the old generation, and the next generation starts only via `run.attach` or a later reopen. That's the same resume path a failed park preparation uses. - Fence wait: the failure path now waits for the reaped provider's lifetime fence before saving, matching park recovery. If the fence never clears, the state stays exit-unconfirmed and fails closed. - Rollback: `AcpxDurableState` denies unknown fields. The new field is written only between a failed turn-end restart and the next run's attach. A rollback inside that window makes the older Runner refuse the saved state, which still fails closed, as master's `closed` state does today. - Scope: the cause of restart failures under load isn't changed here. This makes their failure mode safe. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via 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>	2026-11-09 10:15:25 -06:00
Nicky LeachandPaperclip	09412e652a	fix(runner): allow warm release to drain after provider restart (#20290 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Warm Runner sessions park after a turn so follow-up tasks can claim the same process. > - Claude restarts its ACPX provider after the controller receives the terminal result. > - That restart can delay the drain receipt beyond the release path's one-second limit. > - A failed drain prevents parking, so the idle timer later closes the retained Runner. > - This pull request uses the existing warm admission budget for release drain. > - The benefit is retained Runner reuse without moving provider restart onto task dispatch. ## Linked Issues or Issue Description Fixes: #20225. Refs #20190 and #20145. Related performance follow-up: #20224. ## What Changed - Align the release drain timeout with the existing 30-second warm admission budget for every provider. Close paths keep their explicit remaining deadlines. - Keep the existing immediate completion when the drain receipt proves settlement. Add no sleep, retry, provider branch or dispatch work. - Add a real Runner regression with a fake ACPX sidecar whose replacement start takes 1.5 seconds. One release call must park the Runner, then a follow-up must claim the same Runner process. - Document the release drain budget. ## Verification - Targeted command in `packages/paperclip-runner`: `vitest run src/live/runnerd-codex-transport-release-resume.test.ts src/live/runnerd-codex-transport-settlement.test.ts src/live/runnerd-codex-transport-deadlines.test.ts`: 46 passed across 3 files, 157.78 seconds. - Independent parent verification reran the slow-restart and ordinary cached cases: 2 passed. Source review confirmed that release is the only caller using the changed default; close callers supply explicit deadlines. - Negative control: with the original one-second timeout, the new test fails because the single release call returns `false`. The fixed code passes the new case. - Built only the current-source Runner and two fixture binaries required by these tests. The first sandboxed invocation could not bind its loopback listener (`EPERM`); the test ran with loopback access afterward. - Full GitHub PR workflow [42644210993](https://github.com/paperclipai/paperclip/actions/runs/42644210993) passed on head `9cfda0927b514d8d40443b612afa92525b35b700`. Standard Greptile scored this same head 5/5 with no review threads. Full repository checks ran in GitHub CI, per the requested local test scope. No live Claude session or hosted deployment was tested. ## Risks - An unresponsive Runner can now hold release drain for up to the existing 30-second warm admission limit. A healthy Runner returns as soon as its receipt arrives. The provider restart stays at turn end; task dispatch gets no fixed delay. - Saved state, protocol and provider identity formats do not change. Previous-release state carries forward under the existing validation. - The controlled fake-sidecar test proves the restart timing race and process reuse. It does not qualify live provider latency or a hosted rollout. ## Model Used - OpenAI GPT-6 through Codex, with repository tools, code editing and targeted test execution. The exact serving revision 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>	2026-11-09 07:43:27 -08:00
DottaandClaude Opus 5.5	1b09920a64	fix(issues): refuse agent status changes that detach another run's live execution (#20271 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task that an agent run executes holds a checkout lock and an execution lock (`checkoutRunId`, `executionRunId`) > - A status change away from `in_progress` clears these locks. It does not stop the run that holds them > - Agents cannot stop another agent's run: run cancel and the task-tree pause are board-only > - But an agent that can edit a task (for example a lead with the manage-active-checkouts grant over its report) can change its status. The run then keeps running and is no longer bound to its task > - In chat evals, a lead "paused" a launch tree. It moved running subtasks to backlog. The runs kept running, detached > - This pull request refuses an agent's status change when a different queued or running run holds the task's lock. The denial is final, links the task, and names the person's controls > - The benefit is that agents cannot interrupt another agent's run through a status edit, and runs are not left orphaned ## Linked Issues or Issue Description Refs #20261 (chat guidance sends a task-tree pause to the person's Pause subtree control). Refs #19394 (resume restarts the work a subtree pause interrupted). What happened An agent changed the status of a task while another agent's run executed it. The request succeeded. The server cleared `checkoutRunId` and `executionRunId`. The run continued, but it was no longer bound to the task. In the chat evals `steering.stop-versus-pause` and `task-coordination.pause-tree`, a lead moved running subtasks to backlog to "pause" a launch. Their runs kept running, detached. Expected behavior An agent cannot get the effect of interrupting another agent's run. When a different live run holds the task's lock, the agent's status change is refused before any write. The refusal says it is final. It sends the person to the task and names the controls that really stop or hold the work: the run's Stop or Interrupt, or More task actions, then Pause subtree on the parent task. The lock-holding run, board users, and paths that stop the run themselves continue to work. Steps to reproduce 1. Create agent A and agent B, with B reporting to A (A gets `tasks:manage_active_checkouts` through the manager chain). 2. Give B an `in_progress` task. Set `checkoutRunId` and `executionRunId` to a `running` heartbeat run of B. 3. As A (agent JWT with its own run), send `PATCH /api/issues/:id` with `{ "status": "backlog" }`. 4. Before this change: `200`, the status is `backlog`, both lock fields are `null`, and B's run is still `running`. ## What Changed - `server/src/routes/issues.ts`: new `assertAgentStatusChangeKeepsLiveRunBound`. For an agent actor, it refuses a status change when `executionRunId` or `checkoutRunId` names a different run in the same company that is `queued` or `running`. It runs in `PATCH /issues/:id` after the final status and assignee are known (execution-policy transitions included) and before any run stop or write. It also runs in recovery resolution when the source status changes. - The guard applies to any status other than `in_progress`, also to the current status, because the service clears the locks for those too. - The write is conditional on the lock the guard checked. The route re-reads the lock and passes it to `svc.update` as `expectedExecutionLock`. The service compares it under the row lock and returns `409 issue_execution_lock_changed` if a checkout landed in between. - These cases stay allowed: the lock-holding run itself, board users, requests with no status, a change to `in_progress`, `cancelled` (the route cancels the run), reassignment (the route stops the run first), `done` through PATCH while a Runner goal is active (terminalization pauses the goal and stops the run; recovery resolution does not get this exception), and a lock holder that already finished. - `packages/shared/src/issue-write-denial.ts`: new denial code `issue_status_change_live_run` (409). The copy says the refusal is final, links the task as `[identifier](/prefix/issues/identifier)`, and names Stop or Interrupt and More task actions, then Pause subtree. A new optional `issuePath` context field carries the link. The UI notice renders it through the existing generic path. - `doc/SPEC-implementation.md`: documents the rule in the checkout contract section. - Paths checked with no change needed: there is no bulk issue update route; tree holds and run cancel are board-only; `POST /issues/:id/release` already lets only the assignee's checkout run release; API-catalog actions call the same PATCH route; MCP status changes already refuse with `MCP_ACTIVE_EXECUTION`. ## Verification - Red on master, green with this change: - `server/src/__tests__/issue-stale-execution-lock-routes.test.ts` (embedded Postgres, real issue service): a manager moves its report's running task to backlog. On master: `200`, status `backlog`, `checkoutRunId`/`executionRunId` changed to `null`. With the fix: `409 issue_status_change_live_run`, row unchanged, run still `running`. - `server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts`: refusals for `backlog`, `todo`, `blocked`, `in_review`, `done`, a queued lock holder, and a repeated current status (red on master). Allowed cases: field edits, the lock-holding run, board, `cancelled` (run is cancelled), and finished lock holders; an allowed change carries `expectedExecutionLock`. - Same embedded-Postgres file: `svc.update` with an `expectedExecutionLock` that differs from the row returns `409 issue_execution_lock_changed` and leaves the row unchanged. - Also passed locally: `issue-closed-workspace-routes`, `issue-comment-reopen-routes`, `issue-recovery-actions`, `issue-scheduled-retry-routes`, `low-trust-red-team-routes`, `issue-update-comment-wakeup-routes`, `external-object-routes`, `packages/shared` `issue-write-denial`, `ui` `IssueWriteDenialNotice`. - `tsc --noEmit` shows no errors in the changed files. Full typecheck, build and test are left to CI. ## Risks - Behavior change for agents: a lead or CEO that used status edits to take over a report's running task now gets a 409. The supported paths (comment, cancel, reassign, or the person's Stop or Pause subtree) still work. - A queued run that holds the lock also blocks the change. This is deliberate: the queued run would otherwise start later with no task bound to it. - Agents can still cancel another agent's run by cancelling its task. That is existing, documented behavior ("task cancellation cancels the intent"), and this PR does not change it. ## Model Used - Anthropic 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-09 08:21:40 -06:00

1 2 3 4 5 ...