mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
0417bfea376d7bbf3ec0e0aeb9a82b465f80ecd8
7047
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0417bfea37 |
fix(environments): guard the atomic delete for kept restore sources; tell the operator what deleting them loses
Review follow-up: removeIfDeletable's atomic guard now uses the same kept-sandbox policies as the blast radius and the teardown (one KEPT_SANDBOX_LEASE_POLICIES constant). Each blast-radius holder carries its leasePolicy, and the delete dialog and refusal text say when a holder is a failed run's workspace copy whose files the consented delete destroys, instead of promising a fresh sandbox. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
cc02778296 |
fix(environments): tear down retain-on-failure sandboxes when their environment is deleted
A retain_on_failure lease keeps a failed run's sandbox stopped as a workspace-restore source. Environment deletion counted and destroyed only reuse_by_environment leases, so these sandboxes neither blocked the delete nor were destroyed by it: three survived the Daytona warm wave's environment delete. Count them in the delete blast radius and destroy them with the same consented teardown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
a65626a2a2 |
test(chaos): fail waves on a needed Retry and on speed regressions (outage removed); add M1 multi-agent workload (#20071)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness (tests/restart-chaos) kills and restarts the control plane during real agent tasks, and grades every task on reliability rules R1–R6 plus a task oracle > - Dotta's bar for today's campaign: restarts must continue on their own (Retry is only a fallback), the system must stay fast (speed budgets in `native-speed-budgets.ts`), and realistic scenarios must be covered > - The harness graded a task that needed a user Retry as a pass, graded speed only in crash-point mode, and counted a K4 controller hold as slow TTFT. It also had no multi-agent scenario > - This pull request closes those four gaps > - The benefit is that every chaos wave now fails on a needed Retry and on a real speed regression, but not on the fault's own outage ## Linked Issues or Issue Description Refs #19831 (durable ownership plan) and the restart chaos plan (`doc/plans/2026-11-06-control-plane-restart-chaos.md`). Used by today's campaign waves: - w-20261108T134237-s9401 re-graded to 104/107 under the Retry rule; - w-20261108T153612-s9401: 100/101, with a Speed section showing 6 breaches after outage removal. ## What Changed - **R3 Retry rule** (`grader.ts`): a task that needed a Retry fails R3 as `retry_needed (<failed runs' error codes>)`, and a second click fails as `retry_needed_<n>_clicks`. The harness still clicks Retry so the task can finish and be graded. - **Per-wave speed grade** (new `wave-speed.ts`, wired in `main.ts`): - It reads every task run's run log through the API and reports TTFT (from run created) and agent boot p50/p95 per adapter and start kind, restart recovery (ready → the run's next provider event), and single-run `run.performance.budget_exceeded` events, against the local budgets. - A breach fails the wave. - Output goes to `speed.json` and a Speed section in `SUMMARY.md`. Remote environments report numbers with no budget. - **Outage removal**: each performance span is shortened by its overlap with fault outage windows (fault start to replacement server ready) before it is judged. The summary reports how many spans and how much time were removed. - **M1 workload** (multi-agent delegation): - The parent task creates two subtasks, part 1 delegated to the other Chaos Lab agent. - Each subtask saves a marked document, and the parent writes a summary of both before it completes. - The observer collects the subtasks and their documents, and the oracle checks the parent links, assignees, statuses and markers. ## Verification - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts`: 14 files, 163 tests pass. New tests cover the M1 grade, the R3 Retry rule (one click and two clicks), the wave speed grade (budget judgement, restart recovery, remote no-budget, fault→run attribution) and outage removal. - `node scripts/typecheck-test-projects.mjs tests/restart-chaos` passes. - Real waves: - The R3 rule re-graded w-20261108T134237-s9401 to 104/107. - w-20261108T153612-s9401 produced the Speed section: 923 spans adjusted, 2,452 s of outage removed, 6 breaches. - M1 has not yet run in a live wave. It's next in the campaign rotation. ## Risks - Waves that passed before can now fail: on a needed Retry, or on a speed breach. That is the intended bar. - Speed is judged at wave concurrency on a shared host, which is noisier than the idle baseline the budgets came from. Breaches are reported per row, so a reviewer can see the margin. ## 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>canary/v2026.1108.0-canary.19 |
||
|
|
c55af26821 |
fix(runner): treat a replayed start of a turn lost on restore as history (t48) (#20074)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run must survive a control-plane restart and continue its work > - #20057 made recovery adopt an in-flight turn, or start a new turn when the restored runner reports the old turn lost > - One case was not covered. A graceful drain can detach the controller after the runner accepted `turn.start` but before the provider started the turn > - The runner then records the provider's `turn.started` while no controller is attached. After a restart it replays that start, and the Codex session rejects it as an unexpected turn > - This pull request treats that replayed start as history from the earlier generation > - The benefit is that the run starts its continuation turn instead of stopping with `native_event_replay_conflict` ## Linked Issues or Issue Description Refs #20057 **What happened?** A restart chaos wave ended with one failed task (t48, Codex). A graceful SIGTERM suspended the controller 300 ms after the runner accepted `turn.start`. The provider's `turn.started` came after the controller detached. Recovery classified the run as `resume_dead_runner`. The restored runner reported the turn as `provider_turn_lost_on_restore`, which is correct. But it also replayed the undelivered `turn.started`. The Codex session failed with "Provider started an unexpected turn." (`turn_binding_mismatch`), and recovery stopped with `native_event_replay_conflict`. **Expected behavior** Recovery starts a new continuation turn. The replayed start of the lost turn is recorded as history, and the run completes. **Steps to reproduce** 1. Start a Codex native run. 2. Detach the controller gracefully after the runner accepts `turn.start` and before the provider emits `turn/started`. 3. Let the runner record `turn.started` while it is unattached, then kill the runner and the provider. 4. Restore the run. Before this change, the session fails with `turn_binding_mismatch`. **Paperclip version or commit** master at |
||
|
|
e8a3f1f6f2 |
feat(recovery): prove a replaced container's records gone through a fenced controller epoch (#20054)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane settles in-progress records (leases, claims, intents) only once it proves their owner process is gone > - The proof compares host and PID space, so a stamp from a replaced container or a rebooted machine stays unprovable forever, and its records never settle > - Self-hosted users on Docker, Kubernetes and Fly hit this on every redeploy; a Railway deployment with an exclusive volume is covered by `deployment_replaced` (#20026) > - This pull request adds a fenced controller epoch: each boot holds a database lease, and an expired boot is fenced so its records can be proven gone > - The benefit is that work stranded by a replaced container is recovered, without keeping an idle database awake ## Linked Issues or Issue Description Follow-up to #20026 and the Phase 2 durable-ownership data. Related: #19965 (owner stamps), #19883 (process space). No duplicate PR was found. **What happened?** A record stamped by a server process in another container reports `different_process_space`. A dead PID elsewhere is never proof, so the record stays `unprovable` and is never settled. Every redeploy of a self-hosted container leaves such records behind. **Expected behavior** A record owned by a server boot that was replaced (stopped cleanly, or lost its lease and was fenced) is proven gone and settled exactly once. A live boot's records stay with it. An idle server issues no recurring queries. **Steps to reproduce** 1. Run Paperclip in a container and start work, so an environment lease is stamped by that boot. 2. Replace the container. The new container has a new PID namespace. 3. The ownership reconciler reports `different_process_space` (unprovable) for the old lease, forever. **Paperclip version or commit** `master` before this PR. ## What Changed - New `controller_instances` table (migration 0449): boot id, epoch (bigserial), process space, pid, a lease on the database clock, `fenced_at` / `fenced_by_epoch`, `stopped_at`. - `controller-instance-lease.ts`, registered with the shared delivery-work coordinator (`controller-instance-lease` queue, runs during drain): - **Holds work → renews; idle → silent.** One upsert creates the row on first use and extends the lease only if the boot holds work: a stamp within the last 90 s, or a stamp whose ownership write may still be pending (held until the server's tracked work settles, capped at 10 minutes; a renewal that sees some other held record does not clear it, so a slow ownership write cannot be outlived), or an in-progress record held by its boot id. Those records are active environment leases, in-flight cleanup claims, sandbox creation intents, workspace finalize attempts, agent lifecycle steps, native finalizer and cleanup leases (`lease_holder` bound to the token), open native coordinator claims, and running legacy runs. Renewal runs every 30 s while the boot holds work. When it finds none, the lease lapses and the worker sleeps until the next stamp (`onControllerStamp`). - **No idle trailing statements.** When the server's last tracked work settles, the worker re-checks holdings at once, at most every 5 s (the rate limit also absorbs the worker's own run). So the last check happens with the work, not 30 s later. - **Quiesce, don't crash.** Each renewal has a 10 s bound. A failed or timed-out renewal applies an internal task drain, so no new work is dispatched. A recovered renewal resumes. The boot exits only after 60 s of failure (counted from the first failed attempt's start) while it still holds work, or at once when it finds itself fenced. - **Fence on demand, by a newer epoch.** For a stamp from another host or PID space, the prover reads that boot's row: - `stopped` or `fenced`: gone. - live lease: alive. - otherwise, the boot's last sign of life is the later of its lease expiry and the record's last write plus the TTL. Within 30 s of that, the owner stays `alive` (`controller_lease_grace`), so the settle watch keeps rechecking. After that, the prover fences the boot, but only with a lease row of its own (created on demand) and only an older epoch. - A graceful shutdown records `stopped_at`. - A reconciliation pass that settles nothing (`record: false` or `settle: false`) never fences and never creates the boot's row. - `owner-proof.ts`: these proofs apply only to server-held records. A stamp with a recorded process group stays unprovable, because agent processes can outlive their server. The reconciler passes each record's `updatedAt` to the proof. - `task-admission.ts`: - the quiesce drain never replaces an operator's drain, and has no idle owner, so HTTP stays open; - a `subscribeIdleWorkSettled` hook fires when the last tracked work finishes. - Off on a Railway deployment with an exclusive volume (it has its own replacement proof). `PAPERCLIP_CONTROLLER_INSTANCE_LEASE=on|off` overrides this. A stateless Railway service keeps the lease. A server without its own lease row never fences. - `maintenance-server-routes.test.ts`: in the post-run quiet window, the lease may check once more that the run's records were released. The test now excludes those statements, then asserts that the lease issues nothing in a further window. - Docs: `doc/execution-semantics.md`; the Phase 2 data doc records the two known gaps. ## Verification - `controller-instance-lease.test.ts` (15 cases): - a replaced container with all Railway variables empty is fenced and settled exactly once over two passes; - a live lease is alive; - an owner in grace, and work written after an idle lapse, stay alive and watched; - an older epoch never fences a newer one; - no fencing without its own lease row (off or unregistered), and an idle prover creates its row on demand; - a stopped boot is gone; - process-group stamps stay unprovable; - idle issues zero statements, and one upsert renews only while records are held; - **a 45 s outage:** quiesced, no exit, resumes; - **a renewal that never answers:** bounded, and the boot exits on time; - **a 70 s outage:** exits at 60–65 s, and the successor recovers the work exactly once; - fenced: stops; - the settle re-check; - graceful stop; off on a volume-exclusive Railway deployment, on for a stateless one, and the on/off override; - a running legacy run keeps the lease live. - Red, with the proof's instance check removed: the replaced-container, live, stopped and successor cases fail. - Wider: task-drain, idle-sleep, idle-local-work, owner-proof and ownership suites, plus the maintenance quiet-window tests. Server low-memory typecheck is clean. ## Risks - **Clock skew.** The proof compares a record's `updatedAt` (often the writer's clock) with the lease (database clock). The 30 s grace absorbs ordinary skew. - **A new exit path exists.** A server whose database is unreachable for more than 60 s while it holds work now exits, after first quiescing. This is the intended safety backstop. - **Bounded trailing activity.** A lease that holds work issues one statement every 30 s, plus at most one check when its work settles. An idle server issues none. - **Boots from before this release** have no row, and their records stay unprovable. The board "mark owner gone" action for those follows in a separate PR. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
0a64c9b4ab |
fix(recovery): treat a warm sandbox as abandoned only on positive evidence its task moved (#20070)
## Thinking Path - #20065 made idle warm sandboxes owned records on the shared retained-orphan kind. A warm sandbox no turn can resume is stopped if running, then destroyed. - One branch of that predicate decides that a task "moved" off a workspace. As merged, it was true whenever no task was bound to the lease's workspace. That includes a task whose binding is null: finished, or between bindings. - A null binding is not proof of a move. The warm sandbox may still be resumable, and for a remote workspace it can hold the files. Phase 2's review flagged this as blocking, so this follow-up requires positive evidence. ## Linked Issues or Issue Description Refs #20048, follow-up to #20065 **What happened?** `taskMovedToAnotherWorkspace()` listed a warm lease as abandoned when its task had no bound workspace. The pending-export check was then the only thing preventing the destroy of a sandbox that could still be resumed. **Steps to reproduce** 1. Create a warm (`reuse_by_environment`) lease, released with a confirmed stop, bound to workspace W. 2. Clear the task's `execution_workspace_id` (task finished, or between bindings). 3. Run an ownership reconciliation pass: on master the lease is settled and queued for destruction. **Expected behavior** Only positive evidence counts as a move: the lease's own task is bound to a different, non-null workspace, and no task is bound to W. An unbound task's warm sandbox is kept; a done or cancelled task's sandbox goes through the existing retention paths. ## What Changed - `server/src/services/environment-retained-orphan-settlement.ts`: - `taskMovedToAnotherWorkspace()` now requires that the lease's own task is bound to a non-null workspace different from the lease's, and that no task in the company is bound to the lease's workspace. - `newestRowForSandbox()` breaks `created_at` ties by id, so two rows written in the same millisecond can't both count as newest. - `server/src/__tests__/environment-retained-orphan-sandbox.test.ts`, two keep cases: - the task has no bound workspace, for both a released and a retained warm lease; - two rows share the newest `created_at`. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts`: 32/32 pass. - With master's settlement file, the null-binding case fails, because the lease is settled. The tie case also passes on master, since the settle's existing shared-resource hold protects it; it stays as a regression test for the listing. - The full chain (retained, stop confirmed, listed, handed to destruction) is covered in one test from #20065. ## Risks - **Fewer sandboxes cleaned.** A warm sandbox whose task finished unbound now waits for the existing retention paths instead of this branch. That is the conservative direction, so files are never lost. - **Scope.** No change to the holds or the settle contract. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
f679cf81dc |
fix(runner): refuse a finish or block decided before earlier results reached the agent (#20069)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Restarts must be invisible to a task: an agent that was working keeps working after the control plane comes back > - Restart chaos t03 (Daytona warm wave, K4 holds) ended a task as blocked even though its document was saved > - The cause is in how a terminal decision (paperclip_block or paperclip_finish) is handled after a long controller outage, not in how the Runner journals tool calls > - This pull request refuses a terminal call that the agent made before it had seen results that already returned, so the agent continues with those results > - The benefit is that a controller outage no longer turns into a false "blocked" task ## Linked Issues or Issue Description Restart chaos t03, wave `w-20261108T142906-s9501` (Daytona, codex, A1, three back-to-back K4 holds of 136 s, 252 s and 152 s). Refs #19831 (durable ownership plan). **What happened** The kept instance's run log shows the order of events: 1. The turn is accepted at 14:29:57. The runner then delivers nothing to the controller until 14:39:27. The holds keep the controller down, and the Runner keeps Codex's `item/tool/call` requests until it is reattached. 2. During that outage Codex keeps running. Its `write_document` call stays pending. The model is sampled again about once a minute and says "The save request is also waiting on Paperclip". It then calls `paperclip_block`, with the reason `tool_unavailable`. 3. After the reattach the Runner delivers the backlog in order. `write_document` runs and returns (the document is created: sourceSeq 152). Then the stale block arrives (sourceSeq 247) before Codex has acknowledged the write result: there is no `tool.execution.completed` for it before the block. The task ends blocked. **Codex behavior, checked directly** against the pinned Codex 0.160 app-server. A local probe sent one dynamic tool call and left it unanswered: - Codex does not time out. It re-samples the model at about 60 s and 127 s ("The save call is still waiting to return"). - A reply sent 130 s later is accepted (`durationMs` 130003), and the model finishes ("DONE"). So the reply is not lost. The agent's terminal decision was made without it. **Expected behavior** - A terminal call decided while earlier Paperclip results were still unseen is refused with a retryable error. - The agent continues with those results. In t03 it would see that the document was saved, and finish. ## What Changed - `packages/paperclip-runner/src/live/terminal-call-freshness.ts`: `unseenReturnedToolCalls` reads the turn's committed run events up to the terminal call. It returns each dynamic call that has a returned `tool_result` but no provider `tool.execution.completed`. `staleTerminalCallResult` builds the retryable `terminal_call_before_results` error, which names those calls. - `runnerd-codex-transport.ts` `#handleSemanticToolInput`: when such calls exist, `paperclip_finish` and `paperclip_block` get that error instead of being dispatched. The recovery path after a restart uses the same handler. A long-running call with no result yet does not count. - Crash point `runner.semantic_tool.after_input_before_dispatch`, just before the controller dispatches a dynamic tool call. A crash-point hold (`--hold-ms`, `--crash-point-hold-ms`; a K4-style receipt) keeps the controller down after the self-kill. Together they reproduce t03 on Daytona. ## Verification - Red/green: `cd packages/paperclip-runner && npx vitest run src/live/runnerd-codex-transport.test.ts -t "refuses a terminal call decided"`. - The case `seen: false` failed on master: the handler was called and the stale block was dispatched. It passes with the fix. - The case `seen: true` (the agent saw the result first) dispatches normally, both before and after. - `npx vitest run src/live/runnerd-codex-transport.test.ts`: 289 passed. `src/live/terminal-call-freshness.test.ts`: 5 passed. - `npx tsc --noEmit -p packages/paperclip-runner` passes, and `node scripts/typecheck-test-projects.mjs tests/restart-chaos` passes. - Shared registry: 15 passed. Call-site test: 3 passed. Chaos runner tests: 12 passed. - Codex probe (local, about 2.5 min per run): reproduced the "still waiting" re-sampling and the accepted late reply described above. - Not yet run: the Daytona crash-point repro with a hold. It needs the shared harness slot, and I'll add the result here. The fix is controller TypeScript, which is not an input of the Daytona image, so the current image still matches. ## Risks - A refused terminal call costs the agent one more turn step. It applies only when a result returned that the agent had not seen, which in practice happens only after a controller outage. - The check reads committed run events. If the store evicted the earlier call's events, it finds nothing and keeps today's behavior. - Upgrade-safe run state: no persisted shape changes. The check reads events the Runner already commits. A run saved by the previous release resumes with the same history and the same answer. - ACPX and other providers emit no dynamic `tool.execution.*` events, so the check is a no-op for them. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>canary/v2026.1108.0-canary.18 |
||
|
|
a9cf4ee731 |
fix(outbox): deliver a user comment whose wake was lost on a task its agent then closed (#20066)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user's comment on a task with a live run must reach the agent: it is steered into the run or delivered in a follow-up run. Per-comment exactly-once delivery is plan invariant 4 > - A comment commits before its wake is written. A control-plane restart in between loses the wake, and the reconciler's lost-wake rule (chaos s9401/t69) saves the missing outbox entry > - That rule only looked at `todo` and `in_progress` tasks. In the restart chaos Daytona wave s9501, task t06 got a comment 10 ms before a K1 kill. The live run then finished, its agent closed the task, and the comment was never read > - This pull request extends the rule to a task its agent closed, and reopens that task for the user's message, as the release drain already does > - The benefit is that a message sent while the agent works is never silently dropped by a restart ## Linked Issues or Issue Description Refs #19831 (plan, section E: work-intent outbox, invariant 4), and #19992 (the s9401/t69 and t72 rules this extends). Found by the restart chaos campaign, wave w-20261108T142906-s9501, task t06: - The comment was saved at 14:40:17.048. - The K1 kill came at .058. - No wake request ever carried the comment, and no run event shows it steered in. - The run that started before the comment finished at 14:44:22 and closed the task. ## What Changed - `recovery/ownership-invariant-report.ts` (`checkUserCommentsWithoutWake`): also reports a comment on a `done` task its agent closed (`taskClosedByAgentSql`), when no wake carries it, no run started after it, and no run of the task logged it as a steered `userMessage` item. - `work-intent-outbox.ts` (`saveUndeliveredUserComments`): a new `reopenAgentClosedDone` option. In the same transaction that locks the issue, re-checks the agent close and saves the receipt, it reopens the task to `todo` through the issues service. It also logs `issue.updated` (reopened from done, source `undelivered_comment_backstop`). - `native-finalization-reconciler.ts` (`applyOwnershipReportBackstops`): the lost-wake backstop allows `done` and passes `reopenAgentClosedDone`. The receipt keeps its `wake_lost` / `reconciler_backstop` wait, so the existing release path delivers it once. ## Verification - `heartbeat-held-comment-wake.test.ts`, new test "delivers once a comment whose wake a restart lost mid-run on a task the run then closed". It fails on `master` and passes here. The violation is found, the backstop saves and reopens, the comment is delivered exactly once, and a second pass and backstop deliver nothing more. All 25 tests in the file pass. - `ownership-reconciliation-report.test.ts`, new test "finds a user comment that a restart left without a wake on a task its agent then closed". It fails on `master` and passes here: only the lost comment is reported, not one steered into the run and not one on a task a person closed. All 26 pass. - The server package typecheck passes. ## Risks - **Reopening a done task.** This only happens when the latest recorded close was not made by a person, the comment is a direct human comment, and no run received it or had it steered in. That's the same rule the t72 stale-continuation path and the release drain already apply. - **The steer check** joins the task's runs to their `item.started` events by comment ID. It runs only for candidate comments within the lookback window. ## 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> |
||
|
|
3448c7f9ba |
fix(recovery): own abandoned warm sandboxes on the retained-orphan path (#20065)
## Thinking Path - Paperclip runs agent tasks in remote sandboxes (Daytona). With a warm lifecycle, a sandbox outlives its turn so the next turn on the same workspace resumes it instead of provisioning and uploading again. - #20048 makes warm the default on remote sandboxes. Its merge condition: an idle warm sandbox must be an owned record, adopted by its successor or destroyed or suspended once its owner is proven gone, with zero orphans in a Daytona chaos wave. - That wave (warm environment, K1/K2/K4 faults) left sandboxes alive behind dead lease rows. Reading the wave's database shows three warm-specific shapes that nothing ever tears down: 1. **A Retry moved the task to a new workspace.** `retainedIssueWorkspaceForRetry` reuses only local leases, so a remote task always gets a fresh workspace. Reuse matches only the exact workspace, so the old sandbox is unreachable: `released` and stopped (wave t05), or still `retained` and running. 2. **A failed warm lease.** The plugin stopped the sandbox for reuse, but reuse never resumes a `failed` row (wave t12). 3. **A warm sandbox kept running (`retained`).** Its idle timer lived only in the process that kept it. If that task moves on, nothing stops the sandbox, even after a restart. - The shared ownership path already has a kind for exactly this lifecycle: `environment_retained_orphan_sandbox` destroys a stopped, retained sandbox once nothing references it, but it skipped warm leases on purpose. This PR extends that kind instead of adding a new sweep. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T142906-s9501: - t05: run 8758207a's sandbox was resumed by c99b3a2e and released stopped in workspace 35a678f0. The Retry kept a fresh workspace (8ac860c3) and provisioned a new sandbox, and the old one stayed forever. - t12: run f2bf2f05's lease on sandbox 6d4c91fb was released as `failed` with the sandbox stopped, and the next run on the same workspace provisioned a new one. Both sandboxes were still alive when the wave ended. **Steps to reproduce** 1. Use a sandbox environment with warm reuse (`reuseLease: true`). 2. Complete a turn so the warm lease is kept, retained or released and stopped. 3. Make the next run fail so the Retry keeps a fresh workspace (always the case for a remote task), or so the lease is released as `failed`. 4. The next run provisions a new sandbox; the old one is never resumed or destroyed. **Expected behavior** A warm sandbox that no turn can resume is an owned record. Once its owning run is terminal and nothing references it, the ownership reconciler stops it if it is still running, then destroys it. A sandbox its task can still resume on the same workspace is left for reuse. ## What Changed - `server/src/services/environment-retained-orphan-settlement.ts`: the listing (`retainedOrphanSandboxCondition`) now has two warm branches, each on the newest lease row for its sandbox: - `abandonedWarmSandboxCondition`: a `reuse_by_environment` lease with a confirmed `stopped` receipt for this lease and run, either `failed`, or `released` while the task's bound workspace differs from the lease's. The settle hands it to the cleanup sweep through the existing stopped-cleanup contract, so the old run's successors are never blocked on the destroy. - `supersededRunningWarmSandboxCondition`: a `retained` (still running) warm lease whose task moved workspace. The settle queues a stop-only intent. When the provider confirms the stop, the row becomes `released` with a receipt, and the first branch destroys it on a later pass. - The settle re-checks the same predicate under the row lock. Every existing hold still applies: the run still active, open runs or recovery actions on the issue, pending exports, stop or restore intents, or another live lease on the same sandbox. - `server/src/services/sandbox-stop-and-retain.ts`: new `prepareSupersededWarmSandboxStop`, which prepares the existing stop-only intent from `retained`. The intent carries the recovery marker, so the sweep dispatches its first attempt at once. - `server/src/__tests__/environment-retained-orphan-sandbox.test.ts`, new tests: - a superseded released lease is destroyed; - a failed warm lease is destroyed; - a running superseded lease is stopped, then destroyed; - keep cases: the task can still resume it (released, and retained after a restart), no stop receipt, a newer row on the same sandbox, a run in flight on the task. The positive tests fail on master and pass with this change. **Previous-release state:** the conditions read existing columns only (status, lease policy, workspace binding, stop receipt). Rows the previous release saved are judged as-is. The tests build them in exactly the shape that release writes (no new marker), so old-saved state is handled by the new code without migration. Rows without a confirmed stop are never listed. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts`: 29/29 pass. With the two service files reverted to master, the 3 positive tests fail and the other 26 pass. - Neighbouring suites `environment-run-lease-ownership`, `heartbeat-pending-cleanup-sweep`, `ownership-reconciliation-report`, `heartbeat-orphaned-active-lease-sweep`, `environment-pending-cleanup-queue-owner` and `retained-native-session-cleanup-work`: 150/150 pass. - The wave's lease rows were read from the run's kept database copy to confirm the shapes above. - Not yet covered: the same Daytona warm wave, rerun with this fix. #20048 merges only at ≥11/12 with zero orphans. ## Risks - **Destroying a sandbox someone can still use.** Only the newest row for a sandbox is judged. `released` and `retained` rows qualify only when the task's bound workspace differs from the lease's, which is the exact rule reuse matches on. All existing holds still apply, and a stop must be provider-confirmed before any destroy. - **Unsaved work.** Pending native exports and stop or restore intents hold the sandbox. A stopped sandbox is destroyed only after its owning run is terminal and nothing on the task is still running. - **Blocking a successor.** The destroy uses the stopped-cleanup contract (`workspaceRecoveryCleanup` with the stop receipt), which `hasPendingRunCleanup` treats as resource cleanup. This is covered by a test. - **Out of scope.** A `retain_on_failure` lease kept for workspace restore (wave t12, a095e0b5) stays with restore recovery, which owns its own cleanup. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
0746d1203d |
feat(recovery): let the board confirm that an unrecorded owner stopped (#20067)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery releases an in-progress record only once its owner is proven gone, never by age > - A run lease saved before owners were stamped has no owner Paperclip can judge on an ephemeral (multi-host) server, so it stays stuck and holds its run's sandbox forever > - Only a person can tell that the previous server stopped (for example, its container was replaced) > - This pull request asks the board on the stuck task and lets an owner confirm the stop, with an activity-log entry; recovery then releases the record on its normal path > - The benefit is that legacy stuck records have a way out that never guesses ## Linked Issues or Issue Description Follow-up to the Phase 2 durable-ownership work: #20054 (controller instance lease) and #19883 (process-space recording). It generalizes the existing "confirm the previous server stopped" action for native controllers (`native_controller_host_unverified`). No duplicate PR was found. **What happened?** On an ephemeral server, an active run lease of a finished run that was saved before holders were stamped has no owner stamp. The ownership reconciler reports it `unprovable (no_owner_stamp)` and never settles it. No operator action exists, so the lease and its sandbox stay reserved. **Expected behavior** The task shows the stuck resource, and a board owner can confirm that the previous server stopped. Recovery then releases the lease. Agents and operators without the owner-stop permission cannot. **Steps to reproduce** 1. Run with `PAPERCLIP_EPHEMERAL_SERVER=true`. 2. Have an active environment lease with no `leaseController` stamp, whose run is terminal. 3. Run an ownership reconciliation pass. The lease stays `unprovable` with no action on the task. **Paperclip version or commit** `master` before this PR. ## What Changed - `packages/shared`: `confirmOwnerGoneSchema` (`actionId`, `recordId`, `stopEvidence` of 20 to 12000 characters). - `recovery/owner-gone-attestation.ts`: - `requestOwnerGoneConfirmation(kind)` is an `onUnprovable` hook. For a legacy unprovable owner (`no_owner_stamp`, `process_space_unrecorded`, `missing_identity`) on a record that has a task, it opens one idempotent board recovery action on that task (cause `ownership_owner_unprovable`). - `confirmOwnerGone(...)` verifies the exact action and record. It asks the kind to write the attestation (compare-and-swap), resolves the action, and writes an `issue.ownership_owner_gone_confirmed` activity-log entry with the actor and the stated evidence. - Ownership registry: - `OwnableKind.attestOwnerGone` is a new optional hook. - `environment_run_lease_active` implements it: only an unstamped active lease of a terminal run, with the notification for the cleanup path. - The lease's owner then reads `operator_attested`, which proves gone, so the existing settle releases it. A holder stamped since the board was asked owns the lease, and its stamp decides. - Route: `POST /api/issues/<built-in function id>/recovery-actions/confirm-owner-gone`. It uses `assertBoard` plus `runtime:confirm_owner_stop`, the same board-only permission as confirm-controller-stopped. It is registered in OpenAPI. - UI: `issuesApi.confirmOwnerGone`. The task's recovery card shows the existing `PreviousServerStopConfirmation` for this cause, with resource-specific wording; owners confirm and operators see the wait. `isPreviousServerStopCause` includes the cause, so the generic card does not offer other resolutions. ## Verification - `owner-gone-attestation.test.ts`: - the stuck lease opens one board action on the task; - confirmation resolves the action, writes the activity log, and the next pass settles the lease (`operator_attested`) exactly once; - a mismatched record is refused with 409; - a holder stamped after the ask is refused and the lease stays active; - no action for a judgeable owner or for a record with no task. - Red, with the `onUnprovable` hook removed: 2 of 3 fail. - UI: `WorkspaceExportRecovery.control.test.tsx` checks that the owner confirms with the right payload and that an operator sees the wait without the control (47/47). - Server suites: openapi-routes, ownership and recovery suites, plus the existing native-controller-host recovery. Low-memory server typecheck is clean; `check:token-gates` is clean. ## Risks - The board can attest a server stopped that is in fact running. The action and the UI copy ask for the evidence, and the activity log records who attested what. This is the same trust model as confirm-controller-stopped. - Only run leases opt in so far. Records without a task (login/probe leases) keep the legacy idle sweep. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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>canary/v2026.1108.0-canary.17 |
||
|
|
66a2cbab98 |
fix(runner): adopt the in-flight Codex turn after a restart lands just after turn.start (#20057)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run survives a server restart by resuming its durable provider session and adopting the turn that was in flight > - The adoption rule (RC-59) only adopts a turn named by the latest `turn.start` command's accepted result > - Two gaps made it fail. A restart just after `turn.start` leaves that command `execution_indeterminate` (the runner died after journaling it). Separately, the rule demanded that Codex echo the requested turn id, which it never does. > - The restored session then rejected Codex's own in-flight turn as unexpected, and the user's Retry failed on the first click > - This pull request adopts exactly the turn the runner reconciled for that ambiguous start, and accepts Codex's own turn ids > - The benefit is that a restart landing right after a turn starts no longer costs the user a second Retry ## Linked Issues or Issue Description Found by the restart-chaos campaign: wave `w-20261108T134237-s9401`, task t77 (Codex C1), Retry run `5358f1cb`. Related: #19852 (fast follow-up turns), RC-59 adoption. No duplicate PR was found. **What happened?** The Retry run submitted its first turn at 14:05:25.171. A K2 SIGTERM followed at .690 (graceful drain, then `resume_dead_runner`). - The runner had journaled the `turn.start` and then died, so on restore it settled the command as `failed / execution_indeterminate`. - The runner's restored Codex session reconciled the ambiguous start to the live replacement turn (`session.reconciled`, `activeProviderTurnId`). - Codex then sent `turn/started` for that turn. The restored driver had no active or pending turn, because `controllerAcceptedTurnId()` was null, and failed with `turn_binding_mismatch`. - The run ended `native_event_replay_conflict`, and the user needed a second Retry. **Expected behavior** The restored session adopts the runner-reconciled in-flight turn and finishes it. It does not treat it as a replay conflict. **Steps to reproduce** 1. Start a native Codex run. 2. Send SIGTERM to the server about 0.5 s after the first `turn.start`, before runnerd confirms it. 3. Let the server restart and resume the run (`resume_dead_runner`). 4. Before this PR, the run fails with "Provider started an unexpected turn." **Paperclip version or commit** `master` before this PR. ## What Changed - `controllerAcceptedTurnStartId(commands, options)` (runnerd-codex-transport.ts): - **Codex and OpenCode results.** These providers assign their own turn ids, so an accepted result never echoes the requested `turn_lab_` id, and RC-59 adoption never matched a real Codex result. For them, the accepted turn is now adopted only while it is also the runner's live active turn in its authenticated `session.snapshot`. A turn the restored runner reports lost has no live turn, so it keeps the established restart-continuation path. ACPX still requires the echoed id. - **Indeterminate start.** When the latest `turn.start` (after the latest attach) failed with `execution_indeterminate`, it adopts the runner's live active turn. Runnerd reports one only after it reconciles an ambiguous start to a distinct replacement turn. - The transport records the snapshot's active turn at `thread/read`, which `recover()` reads before it evaluates adoption. - The driver's existing RC-59 guards still apply: the turn must be in provider history, be the only active turn, and not already be terminal. ## Verification - Connected restart test, `runnerd-restart-continuation.test.ts` "adopts the in-flight turn when a restart lands just after turn.start (t77)". It uses the real runnerd and fake Codex: - the runner and provider die after journaling a started turn; - runnerd's own restore settles the command `execution_indeterminate`; - recovery adopts `provider-turn-1` with no second `turn/start`. - It fails without the fix: nothing is adopted. - Unit cases for `controllerAcceptedTurnStartId`: - a Codex-assigned id is adopted only while it is the live turn; - an indeterminate start is adopted with the reconciled turn; - it fails closed with no live turn, an ordinary failed start, a later attachment, or an ACPX id mismatch. - Regression guarded: "starts a new turn when the restored runner reports a lost turn the checkpoint never bound" still passes. - `runnerd-codex-transport`, `codex-app-server-driver.recovery`, `runnerd-codex-transport-adoption`, `runnerd-codex-transport-cold-restore`, `runnerd-restart-continuation` and `runnerd-controller-outage`: 383/383. Runner TypeScript typecheck is clean. - Live: the chaos campaign will run the `runner.turn_start.*` crash points (Codex A1/C1) and a Codex C1 K2 storm, and post the result here. ## Risks - **Previous-release story.** No persisted shape changes. Commands journaled by the previous release carry forward and are read with the new rule. An indeterminate start that the runner did not reconcile to an active turn still fails closed, as before. - **Wider Codex adoption.** Codex adoption now actually fires. If runnerd ever returned an accepted result that names a turn other than the one this command started, the driver would adopt it. Runnerd binds the result to the exact command id, so this is the same trust the rule already placed in ACPX. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
17ad23aafe | fix(workspaces): back off failed terminal cleanup and retire abandoned reservations through remove (#20020) | ||
|
|
5de2842620 |
fix(restart-chaos): bound each skopeo image copy attempt (#20056)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart-chaos harness downloads the signed Daytona runner image with skopeo before each Daytona wave > - #20021 made a failed copy retry from an empty directory, but the retry only runs when skopeo exits > - A hung blob read never exits, and a Daytona pull stalled for 80 minutes > - This pull request bounds each copy attempt, so a stall is killed and retried > - The benefit is that one network stall costs at most one bounded attempt, not the whole run ## Linked Issues or Issue Description Follow-up to #20021. No duplicate PR was found. **What happened?** A Daytona chaos run's image pull stalled inside `skopeo copy` for 80 minutes, even after #20021. `copyImageToDir` awaited `execFile` with no timeout, so a read that hung forever blocked the run forever. **Expected behavior** Each copy attempt has a time bound. A stalled attempt is killed and the next attempt starts from an empty directory. **Steps to reproduce** 1. Run a Daytona wave or crash point with an uncached image on a network where a registry blob read hangs. 2. Before this PR, `skopeo copy` never returns and the run waits indefinitely. **Paperclip version or commit** `master` before this PR. ## What Changed - `copyImageToDir` passes `timeout` (default 20 minutes, `RESTART_CHAOS_SKOPEO_ATTEMPT_TIMEOUT_MS`) and `killSignal: "SIGKILL"` to each `skopeo copy`. A timed-out attempt counts as a failed attempt and retries from an empty directory. - README updated. ## Verification - New test: a fake skopeo that writes a partial blob and then hangs is killed after the 500 ms test bound, and the second attempt succeeds, within 10 s. - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/daytona-image.test.ts`: 18/18. `tsc -p tests/restart-chaos/tsconfig.json` is clean. ## Risks - Test harness only. A very slow but progressing pull longer than 20 minutes would now be retried; the bound is configurable. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
c910651115 |
fix(native): start each rebound run with a fresh restart-continuation marker (#20053)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts during an agent turn, the runner restores the same conversation. A Codex, Claude (ACPX) or OpenCode turn lost in the restart is marked `provider_turn_lost_on_restore`, and the run continues it once in the same conversation > - That one continuation is guarded by a disposition-only recovery marker in the session checkpoint. It is run state: it names a turn of the run that used it > - A follow-up run (a user comment on the same task) reuses the provider session by re-binding the previous run's checkpoint to the new run. Re-binding clears the run-scoped turn, result and request state, but it kept the marker > - The chaos K13 storm (wave w-20261108T134237-s9401) hit this three times (t24 and t45 on Claude, t77 on Codex). The task's first run had already used its restart continuation. The follow-up run inherited a spent marker naming a turn it never had, so when a restart lost its own turn, the driver kept the session closed. The run failed, and the user had to click Retry > - This pull request resets the marker at both re-bind sites, as for every other run-scoped field > - The benefit is that a restart during a follow-up turn continues on its own instead of putting a Retry click on the user ## Linked Issues or Issue Description Refs #19831 (durable ownership plan, restart continuation). Found by the restart chaos campaign. In all three failed follow-up runs: - After `resume_dead_runner`, the run emitted `turn.failed {code: provider_turn_lost_on_restore, recoverable: true}` for its own active turn. - The checkpoint still carried `dispositionOnlyRecoveryConsumed: true`, with `dispositionOnlyRecoveryTurnId` set to the previous run's continuation turn. ## What Changed - `native-session-resume.ts` (`rebindNativeSessionCheckpoint`) and `native-session-executor.ts` (warm checkpoint rebind): a rebound run starts with `dispositionOnlyRecoveryConsumed: false` and `dispositionOnlyRecoveryTurnId: null`. - Nothing changes inside a run: the driver's reopen rule (`mapTerminalTurn`) and `nativeRestartInterruptedTurnId` are untouched. The first commit changed the helper; review found the driver keeps its own copy of the rule, and the second commit reverts the helper in favour of this fix. ## Verification - `native-session-resume.test.ts` "retains provider identity but clears prior turn and event state" now seeds a spent marker on the previous run. It fails on `master` and passes here. - `native-session-executor.test.ts` "starts a new run's usage from the prior run's total when a warm checkpoint resumes" (3 cases) seeds the first run's checkpoint with a spent marker and asserts that the second run starts with it reset. It fails on `master` and passes here. - The other `native-session-resume.test.ts` tests pass. One test, "archives a damaged prior epoch before completing guarded same-task replacement with real runnerd", fails locally identically with and without this change (ENOENT under `authority-epochs`, from the real-runnerd environment). It's left to CI. - Next: a targeted chaos rerun (Codex and Claude C1 under repeated K2) on this branch. I'll add the result here. ## Risks - **Previous-release story (upgrade-safe run state):** no persisted shape changes. A run re-bound by this build gets a fresh marker. A run re-bound before the deploy, still in flight with an inherited spent marker, behaves as before: a lost turn falls back to Retry, once. - Inside a run, nothing changes: the marker still bounds same-run continuations, together with the provider-attempt restart budget. ## 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>canary/v2026.1108.0-canary.16 |
||
|
|
cf9c81d9a7 |
perf(run-log): record named preparation phases before dispatch as run-log spans (#20046)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every native run records performance spans in its run log, the instance-local `heartbeat_run_events` table, so slow turns can be diagnosed without any external service > - The steps between environment acquisition and dispatch are timed by named preparation phases, but those timings reach only OpenTelemetry, which is off unless an operator configures an endpoint > - On a Daytona follow-up in a paperclip-sized workspace, 17–30 s sat in one opaque span, `heartbeat.prepare_after_environment` > - This pull request also records the named preparation phases, and their nested steps, as run-log spans > - The benefit is that the next slow step shows up by name in the run log; this change found a full workspace re-upload on every remote turn ## Linked Issues or Issue Description **What happened?** The run log showed `heartbeat.prepare_after_environment` at 17–30 s on Daytona follow-ups, with nothing inside it. The phases that would explain it (`heartbeat.prepare.*`, `runtime_selection.*`, `workspace_sync.*`) were recorded only as OpenTelemetry spans, and those are a no-op without an OTLP endpoint. **Steps to reproduce** 1. Run a Codex task on a Daytona environment with a large repository as the project workspace. 2. Post a follow-up comment. 3. The run log shows one large `heartbeat.prepare_after_environment` span with no children. **Expected behavior** The run log names the steps that make up the wait. ## What Changed - `execution-preparation-timing.ts`: every named preparation phase (`measure`) and nested run phase (`measureRunPhase`) records its start, end and outcome in a per-run in-memory list. - Bounds: phases under 50 ms are skipped, at most 200 per run, at most 256 runs held. - Without a tracer, the run scope is still set, so nested phases reach the run. - OpenTelemetry behavior is unchanged. - `runtime-execution.ts`: at native dispatch, the recorded phases are drained into the run's preparation spans under `task.prepare`. - `runtime-selection.ts`: the remote Codex CLI version read and the runtime context build get their own phases, `runtime_selection.codex_cli_version` and `runtime_selection.runtime_context`. ## Verification - `execution-preparation-phases.test.ts`: a slow phase is recorded and a fast one skipped, a failure is marked, draining clears the run, runs are kept apart, and a nested `measureRunPhase` step is recorded without a tracer. - `database-operation-attribution`, `company-skills-observability` and `github-catalog-timing`: pass (15 tests in total). - Daytona Codex run in a fresh paperclip clone: the follow-up's 17.8 s gap now reads `heartbeat.prepare.runtime_selection` 15.7 s, then `runtime_selection.workspace_sync` 15.4 s, then `workspace_sync.transfer` 13.3 s. ## Risks - This is a run-log change only, not Telemetry: the data stays in the instance database. It adds roughly 10–40 spans per run, bounded as above. - The recorder is in memory and per process. A run that never dispatches is evicted when the cap of 256 runs is reached. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution, Daytona measurements). ## 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> |
||
|
|
96badeaa42 |
fix(lifecycle): never repeat a passed harness verification; settle a dead verifier's lease at once (#20030)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A new or reconfigured agent runs a harness verification (a hello probe in a test sandbox) before it is ready, and its runs wait until it passes > - In the final Daytona restart chaos wave, a Codex agent's runs waited about 417 s for their first sandbox, while boot took only about 32 s > - Each control-plane restart killed the verifier. The next process waited out the 120 s lifecycle lease and then booted a fresh sandbox. Nothing recorded a configuration that had already passed. > - This pull request records a passed verification by what makes it valid and reuses it, and settles a dead verifier's lease on the shared ownership path at once > - The benefit is that a restart never repeats a passed verification, and never leaves runs waiting out a dead process's lease ## Linked Issues or Issue Description **What happened?** In the final Daytona chaos wave, the Codex agent was created at 09:55:46 UTC and its three runs were queued. Verification ran four times, in four sandboxes, because faults K1, K4 and K2 each killed the verifying server. After K2 the server was ready at 10:00:20, but verification restarted only at 10:02:13: it waited out the 120 s lifecycle lease. The first run sandboxes were created at 10:02:43, 417 s after launch. **Steps to reproduce** 1. Create a Codex agent and assign it a task at once, so the task waits for harness verification. 2. Kill the control plane (SIGKILL) while the agent is verifying. 3. The restarted server waits out the dead verifier's 120 s lifecycle lease, then verifies again in a new test target. The task's first token arrives about 130 s after the fault. **Expected behavior** - A passed verification of the same effective configuration in the same runner image is never repeated. - A verification is never restarted from scratch while its holder process is alive. When the holder is proven gone, its lease is settled at once. - Runs gate only on a configuration that has never passed. **Root cause** `verify()` (`server/src/services/agent-harness-verification.ts`) leased a fresh sandbox on every attempt and recorded nothing on a pass. The lifecycle lease (`modules/agent-lifecycle/adapters/postgres.ts`) named no holder process, so a restart could only wait for it to expire. ## What Changed - Restart chaos TTFT driver: a `verify` phase, reported in its own rows, and a `--verify-fault K1|K2|K6` option. The fault fires 1 s after `verifying` is observed, so it interrupts a held lease. The seed keeps each agent's creation body, so the driver can hire an identical agent. - **Persisted pass:** new columns `agents.harness_verified_digest` and `harness_verified_at` (migration 0448). The digest is an HMAC, keyed with the instance's decision-signing secret, over: - the fully resolved effective configuration: every value the test used, including `cwd`, resolved credentials, and agent-over-environment precedence; - the selected AI account and its credential generation (connection and grant ids and `updatedAt`, never the credential); - the environment and the runner image. The key means the stored value cannot confirm a guessed credential offline. For a sandbox, the runner image is an immutable image digest or a qualified Daytona snapshot ID (`sandboxBinding`, now exported). For local runs it is the server version. A mutable image or snapshot name, or a local environment redirected to the managed sandbox, never reuses a pass. - **Server-only:** `normalizeAgentBaseRow` and `redactAgentRowForResponse` strip the digest and its timestamp from agent API responses. - `verify()` still runs every account, policy and secret check, then returns complete on a matching digest without booting a test target. It records the digest on a pass before the lifecycle step completes, so a restart in between reuses it. - **Lease holder:** the lifecycle `claim()` stamps the claiming controller process on the lease (`lifecycleOperation.leaseHolder`, supplied through the lifecycle effects port) and clears the stamp with the lease. The compare-and-swap release lives in the lifecycle module (`releaseAgentLifecycleLeaseOfHolderInTransaction`). - **Shared ownership path:** a new `agent_lifecycle_lease` kind with `settleAgentLifecycleLease`. - When the reconciler proves the holder gone, settle clears exactly that lease (compare-and-swap on the holder), notifies lifecycle work, and logs `agent.lifecycle_lease_reconciled`. - A live holder is never taken over. The lifecycle state, operation and progress are untouched. ## Verification - `restart-verification.test.ts` (embedded Postgres, real lifecycle store and worker, real reconciler pass): - A verification held by a dead process is claimed and completed at once after the startup pass, with the agent ready in under 10 s instead of waiting out the 120 s lease. It fails without the new kind. - A live holder is never taken over. - The claim stamps the holder, and `defer` clears it. - `agent-harness-verification-preflight.test.ts`: - A second `verify()` of the same configuration allocates no target. A revoked account still fails before reuse. A changed model, `cwd` or literal credential tests again. It fails without the reuse. - A failed verification records nothing. - Ownership report, all lifecycle module tests, remote, provider-API and unit harness verification tests, and crash-point call sites: 13 files, 220 tests pass. - Targeted local run: the TTFT driver's new `verify` phase (`--phases verify --verify-fault K1`, Codex, 2 samples) hires a new agent, assigns it a task at once, kills the control plane (K1) while it verifies, and times fault to first agent token: - Before (master server code): 129.8 s and 129.8 s. The restarted server waited out the dead verifier's 120 s lease. Run-log TTFT from run creation was only 4.1 s. - After (this branch): 15.8 s and 17.2 s. The startup reconciler pass settled `agent_lifecycle_lease` and verification re-ran at once. ## Risks - **A stale pass:** the digest covers every resolved value, the selected account generation, the environment and an immutable runner image, so any change tests again. Rotating the instance signing secret invalidates every digest, so each agent verifies once more. - **Migration:** two nullable columns, no backfill. Existing agents verify once more, then reuse. - **Settle:** it runs only on a `gone` proof, compare-and-swap on the exact holder. It never changes the lifecycle state. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
bfa9002a3c |
test(restart-chaos): reach the stop-and-retain crash point with --daytona-reuse-lease (#20049)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart-chaos crash-point suite kills the server at named points, then checks that recovery restores every invariant > - On Daytona, `environment.stop_retain.after_claim_before_stop` was never reached, because no workload produced a stop-and-retain release > - A per-turn run on a reusable sandbox lease ends every turn with `stop_and_reuse`, which releases through exactly that stop-only claim > - This pull request adds `--daytona-reuse-lease`, so the chaos Daytona environment keeps reusable sandboxes and the point is reachable > - The benefit is live coverage of the stop-and-retain crash window ## Linked Issues or Issue Description This is a follow-up from the Phase 2 durable-ownership data (`doc/plans/2026-11-07-phase2-reconciler-data.md`, "Not covered"). Related: #19937 (crash-point suite) and #20042 (stop-only claim fence renewal). No duplicate PR was found. **What happened?** The Daytona crash-point run `cp-20261108T104102` reported `environment.stop_retain.after_claim_before_stop` as `not_reached`. The point needs a stop-and-retain intent, and the chaos Daytona environment is created with ephemeral leases. **Expected behavior** A flag lets a Daytona wave or crash-point run use reusable leases. A per-turn run then stops and keeps its sandbox, and the point is reached and graded. **Steps to reproduce** 1. Run `node scripts/crash-points.mjs --env daytona --daytona-image <image> --points environment.stop_retain.after_claim_before_stop --adapters codex --workloads A1`. 2. Before this PR the point is `not_reached`. 3. With `--daytona-reuse-lease --lifecycle per_turn` it is reached. **Paperclip version or commit** `master` at `8475826d2d` (the signed image `c24059f1…` revision). ## What Changed - `--daytona-reuse-lease` (wave and crash-point suite, Daytona only) creates the chaos environment with `reuseLease: true`. The crash-point suite passes the flag through. - The R6 sandbox sweep counts a **stopped** sandbox behind a **released `reuse_by_environment`** lease as kept for reuse, not a leak. A later run on the same workspace resumes it, because reuse accepts released and retained leases. A running sandbox, or a stopped one behind an ephemeral lease, is still a leak. - The crash-point registry prerequisite and the README name the flag. ## Verification - Live, Daytona, codex A1, from the image revision `8475826d2d`: - `cp-20261108T132149`: the point was **reached** and settled with no invariant violations. R6 flagged the stopped reusable sandbox as a leak, which led to the sweep fix. - `cp-20261108T133351`, with the sweep fix: **pass**. 0 orphaned sandboxes, 0 held leases, the environment deleted, and nothing left running. - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts` passes `daytona-sweep`, `scheduler` and `crash-points` (52/52). `tsc -p tests/restart-chaos/tsconfig.json` is clean. ## Risks - Test harness only; the registry change is prerequisite text. Without the flag, waves behave as before. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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>canary/v2026.1108.0-canary.15 |
||
|
|
d5cbe70636 |
fix(workspaces): create git worktrees one at a time per repository; retry another process's config lock (#20039)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Tasks in a project with git_worktree isolation each get their own
worktree, created with `git worktree add -b <branch> <path> <base>`
> - That command writes the repository's shared `.git/config` to record
the new branch's upstream
> - When two tasks start at once on one repository, both write
`.git/config`, and one fails with "could not lock config file"
> - The failed attempt already created its branch, so the retry attached
that branch without upstream tracking and without runtime ownership
> - This pull request serializes worktree creation per repository and
retries another process's brief config lock cleanly
> - The benefit is that simultaneous tasks on one repository start
without a failed attempt or a degraded branch
## Linked Issues or Issue Description
**What happened?**
In a TTFT measurement with a fresh clone of the paperclip repository as
the project workspace, two agents started tasks at the same time. The
Codex task's `git worktree add -b` failed twice ("error: could not lock
config file .git/config: File exists"). It then succeeded by attaching
the branch the failed attempt had created.
**Steps to reproduce**
1. Use a project whose primary workspace is a git clone, with
git_worktree isolation and a remote-tracking base ref.
2. Start several tasks in that project at the same moment.
3. Some worktree creations fail with "could not lock config file". The
task falls back to attaching the existing branch, without upstream or
branch ownership.
**Expected behavior**
Every simultaneous task gets a fresh worktree on its own new branch,
with upstream tracking, owned by the runtime.
**Root cause**
`git worktree add -b` writes `.git/config`, which git locks with
`config.lock`. Concurrent creates on one repository contend for that
lock. On a lock failure, git has already created the branch but no
worktree.
## What Changed
- `executeGitCommand` (in `workspace-runtime.ts`) runs every `git
worktree add` from `recordGitOperation`. The recorded and unrecorded
paths both use it.
- Creates in this process run one at a time per repository, keyed by the
realpath of `git rev-parse --git-common-dir`. Linked worktrees and the
primary checkout share one key.
- A create that still meets a config lock held by another process
retries after 100, 250, 500, 1000 and 2000 ms. Before each retry it
removes the branch that same `-b` command created. Git refuses an
existing branch with "already exists" before it touches config, so a
branch present after a config-lock failure is one this command made. The
retry then creates the branch again with its upstream.
## Verification
- `workspace-worktree-concurrency.test.ts`:
- Three rounds of ten simultaneous `realizeExecutionWorkspace` calls on
a fresh clone with base `origin/main`. Every workspace is `created:
true` and `branchCreatedByRuntime: true`, on its own branch. Without the
serialization it failed 3 of 3 runs.
- A create that meets `.git/config.lock`, held for 300 ms as another
process would, succeeds with the branch's upstream recorded.
- `workspace-runtime.test.ts`: 179 tests pass.
## Risks
- Worktree creation on one repository is now serial within a process.
Each create takes about 0.1–1 s, so ten simultaneous tasks add a few
seconds at most. Creates on different repositories still run in
parallel.
- The branch deletion runs only after a config-lock failure of a `-b`
create, when the branch is provably new.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file editing, test execution).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
ba867dea39 |
fix(recovery): keep the orphan marker for a stop intent saved before #19976 (#20043)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane stops and keeps the sandbox of an orphaned run lease, then destroys it once nothing references it > - #19964 marked such a sandbox only inside its stop intent; #19976 added the top-level marker the destroy looks for > - A stop still pending when #19976 shipped lost the intent on success, so the destroy never saw that sandbox > - This pull request carries the orphan marker forward when a stop intent that says `recoveredOrphanedRunLease` is confirmed > - The benefit is that those sandboxes are destroyed like every other retained orphan ## Linked Issues or Issue Description Refs #19964 and #19976. This is a follow-up gap from the Phase 2 ownership migration. No duplicate PR was found. **What happened?** Between #19964 and #19976, the shipped builds were `canary/v2026.1108.0-canary.6` and `canary.7`. In that window the orphaned-run-lease settle wrote `recoveredOrphanedRunLease: true` only on the `sandboxStopAndRetain` intent. `settleStopOnlyCleanup` removes the intent when the stop is confirmed, and the retained-orphan kind lists only leases with the top-level `orphanedRunLeaseRecovery` marker. So an orphan whose stop was still pending at upgrade stays `released` with a stopped sandbox forever. **Expected behavior** Once its stop is confirmed and nothing references it, the orphan is destroyed through the retained-orphan settle, whichever build wrote the intent. **Steps to reproduce** 1. Write a `pending_cleanup` lease with a `sandboxStopAndRetain` intent that has `recoveredOrphanedRunLease: true` and no top-level marker (what canary.6/.7 wrote). 2. Confirm the stop through `settleStopOnlyCleanup`. 3. Run an ownership reconciliation pass. Before this PR the retained-orphan kind never lists the lease. **Paperclip version or commit** `master` before this PR. Rows written by `canary/v2026.1108.0-canary.6` and `canary.7`. ## What Changed - `settleStopOnlyCleanup` writes the top-level `orphanedRunLeaseRecovery` marker (`fromStopIntent: true`) when a confirmed stop's intent carries `recoveredOrphanedRunLease: true` and the lease has no marker yet. - An ordinary stop-and-retain (no recovery flag) is not marked. ## Verification - Red, before the change: "keeps the orphan marker for a stop intent saved before the top-level marker existed" failed. - Green: `environment-retained-orphan-sandbox` and `environment-run-lease-ownership` 36/36; `environment-runtime` and `heartbeat-pending-cleanup-sweep` 246/246. - Counter-case: an ordinary stop-and-retain intent stays unmarked, and the retained-orphan pass does not settle it. ## Risks - Not covered: an orphan whose stop was confirmed *while* canary.6/.7 ran kept no durable marker at all. Its intent was removed and the receipt never carried the flag, so it cannot be told apart from an intentional stop-and-retain. Those rows exist only on instances that ran those two canaries during about three hours, and the provider's own auto-delete bounds them. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
cefd17a82b |
fix(environments): renew the stop-only cleanup claim while the provider stop runs (#20042)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a run ends with a "stop and retain" disposition, the control plane stops the agent's sandbox and keeps it for later recovery > - The release claims the lease with a 15-minute in-flight fence, so no cleanup sweep acts on it at the same time > - The stop-only path never renewed that fence; a provider stop that outlives 15 minutes lets a sweep take the lease over and stop it again mid-flight > - This pull request renews the stop-only claim every 30 s while the provider stop runs, like the destroy claim and the pending-cleanup sweep > - The benefit is one owner per cleanup attempt, however slow the provider is ## Linked Issues or Issue Description Follow-up gap from the Phase 2 ownership migration (durable ownership and reconciliation plan). Related: #19966 (teardown claim on the shared path), #19980 (pending-cleanup queue owner). No duplicate PR was found. **What happened?** `releaseRunLeases` with `providerResourceDisposition: "stop_and_retain"` claims the lease (`pendingCleanupInFlight: true`, `pendingCleanupLeaseExpiresAtMs: now + 15 min`) and calls the provider stop. Nothing renewed the fence while the stop ran. The destroy claim and the pending-cleanup sweep both renew theirs every 30 s. **Expected behavior** While this process is still running the provider stop, its claim stays fenced, so no other sweep starts a second attempt. **Steps to reproduce** 1. Release a run lease with `stop_and_retain` against a provider whose stop RPC blocks. 2. Let more than 15 minutes pass. 3. See that the fence expired while the first stop is still in flight. **Paperclip version or commit** `master` before this PR. ## What Changed - New `renewCleanupClaim(leaseId, attemptId, failure)` in `environment-runtime.ts`. It renews `pendingCleanupLeaseExpiresAtMs` every 30 s, only while the row still names this attempt and is in flight. - The destroy claim now uses the helper; its behaviour is unchanged. - The stop-only claim starts the renewal right after the claim and stops it before it settles the attempt, on success, on failure and in `finally`. ## Verification - Red, before the change: `environment-runtime.test.ts` "renews the stop-only cleanup claim while the provider stop is still running" failed. The fence stayed at its aged value. - Green: `pnpm exec vitest run src/__tests__/environment-runtime.test.ts src/__tests__/heartbeat-pending-cleanup-sweep.test.ts`, 247/247. ## Risks - Low. One extra metadata write every 30 s per in-flight stop, the same as for the destroy claim. The renewal is fenced on the attempt id, so it never extends another attempt's claim. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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>canary/v2026.1108.0-canary.14 |
||
|
|
c966334ec3 |
fix(recovery): settle a run-less lease once its stamped holder is proven gone (#20026)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The control plane recovers in-progress records after a server restart through a shared ownership path > - Run-less sandbox leases (login, probe, harness verification) were released only by a five-minute idle sweep > - That sweep releases by age: it leaves a dead holder's sandbox for minutes and takes a live login session's sandbox > - This pull request settles a run-less lease only when its stamped holder is proven gone > - The benefit is no stuck sandbox after a restart, and no live session loses its sandbox ## Linked Issues or Issue Description Related: #19965 (named the owner of login/probe leases), #19964 (the run-lease settle this mirrors). No duplicate PR was found. **What happened?** The final Daytona chaos wave `w-20261108T083036-s9401` recorded I2 `environment_login_probe_lease` (`pid_dead`) on 2 records. These were the codex agent's harness-verification leases. A restart killed their holder. The leases stayed `active` until the five-minute idle sweep released them. The same sweep releases any run-less lease after five idle minutes, also when its holder is alive. **Expected behavior** A run-less lease is released as soon as its stamped holder is proven gone. A lease whose holder is alive is never released, however long it is idle. **Steps to reproduce** 1. Start a Daytona wave: `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env daytona --daytona-image <image>` (K1/K4 faults). 2. Let a fault kill the server while an agent's harness verification holds its sandbox lease. 3. See the reconciler report I2 `environment_login_probe_lease` until the idle sweep releases it. **Paperclip version or commit** `master` at `8475826d2d`. ## What Changed - New `settleEnvironmentLoginProbeLease`, registered on `environment_login_probe_lease`. It runs only for a stamped holder that is proven gone, and compares and swaps on that exact stamp. - It moves the lease to `pending_cleanup` with the recovered-orphan reason, so the cleanup sweep destroys the sandbox in its next tick. - It leaves the lease alone when a legacy shutdown fence applies, or when another live lease holds the same provider resource. - The idle sweep for run-less leases now covers only legacy rows saved before holders were stamped (no `leaseController` key). Stamped rows are never released by age. - `holderUnchanged` is now exported from the run-lease settle so both settles share it. - The controller stamp (`leaseController`, and every other `ControllerProcess` stamp) now records the platform deployment when the platform names one (Railway: project, environment, service, deployment, volume). `controllerStamp` passes it to `proveOwnerGone`. A container redeploy changes the PID space, and the old stamp would read `different_process_space` (unprovable) forever. Now the existing `deployment_replaced` proof shows the old holder gone. This also covers `environment_run_lease_active` and the other kinds built on `controllerStamp`. - `currentDeploymentIdentity` in `owner-proof.ts` now delegates to `currentControllerDeployment`, so there is one parser. - Data doc: added the final Daytona wave, the six Daytona-only crash points (4 pass, 2 not reached, no violations), and the cause of the roughly 450 s codex TTFT. Runs waited behind the agent's harness verification, which restarted after each fault; boot itself took about 32 s. - Restart-chaos README: corrected which Daytona crash points a plain task reaches. ## Verification - Red, before the change: `environment-login-probe-lease-ownership.test.ts` failed 3 of 6. No settle happened, and the idle sweep released 3 live holders' leases. - Green: `pnpm exec vitest run` passes for `environment-login-probe-lease-ownership`, `environment-run-lease-ownership` and `heartbeat-orphaned-active-lease-sweep` (45/45), and for `environment-lease-owner-stamps`, `heartbeat-observed-workspace-preservation`, `ownership-reconciliation-report`, `heartbeat/recovery` and `workspace-recovery-warm-handoff`. - Redeploy: a stamp from a replaced Railway deployment settles with `deployment_replaced`. A stamp from the same deployment in another PID space stays `active`. This was red when `controllerStamp` dropped the deployment. - Live-owner counter-cases: the current process, another live PID, another PID space, and a lease re-stamped after the proof all stay `active`. A read-only pass never settles. - Not re-run live: the Daytona wave and crash points ran before this fix. The next Daytona wave should show no `environment_login_probe_lease` I2. ## Risks - A stamped run-less lease whose live holder never releases it, because of a bug in that process, is no longer released after five idle minutes. It is released once that process exits. This is the intended trade: never take a live holder's sandbox. - On an ephemeral server without a platform deployment identity (not Railway), a stamp from a replaced container is still unprovable. Such a run-less lease stays `active` until the provider expires the sandbox, and the reconciler reports it as unprovable. Before this PR, the five-minute sweep released it by age. This is the same open gap as unstamped legacy leases on an ephemeral server: there is no operator action yet. It is listed as a Phase 2 follow-up. - Legacy unstamped run-less leases keep the age fallback. These are rows written before #19965, and no current code writes them. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
20cc6faca3 |
feat(recovery): settle legacy shutdown fences of proven-gone servers on the shared path (#20017)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a server shuts down gracefully during a sandbox run, it commits a fence on the run's lease so no cleanup worker destroys the sandbox before its files are exported > - The fence names its server only by boot id. It is released when its owner settles, or when its 30 s deadline (renewed every 10 s) passes. After a restart, the sweep still waits for that deadline even though the old server is gone > - The durable ownership plan proves owners by identity through one shared prover and settles them from one reconciler > - This pull request stamps the committing process on the fence and settles it when the shared prover shows that process gone. The outcome is the same safe one an expired deadline gives: the source is preserved and the restore failure recorded > - The benefit is that the fence releases on proof, not age, and a fence whose owner may still be alive (another host, or an older marker) keeps its deadline ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Legacy graceful-shutdown workspace settlement fence", and "A boot-id mismatch alone does not release"). **What happened?** `legacy_shutdown_settlement_fence` was report-only, and its owner stamp had only a boot id. A fence left by a crashed or replaced server could be released only by its deadline. **Expected behavior** A fence whose committing process is proven gone (a dead or reused PID in the same PID space) settles from the recovery sweep and from the shared reconciler. An unprovable owner keeps the renewed deadline as its fenced takeover. ## What Changed - `legacy-shutdown-workspace-settlement.ts`: - `beginLegacyShutdownWorkspaceSettlement` stamps `controller` (`currentControllerProcessStamp`, no I/O). - New `legacyShutdownSettlementOwnerStamp`. - `allowLegacyShutdownWorkspaceCleanup` takes `ownerProvenGone`, which replaces only the deadline check, and only while the marker still names that process. Every other guard is unchanged: owner stop, competing allocations, in-flight cleanup. - The sweep also proves the owners of fences from other boots that are still before their deadline. - New `settleLegacyShutdownSettlementFence` for the reconciler. - `recovery/ownership-registry.ts`: the kind uses the full stamp when present and registers the settle. - The settling-kinds list in `ownership-reconciliation-report.test.ts` is updated. ## Verification - `heartbeat/recovery.test.ts`: 100 pass. Three new tests fail on `master` and pass here: - the fence stamps the committing process; - a previous server proven gone settles before the deadline (restore failure recorded); - the reconciler settles only the proven-gone owner (a read-only pass changes nothing). Another host and a boot-id-only marker stay pending until their deadline, then expire as before. - `ownership-reconciliation-report.test.ts` (25) and `environment-retained-orphan-sandbox.test.ts` (19) pass. - The server package typecheck passes. ## Risks - A proven-gone owner now expires the fence up to 30 s earlier than before. Expiry preserves the source (stop-and-retain or a `retain_on_failure` hold) and never destroys provider files. - Markers written before this change have no `controller` and behave exactly as before. ## 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> |
||
|
|
5978b7a1e0 |
fix(chat): tell chat agents when the Conference Room is enabled and what it is (#20036)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent Chat lets a person talk to an agent in a standing conversation > - The Conference Room is a separate, experimental board chat surface at `/<prefix>/board-chat` > - A chat agent cannot read the experimental settings, so it cannot know that the Conference Room exists > - When a person asks to "discuss the launch in the conference room", the agent treats the phrase as a metaphor and asks launch questions > - This pull request tells the chat agent about the Conference Room only while the experiment is on > - The benefit is that the agent links the real surface and states its actual scope ## Linked Issues or Issue Description **What happened** A person said "Let us discuss the launch in the conference room." in Agent Chat on a local-trusted instance with Conference Room Chat enabled. The agent's reasoning asked whether the conference room is a Paperclip feature. It found no information, because the experimental settings endpoint is board-only. It replied with launch questions and gave no link to the Conference Room. **Expected behavior** The agent links the Conference Room (`/<prefix>/board-chat`) and says what it is: a separate board assistant whose history does not carry over from the chat. When the experiment is off, the agent does not mention it. **Steps to reproduce** 1. Enable Conference Room Chat in Instance Settings > Experimental on a local-trusted instance. 2. Open a chat with the CEO agent. 3. Send "Let us discuss the launch in the conference room." 4. See that the reply gives no Conference Room link. ## What Changed - Add `CONFERENCE_ROOM_CHAT_DIRECTIVE` in `server/src/services/agent-conversations.ts`. It gives the route as a Markdown link and states the real scope of the surface. - Render the directive in standing-chat task markdown only when `conferenceRoomEnabled` is true (`server/src/services/heartbeat/task-markdown.ts`). - Set `conferenceRoomEnabled` from the experimental settings that the heartbeat already loads (`server/src/services/heartbeat/task-context.ts`). No new query runs before the turn starts. - Add a regression test for the enabled, disabled, and non-chat cases. ## Verification - `cd server && pnpm exec vitest run src/services/heartbeat/task-markdown.test.ts` passes (16 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - End to end: the `agent-chat.conference-room` chat eval runs with this commit cherry-picked onto its case branch. ## Risks - Low risk. The text is added only to standing-chat context and only while the experiment is on. - The directive does not check the deployment mode. It says that the surface works only on a local single-operator instance, and the route itself refuses other deployments with a clear error. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>canary/v2026.1108.0-canary.13 |
||
|
|
c8822ffd4b |
fix(runner): deliver a lead's queued requirement to a busy task's next turn (#20028)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - In chat, a lead agent routes new requirements to tasks that other
agents own
> - The lead uses the native `comment_on_task` tool for this
> - The tool description only says the comment does not reopen or
interrupt the task
> - A lead read this as "the busy assignee never receives it" and tried
other routes, such as the assignee's workspace files
> - The comment API also keeps another agent's comment as a note unless
it sends `resume: true`, and the tool could not send it, so the queued
change was dropped when the worker completed the task first
> - This pull request states the real queued-delivery behavior and adds
an optional `resume` flag to the tool
> - The benefit is that the lead routes the change once, through the
product, and reports it honestly as saved for later
## Linked Issues or Issue Description
**What happened**
The person asked the chat lead to change the headline of an existing
website task. The website assignee was in an active run. The lead read
`comment_on_task` ("without reopening or interrupting it") and concluded
in its reasoning that the running worker would not see a comment. It
then listed the assignee's workspace directory and considered writing
the worker's input file directly. It used its full reply window without
routing the change.
When the lead did post the change with `comment_on_task`, Paperclip
queued it behind the active run. The worker then completed the task. The
wake queue cancelled the deferred wake ("no longer applies to a terminal
task") because an agent comment carries follow-up intent only with
`resume: true`, and `comment_on_task` sent only `{ body }`. The
requirement never reached the worker.
**Expected behavior**
The lead posts the change as a comment on the existing task with
follow-up intent. Paperclip queues it and delivers it to the assignee's
next turn, also when the assignee completes the task first. The lead
tells the person that the change is saved for that next turn, not
already applied.
**Steps to reproduce**
1. Start a task for a worker agent and keep the worker in an active run.
2. In the lead's chat, ask for a change to that task's requirements.
3. Read the lead's reasoning: it cannot tell from the tool description
that a comment reaches a busy assignee later.
4. When the lead comments, let the worker finish the task as done: no
continuation run starts for the queued comment.
## What Changed
- `server/src/services/native-runtime/runner-bridge-tools.ts`: the
`comment_on_task` description now says to use it for a new or changed
requirement on an existing task, instead of a duplicate task or the
task's workspace files. It says that a comment on a running task is
queued and delivered at the start of the assignee's next turn on that
task, and that the agent reports it as saved, not applied.
- The same tool now accepts an optional `resume` boolean. The
description tells agents to set it for a requirement change, and
explains that without it a comment on finished work is only a note.
-
`server/src/services/native-runtime/paperclip-runner-tool-authority.ts`:
`taskBridgeApiRequest` forwards `resume: true` to `POST
/api/issues/{id}/comments` and omits it otherwise. The existing
comment-route permission check for follow-up intent on another agent's
task still applies.
- `server/src/services/native-runtime/runner-bridge-tools.test.ts`:
regression tests assert the advertised description and schema, and the
forwarded request body.
## Verification
- `cd server && pnpm exec vitest run
src/services/native-runtime/runner-bridge-tools.test.ts` passes (11
passed, 6 platform-skipped).
- The wake-queue rule that keeps a resume-intent comment alive after
completion is already covered in
`server/src/modules/wake-queue/application/use-cases.test.ts`
(`done_no_resume` against resume).
- End to end: the `agent-chat.multiple-handoffs` chat eval, run with
this commit cherry-picked onto its case branch. Results are added as a
comment.
## Risks
- Low risk. No route or permission changes. The new `resume` field uses
the existing comment API flag and its existing authorization (assignee,
manager override, or open-write access).
- An agent that sets `resume: true` on a comment to finished work
reopens that work. This is the existing API behavior; the description
limits it to requirement changes.
- A runner with an active session goal can steer a comment into the live
turn. The description describes the normal queued path. It does not
promise the live path.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with
tool use (shell, file edits).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
e3a941372d |
fix(api): tell agents how a person connects their terminal (#20029)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in chat. Agents find API operations with the `search_api` tool, which reads the OpenAPI catalog. > - A person asked a chat agent to connect their terminal to Paperclip as themselves. > - The agent found the CLI auth operations, but their summaries did not say who starts the flow or which command to run. > - The agent then read source code for six minutes and did not answer in time. > - This pull request adds descriptions to the CLI auth operations. The descriptions tell the agent to give the person the `npx paperclipai auth login` command, and to not create or approve a challenge, share an agent key, or ask for a password. > - The benefit is a fast, safe answer that uses the supported browser approval flow. ## Linked Issues or Issue Description **What happened** A chat agent got "Connect my terminal to Paperclip as me." It searched the API catalog and found `POST /api/cli-auth/challenges`, approve, `me` and `revoke-current`. These operations had only short summaries. The agent did not know that the person's own CLI starts the flow. It searched the repository source for the command and used its full 420-second turn without a reply. **Expected behavior** The agent tells the person to run `npx paperclipai auth login --api-base <instance URL>` in their own terminal and approve the printed link in their browser. The agent does not create or approve a challenge, share an agent key, or ask for a password. **Steps to reproduce** 1. Open a chat with an agent that uses the native runner. 2. Send "Connect my terminal to Paperclip as me." 3. Look at the run: the agent calls `search_api` for CLI and terminal words, finds the cli-auth operations with no usage guidance, and then reads source files. ## What Changed - `server/src/routes/openapi.ts`: add descriptions to the four CLI auth operations. Give `POST /api/cli-auth/challenges` a summary that names terminal connection, so a plain request finds it. - The challenge description also tells agents that the URL is the address the person already uses in their browser. An agent must not look it up, because a runner-local API address can be unreachable from the person's machine. - `server/src/__tests__/cli-auth-terminal-guidance.test.ts`: a regression test. It runs `searchRunnerApi` with the user's words and two queries from the failed run. It checks that the challenge operation is in the top five results and that its description gives the command and the safety rules. ## Verification - `pnpm exec vitest run src/__tests__/cli-auth-terminal-guidance.test.ts` (in `server/`): 4 passed. On `master` without the fix, the same 4 tests fail. - `pnpm exec vitest run src/__tests__/openapi-routes.test.ts src/services/native-runtime/runner-api.test.ts`: 2103 passed. - `npx tsc --noEmit -p server`: clean. - Chat eval `access.cli-authorization` with the first commit cherry-picked passed (run 42475844547). The agent found the guidance in its first `search_api` call and gave the correct command. It then used about four minutes to look for the instance URL. The second commit removes that search; its rerun is run 42477301708. ## Risks - Low risk. The change adds text to API documentation only. Routes, permissions and responses do not change. - The new summary changes search ranking for words like "connect", "terminal" and "Paperclip". Other operations can move down by one place for those queries. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
022c73e475 |
fix(restart-chaos): retry a failed image copy from an empty directory; share the image cache (#20021)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart-chaos harness tests recovery on Daytona sandboxes > - The harness extracts the runner from the Daytona image with skopeo before a wave starts > - A skopeo blob read that timed out failed the whole run before any test started > - This pull request makes the image extraction retry cleanly and share one cache > - The benefit is that one network hiccup no longer costs a 20+ minute run ## Linked Issues or Issue Description Related: #19544 (Daytona chaos waves without Docker). No duplicate PR was found. **What happened?** The Daytona wave `w-20261108T082021` and the crash-point run `cp-20261108T101304` stopped before any test ran. skopeo reported `read: operation timed out` while it copied a blob. The retry then reported `not a containers image directory, don't want to overwrite`, because the destination already had partial files. The crash-point suite also used its own image cache, so it downloaded the image again on each run. **Expected behavior** A failed copy retries into an empty directory. Every wave and crash-point run reuses an image that another run already extracted. **Steps to reproduce** 1. Run `node scripts/crash-points.mjs --env daytona --daytona-image <image>` on a slow or unstable network. 2. Let skopeo time out on one blob read. 3. See the harness stop with exit 2 before any point runs. **Paperclip version or commit** `master` at `8475826d2d`. ## What Changed - With a shared cache, two runs can extract the same image at once. The final cache directory is created only by an atomic rename, and no run deletes a cache entry. A run that loses the rename adopts the valid entry. An invalid leftover stays in place, and the run publishes its extraction to a unique sibling directory. - `copyImageToDir` runs up to 3 `skopeo copy` attempts. Each attempt starts from an empty destination directory. - `defaultDaytonaImageCacheRoot()` is now the one cache for both the wave harness and the CLI: `tests/restart-chaos/results/daytona-images` by default, or `RESTART_CHAOS_IMAGE_CACHE` if set. Crash-point runs reuse an image that a wave already extracted. - README updated. - Three more tests: two runs against one cache through a barrier, a corrupt leftover, and two runs that both meet a corrupt leftover. - Unit tests use a fake skopeo that behaves like the real one: it refuses a non-empty destination and leaves partial blobs behind when it fails. ## Verification - `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/daytona-image.test.ts`: 17/17 passed. - `tsc -p tests/restart-chaos/tsconfig.json`: no errors in `tests/restart-chaos`. - Live run: the six Daytona crash points were rerun with this patch, reusing the cached image. ## Risks - Low. Test harness only; no product code changes. - A shared cache means all runs depend on one receipt per image digest. Receipts are already written atomically by renaming the staging dir, and each one is keyed by the immutable digest. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
82a1fda9b1 |
feat(recovery): settle observed-workspace preservation fences on the shared path (#20022)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native run on a remote sandbox fails mid-turn with unsaved files, Paperclip pins an observed-workspace preservation fence. The sandbox is stopped and retained, never deleted, and the run is settled only after the provider confirms the stop > - The fence is held by the run's expired coordinator lease. Its release proof is the provider's stop receipt, and only the environment-settlement worker acted on it > - The ownership reconciler could not see these fences at all: they sit on the preserved-workspace recovery action, which is resolved (no replay) while the fence still waits > - The durable ownership plan has every in-progress record visible to the one reconciler, and settled from it once its owner is proven gone > - This pull request makes the fence visible, proves it by the provider stop, and settles it through the same function the worker already uses > - The benefit is that a fence whose sandbox stopped can no longer sit unseen, and expiry alone never releases one ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Observed-run workspace preservation fence"). **What happened?** `recoveryActionKind` lists only unresolved recovery actions, so `observed_workspace_preservation_fence` reported nothing: its fence lives on a resolved action. The kind registered no settle, and its fence owner never carried the provider proof. **Expected behavior** Every `stop_pending` fence is listed. The fence is `gone` (`provider_stopped`) once its run's one remote lease has a termination receipt and a confirmed stop-and-retain. The reconciler then settles it with `settleStoppedObservedWorkspace`, which re-checks every fence field under its own locks. An expired coordinator lease without that receipt is `unprovable` and is never released. ## What Changed - `recovery/ownership-registry.ts`: - `recoveryActionKind` takes `includeResolved`, which only the observed fence uses. - `resolveFenceHolders` adds `provider: "stopped"` for observed fences whose run lease carries the stop proof. - The kind registers a settle that calls `settleStoppedObservedWorkspace`. A source-busy lock conflict is kept for a later pass. - `native-observed-workspace-preservation.ts`: pinning a fence wakes the ownership worker in the same transaction. - The settling-kinds list in `ownership-reconciliation-report.test.ts` is updated. ## Verification - `heartbeat-observed-workspace-preservation.test.ts`: 29 pass. The new test "settles the fence from the shared reconciler once the provider stop is proven, never before" fails on `master` and passes here: - a pinned fence with an expired lease is `fence_expired_without_provider_proof` and is not settled; - after the provider stop, a read-only pass sees `provider_stopped` and changes nothing; - the settle moves the fence to `stopped` and the coordinator to `terminal_failure`, and the saved files are untouched. All 15 existing "keeps unsafe observed source held" cases and the stale-evidence cases still pass. - `ownership-reconciliation-report.test.ts` (25) passes. - The server package typecheck passes. ## Risks - The kind's listing now includes resolved recovery actions that carry a `stop_pending` fence. Like every recovery-action kind, it is an evidence-predicate scan; no new index is added. - The settle is the existing worker's function with all its fence checks. The reconciler is a backstop for that worker, not a new release rule. ## 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> |
||
|
|
7c56147bb3 |
fix(workspaces): a busy-workspace holder whose controller lease is held is live, however silent (#20023)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Two runs must never write one workspace at once. A run that finds a workspace held (a remote workspace's writer run, or a running holder of a shared local tree) waits behind it > - The holder counted as live only while it produced output: after 60 minutes of silence it was "stale" > - For the shared local tree, that meant the waiter went ahead while the silent holder might still be working, which is a second writer. > - For a remote workspace, the waiter failed instead of waiting. > - Silence is not proof a run stopped. A long tool call can be silent and alive > - The durable ownership plan proves ownership by identity or by a held fence, never by age > - This pull request treats a holder whose controller still renews its lease (the legacy controller lease or the native coordinator lease) as live, however silent. The 60-minute rule applies only once that fence lapses > - The benefit is that a silent but live holder is never overtaken. A dead holder is still settled by the run reapers, which wake the waiters ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C, and Phase 3 E), #19838 (inventory: "Remote workspace writer lane", and the "Released only by TTL/age" list: "Remote writer lane: 60 min"). **What happened?** `WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS` (60 min of output silence) decided whether a running holder still held a workspace. This applied in four places: - the remote reservation deferral; - the parked busy-workspace wait; - `findSharedWorkspaceHolder`, which stopped counting the holder at all; - `findExecutionWorkspaceHolder`, which flagged it stale. **Expected behavior** A running holder whose controller lease is held is live by fence. Only a holder with no held fence is judged by the silence cutoff, which keeps the visible failure instead of parking forever. The check is database-only (`runOwnerFenceHeldSql`): no process probe on the workspace-preparation path before a turn starts. ## What Changed - `remote-workspace-writer-hold.ts`: new `runOwnerFenceHeldSql`. It is true while `heartbeat_runs.controller_lease_expires_at > now()` or the run's native coordinator lease is held. `RemoteWorkspaceWriter.ownerFenceHeld` is reported for a running writer. - `heartbeat/workspace-preparation.ts`: the reservation keeps deferring behind a silent writer whose fence is held. - `run-dispatch/adapters/workspace-busy-wait.ts`: a parked wait behind a running holder with a held fence stays parked and re-checks at the parked cadence. - `heartbeat/retries.ts`: `findSharedWorkspaceHolder` counts a silent holder whose fence is held. `findExecutionWorkspaceHolder` flags a holder stale only when it is silent and its fence lapsed. ## Verification - `heartbeat/workspace-preparation.test.ts`: 138 pass. The extended "waits for the live writer of an inherited remote … at reservation" (both modes) fails on `master` and passes here: a silent writer with a held lease defers, and once the lease lapses it is the 409 as before. - `heartbeat-workspace-busy.test.ts`: 41 pass. The new "still defers behind a silent holder whose controller renews its lease" fails on `master` and passes here. The existing "does not defer when the only holder has been silent past the staleness threshold" (no lease) still passes. - The server package typecheck passes. ## Risks - A silent holder whose controller is alive now blocks waiters for as long as it holds its lease. That is the safe direction. A hung but live run is the active-run silence watchdog's to handle, and a dead controller's lease lapses within a minute. - The extra predicate is an indexed lookup of the run's own row and its coordinator row. ## 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> |
||
|
|
46f32ed33f |
fix(sentry): stop treating short host settings as credentials in run-failure redaction (#20013)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When an agent run fails, Paperclip sends a redacted failure report to Sentry so the team can fix the cause > - Redaction treats the server's own environment values as possible secrets and removes them wherever they appear in the report > - Since #19461, every value under an environment name the allowlist does not know counts as a secret, even very short ones such as `vi`, `RUN`, `off` or `1` > - On a host with such settings, the report text is damaged: `service` becomes `ser***REDACTED***ce`, and a `1` anywhere can vanish. One server test failed on every such host, while CI (which has no such values) stayed green > - This pull request keeps the fail-closed rule for an agent's configured environment, and requires a plausible credential in the server's own environment > - The benefit is that Sentry reports stay readable on every host, and real credentials stay private ## Linked Issues or Issue Description Refs #19461 (introduced the catch-all), #14585 (added the failing test). **What happened?** `heartbeat-process-recovery.test.ts` › "redacts opaque environment-bound credentials from Sentry diagnostics: result" failed locally on `master`: `expected 'ser***REDACTED***ce' to be 'service'`. Instrumentation showed that the server's own `process.env` contributed these redaction values: `["false","67591","true","0x0","RUN","off","vi","64","1","0","8"]`. They came from a desktop agent session's environment. **Expected behavior** Short host settings are not credentials. Declared, secret-named and credential-shaped values stay private. An agent's configured environment keeps its fail-closed rule. **Steps to reproduce** Run that test on a machine whose environment contains `SOME_UNKNOWN_NAME=vi`. ## What Changed - `packages/adapter-utils/src/command-redaction.ts`: `collectDiagnosticEnvironmentSecrets` gets an option, `{ hostProcess: true }`. With it, an undeclared value under an unrecognized name is collected only in these cases: - it has at least 8 characters and is not a bare number, hex number or boolean-like word; or - its name has a credential segment: `TOKEN`, `SECRET`, `PASSWORD`, `BEARER` or `PRIVATE`, or a final `KEY` or `KEY_ID`. Without the option, behavior is unchanged. - `server/src/services/run-failure-report.ts`: the report collects the server's own `process.env` with `hostProcess: true`. The agent's configured environment (`heartbeat.ts`) and the provider environment (`terminal-session-failure.ts`) keep the default. ## Verification - `command-redaction.test.ts` (42 pass). The new test fails on `master` and passes here. It checks three things: the host's short values are not collected; a short value under `AWS_ACCESS_KEY_ID` and a long opaque value stay private; and an agent binding `CUSTOM_BINDING: "1"` stays private. - `run-failure-diagnostics.test.ts` (135 pass, unchanged). These tests pin the agent-environment fail-closed rule. - `heartbeat-process-recovery.test.ts` › "redacts opaque environment-bound credentials…": both modes pass here. The `result` mode failed on unchanged `master` on this host. - The adapter-utils and server typechecks pass. ## Risks - The server's own environment now leaves out a non-credential-named value that is shorter than 8 characters or is a bare number or boolean, for example `MY_PIN=1234`. Values under credential names, declared secrets and credential-shaped values are still redacted. - No Telemetry path changes; this is the Sentry run-failure report. ## 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> |
||
|
|
484170d13b |
feat(recovery): prove copyback owners on the shared path by their PID space (#20018)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a server restarts in the middle of copying a native run's workspace back, the run keeps a receipt that names the controller doing the copyback, so a second controller never writes the same workspace > - A successor may take over only after proving that controller and its copyback children stopped. Three places did this with their own hostname check and then judged the PID on this host: the finalization takeover, the local stop proof, and the board confirmation > - A hostname is not a PID space: two containers can share one, so a dead PID here says nothing about a controller there > - The durable ownership plan proves owners by identity through one shared prover, keyed by PID space > - This pull request records the PID space on the receipt and routes all three checks through one shared prover. It also judges the RC-50 copyback-owner hold on the shared path, and a proven-gone owner wakes its waiting recheck > - The benefit is that a takeover can never be authorized by the wrong host's PIDs, and a hold whose owner is provably gone is rechecked now rather than after its backoff ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Copyback-owner hold (RC-50)", "Native copyback owner receipt", and the "One fact, many proofs" row `proveLocalWorkspaceOwnerStopped`). This follows #19997, which left `proveLocalWorkspaceOwnerStopped` alone. **What happened?** The receipt recorded `hostname` but no PID space, so every check compared hostnames and then probed the PID in this PID space. The registry could not judge the RC-50 hold (`processSpace: null`), so a hold whose owner was dead stayed on its backoff. **Expected behavior** Receipts carry `hostHash`. `proveCopybackOwnerGone` judges the owner in that space; another space is unprovable (`foreign_host` / board attestation). Receipts written before this change keep the previous same-hostname rule. The hold's owner is judged by the shared prover, and `gone` makes its waiting recheck due now. ## What Changed - `native-workspace-owner-stop-proof.ts`: new `copybackOwnerStamp`, `proveCopybackOwnerGone` and `copybackOwnerOnAnotherHost`. `proveLocalWorkspaceOwnerStopped` uses the shared prover. Its process-group orphan check is kept unchanged, because copyback children are a separate proof. - `native-workspace-finalization-ownership.ts`: the receipt records `hostHash`, and the takeover uses `proveCopybackOwnerGone` in place of the hostname compare plus `evaluateNativeControllerTakeover`. - `native-workspace-owner-recovery.ts`: the board confirmation refuses a live or locally unverifiable owner. An owner in another PID space stays the board's to attest, and `hostHash` must match the recorded receipt. - `workspace-recovery.ts`: new `wakeCopybackOwnerRecheck`. It makes a waiting `copyback_stop_unconfirmed` recheck due now (respecting provider cooldown and the deadline), grants no attempt, and only applies while the hold still names the proven owner. - `recovery/ownership-registry.ts`: `copyback_owner_hold` uses the shared stamp and registers a settle that wakes the recheck. It never resolves the hold itself. - The settling-kinds list in `ownership-reconciliation-report.test.ts` is updated. ## Verification - `native-workspace-owner-restart-recovery.test.ts`: 18 pass. Three new tests fail on `master` and pass here: - the receipt records its PID space; - a same-hostname receipt from another PID space is never judged by local PIDs: the takeover and the stop proof both refuse, and the board can still attest; - the RC-50 hold is judged `pid_dead` and its recheck is woken with no attempt spent, while a receipt with no PID space stays unprovable. The guard "a stamped owner in this PID space proven gone resumes copyback" passes on both. - `native-workspace-finalizer.test.ts` (65), `workspace-recovery-preparation-gates.test.ts` (47), `native-workspace-finalization-work.test.ts` (25) and `ownership-reconciliation-report.test.ts` (25) pass. - The server package typecheck passes. ## Risks - Receipts written before this change behave as before: the same-hostname rule, judged in this PID space. - The board confirmation path is unchanged for owners on another host. For an owner on the same hostname but in another PID space, it now asks the board instead of trusting local PIDs. - The hold settle only reschedules an existing recheck. The worker still proves the copyback process group before taking ownership. ## 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> |
||
|
|
beec70ad9f |
fix(native): judge sync-retention run processes with the shared run-process prover (#20025)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run syncs its workspace into a local folder. A retention job deletes old folders, but never one whose run's adapter process may still write it > - That job had its own liveness check. It probed the recorded PID in this PID space, whatever host recorded it, and compared process start times to the millisecond > - So a dead PID here could delete the synced files of a process still running on another host. And a live process whose recorded start differed by under a second (`ps lstart` has 1 s precision) counted as a recycled PID, so its folder could be deleted too > - The durable ownership plan proves owners by identity with one shared prover. The lost-process reapers already use it for exactly this run identity (#20004) > - This pull request makes retention use that prover, so anything but a proof the process is gone keeps the folder > - The benefit is that preserved files are no longer deleted on a wrong-host or sub-second-skew judgment ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2), #19838 (inventory: "One fact, many proofs"). Phase 2's native agent flagged `runProcessMayBeLive` in `native-workspace-sync-retention.ts` as still bespoke after #19997. **What happened?** `runProcessMayBeLive` ignored `process_host_hash` and required an exact start-time match. **Expected behavior** The same verdict as the lost-process reapers (`proveRunProcessGone`): - the PID is judged only in its recorded PID space; - rows saved before `process_host_hash` keep the one-release local fallback on single-host servers; - a reused PID needs a definitely later start; - EPERM is alive. ## What Changed - `native-workspace-sync-retention.ts`: `runProcessMayBeLive` calls `proveRunProcessGone` (reading `process_host_hash`). The injectable retention probe is adapted to the shared probe for tests. Remote-only runs keep their existing shortcut. ## Verification - `native-workspace-sync-retention.test.ts`: 59 pass. Two new tests fail on `master` and pass here: - a run recorded in another PID space is kept, even if that PID is dead here; - a live PID whose start time is within the 1 s precision is kept. The new "deletes a terminal run whose process, recorded in this PID space, has exited" and all existing retention cases (recycled PID, dead process, destroyed sandbox, live group) pass. - The server package typecheck passes. ## Risks - Folders of runs recorded on another host are now kept until their other owners settle, rather than deleted on a local PID probe. That is the safe direction; the folders are bounded by the existing retention limits. ## 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> |
||
|
|
9dcf1e5c43 |
fix(chat): tell chat agents a task they cannot find may be private (#20027)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People talk to agents in chat, and the chat agent acts with the agent's own access > - Tasks can be private, and an agent cannot see a private task unless it has access > - The chat guidance did not tell the agent this, so a hidden task looked like a missing task > - The agent then searched through many other tools, endpoints and files, and the turn ran out before it answered > - This pull request adds one paragraph of chat guidance: a task the agent cannot find may be private, stop searching, and tell the person how to share it > - The benefit is a fast, honest answer that keeps the privacy boundary and gives the person a real next step ## Linked Issues or Issue Description **What happened** A person asked a chat agent to summarize a private task in the same company. The agent has no access to that task, so search returned nothing. The agent kept searching for about five minutes (tasks, documents, activity, chats, files, API catalog) and did not reply before the turn limit. **Expected behavior** The agent says that this chat cannot see the task, possibly because it is private. It does not search for a way around the boundary or guess the content. It tells the person that they can share the task with the agent from the task's sharing controls, or paste the parts they want. **Steps to reproduce** 1. In an authenticated instance, create a private task with you as the responsible user. 2. Do not share it with the CEO agent. 3. Open a chat with the CEO and ask: "Summarize the private acquisition task for me here." 4. Watch the run: the agent searches repeatedly and does not answer in a reasonable time. ## What Changed - `server/src/services/agent-conversations.ts`: add a chat guidance paragraph. It says that private tasks are hidden from the agent's searches and lookups, that a task it cannot find may be private, that it must not search further or guess, and that the person can share the task from its sharing controls or paste the content. - `server/src/__tests__/agent-conversations.test.ts`: add a regression test for the new guidance. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-conversations.test.ts` passes (50 tests). - Chat eval `access.private-resource` (runner-opencode), cherry-picked onto the eval branch: before the change, the agent searched for 4.5 minutes with no reply and the cell timed out. After the change, the run took 39 seconds and passed. The reply: "I can't see a private acquisition task from this chat. ... it's likely private or not shared with me. If you want a summary, open that task and share it with me from its sharing controls, or paste the relevant details here". ## Risks - Low risk. This is prompt guidance only. No API or permission behavior changes. - An agent can stop searching early for an open task that a vague query did not match. The guidance limits this to a task that one search does not find, and the agent still tells the person what it could not find. > 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`), 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> |
||
|
|
b6f58dc6e2 |
feat(recovery): settle runtime-control operations of proven-gone owners on the shared path (#20016)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed runtime control (start, stop, restart, repair, run) on an execution workspace writes a `running` operation row that blocks every other control on that workspace > - If the server that ran it crashes, the row stays `running` until the next control request sweeps it. The sweep had its own `kill(pid, 0)` check, with no process start time, so a reused PID looked alive until 60 s of heartbeat silence passed > - The durable ownership plan proves owners by identity through one shared prover and settles them from one reconciler > - This pull request stamps the owner's boot id and start time, judges it with `proveOwnerGone`, and registers a settle for the kind > - The benefit is that a workspace stranded by a crashed server frees itself in the next reconciler pass, and a reused PID no longer holds it ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Managed runtime-control operation", and the "One fact, many proofs" table row `ownerProcessIsAlive` workspace-operations.ts). **What happened?** `workspace_runtime_control_operation` was a report-only kind with a bespoke liveness check: `ownerProcessProvenGone` probed the PID with signal 0 and had no start time. A server restart that reused the PID kept the operation "alive" until its heartbeat aged out, and nothing settled the row until another control request arrived. **Expected behavior** The owner is proven by the shared prover: a dead or reused PID in the stamp's PID space is gone. Another host, a stamp without a PID space, or a live PID without a recorded start time stays unprovable and keeps the heartbeat lease as its fenced takeover at the next control. ## What Changed - `workspace-operations.ts`: - The owner stamp adds the server's `bootId` and `processStartedAt` (`currentRuntimeControlOwner`, no I/O). - The new `runtimeControlOperationOwnerStamp` is shared with the registry. - The sweep proves the owner with `proveOwnerGone`. `ownerProcessProvenGone` is deleted. - The terminal CAS and log append are factored out (`claimRuntimeControlTermination`, `appendRuntimeControlReconciliationLog`). - New `settleRuntimeControlOperation` for the reconciler: it skips operations this process is driving, requires the same owner as the proof, CAS-fails the row and writes a `workspace_operation.reconciled` activity entry. - `recovery/ownership-registry.ts`: the kind uses the shared stamp and registers the settle. - The settling-kinds list in `ownership-reconciliation-report.test.ts` is updated. ## Verification - `workspace-runtime-control-recovery.test.ts`: 21 pass. The 6 new tests are in "shared ownership path". 5 fail on `master` and pass here: - a reused PID is recovered at once; - the reconciler settles a dead owner with activity and frees control; - an unprovable owner is never settled, and a read-only pass changes nothing; - a re-stamped owner is kept; - new operations carry the boot id and start time. The live-owner guard passes on both. - `ownership-reconciliation-report.test.ts` (25), `workspace-operations-reconciliation.test.ts` (9) and `execution-workspace-runtime-control-conflict.test.ts` (24) pass. - The server package typecheck passes. ## Risks - Rows written before this change have no start time. A live PID on such a row stays unprovable, and the 60 s heartbeat lease still applies at the next control. A dead PID is still proof. - The reconciler now terminalizes these rows. It does so only on `gone`, and only with the same owner and an `updated_at` CAS. ## 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>canary/v2026.1108.0-canary.12 |
||
|
|
2aac7d29ff |
feat(recovery): settle workspace runtime leases of proven-gone owners on the shared path (#20015)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task that starts, stops or repairs a workspace's runtime services takes a runtime lease, so two tasks never drive the same workspace at once > - A lease whose owner task or run has ended must be released. Until now that happened only when the next claimant arrived and checked, or when 30 minutes of inactivity passed > - The durable ownership plan releases a record when its owner is proven gone, by identity and never by age, through the shared reconciler > - This pull request registers the runtime lease with the reconciler. Proof releases a lease; the 30-minute idle rule stays only as a fairness policy at claim time > - The benefit is that a workspace locked by a finished task frees itself at once, and a live task's lease is never taken by age ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Execution workspace runtime lease", and the "Released only by TTL/age" list). **What happened?** `execution_workspace_runtime_lease` was a report-only kind. A lease whose issue or run had ended stayed held until another claim came, or until its 30-minute TTL expired. The claim path also mixed the TTL with the ownership check. **Expected behavior** An issue- or run-owned lease is released when its owner is proven gone: the issue is missing, hidden or terminal, or the run is missing or terminal. Age is not proof. An agent-scoped owner has no identity to prove, so the lease's expiry plus the next claim's locked takeover is the fenced handoff. ## What Changed - New `workspace-runtime-lease-settlement.ts`: `settleExecutionWorkspaceRuntimeLease` locks the row, requires the same owner key, re-proves that owner gone, and only then deletes the lease (compare-and-swap). - `workspace-runtime-leases.ts`: the new `proveWorkspaceRuntimeLeaseOwnerGone` holds the identity proof. The claim path checks it first, then applies the 30-minute idle reclaim as a separate fairness policy. - `recovery/ownership-registry.ts`: the kind registers the settle. Owners are now `issue`, `run`, or a fence on `expires_at` for agent-scoped keys. `RecordOwner` gains an `issue` kind. - `recovery/ownership-invariant-report.ts`: issue owners are classified (missing, hidden or terminal means gone). Salvaged from the stopped Phase 2 workspace agent's uncommitted work. I resolved it against current `master` and verified it. ## Verification - New `workspace-runtime-lease-ownership.test.ts`: 8 pass. Six fail on `master` and pass here: - a crash after the claim; - hidden or missing issues, and terminal or missing runs; - an agent-scoped lease never settles; - a read-only pass changes nothing; - a lease taken over after the proof is kept; - the release reason names the ownership proof, not the idle policy. Two guards pass on both: a live owner is never released by age, and the idle reclaim still works at claim time. - `ownership-reconciliation-report.test.ts` (25), `workspace-runtime-leases.test.ts` (10) and `execution-workspace-runtime-lease-route.test.ts` (4) pass. - The server package typecheck passes. ## Risks - The reconciler now deletes leases whose owner is proven gone. It always re-proves under the row lock against the exact owner key. - The idle reclaim behaves as before at claim time. ## 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> |
||
|
|
a8ab1d3d94 |
fix(skills): resolve slug references in skillId route param (#19808)
The skillId route param handler called getById with the raw path segment. A slug such as inbox-triage then reached the uuid id column, Postgres rejected the cast, and the request returned 500. The handler now resolves the reference with getByRouteRef (UUID, unique slug, or key) and rewrites req.params.skillId to the skill's UUID. An unknown reference returns 404. Fixes PAPERCLIP-BACKEND-6H Co-Authored-By: Devin Foley <devinfoley@users.noreply.github.com> |
||
|
|
2175b8db93 |
fix(chat): link the agent profile page that holds a control, including API keys (#20008)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents send people to controls in the board UI when an action needs a human > - Agent API keys are board-only, so a chat agent must send the person to the agent's API keys controls > - The Refine-v2 agent page (#18378) removed API keys from the profile navigation; the page exists only at `/<prefix>/agents/<id>/api-keys` > - Chat guidance only told agents to link the bare profile, so replies said "open the profile and use the API keys section", which the person cannot find > - This pull request tells chat agents to link the profile subpage that holds the control > - The benefit is a one-click route to the right control, including the unlisted API keys page ## Linked Issues or Issue Description Refs #18378 **What happened** A person asked chat to give an agent a new API key. The agent correctly said that agents cannot create keys. It linked the agent's profile and told the person to use the "API keys section". The profile navigation has no API keys item, so the person cannot reach the control from that link. **Expected behavior** The reply links the agent's API keys page directly, so the person can create the key in one click. **Steps to reproduce** 1. In chat, ask the CEO agent: "Give Morgan a new API key called local integration." 2. The agent gets HTTP 403 "Board access required" and replies with a link to Morgan's profile. 3. Open the link. The profile navigation shows no API keys item. ## What Changed - `server/src/services/agent-conversations.ts`: chat guidance now says to link the profile page that holds a control (`/api-keys`, `/identity`, `/runtime`, `/secrets`, `/permissions`). It says the API keys page is not in the profile navigation. - `server/src/__tests__/agent-conversations.test.ts`: regression test for the new guidance and its presence in the task prompt. ## Verification - `cd server && npx vitest run src/__tests__/agent-conversations.test.ts` (49 passed) - `cd server && npx vitest run src/__tests__/codex-local-execute.test.ts` (25 passed; it embeds the directive) - End to end: the `agents.agent-api-key` chat eval. Its human step follows the reply link to the API keys controls. ## Risks - Low risk. The change is prompt text only. Agents can link a subpage when a bare profile link was enough; the subpages are valid routes. - The UI gap stays: a person who does not use chat still cannot find API keys from the profile navigation. A separate UI change can add it back. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
ada44a669a |
feat(recovery): settle stale issue execution and checkout locks on the shared ownership path (#20006)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task is locked to the run that executes it, and the lock must be released when that run ends > - A restart between the run's terminal write and the lock release left the task locked to a dead run until a periodic sweep noticed > - The durable ownership plan moves each in-progress record onto the shared reconciler, which settles a record in its first pass after its owner is proven gone > - This pull request registers the task execution and checkout locks with the reconciler, and makes the lock release promote any messages saved behind it > - The benefit is that a task stuck behind a dead run is freed in the first reconciler pass, and its waiting messages go out at once ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "Issue execution lock" and "Issue checkout lock"), #19956 (work-intent outbox), #20004 (lost-process reapers). **What happened?** The task locks were report-only kinds in the ownership registry. A lock whose run had already ended was released only by `sweepStaleIssueLocks` on its own schedule. Messages saved behind the lock then still waited out their 30-second refusal backoff. **Expected behavior** A lock whose owner run is proven ended (a terminal or missing run row) is settled by the reconciler on that proof, never by age. A lock that a same-run resume, an explicit-retry claim, a manual admission hold or a newly live run still needs is kept. ## What Changed - New `server/src/services/recovery/issue-lock-settlement.ts`: `settleIssueLock` runs the stale-lock sweep for exactly that task, so its existing compare-and-swap clear and all of its guards decide. It loads the sweep lazily. - `recovery/ownership-registry.ts`: `issue_execution_lock` and `issue_checkout_lock` register `settleIssueLock`. - `recovery/service.ts`: clearing a stale lock now calls `promoteDeferredWorkIntents(trigger: "lock_released")` before its write, in the same transaction. - The periodic sweep stays, because its pre-pass also hands lost-process runs to the single reaper (#20004). The reconciler is now the faster backstop for the lock itself. ## Verification Each focused file ran alone, in one process, on embedded Postgres: - `ownership-reconciliation-report.test.ts` (25 pass). A new test covers dead, live and resuming locks: a read-only pass never settles; the dead run's lock is cleared with an `issue.stale_lock_cleared` row; the live and same-run-resume locks are kept. It fails against `master`'s registry and passes here. The test that lists settling kinds now includes both lock kinds. - `recovery-stale-issue-lock-sweep.test.ts` (49 pass). A new test checks that a message saved behind a stale lock is made due when the lock is cleared. It fails with `master`'s `service.ts` and passes here. - The server package typecheck passes. ## Risks - The reconciler can now clear a lock that only the periodic sweep cleared before. It goes through the same compare-and-swap and guards, so the outcome is the same, only sooner. - More passes call the sweep for a single task. Each call is scoped to one issue. ## 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>canary/v2026.1108.0-canary.11 |
||
|
|
7c508d3e42 |
fix(recovery): collapse the two lost-process reapers onto one stamp and the shared owner prover (#20004)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, it must decide whether each running agent's process is gone before it releases the run > - Two reapers made that decision for legacy runs, with different rules and different outcomes. One also waited 5 minutes even when it could already prove the process was dead (RC-27) > - The durable ownership plan moves every such decision onto one prover, `proveOwnerGone`, and never releases work by age alone > - This pull request collapses the two reapers onto one owner stamp and the shared prover, removes the age wait for a proven-dead process, and never releases a process it cannot prove gone > - The benefit is that a lost process ends one way, at once, with its normal retry, and a process on another host is never released by mistake ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C), #19838 (inventory: "One fact, many proofs" and the age-only list), #19871, #19883, #19953, #19997. **What happened?** - `recoverActiveExecutions` (heartbeat) and `terminalizeOrphanedRunningRun` (recovery service) both judged a lost legacy child. They used different liveness rules (EPERM counted as alive in one, dead in the other) and gave different outcomes: `failed/process_lost` with a retry in one, `interrupted/orphaned_running_run` with no retry in the other. - The active-execution reaper kept a 5-minute age guard even when the child was provably dead. - Hot-restart adoption judged the child with raw PID checks. **Expected behavior** One prover decides. A child that is proven gone is settled at once, through one path with one outcome. An unprovable child (another PID space, a sandbox, or a pre-#19871 row on an ephemeral server) is never released, and Stop is its operator action. ## What Changed - New `server/src/services/run-process-owner.ts`: `lostProcessOwnerStamp` and `proveRunProcessGone`. They wrap the shared `runProcessOwnerStamp` (#19997) with the one-release legacy fallback (#19883) and the group-leader case. - `heartbeat/recovery.ts`: - The active-execution reaper judges a legacy child with the shared prover. A child proven gone skips the age guard; master's exemption for a proven-gone controller (#19953) is kept. - An unprovable child gets the detached marker, whose message says to stop the run. - The legacy host fallback is recorded as `LEGACY_HOST_FALLBACK_EVENT`. - `heartbeat/recovery.ts` (hot-restart adoption): a run is marked lost only on a `gone` proof. An unprovable child stays adopted and held. - `recovery/service.ts`: the stale-lock sweep no longer judges a legacy child. It hands the run to `settleLostRunProcess`, which is the active-execution reaper wired in `heartbeat.ts`, and then reads the outcome back. Native runs keep the sweep's own backstop, now judged with the shared prover. Salvaged from the stopped Phase 2 recovery agent's uncommitted work, which had no tests. I completed and corrected it: the salvaged adoption marked unprovable children lost, which risked duplicate execution. ## Verification - `recovery-stale-issue-lock-sweep.test.ts` (51 pass). Three new tests fail on `master` and pass here: - "hands a legacy run whose child is proven gone to the one lost-process reaper"; - "settles a legacy run whose child is proven gone without waiting out the age guard"; - "never releases a legacy run whose child is unprovable, and names Stop as the way out". Updated tests: - The PID-space test now expects `failed` for the one legacy outcome. - The explicit-retry race now injects `settleLostRunProcess`. - The two tests of the sweep's own terminal write use native runs, which keep that path. - `heartbeat-process-recovery.test.ts`: 418 of 419 pass. The one failure, "redacts opaque environment-bound credentials from Sentry diagnostics", also fails on unchanged `master`. - `heartbeat/recovery.test.ts` (97), `ownership-reconciliation-report.test.ts` (24), `native-remote-retry-admission.test.ts` (92) and `heartbeat-provider-limit-wait.test.ts` (4) pass. - The server package typecheck passes. ## Risks - Behavior change: a legacy run whose child died, when the orphan sweep finds it, now ends `failed/process_lost` with the reaper's retry policy, instead of `interrupted/orphaned_running_run`. - A pre-#19871 row on an ephemeral server, or a child in another PID space, now stays `running` with a detached marker until Stop. It is no longer released by age. - Not changed: a dispatched legacy run that recorded no PID and has no controller stamp keeps its lease-expiry plus age fence. Rows written since #19953 are judged by the controller proof. Older rows, and remote handoff, keep the fence. A different host is never proof, so removing the fence would strand runs after an ephemeral replacement. ## 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> |
||
|
|
946c098bf3 |
fix(opencode): bind the attached run's contract after a restored launch (#20001)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native OpenCode agents run through runnerd and a small OpenCode proxy. The proxy checks each `paperclip_finish` claim against the run's completion contract. > - Each follow-up run has its own contract revision. #19874 lets runnerd keep the OpenCode process across a contract change, and sends the new contract on `turn/start`. > - When runnerd starts again for a follow-up, it restores the saved session first. The restored proxy launches with the saved run's contract, but its environment already names the new run. > - The proxy applied the `turn/start` contract only when the run id changed. Here the run id did not change, so the proxy kept the saved contract and refused every correct finish claim. > - This pull request makes the proxy rebind whenever the turn's contract differs from the one its session holds. The stale-revision error also names the expected revision. > - The benefit: a queued-comment follow-up finishes on its first try, and an agent that sends a stale revision can repair it. ## Linked Issues or Issue Description Refs #19874 (warm runs carry their own completion contract). **What happened** A user sent a follow-up comment while an OpenCode agent was working. After the first run finished, Paperclip started a continuation run for the queued comment. The agent updated the plan and called `paperclip_finish` with the current revision `2`. The tool refused it twice with "Semantic result completion contract revision does not match". The error did not name the expected revision. The agent searched for it for 3 minutes, and cleanup cancelled the run. Root cause: runnerd restores a saved `session_open` session before it applies `run.attach` (`provider_backend.rs`: "execute() restores the durable provider before dispatching run.attach"). The restore launches the OpenCode proxy with the saved contract (revision `1`). Runnerd's environment already names the new run, so the proxy's `PAPERCLIP_RUN_ID` is the new run. `attach_run_in_place` keeps this provider for a changed contract (#19874) and sends `paperclipCompletionContract` revision `2` on `turn/start`. The proxy compared only the run id. The run id matched, so the proxy skipped `attachRun` and kept revision `1`. **Expected behavior** The continuation's first `paperclip_finish` with the current revision is accepted. If a claim has a stale revision, the error names the expected revision. **Steps to reproduce** 1. Use a native OpenCode agent with warm follow-ups (the default). 2. Start a task. While the agent works, post a follow-up comment, so that it queues. 3. Let the first run finish. Paperclip starts a continuation run with completion revision `2` and restores the saved OpenCode session. 4. The agent calls `paperclip_finish` with `contractRevision: "2"`. 5. Before this fix, the tool refuses it with "Semantic result completion contract revision does not match". The chat eval `steering.discard-queued-message` (runner-opencode) shows this in run events: `session.resumed` carries revision `2`, and both `paperclip_finish` results fail with the mismatch. ## What Changed - `opencode-app-server-proxy.ts`: record the contract that the session opened with. On `turn/start`, call `attachRun` when the run id changes or when the supplied `paperclipCompletionContract` differs from the session's contract. - `opencode-proxy-task-envelope.ts`: add `sameOpenCodeProxyCompletionContract`. - `opencode-server-driver.ts`: the stale-revision error now ends with `Use contractRevision "<expected>".`, like the criteria error. - Tests: a real-proxy regression for the restored launch, a driver test for the new error text, and a `repair-revision` mode in the fake OpenCode fixture. ## Verification - `pnpm exec vitest run src/cli/opencode-proxy-input.test.ts src/drivers/opencode/opencode-server-driver.test.ts src/cli/opencode-app-server-proxy.test.ts src/cli/opencode-proxy-thread-read.test.ts` in `packages/paperclip-runner`: 88 passed. - Red/green: without the source changes, both new tests fail. `binds the current contract when a restored launch already names the attached run` never sees the finish call reach the controller. `names the expected revision in a repairable stale-revision tool error` does not find the expected revision in the error. - `npx tsc --noEmit -p packages/paperclip-runner`: no errors in the changed files. (My worktree is missing a link for `@paperclipai/paperclip-eval-kernel`, which causes unrelated errors in `src/eval/workflow-harness.ts`.) - End-to-end: the chat eval `steering.discard-queued-message` (runner-opencode) with this fix cherry-picked. The run link goes in a comment below. ## Risks - Previous-release story (`rules/upgrade-safe-run-state.mdc`): this change does not change a persisted shape. The regression test uses the saved-state shape: launch parameters from the saved run, environment and `turn/start` from the new run. A runnerd that does not send `paperclipCompletionContract` behaves as before, so the proxy keeps its launch contract. The proxy and runnerd ship in the same runner artifact. - The proxy can now call `attachRun` for the same run id when the contract changes. `attachRun` resets the per-run result state. This occurs only at `turn/start`, when no turn is active, and only when the contract is different. A result for the earlier contract is not valid for the new contract. - The error text changed. No code parses it. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
201163793a |
fix(chat): route another company's task identifier to a company switch (#19995)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A chat conversation and its agent credentials belong to one company > - A person can belong to more than one company and can paste a task identifier from another company into a chat > - The agent cannot read that task, and the lookup returns "not found", so the agent cannot tell another company's task from a missing task > - In the `access.other-company` chat eval, the agent searched for more than three minutes, decided the task did not exist, and asked the person to paste the brief > - This pull request adds chat guidance: a different identifier prefix means another company that this chat cannot read, so the agent tells the person to switch to that company and open a chat there, with a link to the task > - The benefit is a fast, correct refusal that gives the person a usable next step and keeps the company boundary ## Linked Issues or Issue Description **What happened** A person in company Acorn asked the Acorn chat agent to "Open BIR-1 and copy its brief into our launch task". BIR-1 belongs to the person's other company, Birch. The agent correctly could not read it (404). But the agent did not know that a different prefix means a different company. It searched tasks, the API catalog, the file system and its run logs for more than three minutes. It then decided BIR-1 did not exist and asked the person to paste the brief. The turn ran out of time before it posted a reply. **Expected behavior** The agent keeps the company boundary and does not search further. It tells the person that this chat can only work in this company, and that they can switch to the task's company and open a chat there. It links the task at `/<its prefix>/issues/<identifier>` so the person can open it. **Steps to reproduce** 1. Sign in as a person who is a member of two companies. 2. Open a chat with an agent in the first company. 3. Ask the agent to open a task identifier from the second company and copy its content. 4. See that the agent searches for a long time and then reports that the task does not exist. ## What Changed - `server/src/services/agent-conversations.ts`: add one paragraph to `AGENT_CHAT_DIRECTIVE`. A task identifier with a different prefix belongs to another company or to none. A "not found" lookup does not prove that the task does not exist. The agent does not search further, guess or copy. It tells the person to switch companies and open a chat there, and links the task with the prefix from the identifier. - `server/src/__tests__/agent-conversations.test.ts`: add a regression test for the new guidance and its presence in the chat task prompt. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversations.test.ts` (47 passed). - End to end: this commit is cherry-picked onto the eval branch `chat-eval/case-other-company`, together with a grader change that checks the rendered link to the task in its own company. The agent's reasoning quoted the new guidance. The full-stack chat eval `access.other-company` passed in https://github.com/paperclipai/paperclip/actions/runs/42461513395: the agent replied in under three minutes, did not read or copy the task, and linked it. Following the link as the person opened the task in its own company. ## Risks - Low risk. The change is prompt guidance only. It does not change permissions or API behavior. - The guidance names the task's own prefix for one link. Other company links still use this conversation's prefix. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
e7e870cfc4 | fix(native): decide warm and adopted Runner process reuse from one key (#20005) | ||
|
|
1444b0d151 |
fix(chat): keep every listed task linked and send long lists to a document (#19994)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People ask agents in chat to show company work, for example all open tasks in a project. > - The chat links each full task identifier (such as `RUN-12`) to its task. The chat guidance tells agents how to write task links. > - The guidance says nothing about long lists. An agent that must list 501 tasks can compact them into ranges such as `RUN-76–82`. Only the first task of each range then gets a link. > - The person then cannot open most of the tasks from the reply. > - This pull request adds one guidance paragraph: write each identifier in full, and put a list that is too long for the reply in a document on the conversation's task, linked from the reply. > - The benefit is that every task in a long answer keeps a source link. ## Linked Issues or Issue Description **What happened** A chat agent was asked to show all open work in a project with 501 open tasks, grouped by owner. Its reply had correct groups and counts. It wrote the identifiers as ranges, for example `RUN-1–2, RUN-76–82`. Chat linked only the first identifier of each range. 161 of the 501 tasks had no link. The agent's reasoning said it chose ranges because 501 separate links "would be enormous". **Expected behavior** Every task that a chat reply lists has a source link. When the list is too long for a reply, the agent puts the complete list in a document on the conversation's task and links that document from the reply. **Steps to reproduce** 1. Create a project with more than 500 open tasks. 2. In chat, ask the CEO agent: "Show me all open work in this project, grouped by owner, with blocked items first." 3. Read the reply. Identifiers written as ranges have a link only on the first task of each range. ## What Changed - `server/src/services/agent-conversations.ts`: the chat directive has a new paragraph after the agent-profile link paragraph. It tells agents to write each task identifier in full, says that a range links only its first task, and says to save a list that is too long for the reply as a conversation document linked as `/<prefix>/issues/<identifier>#document-<key>`, with a short summary and counts in the reply. - `server/src/__tests__/agent-conversations.test.ts`: a new test checks the three parts of the paragraph. ## Verification - `cd server && npx vitest run src/__tests__/agent-conversations.test.ts`: 47 passed. The new test fails without the guidance change (1 failed, 46 passed). - The `#document-<key>` issue anchor is an existing UI route (`ui/src/lib/document-annotation-hash.ts`), and the server already builds it in `company-search.ts` and `feedback.ts`. - End-to-end: the `tasks.organize-open-work` chat eval runs this change on its case branch. ## Risks - Low risk. The change is guidance text only. There is no schema, API, or UI change. - The directive is slightly longer, so each chat turn has a few more prompt tokens. - An agent can still write a long inline list. The guidance does not force a document. > 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`), agentic coding with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
a4dbde6181 |
refactor(recovery): judge native owner processes with the shared prover (#19997)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs record the server and adapter processes that own them, and recovery must prove an owner gone before it takes over or releases its work > - Phase 2 of the durable ownership plan gave the repository one prover, `proveOwnerGone`. The native runtime still had four private proofs of the same fact > - The private proofs disagreed with the shared rules in edge cases: some judged a PID from another PID space, and one accepted an inconsistent start time as proof of reuse > - This pull request moves those four proofs onto `proveOwnerGone` and deletes the private liveness code > - The benefit is one rule for "the owner is gone" across native recovery, with the stricter shared behaviour where they differed ## Linked Issues or Issue Description Refs #19933, #19950, #19953 (the shared ownership path and earlier Phase 2 kinds). **What is the problem?** Native recovery proved dead owners with four private checks instead of the shared prover. The checks used different rules for PID spaces, reused PIDs and unreadable start times. **What should happen?** Every native owner proof uses `proveOwnerGone`. Only a `gone` verdict releases or takes over. The remote fence and the one-release legacy fallback stay as they are. **Why does it matter?** Recovery decisions about the same process must agree. A dead PID in another PID space must never be used as proof. ## What Changed - `native-restart-recovery.ts` `evaluateNativeControllerTakeover`: the local-process branch now calls `proveOwnerGone`. The remote fenced branch (#19830), the `operator_attested` sentinel and the null-host fallback (`processSpaceUnrecorded` with `legacyHostFallback`, recorded by callers as `legacy_host_unknown_dead_pid_proof`) are kept. Reasons are unchanged, plus a new `controller_pid_start_inconsistent`. Owners built from receipts with no PID space are judged in this PID space: their callers check the recorded host first. - `native-failed-run-owner-release.ts`: `recordedProcessMayBeLive` is deleted. `runProcessMayBeLive` and `controllerMayBeLive` release only on a `gone` verdict. Each use of the legacy fallback is recorded as `legacy_host_unknown_dead_pid_proof` when the release commits. - `native-session-executor.ts`: `verifiedRecoveryProcessIsAlive` is replaced by `recoveryProcessMayBeAlive`, which is false only when the prover proves the runner gone. Signalling a recovered runner requires a proven same process. - `native-local-process-stop.ts` `recordedLocalExecutionStopped`: the bound identity (launch event, host, PID, group, start time) is judged by the prover. It uses `process_host_hash` when the row has one. - New `native-runtime/native-owner-stamps.ts`: controller and adapter-process stamp builders, shared with the ownership registry. Not changed: `proveLocalWorkspaceOwnerStopped` (workspaces), `evaluateNativeProviderProcesses`, the stop observation in `recordNativeLocalProcessStop`, and `runProcessMayBeLive` in `native-workspace-sync-retention.ts`. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-restart-recovery.test.ts src/services/controller-process-identity.test.ts`: 51 passed. - `cd server && npx vitest run src/services/native-runtime/native-failed-run-owner-release.test.ts`: 14 passed. - `cd server && npx vitest run src/services/native-runtime/native-controller-host-recovery.test.ts`: 2 passed. - `cd server && npx vitest run src/services/quarantined-task-retry.test.ts -t "keeps quarantine"`: 18 passed. `explicit-native-continuation.test.ts`: 414 passed. `ownership-reconciliation-report.test.ts`: 23 passed. - Red with the master versions of the two modules: 3 new tests fail ("accepts a reused PID only when the live process started definitely later", "never releases on a dead PID recorded in another PID space", "keeps the one-release fallback for a run process saved before its PID space, and records it"). The tests were written after the code. - Two fixtures changed to match the shared rules: - `controller-process-identity.test.ts`: the "recycled" case recorded a start after the live process, which is an inconsistent stamp. It now records an earlier start (a real reuse), and the inconsistent case is asserted as alive. - `quarantined-task-retry.test.ts` `worker_alive`: the live worker now records this process's real start time. The old fixture was a reused PID, which the prover correctly proves gone. - `cd server && npx vitest run src/services/native-local-process-stop.test.ts`: 5 passed (review round 1). With the master module, 2 fail: the other-PID-space case and the reused-PID case. - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks Behaviour changes, all from the shared rules: - A live PID observed to start before the recorded controller is no longer accepted as recycled (`controller_pid_start_inconsistent`; the run waits for evidence). - A failed-run owner whose recorded adapter process or controller is in another PID space, or is a remote sandbox PID, is no longer released by a local probe. It keeps the lease path. - An unreadable start time is no longer exit evidence for a recovered local runner. - A local execution whose PID was reused by a later process now counts as stopped. Before, any live PID blocked the proof. - A local execution whose row records another PID space is no longer counted as stopped by a dead local PID. - A PID space that cannot be read here makes a hostless receipt owner unprovable. No persisted state changes. Rows saved by the previous release keep the one-release null-host fallback, as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, local test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
8475826d2d |
fix(wake-queue): deliver user comments whose wake a restart lost or a stale continuation dropped (#19992)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user's follow-up comment on a task must reach the agent exactly once, through restarts (invariant 4) > - The final unmasked local chaos storm (96 tasks, 31 restarts) passed 94 tasks. The 2 failures were both lost user comments > - In t69, a restart killed the server between saving the comment and writing its wake. In t72, a comment's own run was cancelled as stale, because the earlier run completed the task first without reading the comment > - This pull request adds a reconciler rule that gives a lost comment its wake, and saves the comment before a stale cancellation > - The benefit is that a user's message is never silently dropped when a restart or a completion race hits at the wrong moment ## Linked Issues or Issue Description Refs #19956 (work-intent outbox), #19944, #19831 (plan, invariant 4). **What happened?** In chaos wave `w-20261108T055444-s9401` (local, unmasked, codex and claude, 30-min K13 storm, master `46bf913fb9`), 94 of 96 tasks passed. Two failed: - **t69, R1 `rested_not_done`.** The user's comment was saved at 06:14:59.358 and reopened the task to `todo`. A K1 SIGKILL fired 28 ms later, before the wake was written. No wake row exists. Recovery skips a `todo` task whose last run succeeded, so the task sat with no run until the wave ended. - **t72, R4 `concurrent_runs`.** A comment posted during a run queued its own run. That run was claimed at 06:16:04.874. The first run's completion marked the task `done` 2 ms later. The comment's run was then cancelled with `continuation_task_ownership_changed` before dispatch, and the comment was lost. **Expected behavior** Each accepted user comment is delivered once, unless the task is cancelled or reassigned, or a person closes it. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --replay w-20261108T055444-s9401/t69` (or `.../t72`). ## What Changed - `recovery/ownership-invariant-report.ts`: a new reconciler-owned I4 rule, `user_comment_without_wake`. It finds a user comment that meets all of these conditions: - it is newer than every run of the task; - the task is assigned, is not a conversation, has no live run or lock, and has at least one earlier run; - no wake request carries the comment. A comment younger than 30 s is not reported yet, because it may still get its normal wake. Its end-of-grace time is returned as `nextDueAt`. - `native-runtime/native-finalization-reconciler.ts`: the backstop step is now exported as `applyOwnershipReportBackstops`. For each such comment it saves the missing outbox receipt (`executionWait: wake_lost`) and promotes it. - `work-intent-outbox.ts`: new `saveUndeliveredUserComments`, which the reconciler rule and the stale-run path both use. It saves only live user comments that no completed run received. It writes nothing for a task that is reassigned, is a conversation, or has a status the caller does not allow. - `heartbeat/queue.ts`: the saved-receipt pass that already sends pause-saved messages also sends `wake_lost` receipts. It uses the same per-comment consumption check and the same chat-input exclusion. - `heartbeat/stale-comment-continuation.ts` and `heartbeat.ts`: before a stale continuation is cancelled, its undelivered user comments are saved as a deferred receipt. The release drain then reopens the `done` task and delivers them once. This applies only when an agent run that started before the comment completed the task after it. A task that a person marks done is left closed. - `tests/restart-chaos/grader.ts`: R4 ignores runs cancelled before dispatch (`continuation_task_ownership_changed`) when it looks for overlapping execution. Those runs never executed. Comment consumption still counts them. ## Verification Each focused file ran alone in one process (`--maxWorkers=1`): - `heartbeat-held-comment-wake.test.ts` (22 pass). This includes two new cases: - t69 shape: the reconciler rule fires, the backstop delivers the comment once, and a second pass does nothing; - t72 shape: the stale run is cancelled, the task reopens, and the comment is delivered once. Both fail with the rule or the save disabled, and pass with them. - `ownership-reconciliation-report.test.ts` (23 pass). It includes the rule's boundaries: a carried comment, a too-young comment (deadline only) and an already-read comment. - `tests/restart-chaos/grader.test.ts` (17 pass). The new R4 case fails with the old grader. A cancelled run that is not a pre-dispatch cancellation still fails R4. - Unchanged and passing: comment wake batching (27), refused assignment wake (3), wake-queue postgres adapter (103), stale queue invalidation (68), deferred receipt work (45). The first version also reopened a task that a person had marked done, which broke one deferred-receipt test. That case is now excluded. - The server package typecheck and the restart-chaos harness typecheck pass. - Same-session chaos context on master: K6 crash loop `w-20261108T063326-s9402`: 6 of 6 pass. The K4 long-hold wave is still running. After merge, I'll replay only the targeted fault shapes (t69 and t72). ## Risks - The reconciler rule runs on each report pass. It is bounded by the 1-hour lookback and the per-kind row cap, and it excludes conversation tasks, so chat input keeps its own authorization. - A comment younger than 30 s waits up to the end of its grace period before the backstop acts. - The stale-run save reopens a `done` task only for a comment that the completing run could not have read. ## 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> |
||
|
|
4638e5ea30 |
fix(issues): make the queued-comment discard denial final and link the task (#19990)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People queue follow-up messages on a task while its agent works, and only the author can discard a queued message > - When a person asks a chat agent to cancel their queued message, the agent tries the discard API and gets a 403 > - The 403 text does not say that the denial is final, so the agent searches for a board or admin route before it answers > - In a slow model this search takes many minutes, and the queued message can be delivered before the person gets the answer > - This pull request makes both denials say that only the author can discard the message, where to do it (with the task link), and that no override exists > - The benefit is that the agent can relay the human route at once ## Linked Issues or Issue Description **What happened** A person asked the chat agent to cancel a contact-section request that they had queued on a worker's task. The agent called `DELETE /api/issues/:id/queued-comments/:commentId` and got 403 "Board access required". It then called `DELETE /api/issues/:id/comments/:commentId` and got 403 "Only the comment author can cancel queued comments". In two eval runs the agent then continued to search the API for an override. It answered after 10 minutes and 24 minutes. In the second run, the worker consumed the queued message before the answer arrived. **Expected behavior** The denial tells the agent that the action is author-only and final, and gives the task link where the author can discard the message. The agent can then answer the person immediately. **Steps to reproduce** 1. Start an agent run on a task. Queue a comment on that task as a board user. 2. As a different agent, call `DELETE /api/issues/<task>/queued-comments/<comment>` with the current `queueId` and `revision`. 3. See 403 "Board access required", with no next step and no statement that the denial is final. Refs the chat eval case `steering.discard-queued-message`. ## What Changed - `DELETE /issues/:id/queued-comments/:commentId`: an agent actor now gets a 403 that says only the author can discard the queued comment from its menu on the linked task, and that no board, admin or agent override exists. The task must be in the agent's company. Otherwise the route returns 404 as before. - `DELETE /issues/:id/comments/:commentId` (queued cancel by a non-author): uses the same message. - Tests: the agent denial on the queued-comments route, the cross-company 404 without task details, and the updated non-author cancel message. ## Verification - `cd server && pnpm exec vitest run src/__tests__/issue-comment-cancel-routes.test.ts` passes (9 tests). - I will run the chat eval `steering.discard-queued-message` with this change cherry-picked, and record the result in a comment. ## Risks - Low risk. Only the 403 text changes, and agents now get the 403 after the company access check instead of before it. Board behavior and the author-only rule do not change. - The UI shows this text if a non-author person tries to cancel a queued comment. The text stays correct for that person. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use (shell, file editing) 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> |
||
|
|
dc0534d1b5 |
feat(recovery): destroy a retained orphan sandbox once nothing references it (#19976)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs use remote sandboxes held by environment leases > - #19964 settles an orphaned active lease when its holder is proven gone. It stops and keeps the sandbox, because recovery may still need its files > - Nothing destroyed that retained orphan later, so a stopped sandbox could stay at the provider indefinitely > - This pull request destroys a retained orphan once no recovery, preservation or retention obligation references it, from the reconciler pass only > - The benefit is that orphaned sandboxes do not leak, and a sandbox that recovery still wants is never destroyed ## Linked Issues or Issue Description Refs #19964 Refs #19933 **What happened?** After #19964, an orphaned sandbox is stopped and kept (`released` with a stop-and-retain receipt). No path destroyed it when recovery no longer needed it. **Expected behavior** A retained orphan is destroyed once database facts show that no obligation references it. It is never kept past the environment's retention window (the provider-attested lease `expiresAt`) only because of a soft reference. A sandbox that an active operation or a live recovery path still needs is never destroyed. **Steps to reproduce** 1. Let a run's server die after the run is terminal and before its sandbox lease is released. 2. Let the ownership settle stop and keep the sandbox. 3. Resolve the run's recovery. The stopped sandbox stays at the provider. ## What Changed - `sandbox-stop-and-retain.ts`: an ownership settle's stop intent also writes a top-level `orphanedRunLeaseRecovery` marker. The marker stays after the stop intent is removed. - `environment-retained-orphan-settlement.ts` (new): the settle for `environment_retained_orphan_sandbox`. It locks the lease row and checks database facts: - the run is terminal; - always held: native export resume, stop-and-retain intent, in-flight cleanup claim, workspace restore recovery, workspace recovery cleanup, bound source-recovery consumer, pending legacy shutdown fence, or another live lease on the same provider resource; - held while the stop receipt is a usable source (`expiresAt` not passed): pending native export or preservation fence, open recovery action on the issue, or a queued, scheduled or running run on the issue. - Then it moves the lease to `pending_cleanup` (ephemeral) with a compare-and-swap. The pending-cleanup sweep destroys it from the recorded lease. The settle never calls a provider. - `recovery/ownership-registry.ts`: the kind lists only releasable retained orphans (an SQL mirror of the checks), so a held orphan is never polled. It reports `nextDueAt` = the earliest retention-window end of a softly held orphan. - `recovery/ownership-invariant-report.ts`: an `OwnableKind` may report `nextDueAt`; the pass folds it into the report's next due time, so the reconciler wakes when the window ends. - `environment-cleanup-state.ts`: the first destroy attempt of a released orphan is due at once. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts src/__tests__/ownership-reconciliation-report.test.ts src/__tests__/environment-run-lease-ownership.test.ts src/__tests__/heartbeat-orphaned-active-lease-sweep.test.ts src/__tests__/heartbeat-pending-cleanup-sweep.test.ts` (embedded Postgres). - Red on master, green here (tests written after the code): the settle marks the lease; an unreferenced retained orphan is destroyed through the cleanup sweep; a softly held orphan waits until its window ends and the pass reports that due time. - Counter-cases (green): never destroyed with an open recovery action, a queued run, a scheduled retry, a stop-and-retain intent, a native export resume, a workspace restore recovery, an in-flight claim, a pending legacy shutdown fence, another live lease on the resource, or an active run; a read-only pass changes nothing; a retained sandbox without the orphan marker is left alone. - Full typecheck is left to CI (local memory limit). ## Risks - A retained orphan saved by #19964 before this change has no marker, so it is not destroyed by this path. The window was short (since #19964 merged). - A held orphan with no `expiresAt` is released on the first reconciler pass after its last reference clears. If that clearing writes no signal the reconciler wakes on, the release waits for the next pass. - The SQL listing mirrors the settle's checks except the pending native export, which the settle checks in code. Such an orphan stays on the settle watch until that export settles. > 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 edit, 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>canary/v2026.1108.0-canary.9 nightly/v2026.1108.0-nightly.0 |
||
|
|
2fed77086f |
fix(recovery): date a settled orphan's first cleanup even under a scalar metadata root (#19989)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #19980 made the environment cleanup queue the owner of an unclaimed pending-cleanup item, with a due time and an overdue check (I7) > - A settled orphan's first cleanup attempt is dated by `pendingCleanupRecoveredAtMs`, which the orphaned-run-lease settle writes > - The settle merged that key with `coalesce(metadata, '{}') || ...`. For an array or scalar metadata root that produces an array, so the key is not at the top level > - The queue owner then falls back to the old `updatedAt` and can report a fresh item as overdue > - This pull request builds the merge from the object-root reader that the cleanup sweep already uses > - The benefit is that a fresh orphan is never reported overdue because of a malformed metadata root ## Linked Issues or Issue Description Refs #19980 Refs #19964 **What happened?** The review of #19980 found that the orphaned-run-lease settle wrote `pendingCleanupRecoveredAtMs` under an array metadata root as an array element. #19980 merged before this fix landed on it. **Expected behavior** The key is always top level, so the queue owner dates the first cleanup attempt correctly. **Steps to reproduce** 1. Save an active run lease whose metadata root is a JSON array. 2. Let its run end and the ownership settle move it to `pending_cleanup`. 3. Run the ownership pass: the item can read as `queue_overdue`. ## What Changed - `environment-run-lease-settlement.ts`: the merge starts from `pendingCleanupMetadataObjectSql()`, which reads a scalar or array root as an empty object. - `environment-run-lease-ownership.test.ts`: a legacy lease with an array metadata root is settled with the key at the top level. ## Verification - `cd server && npx vitest run src/__tests__/environment-run-lease-ownership.test.ts src/__tests__/environment-pending-cleanup-queue-owner.test.ts` (embedded Postgres). - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. It changes only how one key is merged into a lease that the settle moves to `pending_cleanup`. A malformed root becomes an object, which the cleanup sweep already does for its own keys. > 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 edit, 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> |
||
|
|
838e9bf3d2 |
feat(recovery): settle sandbox creation intents of dead creators on the shared ownership path (#19978)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before it asks a provider to create a sandbox, the server writes a creation intent that names its creator process > - If the creator dies during the provider call, the intent waits for a 30-minute settle deadline before recovery reconciles it with the provider > - After a restart, creation recovery made such intents due early with its own process check (`promoteAbandonedSandboxCreations`) > - Phase 2 of the durable ownership plan moves each in-progress record kind onto one shared owner proof and deletes its bespoke check > - This pull request registers a settle for `sandbox_creation_intent` and calls the shared path where creation recovery called the bespoke promotion > - The benefit is one proof for every kind, while the provider absence proof and the cross-host handoff stay unchanged ## Linked Issues or Issue Description Refs #19964 Refs #19933 **What happened?** Sandbox creation recovery had its own liveness check (`controllerProcessExited`) for the creator of a pending intent. Each kind with its own check can drift from the shared rules (PID space, PID reuse, current boot, deployment replacement). **Expected behavior** The shared prover judges the creator. On gone, the intent becomes due now and its creator is marked lost. Provider absence stays inconclusive until the settle deadline. A creator on another host is never proven gone; its intent keeps the deadline. **Steps to reproduce** 1. Start a sandbox acquire so a creation intent is written. 2. Kill the server before the provider reply is committed. 3. Restart. The intent must become due through the shared ownership path, not a separate check. ## What Changed - `sandbox-creation-intent-settlement.ts` (new): compare-and-swap settle on the exact creator and a deadline that is still in the future. It sets `nextAttemptAt` to now, marks `creationControllerLost` with the proof reason, and wakes creation recovery in the same transaction. - `recovery/ownership-registry.ts`: `sandbox_creation_intent` registers the settle. An intent whose creator is already lost is no longer listed; it belongs to the creation recovery deadline. - `sandbox-creation-operations.ts`: creation recovery calls `settleOwnableKind(db, "sandbox_creation_intent")`. Deleted `promoteAbandonedSandboxCreations` and its use of `controllerProcessExited`. ## Verification - `cd server && npx vitest run src/__tests__/sandbox-creation-intent-ownership.test.ts src/__tests__/sandbox-creation-operations.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - The existing lost-creator matrix stays green on the shared path: destroy on the next sweep; page past unprovable creators; issue-bound creation after a later run; reused PID; live creator here; live creator in another process; creator on another host; early lookup finds nothing yet (absence inconclusive); committed lease untouched. - New, red on master and green here (written after the code): the reconciler pass makes a dead creator's intent due now and keeps the absence deadline. - New counter-cases (green): live creator, another host, and an intent without a creator keep their deadline; a read-only pass changes nothing. - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. The settle writes the same state the deleted promotion wrote, plus the proof reason. - Previous-release rows: every creation intent already stored its creator with a PID space, so no fallback is needed. > 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 edit, 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> |
||
|
|
02898b98de |
fix(issues): do not wake the assignee for an issue created already closed (#19961)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Creating an issue with an agent assignee queues an `issue_assigned`
wake for that agent
> - The create path skips the wake only for `backlog` and the onboarding
first task
> - An issue created already `done` or `cancelled` is a record of
finished work, but it still wakes the assignee
> - The update path already refuses this case ("Restoring an assignee on
completed work is not a reopen request"), and create-time fast responses
already skip `done` and `cancelled`
> - This pull request applies the same closed-status guard to the
create-time assignment wake
> - The benefit is that recording finished work does not start a run,
spend tokens, or queue behind the agent's current work
## Linked Issues or Issue Description
**What happened**
In a chat eval, setup created a historical issue with `status: "done"`
assigned to an agent that was already running a task. The create queued
a second run for that agent (`invocationSource: assignment`,
`wakeReason: issue_assigned`, `source: issue.create`). The run waited
behind the active run, and then was cancelled or failed. The eval
expected no work for a finished record.
**Expected behavior**
Creating an issue that is already `done` or `cancelled` does not wake
its assignee. This matches the update path, where an assignee change on
closed work is not a reopen request.
**Steps to reproduce**
1. Create an agent.
2. `POST /api/companies/<companyId>/issues` with
`{"title":"Record","status":"done","assigneeAgentId":"<agentId>"}`.
3. See a new `agent_wakeup_requests` row and a queued heartbeat run for
the agent.
## What Changed
- `server/src/routes/issues.ts`: the create route does not call
`queueIssueAssignmentWakeup` when the created issue status is `done` or
`cancelled` (`isClosedIssueStatus`).
- `server/src/__tests__/issue-onboarding-first-task-routes.test.ts`: a
regression test for `done` and `cancelled` creates. The existing test
that an ordinary assigned create still wakes is unchanged.
## Verification
- `cd server && pnpm exec vitest run
src/__tests__/issue-onboarding-first-task-routes.test.ts`: 11 passed.
- Negative control: with the route file from `origin/master`, both new
cases fail.
## Risks
- Low risk. Only creates in a closed status change. A caller that relied
on a create-time wake for a closed issue no longer gets one. To start
work, the caller creates the issue in an open status or reopens it,
which still wakes the assignee.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
c6781f5b2c |
fix(chat): keep company links written as //PREFIX/... inside the app (#19986)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People ask chat agents about their work, and agents answer with Markdown links to tasks, runs and agents > - A chat agent sometimes writes a company link target with two leading slashes, for example `(//ACME/agents/<agentId>/runs/<runId>)` > - A browser reads that target as a link to a website whose host is `ACME`, so the link leaves the app and fails > - Task links still worked because the issue-reference parser finds `/issues/<id>` in any path, but run, approval and agent links did not > - This pull request renders such a link as the company path when its "host" is exactly a known company prefix, and tells chat agents to start link targets with one slash > - The benefit is that a person can open the run, approval or agent the agent linked ## Linked Issues or Issue Description No public issue exists. Related, but different: #17864 (same-origin absolute links). **What happened** In a chat-feature eval, a person asked what happened on a failed website attempt. The agent read the run, explained the failure and linked both runs as `[...](//ACME/agents/<agentId>/runs/<runId>)`. The rendered anchors kept `href="//ACME/..."`, which opens `http://acme/...`. The agent made the same `//` slip in an earlier attempt of the same case. **Expected behavior** A link to a company page opens that page in the app. A target like `//ACME/...`, where `ACME` is a company prefix, can only mean the company path `/ACME/...`. **Steps to reproduce** 1. Render Markdown `[Failed run](//ACME/agents/a/runs/r)` in a chat or comment while the viewer belongs to company prefix `ACME`. 2. Inspect the anchor. 3. See `href="//ACME/agents/a/runs/r"`. Click it, and the browser goes to host `acme`. ## What Changed - `ui/src/components/MarkdownBody.tsx`: add `companyPathFromProtocolRelativeHref`. When a link target is `//<host>...` and `<host>` is exactly one of the viewer's company prefixes, render it as `/<host>...` (query and fragment kept). Other protocol-relative links, absolute URLs and renders without known prefixes stay unchanged. - `server/src/services/agent-conversations.ts`: chat guidance now says to start each link target with exactly one slash, because a `//` target points to a different website. - Tests for both. ## Verification - `cd ui && pnpm exec vitest run src/components/MarkdownBody.test.tsx`: 69 passed. With the normalization removed, the new company-path test fails (red/green checked). - `server/src/__tests__/agent-conversations.test.ts` gets one added `toContain` assertion for the new guidance sentence. I could not run that suite locally in this worktree (its linked workspace packages are from an older checkout), so CI supplies it. - End to end: the change is cherry-picked onto a chat-eval case branch and run through the remote `runs-adapters.inspect-failed-run` eval. ## Risks - Low risk. The rewrite applies only when the protocol-relative host matches a company prefix the viewer already has, exactly and case-sensitively. A real single-label intranet host with the same uppercase name as a company prefix would now open the company path instead. Chat guidance already forbids invented hostnames for Paperclip links. - Guidance text grows by one sentence. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |