mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
9a52a2dfebed26113da07ac559cbfc108f57b225
3364
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
9a52a2dfeb |
fix(native): retry ACPX session.open startup timeouts and give cold Claude a 60 s bound (#19661)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs start an ACPX provider through Runner. `session.open` opens the provider session before any prompt. > - On a loaded host, a cold ACPX Claude `session.open` took longer than its 30-second bound. The controller reported `runner_local_connect_failed: PRP command session.open timed out`. > - The server classified this as the generic `native_session_interrupted` with `ambiguous_state`, so the task was blocked at once. A person had to click Retry, sometimes several times. In one restart-chaos storm, one task needed three Retry clicks, and on another task both Retry runs timed out too. > - The server already has a bounded same-run bootstrap retry for failures that come before provider work (#18854). It did not recognize this timeout. The 30-second bound also cut off normal slow startups. > - This pull request gives cold ACPX Claude a 60-second `session.open` bound, like Pi and OpenCode (#19487). It also retries a startup `session.open` timeout automatically on the same run, from clean state. > - The benefit is that a slow cold start on a busy host completes, or retries itself, instead of waiting for a person. ## Linked Issues or Issue Description Refs #18854 (bounded bootstrap retry for definite `session.open` failures), #19487 (60-second cold OpenCode admission), #19665 (the bootstrap proof that this change reuses). **What happened?** ACPX Claude runs failed with `runner_local_connect_failed: PRP command session.open timed out` about 35 to 40 seconds after they started. The coordinator recorded `native_session_interrupted` with `recoveryMode: "ambiguous_state"` and `providerSessionEstablished: false`. The task was blocked with a board-owned action. No provider event and no checkpoint existed. The host 1-minute load was 17 to 44, sometimes with no restart nearby. **Expected behavior** A normal slow cold start completes. A `session.open` that still times out during runner startup, with no provider event, no checkpoint and no turn, retries on the same run with backoff. At most two automatic attempts follow the first. A run where provider work may have started is never retried automatically. **Steps to reproduce** 1. Run ACPX Claude tasks on a host with a 1-minute load of about 25 or more. 2. Observe a run fail after about 35 to 40 seconds with `PRP command session.open timed out`. 3. Observe the task blocked with `ambiguous_state` and a manual Retry. **Paperclip version or commit** `master` at `6c7cc2c5d6`. ## What Changed The bound (evidence below): - `coldAdmissionTimeoutMs` gives cold ACPX Claude 60 seconds (live adoption keeps 30 seconds). The runner's Rust sidecar `session_open_timeout` and the sidecar's provider handshake (`acpxSessionHandshakeTimeoutMs`) give Claude and Pi 60 seconds too. All three bounds must agree, or the shortest one fails first. Claude's warm run attachment uses the same cold budget, like Pi. The automatic retry: - `nativeSessionFailureSourceCode` returns `provider_session_open_timeout` only for exactly `<startup failure code>: PRP command session.open timed out`. The startup failure codes are `runner_local_connect_failed`, `runner_direct_wss_failed` and `runner_ingress_unavailable`. The transport uses them only before startup completes. After startup the code is `provider_transport_failed`, which is unchanged. - `nativeProviderRecoveryEvidence` treats the cause as pre-session. Durable evidence still decides: a bootstrap retry needs no provider event (`session.started`, `turn.started` and the others) and no checkpoint. The existing budget applies: three attempts with backoff of about 30 seconds, then about 60 seconds. When the budget is used, the task is blocked with the existing "Provider startup failed on 3 attempts before any provider work began" action. - A timeout is indeterminate: the runner can open a provider session after the controller stops waiting. Live runs showed both leftovers, so the retry now starts from clean state: - `prepareNativeSessionOpenTimeoutRetry` (server) runs when the failure settles. It quarantines a local runner root only when the restart-recovery proof (`runnerdStateProvesIncompleteBootstrap`, #19665) shows that no turn began and every recorded process is gone. Any other root, and any remote runner, stays `ambiguous_state` as before. - `resetAbandonedBootstrapRecord` (runner ACPX adapter) runs on a fresh open (no runner-recorded identity). If ACPX has a saved record for the session key with no messages, no prompt and no events, it is marked `reset_on_next_ensure`. Claude saves nothing before the first prompt, so reusing such a record fails the first turn with `session_resume_required`. A record that holds any conversation is unchanged. - A chat retry of an exhausted `provider_session_open_timeout` run is accepted like the other pre-provider causes. - `doc/execution-semantics.md` and the Runner README describe the cause and the bound. ## Verification **Measured startup distribution** (ACPX Claude `runner.session.bootstrap` / `runner.session.startup` spans from restart-chaos run events, 1-minute load 8 to 44): | Source | Bound | Successful cold bootstraps (s) | Failures | | --- | --- | --- | --- | | Earlier storm evidence | 30 s | 2.9, 4.7, 7.4, 8.6, 8.7, 26.2, 27.4, 27.6, 29.2 | startups ended at 34.9 to 40.3 s (the 30 s `session.open` bound plus spawn and `run.prepare`) | | This branch, wave s3601 | 60 s | 6.3, 7.8, 8.1, 13.4, 14.4, 22.3, 22.8, 22.9, **37.3** | 0 `session.open` timeouts | | This branch, wave s3602 | 60 s | 13 samples, p50 10.5, p90 17.9, max 25.7 | 0 `session.open` timeouts | | This branch, wave s3610 (final head, load 28 to 42) | 60 s | 10.4, 11.3, 11.7, 12.0, 13.3, 14.2, 29.6, **30.4**, **30.9** | 0 `session.open` timeouts | Under load, successes clustered just under 30 s and failures sat just above it, so the 30 s bound was cutting the distribution off. With 60 s, cold starts of 30.4, 30.9 and 37.3 s succeeded that the old bound would have failed. 60 s is about 1.6 times the largest observed success, and it matches Pi and OpenCode. **Live restart-chaos waves** (local, ACPX Claude only, 4 in flight, `--rc06-masked`, real provider, K1/K2 restarts): - s3601 (8 tasks, load 19 to 32): 7/8 PASS. No `session.open` timeout. The one failure is an R3 Retry signature after K1 (`native_restart_recovery_blocked`), not this cause. - s3602 (8 tasks, load 17 to 39): 8/8 PASS. No `session.open` timeout. Three Retry clicks were for `native_provider_terminal_failed` (`provider_turn_lost_on_restore`), a different restart signature. - Forced timeout, s3609 (4 tasks, B5, K2): a local, uncommitted fault injection gave every other cold startup a 1.5 s `session.open` deadline. Result 4/4 PASS. Run 4fe6537a timed out, retried itself on the same run, and then succeeded through four K2 restarts with no person involved. Run 02799a85 hit the injection three times in a row; the budget stopped it after attempt 3, and one Retry completed it. - Before the two clean-state fixes, the same forced wave (s3607 and s3608) showed the retry reusing leftover state: `turn.start` failed with `session_resume_required`, and then "ACPX session is not open". Those runs are the red evidence for the two clean-state fixes. - Final head, s3610 (8 tasks, K1/K2, no injection, load 28 to 42): **8/8 PASS**, 0 `session.open` timeouts. The one Retry click (t06) was for `provider_turn_lost_on_restore`, a different restart signature. **Tests:** - Red before the fix: `native-startup-retry.integration.test.ts -t timeout` (embedded PostgreSQL, real executor, real resume claim). The 3 retry cases failed with `native_session_interrupted` / `ambiguous_state`. - Green: `native-startup-retry.integration.test.ts` passes 36/36. New cases: - for each startup failure code, a timeout retries on the same run with backoff, keeps the task `in_progress` with an agent-owned `resume_native_run` wake, and blocks after attempt 3 with a board-owned action; - no automatic retry when `session.started` or `turn.started` is durable, when the code is `provider_transport_failed`, or when another command timed out; - a real runner root with an opened session and no turn is quarantined; a root with a turn command stays `ambiguous_state` and intact; a remote runner is never prepared. - `codex-runtime-adapter.test.ts`: 113 pass. New cases: a 45 s delayed handshake is admitted for Claude and rejected for Codex; a never-prompted record is reset only on a fresh open; records with messages, a prompt or events are kept. - `abandoned-bootstrap-record.test.ts` (real `createAcpRuntime` with `createRuntimeStore` and a controlled ACP peer that, like Claude, cannot load a never-prompted session): without the reset, a fresh open reuses the record and the first turn fails with SESSION_RESUME_REQUIRED after `session/load`. With the reset, the provider receives `session/new` and the turn completes. - `vitest run src/drivers/acpx src/cli` in the runner: 107 files, 1,935 tests pass. - `native-session-executor.test.ts -t "session.open|startup|bootstrap"`: 144 pass. `runnerd-codex-transport.test.ts -t "cold process admission"`: 4 pass. `cargo test -p paperclip-runner-core --lib admission_budget` passes. - `npx tsc --noEmit -p server` and the runner `tsc -p tsconfig.test.json` pass. ## Risks - **This changes Runner Rust code. The Daytona runner image must be rebuilt after merge** for remote sandboxes to get the 60-second sidecar bound. Until then, a remote Claude `session.open` still fails at 30 seconds in the sidecar (as a definite failure, which already retries). A remote timeout stays operator-owned, because a remote root cannot be proven clean here. - A cold ACPX Claude start that hangs now fails after about 60 seconds instead of 30 seconds. - A timed-out `session.open` may have created an empty provider session. No prompt was sent. The retry quarantines that runner root and asks ACPX for a new session. - Same-run retries of a fresh-attempt Retry run can still fail with `isolated_task_directory_binding_mismatch`. This is RC-35 (workspace re-preparation), which has its own fix in progress. - No schema or API change. The new error code appears on runs and in run error codes. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
b94963f91e |
fix: retry temporary repository catalog failures before dispatch (#19690)
## Thinking Path > - Paperclip manages AI agents and their work. > - A task must check repository access before it starts agent work. > - Repository discovery can fail while the user's existing grant remains valid. > - Discovery previously replaced a known temporary provider refusal with an empty catalog. > - The task then reported lost access and did not schedule recovery. > - This change keeps temporary discovery failures distinct and uses the existing bounded retry owner. > - Each retry must pass a fresh access check before it can use the saved repository. ## Linked Issues or Issue Description **What happened?** A task with a saved catalog repository stops with an access denial when GitHub returns an exhausted-quota response or HTTP 503. The task does not start provider work, but the discovery layer discards the temporary failure and returns an empty catalog. **Expected behavior** Known temporary failures before provider dispatch use the existing bounded infrastructure retry. The successor checks the same responsible user's current grants and reads the saved files only after fresh discovery succeeds. A completed denial remains terminal. **Steps to reproduce** 1. Bind a task to an authorized catalog repository with saved local changes. 2. Make its authorized GitHub discovery return HTTP 403 with zero remaining quota, or HTTP 503. 3. Let discovery recover before the existing retry deadline. 4. Observe one successful successor using the original repository bytes. **Paperclip version or commit** Reproduced on master at `b8d94b3ad1`. A search found no existing PR for this temporary catalog failure. ## What Changed - Keep a safe typed failure for known HTTP availability and quota responses. Honor the later provider deadline and bound it to one day. - Propagate that failure only when fresh discovery cannot find a required repository. Preserve successful partial catalogs and the existing connection setup error contract. - Classify the failure only before provider dispatch. Reuse existing bounded retry, interaction ownership, and bootstrap settlement rules. - Keep current grant checks, saved repository selection, normal Stop, and provider-started failure policy. - Keep provider deadlines when accepted interactions use their own retry owner. Recognize secondary HTTP 403 rate limits with valid retry advice while keeping ordinary permission denials terminal. This matches [GitHub rate-limit guidance](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api). - Keep direct repository request responses outside the automatic bootstrap retry contract. Test successful partial discovery and missing-required-repository recovery with two independent eligible connections. - Document task repository discovery recovery. ## Verification - Behavioral RED: real heartbeat and PostgreSQL fixtures reached actual HTTP 403 and503 discovery responses, started no adapter, and found zero successors instead of one. Both cases failed before the production change. - Behavioral GREEN: both cases waited for the real retry deadline, used the normal queue worker, ran the controlled adapter once after fresh discovery, and read the original saved bytes. Two cases passed in 74.73 seconds. No external model or provider request was made. - Deadline RED/GREEN: a combined Retry-After and quota-reset response initially selected the earlier deadline. The regression now selects the later deadline. - Initial repair at `d8d4714a18` (same production source as `018e8c52d345d79972d7907468080dc9908c299f`): seven affected files passed, 69 cases in 185.16 seconds. That test-only cleanup then passed all five real-database acceptance cases in 134.76 seconds. - Commands: `pnpm exec vitest run` with `heartbeat-repository-catalog-recovery.test.ts`, `github-catalog-timing.test.ts`, `github-grant-metadata.test.ts`, `project-repositories-persistence.test.ts`, `task-workspace-repositories-persistence.test.ts`, `github-repository-catalog-failure.test.ts`, and `services/heartbeat/retries.test.ts` from the server source tree. The acceptance file also ran fresh at `018e8c52d3`. A fresh five-case run after the P1 repairs passed at `f490ca6b78` in 122.06 seconds. The final eight-case file passed fresh at `e5b2efab11919467850f55fd6345317132b9b672` in 124.62 seconds, using real owned embedded PostgreSQL. It covers automatic recovery with saved bytes, genuine denial, provider-started classification, normal Stop, the direct HTTP route, and both partial-catalog outcomes. - Review repairs at `f490ca6b787cf77df96d3d6edfb2ab9115eb19a6`: both reported P1s reproduced RED. The real database interaction scheduler chose a retry after 24 seconds before the provider deadline of one hour. A secondary HTTP 403 with valid Retry-After lost its retry classification. The narrow fixes then passed 44 cases across scheduler, header, and metadata-contract files in 37.66 seconds. - A real direct request through the task workspace router and normal error middleware reproduced a 500 response instead of the existing 403. Required-ID retry mode now applies only to admission and its rechecks. The route and both two-connection partial-catalog cases then passed in 29.09 seconds. Both partial cases use actual owned grants and credential bindings. - All seven server type layers passed again at final head `e5b2efab11919467850f55fd6345317132b9b672`. The affected test type layer passed again after the test-only cleanup. Environment: macOS arm64, Node 26.4.0, pinned pnpm 9.15.4, Vitest 5.0.3, owned disposable PostgreSQL 18 on loopback. The current checkout's shared packages, plugin SDK, and Runner were built first. - One local acceptance run under concurrent owned test/typecheck load hit the unchanged 15-second Stop-case timeout. Its four other cases passed. The unchanged complete file then passed serially. The timeout receipt remains recorded; no assertion or timeout was weakened. - A provider-started failure retains the existing generic adapter retry policy. It does not receive the catalog no-provider certificate or a provider retry deadline. Fixture cleanup cancels its scheduled retry through the normal owner. - Final head `e5b2efab11919467850f55fd6345317132b9b672` has 59 successful checks, 3 allowed skips, no pending or failed check, and Greptile 5/5 with no open findings. [Completed CI workflow](https://github.com/paperclipai/paperclip/actions/runs/42322202977). - Additional local diagnostic for the late recovery-owner advisory: on unchanged `e5b2efab`, the dedicated scheduler returned `retry_exhausted` with attempt 3/max 2. The normal stranded sweeper then escalated once, created no extra run, and started no adapter, even after catalog access recovered. One case passed in 17.32 seconds. The existing failed-predecessor path delegates to that bounded scheduler and returns; it does not enter a new generic retry budget. No production registry change was made. This diagnostic was not a CI case. Its local harness is preserved separately at `74e3854eea3f7c4121fdcd9c8a8ad522cd8c7477` on `codex/catalog-exhaustion-acceptance`; its production source is identical to `e5b2efab`. - Module boundary and whitespace checks passed. ## Risks - A temporary failure from another eligible connection can delay a truly missing repository for the existing limit of two attempts. It cannot grant access. - Unknown network errors and genuine access denials retain their existing behavior. This change handles known HTTP failures only. - Historical provider quota causality is unproven. This repair has a separate local producer regression. It does not rewrite past failures or queued user intents. - Acceptance uses real local database and heartbeat owners with controlled HTTP and adapter fixtures. It does not claim a live native-provider recovery. ## Model Used OpenAI GPT-6.1 Codex (`gpt-6.1-sol`), with high reasoning, repository tools, code execution, and controlled local PostgreSQL tests. The host did not expose a context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
254b44f72b |
fix(remote-workspaces): recover retired roots that cannot be renamed (#19674)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Remote workspaces must retain saved files across process and sandbox replacement. > - Restore uses identity receipts to distinguish a retired owned root from an unrelated directory. > - Some overlay filesystems reject a directory rename with EXDEV even within one mount. > - That error currently prevents a valid saved workspace from being restored. > - This pull request replaces only a verified retired root in place when that rename fails. > - Runtime state stays in place. Each restore holds a root-specific OS lock. > - A durable workspace epoch fence rejects delayed older commands. A pending marker keeps interrupted cleanup recoverable. ## Linked Issues or Issue Description **What happened?** Workspace restore fails with `remote_workspace_restore_failed: stage=publish code=EXDEV syscall=rename` when the retired directory comes from a lower overlay layer. Retry repeats the same rename. **Expected behavior** Restore should publish the verified saved workspace and preserve runtime state. An interrupted restore should remain safe to retry. **Steps to reproduce** 1. Create an owned retired root with a valid identity receipt, runtime state, and stale files. 2. Make directory rename return EXDEV for the existing lower-layer root. 3. Restore a saved workspace through `restoreRemoteWorkspaceDirectory`. 4. Repeat with an interruption before the new restore receipt is published. **Paperclip version or commit** The regression fails before this change on base `a2aa69bc6230c8b81de3588d7d5571b38db4dd04`. **Deployment mode** Remote sandbox with an overlay filesystem. Local regression: macOS arm64, Node 26.4.0, source checkout. Refs #19231. Refs #19251. Those changes establish the retained-root restore path. This fix handles its EXDEV publication failure. ## What Changed - Keep the EXDEV fallback limited to a verified retired workspace root. Preserve runtime state and the existing ownership checks. - Hold a private SQLite OS lock across receipt checks, file replacement, cleanup, and receipt publication. Keep the lock file outside the replaced root. Process exit releases ownership; a lost reply does not. - Pass the existing workspace authority epoch from the real restore caller. Write and sync a separate epoch fence before root changes. Reject lower epochs, conflicting identities within one epoch, and missing epochs on fenced roots. The fence survives SQLite rollback after process death. - Publish a pending marker without a completed identity until stale preparation-receipt removal and directory-mode cleanup finish. A retry can finish that cleanup. A higher authorized epoch can then restore its selected save. - Preserve later file and permission edits when adopting a completed restore. Refuse unsafe lock files and malformed authority records. Document compatibility and the stop-evidence boundary. ## Verification - RED before the original fix: real restore commands failed with EXDEV. The test preload changes only the kernel rename result. - RED before the review fixes: a live command with an unconfirmed timed-out reply overwrote edits made after retry. Tests also reproduced incomplete cleanup after receipt publication. - RED before the authority fence: a delayed older command replaced a newer completed restore. It also passed after the newer command was killed before root mutation. The final test uses real separate Node processes, OS lock contention, SIGKILL, and a durable on-disk fence. - Final local checks: 142 transport filesystem tests, 16 real PostgreSQL restore-caller cases, and all seven server typecheck layers. The caller cases verify the published receipt uses the current workspace epoch. - Late review RED: an actual separate SQLite owner made the real PostgreSQL restore caller mark an expired lock wait non-retryable. Corrupt fence and receipt bytes also produced a raw syntax error without the required refusal reason. Both regressions now pass. Only the test clock advances the lock deadline; actual OS lock contention and the production caller run unchanged. - Lock contention stays retryable only under the existing same clean writer, epoch, and snapshot proof. Four negative cases change those fields and remain non-retryable. After the owner stops, normal restore returns the useful saved files. Corrupt authority records stay untouched and non-retryable. - Safety cases cover same and different authorized identities, process-death lock release, an unrelated root making progress, pending cleanup interruption, malformed or missing fences, invalid input, and unsafe lock parents/files. - The first final transport attempt passed 139 cases and failed only because the restricted shell could not open a Unix socket (`listen EPERM`). The full suite was then run with the required local socket permission. No production change was made for that setup failure. - Reproduce the filesystem checks from `server/`: `../node_modules/.bin/vitest run --config vitest.config.ts src/services/remote-workspace-transport.test.ts`. Run the caller file with `-t "restores a newer save over this workspace|preserves the published workspace path"` and the repository test PostgreSQL setup. Run `node scripts/typecheck-layers.mjs` from `server/` for all seven layers. - Agent preflight, module boundary audit, and `git diff --check` passed. No added code comments or private issue references occur in the patch. - Local EXDEV proof uses macOS arm64 and Node 26.4.0. It does not claim a new Linux overlay-kernel run or hosted recovery. Final source: [`8092cc6b2ec448ff372337b1e7a43854eae7bb08`](https://github.com/paperclipai/paperclip/commit/8092cc6b2ec448ff372337b1e7a43854eae7bb08). [Exact-head CI](https://github.com/paperclipai/paperclip/actions/runs/42321313715) completed with 59 successful checks, three conditional skips, and one neutral check. Greptile gave this exact head 5/5. The independent review passed. No review threads remain open. ## Risks The EXDEV path changes files in place. It must finish before execution admission. Participating restore commands use a private permanent SQLite lock file and a separately synced epoch fence. The qualified sandbox Node runtime must support `node:sqlite`. Lock contention has a 120-second limit and does not clear live ownership. A legacy command that never acquired this lock remains outside its protection. Hosted recovery must confirm that command stopped; a timeout is insufficient. The new fence does not backfill that evidence. A conflicting identity in the same epoch or a malformed fence stays blocked. A higher epoch can recover an interrupted pending restore only with the existing retired-root replacement authority. No controller database schema, execution permission, or provider-state guard changes. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`) authored the recovered patch. OpenAI Codex, GPT-6 family, recovered, reproduced, repaired the review findings, and verified the change with filesystem tests, shell tools, and code execution. The Codex exact serving model ID and context-window size are not exposed in this session. No value is inferred for them. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
06bbcea12f |
fix(budgets): price lost receipts and stop restart-derived accounting from hard-stopping companies (#19673)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission and run accounting decide whether any task in a company may start > - #19497 made pending accounting an admission hold and settles a stopped run's lost receipt as an unpriced charge > - Restart-chaos waves still stopped whole companies. A lost Codex receipt with complete token counts settled as unpriced because the rate card only prices complete receipts. Under the default blocking policy that hard-stopped the company at 0.38 of 3000 cents. In another storm, a resumed run's continuation was refused by its own task's recovering-accounting hold. Separately, unpriced charges with zero tokens or a known partial cost hard-stopped a company at 120 of 2000 cents > - This pull request prices a lost receipt's observed usage from the rate card, lets a task's continuation pass its own recovering accounting, and stops restart-derived or evidence-free unpriced charges from tripping the blocking policy > - The benefit is that a restart no longer pauses a company by itself, while genuinely unpriced usage still blocks ## Linked Issues or Issue Description Follow-up to #19497. Refs #19606. **What happened?** - Wave A (K6 crash loop): native Codex run `5037b8d1` was killed mid-turn. Its receipt was lost, but `usage_json` held gpt-6.1-sol with 28690 input and 220 output tokens. The charge was written `cost_status=unpriced`, `pricing_provenance.source=unknown`, because `priceCodexReceipt` returns early when `!receipt.complete`. 0.2 s later the default `unpricedUsagePolicy=block` opened a hard incident at 0.38 of 3000 cents. The company was paused, and 6/6 tasks were blocked with `canRetry=false`. - Wave B (K1 storm): Claude run `2a7a8abf` resumed after the restart. `reserveRunBudget` refused its interaction continuation with "New work must wait while a native run recovers its unfinished accounting", so the task could not settle its own recovery. A 0-token unpriced charge and an unpriced charge with a known 0.81-cent cost then hard-stopped the company at 120 of 2000 cents. **What did you expect?** A restart-lost receipt with known numbers is priced as an estimate, and no restart-derived accounting state pauses a company by itself. A task's own continuation is never held by its own accounting. Real usage above the limit, and genuinely unpriced usage, still stop work. **Steps to reproduce** 1. Give a company a hard-stop budget with the default unpriced-usage policy. 2. Kill a native Codex turn after it has reported tokens, so the final receipt is lost and native recovery ends in terminal failure with a verified stop. 3. Observe an unpriced charge for known tokens, a hard incident, and a paused company. ## What Changed - `run-cost-accounting.ts`: when a lost receipt settles and its usage has known tokens and no known cost, price it with `priceCodexReceipt`/`priceAnthropicReceipt` as a complete per-run checkpoint. The charge is `estimated`, its provenance keeps the rate-card source, and its evidence says "final receipt lost; observed usage is a lower bound". The estimate is also written to the run's usage so a later receipt reconciles against it. Zero observed tokens, and per-model usage, stay unpriced (unknown is never shown as zero, and aggregate totals never take one model's rate). - `budget-exposure.ts`: - The pending/recovering accounting hold excludes the run being admitted and earlier runs of the same task (same `native_issue_id`). A task's continuation and finalization can therefore settle its own accounting. Other tasks stay held, and those runs still count as spend. - The blocking unpriced-usage policy no longer counts these unpriced charges: the settled final charge of a lost receipt (restart-derived, any usage; later growth still counts), a charge with a known partial cost, and metered API or subscription billing with no observed tokens. Unknown billing with no tokens (for example Cursor Cloud) and complete-receipt usage that has no price still block. - `budgets.ts` / `budget-reservations.ts`: admission accepts the admitted `runId` and passes it to the exposure read. `reserveRunBudget`, the final dispatch recheck and the read-only admitted recheck all supply it, so a continuation that reserves can also dispatch. - Daytona wave s12 (run f1d98917, a blocked remote recovery with a 0-cent held reservation and every lease destroyed) is closed by #19606. A Retry for that same task passes the hold through the same-task exclusion here. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - Lost Codex (gpt-6.1-sol) and Claude (claude-sonnet-5) receipts with observed tokens settle as `estimated` rate-card charges with the lower-bound evidence. The company stays active and admission is clear under the blocking policy. - A lost receipt for a model with no rate card stays unpriced but opens no incident and no pause. - A task's continuation reserves budget past its own recovering accounting, while another task's run is still held. - Unpriced charges with no tokens on metered billing, or with a known partial cost, do not hard-stop. - Complete-receipt usage with no price still blocks. - The six new cases fail on master and pass with this change. - Existing budget and accounting suites pass locally: budget-enforcement-scoping, budgets-service, heartbeat-cost-accounting, run-cost-accounting, native-stop-accounting, cost-data-quality, budget-live-attempts, budget-live-usage. Server typecheck passes. ## Risks - A lost receipt's estimate is a lower bound. The provenance says so, and a later complete receipt still reconciles. - Restart-derived unpriced charges no longer trip the blocking policy. Usage lost with a killed provider is not counted against that policy, which is consistent with budget enforcement being best effort. Unknown-billing and complete-receipt unpriced usage still block. - The same-task exemption relies on `native_issue_id`. Legacy runs without it keep the old behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres 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 Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
785ead5c7a |
fix(tasks): hold a Retry while the task's own native session close is retained (#19609)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native provider session close fails, the runner keeps a cleanup owner or quarantine in process memory. Its admission gate refuses new sessions for that task (scoped to the task since #19532) > - The server could not see that state. A task's Retry was accepted (202), its run was dispatched, and the runner refused it at once with `native_session_cleanup_quarantined: prior session cleanup remains incomplete` > - The user saw a Retry that failed instantly, and nothing said what to do > - This pull request lets the server read the runner's per-task cleanup gate and hold the Retry with a clear reason. The Retry starts automatically when the close is released > - The benefit is that a Retry is never dispatched only to fail on a block that is already known ## Linked Issues or Issue Description **What happened?** After two hard restarts, a Codex run's close stayed incomplete. The board Retry for that task was accepted, and its run failed in about 1 s with `native_session_cleanup_quarantined: prior session cleanup remains incomplete`. The next Retry did the same. **Steps to reproduce** 1. Let a native session close fail or stay pending for a task. 2. Press Retry on that task. **Expected behavior** The Retry waits with a clear reason, and starts when the cleanup finishes. If the cleanup needs an operator, Retry is not offered and the one working action is shown. **Actual behavior** The Retry is accepted and its run fails at once. ## What Changed - `native-session-runtime.ts`: - New `nativeSessionCleanupBlock({ companyId, issueId })`. It is a read-only view of the in-process admission gate for one task: `pending` (a retained close or a bounded automatic close recovery still owns the task's provider resources), `operator_required`, or `null`. It grants no authority. - New `onNativeSessionCleanupSettled(listener)`. It is called whenever a cleanup owner or quarantine entry is released. - `explicit-native-continuation.ts` (the continuation gate for a Retry or a new message): - `pending`: the input is held with `local_cleanup` ("Retry is waiting for the previous execution to finish cleanup. Your message is saved."). It starts automatically when the close is released. - `operator_required`: handled exactly like a recorded `native_session_cleanup_quarantined` run. Retry is not offered. A new message runs the existing stopped-process proof (`verifyStoppedNativeSessionForContinuation`), which retires the runner quarantine when the old processes are proven stopped. - `native-cleanup-release-wake.ts` (registered in `server/src/index.ts`): each cleanup release wakes deferred execution for the company in a transaction, so a held Retry re-evaluates. - `server/src/vendor/paperclip-runner/index.ts`: exports the two runtime functions. ## Verification - Red before the gate (server change stashed): `pnpm --filter @paperclipai/server exec vitest run src/services/quarantined-task-retry.test.ts -t "holds a Retry"` dispatched a successor run while the task's close was retained (`expected [ { … } ] to have a length of +0 but got 1`). - Green after the gate: - The Retry is held (`deferred_issue_execution`, `executionWait.reason = local_cleanup`), and no successor is dispatched. - A different task is not blocked. - When the close settles, the release is announced, and the next pass dispatches exactly one successor. - The whole `quarantined-task-retry.test.ts` file passes (28 tests). `src/native-session-runtime.test.ts` passes. Server `tsc --noEmit` passes after the runner package build. - Live chaos replays: results are added in a comment. ## Risks - The gate reads process memory, so it applies only where the server and the runner session share a process (local runner). Other deployments keep today's behavior. - A held Retry re-evaluates on the release wake (sent in its own transaction) or on the existing 30 s deferred-execution pass. When the bounded automatic close budget is spent with nothing else scheduled, the view reports `operator_required`. Retry is then no longer offered, and a new message reaches the stopped-process recovery instead of waiting forever. - No schema or API shape changes. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Refs #19532. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
29e05f7e06 |
fix(heartbeat): sweep orphaned run scratch directories (#19671)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each heartbeat run gets a scratch directory in the system temp folder > - Each process attempt makes a new directory with a marker file > - Cleanup removes only the directory of the current attempt > - A restart kills the old attempt, so its directory stays forever > - One restart storm left 83 directories across 78 runs > - This pull request sweeps those directories when the run ends and at startup recovery > - The benefit is that temp space no longer fills with leftover directories ## Linked Issues or Issue Description **What happened?** Run scratch directories made before a restart are never removed. A restart storm left 83 `paperclip-run-*` directories across 78 runs. Each holds only `.paperclip-run-scratch.json`. **Steps to reproduce** 1. Start a run that makes a scratch directory. 2. Kill the server process and start it again. 3. Let the run finish. 4. List `$TMPDIR/paperclip-run-*`. The old directory is still there. **Expected behavior** The old directory is removed after the run ends, or at startup. ## What Changed - Add `sweepOrphanedRunScratch` in `server/src/services/run-scratch.ts`. - The sweep removes a directory only if it has a valid regular-file marker and its run is terminal in this instance database. It keeps a directory whose run it cannot see, because another instance can share the temp root. - The sweep keeps active runs, unmarked directories, symlinks, and directories outside `paperclip-run-*`. - The sweep checks each directory again before removal and bounds the scan (500, with a rotating start so kept entries do not starve later ones) and removals (100). A per-run sweep filters by directory name first. Marker run ids that are not UUIDs never reach the database query. - Call the sweep after the current attempt's cleanup in `heartbeat.ts`, for that run only. - Call the sweep in `reapOrphanedRuns` (startup recovery) with run status from the database. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/run-scratch.test.ts` - New tests cover: leftover directories of a terminal run are removed, an active run is kept, an unmarked directory is kept, a symlink is not followed, and removals are bounded. ## Risks - Low risk. The sweep acts only on marked directories of terminal runs. - A directory left by a previous attempt could hold a live grandchild process. The sweep does not check old process groups. The run is already terminal. ## Model Used - Claude, claude-opus-4-5 class model 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
0b73c805e8 |
feat(environments): fill an unset instance default with a managed plugin environment (#19551)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed instance gets its instance-level environments from the `PAPERCLIP_MANAGED_CONFIG` document, which can declare plugin environments next to the managed sandbox > - A platform that runs managed instances wants a new instance to start with one of those plugin environments as its default, without changing the default of any instance that already has one > - #19632 made boot fill the instance default only when it is unset, local or deleted, in one locked transaction, and removed the revert, so every existing managed instance keeps a set default > - This pull request adds an `instanceDefault` flag to a plugin entry. That entry fills an unset default through the same locked fill, before the managed sandbox does > - The fill follows the same readiness rule as an admin's default write, so it never moves ready agents onto a plugin that has not prepared them > - The platform also needs a way to know whether a release understands plugin entries, so the release now ships a small capabilities file > - The benefit: new instances get the plugin environment as their default, and existing instances and admin choices are never changed ## Linked Issues or Issue Description Refs #19632 (the locked fill-if-unset rule this builds on), #19508 (plugin entries in the managed config) **Subsystem affected** Server environments: the managed config parser and the managed environment bootstrap. **Problem or motivation** The managed config can declare plugin environments, but it can't make one the instance default on a new instance. An external platform also has no way to tell whether a given release supports plugin entries before it sends one. **Proposed solution** - Add `instanceDefault: true` to at most one plugin entry. Right after that entry's boot ensure succeeds, it fills the instance default through `fillUnsetInstanceDefault`. The managed sandbox fill runs after it. - On a new instance the plugin environment wins. On an existing instance the default is already set, so neither fill changes it. - The fill declines while an active agent that follows the default hasn't been prepared by that plugin. This is the rule the instance-settings route enforces. When the plugin fill declines, the managed sandbox fills as usual. - Ship `server/src/services/managed-config-capabilities.json`, which lists `pluginEnvironments`. **Alternatives considered** - A recorded one-time decision with a retry worker. It needed pending states and extra checks for many writer and trigger combinations. Filling only an unset default, under the existing lock, needs none of that. - Rejecting the fill whenever agents exist. That is stricter than the admin route and blocks harmless cases, such as agents that are paused or already prepared. **Roadmap alignment** This extends existing managed-environment behaviour. It adds no UI. ## What Changed - `server/src/services/managed-config.ts`: - `ManagedPluginEnvironmentSpec.instanceDefault` (boolean, default `false`). - The parser refuses `instanceDefault` on more than one plugin entry, on the sandbox entry (even `false`), or as a non-boolean. - `MANAGED_CONFIG_CAPABILITIES` and `MANAGED_CONFIG_CAPABILITIES_SCHEMA_VERSION` constants. A plugin entry is refused unless `pluginEnvironments` is listed. - `server/src/services/managed-config-capabilities.json`: `{ "schemaVersion": 1, "capabilities": ["pluginEnvironments"] }`. A test pins it to the constants and to this literal contract. - `server/src/services/managed-environments.ts`: - New `fillPluginInstanceDefault`. It runs right after a successful boot ensure of the flagged entry and logs a fill or a decline. Recovery after boot never fills. - The managed sandbox fill now runs once, after the plugin entries. - `server/src/services/managed-instance-default.ts`: `fillUnsetInstanceDefault` runs `agentsNeedSetupForEnvironmentChange` inside its locked transaction and returns `declined: "agents_need_setup"`. A sandbox target never needs agent setup, so the managed sandbox fill behaves as before. - `server/src/services/plugin-environment-driver.ts`: the readiness query is factored into `agentsNeedSetupForEnvironmentChange`. `assertAgentsReadyForEnvironmentChange` wraps it and still throws the same conflict. - `doc/DEVELOPING.md`: documents `instanceDefault`, the fill order, the readiness decline, and the capabilities file. ## Verification - `cd server && npx vitest run src/__tests__/managed-plugin-environments.test.ts` (embedded Postgres), under `instanceDefault (fill only an unset default)`: - A new instance gets the plugin row as its default. - An existing sandbox default is kept, and so is an admin's choice. - If the plugin was down at the first boot and the sandbox filled, a later recovery and boot keep the sandbox default. - An admin-cleared or local default is declined while a ready or verifying agent follows the default without the plugin's setup. The managed sandbox then fills it. - It fills when the agents that follow the default are paused or already prepared, or are pinned to another environment. - Without the flag, nothing happens. - A new agent that follows the default gets the plugin in `requiredPluginIds`. - `npx vitest run src/__tests__/managed-instance-default.test.ts` (embedded Postgres): the earlier fill, lock and overlap cases pass. A sandbox fill also succeeds while active agents follow the default. - `npx vitest run src/services/managed-environments-plugin-entries.test.ts`: - The fill order is plugin then sandbox. - When the plugin is down there is no fill, and recovery never fills. - A failed ensure gives only the sandbox fill, and a failed fill keeps the ensure counted. - When the plugin fill declines, the sandbox still fills. - `npx vitest run src/__tests__/managed-config.test.ts src/services/managed-config-capabilities.test.ts`: parser refusals and the capabilities contract. - `npx vitest run src/services/managed-environments.test.ts src/__tests__/instance-settings-routes.test.ts src/__tests__/environment-routes.test.ts`: pass. The route still throws the same readiness conflict. - `pnpm --filter @paperclipai/server typecheck` passes. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. ## Risks - An instance whose default is unset at the first boot after this release gets the plugin environment, if its agents are ready for it. That covers a new instance, and an instance whose admin cleared the default or chose the local environment. This is the intended behaviour. - The readiness check doesn't lock agent rows. If an agent becomes ready during the boot fill, it can miss the check, which is the same race the admin route has. Agents created after the fill are prepared for the plugin, because setup reads the current default. - If the plugin is down at a new instance's first boot, the instance keeps the managed sandbox default from then on. - No schema 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`), Anthropic, through Claude Code with tool use (file edits, shell, 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
7ac99718a4 |
fix(workspaces): fence legacy sources for online migration (#19642)
## Thinking Path > - Paperclip coordinates AI agents and preserves their saved work. > - Remote workspace storage replaces legacy host directories with durable snapshots. > - Several legacy workspace records can name one physical project directory. > - A task lock alone cannot exclude a new task that selects that directory. > - Terminal recovery records can retain valid sources after physical writers settle. > - This change fences the source through its real preparation and lifecycle owners. > - Idle ordinary sources can then migrate while unrelated remote work continues. ## Linked Issues or Issue Description **What happened?** Online migration rejects ordinary legacy directories when several workspace records name the same path. A source inspection found 45 aliases across three project roots with no root Git marker or previously imported nested child. The instance also had unrelated running remote work. Requiring the whole instance to drain prevented useful migration of these idle sources. Terminal native sources and superseded finalization attempts also remain blocked after their exact physical and durable owners settle. Their files, provider allocations and recovery records must remain available through the change of file authority. **Expected behavior** Migrate an idle ordinary source under a derived physical source fence. Let unrelated object-backed turns continue. Preserve provider and recovery obligations. Keep unknown writers, shared Git stores and uncertain source state blocked. **Steps to reproduce** 1. Create several local workspace records for one managed project directory. 2. Give the aliases different source tasks. Leave their local writers idle. 3. Keep a valid remote workspace turn running on another source. 4. Inspect or apply migration to an alias. The old owner returns `overlapping_workspace_requires_maintenance`. 5. Retain a terminal native prepared source with a null coordinator owner, or an old finalize attempt with an exact later committed receipt. The old online path keeps these sources blocked. **Paperclip version or commit** Current tested head: `7478c817f09f7eed396922c0f7831af07ae6ff14`, based on master `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`. Related recovery work: #19131. **Deployment mode** Hosted native Runner with remote workspace storage. Qualification uses disposable local PostgreSQL databases, actual archive/storage owners and synthetic source trees. The source observations were read-only. ## What Changed - Retain the existing company/task source keys and add one physical managed-project key. Heartbeat source selection, materialization, reopen and lifecycle owners share it. Configured aliases of canonical task directories retain their original task key. - Fence runtime provision and each configured service CWD through both startup owners. Check all unsettled service CWDs for physical source overlap, even when workspace IDs differ. Runs with no services keep their filesystem-free path. - Derive a bounded ordinary alias closure. Lock every alias task and lifecycle key. Reject foreign companies, changed membership, unproven descendants, included Git metadata and other unresolved writer obligations. - Commit remote authority on the connection that holds source exclusion. Refresh a stale selected local workspace after admission if migration published remote authority. Publish activity after commit. - Admit exact terminal native recovery sources under the existing physical finalizer keys. Bind descriptors, allocations and owner checkpoints to journaled fingerprints. Recheck them before capture and publication. - Add a separate company-scoped online entry for exact superseded finalization receipts. Keep the offline entry's deployment prerequisites and keep inventory read-only. - Document the proof and the remaining Git/common-store extension. The original files stay in place. No new scheduler, deployment action or database migration is added. ## Verification - Review found that the real reopen path could hold lifecycle exclusion while waiting for a project source. The new actual nested-reopen regression failed before the repair: lifecycle lock count was 1 while another connection held the project fence. The repair acquires declared and physical project sources before reconstruction and lifecycle. The contention case, ordinary-fetch restore and runtime nested restore passed. The lifecycle-worker restore first completed in 16.64 seconds but exceeded its unchanged 15-second body limit; a bounded warm rerun passed at that same limit. Both wrappers exited and completed cleanup. The source type graph, module-boundary check and diff check pass. Final 7478 CI passed: 58 successful jobs, three skipped, one neutral and a successful Snyk context. Fresh Greptile is 5/5 for exact7478, with no unresolved threads. [Final CI](https://github.com/paperclipai/paperclip/actions/runs/42318150314/job/128240631573). - Full CI on `e4283b3452` passed the other server, build and type checks. One exposure owner case failed: two execution workspaces received one preview pair after source admission serialized their starts. The first local seven-case run reproduced that failure; adding the owner argument alone did not change the hash payload. The completed repair binds run-scoped reuse to its execution-workspace owner. All seven existing real-process/fake-broker cases then passed in 13.81 seconds, with wrapper exit 0 and cleanup complete. Public scope IDs and explicit project/agent sharing are unchanged. The final7478 head passed all required CI and fresh review gates. - The service/source/restore owner canary passed 25 cases across four suites before the final no-service fast-path guard (41.33 seconds). This includes actual project service processes with a different project ID and no execution-workspace ID. Their startup waits for source ownership, migration stays blocked while they run, and the source becomes eligible after their verified stop. The final three-case runtime canary passed at the final source inputs (18.70 seconds). It covers that fast path and both service-CWD variants. Both wrappers exited 0 and completed cleanup. - The boundary-repair canary passed 9 cases across four suites at `7c18f1c170`. It covers the real rehome/conversion gateway, project command provisioning, future task selection, stale imported authority, foreign-company binding, and existing standalone filesystem callers. The shared PostgreSQL wrapper exited 0 and completed cleanup. Duration: 94.15 seconds. - The P1 diagnostic passed 46 cases across five suites at the pre-rebase P1 revision. It covers the actual importer with an export warning created by `recordNativeFinalizationFailure`, strict prepared-source/action bindings, finalizer locks, source/lifecycle races, unrelated remote execution, and bare Git with external alternates. It uses the repository shared PostgreSQL wrapper, `--fsModuleCache`, and a local-only 90-second hook allowance. Duration: 245.44 seconds. No product timeout changed. - The canonical retained-directory compatibility check passed 22 cases across two suites. Missing archived task directories still import from authenticated restore points; they do not need a present-directory source proof. - Both initial P1 regressions were observed before the fix. The real finalizer warning made the importer ineligible. The old archive owner accepted a nested bare repository borrowing external objects. The new cases pass and preserve the original warning, prepared descriptor, allocations and files. - Prior attempts remain non-passes: the first 23-case canary passed on an earlier head; its exact-head rerun hit 30-second database setup/cleanup limits. A read of the owned PostgreSQL server found a template clone waiting on `DataFileExtend` for 58 seconds. A broader owner run was intentionally interrupted during slow existing heartbeat setup. One diagnostic invocation used an invalid CLI option and executed no tests. A later review canary passed 21 cases and failed the new real restore contention case: cwd-only admission missed the declared project of an external worktree. Its fixture also omitted the required file-repository authorization callback and produced an unhandled rejection after the failed assertion. The repaired run passed 22 cases, exited 0 and completed cleanup. These earlier failures are not reported as passes. - The recovery owner passed 37 PostgreSQL cases, including 14 new and 23 existing finalized-recovery cases. The operation reconciliation owner passed 40 cases. The initial archive run passed 103 cases and found a missing closed diagnostic entry; the corrected diagnostic and expanded Git rejection cases pass in the P1 run. - The full server type graph passes with the Runner source exports mapped locally because this checkout has no built Runner declarations. The latest module-boundary check and `git diff --check` pass. Standard package build/typecheck and broad final-head regression coverage remain required in CI. - The initial CI exposed missing admission scopes for standalone filesystem APIs and a changed foreign-company error. The latest repair keeps those APIs independent of a database and holds physical fences at their actual database-backed mutation owners. The branch also includes the existing master fix for unchanged Runner default-model assertions. New review findings and new-head CI are being checked. - The local canary command and bounded source selection are documented in `doc/plans/2026-11-06-online-ordinary-source-fence.md`. Source observations only read data. A fresh bounded metadata probe found 254 pending legacy records and zero unsettled local-process services. No live import, hold, cancellation, provider action or source filesystem write was performed for this change. ## Risks - Run-scoped reuse now includes execution-workspace ownership in its private key. Explicit project/agent sharing keeps its previous identity. - A missing writer owner could make a source proof unsafe. Admission derives physical aliases, includes future project selection and retains all existing source/lifecycle/finalizer locks. Unknown and changed state fail closed. - Cross-company physical alias checks use bounded minimal-path pages. Lexical-prefix filtering cannot safely replace them because a symlink can hide a foreign alias. Unknown paths or more than 10,240 foreign records leave the source explicitly unproven. More than 1,000 same-company local records also fail closed. - The project key can serialize local preparation within one project. A first project waits; additional project acquisition fails on contention to avoid a lock cycle. Remote authority keeps its remote preparation path. - Recovery preservation does not prove provider termination or successful file recovery. Existing provider allocations, checkpoints, task intent and recovery actions remain authoritative. - This slice does not finish shared Git migration. Git administrative/common-object/alternates closure and exact nested imported-child coverage remain the next owned extension. No global writers-stopped shortcut is used online. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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> |
||
|
DottaandPaperclip
|
a0ac81e114 |
fix(ui): show task image results in compact thumbnail grids (#19626)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task feeds show agent messages, documents, and files in time order. > - Each image currently uses a full feed card. > - A turn with seven screenshots makes the conversation hard to scan. > - This pull request groups adjacent images from the same run in a compact grid. > - Users can scan the results and open each image in the existing gallery. ## Linked Issues or Issue Description **What existing behavior does this improve?** Image results in task history and the live task feed. **Current behavior** Each image uses a separate full-width card. Native image upload receipt messages can also separate related screenshots. **Proposed behavior** Adjacent images from one run share a wrapping thumbnail grid. New image results join the grid. Documents, video, and authored messages keep their positions. The grid opens the existing gallery. **Reason and benefit** Users can see a batch of screenshots without scrolling past one large card for each file. **Breaking changes** The feed layout changes. The stored comments, files, APIs, and permissions do not change. Related public work: Refs #17228. Refs #8785. Refs #7667. These changes supplied the artifact cards and media gallery that this pull request reuses. ## What Changed - Add one image grouping function for settled history, turn history, and live results. Group only images with the same known run. Keep the first row key and each image key when results arrive. - Add a token-based grid that reuses the media artifact cards and image gallery. - Add an optional typed attachment-preparation marker to file registration and chat-file reuse. Preserve the marker through comment normalization. Use it with matching image, author, and run links to hide generated receipts. Keep the exact historical sentence as a fallback for old receipts. - Add nine production component stories with seven sample-account screenshots. Cover mobile, Classic, mixed results, single images, missing images, streaming, gallery navigation, and the complete feed projection. - Add grouping and production-feed regressions. Add 18 browser cases across light and dark themes. - Improve contrast for the shared fallback label and gallery counter after the accessibility checks found low contrast. - Match alternate original-image paths when opening the gallery. Keep a separate Open original link beside Download in the gallery. - Record the feed layout in DESIGN.md. ## Verification - Final qualification at `09b12381ce85bee6593050bd8a2fa18cbe2c45a6`: [required CI passed](https://github.com/paperclipai/paperclip/actions/runs/42317080212/job/128237094513), including typecheck, build, all test lanes, E2E, and verify. The snapshot has 58 successful check runs, three conditional skips, one neutral advisory job, and a successful legacy Snyk status. No pending or failed checks remain. Greptile and Paperclip Review both score this exact head 5/5; all review threads are resolved. - Current-head local checks: 544 focused UI tests passed across 11 files, with one existing skip. UI/shared typechecks and token gates pass. All 18 focused Storybook browser cases passed after the CSS merge, with retries disabled. - Two database integration cases verify file registration and chat-file reuse receipt markers. All 52 issue-validator tests and six upstream default-model tests passed. - Regressions failed before the fixes for unknown-run grouping, receipt wording, and authored progress text with an image. The repair uses a typed receipt marker and requires matching author, attachment, image, and run links before hiding a generated receipt. All five review findings are resolved. - Browser checks cover loaded images, wrapping columns, overflow, missing-image fallback, streaming stability, keyboard gallery navigation, original-file links, and downloads. Fixtures use production renderers. These checks do not prove deployed persistence or provider behavior and do not compare visual baselines. - Local limits: the preflight helper could not finish because Cargo was absent. The earlier low-memory server typecheck and dependency-free selector did not replace final-head CI. The now-passing final CI supplies broad typecheck, build, server, Runner, and E2E coverage. - Prior independent CI failures remain recorded: one Slack timer case passed a single diagnosed rerun; its exact timer cause was not proved. A later chat shard lost its self-hosted runner without a test assertion failure. The current revision passes that shard and all applicable gates. Master conflict resolution retained both image-grid and patch CSS rules. ## Risks - Images without a run ID remain separate. Messages and other resource types end each batch. - Receipt suppression requires a matching preparation marker or the exact legacy sentence, plus matching attachment, image work product, run, and author. Tests preserve human messages and native progress text with attachments. The optional marker uses existing JSON metadata and needs no database migration. - Shared gallery and fallback text now have stronger contrast. ## Model Used OpenAI GPT-6, through Codex. The agent used reasoning, repository tools, code execution, and browser tests. The runtime did 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> |
||
|
DottaandPaperclip
|
72f7b42fe3 |
Preserve settled failed controller state during online migration (#19678)
## Thinking Path
> - Paperclip controls agent work across web-server releases.
> - Volume-free servers need retained controller state in shared
storage.
> - Successful settled sources already support online migration.
> - Some retained failed sources have no accepted result and must stay
available for recovery inspection.
> - This change copies those sources under task, coordinator and
publication fences while unrelated work continues.
> - The original files, failed status and recovery warnings remain
unchanged.
## Linked Issues or Issue Description
Refs #19038. Refs #19631.
**What are you trying to improve?**
Preserve retained failed controller files without waiting for global
idle.
**What problem does this solve?**
The online producer skips terminal failed controllers. Their files can
still be needed for recovery inspection even when no accepted provider
artifact proves executable continuation.
**What would the improvement look like?**
A settled exact source can enter the existing encrypted archive store.
An accepted committed failed source can use the existing checkpoint
import. Busy, changed, unowned or foreign sources remain blocked.
## What Changed
- Add a source owner bound to the selected task session, run,
coordinator generation, admitted input and file digests.
- Check the task, physical finalizer lock, released coordinator,
canonical resume source and active related turns before upload and
before publication.
- Preserve failed sources through the existing encrypted archive format.
Do not treat archived bytes as execution authority or old-writer stop
proof.
- Allow the existing accepted committed checkpoint import for a finished
failed run without changing its status.
- Validate admitted input bindings before looking up local controller
files.
- Document the online preservation path and the final static writer
fence.
## Verification
- Real PostgreSQL: 125 cases passed across migration, controller
recovery and durable backup storage. One added assertion inspected an
unadmitted queued fixture; it was corrected while retaining the
queued-turn preservation and stale-publication assertions. The final
migration rerun passes 51/51; the other three affected suites pass
75/75.
- Genuine prepatch RED: online failed-source preservation and accepted
committed failed-source import both fail on the previous producer and
recovery owners.
- New cases cover independent restore after deleting the original
source, an expired but non-null coordinator owner, source mutation after
upload, a new related queued turn during upload, foreign input binding
and an accepted committed failed result.
- The full rebased server source and test-tree typecheck passes after
restoring its missing ignored workspace SDK link and building that
existing dependency.
- Module boundaries and whitespace checks pass.
- Rebased onto current master; both provider-artifact imports and the
new archive owner are preserved. Rebased type and module-boundary checks
pass. All six archive cases pass on the exact rebased head (39.68
seconds). All CI gates pass on exact head
|
||
|
Nicky LeachandClaude Opus 5.5
|
8ed712f00a |
test(server): split workspace-recovery tests into seven feature files (#19664)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, so CI wall-clock time sets the pace of work > - PR CI runs the general server tests in 12 shards, and each shard runs files in parallel on 4 workers > - `server/src/services/workspace-recovery.test.ts` takes about 699 seconds in CI by itself. A file runs whole on one worker, so shard 2/12 takes 11.5 minutes (median) when the other shards take about 7 minutes > - A different shard count cannot fix this, because the time of one file sets the minimum time of its shard > - This pull request splits the file into seven feature files with a shared fixture > - The benefit is that the shards can balance again. This removes about 2 minutes from the median PR CI time and about 5 minutes from the p90 ## Linked Issues or Issue Description **What existing behavior does this improve?** The time of the general server test shards in PR CI. **Subsystem affected** Server tests: workspace recovery (`server/src/services/workspace-recovery*.test.ts`) and the general-server shard weights. **Current behavior** One file holds 460 tests and takes about 699 seconds in CI. Its shard is the slowest in most PR runs. **Proposed behavior** Seven files of about 60 to 140 seconds each. The shard partitioner places them on different shards. **Reason and benefit** The general server shards finish close together. The PR CI critical path becomes shorter. **Breaking changes** None. No product code changes. ## What Changed - Split `server/src/services/workspace-recovery.test.ts` into seven files by feature area: - `retry-copyback` (76 tests); - `preparation` (80); - `preparation-proof` (53); - `preparation-dispatch` (25); - `preparation-gates` (43); - `warm-handoff` (36); - `admission` (147). - Add `server/src/services/workspace-recovery.test-fixture.ts` with the shared hooks, `seed`, helpers and an embedded Postgres per file. - Test bodies, hooks and helpers are moved without change. - Four lines that assigned the shared `db`, `root` or `connectionString` now call `replaceFixtureState(...)`. - `scripts/general-server-shard-durations.json`: replace the old key with the seven new files. The weights divide the current CI cost of the old file (636 seconds, master runs 42296943853 and 42294279748) in proportion to measured local time. A `splitEstimates` entry records the method, as for earlier splits. - Update the file references in `doc/task-recovery.md` and `maps/feature-map/recovery.md`. ## Verification - `vitest list`, before and after: 136 declarations each, with identical sorted names. At run time, 460 tests before and after. - Each new file passes when run alone: 76, 80, 53, 25, 43, 36 and 147 tests. - All seven files run together with `--maxWorkers=1`: 460/460 pass. - A timing check of the original file: - `afterEach` took 4.7 seconds in total and `beforeAll` 4.1 seconds, out of about 1,000 seconds, so the teardown is not the cost; - most of the time is in three `it.each` blocks. - Focused `tsc --noEmit` on the eight new files: clean. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`: 39/39 pass. - `node scripts/check-maps.mjs`: aligned. - With the new weights, the 12-shard partition puts the seven files on seven different shards. ## Risks - The CI time of each new file is an estimate from local runs. If `workspace-recovery-preparation.test.ts` takes more than about 150 seconds in CI, the next split point is the "keeps ancestor Retry held" block. - Each file starts its own embedded Postgres, which adds a few seconds of setup per file. - The optional external-database environment variables keep their old behavior but were not tested. CI does not set them. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent made the split, and the parent session reviewed it and corrected the shard weights. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky LeachandPaperclip
|
d6f4155bad |
Refresh checkpoint writers when controller ownership changes (#19679)
## Thinking Path > - Paperclip manages AI agents and keeps their work attached to tasks. > - Core saves native Runner checkpoints so a session can continue under a later run. > - The checkpoint records can still belong to source run A after run B becomes the current owner. > - The writer cache currently keeps A's first write authority even after Core validates B. > - This pull request renews that writer when the authority, boot, generation, or lease owner changes. > - Repeated registration under the same authority keeps its work history. Stale and expired owners still cannot write. ## Linked Issues or Issue Description Refs: #19611. Related: #19225. **What happened?** A checkpoint writer is cached by source run ID. After Core validates a successor owner, a new registration still uses the cached writer for the old owner. Saving new work can then fail with `native_runner_checkpoint_owner_lost`. **Expected behavior** Use the validated current owner for later writes. Preserve the existing writer when its authority has not changed. Continue to reject stale or expired ownership. **Steps to reproduce** 1. Register a source checkpoint writer and save its first command. 2. Complete the source run and give its task to a successor with a current write lease. 3. Register the same source checkpoint under the successor and save a new command. The unchanged code rejects this write. ## What Changed - Store the write-authority fields with each cached writer. - Replace the writer only when its authority run, boot, controller generation, or lease owner changes. - Add a PostgreSQL regression for successor handover, repeated registration, generation renewal, stale writes, and expired leases. ## Verification - The new regression fails on base `913102870d` at the successor's checkpoint write. - The fixed regression passes: `cd server && node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-runner-checkpoint-authority.integration.test.ts`. - A focused strict TypeScript check covers the source, regression, and their imports. - The maintained general-server selector includes the regression. It needs no native process or new CI group. - The scoped diff passes whitespace and changed-line secret/PII checks. - Targeted database and focused strict TypeScript checks pass. Required CI will supply full regression coverage. - CI and standard review are pending for this draft. ## Risks A renewed writer starts a new in-memory work history. The authority comparison limits that reset to a real ownership change. Each database write still checks the current lease and generation in its transaction. This change adds no schema, API, or automatic session-release behavior. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), high reasoning, with repository inspection, code execution, and test tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (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 N/A: This internal correction changes no documented interface. - [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> |
||
|
Devin FoleyandPaperclip
|
2a4a001d37 |
Preserve GitHub access repair guidance for MCP callers (#19655)
Return GitHub permission denials as failed MCP tool results with trusted, connection-specific recovery guidance. Explain the personal-account and organization App installation boundary without replaying writes or bypassing provider-content checks. Align the repair steps with the Permissions UI. Cover enabled model checks and malicious provider failures across direct calls, connection tests and stored approvals. Repair the CI-exposed synthetic onboarding environment fixture without weakening cancellation assertions. Verified 311 targeted tests, six onboarding Playwright tests, server typecheck, module boundaries, full CI and Apex 5/5 with all threads resolved. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
a3eecf1fc9 |
Preserve admitted runtime skill bundles during server replacement (#19675)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs keep the exact skill files admitted for execution and recovery. > - Older inputs can name immutable local bundles without a skill version ID. > - Installed skill heads alone do not preserve those admitted bundles. > - Server replacement needs a durable source for each retained bundle. > - This pull request copies authenticated bundles through the existing package owner and restores their exact projections. > - The benefit is preservation during online migration without stopping active work or settling recovery warnings. ## Linked Issues or Issue Description Refs #19656. Related: #19660. **What happened?** Older native execution inputs can contain skill bundles with a null version ID. Recovery skips those bundles. Startup installation checks can pass even when those retained inputs still require the old filesystem. **Expected behavior** Preserve each retained admitted bundle in object storage. Restore its original bytes, modes and digests on an empty replacement host. Keep its null pin and all run, controller and recovery state. Block startup and enrollment while a concrete local asset obligation remains. **Steps to reproduce** Retain a native run with an admitted local skill bundle and a null version ID. Convert its installed skill head to durable storage. Replace the server home and attempt recovery. The old bundle path remains in the recovered context. ## What Changed - Add a bounded `runtime-assets` phase to the existing skill package maintenance CLI. It copies exact admitted files under the runtime asset publication lock, verifies the durable package, and appends a run-bound receipt after source revalidation. - Keep original files, null pins, task intent, warnings and controller leases. Support an active run through copy-only preservation. Do not start a turn or claim provider ownership. - Verify and reuse a healthy canonical local bundle before storage reads. Restore missing or damaged bundles through the existing package and native asset materializers. Use the authenticated original manifest to preserve exact read and execute modes. Accept a later contract revision only when the run binding and exact skill selection remain unchanged. - Check retained recovery obligations and current task sessions during both startup and migration eligibility. Ignore ordinary admission that has no completed execution input. - Protect receipt-backed packages from orphan collection. Serialize receipt publication and deletion on the package row, then check references in a fresh statement after the lock. - Add real PostgreSQL and filesystem regressions. Replace a delete-method proxy with an actual upload-renewal row-lock race. Publish mutation activity only after commit. ## Verification - Baseline production startup gate failed the new concrete-asset regression as expected. The fixed gate passed. The actual before-fix preservation owner also failed recovery from a healthy local bundle during a simulated storage outage. A verified local-first path fixes that regression. - Eight preservation tests passed with real PostgreSQL, streaming storage, archive extraction and native materialization. They cover cold recovery, exact modes and binary bytes, active ownership, changed sources and bindings, contract revisions, missing or corrupt receipts, and publication/collection races. - Six affected package collection/deletion controls and three existing startup controls passed. The existing versioned recovery consumer passed a sequential retry. - One fixture attempt failed on its cleanup foreign-key order; the fixture was corrected. One existing suite hit its unchanged 30-second database startup hook before test execution. The unchanged single consumer passed on retry. These failures remain recorded. - Source, changed-test and CLI TypeScript checks passed. Rebase preserved both the local instruction-copy and runtime-asset guards. Current-head changed-test checks passed. Final PostgreSQL rerun passed all eleven preservation/startup cases. Current CLI check passed. Required CI remains pending at the trusted-runner gate. - Current canonical bundled creation uses package preparation plus atomic content/head publication. Native preparation requests immutable version snapshots before delivery. Converted installations and new canonical bundled creation therefore do not recreate the historical null-version asset dependency. - Fresh Greptile review on exact head `54c7dd048c9016c1af9fc68213d5ae053ac16499` passed at 5/5 with zero unresolved threads. - Module boundaries, whitespace, and credential/internal-reference scans passed. No live source writes, provider actions or migration apply were performed. ## Risks Missing, changed or unsafe local files remain blockers. This command does not repair a previously published package whose objects later disappear or become corrupt. It also refuses malformed preservation receipts. It retains the original local files and prior evidence and keeps removal blocked. Automatic repair requires a separate canonical package-owner recovery operation; direct database edits are not an accepted recovery path. Failed publication keeps the original files and any unreferenced package for normal collection. The readiness check is explicit startup or enrollment work, not a background scan. Its cost grows with retained admitted asset obligations. Older readers must be upgraded before the old volume is removed. This change preserves asset bytes; it does not settle business recovery or qualify unrelated volume dependencies. ## Model Used OpenAI GPT-6 (Codex). The runtime did not expose a more specific backend model ID or context-window size. Used reasoning, code editing, and tool 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> |
||
|
DottaandClaude Opus 5.5
|
6d1ecfc420 |
fix(accounting): close a remote run's lost receipt once its sandboxes are terminated (#19606)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission waits while completed runs await accounting; #19497 made that a hold on new work, never a hard stop > - #19497 also closes a lost usage receipt once a local provider is verified stopped, so the hold ends > - A remote (Daytona) run has no local stop event and keeps its recorded remote process identity, so its lost receipt never closed. The company then held all new work until a receipt that can no longer arrive > - This pull request closes a remote run's lost receipt once every sandbox it used carries a provider termination receipt > - The benefit is that a restart that kills a Daytona turn no longer stops a company's new work ## Linked Issues or Issue Description Follow-up to #19497. Refs #19544 (Daytona chaos harness). **What happened?** In a Daytona restart wave, run `a261c30b` failed after native recovery was blocked. Its usage receipt stayed incomplete (`accountingReceiptReady: false`). Both of its sandbox leases were released with a `destroyed` termination receipt, but accounting stayed pending. Admission then refused new company work with "New work must wait while a native run recovers its unfinished accounting". **What did you expect?** Once every sandbox of the run is stopped or destroyed, no runner can report usage for it. The known part of its usage should settle as an unpriced charge, as it does for a verified local stop, and new work should be admitted. **Steps to reproduce** 1. Run a native task on a Daytona environment with a hard-stop budget. 2. Kill the control plane during the provider turn so the turn's receipt is lost and native recovery ends in terminal failure. 3. After the sandbox lease is released with a termination receipt, observe that the run stays `cost_accounting_pending` and new work is held. ## What Changed - `run-cost-accounting.ts`: the closed-receipt predicate gains a remote branch. A native run in terminal failure without a result (failed, cancelled, timed out or interrupted) is closed when it has at least one environment lease and every lease carries a `paperclip.remote-termination.v1` receipt bound to that lease, run, company, provider and provider lease ID, with state `stopped` or `destroyed`, after a successful release. This mirrors `hasRemoteTerminationReceipt`. One SQL predicate still serves the reconciler scan and settlement. - The local branch is unchanged: a failed run with a verified local stop event and a cleared process identity. - A sandbox that may still run (no receipt, lease still active), a receipt bound to another sandbox, or a run with no lease keeps its debt pending. - `budget-enforcement-worker.ts`: the accounting worker also wakes when environment-cleanup or retained-session-cleanup work commits. Those are the signals the lease writers already send when they release or stop a sandbox. - `doc/SPEC-implementation.md`: documents the remote closure rule. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - A failed and a cancelled remote run, with one destroyed and one stopped sandbox, settle as one unpriced charge, settle the reservation, and clear admission. These are red on master. - Debt stays pending for an active sandbox with no receipt, for one of two sandboxes without a receipt, for a receipt bound to another sandbox, and for a run with no lease. - `budget-enforcement-worker.test.ts`: the accounting worker wakes on environment-cleanup and retained-session-cleanup commits, and stops waking after `stop()`. These are red on master. - Existing suites pass locally: native-stop-accounting, budget-enforcement-scoping, heartbeat-cost-accounting, run-cost-accounting, cost-data-quality, budgets-service. Server typecheck passes. - Live Daytona restart wave: results in the comments. ## Risks - A cancelled remote run now closes once its sandboxes are terminated. Stop recovery could otherwise reattach a recorder, but a stopped or destroyed sandbox has no runner to reattach to. Local cancelled runs keep their drain. - Workspace export can restart a stopped sandbox and removes its termination receipt first. A receipt that arrives after settlement still posts through late-receipt reconciliation. - Reusable-lease chains (`resolveRemoteTerminationLease`) and recovered-cleanup stop receipts are not mirrored, so those runs stay pending. That is the conservative side. - Low query cost: the predicate runs only for pending terminal runs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests, live restart-chaos harness). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
b8d94b3ad1 |
test(server): verify status and object deadlines across cold sleep (#19347)
## Thinking Path > - Paperclip manages agents and their useful work. > - Status cards and external objects can have future refresh deadlines. > - A stopped tenant needs a durable Cloud alarm to return for those deadlines. > - A local quiet report alone does not prove that handoff or later useful work. > - Acceptance must exercise the normal producer, authorization, claims and alarm owner. > - This PR adds opt-in signed cold-owner acceptance and bounded launcher controls. ## Linked Issues or Issue Description Refs #19001 and #19191, which have merged. This PR targets master and adds acceptance for the existing status and object owners. **What happened?** The existing local coverage did not join useful status generation or built-in GitHub refresh with signed Cloud alarm persistence, physical child retirement and cold reactivation at the original deadline. **Expected behavior** Keep the feature enabled. Complete useful work through its normal owner. Persist the exact authorized alarm before stopping the child. Wake and process the same original deadline without losing identity, claims, retries or useful results. **Steps to reproduce** Use the checked-in Node24 launcher with separate owned loopback App/Cloud test databases, a compiled Cloud checkout and the pinned Node22 executable. Select one status or object bootstrap, cycle or identity case. The command validates fresh evidence and database ownership before starting its guarded worker. **Paperclip version or commit** Current candidate `ea911278b22c855d6722e43afc026bfc5c031405` normally merges fetched master `07a548073ab48f13cb93b2fdc31553a5da88a070`. All eight owned acceptance files are byte-identical to `c08e64fa857d39bdb79352e94d156db7a4dbebf3`; all unrelated paths exactly match master. The guard-argument repair and source-specific historical cycle proof are preserved. ## What Changed - Add one named opt-in case per owner/stage/diagnostic combination. Keep the shared business body and its original deadlines, assertions and cleanup. - Drive actual managed App startup, authenticated producers, persisted claims and the signed Cloud SQL alarm before physical child stop. - Retain the generated five-secret fixture profile across cold boots. Store it in private directories and retain only names, hashes and modes in evidence. - Add optional query timestamps to the existing proxy and harness. Existing callers keep their default behavior. - Add launcher refusal controls and assert its complete child argument list. Both explicit guard preloads and the exact child `NODE_OPTIONS` must match. ## Verification - The current preflight repair has causal proof: the old controls pass all nine cases with an invalid worker preload flag. The new assertions reject that same command in both success controls (two failures/seven passes). Restoring the correct command passes all nine cases. The original refusal, freshness, symlink and byte-preservation assertions remain unchanged. All 33 recorded Node PIDs and three groups close; temporary copies and private directories are removed. These controls stop at the declared spawn stub. They verify the full command contract and do not claim to execute Vitest. - The prior real named status bootstrap passed in 72.942 seconds: one selected case/six explicit skips. It exercised the actual launcher and Vitest fork, useful normal generation, current authorization, settled claims, original 900000ms interval and signed SQL alarm generation/operation-lock acknowledgement before physical child stop. Recorded processes and final SQL sessions closed; both successful databases were normally dropped. - Earlier normal status and GitHub-object cycles qualified their real 900-second and 300-second original deadlines, cold reactivation and useful second results. Their complete original failure and recovery records remain source-specific; no shortened business clock or manufactured completion is used. - On current integrated head `ea911278`, own frozen offline setup and normal shared/SDK, Runner and App declaration prerequisites passed. The status test and its imported closure pass the affected normal test-tree compiler program with original options/references. The actual named normal status bootstrap then passed: one selected case/six skips, 82.202 seconds in Vitest and 85.616 seconds in the launcher. It used the real checked-in launcher, normal App/SDK/process agent and exact compiled Cloud `389878364b42dd129835d263f91882c5774d5123`. The original 900000ms interval and 120000ms alarm lead remain unchanged; useful v1 and signed SQL alarm generation/operation-lock acknowledgement precede physical App stop. All six recorded process IDs and the runtime group were absent; independent final test-database sessions were zero, with no observer during execution. Both fresh success databases were normally dropped. This refreshes the changed consumer seams through handoff and stop; it is not another cold-wake/useful-v2/full-cycle result. Historical cycles remain tied to their measured source. - Fresh exact-head CI and complete edited bot reviews are required before merge. A whole final-tree local build/test or deployed suspension is not claimed. Local provider, process and S3 boundaries are controlled fixtures; no live GitHub provider or Railway/Neon suspension claim follows. ## Risks - These tests use real SQL and wall clocks and are opt-in. Their fixtures must stay isolated and retain failed evidence. - Stable fixture secrets must remain private and must not appear in logs. Cleanup keeps them when owner closure is uncertain. - The acceptance body stays shared to preserve measured evidence. The named registration and command controls make selection failures visible. - This PR does not change production, schema or feature enablement. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated acceptance work. OpenAI Codex, based on GPT-6, performed managing review, source integration and the preflight argument repair. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window size were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used - [x] I have checked ROADMAP.md and confirmed this does not duplicate planned core work - [x] I have searched related PRs and linked the existing foundations - [x] I have described the issue with the bug-template fields - [x] I have not referenced internal or instance-local issues or links - [x] My branch name describes the change - [x] I have run the stated focused local tests and preserved their limits - [x] I have added or updated tests - [x] I have documented the opt-in command and evidence requirements - [x] I have documented risks - [ ] All current-head CI gates are green - [ ] Current-head Greptile is 5/5 with no open findings - [x] I will address reviewer findings before merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
7c673e695f |
fix(chat): name tasks by identifier, not UUID, in tool rows and agent text (#19635)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - New users meet Paperclip through the onboarding chat with their
first agent, and through task briefs that agents write
> - A browser walk of onboarding found internal handles and
control-plane jargon in those user-facing surfaces
> - A tool row showed `Read taskId: e3ecf9bc-…`, an agent-written brief
cited `task … (id 2b330993-…)`, and a Codex agent told a new user about
"receipts", "execution policy", and a "rejected policy patch"
> - Raw UUIDs and mechanism narration make the product look broken and
give the user nothing to act on
> - This pull request makes tool rows name a task by its identifier, and
tells agents to reference identifiers and to describe outcomes, not
mechanics
> - The benefit is that a new user sees "Read <task identifier>" and
plain progress updates
## Linked Issues or Issue Description
**What happened?**
In the task chat, a collapsed tool row rendered the raw call argument
`taskId: <uuid>`. An agent-written task description included a raw task
UUID next to the task identifier. A Codex chief of staff narrated
internal mechanics (receipts, execution policies, a rejected policy
patch) to a new user in chat.
**Expected behavior**
Tool rows name the task by its identifier. Agent-written text links
tasks and documents by identifier. Agents tell the user what happened,
what it means, and the next step.
**Steps to reproduce**
1. Run onboarding with the Codex API runner and accept a single-task
proposal.
2. Watch the handoff in the chief-of-staff chat.
3. See the task lookup tool row and the narration. With the Claude API
runner and the plan path, open a child task created from the plan and
read its description.
## What Changed
- `ui/src/components/task-chat/transcript-adapter.ts`: a tool row's
target never contains a UUID argument. A `taskId`/`issueId` UUID is kept
as `taskRefId` on the tool item.
- New `ui/src/components/task-chat/task-reference.ts`:
`useTaskReferenceLabel` reads the task from the shared issue detail
query cache and returns its identifier. It returns "task" until the task
resolves or when it cannot.
- `TaskChatRunnerActivityGroup` and `TaskChatToolCard` show that
identifier as the row subject. They load the task only for rows that
have a task reference.
- `skills/paperclip/SKILL.md` (Comment Style): two new required rules.
"No raw IDs in prose" and "Describe outcomes, not mechanics".
- First-task onboarding skill: a "Talk in outcomes" rule. Non-inline
handoff inputs use an identifier link, not a raw UUID.
- Chief-of-staff persona (`AGENTS.md` for the first agent): the same
plain-language rule, so it applies in every chat.
- `skills/paperclip-converting-plans-to-tasks/SKILL.md`: task
descriptions reference other tasks and documents by identifier link,
never by raw UUID.
- No change to `skills-releases/` snapshots. They are frozen releases.
## Verification
Red on master (fix files reverted, new tests kept):
```
ui tool-row-task-reference.test.tsx
FAIL ... > falls back to a plain word while the task cannot be resolved
AssertionError: expected 'ReadtaskId: e3ecf9bc-099a-4b10-8169-9…' not to contain 'e3ecf9bc-099a-4b10-8169-9ce490a91e3d'
Tests 3 failed (3)
server user-facing-wording-guidance.test.ts
Tests 6 failed (6)
```
Green with the fix:
```
ui tool-row-task-reference.test.tsx Tests 3 passed (3)
server user-facing-wording-guidance.test.ts Tests 6 passed (6)
```
Neighbouring suites:
- `cd ui && npx vitest run src/components/task-chat
src/components/TaskChatThread src/components/transcript`: 70 files, 1448
passed, 1 skipped.
- From the repo root: `onboarding-first-task-assets`,
`paperclip-skill-utils`, `onboarding-first-task-admission`,
`hiring-operational-examples`, `cli-invocation-safety`, and the new
guidance test all pass.
- `pnpm check:token-gates`: all gates clean. The change adds no styles.
- `ui` typecheck: no errors in the changed files.
- Not run locally: `agent-skills-routes.test.ts` hit a 30 s `beforeAll`
import timeout on a cold cache. CI covers it.
## Risks
- Low risk. A row that names a task loads that task once through the
shared issue detail cache. The thread usually has it cached already. A
failed load shows "task" and does not retry.
- Rows now show other non-UUID arguments after the identifier (for
example `<identifier> · status: done`). Rows without a task reference do
not change.
- The skill text changes the guidance that agents read. They do not
change permissions, tools, or API behavior.
> 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, run in 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
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
6c7cc2c5d6 |
fix(native): do not spend a recovery attempt resuming a cleanly drained run (#19625)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs have an execution recovery budget of three provider attempts, so a run that keeps failing stops retrying > - A graceful shutdown (deploy or SIGTERM) drains native runs: it suspends them for reattachment by the next server > - When the runner did not survive, the next boot resumed the turn and charged one attempt > - Restarts that the run did not cause used up its budget, so recovery stopped and the task needed a manual Retry > - This pull request does not charge an attempt for resuming a run that a graceful drain suspended > - The benefit is that deploys and graceful restarts no longer exhaust a healthy run's recovery budget ## Linked Issues or Issue Description **What happened?** In restart-chaos testing, native runs reached `execution_recovery_budget_exhausted` (3/3) only because of server restarts. In one wave, two restarts crossed each of two runs. In a second schedule (K1, then K2 when ready, then K2 again), each drain correctly suspended the local ACPX Claude run, yet the fourth boot blocked it with `native_restart_recovery_blocked`. Every resume after a drain was classified `resume_dead_runner` and charged an attempt (`nextNativeProviderAttempt` in `native-restart-recovery.ts`, applied by the executor's execution claim). **Expected behavior** A restart that the run did not cause should not consume its recovery budget. A graceful drain records `restart_suspended` and `awaiting_runner_reattach`. The resume that follows should keep the attempt count. **Paperclip version or commit** `master` at `4ef3fa4271`. **Steps to reproduce** Start a native local run. Send the server SIGTERM three times while the run is active, with a full restart each time. The third resume is blocked with `execution_recovery_budget_exhausted`. ## What Changed - `suspendedByCleanDrain()` in `native-restart-recovery.ts` recognizes a run whose most recent recovery claim is older than the drain's `restart_suspended` entry. Ownership waits and deferred blocks between the drain and the next claim do not erase it. The next claim consumes it. - A scan that waits for the old controller to exit (`awaiting_evidence`) records one history entry per reason, so repeated scans cannot push the drain record out of the bounded history. - Restart claims carry `chargesProviderAttempt`. It is `false` for a claim that follows a clean drain. - `nextNativeProviderAttempt()` takes that flag. The executor passes it when it takes the execution lease, so a drained resume keeps the current attempt. - The first execution is always counted: a run drained at attempt 0 (no execution claimed it yet) still charges its first provider start. - A run drained during its third attempt may continue that attempt. The local resume and remote bootstrap gates skip the `>= 3` exhausted check only for a non-charging resume, and the executor still refuses a fourth attempt. - A crash restart (no drain record) still charges one attempt. Reattachment to a live runner still charges none. - `doc/execution-semantics.md` states the rule next to the three-attempt budget. ## Verification - New test in `server/src/__tests__/heartbeat-native-restart-suspension.test.ts` (embedded Postgres, real heartbeat drain and restart recovery): - Red before the fix: the claim after the drain had no `chargesProviderAttempt: false`. - Green after the fix, for drained attempts 0, 1 and 3: the crash claim charges (`true`). After the drain, a controlled backend lets the executor take its execution lease and records the saved `native_run_finalizations.attempt` at provider dispatch: 1, 1 and 3. Attempt 3 is claimed instead of blocked. - Red at the executor: when the executor ignores the flag, the drained attempt-1 resume saves 2. - Ownership-wait case: the drained controller (the test process) is still alive, so two scans wait. The history keeps `restart_suspended` followed by one `awaiting_evidence` entry. After the controller is gone, the claim does not charge, and attempt 3 continues. - Classifier cases (local resume and remote bootstrap): attempt 3 after a crash is still `execution_recovery_budget_exhausted`. Both new cases were red on the previous revision. - `heartbeat-native-restart-suspension.test.ts` 12/12, `native-runner-restart-recovery.integration.test.ts` and `native-runner-ownership.test.ts` 94 passed. - `tsc --noEmit` for `server` passes. ## Risks - A run that itself makes the server shut down gracefully, again and again, would no longer be stopped by this budget. Graceful shutdowns are operator or deploy actions, so this is unlikely. A crash loop (SIGKILL, OOM) still charges an attempt on every restart, and a resumed turn that fails still goes through the existing failure and replacement budget. - This changes only the attempt count. Ownership checks, checkpoint verification and the exhausted gate are unchanged. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
56c2b28c04 |
Settle saved monitor results after a newer user request (#19644)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native results must finish their durable finalization before later
work can reuse a saved workspace.
> - An accepted result can wait on a monitor while its workspace files
are recovered.
> - A newer user request can replace that old wait before finalization
resumes.
> - The old finalizer then reports lost monitor authority and blocks the
saved-workspace lineage.
> - This pull request proves the newer user request and settles only the
old result bookkeeping.
> - The benefit is released recovery work without replaying an old turn
or changing the current request.
## Linked Issues or Issue Description
Refs #17641 and #16975.
**What happened?**
An older successful native result has saved files and a server-issued
monitor receipt. Its task is now blocked after a newer user-comment
continuation failed during preparation. The old coordinator stays in
retryable_failure with monitor_wait_authority_lost. The retained
allocation ownership check correctly refuses the unresolved predecessor,
so later file recovery also stays held.
**Expected behavior**
The normal finalizer can settle the old accepted result when exact newer
user-request evidence proves the old wait is superseded. It must
preserve the current task, user input, ownership, schedule, accepted
result, and attempt history. It must not start provider work or publish
the old wait summary.
**Steps to reproduce**
Create an accepted successful monitor result and mark workspace
finalization successful. Leave the task blocked. Record a newer
canonical user comment and its ordinary user wake, either queued or
consumed by a same-task continuation that retains the exact comment
revision and source-run lineage. Call the real native finalizer. Before
the repair, both cases remain retryable_failure instead of settling.
**Paperclip version or commit**
|
||
|
Nicky LeachandClaude Opus 5.5
|
55244f1e9c |
test(server): remove two timing flakes in cloud adapter and native cleanup tests (#19668)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Contributors wait for PR CI before merge, and a flaky test forces a
re-run of the whole CI run
> - Four server tests failed on 2026-11-06 on commits that passed on a
later attempt
> - Two of them failed because of real slowness inside a fixed time
limit on contended 4-vCPU runners
> - This pull request removes that slowness from the two tests. It does
not raise their time limits
> - Of the other two tests, one is already fixed on master, and one is
not reproduced (see below)
> - The benefit is fewer whole-run re-runs
## Linked Issues or Issue Description
**What happened?**
These tests failed in PR CI and passed on a re-run of the same commit:
- `server/src/adapters/cloud-external-adapters.test.ts` › "Cloud image
adapter boundary › cold boots built-ins…" failed with `Test timed out in
15000ms` (3 times).
- `server/src/__tests__/retained-native-session-cleanup-work.test.ts` ›
"drains a backlog across discovery pages…" failed with `Test timed out
in 15000ms` (2 times).
**Expected behavior**
Both tests pass on a loaded CI runner.
**Steps to reproduce**
1. Run the test file in a 4-CPU container (`docker run --cpus=4
--memory=16g`) with 4 copies at the same time.
2. Before this change, the adapter case fails in 4 of 4 runs at 15,008
to 15,041 ms.
## What Changed
- `cloud-external-adapters.test.ts`: add a `beforeAll` warm-up with a
90-second limit.
- The warm-up imports `registry.js`, `plugin-loader.js` and
`adapter-plugin-store.js` with an empty temporary home and the managed
signal, so it reads and runs no persisted adapter package.
- Before this change, the first case paid the one-time transform of the
whole built-in adapter graph inside its own 15-second budget. In CI the
first case timed out at 15 seconds, and the next case took 1.1 seconds.
- Each case still gets fresh module state from the existing
`vi.resetModules()`. No assertions change.
- `retained-native-session-cleanup-work.test.ts`:
- `flush()` takes an optional poll `interval`, which defaults to 50 ms,
so other tests do not change.
- The backlog test uses 5 ms. The worker admits one cleanup pass per
1-second fake-time step, so 103 entries need 103 polls in a row.
- The next wake is scheduled before a pass releases its idle-work hold,
so a faster poll cannot see a false quiet state. No assertions change.
## Verification
| Test | Before | After |
| --- | --- | --- |
| Adapter cold boot, container, 4 copies | failed 4/4 (15,008–15,041 ms)
| passed 4/4 (537–756 ms per case; the warm-up takes about 23 s) |
| Adapter cold boot, host | failed 2/2 | passed 3/3 (0.6–1.6 s) |
| Backlog drain, container, 4 copies with load | 6.3–6.6 s | 2.0–2.1 s |
| Backlog drain, host, unloaded | 5.1 s | 1.1–1.3 s (5 runs, 29/29 pass)
|
- Final host run of both files with `--maxWorkers=2`: 33/33 pass.
- The backlog test did not reach the 15-second timeout in local runs.
The evidence for that fix is the measured poll-bound cost.
**Not changed in this PR:**
- `connection-management.integration.test.ts` (expected 3 to be 2):
- Cause: a GitHub webhook recovery worker from an earlier test fetched
through the suite-wide fetch spy 60 seconds later.
- Master already fixes this with commit
|
||
|
DottaandPaperclip
|
6be71028ad |
Prepare online ephemeral enrollment through signed Cloud control (#19660)
## Thinking Path
> - Paperclip manages agent work through a control-plane web server.
> - A replacement web server needs shared durable data and the original
signing identity.
> - Existing enrollment exports that identity only after a global drain
and offline migration.
> - Online enrollment needs a separate preparation operation while agent
work continues.
> - This change adds a purpose-bound Cloud control operation with
existing data and key owners.
> - The result is an audited preparation receipt, with no permission to
stop agents or detach storage.
## Linked Issues or Issue Description
Refs #19038. Refs #19024.
**What are you trying to improve?**
Allow Cloud to prepare an already migrated tenant for volume removal
without the offline migrator or a global-idle gate.
**What problem does this solve?**
Cloud must preserve the original decision key before starting a
replacement with static ephemeral mode. A tenant session must never gain
key-export authority.
**What would the improvement look like?**
A fresh exact-action Cloud assertion obtains a private, deployment-bound
preparation receipt. Unfinished durable-data migration and invalid
replacement configuration block export.
## What Changed
- Add the signed `ephemeral-enrollment:prepare` control action and a
strict UUID-only POST route.
- Check existing durable-data eligibility and ephemeral configuration
before reading the original key. Never create a missing key.
- Audit only public preparation metadata. Keep HTTP diagnostics
secret-sensitive and the response non-cacheable.
- Bind the receipt to the attempt, stack, build commit, Railway
deployment, time, static mode and key digest.
- Always return `enrollmentReady: false`. Cloud still needs to arm a
replacement, verify old writers have stopped, repeat eligibility and
qualify native handoff before detaching the retained volume.
## Verification
- PASS: 80 focused tests across the preparation owner, signed route,
existing Cloud control, HTTP privacy and OpenAPI route coverage.
- PASS: 2 real PostgreSQL cases. Concurrent copies of one signed request
export once; the durable receipt still rejects reuse after clearing the
process fence. Normal run admission before skill pins is allowed;
missing durable skill installation remains blocked.
- Genuine prepatch RED: the same two PostgreSQL cases exported twice and
rejected ordinary admission. The fixed owner was restored automatically
after the comparison.
- PASS: server TypeScript check with workspace dependencies built.
- Fixed the first CI route-coverage failure by declaring this private
control endpoint in the existing explicit OpenAPI exclusion registry.
- Initial local HTTP execution lacked sandbox socket permission; the
reviewed run passed. Initial PostgreSQL fixtures enabled durable files
too late and reused an issue prefix; those fixture defects were
corrected before the red/green comparison.
- All required CI gates pass on
|
||
|
DottaandClaude Opus 5.5
|
e89be6ee83 |
fix(native): re-bootstrap a runner that a graceful drain suspended during bootstrap (#19665)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs start a local `paperclip-runnerd` process. The control plane keeps a durable state root for each session. > - After a restart, recovery gives a run with no provider evidence the `bootstrap_incomplete` disposition. The executor quarantines the old root and starts the bootstrap again, but only when the root proves that no provider work started. > - #19524 made that proof accept a root killed after `run.prepare` and before `session.open` (a hard kill). > - A graceful restart (SIGTERM) leaves different shapes. The drain sends `runner.drain` and `runner.suspend`, and the kill can land just after `session.open` failed or just after it started a fresh session. The proof refused both shapes. The run failed with `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`, although no turn was ever sent. > - This pull request extends the proof to both drain shapes and keeps provider work fail-closed. > - The benefit is that a graceful restart during native bootstrap continues automatically. ## Linked Issues or Issue Description Refs #19524 **What happened?** Restart chaos waves (ACPX Claude, K2 graceful restart about 10 s after the run started) failed with `adapter_failed` and `runner_state_identity_mismatch: bootstrap_not_proven_incomplete`. Startup recovery chose `bootstrap_incomplete`, and the executor proof refused the state root. Two real roots were kept: 1. `session.open` failed (the ACPX sidecar timed out), then `runner.drain` completed and `runner.suspend` completed. There was 1 connection, no committed or acknowledged event, an empty runner outbox, and `nextSourceSeq: 1`. 2. `session.open` completed with a freshly started session (`status: "started"`). The committed events were only `session.started`, `session.capabilities.updated` and `session.goal.snapshot`, and the runner outbox held one `harness.diagnostic` (the sidecar process). No turn command existed, and the sidecar process had exited. The database had not yet recorded the session events, so the classifier saw no provider evidence. **Steps to reproduce** 1. Start an ACPX Claude task on a local runner. 2. Send SIGTERM to the control plane about 10 s after the run starts, before the first turn. 3. Let the control plane restart. **Expected behavior** Recovery starts the bootstrap again, and the task continues without a user action. **Actual behavior** The run fails and the task stops. ## What Changed - `native-bootstrap-proof.ts`, pre-session proof: a `session.open` that is pending or `failed` is now accepted, as are the runner lifecycle commands `runner.drain`, `runner.suspend` and `runner.shutdown` in any state. The controller journal and the runner journal must both agree. Any committed or acknowledged event, any turn command, a completed `session.open`, and any queued runner event still fail closed. - New `runnerdStateIsOpenedSessionBeforeTurn`. It accepts a completed `session.open` only when all of these are true: - the session was freshly started (`status: "started"`, not resumed); - the commands are only `run.prepare`, `session.open` and runner maintenance commands; - every committed event and every event in the runner outbox is a session setup or runner lifecycle event; - `ackedSourceSeq` equals the count of committed events, with no gap; - the runner journal is present; - every recorded provider process ID has exited on this host. - `native-session-executor.ts`: the executor applies the opened-session proof only for a local execution target. Remote runners stay fail-closed for that shape, because their process IDs belong to another host. The remote checkpoint classifier still reads only commands 1 and 2. ## Verification - Red before the fix: "allows a fresh bootstrap after a drain suspended a runner whose session.open failed" and "allows a fresh bootstrap after a drain that landed between session.open and the first turn" both failed with `expected false to be true`. Green after the fix: the `native incomplete-bootstrap evidence` block passes 32/32. - Fail-closed counter-tests: - completed `session.open`; - committed event, acknowledged event, or queued runner event; - turn command, `turn.stop`, or a turn processed by the runner; - `turn.started` or `provider.event`; - resumed session; - compacted events; - no recorded provider process; - missing runner journal; - live provider process; - remote runner. - I ran the new proof read-only against the three real kept roots: the K1 root from #19524, the K2 `session.open` failed root and the K2 opened-session root. All three return `true` for a local runner. The opened-session root returns `false` when the processes are on another host. - `native-session-executor.test.ts` and `native-restart-recovery.test.ts` filtered to bootstrap, restart, remote and drain: 333 passed. `native-runner-restart-recovery.integration.test.ts` and `native-bootstrap-retry.integration.test.ts`: 106 passed. `tsc --noEmit -p server/tsconfig.json` passes. - Live restart chaos replay: see the PR comment. ## Risks - After a `session.open` failure or timeout, the provider sidecar might have created an empty session. No turn command was sent, so no provider work can repeat. - An opened session before any turn is discarded, and a new one is started. The proof requires a fresh `started` session, so no earlier conversation is lost. Resumed sessions stay fail-closed. - The opened-session proof is local only. It requires every recorded provider process to be gone, so a live provider process blocks it. - Out of scope: the Retry offer when the proof fails legitimately (RC-05/06). > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, test execution). Default context window. Extended reasoning was available. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
9197abf673 |
fix(native): preserve controller state when remote archiving fails (#19663)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native continuity replacement must preserve the state needed to recover saved work. > - The current replacement moves local controller files before it knows whether the remote archive succeeded. > - A remote failure then leaves the local controller without its recovery files. > - This pull request confirms the remote archive before it moves those files. > - A user turn that never started can use the existing bounded Retry path after verified preservation and cleanup. > - The task retains its saved work and can continue once under current user authority. ## Linked Issues or Issue Description Refs #19638, #19547, #19399 and #19659. The prerequisite #19638 merged as `0819f4ea78dd37fba87341b314be9dac078c1410`. This PR now targets master. A normal dependency integration at `febed950f4` keeps only the six-file archive repair in the PR diff. Fresh master-based CI and review are required before merge. **What happened?** A native continuity replacement moved its local controller journal and provider checkpoint. The following remote archive failed. A later recovery attempt could no longer read the local recovery files. The first-attempt failure also had no normal Retry receipt for the saved user turn. **Expected behavior** A failed or timed-out remote archive retains the local recovery files and starts no replacement provider. A user turn that has not started can request one normal Retry. The existing user, provider-turn, Stop, preservation, cleanup, budget and approval gates remain in force. **Steps to reproduce** 1. Keep a local controller journal and provider checkpoint for a retained remote conversation. 2. Request continuity replacement and fail or time out the remote archive command. 3. Check the local controller files before any later recovery. 4. Settle the native failure under a current user wake and recover the saved workspace. ## What Changed - Confirm remote continuity archiving before moving the local controller state. - Add the exact remote archive failure to the shared bounded user Retry admission from #19638. - Verify failed, timed-out and successful archive ordering with real local files. - Verify ordinary failure settlement and saved-file recovery produce one fresh successor. Keep Stop, system wake and unsettled-provider holds. - Document the file preservation and Retry contract. ## Verification - RED: failed and timed-out remote archives removed the local controller files. Reading those files failed with ENOENT. The saved-user Retry request returned false instead of true. - Initial archive unit verification: eight cases passed, including the three archive outcomes and five existing Cloud archive safety cases. - Initial stacked revision: `9104ee399b`, based on #19638 at `d22d8f9d5ee3dc36550a94d55c1d8376f3b9aab2`. - Final master-based revision: `febed950f4`. All 13 affected cases passed across three Vitest files after dependency integration. They include the seven-mode real PostgreSQL completion case, saved-file successor and negative-hold cases, and real-file archive outcomes. All seven server typecheck layers passed again. Module boundary, whitespace and added-code-comment checks passed. All required CI passed on this exact head. The final snapshot has 59 successful checks, three intended skips, one neutral result, and no pending or failed checks. Current-head Greptile and the independent review are both 5/5, with no open findings. - Final local revision: `9104ee399b`, macOS ARM64, Node 26.4.0, pnpm 9.15.4 and embedded PostgreSQL. The three real-file archive outcome cases passed. The real PostgreSQL completion test passed all seven internal scenarios, including archive failure, prior provider activity, system wake, Stop before and after terminal settlement, and stale runtime promotion. Own Runner/shared/SDK builds and all seven server typecheck layers passed. Nine saved-work continuation and negative-hold cases passed. They verify exported file contents, one user-attributed fresh successor, idempotent hooks, no second automatic chain, and first-use manual Retry for held cases. - `node scripts/check-module-boundaries.mjs`, whitespace checks and the added-code-comment audit passed. - Focused commands from `server/`: Vitest with `-t 'retains the local controller checkpoint until remote continuity archiving succeeds'`; `-t 'replays saved user intent through normal Retry'`; and `-t 'continues a user turn once|does not continue automatically'`. PostgreSQL cases used a fresh local cluster and a 120-second setup hook budget. Application timeouts were unchanged. - The initial stacked head passed all test, build, typecheck and E2E jobs. Greptile and the independent review both scored it 5/5 with no findings. These results do not replace the fresh master-based qualification now running. ## Risks - A remote command can fail after it changed remote state. This change retains local evidence in that case. It does not claim a distributed atomic transaction. - The exact archive error can request one normal Retry only under the existing saved-user admission. The normal path must prove stop, saved files and cleanup before a successor starts. - The protected launch-argument rejection remains enforced. Its exact hosted argument difference was not available through ordinary access. #19659 separately addresses a saved-thread feature-policy argument difference. This PR preserves local archive evidence and does not claim direct hosted verification of that primary cause. ## Model Used OpenAI GPT-6 family through Codex, with reasoning, tool use and code execution. The model is inherited; no override was used. The exact serving model ID and context-window size are not exposed by this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
1c29992171 |
Observe workspace service failures before asynchronous startup work (#19624)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Workspace runtime services start local child processes and wait for
readiness.
> - The runtime must return startup failures through its normal error
and cleanup path.
> - A child can fail while asynchronous log or registry work is pending.
> - The previous code attached child listeners too late, or rejected
promises before the readiness race observed them.
> - This pull request records the child failure immediately and delivers
it when the readiness race can observe it.
> - The benefit is a preserved startup error without a process-wide
unhandled rejection.
## Linked Issues or Issue Description
**What happened?**
A real child that exits during a pending registry write produces an
unhandled rejection. A spawn error during a pending log close loses the
original error and reaches the readiness timeout instead. This caused
the General server 8/12 CI shard for #19576 to fail after all 2,285 test
assertions passed.
**Expected behavior**
The startup operation returns the original child failure, removes its
registry record, and produces no global unhandled rejection.
**Steps to reproduce**
Run the new readiness test against the base revision. It starts a real
exit-1 shell while holding the registry write through child close. It
also starts a missing shell while holding log close through the spawn
error. The two behavioral checks fail before the production change.
**Paperclip version or commit**
|
||
|
|
c82211251b |
test(chat): cover Slack forwarding deadlines through registered owners (#19348)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People answer agent questions through Slack commands, buttons, and forms. > - Slack requires a response within three seconds. A modal trigger also expires after three seconds and can be used once. > - Durable event delivery does not prove these synchronous response paths. > - This pull request exercises the registered app owners and pinned SDK with original signed requests and controlled forwarding delays. > - The tests record deadline failures without clearing invalid forms, extending triggers, or changing production behavior. ## Linked Issues or Issue Description Refs #19019, which has merged. This PR now targets `master`. Related work: #8300 handles raw-body capture and plugin event fan-out. #19328 handles Slack connection onboarding. This PR changes neither path. I searched related open and closed work and read those PR bodies and changed paths. **What happened?** A forwarding delay can leave an authenticated Slack callback too late to acknowledge a command, exchange a modal trigger, or return field errors. The existing direct modal test did not cover the normal registered ingress or these provider deadlines. **Expected behavior** Tests must distinguish useful app persistence from a response that Slack can still accept. They must preserve current authorization, field validation, original signatures, and exactly-once effects. **Steps to reproduce** Use an owned PostgreSQL test database and the repository launcher below. Run warm first. The default run adds one real 3500ms delay independently before command, form-open action, and invalid submission forwarding. **Paperclip version or commit** Production remains byte-identical to `54fa701d41b4ced62f4ff338d39b946015670ae3`. This test-only candidate is `a3cce4e9868fd3e6e25d9df7d0090899bdf6a204`. It adds a recovery-fixture setup guard after `dac0010649c435f673172c496b3383e99780e8bf`. **Deployment mode** Source-checkout integration test with real PostgreSQL, registered app owners, and the pinned Slack SDK. Provider API responses and the heartbeat receipt helper are controlled fixtures. No Slack account, model process, deployment, or physical tenant suspension was tested. ## What Changed - Route original signed command, action, and modal requests through the normal chat service and real SDK runtime. - Check forged-signature refusal, callback evidence, current-role refusal, invalid-field errors, one corrected answer, and duplicate fencing. - Intercept the pinned SDK HTTP transport. Model trigger expiry and one-use exchange without changing request timestamps or signatures. - Observe the real platform exception through the actual bound `views.open` callable. Delegate once and rethrow the same error. - Restore the network guard after every setup failure. Attempt all owned cleanup and preserve the original error before any cleanup errors. - Guard auxiliary database cleanup when recovery test setup fails before it creates a client. Keep temporary-cluster cleanup reachable and preserve the original setup error. - Add a repository launcher with the current prewire guard, one worker, explicit test-database validation, and closed warm-only and action-delayed-only selectors. The default still selects all four cases. ## Verification - Current candidate `c6ae7957988e220ea096ab678f070d3bd1c5f9c0` normally merges fetched master `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`. All three owned protocol/recovery acceptance files are byte-identical to the preceding head. All other paths exactly match master. The shared Runner default test conflict uses master's independently landed constant-based repair. That supersedes this branch's temporary two-literal repair; no fourth file remains in the PR diff. - Original CI server4/12 had two stale `gpt-5.6-sol` expectations after the normal default changed to `gpt-6.1-sol`. The complete failure log is retained. Before the master repair landed, the focused two-literal correction passed the whole six-case file with explicit-model, non-Codex, lifecycle, permission and skill checks intact. That local result is source-specific; it is not a fresh full merged-tree pass. - The original guard-leak counterfactual reproduced unrestored global network functions after setup rejection. The repaired callback passed four controlled cleanup diagnostics. Cleanup attempts each initialized owner and restoration even after a failure. It preserves the primary error first and retains separate cleanup errors. - A fresh registered SQL/SDK warm case passed on the qualified protocol source with one persisted answer, 17.15 seconds total and normal original assertions. Final independent SQL and recorded PID/group audits were empty. A diagnostic sampler initially hit EPERM, and the worker exit guard line was absent; those limits remain explicit. The cleanup diagnostics are local qualification tools, not four newly checked-in CI tests. - Delayed modes remain explicit diagnostic commands. Ordinary CI runs warm. Earlier mode results do not establish a final-source four-case single-command pass. The provider transport is controlled; no live Slack display, deployed sleep or wake result is claimed. - The original server6/12 cleanup-backlog case hit its 15-second whole-case timeout. Its phase and cause remain unknown. The original E2E shard received runner shutdown without an assertion verdict. Complete original logs are retained; neither failure has been labeled a flake or repaired by extending a deadline. - Own offline dependency setup passed with an unchanged lockfile. An omitted private guard dependency initially prevented test entry; that setup failure is retained separately from product results. The immutable helper closure corrected it. No local full-repository typecheck/build/test pass is claimed. Fresh exact-head CI and complete edited bot reviews are required before merge. ## Risks This is a test-only change. It changes no production code, credential flow, schema, API, or sleep eligibility. The existing large integration fixture remains large; this PR does not cross a file from below 1000 lines to above it and adds no code comments. The provider transport is synthetic. The tests demonstrate deadline consequences and app persistence, not that Slack displayed a late response. They do not make commands or open forms safe while the tenant sleeps. Other Slack lanes and persistent transports remain open work. CI may expose additional harness assumptions. The roadmap's Apps and Connections work is unchanged. This focused regression complements the existing integration; it adds no separate feature or provider setup path. ## Model Used OpenAI, configured model `gpt-6.1-sol` in Codex. The exact served backend revision and context window are not exposed. Assistance used reasoning, source inspection, Git, native TypeScript checks, and controlled integration tests. A coordinating GPT-6-family agent reviewed boundaries and evidence. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [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: Codex <noreply@openai.com> |
||
|
DottaandPaperclip
|
0819f4ea78 |
fix(native): verify controller ownership before retrying saved turns (#19638)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native task turns can keep a provider conversation from an earlier run. > - A copied conversation checkpoint names the new run before its controller has attached. > - A failed attachment can make recovery treat that copied identity as current execution authority. > - The current run needs its own durable controller checkpoint, or a valid staged attachment, before exact resume. > - This pull request checks that ownership and uses the normal Retry path to continue a saved user turn after verified cleanup. > - The task can continue once with its saved work and history. ## Linked Issues or Issue Description Refs #19399 and #18860. **What happened?** A new user comment resumed a completed native conversation. The retained runner process was gone before the new controller attached. The new run still had a copied provider session ID. Recovery selected exact resume for that new run and then failed its unchanged owner and lease checks. Ordinary first-attempt failure settlement also omitted the saved-user Retry handoff that restart recovery already uses. **Expected behavior** Exact resume requires current controller evidence. A saved user turn can request one normal Retry after this infrastructure failure. The normal path must verify provider stop, saved files, current user authority and all execution gates before it starts a fresh session. **Steps to reproduce** 1. Finish a native warm run and retain its provider checkpoint. 2. Admit a user comment as a new run and copy the provider checkpoint through the real resume producer. 3. Stop the retained runner before attachment. The real remote process check reports that the process is not running. 4. Observe that the copied provider ID selects exact current-run resume. Observe that ordinary failure settlement saves no Retry request. ## What Changed - Check a bounded encrypted controller header for fenced remote Runner recovery. Keep local Runner recovery on its existing disk-backed path. Match the run, conversation and runner identity. Preserve the existing staged-attachment path. - Share the existing saved-user Retry admission between restart recovery and ordinary terminal native settlement. Keep its exact infrastructure errors, user wake, comment, provider-turn and retry-chain checks. - Add real PostgreSQL and stopped-process regressions. Verify malformed and foreign checkpoint holds, current-owner resume, user authority and Stop. Verify saved-file recovery creates one fresh-session successor. - Document the ownership and Retry contract. ## Verification - RED: the real checkpoint rebind producer and stopped-process check selected exact current-run resume without a current controller checkpoint. - RED: ordinary settlement created zero Retry receipts for the real native failure code; expected one. - Review RED: a real current local controller file existed with no database checkpoint rows. The initial gate returned ambiguous state. The fix takes a required trusted execution-target kind and applies the database requirement only to the remote persistence path. - Initial local revision: `3662055880ccf8e09c3c50a340897e16c78627e7`, macOS ARM64, Node 26.4.0, pnpm 9.15.4, embedded PostgreSQL. - Initial server Vitest: 42 selected cases passed across controller recovery integration, remote restart adoption, ordinary run completion and retained workspace recovery. The cases cover the stopped-process producer, current/foreign/malformed ownership, automatic user continuation, Stop, unsettled turns, system wakes and a manual Retry on first use. - `pnpm --filter @paperclipai/server typecheck`: passed with this worktree's Runner, shared and SDK builds and all seven server layers. - `node scripts/check-module-boundaries.mjs`, whitespace checks and the added-code-comment audit: passed. - Review correction: `8c795e2e0e44b5209da3e228593c5cd35e948cae`. The real local/remote controller case passed against a fresh PostgreSQL cluster. It verifies local disk recovery without database rows, remote current-owner resume, and remote missing/foreign/malformed holds. Fresh `node scripts/typecheck-layers.mjs` passed all seven server layers. The existing offline chat reader now requests metadata only and retains its full independent native-state safety check. - A broader local recheck hit PostgreSQL setup/cleanup hook timeouts. These are retained as failed setup attempts; the fresh-cluster regression above passed. Required CI supplies broad final-head coverage. - Settlement-order correction: `dafb721c3bbb7ae86fbc3c9c6c953ace29ca088b`. The real PostgreSQL regression first proved both failures: Stop after an already-failed row created one Retry instead of zero; a DB-native promotion with an old legacy caller snapshot created zero instead of one. All six settlement scenarios now pass. The hook requires the local failed outcome and the canonical settled native row. Fresh seven-layer server typecheck passed again. - Required shared fixture base integrated at `d22d8f9d5ee3dc36550a94d55c1d8376f3b9aab2`. The six runner-skills fixture cases and all seven server typecheck layers passed. Required CI passed for this head: [workflow](https://github.com/paperclipai/paperclip/actions/runs/42309419592), including all server layers, tests, build, Runner verification and E2E. The final snapshot has 57 successful checks, three intended skips and one neutral result. Current-head Greptile is 5/5; both concrete review threads are resolved. The independent re-review ended without a final response; its prior Stop finding has a real RED/GREEN repair. The focused command was run from `server/`: ```sh ../node_modules/.bin/vitest run --config vitest.config.ts src/services/heartbeat/run-completion.test.ts src/services/native-runtime/native-controller-recovery.integration.test.ts src/services/native-runtime/native-session-executor.test.ts src/services/workspace-recovery.test.ts -t 'replays saved user intent|does not assign current-run resume authority|remote controller restart adoption|continues a user turn once|does not continue automatically' ``` ## Risks - A terminal failure of a fenced Runner adds one exact-run header read. The read selects one encrypted header and limits inspection to 1 MiB. Invalid or missing ownership evidence stays held. - Saved user input can request one automatic Retry. It uses normal preservation, current authorization, budget, approval, Stop and cleanup gates. Unsettled provider turns and system wakes do not start an automatic successor. - This PR changes no provider process, credential, model or lease authority checks. ## Model Used OpenAI GPT-6 family through Codex, with reasoning, tool use and code execution. The model is inherited; no override was used. The exact serving model ID and context-window size are not exposed by this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
bb621880e6 |
fix(storage): preserve agent and skill files during online conversion (#19656)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent files and installed skills need durable storage before a legacy volume can be removed. > - Their maintenance commands currently require every writer to stop. > - Normal file edits already use agent row locks and skill publication locks. > - Shared agent heads also remain durable while their agents run. > - This pull request uses those owners to copy settled files online and keeps unresolved recovery states intact. > - The benefit is useful migration progress while unrelated work continues. ## Linked Issues or Issue Description Related: #19642. **What happened?** Agent-file adoption and skill installation conversion reject apply requests without a global stopped-writer acknowledgement. Agent-file maintenance also reports an active agent as blocked when its shared head is already durable. Live enrollment omits the existing unresolved-local-instruction-copy guard. **Expected behavior** Copy settled agent files and skill installations under their existing writer locks. Keep old skill directories, versions, and inline bytes. Keep unresolved local copies and unknown historical pin obligations visible to their owners. **Steps to reproduce** Run agent-file maintenance with `--apply` on the original host with durable storage enabled. A managed agent without active execution is rejected without `--writers-stopped`. The same gate rejects skill installation conversion. An agent that already has a shared head is also reported as blocked while it runs. ## What Changed - Adopt settled agent files under the existing agent row lock. Recognize an existing shared head before checking active execution, without rewriting its projection. - Reuse the local working-copy preservation predicate for first adoption and live enrollment. Keep its original public export. - Prepare skill uploads outside the shared run-pinning lock. Verify the exact local tree and source identity under the cache, slug, and row publication locks. Publish catalog origin history in that same transaction. - Retain old skill directories, versions, and inline bytes. Keep pin retention and volume-removal guards unchanged. - Add PostgreSQL and filesystem coverage for active durable heads, unresolved copies, concurrent source edits, and linked files. Update the maintenance docs. ## Verification - Agent-file durability and live enrollment suites: 16 tests passed with real PostgreSQL and filesystem fixtures. - Managed skill suite: 22 tests passed. Seven final controls also passed: upload-time file edits, source relocation, unknown historical pins, hard-linked files, pinning during a blocked upload, and both rejected/successful catalog-origin publication. - Server source typecheck and focused changed-test typecheck passed. - Module boundaries, diff whitespace, and added-line credential/internal-reference scans passed. - Baseline production rejected both online paths at their stopped-writer guards. Review regressions also reproduced a busy pinning lock during upload and extra catalog history after rejection; their fixes passed real PostgreSQL controls. All expected red results are preserved. - One final-control attempt failed the existing 30-second fixture startup hook before executing tests. A sequential retry passed all four controls without changing guards or timeouts; the failed attempt remains recorded. - Exact head `001a5beed9d0e55c7c178906fbad1f9a47f87788`: [required CI passed](https://github.com/paperclipai/paperclip/actions/runs/42309446426), including typecheck, build, tests, Runner and end-to-end gates. Greptile completed successfully at 5/5 with zero unresolved review threads; GitHub reports mergeable/clean. ## Risks Changed or unsafe sources remain blocked. Storage failures keep the original files and recovery state. This conversion does not settle retained provider copies or historical execution. Unknown pin obligations still block retention and volume removal; this PR alone does not qualify removal of the original volume. ## Model Used OpenAI GPT-6 (Codex). The runtime did not expose a more specific backend model ID or context-window size. Used reasoning, code editing, and tool 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> |
||
|
DottaandPaperclip
|
bee0f0e2fa |
fix(server): recognize recoverable result finalization during upgrades (#19651)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native execution saves an accepted result before it finishes
workspace export and status projection.
> - The upgrade report currently recognizes only a live execution in the
observed phase.
> - A durable accepted result can therefore block an upgrade after its
execution lease is released.
> - Existing finalization owners already resume that saved work with
task binding, physical worker stop proof, and an atomic owner claim.
> - This pull request lets the report inspect those existing contracts
without taking authority.
> - Qualified remote work can continue through a normal server
replacement.
## Linked Issues or Issue Description
Refs #18037 and #19596.
**What happened?**
An accepted remote native result remains in result acceptance, workspace
finalization, retry, or committed projection. The upgrade report marks
it blocking because it is no longer a result-less observed execution
with a live execution lease.
**Expected behavior**
Report a saved result as recoverable only when the existing replacement
owners can resume it. Preserve all task, result, contract, remote
workspace, provider lease, physical owner, decision, and workspace
barrier checks. Keep unsupported or unknown work blocking.
**Steps to reproduce**
Run the new PostgreSQL post-result readiness fixture. It persists a real
encrypted Runner header and an accepted terminal result with a bound
remote workspace. The unchanged report returns one blocker.
**Paperclip version or commit**
Regression base:
|
||
|
DottaandPaperclip
|
374a918d67 |
fix(runner): retain qualified OpenCode artifacts for cold recovery (#19622)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native recovery must keep admitted provider state and useful workspace files. > - OpenCode binds its saved session to exact executable paths and hashes. > - A replacement image can remove those files even when session data is saved. > - Retaining a manifest alone cannot restore the missing launch files. > - This pull request saves the qualified files before provider work and restores the same owned paths. > - The benefit is cold continuity without relaxing the Runner guard or replaying a prior turn. ## Linked Issues or Issue Description Refs #19558. That fix retains the selected pack for ordinary continuation. This change adds durable file retention for a cold allocation. Related #19487 adjusts the admission budget; it does not retain launch files. A search of open and closed OpenCode cold/durable artifact PRs found no duplicate. **What happened?** A saved OpenCode session could outlive the image that supplied its Node executable, proxy, and native executable. A new image supplied different bytes. The Runner then correctly refused the changed launch profile. The saved harness did not contain the old files. **Expected behavior** Core must save qualified launch files before the first turn. It must restore those exact files under the same owned account and absolute paths. Unknown ownership, missing files, and changed hashes must still stop recovery. **Steps to reproduce** Start an admitted remote OpenCode session. Save its workspace, encrypted checkpoint, and harness data. Destroy the original allocation and controller files. Restore into a new allocation with an image whose qualified proxy has different bytes. Resume the same session. The new opt-in local qualification command automates this path with actual OpenCode and a local synthetic model. ## What Changed - Store company-bound, encrypted launch objects and canonical run/backup references. Generate migration 0438 through the normal database workflow. - Stage qualified Node, proxy, and native OpenCode files under a private content-addressed OS-account home. Record the account UID and exact launch paths. Preserve the Rust profile guard. - Gate first provider work on durable capture under the current native claim. Bound Stop/deadline waits while the actual late transfer keeps its worker and scratch ownership until settlement. Restore cold caches with verified staging and atomic rename. Keep live reattachment and historical image-rooted sessions on their existing paths. - Reuse bounded canonical expiration owners. Atomically fence old checkpoint authority with a strict company/run-scoped retirement marker before external deletion. Keep its indexed cleanup anchor across failed deletes and restart. Preserve earlier resume handles and reject later header or source-consumer writes. Add no idle scan or network call inside a transaction. - Add storage, race, executor, filesystem, and actual Rust resume regressions. Add the opt-in local Linux qualification command and operating limits in `doc/opencode-cold-recovery.md`. ## Verification - RED before repair: real PostgreSQL/AES cold restore lacked the launch files; normal executor preparation chose the new image pack. Separate regressions exposed lost upload receipts, missing cache repair, last-owner retirement, an account-UID mismatch, and a resume-handle race during external deletion. Each expected failure was captured before its repair. - Local qualification before the final master integration: 1,169 executor tests, 233 storage/archive/admission/expiration tests, all seven server typecheck layers, migration safety, and the actual Rust cold-resume test passed. - The maintained Linux command passed with two cached immutable Paperclip images whose proxy hashes differ. It removed the original account/controller trees, restored the production published cwd under a new lease, resumed the same actual OpenCode session, kept a useful file and prior model context, and made one new synthetic model request. No external model call or network was available in either container. - The object-store fixture uses real encrypted StorageService streams with an independent owned local directory. It is not hosted S3 acceptance or a complete task HTTP/actor workflow. Historical tasks still require normal effects reconciliation and recovery after deployment. - Review regressions reproduced partial-cache publication, resume-versus-deletion, fresh-writer and source-consumer resurrection, and unbounded Stop/deadline waits before each repair. They now pass through real filesystem, encrypted storage, PostgreSQL, and SDK boundaries. File validation is split from database ownership (384/831 source lines). - Clean runtime head `aadab7acc69ff959678030db46ffe2dcfbefe28f`: the maintained actual OpenCode cold qualification passed with `dirty:false`. It preserved the provider session and recorded launch paths, restored useful workspace content and prior model context, and made exactly one resumed synthetic model request. The 314,560,000-byte launch capture took 23.118 seconds; the later harness was 833,536 bytes. Subsequent repairs change backup-reference retirement and governed continuity-cache classification; capture, restore, launch, and published-workspace inputs are unchanged, so that runtime receipt remains scoped to those paths. The continuity-break consumer is verified separately. - Retention repair `c14d386cecf7123257918ba6a8d47307df2da397`: 68/68 affected real PostgreSQL/AES storage tests and 15/15 focused executor authority/admission cases pass. All seven server typecheck layers pass (source layers and unchanged test shards, with the final 590-file test layer repeated after fixture edits). All review threads have evidence-backed resolutions. Its subsequent CI caught the separate lifecycle-classification gap described below. The refreshed Paperclip review reported paused work, not a completed new review. - Final head `4d1cd3e3309e328b11abc85adc6d78c6d0c7f55d`: all 19 focused root-entry/executor cases and all three source declaration layers pass. [Greptile reviewed this exact head at 5/5](https://github.com/paperclipai/paperclip/pull/19622#issuecomment-6695984874). Final-head [CI verification](https://github.com/paperclipai/paperclip/actions/runs/42308908167/job/128214100339) passes: 60 successful checks, five conditional skips, and the explicitly neutral paused Paperclip review. No check is pending or failing. - CI also caught the launch cache missing from the session-root lifecycle contract. Both the writer scan and a real governed-replacement consumer failed locally before repair: old launch bytes remained in the active root. The cache now archives with the prior session state; the consumer verifies exact bytes are preserved in the archive and absent from the active root. - The latest review found that backup retirement could delete an independent run pin while a selected session retained the checkpoint. A real PostgreSQL/AES regression failed before the repair. Backup-only ownership now uses a NULL-safe exact backup predicate; the run pin belongs to fenced checkpoint expiration. The regression verifies checkpoint decryption and launch-file restoration after pending-backup cleanup, followed by deletion only after removing the final resume handle. - An executor first-session case checks new-session durable capture through the real capture helper and receipt propagation into the actual harness manifest at the backup boundary. Real storage cases verify the run-profile receipt and canonical backup/run pins. - Sequential closure reuse and simultaneous first admission are different scopes. A deterministic two-uploader test measures both independent uploads, one verified ready winner, loser-only deletion, and usable restoration for both runs. Each capture is bounded to 512 MiB and five minutes; up to three pending receipts per run are retained. There is no claim of global single-flight capture or a company-wide egress bound. - Earlier optional warm-ownership verification encountered missing local binaries. After exact-source setup, all nine ownership assertions passed; two other suites encountered local PostgreSQL setup/teardown contention, so those interrupted attempts are not passes. Required current-head CI supplies the broad final coverage. No deployment or hosted historical recovery is claimed. ## Risks - This adds two tables and a forward migration. The file object is bounded to 512 MiB. Capture adds one initial transfer; later harness archives exclude those large files. Company-scoped ready objects are shared and retained until the last canonical owner retires. Concurrent initial admissions can duplicate bounded transfers; each keeps independent ownership and cancellation rather than waiting on another controller. A continuity break retains the controller cache with its prior state until normal session-root cleanup, adding up to one bounded launch closure per retired state. - Cold continuity requires the same qualified Linux platform, supported runtime dependencies, owned account UID/home, and published absolute cwd. A mismatch fails closed. No alias or Rust guard change is included. - Existing image-rooted profiles have no retained file object. This change cannot backfill missing historical binaries. A fresh governed session must reconcile prior effects first. - Live reattachment does not read artifact objects. Storage or provenance failures before a new turn hold admission. They do not grant new execution, model, tool, or replay authority. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools, code execution, and local browser tools. The exact served model ID and context window are not exposed in this session. No claim is made for unavailable model metadata. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
a3f80ea043 |
fix(onboarding): close the onboarding task when its interview moves to chat (#19643)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - New users start in an onboarding task. The first agent can run the onboarding interview there or in its chat. > - #19512 moves the interview card to the chat when the agent asks it there. The task's card expires. > - The onboarding task then stays `in_review` with no card, run, wake or reviewer. Review attention calls it "stalled". > - Needs you then shows "Choose how this task continues" with Send back to work / Request changes / Mark task done. A new user cannot know what to pick. The card stays after the interview, plan approval and handoff finish in chat. > - This pull request closes the onboarding task when its interview moves to chat, and links the chat in a system comment. > - The benefit is that the user never decides what happens to an internal task. ## Linked Issues or Issue Description Refs #19512 **What happened?** In a browser walk (Codex API path), the user chose "Interview me" in the onboarding task. Then the user asked in Chat: "Can we do my onboarding interview here in chat instead of in the task?". The chat card worked. The onboarding task stayed `in_review`, and Needs you showed "Choose how this task continues" for it until the end of onboarding. **What did you expect?** The onboarding task follows the conversation. It closes with a link to the chat. No decision about an internal task goes to the user. **Steps to reproduce** 1. Start onboarding. Choose "Interview me" on the onboarding task. 2. Open Chat with the agent. Ask to do the interview in chat. 3. Open Inbox. See "Choose how this task continues" for the onboarding task. ## What Changed - `server/src/services/issue-thread-interactions.ts`: after the chat question supersedes the onboarding task's interview cards, the onboarding task is closed (`in_review` → `done`) when no other card is pending on it. A system comment links to the chat. The change writes an `issue.updated` activity entry (`source: "onboarding_moved_to_chat"`). Activity publications and issue post-commit actions run after commit. - The task stays open if it is not `in_review` or if another card is still pending (for example a card for a teammate). - `first-task` skill: tells the agent that the task closes when onboarding moves to chat. The chat then replaces the task for the rest of the workflow (plan, proposal cards, child tasks, results). The agent skips the steps that keep the task `in_review` or make it wait for a child. - `onboarding-assets/first-task/README.md`: documents the behavior. - Tests in `server/src/__tests__/coordinator-handoff.test.ts`: the new red/green case, plus a case that keeps the task open when another card is pending. ## Verification Red on master (fix reverted, same test): ``` FAIL src/__tests__/coordinator-handoff.test.ts > Coordinator canonical handoffs > resolves the onboarding task when its interview moves to the agent chat AssertionError: expected [ 'stalled' ] to deeply equal [] - [] + [ + "stalled", + ] ❯ src/__tests__/coordinator-handoff.test.ts:257:40 256| // No "Choose how this task continues" stalled-review card for the… 257| expect(await stalledReviewCards()).toEqual([]); Tests 1 failed | 3 passed | 24 skipped (28) ``` The test reads the real Needs you feed (`attentionService.list`). On master it holds a `review` item with `reviewAttentionState: "stalled"` for the onboarding task. That is the "Choose how this task continues" card. Green with the fix: ``` cd server && npx vitest run src/__tests__/coordinator-handoff.test.ts Tests 28 passed (28) ``` Neighbour suites: ``` npx vitest run src/__tests__/onboarding-first-task-admission.test.ts src/__tests__/issue-onboarding-first-task-routes.test.ts \ src/__tests__/issue-thread-interaction-routes.test.ts src/__tests__/issue-thread-interactions-service.test.ts \ src/__tests__/issue-thread-interactions-telemetry.test.ts src/__tests__/coordinator-handoff.test.ts Test Files 6 passed (6) Tests 373 passed (373) ``` - `server` typecheck (`tsc --noEmit`): pass. - `pnpm verify` exits 2 (the README is a broad input). Required CI supplies broad coverage. - Not run: a live browser walk. The Vitest case drives the same service path the chat uses. ## Risks - Low risk. The change runs only when an agent asks a question in its own chat and that question supersedes a pending onboarding-task question (the #19512 path). - The onboarding task becomes `done` without a recorded first-task decision. Later onboarding happens in chat, so nothing waits on this task. A done onboarding parent still accepts child-completion report wakes. - The onboarding task rows are locked before their question rows, the same order question creation on the task uses (Greptile review follow-up). The chat row is locked first; no path locks an onboarding task and then a chat row. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
349b8b3f7b |
Migrate settled controller checkpoints while unrelated work runs (#19631)
## Thinking Path > - Paperclip manages work performed by AI agents. > - Server replacement must preserve retained Runner sessions. > - Older sessions can retain controller checkpoints on the server volume. > - The existing importer requires all execution writers to stop, including unrelated runs. > - This change adds a separate online path for successful, settled sources. > - It locks the source task and finalizer while it reads and publishes that source. > - Unrelated work continues. The offline importer keeps its existing authority. ## Linked Issues or Issue Description **What happened?** A retained controller checkpoint cannot be converted by the existing maintenance importer while another task runs. Requiring a busy server to become globally idle prevents online migration. **Expected behavior** An operator can convert a successful retained source after its finalizer releases ownership, while unrelated tasks continue. An expired owner remains an owner. Related active work and uncertain evidence remain blocked. **Steps to reproduce** 1. Retain a valid legacy controller for a successful run with an accepted result and a released committed finalizer. 2. Start a different task. 3. Run controller preparation. The old importer refuses because execution writers are active. Related preservation contracts: #19038 and #19024. This change does not replace workspace migration or authorize volume removal. ## What Changed - Add `prestageNativeControllers` with task, finalizer, and source-run row locks. Recheck related runs and exact checkpoint identity under those locks. - Allow legacy checkpoint material through normal settled-owner import checks. Keep offline authority separate. - Compare captured local controller and supervisor bytes before commit. A changed source rolls back publication. - Add `--controllers-only` to the existing finite native preparation entry. This avoids re-reading provider archives during controller conversion. - Add PostgreSQL cases for unrelated work, expired owners, related active work, source-lock contention, and source changes. - Document the finite command and its limits. ## Verification - Real PostgreSQL affected suites: 56 passed across controller migration and recovery. A first per-server run was interrupted because it was slow; the completed run used the repository's shared PostgreSQL test wrapper. - Initial online cases: 3 passed, covering unrelated work, an expired owner, and related active work. - Source lock and changed-file cases: 2 passed. Publication rolled back on a source change. - Final source-evidence regression: 2 passed. A first unproven source deferred and a later valid source imported; changed-file rollback remained intact. - Server TypeScript check passed with the SDK and Runner declarations built. - Server package build passed, including the Runner vendor checks and compiled command. - Runtime bundling suite: 10 passed. - Full repository CI and exact-head automated review remain required before merge. This draft is not staging acceptance. - PASS: 7 CLI cases after adding the controller-only dependency fixture and explicit-mode coverage. The first CI run exposed missing mock/export coverage; the fixture repair passed after rebase. - The unrelated model-default failures were fixed on master; this branch now includes that fix. ## Risks - A source task can wait briefly on its own migration lock. A one-second lock acquisition budget defers a contended source. No global admission hold is acquired. - Only successful sources with a released committed finalizer are eligible. Failed or unsettled sources stay deferred and require their preservation owners. - The operator must run the command on the host that owns the original files. The command does not certify enrollment readiness or detach any volume. - The existing offline path remains unchanged and still requires stopped writers. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, tool use, and code execution. The exact serving 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
0c904e2fd5 |
fix(tasks): treat a queued-message Interrupt as a hand-off, not a failed attempt (#19633)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A user can send a message while an agent works. The message waits in
a queue strip above the composer. The strip has an **Interrupt** button
that stops the current turn so the message starts now.
> - In a browser walk of onboarding, Interrupt showed a red "Couldn't
finish this attempt. Your message is saved." banner with Retry. The
thread also showed two rows for the same stopped run ("Stopped for 22s"
and "Stopped after 21s"). After about 20 s it recovered by itself.
> - A stop that the user asked for is not a failure. The red banner and
Retry make a normal action look broken.
> - #19369 stopped resource waits from showing as failed attempts. This
is the same banner, reached through the Interrupt path.
> - This pull request shows the Interrupt hand-off as working, and
renders the stopped turn once.
> - The benefit is that steering a running agent looks calm and correct:
one stopped row, no error, and the queued message starts next.
## Linked Issues or Issue Description
Refs #19369 (same banner class, resource-wait path)
**What happened?**
While an agent ran, the user queued a message and clicked Interrupt on
the queued strip. A red banner "Couldn't finish this attempt. Your
message is saved." with Retry appeared at the top of the task. Two rows
showed the same stopped run: "Stopped for 22s" and "Stopped after 21s".
About 20 s later the banner went away and the message was delivered.
**Expected behavior**
A user Interrupt is not a failure. No red banner or Retry. One "Stopped"
row, and the queued message starts next.
**Steps to reproduce**
1. Start a task with a Paperclip Runner agent.
2. While it works, send a message so it waits in the queue strip.
3. Wait more than 10 s, then click Interrupt on the queued strip.
4. See the red banner, Retry, and two "Stopped" rows.
**Paperclip version or commit**
master
|
||
|
DottaandClaude Opus 5.5
|
15ee6a25a6 |
fix(native): re-bootstrap a claimed Daytona run killed before runner launch (#19616)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs on Daytona start a `paperclip-runnerd` process in a
sandbox. Restart recovery decides what to do with each live run after
the control plane restarts.
> - #19583 lets recovery start the same-run bootstrap again when the
Postgres controller checkpoint proves that the runner received only
bootstrap commands.
> - A live replay found an earlier window. The control plane died after
the executor claimed the run and staged the sandbox workspace, but
before it asked for a runner process. No runner checkpoint existed, so
#19583 did not apply.
> - Recovery classified that run as `ambiguous_remote_recovery_evidence`
and blocked the task. The task also showed "Workspace repair required",
which did not describe the real cause.
> - This pull request proves the "nothing reached a runner" case from
durable evidence and starts the bootstrap again. It also names the
restart in the settled hold note.
> - The benefit is that a restart at any point of Daytona startup before
runner launch continues automatically.
## Linked Issues or Issue Description
**What happened?**
A chaos replay killed the control plane about 2 s after a claimed ACPX
Claude run started on Daytona. The kill came after `stage.workspace` and
environment acquisition, before `native.process_start_requested`. The
restarted server took over because the previous controller process was
dead. It then returned `ambiguous_remote_recovery_evidence` with
disposition `blocked`. The task rested blocked as "Workspace repair
required".
Evidence in the saved database at that point: coordinator `attempt: 1`,
`controllerGeneration: 1`, previous controller identity recorded and
dead. Native workspace sync was `prepared`. There were no runner
checkpoint rows, no session checkpoint, no process PID or start time, no
provider session, and no execution-start or runner-sourced event. The
local session root was an empty directory.
**Expected behavior**
When durable evidence proves that no runner process was requested and no
controller checkpoint exists, recovery continues the run with a fresh
bootstrap. Any evidence that a runner may have started keeps the hold.
**Steps to reproduce**
1. Start a native run on a Daytona environment.
2. Kill the control plane after workspace staging and before the
executor requests the runner process.
3. Restart the control plane.
4. See that restart recovery blocks the task with
`ambiguous_remote_recovery_evidence`.
Root cause:
`server/src/services/native-runtime/native-restart-recovery.ts`,
function `classifyNativeRunnerRecoveryEvidence`, remote branch. The only
no-checkpoint remote rule required an unclaimed coordinator (`attempt:
0`, no controller identity). A claimed run with no launch evidence fell
through to "ambiguous".
Refs #19583, #16349
## What Changed
- `native-restart-recovery.ts`: add the `remote_runner_never_launched`
proof, which leads to `bootstrap_incomplete`. It applies only to a
classic sandbox run with a prepared workspace and the preparation
fencing version. The run must have a real executor claim (controller
identity and lease owner) whose controller is gone (takeover allowed).
It needs no durable checkpoint, no session checkpoint, no process
fields, no provider session, and no provider evidence. The run must also
have no `native.process_start_requested`, process identity,
`adapter.invoke`, `tool.execution.started`, or runner-sourced event. The
same three-attempt budget applies. The shared event query moved into
`hasNativeExecutionStartEvidence`.
- `native-session-executor.ts`:
`restoreRemoteBootstrapCheckpointForRecovery` removes an empty session
root that preparation created. Without that step, the fresh bootstrap
read it as `durable_identity_unreadable`.
- `execution-recovery-resolution.ts`: the new
`executionRecoverySettlementNote` names a settled
`native_restart_recovery_blocked` hold as a stop after a server restart.
It no longer shows "Workspace repair required" because a later workspace
export was retried. Other causes keep their notes.
- `doc/run-log-events.md`: document the new reason.
## Verification
- Red, then green: `cd server && npx vitest run
src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
-t "unlaunched|never requested|Postgres checkpoint"`. Before the fix,
the proven case (lease live with a dead owner, and lease expired) and
the budget case returned `blocked` /
`ambiguous_remote_recovery_evidence`. The empty-root executor case
failed.
- Fail-closed cases: these keep the hold:
`native.process_start_requested`, a process identity event,
`adapter.invoke`, a runner-sourced event, a recorded PID, a recorded
provider session, and a spent budget. A live claiming controller returns
`awaiting_evidence` / `controller_still_alive`. The existing
"contradictory evidence" cases (`claimed` with no controller identity,
`launch`, `process`, and others) stay blocked.
- Wording: `server/src/services/execution-recovery-resolution.test.ts`.
- Regression set (10 files, including `workspace-recovery`,
`native-safe-replacement`, `heartbeat/recovery`,
`native-session-executor`, and `native-restart-recovery`): 2035 passed,
14 skipped.
- Live Daytona replay of the failing chaos task, 3 runs, sequential,
ACPX Claude, double SIGKILL, current master plus this change. Image:
content-matched `paperclip-daytona-runner@sha256:5a29a797…` built from
|
||
|
DottaandPaperclip
|
c9391720a1 |
fix(heartbeat): send saved follow-ups after task completion (#19600)
## Thinking Path > - Paperclip manages agents and their work. > - Task messages remain in a durable queue while an earlier run saves its result. > - That result can mark the task done before the user sends the saved messages. > - Send records current user intent, but its wake guard allowed only active task states. > - The old queue therefore remained pending with an obsolete finalization message. > - This change admits that exact Send receipt and reopens the task through the issue service in the successor transaction. > - The agent can receive the saved input once while the completed result remains unchanged. ## Linked Issues or Issue Description **What happened?** Send on a saved message can leave the message queued after its native predecessor succeeds and commits. The old wait message remains visible. Allowing admission alone also fails: continuation construction rejects a task that remains done. **Expected behavior** Explicit Send must deliver the saved input once through normal admission. It must retain the previous run and its result. **Steps to reproduce** 1. Save a follow-up while a native run finishes its result. 2. Let that run succeed, commit its accepted result, and mark the task done. 3. Send the saved follow-up. Before this change, the receipt stays deferred. Related public work: Refs #19542. That change selects recovered workspace files. This change handles the later saved-message admission. Refs #19602 for the independent workflow fixture repair required by CI. ## What Changed - Allow the exact explicit Send wake guard to accept a done task. Keep automatic recovery and cancelled task guards unchanged. - Reopen the task through the company-scoped issue service after admission gates pass. Commit reopening, queue consumption and the successor together. Publish the audit after commit. - Add a native succeeded/committed predecessor regression with unchanged saved work and one successor after two resume calls. - Verify real heartbeat startup under the clicking operator. Keep automatic input, cancelled tasks and paused agents held. - Check workspace preflight for the prospective reopened task. Use the normal company-scoped transition if preflight blocks a done task, so its completion metadata is cleared. - Check a successful native predecessor for its committed accepted result, current ownership and pending cleanup. Keep approvals and questions effective even without a recovery action. - Document the saved Send contract and update the queue test dependency. ## Verification - Final merge qualification for `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`: all 56 applicable status checks pass, with no pending or failed checks. [Final CI](https://github.com/paperclipai/paperclip/actions/runs/42307623559) includes typecheck, build, all applicable test and E2E shards, and aggregate verification. Current-head Greptile is 5/5; all five review threads are resolved. Any paused or queued application review is not claimed as a new completed review. - Final base qualification at `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`: official preflight passes (1.5s); all six integrated model-normalization cases pass (19.07s); both affected test typecheck layers pass. The integration changes no production server/shared/Runner/adapter inputs, so the preceding 12-case Send proof, two ordinary status guards, and unchanged production typecheck layers remain applicable. Fresh final-head CI supplies broad coverage. - New review regression: both failed and cancelled native predecessors on a task manually marked Done fail before the repair with `continuation_task_ownership_changed` through real `resumeQueuedCommentInterrupt` admission. At `bfb0eff7941090ac3ac43ce4366ba2e3d46612e9`, all 12 affected Send/source/cleanup/workspace/decision cases pass (246.80s), ordinary Done/cancelled rejection cases pass (2; 100.31s), and all seven server typecheck layers pass. History validation uses the prospective reopened status only for an already-verified explicit native Send. No status is changed during a dry run. - Prior `e64e4bb1` CI passed the affected serialized server suites, including the complete saved-message route suite, typecheck and build. General server shard 4 still failed two unrelated stale default-model expectations (`gpt-5.6-sol` versus the new `gpt-6.1-sol` default). The equivalent repair has already merged in #19610 as `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`; the separately prepared #19645 is closed as superseded. This source fix does not weaken or edit those assertions. - Original RED: the committed-source and completed-task startup cases left the saved receipt deferred. - Review RED: Done plus an unrunnable isolated worktree created one successor instead of zero. A successful committed predecessor with pending lease cleanup also created one instead of zero. The new Done-to-blocked path retained stale completion time. Pending approvals and questions each allowed a successor. All failures were captured before their respective repairs. - Integrated production revision `99018264dd1ef002ff3500378184bcd167d9206b`: 417 affected cases pass across the complete native continuation, responsible-user startup, and workspace preflight suites (386 + 18 + 13). These include the original successful startup case and six cleanup/finalization faults that hold the exact saved receipt until settlement, then deliver it once. - `pnpm --filter @paperclipai/server typecheck` passes all seven layers on that integrated head, including the local Runner/SDK dependency build. Module-boundary, whitespace, and added-code-comment checks pass. - Normal master integrations preserve branch history. No force push or assertion weakening was used. Fresh CI and automated review are running on `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`, which normally integrates the already-merged #19610 fixture repair after the reproduced inherited CI failure. - The earlier CI attempt failed in five unchanged workflow fixture cases because IMAGE_ONLY was unset. The merged #19602 repairs that fixture. A later inherited duplicate default in that independent fixture has a separate repair; this PR does not edit it. - Follow-up route-fixture RED: three existing native queue delivery cases failed because their fixture marked a run successful without an accepted result or committed finalization. The same three cases pass after the fixture records the complete native success contract (42.08s); delivery assertions and production gates are unchanged. The affected test-tree TypeScript layer also passes. The full route run passed all 119 assertions but failed its 30-second database teardown hook, so it is not recorded as a suite pass. That result is retained. One unchanged bounded rerun was stopped after more than 15 minutes without a final verdict; it is inconclusive, not a pass. - Final integrated head `e64e4bb1`: all 13 focused route and completed-task admission cases pass across two real-PostgreSQL suites (188.95s), including the three repaired route fixtures and every new cleanup/finalization/decision hold. Fresh preflight passes; all seven server typecheck layers pass. Required final CI supplies the broader route and server coverage. - These tests use real PostgreSQL and normal admission, cleanup, queue consumption, and startup services. The startup suite controls the provider boundary. Hosted provider acceptance remains pending normal deployment. ## Risks - Explicit Send can reopen a done task. This is limited to its validated saved receipt and current assignee. A cancelled task remains held. - Reopening uses the existing company-scoped issue transition and shares the successor transaction. A failed admission does not reopen the task. Provider Stop, workspace, approval, pause and budget checks retain their existing owners. - Cleanup and pending decisions retain the done task and saved receipt. An unrunnable workspace uses the existing blocked-task diagnostic. No schema or runtime package changes. ## Model Used - OpenAI Codex, GPT-6 family; exact serving snapshot/context window not exposed; repository analysis, code editing, tool 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> |
||
|
DottaandPaperclip
|
795c3c4c1d |
fix(recovery): continue saved input after historical retry pause (#19576)
## Thinking Path
> - Paperclip manages work performed by AI agents.
> - Its scheduler parks work when another run holds a shared workspace.
> - A pause can refuse the scheduled retry before provider dispatch.
> - Older schedulers recorded that refusal in the run log without a JSON
marker.
> - After the pause was released, a saved user message could remain held
for execution recovery.
> - This change verifies the original refusal and parent workspace-wait
proof before admitting one continuation.
> - Existing execution, Stop, budget, approval, and workspace guards
remain in force.
## Linked Issues or Issue Description
Refs #19466, #19552, #19580, #19602.
**What happened?**
A saved user message remained deferred after a pause was released. Its
source was a never-claimed workspace retry. The historical source had an
`issue_paused` error and no JSON cancellation marker. The continuation
proof accepted only a different system-cancellation shape.
**Expected behavior**
The normal saved-message worker admits the existing input once when
trusted scheduler evidence proves no provider dispatch occurred and
current admission gates permit work.
**Steps to reproduce**
1. Record a host-certified pre-provider workspace wait and its scheduled
retry.
2. Let the real scheduler refuse promotion under an active task pause,
including after the user presses Retry Now.
3. Keep the historical run-log-only cancellation shape, then release the
pause.
4. Run concurrent normal saved-message recovery scans.
**Paperclip version or commit**
The regression fails on master `f0006c7783`. It leaves zero queued
successors. The branch now normally integrates the fixture repairs in
#19602 and #19628.
## What Changed
- Accept the historical cancellation shape only when a trusted system
lifecycle warning records the exact scheduler pause refusal. A prior
Retry Now event does not invalidate that proof.
- Bind the event to a released same-company pause and its task
membership.
- Reuse the existing parent workspace-wait certificate,
no-provider-output checks, and retained-execution rejection.
- Preserve historical cancellation records and consume the existing
saved input through normal admission.
- Document the contract and add real PostgreSQL coverage using actual
scheduler suppression and pause release.
## Verification
- RED: the real scheduler suppressed the retry under a pause. After
release, two normal recovery scans left zero successors.
- GREEN: the same path creates one successor and coalesces the original
saved request.
- All 389 tests in the complete native continuation suite pass before
the final base update. The only changed continuation source from master
is the separately merged one-line repair in #19552.
- Fourteen focused cases passed at
`2aaeb50f78ba0a22809ab96a00076ce50fcea559`, including a parent pause
covering a child task. Thirteen cases also passed before the base
update. They cover the normal path and missing event, wrong task,
missing membership, active or mistimed pause, provider activity,
retained lease, operator Stop, and invalid ancestry.
- The earlier seven-layer server typecheck passed after building this
worktree's Runner and SDK dependencies. The current review fix passes
the exact CI test-layer typecheck. Fresh required CI will supply
current-head source coverage.
- The first sandboxed database run could not bind loopback. A later
fresh dependency install omitted native PostgreSQL library symlinks and
failed before executing tests. The package's own postinstall hydration
repaired that setup. Previous-head verification passed all 14 focused
cases at `2aaeb50f78ba0a22809ab96a00076ce50fcea559`.
- Module-boundary and whitespace checks pass.
- Commands: `cd server && pnpm exec vitest run
src/services/explicit-native-continuation.test.ts`; `pnpm --filter
@paperclipai/server typecheck`.
- Environment: macOS arm64, Node 26.4.0, pnpm 9.15.4, real local
PostgreSQL.
- Previous head `2aaeb50f78ba0a22809ab96a00076ce50fcea559` had Greptile
5/5 and 56 passing checks, with three skipped. A later advisory review
found the Retry Now event-order case. Those checks do not qualify the
new head. Hosted recovery remains unverified.
- Review RED: the real `retryScheduledRetryNow` producer wrote
lifecycle/info at sequence 1 and the exact pause warning at sequence 2.
After real hold release and two concurrent normal queued-message scans,
both task-rooted and parent-rooted cases produced zero successors
instead of one. Both failed before the production change (17.24s).
- Review GREEN: all 17 focused cases pass at
`f38e9934b8ce3f7ddfb23eaa0f979c3f44e7a990` (4.74s). The previous 14
cases remain covered. Two new cases use the real Retry Now producer, and
one new case rejects contradictory provider execution evidence. No
pause, membership, timing, ancestry, output, retained-execution, or Stop
check was removed.
- Current focused command: `pnpm exec vitest run
server/src/services/explicit-native-continuation.test.ts -t 'historical
pause|Retry Now pause history' --fsModuleCache`. Current test-layer
typecheck: `cd server && node ../node_modules/typescript/bin/tsc
--noEmit --project
node_modules/.cache/paperclip-typecheck/tsconfig.tests.json --pretty
false`. Both pass.
- Final-head verification: all 17 focused cases passed again at
`c88687d69f2c87fe1b81b5fffd670e3cf26a884d` (8.00s). The prior exact CI
test-layer typecheck qualifies the unchanged typed logic; fresh CI
supplies broader final-head coverage.
- The follow-up commit removes only the new code comments to follow
CONTRIBUTING.md. Recovery and test behavior are unchanged. That revision
had Greptile 5/5 with its actionable review thread resolved. It is
retained as historical verification; current-head qualification is
below.
- The first final-head CI attempt failed in the unchanged CLI company
import/export fixture before assertions: its child server did not reach
the runtime-info health check within 30 seconds. Other suites passed.
The exact fixture passes locally with its original timeout. The single
failed-shard retry passed at the unchanged head, together with its
dependent CI checks. [Original failed
job](https://github.com/paperclipai/paperclip/actions/runs/42279773036/job/128106195975).
No assertion or timeout was changed.
- The independent required Runner support failure reproduced five
`IMAGE_ONLY: unbound variable` errors. #19602 repaired its fixture
inputs and merged as `4d554f08218b04df49ec0f2a0d5c5dfdfaf65660`. This
branch normally merges that master with no conflicts or force push. The
current host-preparation log verifier from #19580 remains intact.
- Final integrated head `ffc9e8d9516343a9823b4b80b5fcfccc548844ba`:
17/17 focused recovery cases pass (12.71s), all nine workflow-target
cases pass through the official Runner Vitest config (1.10s), and all
seven current server typecheck layers pass. Worktree preflight rebuilt
current Runner dependencies and reused the Cargo seed; it completed in
61.3s with Rust debug binaries skipped.
- Prior integrated head `ffc9e8d9516343a9823b4b80b5fcfccc548844ba`
finished with 54 successful checks. Its only code-check failure was a
global unhandled readiness rejection after 2,285 assertions passed in
the server8 shard, plus dependent verify. The real-child race is fixed
separately in #19624; it was not hidden by a rerun here.
- Final normal integration head
`550d1732b6960d6b349d03903f448fa1d71c17f6`: exact 17 paused-retry
lifecycle regressions pass (71.06s), all nine official workflow target
cases pass (2.43s), all seven server typecheck layers pass, and
module/whitespace checks pass. Required preflight completed in 45.641s.
Current preparation-log guards from #19580 remain intact. The diff
against master contains only the original three owner/test/doc files.
- Prior head 550d had two inherited model-default assertion failures
plus a retained-cleanup backlog 15s timeout. The exact unchanged cleanup
case passes locally (41.91s total including DB/hooks); no timer or
assertion was weakened and no old CI job was rerun. The model fixture
repair was independently merged in #19610 at
|
||
|
DottaandPaperclip
|
2e340d6235 |
fix(ui): show unread message counts in agent chat pickers (#19627)
## Thinking Path > - Paperclip helps people manage AI agents and their work. > - People switch between agent chats to read replies. > - The agent picker does not show how many messages are unread. > - People need that count before they select a chat. > - The chat list already has the viewer's read state. > - This pull request adds numeric badges from that shared list. > - People can find unread replies without extra requests for each agent. ## Linked Issues or Issue Description **What existing behavior does this improve?** The chat header agent picker and the secondary sidebar's Add chat picker. **Subsystem affected** Agent Chat UI, shared task types, and the chat read-state service. **Current behavior** The pickers do not show the unread message count for each agent. **Proposed behavior** Show the unread count beside each agent. For example, show 3 for three unread agent messages. Hide the badge when the count is zero. Keep switching available while chat data loads. **Reason and benefit** People can see which chats need attention before they switch. The count uses the existing shared request and cache. **Breaking changes** The chat response gains an optional `unreadMessageCount` field. Existing fields and read receipts keep their behavior. There is no database migration. **Additional context** Related: #19425 changes chat structure. This change adds counts to the current agent pickers. The duplicate search found no PR for these count badges. ## What Changed - Count unread agent comments in the existing grouped read-state query. Keep company and viewer scope. - Exclude human messages, deleted comments, system notices, and fast-response receipts. - Pass memoized count maps to both agent pickers. Use a direct lookup for each row. - Show numeric badges and accessible singular or plural labels. - Test count changes, account isolation, cached opens, search, and large agent lists. - Update the chat stories, fixture data, and development guide. - Make the Slack owner retry-deadline test deterministic after reproducing its CI failure. ## Verification - On current head `05f5fa6201`, the [main CI workflow](https://github.com/paperclipai/paperclip/actions/runs/42305845930) passed, including build, typecheck, all applicable test shards, browser shards, and final verification. The prior server shard 2, 6, and 7 failures do not recur. - [Greptile's current-head review](https://github.com/paperclipai/paperclip/pull/19627#issuecomment-6696161631) is 5/5, its check succeeded, and both previous review threads are resolved. Current head check inventory is complete: 55 successful checks, three intentional skips, and no pending or failed checks. The Paperclip application review summary reports paused; no new completed Paperclip review is claimed. Its existing findings are resolved, and the independent current-head Greptile review is 5/5. - Current head `05f5fa6201` merges master `8621bcf53e` to resolve an import conflict. Master now includes the same model-default test repair. This branch keeps its upstream import, so that file no longer differs from master. - After this merge: all 21 server tests in the Slack owner and model-default suites pass, all 22 tests in the three picker/sidebar suites pass, and token gates pass. The standard preflight still cannot build Rust because `cargo` is absent. Fresh CI must supply the broad checks. - Head `7d125fb8c7` replaces the Slack owner test's real timer wait with the worker's existing clock input. It checks that the intent stays pending one millisecond before the retry deadline and clears at the deadline. All 15 Slack owner integration tests pass locally. Runtime retry behavior is unchanged. - On `4cfb1e9d55`, both review checks gave 5/5. CI [server shard 2](https://github.com/paperclipai/paperclip/actions/runs/42302458830/job/128183671349) failed the Slack credential-lease retry test. The same assertion failed in an unchanged local run. A diagnostic run with added logging passed; the exact timer deviation was not captured. The new test removes the real-time wait assumption and checks the clock boundary directly. The earlier model-default and cleanup failures did not recur in that CI run. - On `a3378dfc40`, both Greptile and Paperclip Review gave 5/5 with no open findings. Typecheck, build, all eight browser shards, and most test shards passed. - That CI attempt failed two server shards. The model-default test still expected `gpt-5.6-sol`, while the current default is `gpt-6.1-sol`. I reproduced both failures locally. Commit `4cfb1e9d55` uses the exported default-model constant; all six tests in that suite now pass. Production model selection does not change. - The retained-cleanup backlog test exceeded its 15-second limit in [server shard 6](https://github.com/paperclipai/paperclip/actions/runs/42299858777/job/128175193320). The unchanged test passes locally with the same limit. Its CI timeout cause is not established. No assertion or timeout was changed. The fresh CI run must recheck it. - Review repair: commit `0db631b215` keeps the existing-chat and unread-agent arrays stable when chat data and the active agent do not change. A new 500-agent test fails before the repair and passes after it. The test also checks count refreshes. - The first CI typecheck failed on duplicate `IMAGE_ONLY` and `TESTOPS_REQUESTED` keys in `tests/runner-e2e/workflow-target.test.ts`. This was present in upstream code. Master commit `e482f0f632` removes the duplicate keys. This branch now includes that commit. All nine workflow-target tests pass locally. - After the latest master merge: all 22 tests in the three affected picker/sidebar suites pass. Before that merge, all 38 focused UI tests passed. - The current head is `05f5fa620134ad199a3d85b66fb2eb93d3bb6077`. All current status checks are complete, the aggregate verify gate succeeds, mergeability is CLEAN, and no unresolved review threads remain. The maintainer has instructed normal merge after checks pass. - After merging current master: 37 focused UI tests pass across AgentChatPicker, CoordinatorAgentSwitcher, AgentConversationsSidebar, useAgentChatRead, and SidebarAgentChats. - After merging current master: all 38 proactive-chat integration tests pass with a real disposable PostgreSQL database. They check counts of 2, 1, and 0 at exact read cutoffs. - The prior revision passed UI typecheck and build, shared build, isolated low-memory server typecheck, token gates, and 30 map/catalog tests. - The 500-agent test bounds index work and count lookups. Search does not rebuild the existing-chat index. - Browser checks used production components with simulated API data. Dark, light, mobile, and Add chat cases passed. Counts of 3 and 12 stayed visible through search. Read agents had no badge. Cached opens made no chat API requests. - Twenty local cached opens took 34.5–57.6 ms, with a 38.4 ms median. This is an unthrottled fixture measurement. - Earlier full local checks could not complete because Rust cargo is absent and the workspace has limited memory. The standard worktree preflight also stops at missing cargo. Required CI must supply broad checks for this PR head. - The first server typecheck ran out of memory while Storybook was active. Its isolated low-memory retry passed. No visual baseline comparison or live provider delivery is claimed. ## Risks - Low risk. The count replaces a boolean aggregate in the same grouped query. Very long chat histories still need that existing scan. - A count can be stale until the existing chat-list cache refreshes. Read receipts still control the unread cutoff. - The change adds no polling, per-agent history request, database migration, or permission change. ## Model Used OpenAI GPT-6 through Codex, with code editing, terminal tools, and browser verification. The runtime did not expose a more specific served model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
7941f6b051 |
fix(runner): show OpenRouter's invalid model ID as a model rejection (#19548)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs use the Paperclip runner with drivers such as OpenCode
> - When a provider rejects the selected model, the product shows "Model
unavailable" and offers a model choice
> - OpenCode reports provider HTTP errors as `APIError` with the message
under `data`, not at the top level
> - The runner then reported "Provider turn failed", so an invalid model
ended as a generic adapter failure
> - This pull request classifies OpenRouter's invalid model ID with a
closed provider-failure category, and makes the server keep that
classification
> - The benefit is that a person sees "Model unavailable" and the real
provider message, and can choose another model
## Linked Issues or Issue Description
**What happened?**
A chat evaluation selected a model ID that OpenRouter does not know.
OpenCode reported `turn.failed` with `error: { name: "APIError", data: {
message: "<model> is not a valid model ID", statusCode: 400,
isRetryable: false } }`. The run was saved with `errorCode:
adapter_failed`. The task chat showed "The OpenCode run failed before it
completed." and no "Model unavailable" marker. The provider notice said
only "OpenCode session failed.".
**Expected behavior**
A non-retryable rejection of the selected model is saved as
`native_provider_model_rejected`, so the product shows "Model
unavailable" and the model choice. The notice shows the provider's
message.
**Steps to reproduce**
1. Configure an OpenCode agent with an OpenRouter model ID that does not
exist.
2. Send the agent a message.
3. Before this change, the run fails as `adapter_failed` and the notice
says "OpenCode session failed.".
## What Changed
- `drivers/opencode/provider-failure.ts`: a non-retryable OpenCode
provider error whose message says the model is not valid (`not a valid
model`, `model_not_found`, model not found or not supported) gets the
closed category `model_unavailable`. A provider that allows a retry
keeps the existing category.
- `native-session-runtime.ts`: a non-recoverable failure with
`model_unavailable` is a model rejection. The runtime reads the closed
category, not provider text.
- `native-session-executor.ts`: a non-recoverable
`NativeProviderTerminalFailure` that the runner marked as a model
rejection is saved as `native_provider_model_rejected`. Before, every
such terminal was saved as `native_provider_terminal_failed` before the
marker was read.
- `drivers/opencode/opencode-server-driver.ts`: the provider notice
summary shows the provider message from `data` when there is no
top-level message. Provider text is not used as the turn failure
message, so it does not reach `run.error` or Sentry.
- Tests: provider-failure mapping (retryable and non-retryable), runtime
classification by category, driver notice summary with no failure
message, and an executor test for the saved code and terminal
disposition.
## Verification
- `npx vitest run src/native-session-runtime.test.ts
src/drivers/provider-failure.test.ts src/drivers/opencode` in
`packages/paperclip-runner`: 16 files, 412 tests passed.
- `npx vitest run
src/services/native-runtime/native-session-executor.test.ts` in
`server`: 1,152 tests passed.
- Negative controls: reverting the provider-failure mapping, the runtime
check, or the executor branch each makes its new test fail.
## Risks
- Low risk. Classification uses a closed category and a runner-set
marker, not provider text.
- The model check on the provider message is text-based inside the
OpenCode mapper. A provider that uses "not a valid model" for another
reason with a non-retryable error will show "Model unavailable".
- Provider text appears in the notice summary, as provider notices
already did for top-level messages.
> 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, with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Nicky LeachandClaude Opus 5.5
|
913102870d |
fix(environments): fill the managed instance default once, under a lock (#19632)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On a managed instance under `enableManagedSandboxOnly`, boot points the instance default at the managed sandbox when nothing deliberate is set > - That write read the default and wrote it later without a lock. Two overlapping boots, or a boot and an admin write, could overwrite a value the other had just committed > - A revert path also reset that stamped default whenever the mode was turned off, so a platform-filled default could change again later > - A planned plugin-environment default should fill only an unset default and never change a set one, which needs every instance's default to stay set > - This pull request makes the fill one locked transaction and removes the revert, so a filled default is an ordinary set value > - The benefit is a single, race-free rule: boot fills an unset default once, and nothing changes a set default afterwards ## Linked Issues or Issue Description Refs #19551 (the plugin-environment default that builds on this rule) **Subsystem affected** Server environments: the managed environment bootstrap and the instance settings route. **Problem or motivation** `applyManagedEnvironments` read the instance default, decided, and wrote it in separate steps with no lock, so a concurrent boot or admin write could be overwritten. When `enableManagedSandboxOnly` was not declared, a revert reset a default the platform had filled. That needed a `managedDefaultStamped` marker, plus a marker clear on every admin default write. A later boot could therefore change a default that had already been set. **Proposed solution** Fill the default in one transaction that locks the instance default row and rechecks it. Fill only an unset, local or deleted default. Remove the revert and its marker, so a filled default behaves like any other set value. **Alternatives considered** Keeping the revert and adding locks to it. A revert on a mode change would still move an existing instance's default later, which a fill-only plugin default cannot tolerate. **Roadmap alignment** This narrows existing managed-environment behaviour. It adds no product surface. ## What Changed - New `server/src/services/managed-instance-default.ts`: `fillUnsetInstanceDefault(db, environmentId)`. In one transaction it creates the settings row if needed, locks the instance default row (`FOR UPDATE`), and rechecks the current value. It writes only when the default is unset, names the local environment, or names a deleted environment. Any other set default is kept. - `server/src/services/managed-environments.ts`: - Under `enableManagedSandboxOnly`, the managed sandbox fill now uses that helper, through a new `fillInstanceDefault` test seam. - The mode-off revert and the `managedDefaultStamped` marker are removed. - The unused `instanceSettings` seam option is removed, along with the `getById`, `update` and `findManagedSandboxEnvironment` methods of the environments seam. - `server/src/routes/instance-settings.ts`: an explicit default write is a plain `update` again. The marker clear and its transaction are gone. - `doc/DEVELOPING.md`: the "Managed environments" section states the fill rule. Existing rows may still carry `managedDefaultStamped` in their metadata. Nothing reads it any more, and it is not part of the managed stock hash. ## Verification - `cd server && npx vitest run src/__tests__/managed-instance-default.test.ts` (embedded Postgres): - The fill writes an unset default, including on a fresh instance with no settings row. It also writes a default that names the local environment, or one whose environment was deleted. - It never changes a default set to an ssh, plugin or other sandbox environment, and leaves a default that already names the target alone. - Controlled overlap: an uncommitted admin default write makes the fill wait, and the fill then keeps the admin's value. This test fails without the row lock. - Two overlapping fills: exactly one writes. - Real boot pass: the default is filled under managed-sandbox-only and stays the same after a boot with the mode off. A set default stays on every boot. - `npx vitest run src/services/managed-environments.test.ts src/services/managed-environments-plugin-entries.test.ts`: the fill runs only under managed-sandbox-only, with the managed sandbox row. It never runs for a plugin environment. Nothing is written when the mode is off. A failed fill does not fail the ensure. - `npx vitest run src/__tests__/instance-settings-routes.test.ts`: an explicit default write is a plain update that touches no environment row. - `npx vitest run src/__tests__/managed-plugin-environments.test.ts src/__tests__/environment-routes.test.ts src/__tests__/server-startup-feedback-export.test.ts src/__tests__/instance-settings-service.test.ts`: pass. - `pnpm --filter @paperclipai/server typecheck` passes. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. ## Risks - Behaviour change: when `enableManagedSandboxOnly` is turned off for an instance, a default the platform filled with the managed sandbox now stays. Local execution is still allowed then, but it is no longer the default unless an admin picks it. - No schema change. Stale `managedDefaultStamped` keys stay in some row metadata and are ignored. - The fill holds the instance default row lock for one short transaction at boot. > 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, through Claude Code with tool use (file edits, shell, 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 - [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> |
||
|
Devin FoleyandPaperclip
|
8621bcf53e |
Cancel subscription sign-in requests when leaving agent setup (#19610)
Back during pending subscription sign-in could accept a late response and
reopen agent setup. Abort pending reads, release abandoned sessions, and
ignore stale responses. Wait for cleanup before starting a replacement,
and preserve sessions belonging to another account or environment.
Cover deferred resume/start/prompt responses for both providers, local
login cancellation, quick reselection, and browser request abort. Stabilize
Runner fixtures and normalization assertions exposed by integration CI.
Verified: 483 focused UI tests, six onboarding browser cases, UI typecheck
and token gates, Runner fixture checks, and the complete CI run on
|
||
|
DottaandPaperclip
|
c66b4a55ef |
fix(ui): explain monitor waits during active runs (#19519)
## Thinking Path > - Paperclip manages work performed by AI agents. > - Task monitors schedule later checks through execution admission. > - A native run must save its work before a monitor starts another run. > - The monitor banner can show an overdue warning during this valid wait. > - It also promises the next tick and offers a check that cannot start. > - This change explains the active run hold on both monitor surfaces. > - Users can see why the check waits without changing execution guards. ## Linked Issues or Issue Description Refs #16975 and #17863. The branch also preserves the quiet workspace-wait behavior from #19507. These changes added the native settlement guard and monitor deadline worker. A search found no open PR for this display case. **What happened?** A monitor became due while its native run was still saving work. The banner showed an overdue warning and “Fires on next tick.” The check later ran. The warning did not explain the execution hold. **Expected behavior** Show the active run wait. Keep the saved check time. Restore the normal display when the run clears. **Steps to reproduce** 1. Keep a task run active and set a monitor on that task. 2. Let the saved check time pass. 3. Inspect the monitor banner and composer strip. **Paperclip version or commit** Reproduced in mounted production components at |
||
|
|
ee6c101854 |
Reject failover backups behind the accepted event cursor (#19613)
A verified failover backup can still trail events already accepted by the controller. Compare the backup produced cursor with the registered controller state, or the supported local journal before registration, and reject stale files before remote sync. Keep the permanent replay-conflict classification ahead of generic Runner errors.
Validation: 1,192 native session and execution-driver tests and 10 workflow tests pass locally, together with the maintained low-memory Runner test project typecheck and fresh server declarations. All applicable checks on
|
||
|
DottaandPaperclip
|
44c60c7221 |
Verify default skill sync through two real cold sleep cycles (#19191)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Automatic skill sources check upstream every five minutes. > - The domain deadline contract can hand future work to a durable wake alarm. > - A saved deadline or a worker unit test does not prove that a stopped tenant resumes the real producer. > - This pull request checks the default cadence through the actual app and a matching service checkout. > - It checks two cold starts and three distinct persisted skill revisions with real clocks. > - The benefit is repeatable local evidence without changing production timing or authority. ## Linked Issues or Issue Description Refs: #19001, #17008, #17923. This is a test and documentation follow-up to #19001. #19001 is now merged; this PR targets master. Feature code remains in that merged foundation. #17008 covers the earlier source worker change. #17923 covers separate skill-cache retention; this PR does not change that behavior. A scoped search of public PRs and issues found no duplicate default-cadence cold-cycle acceptance test. No matching public issue was found. The enhancement fields follow. **Pre-submission checklist** This improves checks for an existing feature. It adds no product endpoint or execution authority. The public related-work search is complete. **What existing behavior does this improve?** Verification of automatic skill-source updates across a durable idle handoff and cold restart. **Subsystem affected** `server/` test fixtures and development documentation. **Current behavior** Domain tests cover claims, access and future deadlines. They do not prove the full default-300-second path from ordinary source attachment through signed tenant reports, durable alarms, stopped compute and a changed persisted skill. **Proposed behavior** An opt-in launcher checks bootstrap first, then two full real-clock cycles. Ordinary CI can run the transport controls without the host fixture. The launcher-specific worker assertion runs only when prewire is requested, and fails closed in that lane. **Reason and benefit** The test can detect an early refresh, a lost deadline, a stale proof, an ACK after physical stop, a failed cold restart or a producer that does not publish changed contents. **Breaking changes** None to product APIs, schema, cadence, authorization, claims or retry policy. Existing maintenance-helper defaults stay unchanged. **Additional context** The opt-in host flow needs an authorized matching service checkout, its normal compiled acceptance bridge, Node 22 and two owned PostgreSQL test databases. The standalone controls do not need that service or PostgreSQL. Roadmap review places this under existing Skills Manager and Cloud deployment verification, not a new core feature. ## What Changed - Add an opt-in default-cadence acceptance test and checked-in launcher. - Reuse real server startup, managed tenant authority and the ordinary project attachment route. Control only the remote Git snapshot boundary; keep actual authorization, scanning, claims and publication. - Use actual signed tenant routes and the matching service's gate, sleep controller and PostgreSQL wake-alarm store. Stop and restart real app children through a controlled local provider. - Preload the Node transport guard before coordinator, worker and child module work. Check worker-local and child PID, thread and source-hash receipts. Keep caught unexpected transport attempts fatal. - Require both persisted refresh times and actual upstream handler timestamps to be at or after each original deadline. - Join physical callbacks during bridge close, stop a late wake-created child, and preserve the original cleanup budget. Add a real IPC/HTTP-child counterfactual. - Accept valid owned HTTP options paths without allowing Unix sockets or unknown origins. Remove the unused App helper export. - Add finite transport and launcher controls. Defer only the launcher-specific parent assertion in vanilla CI. Keep all subprocess controls default-runnable. - Add optional child entry, preload and bounded pool settings to the existing test helper. Preserve its defaults and the real observer test body. - Remove added explanatory comments under the contribution policy. Preserve executable syntax, directives and literals. - Document pinned setup, fresh owned databases, bootstrap and full two-cycle commands in `doc/DEVELOPING.md`. The diff against the intended base contains nine test/helper/launcher files and this one document. It adds no production feature code or Cloud source to the App repository. ## Verification The latest two-file CI repair preserves both original failed jobs. A newer master workflow introduced two required step inputs absent from its extracted-step test environment; the faithful false/false inputs now pass all five existing target-authorization cases, after an actual missing-input counterfactual failed. A controlled socket case also reproduces legal reuse of the released runner port; the fixture now holds that reservation until the preview binds, releases it in finally, and asserts distinct ports. All original retry/result/accounting assertions and timeouts stay unchanged. The original Linux listener owner remains unknown; the control proves this fixture collision mechanism, not that original attribution. Fresh current-head full CI and complete reviews are pending. Current merge-integration candidate: `f6f686103b7bfd0a4ab83085634c622af3896ecb`, a normal merge after #19001 landed, including actual master `1c9767d323cf872a266962b9e6b4bd5cbcb57f1c`. All nine acceptance code files are byte-identical to148a82e5; both parents' documentation additions survive. Every other path equals actual master. The only conflict was the old foundation's startup block; current master was retained in full. Fresh exact-head CI and complete reviews are pending. The local measurements below belong to their explicitly named revisions; no fresh full614-second run is claimed. Current candidate: `148a82e54c502759785407357001c6b50c1439dd`, normally carrying the current base `8c912e1ba4fc66397a7341ade59373af8075068f`. All nine acceptance code blobs remain identical to qualified repair `e092746f4da181c8414c63be71474619a84b97e7`; documentation retains both parents. The matching built service for the fresh bootstrap is `389878364b42dd129835d263f91882c5774d5123`, using Node 22.23.4. The App uses Node 24.22.2 and pnpm 9.15.4. Use the exact controlled setup and commands in [doc/DEVELOPING.md](doc/DEVELOPING.md#controlled-default-cadence-sleep-acceptance). Fresh owned databases and the matching compiled service are prerequisites; the launcher does not create a cluster or build dependencies. - **Current affected semantic check passed**, using actual native TypeScript 7 and the current generated CI options/references (0.640 seconds). Normal declaration references passed in 11.726 seconds, with source-matched SDK/shared/kernel/Runner prerequisites and the frozen current lockfile. - **Current guarded controls: 44 passed**, plus the same single explicit real-PG observer deferral, in 4.790 seconds. The executing worker checked its own global PID/thread/preload source hash and matching receipt. Guard hash: `7daf9ae7d2f26e8fcd389b1d560fac1d4e218a8304a2c58e49f646af108165bc`. No unexpected attempts or PG sockets occurred. Vanilla controls on the identical repair files passed 22 cases with one explicit launcher-parent assertion deferral (2.044 seconds). - **Real cleanup counterfactual:** unchanged old helper failed the actual endpoint assertion in 1.568 seconds: its late HTTP child remained reachable after close returned. The same fixed-helper case passed in 1.485 seconds: close waited, the child exited, the port was unreachable and no IPC error occurred. The isolated driver records only the known legacy closed-IPC error long enough to inspect cleanup; current helper requires zero such errors. All recorded processes and ports were independently checked empty. - **Fresh current bootstrap passed once**, 16.159 seconds wall / 14.12 seconds Vitest / 12.237 seconds body. The ordinary enabled producer persisted useful revision 1, cleared its lease and retained exactly 300,000 ms between success and original next deadline. A signed current-generation report produced the matching generation/operation-lock SQL ACK before physical child stop (ACK 06:54:02.020 UTC; stop signal 06:54:02.025 UTC). The alarm retained the original D and D-120 eligibility. Both fixture databases ended with zero sessions; the coordinator independently audited and normally removed them. This bootstrap does not execute a due wake or the new v2/v3 timing assertions. - **Historical full two-cycle result remains 1/1 passed in 614.213 seconds wall**, at measured App `74f7c012ca17ad0d7302d76403f5033affb1430b` and service `2cb54f904031dcf69a32902cd853b43f74d6f301`. Two new processes published distinct immutable v2/v3 at original D+115 ms and D+98 ms. Exact signed SQL ACKs preceded both physical stops; real SQL D-120 claims caused both cold starts. Features, claims and original business deadlines stayed unchanged. - Both historical 10.002-second quiet windows kept app SQL, upstream and outgoing HTTP counts unchanged. Two awake PG sockets remained. Quiet counts do not mean all connections closed. - The stronger timing checks are additional assertions, not retroactively executed results. Offline inspection of the original receipts confirms persisted successes at D+115/+98 ms and earliest upstream handler entries at D+44/+30 ms; all five requests in each refresh group started after D. The exact new pre-sleep request counter and dispatch-before-handler time were not independently recorded historically. - Source review found 19 critical automatic Git producer/runtime files identical to the measured App. The current active cache helper change does not run on automatic Git publication and adds no pool. The matching service carries unchanged measured helper/sleep/controller/alarm seams through its normal master merge. The current bootstrap and focused controls qualify the changed cleanup/guard and parent integration. No fresh current-head 614-second pass or whole-tree identity is claimed. - Runtime is serialized: worker 1, per-pool limits at most 4, configured App potential 10 + shared service pool 2 + one observer against server max 40. Current bootstrap point samples observed at most 3 owned clients plus the observer; those samples are not a continuous aggregate peak. App/service guard peaks were 2 each, with zero remaining sockets at cleanup. Failures remain accounted for. The first historical bootstrap cleanup masked an unknown primary error. Later attempts exposed missing fixture-owned domain metadata and lost origin snapshots; test-only ownership/cleanup corrections preserved the original assertions and timeouts. The new causal driver first lacked a final observation, then encountered sandbox bind EPERM, then exposed the old closed-IPC error; only the final real endpoint failure above is labeled the cleanup counterfactual. An offline dependency miss and a generator-cleared ignored compiler config were retained and corrected without weakening compiler options. The former inherited policy failure (397 findings) is resolved by the genuine current-master base/scanner, with zero findings and seven scanner tests; no unrelated UI site was edited and the gate was not relaxed. Historical real TCP observer coverage passed 1/1 in 3.688 seconds. Comment-only and opt-in-selection carries remain documented in prior evidence; self-hash changes are metadata exceptions with fresh executing-worker checks. The full workspace typecheck, test, build and browser commands were not run locally for this test-only slice. The current base passes its full exact-head CI and Greptile 5/5; this leaf now has all 56 exact-head check runs terminal (54 success, one neutral, one skip), the additional legacy security status passing, complete current Greptile 5/5 and Ponytail 5/5 reviews, and all four review threads resolved across all pages. No live provider or deployed host/database suspension was exercised. ## Risks - The host test requires a matching service checkout and compiled bridge. An unrelated service revision is not a valid substitute. - Controlled localhost Git, S3 and physical process operations do not qualify deployed Railway or Neon suspension, external permissions or provider uncertainty. - The Node guard does not prove native subprocess transport isolation. Only finite local Git metadata and Node version commands were needed. - Two short SQL/HTTP quiet windows do not establish strict 90-second host acceptance. Separate plugin-host LISTEN, MCP wake and other domain gaps remain open. - Explicit test URLs are destructive fixture inputs. Use fresh, owned `_test` databases. The launcher does not start a cluster or choose a database implicitly. - Passing CI and review do not qualify deployed suspension; the current proof boundaries above still apply. > For core feature work, check `ROADMAP.md` first and discuss it in `#dev` before opening the PR. This PR adds verification to the existing feature in its base. ## Model Used OpenAI `gpt-6.1-sol` assisted with implementation, repository review and local code execution. OpenAI Codex, based on GPT-6, performed managing review and integration. The context-window sizes and exact served backend snapshots were not exposed. ## 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 - [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 — fresh merge-head gates pending - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — fresh merge-head review pending - [x] I will address all Greptile and reviewer comments before requesting merge ### Current master reconciliation (2026-11-06) Merged actual master `e28158b353fd274f0322be76d62cd223e0df599f` normally. All nine qualified acceptance code files and the held-port repair remain byte-identical. Upstream independently landed the workflow input fix in #19602; the fixture now uses that exact upstream version once, avoiding duplicate object keys in a squash merge. All other master changes are preserved. The earlier two-cycle local evidence remains source-specific; no fresh 614-second run or deployed suspension is claimed. This new head requires fresh CI and bot reviews. ### Actual combined-tree typecheck failure and repair CI job `128156887548` failed with TS1117 at workflow-target.test.ts:22: two independently landed master fixes declared the same `IMAGE_ONLY` and `TESTOPS_REQUESTED` keys. The complete original log is retained. A further normal master merge now removes the duplicate declarations, retaining exactly one false/false pair and every original assertion. All nine qualified acceptance code paths and the held-port repair remain byte-identical; unrelated master changes are preserved. This is a concrete compiler failure and source repair, with fresh exact-head CI/reviews required. --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
e482f0f632 |
fix(server): shed only retryable object-journal reads under load (#19176)
Client polling can exhaust the shared object journal queue and reject log persistence. Bound ordinary object-log and trace reads, keep persistence and internal verification queued, and return retryable 503 responses when reads are busy. Leave feedback exports unsent until every log page is available.
Validation: 72 latest-master affected tests and 10 workflow tests pass locally, with the workflow fixture passing strict TypeScript checks. The preceding broader run passed 280 server tests with two opt-in S3 skips. All applicable checks on
|
||
|
DottaandPaperclip
|
c09fb60ca6 |
fix(recovery): retain worker-stop audit in preparation retries (#19612)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery must preserve saved files before it admits another execution. > - A failed preparation can need a verified stop of its previous workspace worker. > - Recovery keeps that verified stop receipt in the run profile for audit. > - The preparation retry guard treats the retained receipt as unknown execution evidence. > - This pull request recognizes a valid same-run audit receipt while it retains all current execution and file checks. > - A saved Retry can then continue once after recovery completes. ## Linked Issues or Issue Description **What happened?** A native preparation failed before provider dispatch. The recovery worker verified that the previous workspace worker had stopped and saved its files. The same HTTP Retry stayed deferred because the run profile retained `nativeWorkspaceOwnerStopProof`. **Expected behavior** The saved Retry should continue once after all preparation, stop, file, ownership, and authorization checks pass. The old run and its audit receipt should remain unchanged. **Steps to reproduce** 1. Fail native preparation with a retained remote workspace and no provider activity. 2. Let the recovery worker acquire the old workspace owner through verified platform stop evidence. 3. Submit one HTTP Retry and let preservation complete. 4. Observe that the old guard retains the retry hold even after saved-file verification passes. **Paperclip version or commit** Reproduced against `d728be8fbf`. Related work: #18908, #19021, #19026, and #19466. This change covers the retained workspace-worker stop audit left by a later repair. ## What Changed - Add a typed reader for a host-persisted same-run worker-stop audit receipt. Keep signature and expiry validation at ownership acquisition. - Recognize only that validated audit field in failed-preparation settlement. Keep unknown profiles and active owners blocked. - Exercise real preservation, HTTP Retry delivery, native dispatch, saved files, accepted results, and task completion. Add malformed, foreign-run, unstopped-worker, and active-owner cases. - Document the distinction between retained audit evidence and current execution authority. ## Verification - Failing before the fix: the current-worker preservation case passed; the stopped-worker case produced zero successor runs after the same Retry, where one was expected. - Passing after the fix: 47 recovery cases, 10 native dispatch/preservation cases, and 6 platform-proof tests. The two preservation success cases overlap between the first two groups. - Server typecheck passed all seven layers after the final test changes. The Runner and SDK dependencies were built in this isolated worktree. - Final comment-free commit `44f8f33f8d917dddb51852283a479786102eab4d`: a fresh combined run passed all 61 affected cases in 67.92s. Fresh server typecheck passed all seven layers. - The earlier `95d0376c` combined run passed 60 cases and hit one `socket hang up` in the existing `catalog_provider_warning` HTTP test. That case passed in a bounded isolated run (12.48s), and the final 61-case combined run passed. The socket failure remains recorded as an intermittent HTTP harness limitation. No retry or weaker assertion was added. - Final commit `44f8f33f8d917dddb51852283a479786102eab4d` completed all required CI gates: 57 checks succeeded, 3 intentional skips, and the advisory Paperclip review is neutral with a substantive 5/5 verdict. Greptile is 5/5 and there are no unresolved review threads. [Final CI run](https://github.com/paperclipai/paperclip/actions/runs/42293625696). The follow-up commit only removes six new code-comment lines to follow CONTRIBUTING. - The native provider and signed platform API are controlled test boundaries. The tests use real PostgreSQL, recovery, ownership, file persistence, retry admission, dispatch, and finalization services. Deployed recovery remains unverified until normal rollout. ## Risks - The main risk is mistaking an audit receipt for current stop authority. The reader checks its exact shape and run binding. The receipt cannot replace stopped leases, saved-file receipts, settled preparation, current authorization, or the absence of an active owner or provider evidence. - The negative matrix runs with the audit field present. Provider activity, unknown usage, bad file receipts, Stop, actor changes, and unsettled leases still block continuation. - No schema or migration change is required. ## Model Used OpenAI GPT-6 in Codex. The exact deployment model ID and context-window size are not exposed in this session. Used reasoning, code editing, shell execution, and test tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
02a2299173 |
feat(codex): default Codex to gpt-6.1-sol, half the gpt-5.6-sol price (#19623)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Codex agents run through Paperclip Runner or the legacy
`codex_local` harness, and the runner E2E suites run paid Codex cells
> - The shared Codex default and many selections preferred `gpt-5.6-sol`
> - The reviewed OpenAI rate card in
`server/src/services/codex-pricing.ts` (`openai-standard-2026-09-30`)
prices `gpt-6.1-sol` at $2 input / $0.10 cached / $10 output per 1M
tokens, and `gpt-5.6-sol` at $4 / $0.40 / $20
> - So every default run and every paid Codex eval cell cost twice as
much as necessary
> - This pull request moves every place that prefers or selects
`gpt-5.6-sol` to `gpt-6.1-sol`, and keeps historical facts, rate cards
and compatibility fallbacks on `gpt-5.6-sol`
> - The benefit is half the token price for new default Codex agents and
for the Codex E2E and TestOps cells
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The default Codex model for new agents and the Codex model that the
runner E2E suites and TestOps plans select.
**Subsystem affected**
Adapters (`adapter-utils`, `codex-local`), server native runtime
fallback, UI AI connection pools, Paperclip Runner devtools and live
evals, runner E2E catalog and TestOps mappings.
**Current behavior**
New Codex agents without an explicit model store `gpt-5.6-sol`. The
runner E2E `runner-codex` profile, the Reliability `queue-steer` plan
cell, the Product regression TestOps mapping and the ACPX Codex
qualification selection all use `gpt-5.6-sol`.
**Proposed behavior**
They use `gpt-6.1-sol`. Existing agents that store a model keep it.
`gpt-5.6-sol` stays selectable.
**Reason and benefit**
`gpt-6.1-sol` costs half as much per token on the in-repo reviewed rate
card. The current OpenAI pricing page (retrieved 2026-11-06) shows the
same short-context rates and also lists long-context rates of $4 / $0.20
/ $15 for `gpt-6.1-sol` against $8 / $0.80 / $30 for `gpt-5.6-sol`.
**Breaking changes**
None for stored configurations. See Risks.
## What Changed
Changed defaults and selections (all now `gpt-6.1-sol`):
- `PAPERCLIP_RUNNER_DEFAULT_MODELS.codex` in
`packages/adapter-utils/src/paperclip-runner-permissions.ts`. This also
changes `DEFAULT_CODEX_LOCAL_MODEL`,
`resolvePaperclipRunnerModel("codex", …)`, runner config normalization,
`codex_local` → Paperclip Runner conversion, the UI new-agent and
agent-config defaults, the composer default model and the public MCP
effort lookup.
- `codex-local` `models`: the 5.6 entry is now the literal
`gpt-5.6-sol`, because it used to be `DEFAULT_CODEX_LOCAL_MODEL`. The
list order and contents do not change.
- AI connection pool: a new OpenAI member profile uses
`PAPERCLIP_RUNNER_DEFAULT_MODELS.codex` instead of a hard-coded
`gpt-5.6-sol`.
- Paperclip Runner devtools ACPX Codex default and preset, the
`runner-codex-sol` local provider smoke, and the `acpx-codex-sol` live
workflow candidate.
- `tests/acpx-qualification-models.ts` Codex selection (runner E2E and
runner acceptance ACPX Codex profiles).
- Runner E2E: the `runner-codex` profile follows the default, so all
native Codex cells move. This includes the `runner-codex` cells among
the 52 standard `everyday-workflows` cells and the seven Codex
reliability cells. `runner-codex-mini` stays on `gpt-5.4-mini`. The
Reliability plan `queue-steer` `steeringProfile` is now
`['runner-codex', 'gpt-6.1-sol']`. Its executionId
(`everyday-workflows.runner-codex.daytona.queue-steer`), its fullName
and its TestOps case identity do not change. No separate profile is
necessary.
- `product-regression-testops.mjs`: the declared model for every
`runner-codex` case is now `gpt-6.1-sol` (it was already that for
`configured-repository`).
- `tests/runner-e2e/installed-cli.ts`: the credential-free installed
startup proof opens a session on the default model.
- `tests/runner-e2e/scheduled-prices.json`: add a `gpt-6.1-sol` price
(`openai-standard-2026-11-06`, including 30m/12h cache writes and
long-context rates from the OpenAI pricing page). The weekly scheduled
Codex cells need an exact price for their model, or admission fails
closed.
- Storybook: the composer model picker fixtures, the connection pool
stories and host, and the agent settings preview show `gpt-6.1-sol` as
the default.
- Docs: `doc/DEVELOPING.md` (the Codex conversion default) and
`tests/runner-e2e/EVERYDAY-WORKFLOWS.md` (local matrix, reliability
extras, `queue-steer` row).
- Tests that assert the default now expect `gpt-6.1-sol`. New
assertions: the default has the `0.159.0` CLI floor, the compatibility
fallback is floor-free, the `queue-steer` plan cell runs `runner-codex`
on `gpt-6.1-sol`, and all native Codex everyday cells (standard and
reliability) use `gpt-6.1-sol`.
Deliberately kept on `gpt-5.6-sol`:
- Rate cards: `server/src/services/codex-pricing.ts` and
`packages/paperclip-runner/src/evals/model-pricing.ts`. Old receipts
need them. (The runner eval rate card has no `gpt-6.1-sol` entry. An
unpriced candidate is reported as unpriced, not as zero. I did not add a
rate there because that card has its own version and source.)
- The remote Codex CLI compatibility fallback. A new
`CODEX_LOCAL_COMPATIBILITY_FALLBACK_MODEL = "gpt-5.6-sol"` replaces the
default in `codexModelFallbackCandidates`. `gpt-6.1-sol` needs Codex
0.159.0 or newer, so it cannot be the floor-free last resort. Fallback
behavior is unchanged.
- `tests/runner-e2e/provider-usage.ts`: the implicit request
cache-policy attribution stays qualified only for `gpt-5.6-sol`. I did
not verify that policy for `gpt-6.1-sol`. A `gpt-6.1-sol` response that
omits the write-TTL split keeps its cache-write cost unknown, which is
the existing behavior for unqualified models.
- `tests/runner-e2e/scheduled-measurements.json`: historical
`gpt-5.6-sol` measurements. They no longer match the current suite
definitions, so the forecast reports them as historical only. The
forecast tests now build their synthetic cohorts on the current
definition hash.
- Model catalogs that list `gpt-5.6-sol` as available (`codex-local`,
`cursor-local`, `opencode-local`, Speko voice diagnostics, Pi picker
fixture).
- The `gpt-5.6` → `gpt-5.6-sol` alias for old stored configs.
- Dated or evidence docs: `doc/plans/*`, `doc/adapter-model-audit-*`,
`QUALIFICATION-*`, `doc/connections/MEMORY.md`,
`doc/connections/TASK-REVIEWS-VERIFICATION.md`,
`tests/runner-e2e/CHAT-FEATURES.md`, the Slack eval evidence, and the
S49 soak signature text (the soak failure happened on `gpt-5.6-sol`).
- Test fixtures that parse, price, migrate or round-trip an existing
`gpt-5.6-sol` value (Rust runner tests, driver tests, pricing tests,
stored-config tests, stories that show an existing agent).
## Verification
- `npx vitest run` on every server, UI, adapter-utils and codex-local
test file that mentions `gpt-5.6-sol` or the default, plus all of
`packages/adapter-utils` and `packages/adapters/codex-local`: 5,483
passed. Three timeouts under load (`sandbox-managed-runtime`,
`acp.quota`) passed when rerun.
- `packages/paperclip-runner`: `vitest run` for `src/eval`, `src/evals`,
`src/drivers/acpx`, `src/drivers/codex`, `src/cli`, `src/contracts`,
`src/backends`: 2,641 passed.
- `node --test tests/runner-e2e/reliability-testops.test.mjs
tests/runner-e2e/product-regression-testops.test.mjs`: 28 passed.
- `node tests/runner-e2e/run-unit.mjs`: all model, catalog, reliability,
scheduled and TestOps tests pass. Failures remain only in
`copilot-protection-flow`, `denied-target-isolation-flow`,
`pi-controls-flow`, `workflow-target` and `unit-launcher`. These five
also fail on unmodified master in this local environment (filesystem
watch and git environment), and they do not touch models.
- `vitest run --config tests/runner-acceptance/vitest.config.ts`: 19
passed.
- `pnpm test:e2e:runner -- --list --suite everyday-workflows --group
reliability`: the seven `everyday-workflows.runner-codex.daytona.<case>`
cells show `gpt-6.1-sol` with `OPENAI_API_KEY,DAYTONA_API_KEY`.
`--matrix-json` gives 16 reliability cells with the same credentials as
before. `--list --all` shows no `gpt-5.6-sol` cell.
- Typecheck: `adapter-utils`, `adapter-codex-local`, `server`, `ui`,
`paperclip-runner` (TypeScript), and `pnpm test:e2e:runner:typecheck`.
- `pnpm check:token-gates`: clean.
- Not run: paid cells, Rust tests (no Rust change), the browser suites,
and the full `pnpm -r typecheck` / `pnpm build` (CI runs them).
## Risks
- Agents without an explicit model change model on their next save or
normalization, and Paperclip Runner rows with a blank model use
`gpt-6.1-sol` at execution. Stored models, including `gpt-5.6-sol`, are
never rewritten. No migration is necessary.
- `gpt-6.1-sol` needs Codex CLI 0.159.0 or newer with ChatGPT sign-in.
The runner pins `@openai/codex@0.160.0`. On a remote target with an
older image CLI, the existing compatibility check falls back to
`gpt-6-sol` and then `gpt-5.6-sol`, and that probe now runs for
default-model agents too. A legacy `codex_local` agent created with the
new default on a host with an older Codex CLI gets the existing
actionable "upgrade Codex" Test result.
- Qualification: the native Codex profile uses `adapter_constant` model
qualification, so it follows the default. No qualification gate is
dropped. The ACPX Codex qualification selection moves to `gpt-6.1-sol`.
Its next qualification run uses the new model. The provider-usage
cache-policy attribution stays limited to `gpt-5.6-sol`.
- Runner E2E history: the Codex cells change their suite definition
hashes, so earlier `gpt-5.6-sol` results are a separate cohort and do
not count as current.
- The `gpt-6.1-sol` scheduled price uses the file's existing `expiresAt`
(2026-11-19). It needs the same review as the other entries before then.
- No `.github/` change. The Reliability queue-steer cell keeps the
`runner-codex` profile id, so the existing workflow credential and setup
steps still match.
> 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`, running in
Claude Code with tool use (shell, file edits, web fetch of the OpenAI
pricing page).
## 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
Related: #19604 (commits the Reliability TestOps identities; this PR
changes only `steeringProfile` in the same file).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Nicky LeachandClaude Opus 5.5
|
a7d3d4b78c |
fix(runner): rotate local and external authority through one resumable path (#19547)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The Paperclip Runner moves a native session from one run to the next
with an authority rotation, for Codex and for every ACPX provider
> - A rotation archives the prior run's controller state under
`authority-epochs/` and archives the Runner state, locally or on an
external Runner host
> - If the run fails after the rotation but before the new Runner
starts, retry preparation restores the prior run's controller checkpoint
into the session root
> - The retry then finds an active `control-plane` next to the archive
marker and fails with `native_runner_authority_archive_conflict`. Every
later run for the session fails the same way
> - The local and external rotations were two copies of the same steps,
with different retry rules, and the sandbox archive script had a third
set of rules
> - This pull request gives every provider and Runner placement one
resumable rotation path, with the same retry rules for controller state
and Runner state
> - The benefit is that one failed startup no longer makes a native
session unusable
## Linked Issues or Issue Description
No public issue exists. Bug description:
**What happened?**
A warm Codex session on an external (plugin) Runner failed its first
attempt after the authority rotation finished. The two automatic retries
both failed with `native_session_recovery_failed: Error:
native_runner_authority_archive_conflict`, and the run stopped. On disk,
the session root had
`authority-epochs/epoch-<key>/control-plane/control-plane-state.json`
and an active `control-plane/control-plane-state.json` with the same
identity (in this case, also the same sha256). The remote Launcher had
already archived the Runner state under the same key.
**Expected behavior**
A retry resumes the interrupted rotation. The rotation already supports
this when only the archive marker exists.
**Steps to reproduce**
1. Run a native Codex or ACPX session on an external Runner.
2. Start a new run for that session. Make the new Runner fail to start
after the rotation finishes.
3. Let the run retry. Retry preparation (`restoreWarmRunnerdController`
/ `restorePerTurnRunnerdController` →
`restoreNativeControllerProjections`) restores the prior run's
controller checkpoint into the session root.
4. The retry fails with `native_runner_authority_archive_conflict`. All
later runs for the session also fail.
The new `authority rotation after the prior controller state is
restored` tests reproduce steps 2–4 directly.
**Paperclip version or commit**
`master` at
|
||
|
|
2e35730637 |
feat(chat): receive sealed Slack events through the current SDK owner (#19019)
## Thinking Path > - Paperclip manages AI agents and their work. > - Slack connectors turn authenticated messages into company work. > - Direct callbacks need an awake tenant. > - Cloud can retain authenticated asynchronous events until the tenant wakes. > - Replay must preserve the original callback proof and current endpoint authority. > - Configuration changes must retain registration responsibility through failures. > - This PR enrolls verified Slack identities and drains sealed events through the normal SDK owner. ## Linked Issues or Issue Description **Subsystem affected** Server chat connectors and signed Cloud connector delivery. **Problem or motivation** Direct Slack Events API delivery depends on an awake tenant. Broker outages, lost responses and configuration changes must preserve delivery and registration responsibility. Enrollment failures must also leave other connector events able to drain. **Proposed solution** Stage nonsecret enrollment or retirement intent inside the local configuration transaction. Let the existing connection-event owner perform broker requests and retries. Replay sealed events through the current Slack SDK with their original authentication evidence. **Alternatives considered** An early acknowledgment for every callback changes command and modal semantics. A second delivery ledger duplicates existing ownership. Reusing a new stack identity to retire old registrations crosses an authorization boundary. A companion Cloud broker change is required. Related setup fixes: Refs #18837, Refs #18878. Kickoff searches found no competing async enrollment consumer. The ROADMAP's connector setup and access controls stay in place. ## What Changed - Persist enrollment and credential-free retirement intent before local configuration commits. Broker failure does not make a completed pause or removal look unsuccessful. - Bind company, endpoint, connection, public ID, generation and authorized actor. Preserve lost-response responsibility and recover only the supported same-instance configuration outage. - Permit an explicit move to a new authorized stack only after confirmed old retirement. Keep the old identity until the new broker receipt matches the current intent and generation. - Show permanent signing-credential failures as attention. Preserve transient retries and reject uncertain inbound authentication before SDK admission. - Drain events and acknowledgments before enrollment reconciliation. Persist a bounded retry on the exact busy intent so other due endpoints can enroll or retire. Preserve current custody under concurrent changes. - Stage retirement in the Apps removal transaction too. Archive the endpoint and connection, retain the original subject and instance, then revoke credentials. Retirement does not need the deleted Slack credential. - Acknowledge a malformed sealed Slack receipt as invalid once. Keep actual broker outages on the retry path. - Run the real configuration, SDK ingress, generation fences and public Apps removal paths in the default SQL suite. Keep Vitest transport helpers outside production services. - Preserve original callback bytes, signature, time and public URL evidence. Use the existing transactional chat-delivery ledger for redelivery. - Provide a checked-in prewire guard and launcher using the current Node executable. Add broker, worker, SDK, SQL and controlled-process acceptance. - Stabilize AI account summaries by ID and grant ID. This fixes a CI migration-repeat equality failure without changing SQL, authorization or reconnect behavior. - Open the four configured database connections before the enrollment test freezes its clock. Keep all 35 original assertions and all timeouts. - Measure status-card dispatch entry before its mock does database work. Keep the original strict 500 ms bound and check any still-armed earlier deadline. Bound that fixture's pool to four connections. ## Verification - Candidate: `54fa701d41b4ced62f4ff338d39b946015670ae3`. Normal merges preserve master `47027e7` and all owned changes. A read-only merge assessment against fetched master `cb5d7e44` is conflict-free. The only overlapping file has separate AI test additions. The branch has not merged that later master; fresh remote checks must validate the current PR test tree. - At this head, normal server production typechecking and declaration emission passed in 35.24s. The affected actual CI test shard passed in 6.27s. Other unchanged CI layers, 92 affected pure controls and 12 guard controls retain their source-matched results. - All **15 default real-PostgreSQL cases passed in 34.66s**, with no skips or filtering. They cover normal configure and SDK ingress, rollback, stale generations, malformed receipts, busy-lease fairness, concurrent intent custody, unknown errors, and public Apps removal. Removal tests check actual credential revocation, retained retirement through outage and repeat removal, one ordinary retry and confirmed retirement, transaction rollback and foreign-company refusal. - The public removal counterfactual failed before the fix: archival and credential revocation passed, but the inactive intent was absent. The same test source passed after the staging call. Separate counterfactuals also reproduced busy-endpoint starvation and malformed-receipt retries. Original failures remain retained. - The paired broker/SDK/process journey passed in 31.98s at ancestor `0324c011`. It preserves callback proof, lost-ACK redelivery, exactly one comment, one delivery and one successful controlled process. Its test, producer, SDK, guard and runtime bytes are unchanged. The new public Apps archive branch is not called by that journey; its evidence carries on that checked basis. - The new SQL cases need no companion Cloud checkout. Normal CI runs them with the repository fixture. For an existing dedicated PostgreSQL fixture, set `PAPERCLIP_SLACK_OWNER_TEST_DATABASE_URL` and `PAPERCLIP_TEST_POSTGRES_ADMIN_URL`, then run `node server/scripts/test-slack-cloud-owner.mjs` with Node24. The optional paired launcher still requires the normally built companion Cloud checkout. - Every pool was bounded to four or fewer connections. The final 15-case run sampled three runtime clients plus one observer. The independent final audit found zero clients and all five recorded processes absent. Earlier closing-socket observations are kept separate from final cleanup. - Earlier unrelated CI failures are documented: [enrollment timeout](https://github.com/paperclipai/paperclip/actions/runs/41944373937/job/126973483754) and [status-card observation bound](https://github.com/paperclipai/paperclip/actions/runs/41944373937/job/126973483843). The enrollment test retains all 35 assertions and its original timeouts. A controlled connection probe established the frozen-clock mechanism; the original remote access history remains unknown. The status-card fixture measures dispatch entry with the original strict 500ms bound. A real delayed-dispatch control fails that bound. - Full local workspace typecheck, build and test inventory was not run because its embedded-cluster and wider resource paths exceed this local lease. Exact-head remote CI now passes: 54 successful checks, three skips and Ponytail's neutral check, with aggregate verification green and the legacy security context independently confirmed successful. Both complete bot reviews are 5/5, with all 13 paginated review threads resolved. Seven execution jobs originally reported runner shutdown; one diagnostic rerun recovered them and the dependent aggregate check. Original logs remain retained, and the underlying shutdown cause is unknown. This is recovery evidence, not a product-fix claim. - Provider responses are controlled. This proves local SQL, broker, SDK and process behavior. It does not prove live Slack, agent tools, synchronous callbacks or deployed suspension. ## Risks The companion broker is required. Outage or conflicting authority can leave explicit pending or attention responsibility. A move between stacks requires confirmation from the old authorized owner first. Synchronous commands, actions, modal validation, options and challenges retain their deadlines and endpoint blocker. This slice grants no full Slack endpoint sleep exemption. No deployed suspension or savings claim is made. ## Model Used OpenAI `gpt-6.1-sol` workers through Codex, with reasoning, repository tools and code execution. The served backend revision and context limit are not exposed. A GPT-6-family coordinator performed technical review and integration. ## 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: Codex <noreply@openai.com> |
||
|
DottaandClaude Opus 5.5
|
876a2e4857 |
fix(recovery): recover a prior boot's unstarted run at lease expiry (#19578)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a server restart, a task must continue automatically and promptly when that is safe > - A user clicks Retry, and the server is killed before the new run starts a process > - Recovery leaves that run `running` until the orphan reap's five-minute stale-age threshold passes > - The user sees nothing happen for about five minutes > - This pull request recovers that run when its controller lease from the earlier boot expires > - The benefit is that the automatic retry starts about one lease period after the kill, not five minutes later ## Linked Issues or Issue Description **What happened?** A Retry admitted a legacy run. The server was killed before the run recorded a process. The run kept a controller lease from the killed boot and `executionStage: preparing`. After the restart, live recovery skipped the run until `updatedAt` was five minutes old (`server/src/services/heartbeat/recovery.ts`, `staleThresholdMs ?? 5 * 60_000`). Restart-chaos measured 5 m 01 s from run creation to `process_lost` and the automatic retry, twice (4 m 20 s after the server was ready). **Expected behavior** Recovery should claim the run as soon as the earlier boot's lease expires. The lease is the only liveness proof for a legacy controller, and no process was ever recorded. **Paperclip version or commit** `master` at `71313207eb`. **Steps to reproduce** Click Retry on an interrupted task, then SIGKILL the server within about one second. Observe the retried run stay `running` for about five minutes. ## What Changed - `recoverActiveExecutions` skips the stale-age gate for a legacy `running` run when all of these are true: its controller lease is expired (checked by the existing live-lease gate), the lease belongs to an earlier boot (`controllerBootId !== legacyControllerBootId`), the run is still `preparing` (no adapter dispatch), and the run has no recorded `processPid` or `processGroupId`. - The stage and process fields committed with the revocation are checked again. If the old controller dispatched before its lease expired, the run keeps the age guard. - The existing revocation (`revokeExpiredLegacyControllerWithState`) still requires the expired boot ID, so a concurrent renewal wins. The existing `restartInterruptedBeforeDispatch` rule still decides the single process-loss retry. - `doc/execution-semantics.md` describes the rule in section 10. - Two regressions in `server/src/__tests__/live-recovery-work.test.ts` (embedded Postgres, real residual-recovery workers). ## Verification - Red before the fix: `recovers a fresh run from a prior boot at lease expiry ...` failed with `expected 'running' to be 'failed'`. - Green after the fix: `pnpm exec vitest run src/__tests__/live-recovery-work.test.ts -t "stale-age|fresh foreign controller|suppressed live runs"` passes 4/4. The run fails `process_lost` with `executionRecovery.providerWorkStarted: false` within seconds of lease expiry, and exactly one retry runs to `succeeded`. - Race case: a DB trigger makes the row committed with the revocation say `dispatching` while the scan saw `preparing`. The run stays `running`, and no retry is created. - Guard cases: a run with an expired lease of the current boot, a run from an earlier boot with a recorded process, and a run from an earlier boot that was `dispatching` (for example a remote HTTP adapter) all stay `running` after four minutes. - Related suites: `heartbeat-process-recovery`, `heartbeat-observed-workspace-preservation`, `native-startup-cleanup-recovery`, `services/heartbeat/recovery.test.ts`, full `live-recovery-work`. - Live K8 probe (restart-chaos supervisor, local instance, ACPX Claude agent): the first run fails with a provider error, the probe clicks Retry, and the server is SIGKILLed 100 ms later, before the Retry run records a process. The probe then measures how long the Retry run stays `running`. | Build | Retry run created to `process_lost` | Server ready to `process_lost` | Automatic retry run | | --- | --- | --- | --- | | Base (`71313207eb`) | 300.1 s | 293.5 s | started at 5 m 00 s | | This PR | 60.1 s | 54.5 s | started 0.1 s after `process_lost` | The claim now happens when the killed boot's 60-second controller lease expires. In this probe the restart took about 6 s, so the claim came about 54 s after ready. In the restart-chaos waves, kill to ready took about 41 s, so the claim would come about 20 s after ready. - A chaos wave with the `retry` trigger (`w-20261106T195715-s2727`, branch #19513 merged locally) missed the trigger: no Retry was offered because recovery completed the task automatically (claude t01 PASS). The probe above makes the Retry-then-kill timing deterministic. ## Risks - Low risk. The change only removes an age wait. Ownership is still proved by the expired lease of another boot. A rolling deploy or hot restart whose earlier instance is still alive keeps renewing its lease, so the run is not touched. - Runs with a recorded process, and dispatched runs, keep the five-minute guard. A process on another host or a remote request is not inferred dead. - Native runs are not changed. Their ownership is in `native_run_finalizations` and startup native recovery. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
da9a4cb96c |
fix(native): retire a finished local Runner that stopped before suspending (#19597)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The native Runner path runs each per-turn agent turn through a
runnerd process, and the next turn must prove that the previous Runner
is finished before it takes over the saved session root
> - Under load, a local turn can have its result accepted while its
`runner.suspend` command does not finish, so the Runner's own receipt
stays unsuspended
> - The retirement proof that handles the same case for sandbox Runners
refused every local Runner with `target_not_sandbox_runner`
> - So the next turn failed with `runner_state_identity_mismatch:
prior_owner_terminal_state_indeterminate` and its session root was
quarantined
> - This pull request extends the retirement to a local Runner when the
prior Runner process is proven stopped by exact PID and start time
> - The benefit is that a follow-up turn (for example, after a user
answers a question) continues instead of failing
## Linked Issues or Issue Description
Refs #19525 (keeps the accepted result in the same incident; this is the
next run).
**What happened?**
In the runner E2E cell
`local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume`,
the first turn asked a question and its result was accepted. The Runner
then logged `runner did not prove durable suspension before checkpoint`
(`runner.suspend` pending, Runner lifecycle `ready`). After a server
restart and the answer, the next run failed with
`runner_state_identity_mismatch:
prior_owner_terminal_state_indeterminate;
retirement_refused=target_not_sandbox_runner`, and its root was
quarantined.
**Expected behavior**
When the prior local Runner is provably stopped and every other
retirement proof holds, the next run retires that owner as a continuity
break and continues. If the stop cannot be proven, the run keeps the
named rejection.
**Steps to reproduce**
1. Run a local per-turn native turn whose `runner.suspend` does not
finish after its result is accepted (host load average ~40).
2. Start the next turn for the same session.
3. See `retirement_refused=target_not_sandbox_runner`.
## What Changed
- `server/src/services/native-runtime/native-restart-recovery.ts`: add
`proveNativeLocalRunnerStopped`. It uses the existing restart-recovery
checks: the recorded Runner PID is dead or now belongs to a process with
a different start time; its recorded process group is not alive; each
recorded provider process is dead or recycled. A missing PID, start time
or process group, a live recorded process, or a start time that cannot
be compared is not proof. When settlement already cleared the run's
process fields, the saved `native.local_process_stopped` receipt is the
Runner proof. Extract `nativeProviderProcessIdentities` (checkpoint plus
run events) so restart recovery and this proof read provider PIDs the
same way.
- `server/src/services/native-runtime/native-session-executor.ts`
(`retireFinishedPerTurnRunnerdOwner`, `readSettledRunnerdProjection`): a
local target is now eligible. For a local Runner, the projection accepts
the Runner's own unsuspended receipt for the same identity and
unfinished Runner lifecycle commands (`runner.suspend`, `runner.drain`,
`runner.shutdown`) only. All other proofs are unchanged: committed
result, released lease, no recovery state, stopped controller, delivered
terminal, other commands complete, tool inputs answered, quarantine
copies. The process proof runs before retirement and again just before
the archive.
- `doc/execution-semantics.md`: describe the local case.
- Tests: seven integration cases in
`native-recovered-owner-followup.test.ts` and ten unit cases in
`native-restart-recovery.test.ts`.
## Verification
- Red before the fix: the four new integration cases failed with
`retirement_refused=target_not_sandbox_runner`, which is the incident
error.
- Green after the fix: `pnpm exec vitest run
src/services/native-runtime/native-recovered-owner-followup.test.ts`
35/35. The new cases prove that a stopped Runner is retired (root
archived under `continuity-breaks`, no quarantine), and that a live
recorded Runner (`runner_process_alive`), a missing identity
(`runner_process_identity_missing`), and a pending non-lifecycle command
(`journal_commands_unsettled(turn.start.pending)`) keep the rejection.
All existing sandbox cases still pass, including "keeps an unsuspended
Runner receipt decisive" for sandbox targets.
- `native-restart-recovery.test.ts` unit cases: recycled PID, live
process, unknown start time, live group, live provider, missing PID or
start time.
- `native-session-executor.test.ts` + `native-restart-recovery.test.ts`:
1190 pass. `native-runner-restart-recovery.integration.test.ts` +
`native-restart-recovery-isolation.test.ts` (shared provider-PID
extraction): 81 pass. `tsc --noEmit -p server/tsconfig.json`: pass.
- Live: `pnpm test:e2e:runner -- --max-parallel 1
--max-automatic-retries 0 --id
local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume`,
3 runs at
|
||
|
DottaandClaude Opus 5.5
|
586caa5f49 |
fix(native): hand a shutdown-interrupted startup to the next controller (#19596)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native Runner path runs an agent turn through a runnerd process that the control plane owns > - A restart of the control plane must keep the task going without a click, if that is safe > - A graceful shutdown closes the HTTP listener first and then drains the heartbeat runs > - A native run that was between provider dispatch and runnerd backend setup in that gap failed with `local_runtime_api_listener_unavailable`, so the drain did not see it as running and did not suspend it for the next boot > - This pull request makes that failure a restart detach: the run stays running with its claim, the drain suspends it, and the next controller adopts it > - The benefit is that a SIGTERM in this window no longer spends a failure and a same-run retry, and the task continues by itself ## Linked Issues or Issue Description Refs #19530 (the same restart window, before provider dispatch). **What happened?** In a restart chaos replay (ACPX Claude, local runner), a restart-recovered run got SIGTERM while `createRunnerdBackendWithinSessionClaim` built the runnerd backend. `resolveLocalRuntimeApiUrl()` threw `local_runtime_api_listener_unavailable` because shutdown had already closed the listener. The run settled `failed`. The drain then logged `restartSuspendedRunIds: []`. The next boot used the one bootstrap retry. A second SIGTERM hit the same window, and the task stopped at a manual Retry. **Expected behavior** A run that this process cannot continue because it is shutting down is handed to the next controller. It is not settled as failed. **Steps to reproduce** 1. Start a native Runner task and restart the server (K1) so that startup recovery resumes the run. 2. Send SIGTERM while the resumed run sets up its runnerd backend. 3. See the run fail with `local_runtime_api_listener_unavailable` and the drain report no suspended runs. ## What Changed - `server/src/runtime-api.ts`: `resolveLocalRuntimeApiUrl()` throws the new `LocalRuntimeApiListenerClosedError` when this process had a listener and closed it (the `listening` flag is false or the `close` event fired). The error message stays `local_runtime_api_listener_unavailable`, so the existing classifiers do not change. A listener that never started still throws the plain error. - `server/src/services/native-runtime/native-session-executor.ts`: for a local Runner, the startup failure handler treats `LocalRuntimeApiListenerClosedError` like a restart detach. It stops lease renewal, keeps the claim and throws `NativeControllerDetachedForRestartError`. Heartbeat already keeps such a run `running`. A remote Runner keeps the same-run bootstrap retry from #19462, because startup recovery has no process evidence to adopt a claimed remote startup (`native-runner-restart-preparation.integration.test.ts` still covers that path). - `doc/execution-semantics.md`: describe the new rule. - Tests: a heartbeat regression that closes the real runtime API listener before backend setup, then drains and restarts; a unit test for the closed versus never-started listener; two `vi.mock` factories keep the real module exports. ## Verification - Red before the fix: `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t listener` failed with `restartSuspendedRunIds: []` and the run `failed` (`local_runtime_api_listener_unavailable`, `bootstrap_retry`). - Green after the fix: the same test passes. The drain suspends the run, the run stays `running` with `awaiting_runner_reattach`, and the next controller claims it and reaches the backend once. - `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts src/http/server.test.ts src/__tests__/server-startup-feedback-export.test.ts src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-recovered-owner-followup.test.ts`: 1333 tests pass. One timing test in `native-session-executor.test.ts` failed once under host load average ~30 and passed alone. - `pnpm exec tsc --noEmit -p server/tsconfig.json`: pass. - Live, restart chaos harness, local ACPX Claude (`claude-sonnet-5`), `--rc06-masked`, this branch at |