mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
2175b8db933b5ce85b6531d44d5fbb7319e9ed46
2592
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
3ff62fc6db |
feat(recovery): model an unclaimed pending-cleanup item as owned by the cleanup queue (#19980)
<!-- 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 > - A sandbox that still needs teardown waits as a `pending_cleanup` environment lease until the cleanup queue retries it > - The ownership reconciler checks invariant I1: every in-progress record names an owner > - An unclaimed pending-cleanup item names no process by design, so I1 reported every one (61 in the Phase 2 data) > - That noise hides real ownership gaps > - This pull request models the item honestly: the cleanup queue owns it, with a visible next action at its saved retry deadline > - The benefit is that I1 measures real gaps, and a queue that stops acting shows up as an I7 gap instead of silence ## Linked Issues or Issue Description Refs #19933 **What happened?** `environment_pending_cleanup` listed every unclaimed pending-cleanup item with owner `none`, so I1 reported each as `no_owner_stamp`. **Expected behavior** The item names the queue that acts on it and its next due time. It is reported only when the queue is not acting on it (long past the deadline). No owner is invented, and nothing is released. **Steps to reproduce** 1. Let a sandbox teardown fail so its lease waits in `pending_cleanup`. 2. Run the ownership reconciliation pass. 3. See an I1 `no_owner_stamp` violation for a healthy queue item. ## What Changed - `recovery/ownership-registry.ts`: new owner kind `queue` (`queue`, `dueAt`, `graceMs`). `environment_pending_cleanup` names the environment cleanup queue. Its due time is the same retry deadline the cleanup worker sleeps until, including the at-once first attempt of a settled orphan. - `recovery/ownership-invariant-report.ts`: a queue owner is alive (`queue_scheduled`) while its deadline is current. When it is more than two retry intervals past, it is unprovable (`queue_overdue`) and reported as `I7_visible_next_action`. A queue is never proven gone and nothing is settled. - The listing reads the in-flight flag as text, so a malformed value cannot break it. ## Verification - `cd server && npx vitest run src/__tests__/environment-pending-cleanup-queue-owner.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - Red on master, green here (written after the code): a current item names the queue (no I1); a long-overdue item is an I7 `queue_overdue` gap. - Green on both: an in-flight item is not listed here (it is a teardown claim). - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. Report only: no record is changed. - The grace (two retry intervals) is a reporting threshold, not a release rule. A worker that sleeps longer than that for a legitimate reason would show as an I7 gap. - `#19966` changes the same listing predicate to the same text comparison; a rebase of either resolves trivially. > 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> |
||
|
|
95f3cab0cd |
fix(recovery): settle a workspace finalize attempt whose process died mid-copyback (#19985)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The durable-ownership plan settles in-progress records whose owner is proven gone, on one shared path. > - The crash-point suite (#19946) found two windows where a workspace finalize attempt stays `running` forever after a restart, even though its run succeeded through a later attempt. > - #19807's supersession covers only native-finalizer attempts. The local live-finalize path writes rows without that marker. > - This pull request stamps each finalize attempt's owner and settles an abandoned attempt on the shared path. > - The benefit is that no finalize row is left `running` with a dead owner, and the run's outcome never changes. ## Linked Issues or Issue Description Refs #19946 (crash-point suite), #19807 (RC-64 supersession), #19933 (shared settle path). **What happened?** At crash points `workspace.finalize.after_start_before_copyback` and `workspace.finalize.after_copyback_before_done` (run `cp-20261108T040253`, tasks t07 and t08), the run succeeded after the restart. The original `workspace_finalize` operation stayed `running`, and the ownership pass reported I2 (`owner_run_terminal`). **What did you expect?** The interrupted attempt is settled once its owner is proven gone, and the run outcome is left alone. **Steps to reproduce** 1. `node scripts/crash-points.mjs --point workspace.finalize.after_start_before_copyback`. Or run the new report test case below. ## What Changed - `workspace-operations.ts`: every `workspace_finalize` attempt records `finalizeOwner` (the process stamp, with no I/O) and is tracked in a process-local set while it runs (`isWorkspaceFinalizeOperationLive`). - `ownership-registry.ts`: `native_workspace_finalize_attempt` takes its owner from the stamp, or from the run for rows written before the stamp. It now registers a settle. - New `recovery/workspace-finalize-attempt-settlement.ts`: a compare-and-swap that marks the attempt `failed` with `abandonedFinalization` provenance and logs a `workspace_operation.reconciled` activity. It skips an attempt live in this process, an uncertain export and a save-work record. It does not change the run. ## Verification - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 19 pass. The new case covers a dead stamped owner (settled), a current-process owner (kept), another PID space (kept), an uncertain export (kept), a pre-stamp row of a terminal run (settled) and of a running run (kept), a read-only pass (no change), and the run status unchanged. - The finalize-related suites pass (100 tests in 5 files): `heartbeat-workspace-finalize-branch`, `native-workspace-finalization-work`, `workspace-operations-reconciliation`, `offline-workspace-operation-reconciliation`, `finalized-workspace-recovery-migration`. - Red/green: the new case fails on master, where the attempt stays `running`. I wrote the test after the code. - Crash-point re-proof with `--point` for both windows: to be run once the harness slot is free, and reported on this PR. ## Risks - Fail-open risk: settle needs a `gone` proof from the shared prover, and another PID space is never proof. It re-checks the stamp in the transaction and skips attempts this process is running. - Previous-release story: rows without `finalizeOwner` are owned by their run, and they settle only when that run is terminal. - Nothing runs before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
ccce55ab11 |
fix(issues): tell callers when a task list page is capped (#19972)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents read company tasks through `GET
/api/companies/{companyId}/issues`, often through the runner `call_api`
tool
> - The route caps each page at `limit` (500 by default, 1000 at most)
and returns a bare array
> - A capped page and a complete list look the same, so an agent cannot
tell that rows are missing
> - In an eval with 501 open tasks, the agent got 500 rows and reported
498 tasks as the complete list
> - This pull request adds a "more results exist" signal and the
next-page position, and surfaces it to agents
> - The benefit is that agents page until the list is complete instead
of presenting a partial list
## Linked Issues or Issue Description
**What happened**
The chat eval `tasks.organize-open-work` seeds 501 open Launch tasks.
The agent called `GET /api/companies/{companyId}/issues` through
`call_api`. The route returned exactly 500 rows (the default limit) and
gave no signal that more rows existed. The agent noted that exactly 500
rows was suspicious and that some tasks were missing. It still reported
the partial list as complete.
**Expected behavior**
A capped list response tells the caller that more results exist and how
to get the next page. The agent-facing tool result shows the same signal
next to the data.
**Steps to reproduce**
1. Create 501 tasks with status `todo` in one company.
2. Call `GET /api/companies/{companyId}/issues?status=todo`.
3. See that the response has 500 rows and no header or field that says
more rows exist.
## What Changed
- `server/src/routes/issues.ts`: the company issue list fetches one row
past the page. It then sets `X-Paperclip-Has-More` (`true`/`false`) and
`X-Paperclip-Limit` on every page. A capped page also sets
`X-Paperclip-Next-Offset`, and `X-Paperclip-Next-After-Id` for
`sortField=id&sortDir=asc` cursor paging. Pagination is reported only to
actors who can read the whole company's task list; actors whose rows are
filtered after the query (such as low-trust agents) get no pagination
headers, so the raw page cannot reveal tasks they cannot read.
`oneChildPerParent` lists are ordered by parent first, so they page by
offset and never advertise an id cursor. The body is unchanged (still an
array). The Mine inbox path does not change.
- `server/src/services/native-runtime/runner-api-client.ts`: `call_api`
results for successful responses that carry these headers include
`pagination: { hasMore, truncated, limit, nextOffset, nextAfterId?,
guidance? }`. The guidance tells the agent to repeat the call with the
next `offset`/`afterId` until `hasMore` is false. The field is also
present when the body is saved as an artifact. Routes without the
headers get no new field.
- `server/src/routes/openapi.ts`: the API catalog entry documents
`limit`, `offset`, `sortField`, `sortDir`, `afterId`, the headers and
the `call_api` pagination field.
- Tests for capped, cursor, last, exact-size and default-limit (501
rows) pages in all list projections, for offset-only paging of
`oneChildPerParent` lists, for a low-trust agent receiving no pagination
headers, and for the `call_api` pagination field.
## Verification
- `cd server && npx vitest run
src/__tests__/issues-list-query-parsing.test.ts
src/__tests__/low-trust-red-team-routes.test.ts` — 31 passed.
- `cd server && npx vitest run
src/__tests__/issues-list-query-parsing.test.ts
src/services/native-runtime/runner-api.test.ts` — passes. Without the
source change, 8 of the new tests fail.
- `cd server && npx vitest run src/__tests__/inbox-mine-routes.test.ts
src/__tests__/issue-list-assignee-filter-routes.test.ts
src/__tests__/issue-list-updatedsince-filter-routes.test.ts
src/__tests__/openapi-routes.test.ts
src/services/native-runtime/runner-api*` — 2237 passed, 2 skipped.
- `tsc --noEmit -p server/tsconfig.json` shows no errors in the changed
files.
## Risks
- Low risk. The response body shape does not change. Existing clients
ignore the new headers.
- Each list query reads one more row. The extra row is enriched and then
dropped. The cost is small.
- `call_api` results gain an optional `pagination` field. Consumers that
do not know the field ignore it.
- Agents whose task reads are filtered after the query (low-trust
agents) still get no truncation signal, as before; a raw-page signal
would leak hidden tasks. Pushing their boundary into the list query
would let them have one later.
- Other capped list routes do not set the headers yet. They can adopt
the same headers later.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude
Code with tool use (shell, file edits, test 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>
|
||
|
|
bf0d45a4d2 |
feat(recovery): settle teardown claims of dead claimants on the shared ownership path (#19966)
<!-- 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 > - A sandbox teardown writes a claim on its environment lease that fences other cleanup for 15 minutes > - After a restart, the cleanup sweep ended the fence of a claim whose claimant had exited, with its own process check (`releaseLostPendingCleanupClaims`) > - 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 `environment_teardown_claim` and calls the shared path where the sweep called the bespoke release > - The benefit is one proof for every kind: a local claimant proves death by its stamp; another host stays a fence handoff ## Linked Issues or Issue Description Refs #19965 Refs #19964 Refs #19933 **What happened?** The teardown claim had its own liveness check (`controllerProcessExited`) in the pending-cleanup sweep. Each kind with its own check can drift from the shared rules (PID space, PID reuse, current boot). **Expected behavior** The shared prover judges the claimant stamped for this exact attempt. On gone, the fence ends now and a counted sweep attempt is refunded once. A claim held as a fence (another claimant or another host) waits for its deadline. **Steps to reproduce** 1. Start a sandbox teardown so the lease holds an in-flight claim. 2. Kill the server. 3. Restart. The claim must be released by the shared ownership path, not by a separate check. ## What Changed - `environment-teardown-claim-settlement.ts` (new): compare-and-swap settle on the exact attempt and claimant. It ends the fence, refunds a counted attempt, and is single-use. It never acts on a fence owner. - `recovery/ownership-registry.ts`: `environment_teardown_claim` registers the settle. - `heartbeat/recovery.ts`: the sweep calls `settleOwnableKind(db, "environment_teardown_claim")`. Deleted `releaseLostPendingCleanupClaims` and its use of `controllerProcessExited`. - `environment-cleanup-state.ts`: the attempt cap, attempt key and their SQL readers move here, so the settle and the sweep share one reader. - `heartbeat-pending-cleanup-sweep.test.ts`: the overlap barrier now holds the shared prover instead of the deleted check. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-pending-cleanup-sweep.test.ts src/__tests__/environment-lease-owner-stamps.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - The existing lost-claim matrix stays green on the shared path: retry now; no failed-attempt charge for a dead server; one refund when sweeps overlap; keep a live claim; keep a newer claim that kept an older record; keep another host's claim; keep a claim without identity; page past fenced claims. - New, red before and green here (written after the code): the reconciler pass ends a dead claimant's fence and refunds its counted attempt. - New counter-cases (green): a read-only pass never settles; a live claimant, another PID space, and an unstamped fence claim keep their fence. - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. The settle keeps the previous release semantics (fence ends, lease stays `pending_cleanup` and in flight until the sweep claims it again). - The stop-only claim still does not renew its fence; a live stop that runs longer than 15 minutes can still be taken over. This change does not alter that. - Stacked on #19965 and #19964. Merge those first. > 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> |
||
|
|
d33b3ac263 |
fix(wake-queue): save temporarily refused work intents and promote them through one path (#19956)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Users send work to agents as assignments, comments and Retries. The wake queue delivers each one as a run > - When an admission check refused a wake for a temporary reason, the wake often ended as a terminal skip, so the work was lost > - Restart-chaos waves found dropped comments and Retries (RC-37, s7532) and double delivery (RC-53). Each fix covered one hold type > - The durable ownership plan (section E) asks for one outbox rule: a closed set of terminal outcomes, a saved deferral for every temporary refusal, and one promotion path > - This pull request adds that rule. It saves messages refused by a pause, promotes deferred intents from every hold-release event through one function, and adds a reconciler backstop > - The benefit is that an accepted message is delivered exactly once after a pause lifts, and the reconciler reports any intent that is stuck or dropped ## Linked Issues or Issue Description Refs #19831 (plan, section E), #19838 (inventory, wake-queue rows and section 1b), #19927 (Phase 2 data: the I4 false positive), #19702, #19741, #19829, #19826. Builds on #19944 (section D, merged). **What happened?** - A user commented while the agent was paused, or while the company was paused. Admission refused the wake. Nothing saved the message as pending, so when the pause lifted, nothing delivered it. - Hold-release events woke different subsets of workers. A saved intent then waited out a 30-second refusal backoff after its hold cleared. - The I4 reconciler check matched free text with a regular expression. Phase 2 found a false positive: a delivered retry whose error said "retrying once". **Expected behavior** An accepted work intent ends only as delivered, consumed by a later run, task cancelled, or hard stop. Every temporary refusal saves the intent as `deferred` with its reason and its release trigger. Every hold-release event promotes deferred intents through one function (invariant 4). **Steps to reproduce** Pause an agent. Post a user comment on its task. Resume the agent. On `master` the comment is never delivered as a wake. ## What Changed - New `server/src/services/work-intent-outbox.ts`: - `WORK_INTENT_TERMINAL_OUTCOMES`: the closed set of terminal outcomes. - `TEMPORARY_REFUSAL_TRIGGERS`: maps each temporary refusal code to its release trigger. - `promoteDeferredWorkIntents`: the one promotion function. It removes the 30-second refusal backoff from the matching deferred intents. It then wakes run dispatch, the saved-receipt worker and stranded-work recovery, and hints each saved intent of the issue. - `heartbeat/queue.ts`: the #19829 save-as-deferred block is now a shared helper. A user's message refused by a paused agent, a paused company, or a budget pause that is not a reached hard stop is saved with `executionWait: { reason, trigger, message }`. The caller still gets a 409. The message is saved only when the task's latest run is terminal, because the release drain promotes a saved message behind that run. A task that never ran keeps the message in the task, and stranded-work recovery starts the task once the pause lifts. A `pending_approval` agent and a task-tree hold are not changed here: the release drain still fails a promoted wake for a pending approval, and tree holds have their own resume path. - Hold-release events call `promoteDeferredWorkIntents`: - accounting settled (`run-cost-accounting.ts`); - environment lease released (`environments.ts`, two sites); - native run projected terminal (`native-run-projection.ts`); - budget pause lifted (`budgets.ts`); - company reactivated (`companies.ts`); - task-tree hold released (`issue-tree-control.ts`); - agent lifecycle ready (`agent-lifecycle-driver.ts`). - Reconciler (`recovery/ownership-registry.ts`, `ownership-invariant-report.ts`, `native-finalization-reconciler.ts`): - registers three new kinds: `wake_request_deferred`, `queued_run` and `scheduled_retry_run`; - I4 now reads the closed refusal code (the wake `reason`) of accepted intents, not free text; - a new I4 check reports a deferred pause refusal whose pause has lifted for more than 5 minutes. The worker then promotes it as a backstop; - a lifted pause inside its 5-minute grace window returns its deadline through `nextDueAt`, and the worker also wakes on agent-lifecycle signals (company and agent pause changes). So a missed release event is still checked. - `doc/SPEC-implementation.md`: adds the rule "Accepted work intents are delivered exactly once". ## Verification Each focused file ran alone in one vitest process (`--maxWorkers=1`) on embedded Postgres. Results at `2875c85fd2`: | Question | Loop | Elapsed | Result | |---|---|---|---| | Is a comment refused by a pause saved, then delivered once after the pause lifts? | `heartbeat-held-comment-wake.test.ts -t "pause lifts"` | 8 s | 2 pass. With `queue.ts` from `master`, both fail: no intent is saved | | Does I4 use the closed code, and does it catch a stuck deferral? | `ownership-reconciliation-report.test.ts` | 45 s | 20 pass. This includes the Phase 2 "retrying once" false positive and the case "paused, then lifted" | | No regression on the paths that changed | held-comment (15), archived-company guard (8), undispatched execution wait (32), deferred receipt work (45), recovery pause durability (16), native run projection work (55), tree-control routes and services (19, 1, 7) | 8–334 s each | All pass | - Exactly-once: each pause test runs two promotions (a release event, then the reconciler backstop) and many dispatch passes. It asserts one delivery. The existing consumption tests (RC-53) still pass. - Server package typecheck passes. - Speed: nothing new runs before `turn.start`. The new admission work happens only on a refusal. Promotion runs in the release transaction and in the background reconciler. - Not run here: chaos waves. The coordinating session schedules them on the shared harness. ## Risks - `promoteDeferredWorkIntents` moves `updated_at` back for deferred intents that were refused in the last 30 seconds. A busy company with many release events can retry refused intents more often. Each retry re-runs normal admission, and the refusal resets the backoff. - More release sites now wake run dispatch and stranded-work recovery. These notifications are coalesced per company. - Lifting a pause is now the operator's explicit authorization to send what the pause refused, as Send is after a Stop. When the pause stopped the task's run, the lifted pause issues the task's next explicit wake, which adopts the saved message once. Other messages queued behind an acknowledged Stop keep the existing rule. If Stop recovery still holds the task, the message waits for that recovery. - An agent pause's stop sweep keeps the messages that the pause deferred (`preservePauseDeferred`). Terminate and cleanup stops still end them. - The backstop check reads only pause refusals (`company.inactive`, `agent.not_invokable`), because those holds can be checked exactly. Other deferred kinds keep their existing promotion paths. ## 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> |
||
|
|
5742c42b75 |
fix(chat): tell agents to write company links as Markdown links (#19981)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In chat, an agent often gives the person a link to the next place to act: a task, an approval, a run or an agent profile. > - Chat guidance gives the correct paths, but it does not say how to write them. > - The chat makes links from Markdown links and issue identifiers. It does not make a link from a bare `/<prefix>/approvals/<id>` path. > - An agent proposed a hire and wrote the approval as a bare path. The person could not click it to decide the approval. > - This pull request adds one sentence to the chat guidance: write every company link as a Markdown link. > - The benefit is that the person gets a link they can open. ## Linked Issues or Issue Description Refs #19344 (similar guidance for completion feedback about created resources). **What happened** In the `agents.hire-needs-approval` chat eval, the CEO proposed a hire and replied with `- Review the pending hire: /RUN/approvals/<id>`. The chat rendered the path as plain text. No link to the approval was rendered. In an earlier run of the same case, the agent wrote `[/RUN/approvals/<id>](/RUN/approvals/<id>)`, which rendered as a link. The guidance did not say which form to use. **Expected behavior** Company links in chat replies are Markdown links that the person can click. **Steps to reproduce** 1. Set a company to require board approval for new agents. 2. Ask the CEO in chat to hire a QA engineer. 3. Look at the reply. Sometimes the approval path is plain text, and you cannot click it. ## What Changed - `server/src/services/agent-conversations.ts`: the chat directive now says to write every company link as a Markdown link, for example `[Review the approval](/<prefix>/approvals/<approvalId>)`. It says that a bare path or a path in inline code is not clickable. - `server/src/__tests__/agent-conversations.test.ts`: assert the new guidance. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversations.test.ts src/__tests__/codex-local-execute.test.ts`: 71 passed. - An end-to-end rerun of the `agents.hire-needs-approval` chat eval with this change follows on the eval branch. ## Risks - Low risk. This is a change to guidance text only. The prompt gets one sentence longer. - The model can still write a bare path. This change makes that less likely. It does not rewrite replies. ## 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> |
||
|
|
ab82ac0191 |
fix(workspaces): retire recovered workspace copies after a successful turn (#19979)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - With remote (sandbox) execution, every turn ends by saving the whole working folder, so the next turn can continue on any sandbox. > - When a run is cut off mid-turn and its task continues in a new workspace, preparation attaches the dead run's saved workspace under `.paperclip-recovered/<operationId>`, so the agent can recover unfinished work. > - Nothing ever removes that attachment. Every later save archives and uploads it again, and a new sandbox restores it again. > - The attachment is a full workspace, `.git` included, so every save for that task is roughly twice as big. The extra size lasts until the task ends. > - This pull request removes an attachment after a run that was shown it succeeds, and keeps a retired record so it is never attached or shown again. > - The benefit is that affected tasks go back to normal save sizes after their first successful turn, without changing how recovered work reaches the agent. ## Linked Issues or Issue Description **What existing behavior does this improve?** Recovery attachments in remote workspaces (`.paperclip-recovered/<operationId>`). **Subsystem affected** Server: remote workspace preparation and recovery attachments. **Current behavior** `retainRemoteRecoveryAttachments` keeps every recorded attachment on every later preparation, and no code path removes one. On one hosted instance, 10 active workspaces carried 18 attachments. One task's saves went from 746 MB to 1,472 MB at a single save and stayed there. The largest grew from 709 MB to 3.3 GB. A 3.3 GB workspace needs most of a 10 GB sandbox disk during each save. When a task moves again, the new attachment contains the older one, so the copies nest. **Proposed behavior** Each live attachment records the run it was shown to. When that run has succeeded, the next preparation removes the attachment from the sandbox, after checking its exact provenance. It keeps a `retired` record, so: - a sandbox restored from an older save is cleaned up too; - the attachment is never attached or shown to an agent again. **Reason and benefit** A run that saw the attachment and succeeded had a full turn to use it. Removing it then returns the task's saves to normal size and frees sandbox disk. **Breaking changes** None. Records written before this change have no `presentedToRunId`. They are stamped on their next preparation and retired after the following successful run. Related: none found. ## What Changed - `remote-workspace-recovery-attachments.ts`: - New `retiredRecoveryAttachments`. It returns attachments already retired, and attachments shown to a different run that has since succeeded. - `retainRemoteRecoveryAttachments` removes those attachments from the sandbox and keeps a `retired: true` record for each. - `attachPreservedRemoteWorkspaceSnapshots` skips retired operation IDs. - `workspace-recovery-snapshot.ts`: `attachPreservedWorkspaceSnapshots` also skips retired operation IDs. - `remote-workspace-transport.ts`: new `removeRemoteRecoveryAttachment`. - It reuses the attachment probe's checks: a real directory under the workspace root, and an exact `provenance.json`. - It makes read-only directories writable before removing them. - It removes `.paperclip-recovered` once it is empty. - Anything else at that path fails the preparation instead of being deleted. - `remote-workspaces.ts`: computes the retired set before attaching. It writes `presentedToRunId` on each live record, and keeps retired records unchanged. - `heartbeat/workspace-preparation.ts`: never shows retired records to an agent. - `doc/execution-semantics.md`: describes retirement. ## Verification - `pnpm --filter @paperclipai/server typecheck`: exit 0. - New tests: - `remote-workspace-transport.test.ts`: removal against real sandbox commands. It removes an attachment with exact provenance, including a read-only directory, and leaves sibling attachments. It refuses a foreign proof or a symlinked directory without deleting anything. It reports a missing attachment, and removes the empty parent folder. - `workspace-recovery-preparation.test.ts`, new `retired_followup` case. The run shown the attachment succeeds. The next preparation, on a fresh sandbox restored from that run's save, removes the attachment and records it as retired. A later preparation after a failed run still neither attaches nor shows it. - `ordinary_followup` (the agent deleted the attachment itself) now also ends with a retired record instead of an empty list. - A retired operation ID is not attached again, even to an empty destination. - 13 recovery and remote-workspace suites: 836 passed. With the source change reverted, the 3 new or changed preparation cases and the 5 new transport cases fail. ## Risks - **An agent can lose access to recovered work after one successful turn.** That happens if a successful run never looked at the attachment. The source run's saved snapshot in storage is not touched, so an operator can still recover it. - **Retired records stay in workspace metadata.** There is one small record per attachment, and each later preparation sends one sandbox probe per record. - **Nested attachments are unchanged.** If no turn succeeds between two moves, the newer attachment still contains the older one. That older copy may hold work nobody has recovered yet, so this change does not drop it. - Low risk overall: the change is limited to remote workspace preparation and is fail-closed on provenance. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs, read-only database queries). AI-assisted investigation, change and tests, with human direction. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
576ba29fff |
feat(recovery): settle the legacy controller lease on the shared ownership path (#19953)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Legacy (non-native) runs hold a controller lease on `heartbeat_runs` while the server prepares and dispatches the adapter. The shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The lease stamped only a boot id. The reconciler could never prove its owner gone (`missing_identity` in the Phase 2 data), and recovery released it by age: lease expiry (60 s), then a 5-minute age guard on dispatched runs > - A hard-killed server's legacy run therefore stayed `running` for up to 6 minutes after the next boot > - This pull request records the controller process with the claim, registers a compare-and-swap settle, and makes live-run recovery use the shared prover instead of the age guard when the controller is proven gone > - The benefit is that a dead controller's run is settled at once, while a live, unprovable, or previous-release controller keeps today's behavior ## Linked Issues or Issue Description Refs #19933 (the settle hook, merged). Stacked on #19950 (migration 0446, registry) and targets its branch until it merges; then it retargets to master. **What happened?** After a hard kill, a dispatched legacy run stayed `running` until its controller lease expired and its last update was 5 minutes old. **What did you expect?** The next boot proves the old controller gone and fails the run as a lost process at once. **Steps to reproduce** 1. Start a legacy (non-native) run and let it dispatch the adapter. 2. Kill the server (SIGKILL). 3. Start the server. The run stays `running` for up to 6 minutes. ## What Changed - `heartbeat_runs.controller_process` (jsonb, migration 0447): `{bootId, pid, hostHash, processStartedAt}` of the claiming server process. It names the controller only while its `bootId` equals `controller_boot_id`. Revocation swaps the boot id, so a revoked lease never keeps it. - `legacyControllerClaim` writes it in the same UPDATE (from `currentControllerProcessStamp()`, no I/O). - `legacy-controller-lease.ts`: `legacyControllerOwnerStamp`, `legacyControllerProvenGone` (calls `proveOwnerGone`), and `settleLegacyControllerLease` (compare-and-swap expiry of the proven controller's outstanding lease after it wakes live-run recovery). - `ownership-registry.ts`: `legacy_controller_lease` registers `settle` and `listForRuns`. The owner is the bound process; other rows are boot-id fences. The listing covers outstanding leases only. - `heartbeat/recovery.ts` (3 small hunks): a controller of another boot that the shared prover proves gone has its live lease settled through `settleOwnableKind` and skips the age guard. The local child-process check after it is unchanged, so a live child still keeps the run running. - Migration 0447 builds the partial index with `CREATE INDEX CONCURRENTLY` (heartbeat_runs is a known-large table) and is added to `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS`. Bespoke code deleted: none. The kind had no liveness proof; the age guard stays for controllers that are not proven gone. ## Verification - `cd server && npx vitest run src/services/legacy-controller-lease.test.ts`: 33 passed. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red with `recovery.ts` and `ownership-registry.ts` from the base commit (tests, schema and lease module from this branch): 4 failed, 29 passed. Red: the crash-point reaper test, the expired-lease age-guard test, the reconciler settle test, the exact-controller CAS test. The tests were written after the code. - New tests: - the claim stamps this process, and a changed boot id unbinds it; - crash point: a controller dispatched the adapter and was killed with its lease live; the startup reaper (default 5-minute age guard) fails the run as `process_lost` at once; - a dead controller's expired lease skips the age guard; - counter-cases keep the run and lease through the reaper and a reconciler pass: a live earlier-boot PID, a dead PID in another PID space, and a previous-release row with no `controller_process`; an expired lease of a live controller stays behind the age guard; - a read-only pass (`record: false`) never settles; a recording pass expires the lease and leaves the run to live-run recovery; - settle refuses a controller that changed after the proof. - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `controller_process`. They stay boot-id fences with today's release (expiry, revocation, age guard). A rollback ignores the column. - Unprovable controllers (another host or PID space, unknown space, a live PID with an unreadable start time) never release early. Their handoff stays the lease expiry plus revocation, and the age guard. A different host is never proof. - Behavior change: a dispatched run whose controller is proven gone is failed as `process_lost` without the 5-minute wait. The retry decision is unchanged. - The settle expires the lease only. Live-run recovery still settles the run under its own checks. Accounting and workspace obligations are not touched. - The index build is concurrent. A cancelled build is retried by the migrator's invalid-index handling. ## 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> |
||
|
|
0cb9770d1c |
feat(recovery): name the owner of teardown claims and login/probe leases (#19965)
<!-- 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 > - Environment leases carry several in-progress records: login/probe leases and in-flight teardown claims among them > - The ownership reconciler (durable ownership plan, Phase 2) must know who owns each record, or invariant I1 reports a gap > - The ownership data showed one teardown claim and one login/probe lease with no owner stamp > - A teardown claim could also show the wrong owner: a controller stamp left by an earlier attempt named a dead process while a live claimant held the claim > - This pull request names an honest owner for both kinds, without a new write before `turn.start` > - The benefit is that I1 measures real gaps, and the later settle for these kinds can trust the owner it is given ## Linked Issues or Issue Description Refs #19964 Refs #19933 **What happened?** The ownership reconciler reported `environment_teardown_claim` and `environment_login_probe_lease` records as I1 `no_owner_stamp`. Workspace recovery and native export retry claim teardown without a controller stamp. A login/probe lease never had an owner stamp. A teardown claim that kept an older attempt's controller stamp was attributed to that older process. **Expected behavior** Each in-progress record names the owner that can still move it: a process proved by identity, or a fence whose expiry is never proof by itself. A stamp for a different attempt never names the owner. **Steps to reproduce** 1. Let workspace recovery claim a pending-cleanup lease (no controller stamp), or open a login session lease. 2. Run the ownership reconciliation pass. 3. See `no_owner_stamp` for the record, or a dead earlier controller for a claim that a live claimant holds. ## What Changed - `recovery/ownership-registry.ts`, `environment_login_probe_lease`: the owner is the holder process stamped by the write that makes the lease active (`metadata.leaseController`, added in #19964). A lease saved by the previous release still names no owner; it ends with its login session. - `recovery/ownership-registry.ts`, `environment_teardown_claim`: a controller stamp counts only when its `attemptId` equals the claim's `pendingCleanupAttemptId`. Any other claim with an attempt id is a fence: the attempt id is the token and `pendingCleanupLeaseExpiresAtMs` (renewed by the claimant) is the expiry. An expired fence stays unprovable. A claim with no attempt id names no owner. - Report-only. No settle is registered for these kinds in this change. ## Verification - `cd server && npx vitest run src/__tests__/environment-lease-owner-stamps.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - Red on the previous registry, green here (tests written after the code): - login/probe lease is owned by the process whose write made it active - login/probe lease is proven gone when that process has exited - teardown claim never reads a controller stamped for an earlier attempt (fence held by its token) - an unstamped teardown claim whose fence expired stays unprovable, never gone - Green on both: a stamped claim for this attempt with a dead PID is gone; an unstamped login lease and an attempt-less claim report I1. - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. The change only alters the owner the report-only reconciler reads. No record is changed. - Stacked on #19964 (the lease holder stamp). Merge #19964 first. > 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> |
||
|
|
fda58d8cc1 |
fix(chat): post the complete final reply, not a 4,000-character run-log preview (#19971)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in chat. The agent's final message becomes the chat reply. > - The runner cuts every completed agent message to a 4,000-character run-log preview. The server then posts that preview as the reply. > - A long answer, for example a list of 501 open tasks, reaches the person cut off with "…[truncated]". > - PR #19959 fixed only the case where the accepted summary starts with the exact preview text. It missed runs where the final prose differs from the summary. > - This pull request keeps the complete final answer in the runner, and makes the server never prefer a preview over a complete source. > - The benefit is that the person receives the agent's complete reply. ## Linked Issues or Issue Description Supersedes #19959. **What happened** A chat agent wrote a final answer of about 4,985 characters (all 501 tasks). The posted chat reply was a 4,000-character preview that ended with "…[truncated]". **Expected behavior** The person receives the agent's complete final message. A preview of a message is never posted when a complete text exists. **Steps to reproduce** 1. Ask a chat agent (OpenCode or Codex) to list a large set of open work, so that its final answer is longer than 4,000 characters. 2. Let the run finish. 3. Read the chat reply. It ends with "…[truncated]" at 4,000 characters. Root cause: - `packages/paperclip-runner/runner/crates/runner-core/src/provider_events.rs`: the codex/OpenCode facade projection of `item/completed` and the ACPX `AssistantMessage` projection cut the final answer with `bounded_text(text, MAX_TEXT_CHARS)` (4,000 characters). - `durable/state.rs` `sanitize_value` caps every string again at 4,096 bytes before the event goes to the server. - `server/src/services/heartbeat-run-summary.ts` `resolveHeartbeatRunResponse` gives an explicit `final` agent message top precedence, so it posts the preview. ## What Changed - Runner (`provider_events.rs`): a completed `final_answer` agent message keeps its complete redacted text, up to a new 32,000-character `MAX_FINAL_AGENT_MESSAGE_CHARS` bound. Progress prose and in-flight items keep the 4,000-character preview. - Runner (`durable/state.rs`): `preserve_final_agent_message_text` keeps the complete redacted text of a completed final agent message when it fits in half of the transport frame. Otherwise the marked preview stays, so a large message never fails the frame check. - Server (`heartbeat-run-summary.ts`): `readCompletedAssistantMessageCandidate` marks a stored preview as `bounded`. The resolver never posts a bounded preview while a complete source exists. It posts a marked adapter final response or the accepted semantic summary instead (`bounded_final_agent_message_replaced_by_semantic_summary`). It posts the marked preview only when no complete text exists (`bounded_final_agent_message_without_complete_source`). This also fixes replies from runners that are already deployed. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-run-summary.test.ts`: 86 passed. Six new tests: a complete final message longer than 4,000 characters is posted in full; the old exact-prefix case posts the complete accepted answer; a preview whose prose differs from the summary posts the complete summary (also with chat precedence); a marked adapter final response wins over a preview; the marked preview is posted only when nothing complete exists; a stored preview candidate is marked `bounded`. Five of the six fail against the `master` resolver. - `cargo test -p paperclip-runner-core --lib -- final_answer complete_final`: 3 passed. Both new runner tests fail when the fix is reverted. - `cargo test -p paperclip-runner-core --lib`: 581 passed, 3 failed. The failures are `task_memory::tests::monitor_*` (OOM-kill monitor timing tests). They do not touch the changed code. - `cargo test -p paperclip-runner-core --test acpx_event_projection --test acpx_rich_events --test local_integrity_boundary_golden --test acpx_provider_events`: all passed. `cargo fmt --check` passes. - Evidence from the failed eval run: the `item.completed` final `agentMessage` text was exactly 4,000 characters (3,988 retained plus the marker). The `run.result.proposed` summary was complete at 4,985 characters. ## Risks - Larger durable events: a final answer can now be up to 32,000 characters (at most about 128 KB). The outbox keeps the preview when the text exceeds half of the frame size, so the frame check does not fail. - Redaction stays the same. The complete text goes through `redact_sensitive_text_values`, which the preview also used. - The server treats any final message that ends with "…[truncated]" as a preview. If an agent writes that text itself, the server posts the complete accepted summary instead. That summary is also the agent's own complete answer. - The runner part reaches users only after a runner release. The server part fixes replies at once when an accepted summary exists. > 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> |
||
|
|
cc072e7c4e |
feat(recovery): settle orphaned active environment leases on the shared ownership path (#19964)
<!-- 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 hold environment leases (local workspaces and remote sandboxes) while they execute > - A crash between the terminal run write and the lease release leaves the lease `active` with no holder > - A sweep released such a lease when the run was terminal and the lease was idle for five minutes. The age was the only guard against a race with the live executor, which releases the lease itself with the run's resource disposition > - Phase 2 of the durable ownership plan moves each in-progress record kind onto one shared owner proof (#19933 added the settle hook) > - This pull request moves `environment_run_lease_active` onto that path: the lease names its holder, the shared prover judges the holder, and the settle never destroys a sandbox > - The benefit is that a dead holder's lease settles on the next pass, a live holder is never raced, and the five-minute age gate is gone ## Linked Issues or Issue Description Refs #19933 **What happened?** An orphaned active environment lease was released by age: "run terminal plus five minutes" (the reaper in `heartbeat/recovery.ts` and the live-recovery `orphanLease` worker). The ownership data showed one such lease settle 119 s after it was ready. Without the age gate the sweep could flip a lease to `pending_cleanup` (destroy) while the executor or a recovery path was about to retain it. **Expected behavior** A run lease settles when its holder is proven gone, by identity and not by age. A live holder is never raced. A settle never destroys a sandbox whose files may still be needed. **Steps to reproduce** 1. Start a run that holds an environment lease. 2. Kill the server after the run status becomes terminal and before the lease release. 3. Restart. The lease stays `active` until the five-minute age passes, then the sweep destroys the sandbox. ## What Changed - `environments.ts`: the write that makes a lease active (insert and same-run reacquire) stamps the holder process in `metadata.leaseController`. The stamp does no I/O (`environment-lease-controller.ts`). - `recovery/ownership-registry.ts`: `environment_run_lease_active` lists leases with their run status. The owner is the run while it is active, then the stamped holder. It registers `settle` and `listForRuns`. - `environment-run-lease-settlement.ts` (new): compare-and-swap settle on the exact row (same run, status `active`, same metadata, terminal run). A sandbox lease gets the stop-and-retain intent (stop and keep, never destroy). Other leases move to `pending_cleanup` as before. It keeps a lease that a pending native export, a legacy shutdown fence, a shared provider resource, or an accepted native result still needs. - `recovery/ownership-invariant-report.ts`: `settleOwnableKind` accepts `currentProcessIdle`. A caller that passes every run this process executes in `skipRunIds` may treat a current-process owner of another run as gone (`current_process_idle`). The periodic pass never does this. - `heartbeat/recovery.ts`: `sweepOrphanedActiveLeases` no longer releases run leases by age. It runs native workspace preservation first (a separate obligation), then calls `settleOwnableKind` with this process's executions. Login and probe leases (no run) keep the idle sweep until their own change. - `heartbeat/live-recovery-work.ts`: a run lease is due now; only run-less leases keep the five-minute idle deadline. - `sandbox-stop-and-retain.ts`: an ownership settle records a recovery marker on the intent and keeps `updatedAt`, so the pending-cleanup sweep stops the sandbox in the same tick. - Deleted: the age gate for run leases and the page-order bump for run leases. ## Verification - `cd server && npx vitest run src/__tests__/environment-run-lease-ownership.test.ts src/__tests__/heartbeat-orphaned-active-lease-sweep.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - Red on unchanged master sources, green on this branch (tests were written after the code): - stamps the lease holder in the write that makes a lease active - settles a lease whose holder died after the run went terminal, on the next pass (crash-point state: terminal run, dead holder in this PID space) - stops and keeps a dead holder's sandbox instead of destroying it - leaves a current-process lease to this process: settled only when it is not executing the run - keeps the run fact alone for one release on a single-host server - names no owner on an ephemeral server (`PAPERCLIP_EPHEMERAL_SERVER=true`) - the three updated sweep tests (sandbox stopped and kept, not destroyed) - Live-owner counter-cases (green): run still active; another live PID; another PID space; current process while executing; read-only pass (`record: false`) never settles; a lease re-stamped after the proof is not settled; a legacy lease whose run this boot owns is kept. - Also run green: `recovery.test.ts`, `heartbeat-observed-workspace-preservation.test.ts`, `workspace-recovery-warm-handoff.test.ts`, `environment-service.test.ts`, `heartbeat-pending-cleanup-sweep.test.ts`, `native-environment-settlement-work.test.ts`, `environment-cleanup-work.test.ts`. - Full typecheck is left to CI (local memory limit). ## Risks - Behavior change: an orphaned sandbox lease is now stopped and kept, not destroyed. Workspace preservation and provider auto-delete decide when it goes away. This can keep stopped sandboxes longer than before. - A recovery path on another process can mark a run terminal while the dead executor's stamp is still on the lease. The settle can then win before that path releases. Because the settle only stops and keeps the sandbox, files are not lost; the later release continues from the stop-only intent. - Previous-release rows: a single-host server keeps the run fact for one release (a run this boot owns is excluded). An ephemeral server never releases an unstamped lease; it stays visible as an I1 gap with no operator action yet. - No migration. The stamp lives in existing lease metadata. > 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> |
||
|
|
a480335e69 |
fix(chat): tell agents how to read the company link prefix from a task identifier (#19967)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents reply with links to tasks, approvals, runs and agent profiles > - Each link starts with the company prefix, for example `/RUN/agents/<id>` > - The chat guidance said to derive the prefix from the task identifier, but it did not say how > - An agent decided that `RUN-1` does not give the prefix and invented one, so its links were broken > - This pull request states how to read the prefix and forbids prefixes built from names, slugs, hostnames or paths > - The benefit is that agent profile links and other company links in chat replies open the correct page ## Linked Issues or Issue Description **What happened** A person asked a CEO agent in chat to preview and then import a company package. The chat task was `RUN-1`. The agent's reasoning said: "Task identifier RUN-1 doesn't give prefix directly." The agent then linked agent profiles as `/runner-e2e/agents/<id>`. Each link opened "Organization not found: No organization matches prefix RUNNER-E2E". **Expected behavior** The agent links `/RUN/agents/<id>`. The guidance tells the agent that the prefix is the identifier part before the final hyphen and number. **Steps to reproduce** 1. Open a chat with a CEO agent in a company whose prefix differs from its slugified name. 2. Ask it to do something with another agent, for example import a package that creates an agent. 3. See that the reply can link `/<invented-prefix>/agents/<id>`, which opens "Organization not found". ## What Changed - Replace the task-link sentence in `AGENT_CHAT_DIRECTIVE` (`server/src/services/agent-conversations.ts`). It now says that the prefix is the identifier part before the final hyphen and number (`RUN-1` gives `RUN`). It says that all company links use that prefix. It forbids a prefix built from a company name, slug, hostname or path. - Add a regression test in `server/src/__tests__/agent-conversations.test.ts`. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversations.test.ts` passes (46 tests). - An end-to-end check runs the `companies.import-preview` chat eval with this commit cherry-picked onto the eval branch. ## Risks - Low risk. This changes only prompt guidance text. Runtime behavior does not change. ## 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> |
||
|
|
f755361ffe |
fix(recovery): deliver a steered message the restart lost before the model read it (#19962)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user can steer a running Codex or Pi turn: a comment goes into the live turn instead of waiting for the next run > - The provider acknowledges the steer at once, but the model reads it only at its next step, after the in-flight tool call > - A control-plane restart in that window kills the provider with the message in memory. Restart recovery starts a continuation turn that does not carry the message. The message is lost and no error shows > - The restart chaos harness showed this in 6 of 6 steered runs (Codex, K1, K2 and K6, 2 samples each): steer acknowledged, run succeeded, no reply to the steered message > - This pull request makes restart recovery deliver every acknowledged steer that the model did not read, exactly once > - The benefit is that manual Steer no longer loses messages on restart, and the planned automatic steering can ship on top of it ## Linked Issues or Issue Description **What happened?** A comment steered into a running Codex turn got no reply when a control-plane restart (SIGKILL, SIGTERM or crash loop) happened during the agent's tool call. The run succeeded, and the steered comment had no effect. **Steps to reproduce** 1. Start a Codex task that runs a 40-second command. 2. During the command, post a comment and click Steer. The provider acknowledges it. 3. Restart the control plane before the command ends. 4. Recovery resumes the turn with a continuation. The agent finishes the task and ignores the comment. **Expected behavior** The agent reads and handles the steered comment once, after recovery. **Root cause** The provider keeps acknowledged steering input in memory until its next model step. The restart continuation (`paperclip.native-continuation.v1`) only told the model to continue the current request, so the acknowledged input was gone. ## What Changed - `unreadSteeredMessageIds` (runner package): derives acknowledged-but-unread steers from run-log events in order. A `steering_acknowledgement` item adds the message id (once per id). A `userMessage` item in the same turn reads the oldest unread steer. Codex records each steered input as one `userMessage` item when it enters the conversation. A `userMessage` item in a later turn reads all earlier steers, because the continuation carries them. `turn.completed` reads all steers. - `readUnreadSteeredMessages` (server): reads those facts from `heartbeat_run_events` (only transport-validated events with a `sourceEventId`). It also reads the comment or interaction text for the same company and issue. The run log is the durable record of both "acknowledged" and "read", so no new column is needed. - The restart continuation now includes `steeredMessages` and one instruction to handle them, oldest first. The runtime reads them through a lazy `getUnreadSteeredMessages` option, only when a restart continuation starts. Native recovery wires it for every run. - `queuedInteractionSteeringBody` is extracted, so recovery rebuilds a steered interaction response with the same text the steer sent. - Fake Codex gets `--consume-steer`, which emits the `userMessage` item for an accepted steer, like real Codex. ## Verification - Red/green live test with real runnerd and fake Codex (`runnerd-restart-continuation.test.ts`, "delivers a steered message the restart lost exactly once"): start a held turn, steer it, SIGKILL runner and provider, restore and recover. - "unread": the continuation carries the message exactly once. Without the runtime change this test fails. - "read" (`--consume-steer`): the continuation does not carry it again. - Both: after the continuation completes, a later restart has nothing to deliver. - `steering-delivery.test.ts`: ordering, dedupe, initial-input, later-turn and completed-turn rules. - `unread-steered-messages.test.ts` (embedded Postgres): ignores events without a source id and messages from other issues. It reads interaction text, ignores model-output items as read evidence, and returns nothing after the continuation input. - `pnpm vitest run src/steering-delivery.test.ts src/live/runnerd-restart-continuation.test.ts` (runner) and `pnpm vitest run src/services/native-runtime/unread-steered-messages.test.ts` (server): pass on the rebased revision. - `issue-queued-comments-routes.test.ts` and `native-session-runtime.test.ts -t restart`: pass. `native-session-resume.test.ts` "archives a damaged prior epoch…" fails locally on plain `origin/master` too (local runnerd binary); CI runs it with a fresh build. - Restart chaos (Codex steer mode, K1/K2/K6) runs after this lands in the harness slot. Before: 0 of 6 steered messages answered. ## Risks - Duplicate delivery: a provider that records steered input without a `userMessage` item, or a `userMessage` event lost in the crash window before persistence, makes recovery send the message again. The model then sees it twice. That is safer than losing it, and the instruction asks the model to handle each one once. - Pi (ACPX) steering has no `userMessage` read evidence today, so a Pi steer that the model already read before a restart may arrive again in the continuation. - Only the restart continuation carries the messages. Other recovery paths start after a completed turn, which already read every steer. - No schema or migration change. A session saved by an earlier release resumes normally; its run log already holds the acknowledgement events. ## 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> |
||
|
|
bf3a067076 |
feat(recovery): settle the finalizer and cleanup leases on the shared ownership path (#19950)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs keep their ownership in `native_run_finalizations`, and the shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The finalizer lease (`native-finalizer:*`, 5 min) and the retained-session cleanup lease (`native-cleanup:*`, 60 s) recorded only a random token. They named no holder process > - A server killed while it finalized or cleaned up kept the run waiting for the whole lease. The lease was released by age alone. A graceful shutdown also keeps an in-progress finalizer lease for the next boot to wait out > - This pull request records the holder process with each claim and moves both kinds onto the shared prover and a compare-and-swap settle > - The benefit is that a dead holder's run continues in the first pass after boot, and a live or unprovable holder is never released ## Linked Issues or Issue Description Refs #19933 (the settle hook and the first moved kind, merged). **What happened?** A finalizer or cleanup lease whose server process died stayed held until the lease expired (5 min or 60 s). The run stayed `running` and its follow-up work waited. **What did you expect?** The next boot proves the dead holder gone and frees the lease at once. A live holder, or one that cannot be proven gone, keeps its lease. **Steps to reproduce** 1. Accept a native result and let the finalizer claim it. 2. Kill the server (SIGKILL) before the finalizer commits. 3. Start the server. The run stays `running` for up to 5 minutes. ## What Changed - `native_run_finalizations.lease_holder` (jsonb, migration 0446): `{token, bootId, pid, hostHash, processStartedAt}` of the process that took a token-only lease. It names the holder only while `token` equals `lease_owner`, so renewals keep it and a later lease never inherits it. The executor's `controller_*` columns are not changed. - The finalizer claim (`native-run-finalizer.ts`) and the cleanup claim (`native-session-executor.ts`) write it in the same UPDATE as the lease, from `currentControllerProcessStamp()` (no I/O). - New `native-token-lease-settlement.ts`: compare-and-swap settle that expires exactly the proven-gone holder's outstanding lease, after it notifies the finalization or cleanup work that the expiry frees. - Both claims also wake the ownership worker queue in their transaction, so a quiet worker watches the new holder (Greptile round 1). - `ownership-registry.ts`: `native_finalizer_lease` and `native_cleanup_lease` register `settle`. Their owner is the holder process; rows without a holder keep the token fence. Listings cover outstanding leases only. - `heartbeat/recovery.ts`: restart recovery settles both kinds before it finalizes and schedules cleanup. A failure is logged and the reconciler pass retries it. - Two sparse partial indexes on `lease_expires_at` for the two listings, so the settle watch re-check stays indexed. Bespoke code deleted: none. These kinds had no liveness check; their only release was lease age (the takeover in the claim), which stays as the fence for unprovable holders. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t "finalizer|cleanup|holder|exact lease"`: 12 passed (rerun after the rebase on master). The crash-point test also asserts the ownership wake for its run; it fails without the signal. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red on the base branch (server code at `bea3bfb20b`, tests and schema from this branch): 7 new tests fail, 5 pass (the 5 are existing tests). Red: the crash-point restart test, the two reconciler settle tests, the three live/unprovable counter-cases (owner was a token fence), and the exact-lease CAS test. The tests were written after the code. - New tests: - crash point: a finalizer claim was written and its holder killed before commit; the next restart finalizes the run, and this boot's claim stamps itself as holder; - a dead finalizer or cleanup holder is settled by a recording pass, never by a read-only pass (`record: false`); the accepted result and phase are untouched; - live counter-cases: a live earlier-boot PID (`pid_alive`), the current boot (`current_process`), a dead PID in another PID space (`different_process_space`, unprovable) keep the lease through a pass and a restart; - a holder written for another token, and a holderless row of the previous release, stay token fences, also with `PAPERCLIP_EPHEMERAL_SERVER=true`; - settle refuses a lease taken over after the proof (`owner_changed_since_proof`). - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `lease_holder`. They stay token fences and the existing claim takes them over at expiry. Neither lease outlives 5 min, so this covers at most 5 min after the upgrade. A rollback ignores the new column. - Unprovable holders (other host or PID space, unknown space): no operator action. The lease is short and self-expiring; at expiry the next claim takes it over and the old holder's next token compare-and-swap fails. That fence is the handoff. A different host is never proof. - The settle only expires a lease. Finalization and cleanup still run under their own checks (cleanup keeps its maintenance-history proofs). Settling ownership does not settle the accepted result, workspace preservation, or accounting. - Migration adds a nullable column and two partial indexes on a sparse predicate. Low risk. ## 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> |
||
|
|
ca34bed257 |
test(native): exited-remote-runner budget case follows provider-work attempt charging (#19963)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Two restart-recovery PRs merged independently: #19951 lets a remote runner that exited before provider work bootstrap again, and #19948 charges a restart attempt only when the provider accepted a turn. > - Together they change one of #19951's test expectations: at the last attempt, the exited runner is no longer charged, so it bootstraps again instead of hitting the budget. > - That test now fails on master in `Verify Paperclip Runner (vitest native server)`, which blocks other PRs. > - This pull request updates the test to the combined behavior. Product code does not change. > - The benefit is a green master, with the generation-cap bound on repeated exits still under test. ## Linked Issues or Issue Description Refs #19951, #19948, #19950 (blocked by this failure). **What happened?** On master, the test "keeps an exited remote runner's retry held when the bootstrap budget is spent" receives `bootstrap_incomplete` with `chargesProviderAttempt: false` instead of `blocked / execution_recovery_budget_exhausted`. **What did you expect?** The test should match the combined behavior: uncharged at the last attempt, and blocked once the controller generation cap is reached. **Steps to reproduce** 1. `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner"` on master fails. ## What Changed - Removed the stale budget row from the exited-runner counter-cases. - Added two cases. An exited runner at attempt 3 bootstraps without being charged. At `NATIVE_PROGRESS_CREDIT_GENERATION_LIMIT` it blocks with `execution_recovery_budget_exhausted`. These mirror #19948's own cases. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner|relaunched remote runner|generation cap"`: 11 pass. ## Risks - Low. Test-only change. Repeated runner exits before provider work stay bounded by the generation cap, which the new case covers. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
f6cb3e43f4 |
fix(admission): admit new work independent of other runs' cleanup and accounting (#19944)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission decides if new agent work can start in a company > - Five admission gates waited on the cleanup or accounting of other runs > - The restart-chaos campaign found that these gates froze companies for up to 10 minutes, caused false budget pauses, denied in-run writes, and dropped wakes, comments and Retries > - The durable ownership plan (Decision 1) removes the company-wide accounting gate. It does not narrow it again > - This pull request removes the five gates. Unfinished accounting now adds its bounded estimate to exposure > - The benefit is that one run's unsettled accounting never stops the other work in its company ## Linked Issues or Issue Description Refs #19831 (plan, section D), #19838 (inventory, finding 3), #19927 (Phase 2 reconciler data), #19497, #19724, #19826. **What happened?** Admission refused new work in these cases, with no reached hard stop and no pause: - a terminal run awaited accounting (`waitingRunCount`); - a native run recovered its accounting (`recoveringRunCount`); - a budget-owned scope hold awaited reconciliation, after the hard stop no longer applied; - a budget input was older than 15 s (`budget_evaluation_work`); - the predecessor run row of a task was missing (`hasPendingRunCleanup` failed closed). **Expected behavior** Admission never waits on the cleanup or accounting of another run. Only a reached hard stop, an explicit pause or an inactive company refuses new work (invariant 3). **Steps to reproduce** Make a company with an active budget policy. Leave one failed run with `cost_accounting_pending = true`. Request work for another agent. On `master` admission refuses with "Company cannot start work while completed runs await accounting." ## What Changed - `budgets.ts`: admission refuses only for a reached hard stop or an explicit pause. The code removes `observedBlocks`, the `recoveringRunCount` check, the stale `budgetHeld` refusal and the 15 s input-age gate. It also removes the unused `inspectInvocationEligibility`. `reconcileScope` no longer returns accounting holds. - `budget-exposure.ts`: removes the `waiting` and `recovering` aggregates and the own-task subquery. The admission query is smaller. Unfinished accounting stays exposure: the larger of the held reservation and the observed cost. - `budget-evaluation-work.ts`: removes `oldestBudgetInputAt`, which nothing reads now. - `execution-blocker.ts`: `hasPendingRunCleanup` returns false for a missing run row. A run row that is deleted removes its finalization coordinator and nulls its lease references, so the row owns nothing. A predecessor that owns a lease or a coordinator still holds the successor of its own task. - `recovery/ownership-invariant-report.ts`: the I3 check no longer mirrors the removed gate. It reports a wake request or a run that was refused with an accounting-hold reason. Any such report is a regression. - `doc/SPEC-implementation.md`: adds the product rule "Admission is independent of cleanup and accounting". `doc/execution-semantics.md` is aligned. ## Verification - New `server/src/__tests__/admission-independent-of-cleanup.test.ts`, on embedded Postgres. It has one case per gate. All six cases fail on `master` and pass here. A seventh case (an explicit pause still refuses) passes on both. One case also asserts invariant 3 together with exposure: two failed runs that each hold a 400-cent reservation fill a 1000-cent budget, and a hard stop still refuses. - Existing tests that asserted the old gates now assert admission (invariants 3 and 4): `closed-native-receipt-accounting`, `budgets-service`, `budget-evaluation-work`, `heartbeat-held-comment-wake`, `ownership-reconciliation-report`. In `heartbeat-held-comment-wake`, each saved comment, Retry, assignment and continuation is delivered exactly once while another run's accounting is pending. Settling accounting later does not deliver it again. - All six files pass locally, one file per process. The server package typecheck passes. - Speed: admission is on the turn path. This change only removes work from it: two correlated aggregates, one own-task subquery and one `budget_evaluation_work` read per policy. No wait or I/O is added before `turn.start`. - Not run here: the chaos waves (K13 storm, K6 crash loop, K4 long holds). The coordinating session schedules them on the shared harness. ## Risks - Behavior change: work starts while accounting for other runs settles. Spend stays bounded by the reservation capacity check and the hard stop, which read the ledger plus observed cost. A run that has no reservation and no observed cost adds zero exposure until its charge commits. This is the gap that Decision 1 accepts. Budget enforcement was already best effort (SPEC 11.6). - A budget-owned pause with no reached hard stop no longer refuses work in budget admission. The paused company or agent is still refused by the existing pause checks until the budget worker lifts the pause. ## 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> |
||
|
|
7213f64962 |
fix(heartbeat): write spawn identity and event sequence in one run update (#19954)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The heartbeat records the provider process identity when a native or legacy run spawns its process > - This write updated the run row and then reserved the event sequence with a second UPDATE of the same row > - PostgreSQL re-checks every foreign key on a row that the current transaction already changed, so the second UPDATE took KEY SHARE on the issue, the predecessor run, and the other parent rows > - A transaction that held one of those rows FOR UPDATE blocked spawn persistence, and the failed write ended a user's Retry as `native_session_interrupted` > - This pull request writes the identity and reserves the sequence in one UPDATE, and it retries deadlock and serialization failures of the spawn write > - The benefit is that a user's Retry no longer fails because an unrelated transaction holds the issue or the failed predecessor run ## Linked Issues or Issue Description **What happened** A user clicked Retry on a failed native run in the `runs-adapters.inspect-failed-run` chat eval (embedded PostgreSQL, one server process). The retry run failed with `native_session_interrupted`. Its error was `Failed query: update "heartbeat_runs" set "next_event_seq" = "heartbeat_runs"."next_event_seq" + 1 ...`. The query started at 01:51:04 and failed at 01:51:16. The OpenCode driver awaits `onSpawn` before its health check, so `native.session.execute` failed after 12002 ms. The stack showed `allocateHeartbeatRunEventSeq` inside `appendHeartbeatRunEvent`, called from `persistHeartbeatRunProcessMetadata`. The process identity was lost (`processPid` stayed null). The server log does not include the PostgreSQL cause code. **Expected behavior** Spawn persistence must not wait on locks for unrelated parent rows. A transient deadlock or serialization failure must be retried. It must not end the run. **Steps to reproduce** 1. Create a run whose `issue_id` and `retry_of_run_id` point at an issue and a predecessor run. 2. In a second connection, begin a transaction and run `SELECT ... FROM issues WHERE id = <issue> FOR UPDATE` (or lock the predecessor run). Keep the transaction open. 3. Call `persistHeartbeatRunProcessMetadata` for the run. 4. Before this change, the call blocks on the `next_event_seq` UPDATE until the second transaction ends. If that transaction then waits on the run row, PostgreSQL reports a deadlock. `server/src/services/heartbeat/process-identity-persistence.test.ts` reproduces step 4. Root cause: `server/src/services/heartbeat/run-lifecycle.ts` (`persistHeartbeatRunProcessMetadata`) updated the run row, then `appendHeartbeatRunEvent` called `allocateHeartbeatRunEventSeq` (`server/src/services/heartbeat-run-events.ts`), which updated the same row again. PostgreSQL's `RI_FKey_fk_upd_check_required` always fires the foreign-key checks when the old row version was written by the current transaction. The checks take KEY SHARE on each referenced row, and KEY SHARE conflicts with FOR UPDATE. ## What Changed - `appendHeartbeatRunEvent` accepts a narrow `runUpdate` option (process identity columns only). `allocateHeartbeatRunEventSeq` applies these columns in the same UPDATE that reserves the sequence. Deduplicated receipts (native source, retry exhaustion) reject `runUpdate`, because they do not allocate. - `persistHeartbeatRunProcessMetadata` reads the run binding, appends `native.process_identity_recorded` with the identity as `runUpdate`, and returns the updated row. The run row now gets one UPDATE in this transaction. - New `retryTransientRunWrite` retries SQLSTATE `40P01` and `40001` up to four times. The lifecycle `persistRunProcessMetadata` and the legacy cleanup-intent wrapper in `heartbeat.ts` use it. The legacy wrapper still updates the run row a second time when it records the cleanup intent, so the retry covers a deadlock there. - New regression test `server/src/services/heartbeat/process-identity-persistence.test.ts`. ## Verification - `npx vitest run src/services/heartbeat/process-identity-persistence.test.ts` (from `server/`): 3 passed. Without the `run-lifecycle.ts` change, both lock cases fail with `expected 'blocked' not to be 'blocked'` after 3 s. - `npx vitest run src/__tests__/heartbeat-run-event-sequencing.test.ts src/services/heartbeat/run-lifecycle.test.ts`: 42 passed. - `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "<the four tests that call persistHeartbeatRunProcessMetadata>"`: 5 passed. - `tsc --noEmit -p server`: no errors in the changed files. The only error comes from a local workspace-link mix in `packages/adapter-utils`, which this change does not touch. - I did not run `pnpm verify` or the full suite locally. CI supplies that coverage. ## Risks - Low risk. The written columns, the event type, the payload, and the sequence order do not change. - The event payload `processStartedAt` now comes from the value that the transaction writes, not from a re-read of the row. The value is the same. - An event insert still checks its own `agent_id` and `company_id` foreign keys. A transaction that holds the agent row FOR UPDATE can still delay the spawn write. That wait is the same for every event writer, and this change does not alter it. - Other paths that update a run row and then append an event in the same transaction keep the second-UPDATE foreign-key checks. This change fixes the spawn identity path that lost the run. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, embedded PostgreSQL experiments). ## 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> |
||
|
|
779913333a |
fix(documents): say whether an exact revision read is still current (#19939)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Task documents keep a revision history, and agents read and restore
revisions through the API
> - The exact-revision read returns one revision, but it does not say if
that revision is still current
> - An agent read the revision that it restored earlier by its ID and
reported it as "latest", but another person had saved a newer edit
> - This pull request adds the document's current revision to the
exact-revision read
> - The benefit is that a pinned read cannot pass as a read of the
current document
## Linked Issues or Issue Description
**What happened**
In a chat eval, the agent restored a plan revision. Then another person
saved a new revision. The user asked the agent to read the current plan.
The agent called `GET
/api/issues/{id}/documents/plan/revisions/{revisionId}` with the ID of
its own restored revision. The response did not show that a newer
revision existed. The agent reported the old revision as the latest and
said that the read confirmed this.
**Expected behavior**
A read of one exact revision must show if that revision is still
current, and which revision is current. Then the reader can see a newer
edit and read the current document.
**Steps to reproduce**
1. Save two revisions of a task plan.
2. Call `GET
/api/issues/{id}/documents/plan/revisions/{firstRevisionId}`.
3. See that the response gives no sign that revision 2 is the current
revision.
## What Changed
- `getIssueDocumentRevision` now also returns `latestRevisionId`,
`latestRevisionNumber` and `isLatest`.
- Shared type `ExactDocumentRevision` describes this response. The UI
client uses it.
- The OpenAPI description of the exact-revision read names these fields
and points to `GET /api/issues/{id}/documents/{key}` for the current
text.
- The service test checks `isLatest` for an older and a current
revision.
## Verification
- `cd server && pnpm exec vitest run
src/__tests__/documents-service.test.ts
src/__tests__/openapi-routes.test.ts` (42 passed).
- `npx tsc --noEmit -p packages/shared` passes.
## Risks
- Low risk. The change only adds fields to a read response. No caller
removes or renames a field. No migration.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code
execution.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
a4a3e342ec |
fix(recovery): charge a restart attempt only when the provider accepted a turn and made no progress (#19948)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run has an execution recovery budget: three provider attempts for native runs, and two failure retries for legacy runs > - In the restart chaos campaign, control-plane restarts used up that budget for runs that were healthy (RC-22, 22b, 22c, 40 and 60) > - #19720 and #19747 fixed two variants with watermark heuristics, and legacy interrupted preparation still spent the failure budget > - The durable-ownership plan (section F, invariant 5) asks for one rule: charge an attempt only when a recorded outcome proves that provider work started and no progress followed > - This pull request adds that rule and moves the native restart charge and the legacy preparation retry onto it > - The benefit is that a restart never spends budget for work the provider never started, while real failure loops and crash loops stay bounded ## Linked Issues or Issue Description Refs #19720, #19747, #19856 **What happened?** A control-plane restart charges a provider attempt by heuristics. The first crash restart always charges. A later restart charges unless the previous resume had no start evidence (#19747) or completed work (#19720). A runner that spawned but whose provider never accepted a turn counted as started, so a K6 crash loop could still spend the budget. Legacy runs whose preparation a restart interrupted (`providerWorkStarted: false`) spent `failureRetries`, the same counter as real failures (`execution-recovery-attempt.ts`). **Expected behavior** An attempt is charged only when the provider accepted a turn (`turn.accepted` or `turn.started`) and no accepted turn then completed work. A startup before spawn, a runner that spawned but was never admitted, and a `turn.start` the provider did not accept are not charged. The generation-12 cap still bounds uncharged restarts. **Steps to reproduce** Start a native run at its last attempt. Kill the controller after the runner spawns but before the provider accepts a turn. Before this change, the next restart claim blocks the run with `execution_recovery_budget_exhausted`. ## What Changed - New `server/src/services/native-runtime/provider-attempt-outcome.ts`: the one rule. `readProviderAttemptOutcome()` returns `provider_not_started`, `provider_started_without_progress` or `provider_progressed` from run events. `providerAttemptChargesBudget()` charges only the second. `providerAttemptWindowStart()` starts the attempt window at the latest `native.process_start_requested`, so earlier work never counts for a later attempt. `PROVIDER_ATTEMPT_CREDIT_GENERATION_LIMIT` (12) is the shared cap. - `restartResumeKeepsAttempt()` (`native-restart-recovery.ts`) now applies that rule to the attempt the restart interrupted, including the first restart. It keeps the unstarted-resume carry from #19747, the reattachment rule (an adopted provider counts as started), the drain exemption from #19625, and the generation-12 cap. A history entry from an older release without a recorded position still charges. - Legacy runs: `interrupted_preparation` is now an uncharged retry lane (`execution-recovery-attempt.ts`). It keeps its own counter, bounded at the same cap (12), and never changes `failureRetries`. `retries.ts` uses the longest transient delay after the second lane attempt. - `doc/execution-semantics.md` states the rule. - Accounting has its own "provider work started" readers (inventory Summary finding 5). Phase 3 owns those; this PR exposes the rule for them to adopt and does not change them. ## Verification - `native-runner-restart-recovery.integration.test.ts` (embedded Postgres, real `claimNativeRestartRecoveries`): 129 passed. New cases, red on master: - not charged at the last attempt for a claimed remote bootstrap, a remote bootstrap retry, and a claimed unlaunched remote run; - not charged when the runner spawned but was never admitted, and when the provider did not accept `turn.start`; - not charged on the first restart when the original attempt already made progress; - a normal retry that spawned but was never admitted is not charged, and earlier work still does not count for it. - Counter-cases, green on master and with the change: an accepted `turn.accepted` or `turn.started` with no progress is charged; the generation cap blocks at the last attempt; crash loops with and without progress stay bounded; the #19720 and #19747 cases still pass. - `execution-recovery-attempt.test.ts`: new lane tests (red on master). - `workspace-recovery-preparation-dispatch.test.ts` (25 passed), `heartbeat-process-recovery.test.ts` (interrupted-run cases), `run-dispatch/adapters/postgres.test.ts` (interrupted cases). - `server` typecheck (`scripts/typecheck-layers.mjs`, low-memory mode): no errors. - Not run: chaos waves (the coordinator schedules them on this host), and the full server suite (CI covers it). ## Risks - Behavior change: the first crash restart no longer charges when no accepted turn is recorded. If a crash lands before the controller projects the provider's `turn.accepted`, that attempt is not charged. This fails toward more retries, and the generation-12 cap bounds it. - A legacy preparation crash loop now gets up to 12 retries instead of 2. Each retry still passes the normal issue gates and keeps the transient delay. - The new queries use the `(run_id, seq)` index, once per restart claim. Nothing new runs before `turn.start`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
c6e4ed1547 |
feat(recovery): settle a claimed wake whose run finished without settling it (#19934)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A wake request is claimed by the run that handles it, and that run marks it finished. > - In a K4 chaos wave (#19927), native restart recovery blocked a run and ended it, but left its assignment wake `claimed`. Nothing ever released it. > - The durable-ownership plan settles such records on one shared path, by proof, never by age. > - This pull request registers a settle for `wake_request_claimed`. It is Phase 2's second kind and is stacked on the accepted-result lease PR. > - The benefit is that no run-ending path can strand a claimed wake. ## Linked Issues or Issue Description Refs #19831 (plan), #19861 (registry), #19927 (the data that found this). Stacked on #19933 (it adds the `settle` hook). **What happened?** Run `7c45860b…` failed with `native_restart_recovery_blocked` (`execution_recovery_budget_exhausted`) at 23:19:41. Its wake `6b3c4cce…` stayed `claimed` until the end of the wave (23:44 and later). Rule I2 flagged it. **What did you expect?** A claimed wake ends when its run ends, with the run's outcome. **Steps to reproduce** 1. `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --faults K4 --k4-hold-max-seconds 600 --tasks 6 --concurrency 3 --seed 9203 --keep` 2. Look for `agent_wakeup_requests` rows with status `claimed` whose run is terminal. ## What Changed - New `recovery/wake-request-claimed-settlement.ts`: when the owner run is terminal, and for a native run its coordinator is `committed` or `terminal_failure`, the claim gets the run's outcome. That is `completed` for a succeeded run and otherwise the run's status, as run completion and native terminal projection already do. It is compare-and-swap on the claim (id, run, status `claimed`). - `ownership-registry.ts`: `wake_request_claimed` registers that settle. - `native-finalization-reconciler.ts`: the pass log line includes the settled count per kind. ## Verification - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 pass, including the new case. It covers a blocked native run, a native run whose projection is still pending (kept), a succeeded legacy run, a live run (kept), and a read-only pass (no change). - Red/green (test written after the code): the new test fails on master and passes here. - Server typecheck passes. ## Risks - No persisted shape changes, so there is no previous-release story beyond this: claims saved by an older release are settled the same way. - Race with the run's own settle: it waits for a native coordinator to be final. For a legacy run, both write the same status, so the race is idempotent. - No change adds work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
83dec71a8f |
fix(runner): return the edited agent's link and effect in instruction save receipts (#19940)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In chat, a person can ask the CEO agent to change another agent's saved instructions > - The native instruction tools save the change, but their receipts contain no link to the edited agent > - Completion feedback tells the agent to use only hrefs that tools returned and not to guess routes > - So the agent can not link the agent it changed, and it has no grounded fact about when the change applies > - This pull request adds the edited agent's real profile href and the save's effect to the save and restore receipts > - The benefit is a chat reply that lets the person open the changed agent and know which runs use the new rule ## Linked Issues or Issue Description Refs #19344 **What happened** A person asked in chat: "Add a rule for Morgan to run the relevant tests before handing work back." The agent saved the rule correctly and kept concurrent human guidance. The reply did not link Morgan and did not say when the rule applies. The run's tool events show the cause: `update_agent_instructions` returned the revision but no href, and the accepted `paperclip_finish` feedback said "Do not guess routes or invent links when no resource href was returned." **Expected behavior** After an instruction save or restore, the agent has a real profile href for the edited agent and a statement of when the saved instructions take effect. The reply links the agent and explains that later runs use the change. **Steps to reproduce** 1. In a chat with the CEO agent, ask it to add a rule to another agent's instructions. 2. Let the agent call `update_agent_instructions`. 3. Read the tool receipt. It contains no href for the edited agent and no effect statement. ## What Changed - `update_agent_instructions` and `restore_agent_instructions` receipts now include `resourceLinks` with the edited agent's company-scoped profile href (`/<prefix>/agents/<agentId>`) and name. - The same receipts include `appliesTo`. It states that runs that start after the save use the new instructions and that runs in progress keep their snapshot. This matches `materializeInstructionBundle`, which reads the committed head when it builds the run context. - The link uses the agent id, which is unique. It does not use the name-derived URL key. ## Verification - `cd server && pnpm exec vitest run src/services/native-runtime/agent-instruction-tools.integration.test.ts src/services/native-runtime/agent-instruction-tools.test.ts`: 14 passed. - Without the source change, the new assertions fail in 3 tests (update receipt and both lost-acknowledgement recovery receipts). - The recovery test confirms that the persisted receipt, which recovery replays, also carries the link. - The end-to-end chat eval `agents.edit-instructions` runs this change on its case branch. ## Risks - Low risk. The change adds fields to a tool receipt. Existing fields do not change. - Receipts that were saved before this change replay without the new fields. That is acceptable: the agent then has no link, as before. ## 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> |
||
|
|
196108acb9 |
fix(native): bootstrap again when a relaunched remote runner exited before provider work (#19951)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Restart recovery decides whether an interrupted native run can resume, bootstrap again, or must wait for a person. > - A remote run is blocked as "ambiguous" when recovery cannot prove what reached the sandbox runner. > - In a Daytona chaos wave (#19947), two runs were blocked even though the controller had recorded that the relaunched runner exited before any provider work. > - This pull request accepts that recorded runner exit as proof and bootstraps the run again, under the existing attempt budget. > - The benefit is that a restart no longer turns a run with no provider work into a failed, blocked run. ## Linked Issues or Issue Description Refs #19947 (Daytona data), #19831 (durable-ownership plan), #19934 (the stuck claimed wake from the same runs). **What happened?** Wave `w-20261108T012041-s9301`, runs `450f277d…` and `9b93d40c…`, went through this sequence: 1. Restart recovery proved the remote bootstrap incomplete and relaunched it. 2. The new runner exited: "runnerd exited while waiting for run.prepare". The executor settled `bootstrap_retry` with `providerSessionEstablished: false`, `providerEventsExist: false` and `checkpointExists: false`. 3. The next restart recovery saw the recorded runner PID and blocked the run as `ambiguous_remote_recovery_evidence`. **What did you expect?** Bootstrap again, because the runner is proven gone and no provider work happened. **Steps to reproduce** 1. `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "relaunched remote runner"` fails on master with `blocked / ambiguous_remote_recovery_evidence`. ## What Changed - `native-restart-recovery.ts`: new proof `remoteRunnerExitedBeforeProviderWork`. It applies to a remote sandbox with a prepared workspace and a settled `bootstrap_retry` failure whose original code is `native_runner_process_exited`, with no provider session, events or checkpoint and no lease or successor. Such a run is classified as `bootstrap_incomplete` (`remote_runner_exited_before_provider_work`). It passes the same retry admission check as `remote_runner_never_launched`, and the same three-attempt budget. - Tests: the s9301-shaped retry plus five counter-cases. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner|relaunched remote runner|remote bootstrap retry|never requested a runner|exact-checkpoint retry"`: 14 pass. - Red/green: the new bootstrap test and the budget counter-case failed on master (both returned `blocked / ambiguous_remote_recovery_evidence`, the same as the s9301 rows) and pass here. The other four counter-cases (not a runner exit, session established, provider events, checkpoint) block on both. - Loop: focused vitest with one embedded Postgres, 26–50 s. - Not run locally: the server typecheck and the full suite (memory limits on this host). Required CI covers them. ## Risks - Fail-open risk: the proof needs the executor's own settled record of a runner exit with no provider work, plus a fresh check that no checkpoint or provider evidence exists. A recorded PID without the exit record still blocks, as before. - Previous-release story (rules/upgrade-safe-run-state.mdc): no persisted shape changes. Rows without these failure fields keep today's outcome. - Nothing runs before `turn.start`. This is restart recovery only. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
38d8ef89ca |
fix(chat): link a person's own profile editor instead of attempting an agent write (#19943)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People talk to their agents in chat. The chat guidance tells the agent which links to give back. > - A person can ask the agent to change their own display name. Only the person can do this, in their profile settings. > - The chat guidance did not say where that page is. The agent searched the API and tried the human-only profile endpoint. The product correctly refused it. The reply then gave only vague "profile/account settings" directions. > - This pull request tells chat where the personal profile editor is, and that agent credentials cannot change a person's profile. > - The benefit is a reply with a link that the person can open to make the change. ## Linked Issues or Issue Description **What happened** In chat, a person wrote "Change my display name to Alex Chen." The agent spent about four minutes on API searches. It tried `PATCH /api/auth/profile`, and the product refused it. The reply told the person to use their "profile/account settings" but gave no link. **Expected behavior** The agent does not try to change the person's profile. It links the person's profile editor at `/<prefix>/company/settings/instance/profile`, so the person can change the name. **Steps to reproduce** 1. Sign in as a person in an authenticated deployment. 2. Open chat with the CEO agent. 3. Send "Change my display name to Alex Chen." 4. See that the reply has no link to the profile editor. ## What Changed - `server/src/services/agent-conversations.ts`: the chat directive now says that a person's own name and avatar change only in their profile settings at `/<prefix>/company/settings/instance/profile`. It says that agent credentials cannot edit a person's profile, and that renaming an agent, member record or company does not change it. The agent links that page instead of trying the change. - `server/src/__tests__/agent-conversations.test.ts`: a regression test checks the directive and the built chat prompt. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversations.test.ts -t "chat prompt policy"`: 10 passed. - The chat eval case `navigation-preferences.edit-profile` checks this end to end. It requires a link to the profile editor, and the person follows that link to save the new name. ## Risks - Low risk. This is a guidance change only. It adds no permission and changes no route. - Deployments that hide the Profile settings page get a link to a hidden page. The page gate still applies. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
47dc3dd9d0 |
fix(chat): ask which record is meant when a change matches several (#19942)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People use the agent chat to change records, for example task priority > - A request can name one record when several accessible records match it > - The chat guidance tells the agent to ask focused questions, but it does not say what to do when a reference matches several records > - In an eval run, the agent spent more than two minutes. It considered picking one task, changing both tasks, and asking. It did not reply before the run was cancelled > - This pull request adds one rule: ask which record is meant before changing any of them, and link each candidate > - The benefit is a fast, safe reply that does not change the wrong record ## Linked Issues or Issue Description **What happened** The person wrote "Mark the landing-page task as high priority." Two accessible tasks have the title "Landing page", in the projects Launch and Experiments. Nothing in the chat selected one. The agent searched, read both tasks, listed projects and searched the API. Its reasoning wavered between "ask", "change both" and "pick one". After about 130 seconds it had not replied, and the run was cancelled. **Expected behavior** The agent asks which task the person means. The question links both tasks, names what makes them different, and keeps the requested change. The agent changes no task until the person answers. Then it changes only the selected task. **Steps to reproduce** 1. Create two tasks with the same title in two different projects. 2. In the agent chat, ask to set "the <title> task" to high priority. 3. See that the guidance gives no rule for several matches, and the agent can deliberate, guess, or change both. Refs: chat-feature eval `tasks.ambiguous-task`. ## What Changed - `server/src/services/agent-conversations.ts`: `AGENT_CHAT_DIRECTIVE` now says: when a change request matches several accessible records and nothing selects one, ask which one before changing any. Link each candidate, name the difference, and restate the change. Do not pick the first match, change every match, or keep searching for a hidden answer. - `server/src/__tests__/agent-conversations.test.ts`: a regression test checks the rule in the directive and in the built chat prompt. ## Verification - `cd server && npx vitest run src/__tests__/agent-conversations.test.ts -t "chat prompt policy"` passes (10 tests). - End-to-end: the chat-feature eval `tasks.ambiguous-task` runs with this commit on an eval branch. The result is in a comment below. ## Risks - Low risk. This is a prompt-guidance change only. No API, schema or permission changes. - The agent can ask in more cases. The rule applies only when several records match and nothing in the conversation selects one. ## 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> |
||
|
|
f5400e2594 |
feat(recovery): settle the accepted-result lease on the shared ownership path (#19933)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A control-plane restart can kill the controller after an agent's result is accepted. The result then stays leased to a dead process. > - Today a private sweep (`native-dead-controller-result-lease.ts`, #19713) proves that controller dead with its own liveness rules. > - The durable-ownership plan (#19831) replaces private proofs with one prover (`proveOwnerGone`, #19853) and one registry (#19861). > - This pull request moves the accepted-result executor lease to that shared path and deletes the private sweep. It is Phase 2's first kind. > - The benefit is one tested rule for when a dead owner's result lease is released, and the reconciler now repairs a lease that restart recovery missed. ## Linked Issues or Issue Description Refs #19831 (plan), #19853 (prover), #19861 (registry), #19713 (old sweep), #19883 (legacy host fallback), #19927 (Phase 2 data). **What happened?** The accepted-result lease had its own liveness proof, separate from the shared prover. The reconciler could only report a dead owner, not settle it. When restart recovery missed a lease, it waited out the 20-minute lease. **What did you expect?** One prover decides, and both restart recovery and the periodic reconciler pass settle a lease whose controller is proven gone. **Steps to reproduce** 1. Accept a native run result. 2. Kill the controller (SIGKILL) and start a new server. 3. If the restart sweep does not see the row, the result stays leased until the lease expires. ## What Changed - `ownership-registry.ts`: a kind may register `settle` and `onUnprovable`. `native_accepted_result_lease` registers both. Other kinds stay report-only. - New `native-accepted-result-lease-settlement.ts`: a compare-and-swap settle that expires exactly the proven owner's lease. It keeps the old guards: the run must still be `running`, remote copyback stays fenced, and a lease taken over since the proof is left alone. - `ownership-invariant-report.ts`: the pass calls `settle` on `gone` and `onUnprovable` on `unprovable`. A read-only pass (`record: false`) never settles. New `settleOwnableKind()` for restart recovery. - `heartbeat/recovery.ts`: restart recovery calls `settleOwnableKind("native_accepted_result_lease")` instead of the old sweep. - Deleted `native-dead-controller-result-lease.ts`. - `owner-proof.ts`: accepts a board attestation (`operator_attested`). It also keeps the one-release local fallback for rows saved before their PID space was recorded, on single-host servers only, and flags each use so it is logged (`legacy_host_unknown_dead_pid_proof`, #19883). An ephemeral server returns `unprovable` (`process_space_unrecorded`) and the existing "confirm the previous server stopped" board action is raised. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts`: 27 pass, 7 of them new. - `npx vitest run src/services/owner-proof.test.ts src/__tests__/ownership-reconciliation-report.test.ts`: pass. - Red/green (tests written after the code): 5 new behaviour tests fail on master and pass here. They cover the ephemeral board action, the reconciler-pass settle, the live-owner counter-case in that pass, the legacy marker, and operator attestation. The stamped-dead-controller and foreign-PID-space cases pass on both, as the old path behaved the same. - Server typecheck passes (`npx tsc --noEmit -p server/tsconfig.json`). - Not run locally: the full server suite (`pnpm verify` asks for broad coverage because a file was deleted). Required CI covers it. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): NRF rows saved before #19868 have no `controller_host_hash`. They keep the same one-release local proof as before on single-host servers, and each use is logged. On ephemeral servers they stay unprovable and ask the board, as before. Rows on another PID space keep their lease (the fence), as before. - Fail-open risk: settle is compare-and-swap on the lease token, boot, PID, host hash and start time of the proven owner. An `unprovable` owner is never released. Live-owner and foreign-space cases are tested. - No change adds work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
18644a3263 |
feat(recovery): instrument 20 durable write boundaries with crash points (#19938)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A control-plane restart between two durable writes leaves an in-progress record owned by a dead process > - #19937 added `crashPoint(name)` and a registry of 20 named boundaries from the Phase 0 inventory > - A name without a call site cannot fire, so the crash-point suite cannot test it > - This pull request puts one call at each registered boundary and adds a test that keeps the registry and the call sites in step > - The benefit is that the on-demand suite (next pull request) can kill the control plane at every known write-ahead gap ## Linked Issues or Issue Description Refs #19831 (plan, Phase 5). Refs #19846 (inventory, "Write-ahead coverage"). Stacked on #19937 (helper and registry). Refs #19861 (ownable kind ids). **Problem** The registered crash points have no call sites. **Expected behavior** Each registered point is called at its gap: after the first durable write or outside effect returns, before the next one starts. Disarmed (always in production) each call is one comparison. **Scope** Call sites and a coverage test only. The suite runner and CI workflow follow. ## What Changed - Server call sites (`@paperclipai/shared/crash-points`): - `sandbox-creation-operations.ts`: around the provider `create` call (intent → provider → lease). - `native-session-executor.ts`: after `native.process_start_requested`, before spawn. - `paperclip-control-plane-port.ts`: after the result-accept transaction, before finalization starts (only when a result was inserted). - `native-run-finalizer.ts`: after `commitNativeStatusDecision`, before the run projection. - `workspace-operations.ts`: `workspace_finalize` only — after the running row, and after `settle()` before the done row. - `environment-runtime.ts`: provider release → lease row; teardown claim → destroy; reusable destroy → lease release; stop-and-retain claim → stop. - `heartbeat.ts`: lease release → runtime services; budget reservation hold → dispatch. - `heartbeat/queue.ts`: run claimed `running` → wake request `claimed`. - `run-cost-accounting.ts`, `usage-receipts.ts`: accounting commit → spool unlink; receipt spool → DB row → unlink. - `packages/paperclip-runner/src/crash-point.ts`: a small copy of the helper for controller code in this package, which does not depend on `@paperclipai/shared` (adding that dependency would change the runner content ID). It arms only `runner.*` names. runnerd never sees the variable: its environment is an allowlist. - Runner call sites: after the runnerd spawn, before its identity is published (both spawn paths); `turn.start` result received → controller journal saved → next send. - `server/src/__tests__/crash-point-call-sites.test.ts`: every registered point has a call site, every call names a registered point, and every `kinds` entry is an ownable kind id of the Phase 1 registry. ## Verification - `cd server && npx vitest run src/__tests__/crash-point-call-sites.test.ts` → 3 passed. - `pnpm --filter @paperclipai/server typecheck` → clean. `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit -p .` → clean. - Behavior with nothing armed is unchanged: each call returns at its first comparison (measured in #19937: about 3.7 ns). ## Risks - Low risk. No call changes control flow unless `PAPERCLIP_CRASH_POINT` names it and `NODE_ENV` is not `production`. - Points on the path before `turn.start` (process start, runner spawn) cost nanoseconds, within plan invariant 8. - `usage-receipts.ts`: the persist-then-unlink one-liner became a block with the same statements in the same order. - The runner package change gives the runner a new content ID, as any runner source change does. No persisted run state changes (rules/upgrade-safe-run-state.mdc). ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits) and a read-only exploration subagent that mapped the boundaries. ## 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> |
||
|
|
f35eb1e5ef |
fix(observability): judge the TTFT budget from run creation; report the wait behind a live turn (#19941)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Invariant 8 of the durable-ownership plan says turns must start fast. #19863 added per-run speed budgets that write `run.performance.budget_exceeded` to the run log. > - The Phase 2 chaos data (#19927) showed follow-up TTFT about 3x over the claude budget. > - The cause is measurement scope. A follow-up comment posted during a live turn waits for that turn to end, and TTFT is anchored at the comment. The budgets came from idle follow-ups. > - This pull request judges the TTFT budget from run creation and reports the wait separately. > - The benefit is that budget breaches point at real start-up slowness, not at time spent behind a turn the user is already watching. ## Linked Issues or Issue Description Refs #19863 (speed budgets), #19927 (Phase 2 data), #19852 (startup sub-spans), #19888 (warm default). **What happened?** In two local storm waves, C1 follow-up runs breached the claude warm TTFT budget (p50 27.6–29.9 s against 9.5 s). Their spans show `comment.to_run_created` at 6–62 s (waiting for the live turn), then session resume at 1.4–3.8 s and submit-to-first-event at 2–5 s. Codex without the wait was about 4.8 s, the same as the warm-follow-up baseline. **What did you expect?** The budget should judge start-up latency, which is what its baseline measured. The wait behind a live turn should be visible but not judged. **Steps to reproduce** 1. Run a local chaos storm with C1 tasks. 2. Read `run.performance.budget_exceeded` rows with metric `ttft` for `issue_commented` runs, and compare them with each run's `comment.to_run_created` span. ## What Changed - `native-run-trace.ts`: the TTFT budget check measures from run creation (start of `heartbeat.queue`, or end of the ingress span). The TTFT span itself does not change. - `native-speed-budgets.ts`: breaches can carry `inputWaitMs` (TTFT only). - `tests/restart-chaos/ttft-baseline-stats.ts` and `ttft-baseline.ts`: the wait behind a live turn is its own measured step (`inputWait`), next to the full TTFT and the TTFT from run creation. The TTFT table gains a "Wait behind live turn" column. The full comment-to-first-token time stays the user-visible metric. - `doc/run-log-events.md` and the plan's speed-budget section describe the rule. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-speed-budgets.test.ts src/services/native-runtime/native-run-trace.test.ts`: 43 pass, 2 new. `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/ttft-baseline-stats.test.ts`: 12 pass, 1 new. They cover a follow-up under budget after a 60 s wait (no breach), a follow-up over budget (breach with `inputWaitMs`), and queueing after run creation still judged. - Server typecheck passes. ## Risks - Low. This is a run-log-only change (AGENTS.md §5.7): no Telemetry and no span attributes change. - A slow wait before run creation is no longer a TTFT budget breach. The TTFT span still records it, and the breach payload names it. Admission holds after run creation are still judged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
2fb4837cca |
fix(chat): tell chat how to link execution runs and name their agents (#19889)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Chat lets a person ask an agent about work, such as the runs behind
a task
> - The chat guidance gives a link template for tasks only. It does not
give one for execution runs
> - The issue runs API returns `agentId` and the run `id`, but no agent
name and no link
> - So when a person asks "Show me the runs behind this website task",
the agent lists raw UUIDs. It does not link each run. It does not name
the agent that did the work
> - This pull request adds the registered run route to the chat link
guidance. It also tells chat to read the agent and name it, not show a
raw ID
> - The benefit is that a person can open each run (including the
cancelled attempt) and sees who ran it
## Linked Issues or Issue Description
**What happened?**
A chat agent was asked to show the runs behind a task that had a
cancelled attempt and a successful retry. It read `GET
/api/issues/{id}/runs` correctly, but its reply showed both runs as raw
UUIDs in code spans and named the agent as "`fe03dd15…`, the Website
Brief worker" instead of its name. It gave no run links.
**What did you expect?**
Each run links to its run detail page
(`/<prefix>/agents/<agentId>/runs/<runId>`), and the reply names the
agent that executed it.
**Steps to reproduce**
1. Create a task assigned to an agent, cancel its first run, and retry
it so a second run succeeds.
2. In chat, ask "Show me the runs behind this website task."
3. The reply lists run UUIDs without links and identifies the agent by
ID.
Refs #19614 (the chat catalog eval case `activity.find-runs` shows this
failure).
## What Changed
- `server/src/services/agent-conversations.ts`: the chat link guidance
now gives the run route `/<prefix>/agents/<agentId>/runs/<runId>` and
tells chat to read and name the agent instead of showing a raw ID.
- `server/src/__tests__/agent-conversations.test.ts`: a regression test
checks that the run template matches the registered UI route and reaches
the chat task prompt.
## Verification
- `cd server && npx vitest run src/__tests__/agent-conversations.test.ts
-t "chat prompt policy"`: 7 passed. The new test fails without the
guidance change.
- `src/services/heartbeat/task-markdown.test.ts` passes.
- The embedded-Postgres suite in `agent-conversations.test.ts` did not
start in my local sandbox (Postgres init error). It does not depend on
this prompt text. CI runs it.
- End-to-end: the chat catalog case `activity.find-runs` is rerun with
this commit on its eval branch.
## Risks
- Low risk. This is prompt guidance text only. No API, schema, or UI
change.
- It conflicts textually with #17985, which edits the same guidance
line. The merge is simple: keep both sentences.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude
Code with tool use (shell, file edits). Default reasoning mode.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] 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>
|
||
|
|
7fd98682ca |
fix(workspaces): give every subtask its own execution workspace (#19860)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents split work into child tasks, and each task runs in an execution workspace (a checkout, branch, or sandbox folder) > - Since early 2026, a new child task used its parent's realized workspace by default: the same checkout, branch and sandbox > - A parent and its child often run at the same time. Two writers in one folder lose each other's files, the waking parent is cancelled with `workspace_busy`, and sandbox lease ownership crosses from the parent into the child, so retained-file recovery refuses the workspace as ambiguous > - The product rule is simple: a subtask gets its own workspace, independent of the parent's saved files. Paperclip never needs to carry a parent's unmerged changes into a child > - This pull request makes that the only behaviour on every creation path, and moves existing shared children to their own workspace on their next run, with no user or admin action > - The benefit is that parents and children never block or overwrite each other, and recovery ownership stays with one task ## Linked Issues or Issue Description **What happened?** A child task inherited its parent's realized execution workspace by default (`createChild` defaulted to `executionWorkspaceInheritanceMode: "linkage"`, and `issueService.create` fell back from `inheritExecutionWorkspaceFromIssueId` to `parentId`). When the parent woke while the child wrote to the shared workspace, the parent run was cancelled with `workspace_busy`. In sandbox environments, the lease ownership chain crossed from the parent into the child, and retained-file recovery refused the workspace with `allocation_ownership_ambiguous`. **Expected behavior** Every new child task gets its own execution workspace, realized from the project base exactly as a top-level task in that project. It never uses the parent's checkout, branch, saved snapshot or unmerged changes. A parent and its descendant never share a workspace by any route. **Steps to reproduce** 1. Run a parent task in a project with isolated (git worktree) or sandbox workspaces, so the parent has a realized execution workspace. 2. From the parent run, create a child task with only `parentId` (or through the runner `create_task` tool, chat suggestions, decision effects, plugins, or the New sub-task dialog). 3. See that the child's `executionWorkspaceId` is the parent's, with `executionWorkspacePreference: "reuse_existing"`. 4. Wake the parent while the child runs in a sandbox environment. The parent run is cancelled with `workspace_busy`. **Additional context** Refs #9233 (the existing `strategy_only` opt-out). The opt-out `executionWorkspaceInheritanceMode: "strategy_only"` already existed for accepted-plan decomposition. This change makes that behaviour (tightened to drop a pinned existing branch) the only behaviour. ## What Changed - **One rule for every creation path (`server/src/services/issues.ts`).** `issueService.create` still uses the parent (or `inheritExecutionWorkspaceFromIssueId`) as a source for the project, project workspace and workspace *strategy*, but it never copies the source's `executionWorkspaceId`, `reuse_existing` preference, or pinned existing branch. All server paths that create a task go through `create()`/`createChild()`: issue routes (`POST /issues`, `POST /issues/:id/children`, the run-bound default parent and run workspace source), runner `create_task`, chat suggestion acceptance, native status decisions, accepted-plan decomposition, decision effects, plugins (`ctx.issues.create`), external agents, watchdog follow-ups, monitor recovery issues, case handoffs and email channels. - **Explicit ancestor workspaces are dropped.** If a create names an `executionWorkspaceId` (or an `existing` workspace selection) that an ancestor of the new task created or is bound to, `create()` drops it and keeps only the strategy. Older clients and skills that copied the parent's id keep working and get an independent workspace. - **`createChild` no longer has a "linkage" mode.** `executionWorkspaceInheritanceMode` is still accepted from internal callers and is ignored. - **`inheritExecutionWorkspaceFromIssueId` decision.** I kept the field for API compatibility, but it now copies only the source issue's project and workspace strategy. I removed its workspace-sharing behaviour for every pair, not only parent/child: the source task and its follow-up can run at the same time (the runner, watchdog and status-decision callers all create the follow-up while the source run is live), so sharing would again let two tasks write one folder. Explicit sharing between unrelated tasks stays available through an explicit `existing` workspace selection. - **New `server/src/services/task-workspace-lineage.ts`.** `buildIndependentTaskWorkspaceSettings` (strategy only: mode, environment, runtime, network egress, shared-workspace concurrency, base ref, branch template, commands; no `reuse_existing`, no `operator_branch`, no `existingBranch`), `loadTaskLineageIssueIds` (the parent chain plus, for a task that depth placement put beside its requester, the requester's chain via `privacyParentIssueId`) and `isAncestorExecutionWorkspace` (the workspace's `sourceIssueId` is in that lineage, or a lineage issue is bound to it). A child in another project does not take the parent's strategy. - **Upgrade-safe rebind at admission.** `executionWorkspaceService.releaseAncestorWorkspaceBindingAtAdmission` runs at the same admission point as the pending workspace selection (`heartbeat.ts`). For a child (or a flattened task beside its requester) that an older release bound to an ancestor's workspace, it clears the child's binding (keeps the project workspace and strategy, bumps the binding revision) so the run realizes the child's own workspace. It does nothing while another run of the child is running, while the child's earlier run still owns workspace finalization or recovery, for native restart/lease-owner recovery, for a process-loss retry, or for a Retry that keeps the current workspace. The parent and its workspace are never changed. - **Projects without a project workspace (per-task repository acquisition).** A parent with no project workspace worked in its own task files, so its settings (for example `isolated_workspace`) do not describe the project base. A child of such a parent, and an older child released from one, takes the project's defaults exactly like a top-level task, instead of an isolated mode that would resolve to a git worktree with no git base ("needs a project / project workspace"). When the child's new workspace is created at admission, `executionWorkspaceRepositoryService.prepareForAdmission` repeats the parent's (or requester's) repository requests as the child's own pending requests (identity, URL, catalog id and requested ref only; no pinned commit, branch or files). The normal acquisition path then clones a fresh copy into the child's task files with the child's own access checks. This runs only for a workspace created by that admission, so existing tasks never gain repositories. - **UI.** The New sub-task dialog no longer pre-selects the parent's workspace. `buildSubIssueDefaults` drops `executionWorkspaceId` and `reuse_existing`, and derives the parent's independent mode (`shared_workspace`, `isolated_workspace` or `agent_default`) from its preference, saved settings or current workspace. The "This sub-task will no longer use the parent task workspace" warning is removed. - **Contracts and docs.** Shared validator and plugin SDK comments describe the new meaning of `inheritExecutionWorkspaceFromIssueId`. Updated `doc/execution-semantics.md` (new rule in §6 and a wording fix in the retained-sandbox paragraph), `doc/SPEC-implementation.md` (child workspace rule, requester keeps *strategy* inheritance, watchdog follow-ups), `doc/SPEC.md` (task workspace ownership), `docs/specs/external-task-protocol.md`, `skills/paperclip/SKILL.md` (step 9 and the workspace bullet), `skills/paperclip/references/issue-workspaces.md`, the CEO onboarding `HEARTBEAT.md`, and the plugin SDK README. ### Previous-release story (`rules/upgrade-safe-run-state.mdc`) - **Migrated on next admission.** A child that the previous release bound to its parent's (or any ancestor's) execution workspace keeps that binding until its next normal admission. Then the server clears the binding and the run realizes the child's own workspace from the project base. No schema migration is needed. No user or admin action is needed. - **In-flight writers finish first.** A running child is not interrupted. Its current run, its native restart recovery (admitted native input or lease owner), a process-loss retry, and a Retry that keeps the current workspace all finish in the shared workspace. While a child's earlier run is still running or its workspace finalization or recovery is unsettled, the binding stays and a later admission releases it. - **No files are dropped.** The release changes only the child's binding row. The parent keeps its binding, and the shared workspace row, folder, branch and saved snapshots are not touched. Anything the child left there stays in the parent's workspace. - **No new refusal codes.** Saved run identity (native execution input, leases, checkpoints) is not changed or refused; the rebind only affects new admissions. ## Verification - New `server/src/__tests__/subtask-workspace-independence.test.ts` (embedded Postgres): `create` with `parentId`, `createChild` with no mode / `"linkage"` / `"strategy_only"`, `inheritExecutionWorkspaceFromIssueId` pointing at the parent, explicit parent and grandparent `executionWorkspaceId`, a non-child follow-up, and unchanged top-level defaults and explicit top-level reuse. 7 creation cases fail on `master` and pass with this change. Admission cases: an old-release child is rebound while its parent is running and the parent keeps its workspace; a running child is not rebound (deploy lands mid-run); an unrelated explicit share is kept; the parent is never rebound. - New heartbeat case in `server/src/__tests__/heartbeat-workspace-busy.test.ts`: an old-release child bound to its running parent's workspace in a sandbox environment runs to `succeeded` with no `workspace_busy`, gets a new workspace, and the parent run and binding stay unchanged. It fails on `master` (the child waits with `workspace_busy`). - Updated tests that asserted the old inheritance in `issues-service.test.ts`, `task-workspace-binding.test.ts`, `chat-project-tools.test.ts`, `task-workspace-child-trust.test.ts`, `heartbeat-git-restore-point-resume.test.ts` (a child bound to a closed parent's expired workspace now starts from the project base and leaves the parent's restore point alone), and `ui/src/lib/subIssueDefaults.test.ts`. Remote-workspace reservation, continuation and writer-wait suites now seed the older-release shared shape directly (or an unrelated explicit share), so those safety paths stay covered. - `npx vitest run` on the changed suites and about 45 related server suites (heartbeat workspace, recovery admission, issue routes, routines, plugins), `ui` `subIssueDefaults` and `NewIssueDialog` tests, `pnpm check:token-gates`, `pnpm -r typecheck`. - New `server/src/__tests__/heartbeat-subtask-acquired-repository.test.ts`: a project with no project workspace, a parent whose task files hold an acquired repository with an unmerged change, and a child (new, and the older-release shape bound to the parent's workspace) that runs to `succeeded` in its own task files with a fresh clone at the project base and its own ready repository receipt. The parent's checkout keeps its change. Both cases fail on the previous head with "needs a project / project workspace". ## Risks - **Project-level configuration still applies to children exactly as to top-level tasks.** In a project configured for `shared_workspace`, every task (top-level or child) uses the project's one folder under the existing shared-workspace concurrency policy. A branch that the project itself pins applies to every task in the project. This PR removes only the parent-to-child carry-over of the realized workspace; it does not change those project defaults. - **Standalone follow-ups that an older release bound through `inheritExecutionWorkspaceFromIssueId`** are migrated only when the source is in their recorded lineage (parent or privacy parent). Others keep their explicit share and the existing writer-wait serialization. - **Behaviour change for agents that relied on a shared checkout.** A child or follow-up no longer sees the parent's unpushed or uncommitted changes. Agents must push or merge code they want a subtask to build on. The skill and onboarding text now say this. - **Explicit ancestor workspace ids are silently dropped** instead of rejected, so older clients do not break. The task gets an independent workspace. - **First run of a migrated child starts from the project base.** If an older child had uncommitted work in the parent's workspace, that work stays in the parent's workspace and is not copied into the child's new workspace. - **Admission adds one indexed read** for tasks with a parent and a bound workspace; the locking transaction and ancestor query run only when the bound workspace was created by another issue. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (file edits, shell, test execution) and extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
5f1b1bb81d |
fix(recovery): continue from the last save when a retained source cannot be verified (#19867)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a native run cannot save its files, a retained-file recovery hold blocks the task until the worker saves them from the retained sandbox. > - The worker verifies the recorded source strictly. This includes the reusable-lease chain of the sandbox. > - A reusable sandbox chain can cross from a parent task into a child task. The check then refuses the source with `allocation_ownership_ambiguous` on every attempt. > - Retry repeats the same refusal. The task stays blocked by three gates: the saved-work wait, the repair flag and the remote writer hold. The only text left tells a person to inspect the run and its environment. > - The source check must stay strict. Recovery must still work without a person. > - This pull request adds an automatic fallback: the task continues from its last committed workspace save, the unverified files are attached to the task but never applied, and the unverified workspace epoch is abandoned so its root is never reused. > - The benefit is that tasks with an unverifiable retained source continue by themselves, with no lost saved work and no admin action. ## Linked Issues or Issue Description Refs #19840 (removes the "inspect the run and its environment" text and keeps Retry; this change removes the need for that Retry). **What happened?** A child task ran in a reusable sandbox that it inherited from its parent task. The run saved a checkpoint, wrote more files, and then failed to save them. Retained-file recovery refused the source with `allocation_ownership_ambiguous`, because the lease history of the sandbox crosses into the parent task. The hold was exhausted after one attempt. Retry made a new generation that failed the same way. The task stayed blocked with `saved_work_recovery_failed`, `workspaceRepairRequired: true`, and the failed run still the dirty writer of the workspace. A second task had a stale exhausted hold for a run whose files a sibling recovery action had already saved. That hold also kept the task from continuing. **Expected behavior** Recovery works without a person. When the source cannot be verified, the task continues from its last saved version. The unverified files are not applied, but they stay available. A stale hold whose files were already saved settles as recovered. **Steps to reproduce** 1. Use a native run with a remote workspace whose reusable sandbox lease history includes a lease of another task. 2. Let the run save a checkpoint, write more files, and fail its final save. 3. Let retained-file recovery run, or press Retry. 4. Before this change, the hold is exhausted with `allocation_ownership_ambiguous` and the task stays blocked. **Paperclip version or commit** Observed on deployed builds that include `630411af53`. Holds saved by the Nov 3–6 releases have this shape. ## What Changed - New `server/src/services/workspace-recovery-fallback.ts`: - `continueFromLastSavedWorkspace` runs when a hold is exhausted by a permanent source refusal: `allocation_ownership_ambiguous`, `source_binding_invalid`, `source_unavailable` or `historical_source_missing` with retry class `service_required` or `use_alternative_strategy`. It applies only to remote workspace sources in the `prepared` state. - Quarantine (secrets omitted under the workspace file policy: `.env`, keys, repository metadata, dependency directories): if the retained root is readable, the worker copies it with the existing read-only capture (no write to the sandbox), converts it to a `.tar`, stores it, and attaches it to the task as `unverified-files-run-<runId>.tar`. A `workspace_recovery_quarantine` operation records it as `verified: false, applied: false`. If the root is not readable, the worker records a closed reason label. - Stop proof: every allocation of the source run must carry a provider stop receipt. A missing receipt is obtained through the same stop-only cleanup as normal recovery (`prepareSandboxStopAndRetain` / `prepareHistoricalSandboxStopVerification`, `retryPendingSandboxTeardown`, `settleStopOnlyCleanup`). A sandbox that another lease holds (`active` / `pending_cleanup`) is never read or stopped from here; the hold waits. - One transaction (lock order: task, coordinator, run, recovery action, workspace) re-checks all facts including the stop receipts, abandons the epoch, resolves the hold with the named outcome `retained_source_unverifiable_continued_from_last_saved`, records an `executionReconciliation` decision with `retainedSourceFallback`, and denies native replacement. A Blocked task belongs to the hold only while the hold's own `workspaceFinalizationBlock` receipt still owns that status version, or, for receipt-less holds saved by older releases, while the block was set by the run's terminal failure (within its finalization window) and no status decision followed. Only then is the task restored to `todo` and a continuation scheduled; a newer block is kept and no continuation starts. The decision carries machine `workspaceRepairEvidence`, which the reconciled admission requires for runs with a restore failure. - Continuation: with no waiting user input, the hold uses `continuationDelivery: "pending"`, so the existing reconciled delivery starts one continuation. Waiting user input continues the task itself (`not_requested`), so there is never a second continuation. - Agent note: the decision's `outcomeEvidence` (part of the continuation's recovery outcomes) says that the run's unsaved changes could not be verified and were not applied, where the attached copy is, that the agent should check what to redo, and that completed external actions must not be repeated. - `settleSiblingRecoveredHold`: an exhausted hold whose run has a verified repair receipt from a different recovery action settles as recovered. It runs for every exhausted hold, including adoptable native sources. A sibling marked recovered whose receipt no longer verifies records a permanent deferral, so discovery does not reselect it. - Deferral: when the fallback cannot settle yet, the hold records `retainedSourceFallbackCheck` (reason, generation, count). Transient reasons (live writer, running operation, missing stop, sandbox in use, pending status effect, lock busy, storage failure) are checked again after 5 minutes, at most 12 times. Permanent reasons (uncertain external action, non-remote source) wait for new input, Retry or a new generation. Storage or database failures after a successful read are retryable and never recorded as "unreadable". - New `server/src/services/workspace-recovery-fallback-discovery.ts`: SQL for pending and due fallbacks. `retainedWorkspaceCandidates` (the production worker's obligations sweep) and `nextWorkspaceRecoveryAt` (its scheduler) include them, so the worker reaches old holds without saved input. - New `server/src/services/remote-workspace-epoch-abandonment.ts` (kept out of `remote-workspaces.ts`, which is near 1,000 lines): authority transition `abandonUnverifiedRemoteWorkspaceEpoch`. It clears the writer and the dirty flag, keeps `snapshotId`, drops `location`, advances the epoch, clears `providerRef` and the migration root, and records the abandoned epoch. The next preparation restores the last save into a new `<workspace>-<epoch>-<lease>` root. - `server/src/services/workspace-recovery.ts`: hooks. An exhausted hold first tries the sibling settlement, then the fallback (under the existing workspace ownership lock). A fresh permanent refusal in an attempt runs the fallback in the same pass, under the same ownership. - `remote-workspace-transport.ts`: export the existing read-only capture. - `packages/shared`: optional `retainedSourceFallback` on `ExecutionReconciliation`. - `doc/execution-semantics.md`: document the fallback. - No new polling or worker: the fallback runs inside the existing retained-workspace recovery pass (`rules/sleep-aware-background-work.mdc`). Safety: - The cross-task lease check (`retiredReusableLeasePredecessors`) is unchanged and stays strict. Unverified files are never applied. - No second executor: the source run, its controller and its finalizer must be stopped. A live writer of the task or workspace, or a running workspace operation, keeps the hold. - No repeated external side effects: pending status effects, non-completed or unknown API receipts, and uncertain tool invocations keep the existing reconciliation path. The continuation keeps the existing API receipt fences. - Never overwrite another run's files: only the exact epoch that the failed run wrote is abandoned, and the old root is never reused. - Bounded and idempotent: a settled hold is never revisited. The quarantine record is reused. A changed generation or revision leaves the hold unchanged. Previous-release story (`rules/upgrade-safe-run-state.mdc`): holds saved by earlier releases carry forward. A hold saved by the Nov 3–6 releases (`systemRecovery` exhausted with `allocation_ownership_ambiguous` / `service_required`, board owner, old admin text) takes the fallback on the next worker pass after deploy (restart sweep, Retry, or a new message). No migration is needed. Refusals that do not qualify keep today's behavior. The only new persisted shapes are additive: `retainedSourceFallback` and `siblingWorkspaceRecovery` on the hold, `remoteWorkspaceAbandonedEpochs` on the workspace, and a `workspace_recovery_quarantine` operation. Earlier releases ignore them. ## Verification - `cd server && npx vitest run src/services/workspace-recovery-fallback.test.ts` — 15 tests: - cross-task hold, saved by the previous release and refused fresh: continues from the checkpoint snapshot, the quarantine `.tar` holds the unverified file, the epoch is abandoned, `getExecutionBlocker` is `null`, the task is `todo`, explicit continuation is not blocked by `saved_work_recovery_failed` or `workspace_repair_required`, one reconciled continuation is delivered, and the next writer restores into a new root while the old root keeps its files; - unreadable root: continues and records the reason; - idempotency: a second pass changes nothing and delivers no second continuation; - waiting user input: no second continuation; - negatives (live writer, pending side effect, uncertain external action, sandbox held by another lease): the hold stays, the authority is unchanged, nothing is attached or delivered, the deferral reason and recheck time are recorded; - an allocation without a stop receipt is stopped (stop-only) before the hold settles; - the production worker (`reconcileRetainedWorkspaces` with `obligationsOnly`) and `nextWorkspaceRecoveryAt` discover and settle an old hold with no saved input; a transient deferral is not revisited until due, then settles; - an old-release hold without a Blocked receipt keeps its status; a current hold with the receipt is restored to `todo`; - stale exhausted hold with a sibling-saved receipt settles as recovered; one whose sibling receipt no longer verifies is recorded and no longer discovered; - a newer Blocked decision is kept and no continuation is delivered, while the file hold settles; - the attached copy omits `.env` and key files. - RED: with `workspace-recovery.ts` and `system-recovery.ts` reverted to master, all tests fail. - Regression suites pass: `workspace-recovery-warm-handoff`, `workspace-recovery-retry-copyback`, `workspace-recovery-preparation-gates`, `workspace-recovery-source`, `task-retry`, `execution-blocker-read` (288 tests), and `workspace-recovery-admission`, `workspace-recovery-preparation*`, `remote-workspaces.integration`, `native-failed-workspace-recovery`, `remote-workspace-transport`, `workspace-restore-repair-route` (683 tests). - `server` typecheck passes. ## Risks - A task now continues without the unverified changes of its failed run. The agent receives a note and the attached copy, so it can redo or re-apply the work. This replaces an indefinite block. - Abandoning an epoch on a workspace shared with a parent task also moves the parent to the last save. This is correct: the unverified changes are not part of the saved lineage. - Clearing the migration root means sessions bound to the old root start fresh. The fallback continuation already forces a fresh session. - The quarantine capture reads a sandbox that can be shared with another task. It writes nothing there, and it verifies the copy against inventories taken before and after. ## Model Used - Provider: Anthropic. Model: Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, read-only browser for API inspection) 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> |
||
|
|
35cbd15175 | fix(native): recover a stopped Runner whose journal stayed ready at a settled cut (#19907) | ||
|
|
94718ce168 | fix(native): continue on a replacement when a retained Runner retires before claim (#19897) | ||
|
|
91b044858c | fix(runner): send the prepared task scope with retained release and resume (#19896) | ||
|
|
5020f38f90 | fix(runner): give a later attempt a new task when its stopped task kept no session (#19899) | ||
|
|
eb7c7b083a |
Recover Muse verification after contact, restart and access restoration (#19909)
## Thinking Path > - Paperclip manages AI agents and their work under company access rules. > - Muse readiness uses a saved connection, receiver contact and an authenticated challenge reply. > - A failed challenge write could leave a saved contact with no later recovery trigger. > - A server restart or restored access must recover that contact under current authority. > - This pull request uses the existing delivery coordinator and saved Muse state for recovery. > - It keeps the normal receiver schedule, authorization checks and real deadlines. > - It lets eligible verification recover and lets idle verification stop work. ## Linked Issues or Issue Description **What happened?** The first Muse receiver contact could persist while readiness challenge creation failed. The contact then had no durable retry or startup recovery path. Later membership, lifecycle or feature restoration could also leave pending verification without a notice. **Expected behavior** Retry a temporary failure through the existing worker. Recover pending contact after restart. Recheck the current company, agent, operator, credential and binding generation before issuing a challenge. Stop verification work while it is ineligible or complete. Do not revive revoked credentials. **Steps to reproduce** 1. Use the normal invitation and OAuth registration, PKCE and token flow. 2. Send one receiver contact and let its normal persistence schedule run. 3. Fail the first readiness challenge operation before it commits. 4. Observe whether the existing worker retries without another contact or a manual Verify action. 5. Restart the App with a new database owner. Check that it creates one usable challenge from the saved contact. **Paperclip version or commit** The causal baseline test used the unchanged production implementation at `90148317b2a447794ff29a4acae5648ad11a9770` plus its normal fixture. The accepted positive test used `9d383041028776ad8469ac4af3fe14ba6c369425`. The initial published head was `fe518c9b72504daf6374225987f24d45bc7bcf75`. The lease/auth correction was locally qualified at `cfd515a207416097fe8b586277d70c4fd4a679fa`. The current candidate is `6efe722e7eb1e22acd466125c7b4a627611d0273`; its added Cloud actor fixture correction passed the full normal owning program and all 35 tests. Its current normal build, lifecycle owner controls, healthy-wait and restoration measurements are described below. Earlier results below are bound to their stated revisions. Related public changes: Refs #19819 for original-principal authentication. Refs #19750 for the transaction-owned external feature read. These are related foundations, not duplicate fixes. The current Muse title and head-branch searches found no duplicate contact-verification recovery PR. Related maintenance work is #17000 and terminal assignment settlement is #17793. ## What Changed - Add a typed, exact-binding verification notice through the existing Muse delivery queue. - Register the first-contact notice before its durable write in the same transaction. - Recover pending contact from saved state at startup. Retry temporary failures through the existing scheduler. - Recheck current feature, lifecycle, credential and access state after commit. Park unavailable authority without recurring verification scans. - Add narrowly scoped notices to the existing access, lifecycle, configuration, company, budget and settings write owners. - Save a healthy lifecycle verification wait in the existing operation JSONB. Keep generic unresolved-work and error retries unchanged. - Commit authenticated confirmation and its lifecycle recheck in one transaction. Retain a raced recheck under the active lease, and recover saved waits once at startup. - Retain a committed lifecycle recheck while an expired attempt still owns the row, so its late wait cannot park accepted confirmation. Replacement claims keep their existing owner fence and fresh fact read. - Keep the receiver gap, persistence and per-segment contact limits unchanged. - Add real App OAuth, failure, retry, restart, challenge and quiet evidence. Add two normal-route restoration tests. - Document recovery, reconnect requirements and the existing connected-receiver sleep restriction. ## Verification - Causal baseline: the real invitation and OAuth flow saved one contact. The first challenge operation failed. The normal test failed because it observed one verification attempt instead of a retry. No setup or cleanup failure was counted as this regression. - At `9d383041`: normal own Shared/SDK, DB with migration 0441 assets, Runner TypeScript core and surfaces, module boundaries, and all seven normal App programs passed. One new redundant role comparison first failed typecheck; that diagnostic remains recorded and the corrected full App gate passed. - At `9d383041`: all 17 maintenance component tests passed with zero skips. All original deadline, rollback, drain, terminal settlement and startup controls remain. - At `9d383041`: the one normal App case passed. Real OAuth authorized one receiver signal. A retry occurred about 5.01 seconds after the first failure. A new Db/App owner created one challenge, advanced revision 2 to 3, and accepted authenticated confirmation and mailbox ACK. Query instrumentation had an active positive control. All measured query, callback, session and verifier counters stayed equal across the unchanged scheduled 15-second quiet window. Token revocation and old-token/signal rejection passed. - Known App, worker, HTTP and database owners closed without cleanup errors. The tests used bounded local PostgreSQL and HTTP fixtures. They did not call a live Muse model. - Current integrated semantics: the full normal App driver completed all programs. Only a new test callback had two undefined-value diagnostics. An explicit defined-value guard corrected that test. The full 416-root owning program then passed with empty diagnostics. A separate collector incorrectly required an explicit child `noEmit` field; the normal inherited setting was verified without repeating the compiler. The original failures remain recorded. - The first restoration run failed both cases. Permission parking recorded 120 main-pool queries without more verifier calls. The lifecycle counter recorded two denials without their exact scope. These observations remain unexplained at the individual-query/call level. Source separately shows a lifecycle readiness retry every two seconds while the real external reply is absent. - At `fe518c9b`: normal Shared/SDK and DB producers, module policy, all seven full App programs, and all 33 pure diagnostic controls passed. The lifecycle owner file passed all 19 cases, including both confirmation/park orders, rollback, duplicate notices, startup recovery and authority controls. - At `fe518c9b`: the normal OAuth failure/retry/new-Db reconstruction case passed. Its new healthy-wait window measured 15,002 ms with unchanged total activity before genuine confirmation; the existing ready quiet and revocation checks also passed. - At `fe518c9b`: both normal restoration tests passed in Vitest. Ordinary viewer-to-operator restoration and Pause/Resume recovered the same binding, grant and single contact; each committed one challenge and advanced revision 2 to 3. All six parked, restored and revoked/disabled quiet windows measured at least 15 seconds with active positive read controls and unchanged total queries, callbacks, sessions and verifier calls. Both cleanups passed. - The restoration wrapper still recorded failure: it required lifetime lifecycle denial count zero, whereas the measured count was one before the parked quiet window. The reviewed fixture explicitly permits a current-authority denial from an earlier observation and requires no committed challenge before restoration. Precise denial timing relative to Pause was not serialized. The original wrapper failure is retained; no rerun or retrospective explanation of the earlier failures is claimed. - Review exposed an expired-lease race. The added real-store baseline test failed after the late defer restored `waitingForSignal`; no setup or timeout was counted as the regression. On `cfd515a`, the whole lifecycle owner file passed all 20 tests, including that case. The full normal production declaration program (1067 roots), verification program (662), and auth owning program (405) passed; all 1942 declarations remained byte-identical. - The original remote auth-session shard failed three tests because their partial query adapters lacked the new binding query's join/order/limit chain. A fixture-only correction preserved the real notifier and existing authorization assertions. All 12 auth-session tests passed locally. - Complete remote inspection also found 33 failures in the 35-test Cloud actor fixture with the same missing `innerJoin` method and downstream incomplete-auth assertions. Its original full log and exact synthetic Source comparison are retained. At `6efe722e`, the three-line fixture adapter repair passed the complete normal 662-root test program and the whole 35-test file with zero failures or skips. All existing lock, authorization, settings, debounce and unfinished-auth assertions remain. - The earlier healthy/restoration quiet measurements remain bound to `fe518c9b`. Those steady 15-second windows did not exercise a still-owned attempt passing its 120-second lease. The targeted expired-owner regression qualifies that new boundary; it is not a fresh measurement of the earlier windows. - The initial `fe518c9b` remote gates failed the auth fixtures and reported a Greptile 4/5 P1. The aggregate failure reported the underlying shard. Those actual causes were fixed and qualified above. - Current `6efe722e` remote gates completed with **57 successful checks and three explicit skips**. The full latest edited Greptile summary is **5/5**, explicitly reviews this exact head, and has no new finding. The original P1 thread is resolved. All general, inline and nested review pages were read. - One current-head CI attempt timed out after 30 seconds in the Runner evidence/privacy file, with 6,864 passes and one failure. That file and its transitive inputs are byte-identical to the CI base. A separate local diagnostic preserved every assertion and the original timeout, ran the full normal Runner test project and the entire 41-case file, and passed all 41 with zero skips. Its private bounded timing records measured the target body at 74 ms. The diagnostic was then removed, restoring the exact original test. This is local recovery evidence; the original CI stalled await was not recorded and its cause remains **unproven**. - One retry of only the failed jobs on the unchanged head passed the Runner support suite and dependent aggregate verification. The complete second attempt and original failure logs are preserved. No test, timeout, privacy guard or product code was weakened. This result does not turn the original timeout into an explained failure or a diagnosed flake. - Full recursive typecheck, full `test:run` and a full release build were not repeated locally. The normal full App programs, focused producers and tests above passed, and the required current-head remote gates passed. Final review is against the exact unchanged head; local quiet remains separate from deployed suspension. Reviewer commands use the normal project entry points: `node server/scripts/typecheck-layers.mjs`, the whole `server/src/services/muse-maintenance.test.ts` file, and both normal App PostgreSQL files. PostgreSQL fixtures require explicit owned fixture/admin URLs and normal migrations before imports. Run the files serially with one worker. The private controller retains exact Source, output, worker and owner-closure receipts. ## Risks - Missing or early producer notices could lose a restoration trigger. Notices stay in the transaction that commits the write. The worker checks current authority after commit. - A stale notice must not grant access. Binding generation, current membership, company, agent and credential checks remain authoritative. Revoked credentials require reconnecting. - The change adds no table, migration, alternate task queue or periodic verification scan. It adds two optional fields to existing lifecycle operation JSONB. Generic pending/error deadlines, setup holds and manual repair remain obligations. - Confirmation can race with an active verification attempt. The park compare-and-set must retain the durable recheck. Startup recovery uses a fixed cutoff and keyset pages; it must not become another recurring scan. Wrong company, stale version, pause and revoked credentials must stay fenced. - The connected receiver still has its documented tenant-sleep restriction. Worker quiet is not proof of deployed suspension or external wake. - The normal challenge reply is sent by an authenticated protocol fixture. Live Muse background replies, cost/usage, full Muse lifecycle and full hosted Dot behavior remain unqualified. Partial Dot profile ACK and unsigned-device cutover stay withheld. - Restoration coverage for the additional company, budget, configuration and feature producers remains a separate qualification obligation. This pull request does not mark the whole hosted area complete. ## Model Used OpenAI `gpt-6.1-sol` authored the implementation and tests with local code and tool use. A GPT-6-based Codex coordinator reviewed the changes and owns integration. Its exact underlying model ID is not exposed. Context window sizes are unavailable. No context size is inferred. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
2d89486c57 |
fix(native): record OpenCode's rejected model as a model rejection (#19865)
## Thinking Path - Paperclip runs agents through native providers; when a provider rejects the selected model, the person needs to see "Model unavailable" and choose another model. - #19548 made the OpenCode driver report OpenRouter's invalid model ID as a closed `model_unavailable` provider failure. The task banner now shows it. - The server's terminal classifier, however, recognized a model rejection only from Codex's exact ChatGPT error envelope. OpenCode's structured failure was stored for the banner but the run was recorded as `adapter_failed`. - So chat showed a generic failure, and the task was not blocked for a model choice. The `runs-adapters.inspect-failed-run` chat eval caught this. - This change maps an action-required `model_unavailable` failure to `native_provider_model_rejected` for any provider. ## Linked Issues or Issue Description Refs #19548. An OpenCode run whose model OpenRouter rejects (`... is not a valid model ID`, HTTP 400) finishes with `errorCode: adapter_failed`. Expected: `native_provider_model_rejected`, as for Codex. ## What Changed - `native-provider-failure.ts`: `forTerminal` returns `native_provider_model_rejected` when the failed turn's provider failure has category `model_unavailable` and retryability `requires_action`. Transient or unknown retryability still returns null. The diagnostic is the closed turn diagnostic; no provider text or model name is copied. - `NATIVE_MODEL_REJECTION_MESSAGE` no longer names a ChatGPT connection, since it is now the blocker text for any provider. - Tests for both providers, plus negative cases for transient/unknown retryability. - `wake-queue/adapters/postgres.ts`: a native run that failed for a rejected model or incomplete configuration, on a task the arbiter already blocked, is released without opening a `native_continuation_requires_reconciliation` incident. That incident put the run in `recovery_needed`, which hid the run page's Retry, though the provider refused the request and nothing ran to reconcile. Queued messages stay deferred. ## Verification - `pnpm exec vitest run src/services/native-runtime/native-provider-failure.test.ts src/services/native-runtime/status-arbiter.test.ts src/services/native-runtime/native-provider-failure.integration.test.ts` (server): 98 passed. - `pnpm exec vitest run src/modules/wake-queue` (server): passes, including a new test that fails on the old code (it opened the incident). - Server `tsc --noEmit`: clean. - End to end: the `runs-adapters.inspect-failed-run` chat eval will be rerun on the chat-eval candidate with this change. ## Risks - Codex model-rejection blocker wording changes from "current ChatGPT connection" to "current AI connection". - Any provider driver that reports `model_unavailable` with `requires_action` now blocks the task for a model choice instead of the generic failure path, which disables automatic adapter-failure retries for that run. That is the intended behavior for a rejected model. - Configuration-failure native runs on a blocked task no longer get a board reconciliation incident. If such a run had already produced side effects in earlier turns, they are not flagged for reconciliation; a provider model rejection happens before the turn does work. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## 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> |
||
|
|
0f19eb39d3 |
fix(chat): link the profile of an agent chat acted on (#19893)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent Chat lets a person ask the CEO agent to act on other agents, for example to resume a paused agent > - The chat guidance tells the agent how to link tasks, but it does not tell the agent how to link an agent profile > - In a chat eval, the CEO resumed Morgan and reported the result, but the reply had no link to Morgan > - The person then has no direct route to check the saved availability of the agent > - This pull request adds one guidance sentence for agent profile links > - The benefit is that a reply about an agent gives the person a link to that agent ## Linked Issues or Issue Description **What happened?** A person asked the chat agent: "Resume Morgan so they can pick up their work." The agent resumed Morgan through the API and reported the new status. The reply did not link Morgan's profile. The chat guidance gives only the task-link template (`/<prefix>/issues/<identifier>`), so the agent had no route for an agent. **Expected behavior** When a reply is about a specific agent, the reply links that agent's profile at `/<prefix>/agents/<urlKey or id>`. The link is a plain profile link. It is not an `agent://` mention, because a mention notifies the agent. **Steps to reproduce** 1. Pause an agent named Morgan. 2. In Agent Chat with the CEO, send "Resume Morgan so they can pick up their work." 3. Read the reply. It reports the resume but has no link to Morgan. Refs #19889 (adds the run-link template in the same guidance; this change merges cleanly with it). ## What Changed - `server/src/services/agent-conversations.ts`: add one sentence to the chat directive. It tells chat to link an affected agent's profile at `/<prefix>/agents/<urlKey or id>`, and to use `agent://` only to notify an agent. - `server/src/__tests__/agent-conversations.test.ts`: add a regression test that checks the directive and the built task prompt contain the profile-link guidance. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversations.test.ts -t "chat prompt policy"` passes (7 tests). - The new test fails on `origin/master`, because the directive does not contain the sentence. - The route exists in `ui/src/App.tsx` (`agents/:agentId`), and the UI accepts a url key or an id. - End to end: the `agents.resume-agent` chat eval runs with this change on its case branch. ## Risks - Low risk. The change adds prompt text only. No API, schema, or UI behavior changes. - The prompt becomes one sentence longer. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use (shell, file edit), extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
29b6411660 |
fix(chat): give chat replies the approval route (#19892)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent Chat lets a person ask their agent about company state, such as "What decisions are waiting on me?" > - The chat directive tells the agent how to link a task: `/<prefix>/issues/<identifier>` > - The directive gives no route for an approval, and the approval tools return no href > - The completion feedback tells the agent not to guess routes, so the agent names a pending approval but cannot link it > - The person then cannot open the approval from the chat to decide it > - This pull request adds the approval route that the board UI already serves to the chat directive > - The benefit is that a chat reply can link each pending approval to its exact source ## Linked Issues or Issue Description **What happened?** In the Agent Chat eval `inbox-search.show-decisions`, the person asked "What decisions are waiting on me?". The agent found the pending hire approval and listed it. It did not link it. Its recorded reasoning said that it had no known href for approvals and that it must not invent URLs. Task sources in the same reply had working links. **Expected behavior** The reply links the pending approval, so the person can open it and decide it. **Steps to reproduce** 1. Create a pending `hire_agent` approval in a company. 2. In Agent Chat, ask "What decisions are waiting on me?". 3. See that the reply names the approval but gives no link to it. **Paperclip version or commit** `master` at the base of this branch. **Agent adapter(s) involved** Not adapter-specific (core bug). The chat directive is shared by all runners. ## What Changed - `AGENT_CHAT_DIRECTIVE` now says to link approvals as `/<prefix>/approvals/<approvalId>`, with the same company prefix as task links. The board UI serves this route (`approvals/:approvalId` in `ui/src/App.tsx`). - Added a regression test that the directive gives routes for both tasks and approvals and keeps the "never invent a hostname" rule. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-conversations.test.ts` passes (41 tests). The new test fails without the directive change. - End to end: the change is cherry-picked onto the eval branch `chat-eval/case-show-decisions` and run in the Runner Full-Stack E2E workflow for `chat-features.runner-opencode.local.inbox-search.show-decisions`. ## Risks - Low risk. The change adds one sentence to the chat prompt. It does not change routes, permissions or tool results. - The prompt becomes slightly longer for every chat turn. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, GitHub CLI). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |