mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 21:35:36 +01:00
d8078875344e5f8ebcabf7ad1d5afa74645bb583
3207
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d807887534 |
fix(ui): use one status glyph mapping for every task view (#19329)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task status is the main signal on every task surface: sidebar rows, the task header, the status menu, lists, the inbox, pills and chat cards. > - One task state rendered different status icons in different views. A blocked task that waits on an active sub-task showed the blue "in queue" glyph in the task header. The status menu, the chat chip and the sidebar rows showed the red "Blocked" glyph for the same task. > - The cause: each view passed its own subset of the presentation fields to `StatusIcon`. Many views dropped `blockerAttention` and some dropped `externalConversationState`. The status menu rendered its current row from the bare status. The summaries API did not return `blockerAttention`, so the recent-task sidebar could not know the state. > - This pull request puts the state-to-glyph/label mapping in one resolver and makes every task view pass all of its fields. > - The benefit is that the same state uses the same icon and label everywhere. ## Linked Issues or Issue Description No public issue. Found in a hands-on onboarding test on a staging cloud workspace. **What happened?** The sidebar and the task pane/status menu showed different status icons for the same state. On the test task (blocked, waiting on an active sub-task), the task header and the status button showed the blue "in queue" glyph. The selected "Blocked" row in the status menu showed the red blocked glyph. The coordinator sidebar "Recents" and project rows also showed the red blocked glyph, and the chat "Subtask created" chip showed a red "Blocked" chip. **Expected behavior** The same underlying state uses the same icon and label everywhere. **Steps to reproduce** 1. Create a task with a sub-task. Block the parent on the sub-task, and let the sub-task's agent run (the parent is "blocked, covered"). 2. Open the parent task. The header shows the blue "in queue" glyph. 3. Open the status menu. The current "Blocked" row shows the red glyph. 4. With the coordinator sidebar on, look at the task in Recents or in its expanded project. It shows the red glyph. **Paperclip version or commit** `master` at |
||
|
|
131c0c4609 |
fix(server): redact the credentialValues envelope in HTTP logs (#13742)
> **Attribution:** this pull request was written and opened by the
Paperclip agent **Arthur Dent** (Platform & Reliability Engineer,
Tharsia) from an automated run under the host GitHub credential. GitHub
records the author as `stefanriegel`; Stefan Riegel did not write it. No
other edit was made to this description.
## Thinking Path
> - Paperclip is the open source app that people use to manage AI agents
for work.
> - The server writes request details to the user journal. A middleware
module names the fields that are secret.
> - The connect endpoint accepts a `credentialValues` object. The object
maps a provider config path to a secret string.
> - One provider config path is `credentials.authorization`. The caller
chooses the key names. Therefore the key is one flat string, and it is
not a nested path.
> - The redaction path list did not cover the `credentialValues`
envelope. The recursive redactor compares keys by exact match, so
`credentials` and `authorization` in the key set did not match the flat
key `credentials.authorization`.
> - A failed connect request therefore wrote a GitHub token to the
journal in cleartext.
> - This pull request redacts the `credentialValues` envelope in both
layers.
> - The benefit is that no provider secret reaches the journal, for any
key name that a caller chooses.
## Linked Issues or Issue Description
No public issue exists. The report follows the bug report template.
**What happened?**
A `POST /api/companies/{companyId}/tools/apps/connect` request returned
HTTP 502. The server logs request bodies for non-2xx responses. The
journal line contained a GitHub fine-grained personal access token in
cleartext. The token was 93 characters long and started with
`github_pat_`. The line contained no redaction marker.
The logged body had this shape:
```
"reqBody":{"galleryKey":"github","connectionMethodKey":"mcp-key","name":"GitHub",
"credentialValues":{"credentials.authorization":"<TOKEN>"}, ... }
```
**Expected behavior**
The journal line shows a redaction marker in place of the token. No
cleartext credential reaches the journal on any response status.
**Steps to reproduce**
1. Run a Paperclip server on version 2026.916.0.
2. Send a `POST` request to
`/api/companies/{companyId}/tools/apps/connect`.
3. Put a secret under `credentialValues` with the key
`credentials.authorization`.
4. Make the request fail, so that the server returns a non-2xx status.
5. Read the journal line for the request.
Result: the secret appears in cleartext. The two redaction layers both
miss it.
**Paperclip version or commit**
2026.916.0. Reproduced against `master` at
`c65fc9e3c81c41aafe421aa90a00514b84343285`.
The two middleware files are unchanged at `master`
`0f5fafe16b069b8f0c7f17eb9080d010aa6cd513`, so the defect is present at
the current head of `master` as well.
**Deployment mode**
Self-hosted.
**Installation method**
npm package.
**Access context**
Authenticated user, company-scoped route.
**Relevant logs or output**
Token value removed. The journal line has the shape above.
## What Changed
- Add `reqBody.credentialValues` to `HTTP_LOG_REDACT_PATHS` in
`server/src/middleware/http-log-redaction.ts`. This redacts the whole
envelope at the pino serialization boundary.
- Add `credentialvalues` to `SENSITIVE_KEYS` in
`server/src/middleware/redact-sensitive.ts`. This redacts the envelope
by name in the recursive pass.
- Add a test that sends a `credentialValues` envelope through a real
pino logger with the production path list. The test asserts that the
secret is absent.
- Add a test for the flat-dotted-key shape that leaked, and for the
nested shape as a regression.
- Add a test that the envelope is redacted below the top level.
The key-set change is the durable half. The path list only covers body
shapes that someone has already seen leak. The recursive pass matches
the envelope by name for any shape.
## Verification
Run these commands:
```
npx vitest run server/src/__tests__/http-log-redaction.test.ts server/src/__tests__/redact-sensitive.test.ts
```
Against the patched source, both files pass: **72 tests passed, 0
failed**. That is 54 tests in `http-log-redaction.test.ts` and 18 tests
in `redact-sensitive.test.ts`.
The same two files were then run against the **unpatched** source, which
is the parent commit of this branch. **Four tests fail there.** Those
four are exactly the tests this pull request adds:
- `http-log-redaction.test.ts` — "defines the HTTP auth and cookie
header paths that must be redacted". The path list does not contain
`reqBody.credentialValues`.
- `http-log-redaction.test.ts` — "redacts a credentialValues envelope at
the serialization boundary". This is the leak: a real pino logger writes
the token in cleartext.
- `redact-sensitive.test.ts` — "redacts the connector credentialValues
envelope on an app-connect body".
- `redact-sensitive.test.ts` — "redacts a credentialValues envelope
nested below the top level".
The four failures show that each half of the change is load-bearing, and
that both layers failed, not one. The control run also confirms that the
68 tests which pass on the unpatched source still pass on the patched
source, so the change regresses nothing that already held.
The tests assert that the secret string is absent, and they do not pin a
redaction marker. This is deliberate: the two layers write different
markers. `fast-redact` writes `[Redacted]`. `redactSensitive` writes
`[REDACTED]`. An assertion that requires `[REDACTED]` alone fails on a
correct path-list-only fix.
## Risks
Low risk. The change adds two redaction paths. It removes no existing
path.
One behavioral shift is intentional. The recursive pass now replaces the
whole `credentialValues` subtree with one marker, instead of replacing
individual leaves. Any non-secret sibling field placed inside
`credentialValues` is also redacted. This is correct, because the
envelope holds only provider secrets by definition.
One adjacent pull request, `#10784`, adds header paths to the same array
in `http-log-redaction.ts`. The two changes touch different regions of
the array. The second one to merge needs a trivial rebase at most.
## Model Used
- Provider and model: Anthropic Claude, via the Paperclip `claude_local`
agent adapter
- Capability details: tool use and code execution in a sandboxed
workspace
- The patch was authored and verified by automated agent 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
- [ ] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes
- [ ] 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
Left unchecked on purpose: the CI and Greptile items cannot be true
before the pull request exists.
"I have run tests locally and they pass" stays unchecked as a
whole-workspace statement. The two test files named under Verification
were run locally, against both the patched and the unpatched source, and
the numbers above are from those runs. The full workspace suite has not
run.
## Related pull requests
Refs `#10784`, `#6373`, `#8841`.
Co-authored-by: Arthur Dent <arthur.dent@paperclip.local>
|
||
|
|
b86f55f0ba |
test(server): remove two more server test races (#19330)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server Vitest shards gate every pull request > - Two more server tests failed on pull requests that do not touch them (#19309, #19265) > - #19314 fixed four other flakes. These two have different causes > - Random CI failures slow every contributor and hide real regressions > - This pull request removes both root causes without retries or skips > - The benefit is fewer false CI failures ## Linked Issues or Issue Description Refs #19309, #19265, #19314 **What happened?** - #19309, job 127459915333: `workspace-recovery.test.ts` > "keeps ancestor Retry held when current proof changes: sibling_hold" received `requestId: null` from a second Retry request. - #19265, job 127481105563: `paperclip-control-plane-port.test.ts` > "retains a durably recorded governed terminal while its postcommit observer is pending: checkpoint timeout" failed with "native session checkpoint timed out after 100ms" before the terminal was recorded. **Root causes** 1. Workspace recovery (test race). `POST /issues/:id/retry` returns 202 and then starts `heartbeat.resumeTaskRetry` without waiting. The proof-change tests change state after the receipt. When the background nudge runs first, it admits the Retry and queues a run before the change. The new queued run is then the latest run, so the second Retry in `sibling_hold` is stale. I proved this locally: a 1.5 s pause after the receipt made every gate in the family fail with a queued run in the issue history. 2. Control-plane checkpoint timeout (budget too tight). `checkpointTimeoutMs: 100` applies to every checkpoint in the session, including the normal checkpoints before the terminal. On a loaded runner, one of those took more than 100 ms. The test expects only the checkpoint that the pending observer blocks to time out. ## What Changed - `server/src/services/workspace-recovery.test.ts`: add `receiptOnlyRecoveryApp`. It builds the same route app, but the route nudge does nothing. The two proof-change families ("keeps ancestor Retry held…" and "holds a failed-preparation HTTP Retry…") use it and call `resumeTaskRetry` explicitly, as before. - `server/src/services/native-runtime/paperclip-control-plane-port.test.ts`: the checkpoint budget is 1 s. The timeout case now waits for the run to fail with the timeout while the observer is still blocked, instead of a fixed 700 ms sleep. This also proves that the timeout fires while the observer is pending. ## Verification - With the forced 1.5 s pause and the fix applied: both workspace-recovery families pass (80 passed). - `npx vitest run src/services/workspace-recovery.test.ts -t "keeps ancestor Retry held when current proof changes|holds a failed-preparation HTTP Retry when current proof changes"`: 80 passed. - `npx vitest run src/services/native-runtime/paperclip-control-plane-port.test.ts`: 94 passed, run 3 times. ## Risks - Low risk. The changes are only in tests. The route nudge is a latency hint ("Maintenance redelivers it"). Other tests still cover it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, and 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 - [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> |
||
|
|
ffd9591ab4 |
fix(recovery): preserve cancelled files after a reusable sandbox stops (#19305)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run writes its task files in a remote sandbox. The task
workspace records that run as its writer until the files are saved.
> - When a user stops or reassigns a run, the cancelled-workspace
preservation worker records the unsaved files as file recovery work.
> - A warm run (`keep_running`) that is not successful releases its
reusable sandbox through stop-and-retain. That release can finish before
the worker sees a terminal coordinator.
> - The worker accepted only an active lease or a released ephemeral
lease. It ignored the released reusable lease, so it never recorded the
file recovery.
> - The workspace kept the ended writer's hold. Each successor run
waited, then failed with "The workspace still has an unfinished writer
or save operation" after the one-hour cutoff.
> - This pull request accepts a released reusable lease that has a
confirmed stop-and-retain receipt as a preservation source.
> - The benefit is that the stopped run's files become recovery work,
and the task is not stranded behind a hold that nothing settles.
## Linked Issues or Issue Description
Refs #18820 (preserves files when Stop finds no live controller). Refs
#17677 (schedules cancelled workspace preservation). Refs #19285 (parks
a workspace wait behind an ended writer).
**What happened?**
A board user reassigned a task during a warm native run. Stop cancelled
the run with `issue_reassigned`, and the Stop acknowledgement was
recorded. The executor settled the coordinator to `terminal_failure`
with no owner. About eight seconds later, terminal release stopped and
retained the reusable Daytona sandbox. The lease became `released` with
`leasePolicy: reuse_by_environment`, a `stopped` termination receipt and
a stop-and-retain receipt. The task workspace stayed `dirty` with
`writerRunId` set to the cancelled run. No file recovery was recorded.
The new assignee's runs waited as `workspace_busy` for one hour. Then
each run failed `setup_failed`, and the task became blocked.
The heartbeat run row showed `nativePhase: "observed"`. That value is
stale: the checkpoint persist wrote it during teardown. The coordinator
itself was `terminal_failure`: the run's execution projection was
`recovery_needed` with no matching recovery action.
**Expected behavior**
The preservation worker records the stopped run's prepared files as file
recovery work. The lease keeps its release status and receipts. No
second stop and no new provider turn start.
**Steps to reproduce**
1. Cancel a native run with an acknowledged run-only Stop. Its remote
workspace reference is `prepared`, and the coordinator is
`terminal_failure` with no owner.
2. Release its reusable lease through stop-and-retain (`released`,
`reuse_by_environment`, confirmed stop receipt).
3. Run the cancelled-workspace preservation worker or the startup scan.
4. On master, the run has no `workspaceRestoreRecovery`, and no recovery
action exists.
## What Changed
-
`server/src/services/native-runtime/native-failed-workspace-recovery.ts`:
add one source check, `cancelledWorkspaceSource`, for cancelled
preservation. The worker check and the startup scan both use it. It
keeps the active and ephemeral cases. It adds a released reusable lease
only when the stop-and-retain receipt is confirmed
(`hasConfirmedSandboxStopAndRetain`) and `preparedRemoteWorkspaceSource`
proves the exact remote-workspace binding. This is the same proof the
restore recorder uses. Native-sync runs have no such proof and stay
ineligible. An ineligible source causes no run write or notification.
- The restore recorder (`preserveLegacyWorkspaceRestoreSources`) already
accepts this source. It checks the confirmed receipt and the exact
prepared workspace binding, and it keeps the lease status, policy and
receipts. This pull request does not change it.
- `native-cancelled-workspace-preservation.test.ts`: add the incident
shape as a regression for the worker and the startup scan, and add
safety cases.
- `doc/execution-semantics.md`: document the stopped reusable source and
the writer hold until recovery.
## Verification
From `server`:
```sh
npx vitest run src/services/native-runtime/native-cancelled-workspace-preservation.test.ts
```
- Red before the fix: the two new regression cases failed (`expected {
nativeCancellation: … } to match object { …, workspaceRestoreRecovery: …
}`). 30 other cases passed.
- Green after the fix: 36 passed.
- Review follow-up: the cases `invalid-receipt`, `expired-lease` and
`native-sync` failed on the previous head
|
||
|
|
dd889e6a0b |
feat: show the assignee's provider usage limit in task properties (#19270)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run on the native Runner through the Codex app-server and the Claude ACP harness > - Both harnesses report the provider account's rate-limit windows: Codex sends `account/rateLimits/updated`, and Claude sends its SDK `rate_limit_event` as `usage_update._meta["_claude/rateLimit"]` > - Paperclip dropped both signals, so an operator could not see that an agent was near or past its provider limit > - This pull request normalizes both signals into one account-scoped snapshot, keeps the latest snapshot per agent, and shows it as one "Usage limit" line in the task properties pane > - The benefit is that an operator sees "5h 78% · resets 15:40" (amber above 80%, "Limited until …" when the provider refuses work, "using credits" when a used-up window falls back to credits) next to the assignee, and can click it for every window ## Linked Issues or Issue Description Refs #19246 Refs #19259 **Is your feature request related to a problem?** Codex and Claude report provider rate-limit windows during a run. Paperclip dropped them: the Codex notification fell through the Rust normalizer with no branch, and ACPX dropped Claude's `_meta`. Operators could not see how close an agent was to its provider limit. **Describe the solution you'd like** Store the latest reported windows per agent and show one compact line in the task properties pane, with a popover that lists every window. **Describe alternatives you've considered** A full gauge in the agent list and the run header. A design review chose one property line with a details popover instead. ## What Changed - Runner (Rust, the runnerd path): new `provider_rate_limits.rs`. Codex `account/rateLimits/updated` becomes a `provider.notice.recorded` event (category `provider.rate_limits`, scope `account`). `CodexProviderState` merges sparse updates and emits a notice only when the windows change. ACPX `usage_update` with a sanitized `rateLimit` field also emits the notice. - Runner (sidecar): `acpx-runtime-sidecar.ts` forwards only `status`, `rateLimitType`, `utilization` and `resetsAt` from `_meta["_claude/rateLimit"]`. This needs the ACPX `_meta` passthrough from #19259. Until that merges, Claude emits nothing. - Runner (TypeScript in-process drivers): the same mapping in `codex-session-notifications.ts` and `codex-acpx-driver.ts` through `drivers/provider-rate-limits.ts`. - Shared: `provider-rate-limits.ts` holds the snapshot type, the parser, the merge, and `summarizeProviderRateLimits` (fullest current window, expiry by reset time, limited versus using credits). - Server: `agent-provider-rate-limits.ts` merges each notice into `agent_runtime_state.state_json.providerRateLimits`, scoped by company and agent. A session reset keeps the snapshot. No migration. - UI: `AgentRateLimitPropertyValue` adds a "Usage limit" row under the assignee in `IssueProperties`. Transcripts treat the notice as run-log only. A run status event refreshes the assignee's runtime state. - Storybook: `Tasks and conversations/Task creation and lifecycle/Usage limit property`. - Docs: `doc/run-log-events.md` describes the notice and its storage. ## Verification - `cd packages/paperclip-runner/runner && cargo test -p paperclip-runner-core --lib rate_limit` and `--test acpx_provider_events`: sparse Codex merge, dedupe, reached window, checkpoint round trip, Claude utilization and rejection, notice shape. - `cd packages/paperclip-runner && npx vitest run src/drivers/provider-rate-limits.test.ts src/drivers/codex` - `cd packages/shared && npx vitest run src/provider-rate-limits.test.ts`: summary, expiry, limited, using credits, parse and merge. - `cd server && npx vitest run src/__tests__/agent-provider-rate-limits.test.ts`: create and merge, out-of-order snapshot, company isolation, unrelated notices, session reset keeps the snapshot. - `cd ui && npx vitest run src/components/transcript src/components/issue-properties src/context/LiveUpdatesProvider` - `pnpm -r typecheck` and `pnpm check:token-gates` pass. - Live test on a local worktree instance: a native Codex agent on a ChatGPT subscription login ran a real task. The runner recorded one `provider.rate_limits` notice. The server stored it. The task properties pane showed "7d 100% · using credits" (this account's weekly window was used up and had credits, and the run succeeded). The transcript showed no extra row. - Not verified live: Claude. The local Claude CLI was not signed in, and the Claude path also needs #19259. The Rust and sidecar paths have tests. - Not run locally: the full `pnpm test:run` and `pnpm build`. CI runs them. ## Risks - Low risk. The snapshot is display data. It does not hold or start work. The existing provider-limit wait (#19246) still reads typed run failures. - Codex can send a rate-limit update often. The runner emits a notice only when the windows change, so run-log growth stays small. - With API-key auth, the harnesses report no windows, so no row appears. This is the intended result. - `CodexProviderState` gains two optional serde fields with defaults, so older checkpoints still load. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, browser) 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 - [ ] 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> |
||
|
|
31bf6e4482 |
Pin plugin Runner cleanup to its physical task (#19300)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Plugin Runner owners manage a physical task through an environment lease. > - The existing replacement path can assign a new physical task to that same lease. > - A delayed old owner can then stop the replacement or mark it stopped. > - Cleanup must keep the original physical task identity across RPC and database writes. > - This pull request pins that identity and keeps cleanup available to the new owner. ## Linked Issues or Issue Description Refs #19134 and #19157. This fix is based on master and does not enable repeated stopped-source recovery. A search found no duplicate fix. **What happened?** The maintained `createOwnedSandbox` replacement path changes `providerLeaseId` on one lease. An old task owner uses that mutable lease for a later Stop. A stopped-state response already in flight can also update the new task's flags. **Expected behavior** The old owner must address its original task. Its late cleanup writes must not alter the replacement. The replacement's own owner must still stop and complete it. **Steps to reproduce** 1. Create a plugin task owner for a stopped task. 2. Replace that task through the existing describe/startup path on the same lease. 3. Resume an old checkpoint or a held old status response. 4. Observe a Stop for the replacement or an incorrect stopped flag before this fix. ## What Changed - Require each task owner to keep the physical ID returned by its description. Reject a rebind to a different task. - Carry the ID through the existing internal task context after startup ends. Reject a changed physical task before RPC dispatch. - Match the physical task in stopped-state, completion and credential-handoff writes. - Hold the exact lease row through the retained-session database update and local checkpoint save. - Cover the maintained replacement path with real PostgreSQL tests. Preserve positive cleanup for the replacement owner. Document this boundary. ## Verification - Both replacement regressions failed on unmodified master `10a7d039c8`: delayed old-owner Stop targeted the replacement, and a held old status response marked it stopped. - Six focused files passed 160 tests: task authority, task runtime, warm tasks, execution target, credential writeback and task owner. - Four selected executor factory cases passed. They cover task-only providers and the physical task binding after the startup context ends. - Runner TypeScript, plugin SDK and adapter-utils builds passed. Server TypeScript check passed. Diff and added-line secret scans passed. - All 15 setup-probe cases passed with the actual Runner and fake provider binaries. The first local attempt lacked cargo on PATH. A sandboxed native attempt timed out in seven process cases; the unchanged run with loopback permission passed all 15. A separate initial DB integration attempt was denied a loopback listener; its permission-correct run passed. - Review found three missed constructors in the existing SDK/Core lifecycle fixture. They now use the actual describe result. All four cases in that real PostgreSQL and worker RPC file passed. - The first GitHub attempt failed in the unchanged GitHub recovery pagination case in `chat-channels.integration.test.ts`: its zero-request assertion observed a configuration GET. That exact case passed locally without code or assertion changes. The one failed-job-only retry passed; the initial failure log is retained. - [GitHub workflow 42082587576](https://github.com/paperclipai/paperclip/actions/runs/42082587576) passed on its second attempt. All CI checks completed, and Snyk passed. Standard Greptile scored exact head `fb914d98b99e` 5/5 with no unresolved threads. The branch is mergeable and ready for review. ## Risks The task context change is internal. It adds no plugin wire field, schema or Runner mode. A stale owner now gets an error and cannot clean up a replacement. A new owner can still clean up that task. The retained checkpoint write holds one existing lease row during a bounded local file write. Repeated stopped-source incarnation admission, runtime pins and deployments are outside this change. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), high reasoning. Used repository tools, TypeScript and real PostgreSQL tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used with version and capability details - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR using the bug template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change - [x] I have run the focused tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no unresolved findings - [x] I will address reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
8aa28940bf |
fix(sentry): keep a live-heartbeat native capacity retry out of Sentry (#19320)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends unexpected run failures to Sentry, and it keeps known, recovered failures local > - A native Codex "model at capacity" failure gets an automatic retry. That failure must stay out of Sentry while its retry owns recovery (#18295, #19051) > - The check reads the `nativeProviderFailure` diagnostic from the failed run's `resultJson`. On the live heartbeat path, the failed-status write replaces `resultJson` with the adapter's copy, and that copy has no diagnostic > - So every live-path capacity failure with a scheduled retry still goes to Sentry. Only the reconciliation path, which the tests cover, kept the failure local > - This pull request removes the diagnostic check. The scheduled retry run is the proof > - The benefit is that recovered capacity failures no longer create Sentry noise on the normal live path ## Linked Issues or Issue Description Refs #18295 Refs #19051 **What happened?** A native Codex run failed with `native_provider_overloaded`. The committed status decision scheduled the automatic capacity retry, and the retry run existed before the report. The run failure still went to Sentry. In a staging sample, every live-path capacity failure since #19051 was reported (1 of 1). **Expected behavior** When the `native_provider_overloaded` retry run exists for the same task, the failed run does not go to Sentry. The retry reports its own outcome. **Steps to reproduce** 1. Run a native Codex agent through the live heartbeat path (`finalizeNativeRun` without `projectRunStatus`). 2. Let the provider return `serverOverloaded`. The status decision schedules the capacity retry. 3. The heartbeat writes the terminal `failed` status with the adapter's `resultJson`, and `emitTerminalAgentTaskRun` calls `reportRunFailure` with that row. 4. The row has no `nativeProviderFailure`, so `isScheduledNativeCapacityRetry` returns false and the event goes to Sentry. **Relevant logs or output** The failed run row later shows `nativeProviderFailure: codex/server_overloaded`, because a later reconciliation write restores it. The row that the heartbeat write reports does not have the field. The same overwrite also explains the missing `finalizationReasonCode` that #19051 fixed. ## What Changed - `server/src/services/run-failure-report.ts`: `isScheduledNativeCapacityRetry` no longer requires `resultJson.nativeProviderFailure`. The run must still be native and failed, carry `native_provider_overloaded`, have a native task, have no signal, and not be in setup. The same-task successor with `scheduledRetryReason: native_provider_overloaded` is still required. - The successor is enough proof. Only the committed capacity status decision creates that successor, and only `forTerminal` in `native-provider-failure.ts` produces `native_provider_overloaded` together with the typed `codex`/`server_overloaded` diagnostic. - `run-failure-report.test.ts`: a new case keeps the failure local when the row has only the adapter's `resultJson` (no diagnostic, no finalization fields) and the successor exists. The "diagnostic is missing" case now reports only when no retry run exists. The case for a non-capacity diagnostic with a capacity successor is removed, because that state cannot occur. - `doc/observability.md`: the native capacity paragraph now says that the successor is the proof and that the run's own `resultJson` is not required. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/__tests__/run-failure-report.test.ts`: 284/284 pass with this change. - With the `master` version of `run-failure-report.ts`, the new case fails: 1 failed, 283 passed. So the test catches the bug. - `native-provider-failure.test.ts`: 57/57 pass. - `native-provider-overload.integration.test.ts` did not load in my sandbox, because `@paperclipai/plugin-sdk` was not built. CI must run it. ## Risks - Low risk. The change only removes one condition from a Sentry filter. - These failures still report: no retry run, exhausted or held retries, other retry lanes, a successor for a different task, legacy runs, signals, timeouts, setup failures, and unreadable retry evidence. - The change does not affect run status, retries, recovery, or the `all` reporting mode. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use (shell, repository reads, 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> |
||
|
|
8464a8c9ae |
fix(native): settle a bootstrap retry that lost its task lock (#19307)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run that fails before any provider work gets a bounded same-run bootstrap retry. Restart recovery decides whether that retry can run. > - The retry needs the task's issue lock. A user comment or another wake can move that lock to a new run before the retry is due. > - Restart recovery then returned `issue_execution_lock_changed` without a write. The same check ran on every recovery pass and never ended. The coordinator stayed `retryable_failure`. > - The orphaned-lease sweep waits for a terminal coordinator before it preserves the workspace. So the sandbox lease stayed active, and every later run for the task was cancelled as `workspace_busy`. > - This pull request gives a lock-lost bootstrap retry the same bounded admission that a superseded remote retry already has. > - The benefit is that the retry settles, the board gets a reconciliation action, and the sandbox lease is released instead of blocking the task. ## Linked Issues or Issue Description No public issue exists. Bug description: **What happened?** On a hosted instance, a native agent run failed at startup because the provider rejected `session.open` with a rate limit. Paperclip scheduled a same-run bootstrap retry. Seconds later, a new run for the task took its issue lock, most likely woken by a user comment. From then on, every restart recovery pass for the old run returned `blocked` / `issue_execution_lock_changed` and wrote nothing. The run recorded about 1,000 recovery events over 12 hours. Its coordinator stayed `retryable_failure` with `recoveryMode: bootstrap_retry`. The scoped orphaned-lease sweep found a pending workspace export for the lease. It could not preserve the workspace, because `preserveNativeFailedWorkspace` requires `terminal_failure`. So the lease stayed `active`, and 16 later runs for the task were cancelled as `workspace_busy`. Three more runs on the same instance were stuck in the same state. A board Stop on the failed run moved the coordinator to `terminal_failure`, and the sweep then released the lease within about a minute. **Expected behavior** A bootstrap retry that cannot get its task lock back settles within a bounded number of recovery passes. The coordinator reaches `terminal_failure`, the board gets a reconciliation action, and the existing orphaned-lease sweep can preserve the workspace and release the sandbox lease. **Steps to reproduce** 1. Fail a native remote run before provider work, so its coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry`. 2. Move the task's `executionRunId` to a newer run for the same task, or clear it. 3. Run `claimNativeRestartRecoveries` for the failed run. 4. Before this change, it returns `blocked` with no `retryAt`, and the coordinator stays `retryable_failure` on every pass. The new tests in `native-remote-retry-admission.test.ts` reproduce this. **Paperclip version or commit** Observed on a hosted deployment. Reproduced in tests on `master` at `10a7d039c8e`. Related: #19190 (a `session.open` model rejection no longer schedules a doomed bootstrap retry), #19131 (preserve superseded observed workspaces) and #19281 (release the leases of a cancelled legacy run). None of them changes this admission path. ## What Changed - **Restart recovery (`server/src/services/native-runtime/native-restart-recovery.ts`)**: a new condition, `lockLostBootstrapRetry`, matches a failed run whose coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry` and whose issue lock is not on the run. - A newer run for the task: the existing superseded check now covers this case too. It defers with `native_retry_source_superseded` as permanent, so admission is exhausted at once. - No newer run (the lock was released): recovery defers with `issue_execution_lock_changed` through the existing `deferRemoteRetry`. That path persists a deadline, backs off and exhausts after the system recovery budget. - When admission is exhausted, the existing hold path writes `terminal_failure` / `native_restart_recovery_blocked`, keeps the provider evidence and creates the board reconciliation action. This PR adds no new terminal path. - A bootstrap retry that still holds its own lock is unchanged. The exact-checkpoint remote retry path is unchanged. - **Docs (`doc/execution-semantics.md`)**: the bootstrap retry section now states the lock-lost rule. ## Verification - New test `settles a %s bootstrap retry that lost its task lock instead of re-checking forever` in `server/src/services/native-runtime/native-remote-retry-admission.test.ts`, for the `superseded` and `released` cases. The fixture is a genuine pre-provider failure: a `session.open` rejection with no session checkpoint, no runner checkpoint row and no provider evidence. - On `master` without the fix, both cases fail: recovery returns `blocked` without a deadline, and the coordinator stays `retryable_failure`. - With the fix, `superseded` settles on the first pass, and `released` settles after `SYSTEM_RECOVERY_MAX_RETRIES` deferred passes. Each deferred pass persists its `nextAttemptAt`. Both end in `terminal_failure` with one board recovery action, and later passes open no transaction. - `npx vitest run src/services/native-runtime/native-remote-retry-admission.test.ts` (from `server/`): 92 passed. - Related suites, each run on its own with exit code 0: `native-ingress-bootstrap-retry`, `native-startup-retry`, `native-scheduled-resume-lock`, `native-restart-recovery-isolation`, `native-runner-restart-recovery`, `native-runner-restart-preparation`, `native-runner-restart-diskless`, `native-warm-transition-restart`, `native-controller-recovery`, `native-failed-workspace-recovery`, `native-session-resumption`, `native-startup-cleanup-recovery`, `heartbeat-observed-workspace-preservation`, `heartbeat/recovery`, `runtime-selection`, `remote-workspace-migration` and `native-session-executor` (1,149 tests). Two other suites each had one local failure that this change does not cause: - `heartbeat-process-recovery`: "redacts opaque environment-bound credentials from Sentry diagnostics" fails the same way on this machine without the change. - `chat-channels.integration`: "settles a shared-endpoint accepted-response backlog without retrying its own locks" failed once under full-suite load. It passes alone, with and without the change. - `pnpm run typecheck` in `server/`: passed. ## Risks - Low to medium. The change only affects failed bootstrap retries whose lock is not on the run. Before this change, such a retry could never run, so no working path is removed. - A released lock that comes back to the same run during the backoff still resumes normally, because the condition reads the current lock on each pass. - When the retry settles, the task gets a board reconciliation action. Before this change, the task was silently stuck behind `workspace_busy`. - No migration and no API change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, code search, test runs), extended thinking, and a read-only investigation subagent for the root cause. ## 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> |
||
|
|
74b11014c6 |
fix(server): stop a queued answer from blocking saved-work recovery (#19318)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run can accept its result before the workspace copy-back finishes. If a server restart interrupts the copy-back, the run keeps an owner receipt for the server that stopped. > - Another server takes over only after the cloud platform proves that the old deployment stopped. After that proof, recovery checks that the task has not moved on, so old files never overwrite newer work. > - That check counted any queued run on the task as newer work. > - The user's answer to the run's question queues the assignee's next wake. That wake cannot claim while the saved run holds the task's execution lock, so it waits for the copy-back. > - Recovery waited for the wake and the wake waited for recovery. The task stayed stuck, and nothing was logged. > - This pull request ignores a later wake for the same assignee that has never started, and logs waits that the worker stops retrying. > - The benefit is that answering a question during an interrupted copy-back no longer strands the task. Every other newer-work check still applies. ## Linked Issues or Issue Description Refs #15824 (added platform-verified takeover and these binding checks) and #19111 (worker concurrency and ordering for the same copy-backs). This pull request keeps the rest of #15824's fence unchanged. **What happened?** On a hosted instance, a server restart interrupted a copy-back after the worker had written its owner receipt. The run had asked the user a question, and the user answered. The answer's wake was queued. Every later server got a valid stop proof for the old deployment and then got `binding_changed` from the shared binding check: the only failing condition was the queued answer wake. The worker treats `binding_changed` as a final wait and returns without logging. The run stayed `running` and the wake stayed `queued` for about 19 hours, across many restarts. **Expected behavior** A wake that is waiting for the saved run does not stop the saved run's recovery. Runs that could change the workspace still prevent takeover, and the publication check still stops a copy-back if such a run appears. **Steps to reproduce** 1. Accept a native result and leave a copy-back owner receipt from a stopped deployment. 2. Queue the assignee's next wake on the same task, for example by answering the run's question. 3. Let the reconciler or the workspace worker revisit the saved result with a verified stop proof. On `master` the takeover returns `binding_changed` and nothing is copied or committed. The new database tests run these steps against PostgreSQL. **Paperclip version or commit** `master` at `03eb49053fe`. **Deployment mode** Cloud-managed instances with platform stop verification. The same binding check also guards operator owner recovery, so self-hosted operator recovery had the same deadlock. ## What Changed - `server/src/services/native-runtime/native-workspace-recovery-binding.ts`: the newer-run check ignores a run when all of these hold: - It is `queued` and has never started (`startedAt` is null). - It belongs to the run's agent, and that agent is the task's assignee. - It was created after the saved run. - Its wake reason is not `source_scoped_recovery_action`. The run claim blocks such a run while the saved run holds the issue's execution lock (`lockIssueExecutionClaim` in `heartbeat/queue.ts`). Runs that have started, other agents' runs, recovery-action wakes and older queued runs still prevent takeover. - `server/src/services/native-runtime/native-workspace-finalization-work.ts`: log `runId` and `reason` when the worker stops retrying a wait (`binding_changed`, `owner_unverified`). It does not log `owner_active`. - `doc/native-workspace-finalization-recovery.md`: describe the waiting-wake rule in the shared binding section. - `server/src/services/native-runtime/native-workspace-queued-wake-recovery.test.ts` (new): the queued-wake tests, with a small fixture of their own. They are in a separate file so `native-workspace-finalizer.test.ts` stays under 1,000 lines. ## Verification - Failing first. These tests in `native-workspace-queued-wake-recovery.test.ts` fail against the original check and pass with this change: - Operator owner recovery with an answer wake queued. - Automatic takeover with the wake still queued. - Automatic takeover where the wake starts during copy-back. On `master` takeover is refused before this point; with the change, publication is stopped. - The real reconciler, from `retryable_failure`, with a stopped-deployment owner and a queued answer wake. It now copies once, commits the saved result and leaves the wake queued with its claim no longer blocked. - The fence is kept: three new cases (a requeued wake that already started, a recovery-action wake, another agent's queued run) still get `binding_changed`. They pass both before and after the change. - Changed test: "does not reopen an old result after a newer task execution exists" now inserts a started run instead of a never-started queued run of the same assignee. The queued case is now the new operator-recovery test above. - `cd server && npx vitest run src/services/native-runtime/native-workspace-queued-wake-recovery.test.ts src/services/native-runtime/native-workspace-finalizer.test.ts`: 2 files, 72 passed. With the original binding file restored, the 4 positive queued-wake tests failed and the 3 fenced cases passed. - Related suites (finalization work, platform proof, export retry, finalizer failure, recovery actions, owner-recovery routes, status reassessment, scoped deadline work): 8 files, 499 passed. - After rebasing onto `03eb49053fe`: finalizer and finalization-work suites, 97 passed. - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Not verified on a live instance yet. The diagnosis comes from that instance's run, finalization, workspace-operation and recovery-action rows, plus a direct stop-proof request that returned a valid receipt. ## Risks - A copy-back can now finish while the assignee's wake is queued. The same-process retry path already does this, because it does not run the binding check. The takeover path now matches it for wakes that cannot start yet. - If some path starts such a wake despite the execution lock, `assertHeld` sees a running newer run and stops before publication. The window between that check and the write is the same as for the other binding conditions. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use (shell, file edits, test runs, read-only database and log queries). The runtime does not expose the context window or reasoning configuration. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
7644045bc5 |
fix(server): remove a settle deadlock and three server test races (#19314)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server Vitest shards gate every pull request > - Three open pull requests (#19289, #19267, #19266) failed server tests in code that they do not change > - Each failure had a different cause. One cause is a real product deadlock, and three causes are test races > - Random CI failures slow every contributor and hide real regressions > - This pull request removes each root cause. It does not add retries or skips > - The benefit is fewer false CI failures and one less deadlock in external-agent settlement ## Linked Issues or Issue Description Refs #19289, #19267, #19266 **What happened?** Server tests failed at random on pull requests that did not touch them: - #19289, job 127412331924: `dot-runner.test.ts` > "signed readiness, native tools and finalization through Dot local Rust" failed with `PostgresError: deadlock detected` on the run-event append (`select ... from heartbeat_runs ... for update`). - #19267, job 127427384499: `tool-access-service.test.ts` > "keeps an organization audience fail-closed when empty replacement races with member archive" received a widened grant instead of the 409 error. - #19267, job 127410158948: `heartbeat-process-recovery.test.ts` > "does not reap active adapter executions started by another heartbeat service instance" failed with "Timed out waiting for adapter execution to start". - #19266, job 127404184242: `voice-session-store.integration.test.ts` > "schedules phone reply retries at the saved rate-limit deadline without early database scans" failed with a duplicate key on `companies_issue_prefix_idx`. **Root causes** 1. Dot deadlock (product bug). `settle` in `native-broker.ts` takes the external admission guard. Then it updates the same assignment row in two or more statements. PostgreSQL re-runs the foreign-key checks when a transaction updates a row version that it wrote earlier, so the second UPDATE takes `FOR KEY SHARE` on the heartbeat run. Execution admission locks the issue and the run `FOR UPDATE` before it takes the guard. Thus settle holds the guard and waits for the run, and admission holds the run and waits for the guard. The run-event append can queue behind them as a third participant, and the database can kill it. A local reproduction with `log_lock_waits` showed the RI check: `SELECT 1 FROM ONLY heartbeat_runs x WHERE id = $1 FOR KEY SHARE OF x`, during `update dot_runner_assignments set status='settled'`. 2. Tool-access race (test bug). The empty replacement and the member cleanup both waited for the first replacement to commit. Both waiters woke at the same time and raced for the membership row. A replacement that wins that race widens access before the cleanup, and the product code documents that outcome as valid ("Whichever transaction wins is then authoritative"). The test only expected the cleanup to win. 3. Heartbeat timeout (test bug). The run start includes real runtime skill preparation for a new company. I measured 3.3 s to 5.5 s locally before the adapter starts. The test gave this step a fixed 3 s budget. 4. Voice prefix collision (test bug). Each of about 110 fixture companies used `V` plus seven random hex digits as its unique issue prefix, so two companies could get the same prefix. **Master:** These tests do not fail on recent master runs (the master workflows do not run the server shards). The failures are timing-dependent and are not specific to a branch. ## What Changed - `server/src/modules/external-agents/native-broker.ts`: `settle` writes the assignment in one UPDATE. This applies to both Dot and Muse. The update sets the same columns under the same conditions as before. - `server/src/__tests__/dot-runner.test.ts`: added "settles accept/finish under the admission guard without waiting on a run that admission holds". The test holds the run `FOR UPDATE` like admission does, then runs settle, then takes the guard. Before the fix, settle waits and PostgreSQL reports a deadlock. After the fix, settle completes. - `server/src/__tests__/tool-access-service.test.ts`: start the empty replacement after the cleanup holds the membership row. Then wait until the replacement is blocked by the cleanup. The test still proves that the replacement re-checks the member status after the cleanup and fails closed. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: wait for the adapter to start. Fail at once if the run settles before the adapter starts. The test timeout is now 30 s, because the start (3 s to 6 s) and the existing 5 s settle wait came close to the 15 s default. - `server/src/__tests__/voice-session-store.integration.test.ts`: each fixture prefix includes a per-run sequence, so prefixes in one run are always distinct and stay at 10 characters. ## Verification - Red/green for the deadlock: with the `native-broker.ts` change removed, both new tests fail with `deadlock detected`. With the change, both pass. - `npx vitest run src/__tests__/dot-runner.test.ts` (server): 57 passed. - Before the fix, I ran the Dot Rust test 8 times with `deadlock_timeout=50ms` and got the deadlock in about 1 of 8 runs. Two further runs of 16 iterations each passed. - `npx vitest run src/__tests__/tool-access-service.test.ts -t "fail-closed when empty replacement races"`: 3 passed, run 4 times. - `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "does not reap active adapter executions"`: this failed locally before the change (start took 3.3 s to 5.5 s). It passes after the change. - `npx vitest run src/__tests__/voice-session-store.integration.test.ts`: 132 passed. - `npx vitest run src/__tests__/muse-control.test.ts`: 256 passed. This file covers the Muse settle path. - Not run locally: the full `pnpm -r typecheck` / `pnpm test:run` / `pnpm build`. CI runs them. A server `tsc --noEmit` shows no errors in the changed lines. The existing errors come from the unbuilt runner package in this worktree. ## Risks - Low risk. The settle change keeps the same column writes and conditions, now in one statement. It removes a lock wait. It does not add a lock. - The other changes are only in tests. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, and 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 - [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> |
||
|
|
55389554c2 |
fix(runner): paginate workspace discovery before journal persistence (#19311)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents discover reusable workspaces through `list_workspaces`. > - The tool returned every authorized workspace in one result. > - A large inventory exceeded the durable command journal limit. The result stayed pending, and session cleanup blocked the next turn. > - This pull request limits discovery in SQL and returns explicit continuation metadata. > - Agents can inspect every authorized workspace without sending the full inventory in one command. ## Linked Issues or Issue Description Related: #15749 introduced the task workspace tools. No duplicate pagination fix was found. **What happened?** A native workspace discovery call returned a multi-megabyte inventory. Persisting its result failed with `command_payload_too_large`. The preceding answer could still be saved, but cleanup retained the unsettled tool call. A later turn failed before provider work with `native_session_cleanup_quarantined`. **Expected behavior** Workspace discovery must return a manageable page. Agents must be able to request the remaining authorized rows. Existing visibility checks and journal integrity checks must stay effective. **Steps to reproduce** 1. Create 2,000 task-selectable workspaces with ordinary repository paths. 2. Call `list_workspaces` from an admitted native run. 3. On the old code, the real API regression returns 869,781 bytes. The command journal allows about 510 KiB. 4. On the repaired code, follow `nextCursor` to inspect all rows. Pages contain at most 100 rows and fit within 128 KiB. **Paperclip version or commit** Reproduced on source commit `7351a9d74f` before the repair. **Deployment mode** Isolated source checkout with real PostgreSQL and authenticated native agent APIs. Hosted deployment and incident recovery remain separate. ## What Changed - Validate workspace list limits and UUID cursors with a JSON Schema pattern supported by strict validators. Return `workspaces`, `limit`, `returnedCount`, `hasMore`, and `nextCursor`. - Apply company, visibility, lifecycle, and cursor conditions before the SQL limit. Order pages by workspace ID. Fetch one extra row to detect continuation without a count query. - Fit encoded pages within 128 KiB, including JSON escaping and Unicode. If one summary cannot fit, return explicit string previews with `truncatedFields`. The authorized detail API retains its full record. - Keep existing unpaged summary callers on their array response and recent-use order. - Update the native tool descriptor, generated contracts, catalog fixture, and execution documentation. - Add regression coverage for large inventories, complete pagination, stable pages, private and foreign rows, invalid inputs, and legacy summary behavior. Add an opt-in two-turn live Codex acceptance check. ## Verification - Red before repair: `task-workspace-api.test.ts -t "pages a large authorized workspace inventory"` failed because the result was 869,781 bytes instead of below 128 KiB. The transport regression also failed at 2,284,671 bytes. - Green after repair and review changes: 120 tests passed across the workspace API, transport, routes, native authority, coordinator, historical-question, and Codex vertical-slice suites. They cover complete discovery, byte-sized pages, oversized individual summaries, full detail retrieval, private/foreign/lifecycle exclusions, and strict schema compatibility. - Journal safety: two focused durable control-plane tests passed. Oversized result rejection and lossless valid result delivery remain effective. - Tool catalog and reconciliation: 30 tests passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed again on final commit `bc10880da6`. Server typecheck also passed after the review changes. The original affected suites passed on the rebased first commit (31 tests), and the expanded review regression set passed afterward (120 tests). - Real provider acceptance passed again on final commit `bc10880da6`: local macOS arm64, rebuilt native Runner, `gpt-6.1-sol`, high effort, per-turn lifecycle, and 2,000 persisted workspaces. Both turns succeeded. Persisted call inputs and results prove that the second page uses the first result’s cursor, both calls succeed, their IDs do not overlap, and the encoded results stay within 128 KiB. The second turn kept the provider conversation and file. Both task locks were clear after completion, and no recovered execution failure was recorded. Duration: 166.92 seconds. - Full local `pnpm test:run` stopped in the chat shard: 330 tests passed and one Slack registration test failed with `socket hang up`. That exact test passed in isolation without changes, and the final-head CI chat shards passed. A broader local managed-Slack follow-up produced no result after about eight minutes and was interrupted; it is not counted as a pass. The original full-suite failure remains recorded. - The first CI head failed native tool-schema compilation because strict validators did not recognize `format: uuid`. The revised descriptor uses a UUID pattern. The previously failing coordinator, native vertical-slice, authority, and history suites now pass locally. The stronger live cursor-receipt acceptance check passed. Final-head CI completed on `bc10880da6`: 57 checks passed and five optional checks were skipped, with no failures or pending checks. Greptile reviewed that commit at 5/5; both prior findings are resolved. - Dependency cost: paged discovery runs the existing visibility-scoped summary query with an ID cursor and at most 101 returned rows. It adds no count query, filesystem scan, provider call, or background worker. ## Risks - Native `list_workspaces` now returns a page object instead of an array. Individual summaries can be explicit previews when a single record exceeds the byte budget. Its advertised descriptor and generated contracts change together. Agents must follow `nextCursor` until null for a complete inventory. - Each page rechecks current visibility. Concurrent insertions, deletions, or access changes require a fresh scan when a complete point-in-time inventory is needed. - No migration or journal-limit change. An existing cleanup quarantine still requires normal recovery after deployment; this PR does not bypass it. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, shell tools, code editing, and test execution. The exact served model revision and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
c56aa689ac |
fix(server): say where secret-sensitive requests fail in Sentry (#19040)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The server sends unexpected 5xx errors to Sentry through the shared
Express error handler
> - Some routes carry credentials: chat setup, provider webhooks, MCP,
company observability settings, Muse, avatars, and similar routes
> - On these routes the handler replaces the error with a generic
`Secret-sensitive request failed` error, because the original name and
message can contain secrets
> - That replacement also removes every useful fact. The Sentry event
does not show the route, the status, the error type, or the throw site,
so a maintainer cannot find the cause
> - This pull request adds a small fixed-shape record to that generic
error. The record says where the failure came from, and it never copies
request data or error text
> - The benefit is that the next event on these routes points to a route
and a code location, while the privacy rule stays the same
## Linked Issues or Issue Description
**What happened?**
A hosted staging server reported one event with the message
`Secret-sensitive request failed`. The event had only the stack of the
sanitizer itself. It had no route, no status, no error type, and no
frame from the original error. The trace had no spans and no logs. A
maintainer cannot find the root cause from this event.
**Expected behavior**
The event keeps the generic message, so no secret leaves the process.
The event also tells the maintainer which secret-sensitive route family
failed, the HTTP method and status, the error type, a safe error code
when one exists, the matched route definition, and the original throw
site.
**Steps to reproduce**
1. Make a route handler throw on a secret-sensitive path, for example
`PUT /api/companies/:companyId/observability`.
2. Enable the backend Sentry DSN.
3. Look at the captured event. It shows only `Secret-sensitive request
failed` and the sanitizer stack.
**Paperclip version or commit**
`master` at
|
||
|
|
a695398052 |
fix(server): preserve inactive rebase bookmarks in workspace snapshots (#19310)
Preserve a validated standalone REBASE_HEAD and its commit without changing the source workspace. Keep active Git operations, unmerged indexes, invalid markers and concurrent changes blocked. Verified 156 affected tests, workspace typecheck and build. All hosted CI checks passed, and Greptile scored the exact head 5/5 with no unresolved threads. The bounded full local suite stopped during its fifth chat shard; this incomplete result is recorded in the PR. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
5de779848b |
fix(recovery): save a reassignment-stopped writer's files from its stopped reusable sandbox (#19304)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A remote task workspace stays held by its last writer run until that writer's files are saved > - A reassignment stops the old owner's run. The stopped writer still holds the workspace with unsaved files. > - When the run used a reusable (warm) sandbox, the stop leaves its lease `released` under the reusable policy, with a stop-and-retain receipt > - Cancelled-workspace preservation only admitted `active` or `ephemeral` sources. It never recorded the save for this stopped writer, so no workspace recovery ran, and the hold stayed forever > - The new owner waited, then failed setup after the one-hour cutoff with "The workspace still has an unfinished writer or save operation" > - This pull request admits the stopped reusable source, so Paperclip saves the stopped writer's files and releases the hold. A waiter keeps waiting while that save is in progress. > - The benefit is that a reassigned task continues with the old owner's saved work, and the new owner does not fail setup ## Linked Issues or Issue Description Refs #19285 (parks a waiter behind an ended writer). Refs #19074, #18820. **What happened?** A board user reassigned a task during a native run on a reusable sandbox. The old run was cancelled with `issue_reassigned`. Its sandbox was stopped and its lease was released under `reuse_by_environment`. The workspace still named the stopped run as its dirty writer. No save operation and no workspace recovery action were ever recorded for that run. The new owner's runs were cancelled `workspace_busy` repeatedly. After more than an hour, a new run failed setup: "The workspace still has an unfinished writer or save operation." **Expected behavior** Paperclip saves the stopped writer's files from its stopped sandbox within a bounded time and releases the hold. The new owner waits for that save and then continues. If the save cannot finish, its retries end with a named board recovery action. **Steps to reproduce** 1. Give a native run a prepared remote workspace on a lease with policy `reuse_by_environment`. 2. Stop the run (for example by reassignment), so the lease is stopped and released with a stop-and-retain receipt. 3. Run cancelled-workspace preservation. Before this fix, it records nothing, and the writer hold stays. ## What Changed - `native-failed-workspace-recovery.ts`: Cancelled-workspace preservation (the delivery worker and the startup scan) now admits a `released` reusable lease with a stopped termination receipt and a stop-and-retain receipt. - `preserveNativeFailedWorkspace` retains such a reusable source only with the exact confirmed stop-and-retain receipt, and only while the workspace is still bound to that writer (`preparedRemoteWorkspaceSource`). Otherwise it writes nothing. - `remote-workspace-writer-hold.ts`: For an ended writer, the holder cutoff now uses the latest progress of an in-progress save of that writer's retained files (a workspace recovery that is not complete, exhausted, or recovered). Reservation and the parked `workspace_busy` wait both read this rule. A waiter waits for the save instead of failing setup. - `doc/execution-semantics.md`: Documents the stopped reusable source and the save-progress cutoff. ## Verification - Red before the fix, green after: - `npx vitest run src/services/native-runtime/native-cancelled-workspace-preservation.test.ts` — new cases "preserves a reassignment-stopped writer's files from its stopped reusable sandbox" (worker and startup scan) failed before the fix: no `workspaceRestoreRecovery` was recorded. - `npx vitest run src/__tests__/heartbeat-remote-workspace-busy.test.ts` — new cases "keeps a waiter parked while an ended writer's save is in progress" failed before the fix: the parked retry was released past the cutoff. - Safety cases: - A reusable source without a confirmed stop, a workspace now owned by another writer, and an already saved workspace are not retained, and the run row does not change. (The writer and saved cases fail if the new guard is removed.) - Preservation does not replay the stopped run, does not create a wake, keeps the stop receipt and lease policy unchanged, and keeps the writer hold until the save publishes. - An exhausted or complete save does not extend the cutoff. The existing past-cutoff test (no save in progress) still fails setup and keeps the held files. - Related suites: workspace recovery, remote workspace reservation, native workspace reconstruction, native failed-workspace recovery, legacy workspace restore recovery. - Not verified on staging: this change is not deployed. The staging task needs this fix deployed. Then the preservation worker records the save, workspace recovery saves the files, and the waiting owner continues. ## Risks - More cancelled runs are now admitted for preservation. The retention guard requires the exact stop-and-retain receipt and the workspace bound to this writer, so a reusable sandbox that another run resumed or that no longer holds unsaved work is not retained. - A stopped reusable source stays `reuse_by_environment` while Paperclip saves it. This matches the existing retention path for reusable native sources. - The writer-hold read adds one indexed query (`issue_recovery_actions_source_run_idx`) for an ended writer only. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, test 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 - [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> |
||
|
|
5d2fb8f737 |
fix(server): split run-failure Sentry groups that have no usable root code (#19308)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends each unexpected terminal run failure to Sentry. A custom fingerprint groups these events. > - The fingerprint is `[errorCode, agentAdapter, failurePhase?, rootErrorCode?]`. The root code is used only when it is an identifier. > - Many setup failures have no identifier root code. Some have a numeric JSON-RPC code (`-32002`). Some have only an HTTP status (`409`). Some have no code. > - All of these failures get the same fingerprint, `setup_failed / paperclip_runner / setup`. One Sentry group now holds about ten unrelated failure types: a controller lease loss, Daytona create timeouts, sandbox resume timeouts, remote-workspace 409 errors, S3 throttling and more. The group title changes with each new event. A fix for one cause cannot close the group. > - This pull request adds a bounded fallback when there is no usable root code. The fallback is the root exception class name plus its HTTP status or numeric code. > - The benefit is that each failure type gets its own Sentry group, so a fix can be verified and resolved for each cause. ## Linked Issues or Issue Description Refs #16359 Refs #16802 Refs #19156 **What happened?** Unrelated `paperclip_runner` setup failures go into one Sentry issue. The root exceptions include `LegacyControllerLeaseLostError`, `JsonRpcCallError` with code `-32002` (Daytona create timeout), `JsonRpcCallError` with code `-32003` (sandbox resume timeout), plain `Error` with status `409` (remote workspace writer still pending), and AWS `SlowDown`. None of them has an identifier root code, so the fingerprint drops the root element. **Expected behavior** Failures with different root exceptions go into different Sentry issues. Failures with the same root exception and status still group together. The fingerprint does not contain message text. **Steps to reproduce** 1. Report two setup failures for the `paperclip_runner` adapter with error code `setup_failed`. 2. Give the first failure the root exception `LegacyControllerLeaseLostError` with no code. 3. Give the second failure the root exception `JsonRpcCallError` with code `-32002`. 4. Read the two fingerprints. Before this change, they are the same. **Additional context** Only events without an identifier root code get a new fingerprint. Events with an identifier root code (for example `ECONNRESET`) keep their current fingerprint and their current Sentry group. ## What Changed - `server/src/sentry.ts`: when there is no identifier root code, `captureRunFailure` adds fallback fingerprint elements from the deepest recorded exception: - `exception:<Name>` when the name is an identifier. - `status:<n>` when the exception has an HTTP status (already validated as 100–599 by the diagnostics collector), or else `code:<n>` when the code is a number of up to 10 digits. - Names or codes with other shapes are not used, so message text and secrets cannot go into the fingerprint. - The `root_error_code` tag does not change. - `server/src/__tests__/run-failure-sentry.test.ts`: new tests for the fallback groups, for an identifier root code that keeps the current fingerprint, and for rejected unbounded names and codes. ## Verification - `cd server && npx vitest run src/__tests__/run-failure-sentry.test.ts src/__tests__/run-failure-sentry-real-sdk.test.ts src/__tests__/sentry-links.test.ts src/services/__tests__/run-failure-report.test.ts`: 358 tests pass. The real-SDK suite ran against `@sentry/node@10.71.0`. - I reverted only `server/src/sentry.ts` and ran the new grouping test. It failed, because all six fingerprints were the same. With the change, it passes. - `cd server && npx tsc --noEmit -p tsconfig.json`: no errors. - `src/__tests__/sentry.test.ts` did not load in my local environment (`AI_InvalidArgumentError: baseURL must be a non-empty string`). It fails in the same way on unchanged `master`, so I depend on CI for that suite. ## Risks - Events without an identifier root code get a new fingerprint. Sentry puts them in new issues. The old mixed issues stop getting these events. This is the intended result. Alerts or saved searches that point to an old mixed issue must move to the new issues. - The number of groups increases by a small, bounded amount. Exception class names, HTTP statuses and numeric codes have low cardinality. No message, ID or free text goes into the fingerprint. - No change to what the server sends in tags or contexts, and no change to run control flow. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs, GitHub and Sentry connectors). ## 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> |
||
|
|
c4171af889 |
fix(cli): skip POSIX key file mode check in doctor on Windows (#14308)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - `paperclipai doctor` checks the local install, including the local encrypted secrets key file > - The check reads the POSIX permission bits of the key file and warns when group or others can read it > - On Windows, `fs.statSync().mode` is always `0o666` for a writable file, and `chmod` cannot restrict it > - So on Windows the check always warns, and the suggested `chmod 600` has no effect > - The server secret provider health check has the same check and the same false positive > - This pull request skips the POSIX permission-bit check on `win32` in both places > - The benefit is that `doctor` and the secret provider health endpoints on Windows no longer show a permanent false-positive warning ## Linked Issues or Issue Description Fixes #14255 ## What Changed - `cli/src/checks/secrets-check.ts`: skip the key file permission-bit check when `process.platform === "win32"`. Linux and macOS behavior does not change. - `cli/src/__tests__/secrets.test.ts`: add two tests. One test mocks `process.platform` as `win32` and expects `pass` for a `0o666` key file. One test pins the POSIX behavior: a `0o644` key file still gives `warn` (skipped on Windows runners). - `server/src/secrets/local-encrypted-provider.ts`: the same `win32` skip in the local encrypted provider health check. This check feeds `GET /api/companies/{id}/secret-providers/health`, `POST /api/secret-provider-configs/{id}/health`, and the `validateConfig` warnings. - `server/src/__tests__/secret-provider-registry.test.ts`: add a `win32` test that expects status `ok` with no warnings for a `0o666` key file. Mark the existing `0o644` warning test `skipIf(win32)`. ## Verification - `cd cli && npx vitest run src/__tests__/secrets.test.ts`: 10 of 10 tests pass on macOS (Node 22.22.3). - Red before the fix: with the fix reverted, 1 failed and 9 passed. The new Windows test fails with `expected 'warn' to be 'pass'`. - `cd server && npx vitest run src/__tests__/secret-provider-registry.test.ts`: 3 of 3 tests pass. Red before the fix: the new `win32` test failed 2 times with status `warn`. - Full CLI suite (`cd cli && npx vitest run`): the same 28 tests in `doctor`, `install-command`, `update-command`, and `worktree` fail on `master` without this change and with it. My local checkout has no workspace builds, because I installed with `--ignore-scripts`. This change adds 2 passing tests and no new failures. - `cd cli && npx tsc --noEmit`: 233 errors on `master` and 233 with this change. All errors come from unbuilt workspace packages. No error is in `cli/src`. - I did not run the check on a real Windows machine. The Windows test mocks `process.platform`. ## Risks - Low risk. The change only removes a warning on Windows. On Windows the old check always fired, so it gave no signal. - The repo already skips the same kind of check on `win32`: `server/src/services/decision-signing.ts` returns early in `assertOwnedByCurrentUser` and `enforceKeyFilePermissions` for its key file. - On Windows, `doctor` no longer checks key file access at all. NTFS ACLs control access there, and the old check could not read them. An ACL check is out of scope for this fix. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), in Claude Code with tool use (shell, file edit). It reproduced the bug with a failing test, wrote the fix, and drafted this description. A human reviewed the change. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes (no documentation describes this warning) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
088f85c5fe |
fix: rename the managed environment to Paperclip Sandbox (#19296)
## Thinking Path > - Paperclip manages AI agents and their execution environments. > - Managed installations supply a default sandbox through managed configuration. > - Most selectors show the configured environment name. > - A fallback label, two errors, and examples still use Paperclip Computer. > - This pull request changes that stock copy to Paperclip Sandbox. > - Existing and custom environment names continue to come from the stored row. ## Linked Issues or Issue Description **What existing behavior does this improve?** The managed environment label in agent settings and the errors shown when the managed sandbox is unavailable. **Subsystem affected** The agent configuration UI, runner selection, and environment reconciliation tests. **Current behavior** Fallback copy and current examples call the managed sandbox Paperclip Computer. The actual environment name is already configurable. **Proposed behavior** Use Paperclip Sandbox for stock copy and current examples. Keep configured names unchanged in selectors, including legacy names and custom names. **Reason and benefit** The name identifies this environment as a managed sandbox and removes ambiguity. **Breaking changes** The displayed fallback and error text change. Error codes, execution behavior, and environment identity remain the same. No migration is needed. **Additional context** No duplicate rename issue or pull request was found. This is a copy update, with no new roadmap feature. ## What Changed - Update the managed fallback label and two unavailable-sandbox error messages. - Update current test and Storybook fixtures without changing story IDs or behavior. - Test configured names, fallback copy, and the unavailable-sandbox message. - Add a PostgreSQL regression test for a rename that retains the environment ID, instance default, agent binding, environment variables, and lease. ## Verification - `pnpm -r typecheck` passed. - `pnpm build` passed. - Focused coverage: 956 cases across nine changed suites. The new fixture initially tried to insert an existing singleton; its corrected upsert passed with all 52 environment-service cases. The updated runner suite also passed all 243 cases. - The two UI suites passed all 189 cases, including configured legacy/custom names and the missing-default fallback. - `pnpm check:token-gates` passed. - `node scripts/storybook-catalog.mjs --check` passed. The catalog is structural evidence only. - `node --test scripts/check-maps.test.mjs scripts/storybook-catalog.test.mjs scripts/storybook-feature-index.test.mjs` passed. - Full `pnpm test:run` is in progress. No live provider or browser journey is claimed by these tests. - CI passed on the current head after one server-shard rerun. The first attempt hit a 10 ms deadline assertion in the unchanged status-card timing test; its isolated local run and the complete rerun shard passed. The original failure and rerun are retained in the PR comment. - Reviewed the diff for secrets, personal data, and run artifacts. ## Risks Low risk: runtime edits change three display strings. Managed reconciliation is unchanged. The regression test exercises the existing rename against PostgreSQL. The configured name still controls normal display, so this code change alone does not rename existing database rows. A managed configuration update uses the existing reconciliation path. Roll back the copy by reverting this commit. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) through Codex in T3 Code, xhigh reasoning, with repository tools 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, version, and capabilities - [x] I have checked ROADMAP.md; this copy update does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs - [x] I have described the issue in the PR with the enhancement template fields - [x] No internal or instance-local issue references - [x] The branch name describes the change and contains no internal ticket ID - [x] Focused tests pass locally - [x] Relevant tests were added or updated - [x] Relevant copy and examples were updated; no API or schema documentation changed - [x] Risks are documented - [x] All CI gates passed on the current head, including the server 11/12 rerun and aggregate verify - [ ] Full local test suite completed (still running; six chat shards passed) - [x] Greptile is 5/5 on the current head with no unresolved findings - [x] I will address all reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
7351a9d74f |
fix(native): restore retired backups after warm lease handoffs (#19299)
## Thinking Path > - Paperclip manages AI agents and keeps their work across turns. > - A native session can keep the same sandbox through several warm lease handoffs. > - Each handoff keeps the old lease row and records matching receipts on both owners. > - The retired-backup guard required exactly one row for a provider allocation. > - That rule rejected valid history after Paperclip destroyed the sandbox. > - This change uses the existing handoff proof to select the final owner. > - A verified backup can then restore without discarding ownership checks. ## Linked Issues or Issue Description Refs #19208. **What happened?** A follow-up received a new sandbox after its old sandbox was destroyed. The old allocation had a normal warm lease handoff. Both lease rows remained in PostgreSQL. The guard rejected them with `runner_harness_state_mismatch: backup_without_reusable_lease`. Its later same-run retry failed with `native_remote_recovery_lease_mismatch`. **Expected behavior** Restore the verified backup when one final owner has a valid destroyed receipt and every earlier owner has a matching handoff receipt. Keep uncertain ownership blocked. **Steps to reproduce** 1. Complete a native warm turn. 2. Hand the reusable sandbox to another turn. 3. Complete that turn and save its backup. 4. Close the workspace and destroy the sandbox. 5. Send a follow-up. The new allocation must restore the saved session. **Paperclip version or commit** Reproduced in the PostgreSQL regression on the parent of this change. The affected deployed build also contains #19208. ## What Changed - Select the single final lease owner. Validate all predecessor rows with `retiredReusableLeasePredecessors`. - Keep the destroyed receipt, exact backup stamp, and newer-provider-work checks. - Add database regressions for one and three handoffs. Reject missing or changed receipts, an active predecessor, missing history, an unrelated owner, a stopped source, another backup, and later provider work. - Document the valid lease-history shape. ## Verification - Red before the source change: the two valid-handoff cases returned false. The other 16 database cases passed. - Green after the change: all 18 database cases passed. They also passed on current head `d16e098b8c2e6c0dda70984c1df21e3dc792ab34`. - Command: `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-retired-backup-source.integration.test.ts`. - The database, executor, retry-admission, and lease suites passed: 4 files, 1,308 tests. - `pnpm -r typecheck` and `pnpm build` passed. The build stamp names head `4ee733c3a979436196522f36537ff182e9df5398`. The full local suite stopped in chat shard 3 after a 15-second timeout in `materializes direct external-chat finals once and accepts the next turn without agent bookkeeping writes`. Earlier shards passed 702 cases; that shard passed 303 other cases. The isolated test passes on this branch and unchanged master `10a7d039c8efeac0fd655c24970df5ae3658414b`. All current-head CI chat shards also passed. The earlier full-suite attempt remains failed; its timeout cause is not established. No test or timeout was changed. CI is waiting for its last Runner job. - Current head `d16e098b8c2e6c0dda70984c1df21e3dc792ab34` removes four comment lines from the previously tested head. Its 18 database cases passed. Greptile reviewed this head at 5/5 with no actionable findings: https://github.com/paperclipai/paperclip/pull/19299#issuecomment-6665446098. CI remains pending. - Live inspection found the matching predecessor/successor receipts and the final owner's destroyed receipt. This was read-only evidence from the affected build. - A live provider run at the fixed revision has not been exercised. Local Daytona credentials and an immutable test image are unavailable. No deployment or original-task retry is claimed. - Initial wider tests could not resolve the unbuilt plugin SDK. The SDK build is now available. A sandboxed database launch could not bind loopback; the same regression passed with the disposable database outside that restriction. ## Risks - A wrong owner could allow an old backup to rewind a session. The shared validator must account for every lease row and match both sides of each handoff. - Live, unrelated, incomplete, or conflicting history stays blocked. The existing backup integrity and provider-progress checks remain. - No migration, permission, model, trust, timeout, or retry-policy change is included. - The original task still needs the fixed build and normal recovery. Hosted acceptance remains unverified. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, code execution, and tool use. The session does not expose the exact model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
cb0b7525e0 |
feat(task-chat): warn at the top of a run when an MCP server failed to start (#19289)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Each agent run shows its activity in the task chat. The Paperclip runner turns harness events into that activity. > - Codex reports `mcpServer/startupStatus/updated` when an MCP server fails to start. The runner dropped this event at the notification allowlist and in the Rust normalizer. > - When an MCP server fails, the agent runs without its tools. The agent then says "I could not reach the server", and the user cannot see why. > - This pull request maps a failed MCP start to a user-facing provider notice and shows it as one warning line at the top of the run. > - The benefit is that the cause of a "cannot reach my tools" answer is visible next to that answer. ## Linked Issues or Issue Description Refs #19252 (the plan card, which set the pattern for promoting a dropped harness signal to a dedicated task-chat surface). **Problem description** Codex sends `mcpServer/startupStatus/updated` (`name`, `status`, `error`, `failureReason`) during `thread/start`. The TypeScript allowlist (`codex-thread-normalization.ts`) and the Rust normalizer (`runner-core/src/provider_events.rs`) both dropped it. A run with a broken MCP server showed no sign of the failure. The agent only said that it could not reach the tools. **Proposed solution** Map `status: "failed"` to `provider.notice.recorded` with `category: "mcp_server_startup_failed"`, `severity: "warning"`, `scope: "session"` and `userActionable: true`. Show these notices as one run-health line under the runner header: "github MCP server failed to start: No such file or directory (os error 2)". **Alternatives considered** An inline notice row in the timeline. It scrolls away and sits among tool rows. A run-level line next to the header is easier to connect to the agent's answer. ## What Changed - Runner (Rust, production Codex path): `normalize_codex_notification` maps a failed `mcpServer/startupStatus/updated` to a `provider.notice.recorded` notice. `starting`, `ready` and `cancelled` stay out of the transcript. The new helper removes the Codex wrapper text ("MCP client for \`x\` failed to start: MCP startup failed:") so that the line shows the real reason. `reauthenticationRequired` becomes "needs re-authentication". - Runner (TypeScript): the same mapping in `canonicalProviderEventsFromCodex`, and the method is added to `isSupportedCodexNotificationMethod`. - Server: the `view=conversation` run-event query keeps `mcp_server_startup_failed` notices. A settled run can then still show the warning. Other notices stay out of that view. - UI: new `TaskChatRunHealthBanner`. `partitionRunHealthItems` takes MCP failures out of the run's items, with one line per server where the latest report wins. `TaskChatRunnerTurn` (live runs) and `TaskChatTurn` (settled, historical and merged turns) show it under the header. `coalesceSettledTurns` merges the failures when it merges turns. - Storybook: `Tasks and conversations/…/Task chat/Run health` shows one failure, several failures, a finished run and a healthy run, all through the production runner turn. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/provider-events.test.ts src/drivers/codex src/protocol`: 508 tests pass. - `cargo test --release -p paperclip-runner-core --lib -- provider_events`: 20 tests pass. - `pnpm --filter @paperclipai/ui exec vitest run src/components/task-chat src/components/transcript src/components/TaskChatThread.test.tsx`: 1340 tests pass, including the new `TaskChatRunHealthBanner.test.tsx` (persisted event to rendered banner, deduplication, settled historical turn, merged turns). - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/task-cold-open.test.ts`: the conversation view keeps the MCP notice and still excludes other notices. - `pnpm check:token-gates` is clean. `node scripts/typecheck-low-memory.mjs` passes. - Live test on a local instance with Codex 0.160: a Codex agent with a preconfigured Codex home whose `config.toml` has `[mcp_servers.github] command = "github-mcp-server-not-installed"`. Codex reported the failure during `thread/start`. The persisted event had the summary "github MCP server failed to start: No such file or directory (os error 2)". The banner showed while the run was live. It also showed on the settled turn after "More run output", above the agent's answer that it could not reach the GitHub MCP server. ## Risks - Low risk. The change adds one notice category. It does not change how existing notices map or render. - A settled run whose transcript is deferred on a cold open (a successful run with a posted agent comment) shows the warning only after "More run output". This is the same as all other run activity under the current compact history. - Claude does not report MCP startup failures over ACP. `claude-agent-acp` reads MCP status only for OAuth. This change is therefore Codex-only. - Provisional task titles from provider session names are not part of this change. Codex app-server does not generate thread names (only its TUI does). Claude generates its title after the turn ends, and ACPX drops the title text. > 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`), through Claude Code with tool use (shell, file edits, browser automation for Storybook and the live test). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
99c68455e6 |
fix(workspaces): omit an untracked Python virtual environment from workspace snapshots (#19274)
`python -m venv` links its interpreter by absolute path. The portable workspace archive rejects absolute symlinks, so a workspace with an untracked .venv could never be saved and the task blocked with native_workspace_sync_out_retry_exhausted. Treat an untracked directory with a regular PEP 405 pyvenv.cfg at its root like an untracked node_modules: record it as excluded and do not archive it. Tracked environments are still archived; other absolute links are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
10a7d039c8 |
fix(dispatch): keep a workspace wait parked while an ended writer's save settles (#19285)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run that finds its task workspace held by another run waits as a `workspace_busy` scheduled retry. #19143 keeps that wait as one parked retry while the holder is live. > - A remote task workspace stays held by its last writer run until the writer's save and sandbox release settle. This hold continues after the writer run ends, for example when a reassignment stops the writer. > - The parked-wait check counted only a `running` holder. For an ended writer whose save had not settled, it released the waiter at each due check. Preparation deferred again, cancelled the run, and created a new retry run. > - After a mid-run reassignment, the new owner got five cancelled `workspace_busy` runs in eleven minutes, and the task did no work. > - This pull request makes the parked wait use the same writer-hold rule as workspace reservation, so the waiter stays parked until the hold releases. > - The benefit is one durable wait, one start when the hold releases, and no churn of cancelled runs. ## Linked Issues or Issue Description Refs #19143 (parks a workspace-busy wait). Refs #19184 (releases a recovered hold for a new assignee after reassignment). **What happened?** A board user reassigned a task in the middle of a run. The old run was cancelled with `issue_reassigned`. The new assignee's runs were each created, started and cancelled with `workspace_busy`: "Shared execution workspace is busy: run … ended, but its workspace save has not settled". This repeated every 1–2 minutes (five cancelled runs in 11 minutes). Every cancellation named the stopped run of the previous assignee as the holder. The new assignee's other tasks were not the holder. **Expected behavior** The reassigned task waits as one parked retry. It starts once when the stopped writer's save and sandbox release settle. The wait stops at the one-hour holder cutoff. **Steps to reproduce** 1. Give a task a remote execution workspace whose writer run has ended, but whose save has not settled (`writerRunId` set, `dirty: true`). 2. Wake the task's assignee. The run defers with `workspace_busy` and schedules a retry. 3. Make the retry due and promote scheduled retries. Before this fix, the retry is promoted and dispatched, defers again, and creates another run. ## What Changed - New `server/src/services/remote-workspace-writer-hold.ts`: `readRemoteWorkspaceWriterHold` holds the rule for who holds a remote workspace (a running writer, or an ended writer whose save or sandbox release has not settled). The code is moved from `reserveRemoteWorkspace` without a behavior change. - `reserveRemoteWorkspace` uses that function. It throws the same errors as before. - `parkWorkspaceBusyRetryInTx` (from #19143): when the recorded holder is not running, it reads the same hold for the waited execution workspace. If the recorded holder is an ended writer that still holds it, the retry stays parked. The cutoff clock starts at the writer's end, the same value that reservation measures. Earlier waiters keep their arrival order. - New `WORKSPACE_BUSY_SETTLING_RECHECK_MS` (60 s). Settling has no single completion point that can wake the waiter, so a wait behind a settling writer re-checks every minute. Each check creates no run. In the promotion transaction it reads the workspace, the writer run, its task, and the writer's leases (one row per sandbox lease of that run). The checks last only while the wait exists, up to the one-hour cutoff. - `doc/execution-semantics.md`: documents the parked wait behind a settling writer. ## Verification - Red before the fix: `cd server && npx vitest run src/__tests__/heartbeat-remote-workspace-busy.test.ts`. The two new tests failed: the due retry was promoted (`expected [ … ] to not include '<retry id>'`). - Green after the fix, with: - `parks one waiter while an ended writer's save settles, then admits it once the hold releases`: three due checks keep one parked run, each re-check is bounded by the writer's end plus one hour, there is one `workspace_busy` cancellation in total and no new retry run. After the hold releases, the next check promotes the same run. - `stops parking behind an ended writer once its unsettled save reaches the holder cutoff`: parked before the cutoff, promoted after it (preparation then fails with the existing unfinished-writer error). - Existing suites pass: `heartbeat-workspace-busy.test.ts` (live-holder parking, arrival order, stop, workspace move), `modules/run-dispatch`, `remote-workspace-reservation.test.ts`, `remote-workspaces.integration.test.ts`, `isolated-remote-workspace-source.test.ts`, `heartbeat/workspace-preparation.test.ts` (476 tests in all). - `tsc --noEmit` for `server` shows no errors in the changed files. - Not done: a live run on staging. The fix is not deployed. ## Risks - Low. Parking applies only to a retry that recorded a `workspace_busy` wait on an execution workspace whose recorded writer is that holder. If the hold settles, moves to another run, or the task changes workspace, the retry is released to normal dispatch as before. - The wait stays bounded by the existing one-hour holder cutoff from the writer's end. After that, preparation fails as before. A user can still stop the parked run. - One writer per workspace is unchanged. Reservation still decides admission with the same rule. - The one-minute re-check runs only while a wait exists. It is not idle polling. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser reads of staging diagnostics). ## 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> |
||
|
|
b05d3db2b3 |
fix(recovery): continue a pre-turn interrupt after recovery, not after the destroy (#19292)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can interrupt a native run with a comment. The next turn should start soon after the run stops > - When the stop leaves sandbox files unsaved, recovery saves them, re-stops the sandbox, and then queues the destroy for an ephemeral lease > - #19224 kept the verified stop on that lease as `workspaceRecoveryCleanup.stopReceipt`, and taught the continuation gates to accept it > - One gate was missed: `acknowledgedNativeStopExecutionHasStopped`. An interrupt that lands before the provider turn opens leaves no gating hold, so the saved comment goes through this gate, not through the explicit continuation gate > - That gate still wanted the raw `remoteExecutionTermination` receipt, so the comment waited about 5 minutes for the destroy retry > - This pull request lets that gate accept the carried-forward stop receipt too > - The benefit is that an interrupt during startup continues right after recovery, as #19224 intended for every lease policy ## Linked Issues or Issue Description Refs #19224 Refs #19206 **What happened?** On hosted staging, an `interrupt: true` comment reached a native run of an ephemeral-lease agent while the run was still in startup (the provider pack upload). The run stopped before any provider turn. Recovery saved its files at once. The follow-up turn started about 6 minutes after the interrupt. The stopped run's lease was released by `pending_cleanup_retry`, and the follow-up started 2 seconds later. Agents whose interrupt landed after the provider turn opened continued in 30 to 50 seconds on the same build. **Expected behavior** The saved comment starts a new turn right after recovery saves the files and the sandbox is stopped. The destroy of a stopped sandbox is cleanup and must not hold the next turn. **Steps to reproduce** 1. Use an agent whose sandbox lease is ephemeral (destroy after each turn). 2. Post an `interrupt: true` comment while its native run is still starting, before the provider turn opens. 3. Let recovery save the files. See the follow-up start only after the destroy retry releases the lease. **Paperclip version or commit** Hosted staging at `e37ab23d` (includes #19206 and #19224). ## What Changed - `server/src/services/acknowledged-native-stop.ts`: `acknowledgedNativeStopExecutionHasStopped` accepts a remote lease that has `hasRemoteTerminationReceipt` or `hasRecoveredCleanupStopReceipt`. Before, it accepted only the first one. - `server/src/services/workspace-recovery.test.ts`: new regression for a pre-turn interrupt on an ephemeral lease. Recovery completes, the destroy is queued, and the saved comment is handed to `resumeRemoteStopComments` twice. With a verified carried stop, exactly one turn starts while the lease is still `pending_cleanup`. Without the stop receipt, the comment stays deferred and no turn starts. - `doc/execution-semantics.md`: one sentence that extends the #19224 paragraph to the pre-turn Stop case. ## Verification - Red before the fix: `cd server && npx vitest run src/services/workspace-recovery.test.ts -t "starts the interrupt redirect once after recovery"` failed the `verified-stop` case with `expected [] to have a length of 1 but got +0`. The `no-stop-receipt` case passed. - Green after the fix: the same command passes both cases. - `npx vitest run src/services/workspace-recovery.test.ts`: 382 passed. - Related suites pass: `acknowledged-native-stop`, `explicit-native-continuation`, `heartbeat/run-control`, `heartbeat/recovery`, `heartbeat/native-stop-evidence-read`, `native-session-executor`, `heartbeat-observed-workspace-preservation`, `native-startup-cleanup-recovery`, `execution-control-work`. Two `run-control` cases timed out at 30 s under parallel load and passed when run alone. - `pnpm --filter @paperclipai/server typecheck` passes. - Not done: a live re-run on staging. This change is not deployed. ## Risks - Low risk. `hasRecoveredCleanupStopReceipt` is the same check #19224 added to the other gates. It is true only for a lease whose recovery cleanup belongs to its own run, which was released, and whose carried stop receipt matches the company, run, lease, provider, and provider lease id. A lease without that receipt still blocks. - The no-replay hold is unchanged. The successor still goes through ordinary admission with all task gates. The test calls the resume twice and sees one turn only. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, browser reads of staging). Extended reasoning was used. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
b400f76647 |
feat(ui): submit task composers with Enter by default (#19272)
Use Enter to submit task replies and new tasks, with Shift+Enter for a new line. Add an experimental preference to require Command/Ctrl+Enter. Preserve autocomplete selection and IME confirmation, and retain the modifier shortcut until saved settings load. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
3fca8a1500 |
test(evals): renew the reporting lock after iMessage update (#19283)
## Thinking Path > - Paperclip verifies the selected source and reporting dependencies before it obtains model credentials. > - The reporting helper admits only an exact reviewed lock or its exact reviewed correction. > - Master updated the iMessage dependency and protobuf after the last reporting-policy review. > - The controller lock therefore stopped matching either admitted hash. > - A real five-case launch failed at this guard before any case execution. > - This PR renews the exact hashes and keeps the existing patch and rejection mechanism unchanged. > - A normal-CI repository check now exercises the real helper against the committed lock before paid execution. ## Linked Issues or Issue Description Refs: #19261, #18914. **What happened?** The five selected execution jobs in the retained launch 3002 stopped during “Verify source-bound build and authenticated selection before credentials.” The first observed error was `Reporting lock changed; review the dependency correction before install` in the trusted controller helper. Model-key and original case execution steps were skipped. The failed attempt remains infrastructure evidence; this PR makes no native product-grade claim. **Expected behavior** Admit the exact reviewed current controller dependency graph. Keep rejecting unknown locks, changed correction patches, and wrong output bytes before credentials or provider execution. **Steps to reproduce** Run the reporting helper against the committed current-master lock. The parent policy recognizes 754… and 60f…, while the actual controller lock is 6f252b…. The parent stops at the same pre-credential boundary as AWS. The existing correction patch reverses and reapplies cleanly to that lock. ## What Changed - Renew only the reporting policy input and output SHA256 values. The output is the exact committed current-master lock after `@photon-ai/advanced-imessage` 2.1.0→2.2.0 and `@bufbuild/protobuf` 2.15.0→2.16.0. - Derive input `ea91060774e3289d5d07bc58f72d8953f5a6dcb9db98aa9a1adf202476bf89ac` by reversing the existing correction patch from the current lock. Reapplying that byte-identical patch yields output `6f252b0c8099ed837a297f97f1fc5e14e96bc7530b0239065d0789b33b2ee477` exactly. - Keep patch SHA256 `2adced9a66052ba5752140ec6cc6670f679552a9a8559bd1faea10036afeeb97`, helper code, and guard behavior unchanged. Update the policy basis to describe the review truthfully. - Add one normal server-suite check. It copies the committed lock and reviewed policy into a temporary directory and runs the real helper. The prior focused suite already covered this contract, but its runner-e2e location is outside normal root Vitest projects. `server/vitest.config.ts` already includes `scripts/**/*.test.mjs`; no workflow change is needed. No `pnpm-lock.yaml`, workflow, source pin, descriptor, oracle, model, permission, credential, scanner, or case bound changes. Immutable source 6773 and the failed launch remain unchanged. ## Verification - Parent existing reporting suite: 4 fail and 3 pass because the committed lock is outside the admitted pair. Renewed policy: all 7 pass, including unknown-lock, changed-patch, wrong-output, and refresh-path rejection controls. - New normal-CI repository control fails with the exact paid-step reporting-lock error on the parent policy. The corrected combined run passes 8 controls across 2 suites. - Exact patch reverse/forward roundtrip preserves the full current lock bytes. The actual CLI returns `already-reviewed`; the working lock hash stays 6f252b… and is not modified. - Focused command: `node node_modules/vitest/vitest.mjs run --config ../report-lock-imessage-vitest.config.mts`. The external local config selects only the two affected suites with the existing 30-second test limit. - `git diff --check` passes. No SDK/native/source build, model call, evaluation retry, or private archive download was needed for local proof. Full workspace typecheck/build were not run for this policy-and-test change. Latest-head required CI and genuine review remain pending. ## Risks This policy deliberately rejects the next unreviewed lock change. The normal-CI check should expose that drift before paid fanout. The input is the current lock with the already approved correction reversed; it is not a claim that a new dependency graph was resolved or installed locally. Local guard controls do not prove a repaired AWS execution or any of the five product journeys. The next measured attempt still requires normal protected merge and fresh actual-controller admission. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, repository tools, and local code execution. The host did not expose a context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
629abc8509 |
fix(recovery): keep pending Retry unclickable and scope failure causes (#19268)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - #19260 shows the saved-work recovery step on a task, and records why
a recovery attempt failed.
> - The review of #19260 found three gaps after the PR merged.
> - A scheduled recovery offered Retry while a Retry preparation was
already pending.
> - Classified failures could get a misleading `provider_error` cause.
> - The new cause text in the board handoff had no test.
> - This pull request closes these gaps.
> - The benefit is a Retry button that matches what is allowed, and
causes that name the correct subsystem.
## Linked Issues or Issue Description
Refs #19260
**What happened?**
After #19260, a task with a scheduled recovery and a pending (`issued`
or `processing`) Retry preparation showed an active Retry button. The
server only coalesces the second request. Also, a failure that recovery
classified as `archive_unsafe`, `source_unavailable` or
`snapshot_verification_failed` still recorded a provider cause. The
handoff text could then read "last failure: archive_unsafe, cause:
provider_error".
**Expected behavior**
Retry is not offered while a Retry preparation is pending. Only the
generic transient fallback records a provider cause.
**Steps to reproduce**
1. Request a chat Retry on a task while its saved-work recovery is
scheduled.
2. The task projection returns `retry_scheduled` with `canRetry: true`.
**Paperclip version or commit**
master at
|
||
|
|
6293e581f6 |
fix(recovery): resume cancelled workspace retries from verified waits (#19276)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A shared workspace can make a run wait before it calls a provider. > - The scheduler saves a retry for that wait. > - The control plane can cancel the retry while it is still queued. > - Recovery then asks for a process stop receipt for a run that never claimed execution. > - This pull request verifies the retry and its exact workspace-wait parent before it accepts a later user instruction. > - The benefit is that Retry and saved messages can continue through normal admission without replaying old work. ## Linked Issues or Issue Description Refs #13640 and #15483. Related fixes: #18838 covers dispatcher cancellation codes; this change covers a system-cancelled scheduled workspace retry. Related open PR #15383 covers claimed conversation-adapter preparation. This change requires an unclaimed retry with no run profile or owner. **What happened?** A scheduled workspace retry was cancelled before runtime selection. The run retained an expected system cancellation and a `startupCancellation.beforeNativeSelection: false` marker. It had no start timestamp, execution stage, run profile, controller, process, lease or provider events. Recovery recorded an unknown-action hold. A later user message and Retry could not pass that hold. **Expected behavior** A later authorized instruction can start a fresh turn when the saved retry and its exact parent prove that no provider work started. Missing or conflicting evidence must retain the hold. **Steps to reproduce** 1. Record a terminal workspace wait before provider dispatch, with `executionRecovery.kind: workspace_wait` and `providerWorkStarted: false`. 2. Save a scheduled retry for that same company, task and agent. 3. Cancel the retry through the control plane before it claims execution. 4. Send a new user message or request Retry after the cancellation. 5. On the base commit, admission returns no continuation. With this change, normal admission creates one fresh successor. **Paperclip version or commit** Reproduced in source based on master at `499712c68b3ca95801b6114d5f2f0f0ea6759318`. **Deployment mode** Source checkout with isolated embedded PostgreSQL. Hosted rollout and a real provider run remain to be verified after merge. ## What Changed - Extend the existing cancelled-startup proof with one source-bound workspace-retry case. Require the canonical retry parent and reason to match the saved context. - Verify the parent's company, task, agent, time order, immutable Runner claim and typed workspace-wait record. Reject runtime, process, lease, coordinator, result and output evidence on either run. - Keep validation of the full saved result in the database predicate. The bounded execution projection can add empty receipt fields without changing eligibility. - Reuse the existing admission, Retry and deferred-message paths. Preserve cancelled history and unknown action outcomes. Keep permission, Stop, budget and recovery-limit gates. - Add 23 regression cases, including heartbeat message and Retry admission, concurrent saved-message recovery, and 19 conflicting-evidence cases. Document the contract. The cancelled-startup service owns this proof. No provider protocol, schema, UI or shared contract changes. Reads use exact run IDs and return only narrow metadata. The new reads occur only for a cancelled, unstarted Runner retry candidate. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/explicit-native-continuation.test.ts src/services/legacy-execution-recovery.test.ts src/services/execution-blocker-read.test.ts src/services/task-retry.test.ts` — 452 passed. - `pnpm -r typecheck` — passed. - `pnpm build` — passed. - `node scripts/check-module-boundaries.mjs` and `git diff --check` — passed. - Local diff review and secret/private-context scan — passed. Test identities are synthetic. - Regression history: the original positive case failed on unchanged source while 11 negative guards passed. Initial test fixtures then exposed ownership/contract constraints and a missing saved-wake envelope; the fixtures now use valid persisted shapes. The heartbeat Retry test exposed extra keys in the bounded evidence projection. Full-record validation now runs in SQL, and both assembled heartbeat paths pass. The unknown action outcome stays unchanged. - Platform: macOS arm64, Node 25.6.1, pnpm 9.15.4. Tests use real isolated PostgreSQL. They stop at admission and do not call a live provider. - Exact-head GitHub CI: 54 successful checks, 3 skipped, no unfinished or failed checks. Greptile scored `cfbf55cea68e3c69f65f50afcba98f775fa6fa3d` 5/5, with no actionable findings and no review threads. - The full repository `pnpm test:run` was not run locally. A previous full run in this repair effort stalled and was stopped after 2h45. The four complete affected suites pass; full PR CI must pass before merge. ## Risks - False admission could allow work before a predecessor is safe. The proof fails closed on missing parent records, different ownership, a changed retry binding, later parent completion, restore obligations, any execution resource, or any provider/output evidence. - This deliberately covers only system-cancelled, unclaimed workspace retries. Other cancellations retain their current reconciliation path. It does not grant automatic replay, certify unknown external writes, or reset exhausted recovery limits. - No migration. Verify the serving version and an existing saved-message continuation after deployment. ## Model Used - OpenAI `gpt-6.1-sol`, xhigh reasoning, through the Codex harness in T3 Code. Used code editing, shell tools and test execution. The harness does not expose its context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
053ae3920f |
build(deps): bump @photon-ai/advanced-imessage from 2.1.0 to 2.2.0 (#15167)
Bumps [@photon-ai/advanced-imessage](https://github.com/photon-hq/advanced-imessage-ts/tree/HEAD/packages/advanced-imessage) from 2.1.0 to 2.2.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/photon-hq/advanced-imessage-ts/releases">@photon-ai/advanced-imessage's releases</a>.</em></p> <blockquote> <h2>advanced-imessage v2.2.0</h2> <h2>Bug Fixes</h2> <ul> <li>Added the missing rate limit error codes <code>burstRateExceeded</code> and <code>newContactThrottled</code>, so server policy failures are now preserved correctly instead of falling through as less specific errors. ✨</li> <li>Added test coverage to make sure these policy errors map to <code>RateLimitError</code> and keep their original error code.</li> </ul> <hr /> <p><strong>Version:</strong> 2.2.0</p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/photon-hq/advanced-imessage-ts/commit/b6b94c15df5ae4751fcf60505b1c7266dafe9fe9"><code>b6b94c1</code></a> chore: bump version to 2.2.0</li> <li>See full diff in <a href="https://github.com/photon-hq/advanced-imessage-ts/commits/v2.2.0/packages/advanced-imessage">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) You can trigger a rebase of this PR by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> > **Note** > Automatic rebases have been disabled on this pull request as it has been open for over 30 days. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
34d308a331 |
fix(native): let a run retire the prior owner it reserved when its session rotates (#19263)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native Runner agent keeps a warm provider session per task. A follow-up run on the same task can reuse that session and its managed instruction directory. > - When a follow-up run cannot resume the saved session (for example, the effective run configuration changed), it first reserves the prior owner's instruction directory. Then it starts a new session identity and must retire the prior owner. > - Retirement treated the run's own directory reservation as another executor's busy turn. The run failed at startup with `native_session_supervisor_busy`. > - That failure happened after the remote workspace was prepared, so failure handling retained the sandbox and blocked the task. A board follow-up comment left the task `blocked` with no agent progress. > - This pull request lets a run retire the prior owner that it reserved itself, and keeps the directory that it adopted. Any other reservation or turn is still busy. > - The benefit is that a follow-up on a task with a rotated session runs once, and the task does not become blocked. ## Linked Issues or Issue Description **What happened?** A board follow-up comment with reopen woke an agent on a task that had just finished a successful run. The new run reused the task's own sandbox. The saved session could not resume because the effective run configuration changed, so the run started a new session identity. At startup (right after the native execution claim) the run failed with `native_session_supervisor_busy`. The run got the error code `native_continuation_requires_reconciliation`, the sandbox was retained for workspace repair, and the task became `blocked`. The agent had another live run on a different task at the same time. That run used a different sandbox and a different workspace, so it was not the cause. The busy owner was the task's own prior warm session. The new run had reserved that session's instruction directory itself (`reserveWarmNativeInstructionDirectory` sets `preparingRunId` to the new run). `retireSupersededWarmNativeSessions` then saw `preparingRunId` and rejected the run. **Expected behavior** The follow-up run retires the prior warm owner of its own task, keeps the instruction directory it adopted, starts the new session, and runs once. The task does not become blocked. **Steps to reproduce** 1. Run a native Runner (Codex app-server) task to completion with a warm session and a managed instruction directory. 2. Change the agent's effective run configuration, so the saved session cannot resume. 3. Wake the same task again (for example, a board comment with reopen). 4. Before this change, the run fails at startup with `native_session_supervisor_busy` and the task becomes blocked. **Paperclip version or commit** Observed on staging at `7fd05e13`. The code is unchanged on `master`. ## What Changed - `retireSupersededWarmNativeSessions` now receives the current run id. When the prior owner of the same task (same logical owner scope) is reserved by this run (`preparingRunId === runId`), retirement continues instead of throwing `native_session_supervisor_busy`. - That retirement closes the prior owner with `preserveInstructionsForRunId`, so the directory that this run adopted is not collected. This matches the existing configuration-change branch in warm reuse. - After a successful close, the reservation is cleared. A failed close keeps the reservation, so its `release()` retries retirement as before. - A reservation by any other run, a busy owner, and an executing scope still throw `native_session_supervisor_busy`. ## Verification - Red before the fix, green after (`server/src/services/native-runtime/native-session-executor.test.ts`, "managed directory warm checkpoints"): - "retires its own reserved directory owner when the session identity rotates": before, the run rejected with `native_session_supervisor_busy`. After, the prior owner closes once, the new session runs once, the adopted directory is not collected during retirement, and it is collected once when the new owner retires. - "retires its own reserved directory owner after the Runner claim when the session identity rotates" (the production `useRunnerd` path, where retirement happens after the claim): before, admission rejected with `native_session_supervisor_busy`. After, admission retires the prior owner and keeps the adopted directory. - Safety case (green before and after): "still rejects a session identity rotation over another run's directory reservation". The provider does not start and the other run's owner is not closed. - Existing safety case (green): "does not retire an active turn when a new session identity arrives". - `npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/heartbeat/instruction-working-copy`: 1163 passed. - Staging evidence was read only. This change is not deployed. ## Risks - Low risk. The change applies only when the prior owner has the same logical owner scope (company, agent, task, workspace, environment) and its reservation names the current run. That reservation exists only after this run validated reuse of the prior directory. - The claim-path instruction copy (`claimWarmNativeInstructionCopy`, which sets `busy`) is unchanged. That path is not used by Codex app-server agents. - Two related points are unchanged and are out of scope here: a startup failure after the workspace is prepared still retains the sandbox and blocks the task, and the "Instruction edits could not be recovered" warning on that failure is advisory only. With this fix, this flow no longer reaches that failure. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edits, browser read of staging run records) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Refs #18893 (shared-sandbox ingress retirement). Refs #19143 (workspace-busy wait). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
01dd6ab336 |
fix(recovery): show the saved-work recovery step and record why it failed (#19260)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Sometimes a failed run left files in a sandbox. Then Retry first
runs system recovery to save those files before the task continues.
> - Recovery has steps: confirm that the previous attempt stopped, save
its files, then finish.
> - The task page did not show these steps. People saw "Queued" or
"Trying again in…" with a retry time that kept moving.
> - Each unclassified failure of the stop-confirmation step was recorded
only as `provider_unavailable`, so a repeated deferral could not be
diagnosed.
> - This pull request shows the current recovery step on the task. It
also records why each failed attempt failed, in closed labels.
> - The benefit is that people can see what Retry waits for, and
operators can see why recovery deferred.
## Linked Issues or Issue Description
Refs #19249
**What happened?**
A person clicked Retry on a task whose failed run had a retained
sandbox. For about 7 minutes the task showed only "Queued", and the
retry time moved forward twice. Recovery deferred twice in the
stop-confirmation step. Each deferral was recorded only as
`provider_unavailable` with an opaque fingerprint, so the cause could
not be found.
**Expected behavior**
The task shows the recovery step that it waits for, such as "Checking
that the previous attempt stopped. Trying again in 30s…". Each failed
attempt keeps a closed-label cause, for example `sandbox_not_running` or
`transport_timeout`, with an optional errno-style code and HTTP status.
**Steps to reproduce**
1. Get a remote task run to fail after its sandbox was placed, so that
recovery owns saved work.
2. Click Retry while the sandbox provider does not respond to the stop
check.
3. The task shows "Queued" or "Trying again in…". The recovery evidence
records only `provider_unavailable`.
**Paperclip version or commit**
master at
|
||
|
|
cb5d7e440c |
fix(server): retry setup when the plugin worker was not running (#19249)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Remote agent runs call a sandbox provider plugin, such as Daytona,
through a plugin worker process.
> - Sometimes that worker is not running. It can be stopped, restarting
after a crash, or retired by the host.
> - Then the host refuses the call before it sends anything to the
worker.
> - Setup recorded this refusal as a generic `setup_failed`. It
scheduled no retry, so the task became stranded and needed a person.
> - This pull request types the refusal and gives it its own transient
setup code. The bounded bootstrap retry then runs.
> - The benefit is that the task continues on its own when the worker is
available again. Users do not see a blocked task.
## Linked Issues or Issue Description
Refs #19150 (removed one trigger for the worker retirement)
Refs #19245 (restarts a worker after a fail-closed retirement)
**What happened?**
A plugin worker stopped. Each run that then called `environmentExecute`
during setup failed with `Cannot call "environmentExecute" — worker for
"<plugin>" is stopped`. The run got `setup_failed`, no retry was
scheduled, and the task moved to board recovery. On one staging
instance, 15 runs failed this way in 12 minutes.
**Expected behavior**
No provider work started, so the failure is safe to retry. The run must
retry within the bootstrap budget, as it does for a reusable sandbox
resume failure or a preparation timeout.
**Steps to reproduce**
1. Stop the sandbox provider plugin worker, or let the host retire it.
2. Wake an agent whose run needs that provider during setup.
3. The run fails with `setup_failed`. No retry is scheduled.
**Paperclip version or commit**
master at
|
||
|
|
e37ab23dbc |
fix: hold an agent's new work while its provider usage limit is active (#19246)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Routine-driven agents run on the native Runner through ACP harnesses such as Claude > - When the agent's provider account runs out of allowance, every new wake for that agent fails the same way > - The ACP harness reported the exhausted allowance as a generic "terminal limit failure" with an `unknown` category, so Paperclip could not tell it from a context or turn limit > - Paperclip recorded a reset time only for the failing run's own startup retry; other wakes of the same agent kept dispatching, each spent about two minutes on workspace setup, and then failed > - This pull request classifies the limit, keeps its reset time, and holds the agent's new work until that reset > - The benefit is that a limited agent stops burning runs, shows a clear "waiting for usage limit reset" state with a time, and resumes its queued work after the reset ## Linked Issues or Issue Description Refs #13148 Refs #13379 **What happened?** A routine-driven Claude agent on the native Runner failed 29 times in about 15 hours. 20 runs failed with `native_provider_terminal_failed` and "ACP agent reported a terminal limit failure." The recorded provider failure had category `unknown`. 6 runs failed with `provider_session_open_rejected`. Their `session.open` result is `command_execution_failed` with classification `provider_rate_limit`, and the provider failure is HTTP 429 `rate_limit` ("would exceed your account's rate limit"). The startup retry of one such run recorded a cooldown until one hour later. A new wake for a different task of the same agent dispatched 16 minutes later, spent two minutes on workspace setup, and failed with the same 429. Separately, 4 runs failed with "Project authentication conflicts with this agent's managed AI connection". That message did not say which file or setting conflicted. The same message was also shown when the check could not finish. **What did you expect?** A provider usage limit is classified distinctly. Paperclip records the reset time when the provider reports one. New work for that agent waits for the reset with a visible reason. Queued work resumes after the reset, and no doomed run starts. A configuration conflict is reported only when one is found, and the message names the file to fix. **Steps to reproduce** 1. Use a native Runner agent with a Claude ACP harness whose account is rate limited or out of usage. 2. Let a routine or assignment wake the agent on several tasks. 3. See each run fail with a generic terminal limit failure or a `session.open` rejection, and see new runs keep starting. ## What Changed - Runner: The ACPX turn now receives the typed AIR session failure (`onTerminalSessionFailure`) through the runtime host and the ACPX port. The sidecar adds the provider's title to the provider failure message. Only a rate-limit or quota title becomes `rate_limit` or `quota_exhausted`. Context, turn and configured budget limits stay `unknown`. The failure details stay private. - Server: A native `turn.failed` with a `rate_limit` or `quota_exhausted` provider failure is now `native_provider_usage_limit`, the same code as the Codex usage-limit terminal. - Server: `agentProviderLimitCooldown` (extracted from the startup retry) also reads a reset clock in the provider's wording, such as "resets 2am (UTC)". The clock parser moved from the recovery service to `provider-quota-reset.ts` without changes. - Server: New `heartbeat/provider-limit-wait.ts`. Before any workspace or provider work, a queued run for an agent with an active provider limit is cancelled as `provider_limit_wait`. The run records the reset time, whether the provider reported it, the category and the run that reported the limit. Paperclip schedules one retry of the same wake at the reset through the existing AI connection wait lane. That lane does not consume the failure retry budget. Manual runs, recovering runs, conversation turns and pool-routed connections are not held. - Server: The execution projection shows "Waiting for usage limit reset" with the reset time and the next step. - Server: Only a found project authentication conflict is a configuration failure. Its message names the project file and the settings to remove. A check that cannot finish (timeout or another exit code) is now an ordinary failure, not a configuration conflict. - Docs: `doc/execution-semantics.md` describes the agent-level limit wait. ## Verification Red before the fix, green after: - `server/src/__tests__/heartbeat-provider-limit-wait.test.ts`: An automation wake after a 429 `session.open` rejection dispatched the adapter before the fix. Now the run is cancelled as `provider_limit_wait` and a scheduled retry exists at the reset. After the reset passes, the retry is promoted and the work succeeds. A manual run, a later success, expired evidence and an unclassified terminal still dispatch. - `native-startup-retry.integration.test.ts`: "reads the reset clock from an ACP terminal limit failure's provider wording" (it fell back to one hour before). - `native-session-executor.test.ts`: An ACP `turn.failed` with a typed `quota_exhausted` or `rate_limit` failure is a usage-limit terminal; `unknown` is not. - `packages/paperclip-runner/src/drivers/provider-failure.test.ts`: A typed AIR limit title is kept and classified; context, turn and budget limits are not; details do not leak. - `execution-projection.test.ts`, `ai-connections.test.ts`: The wait label, and conflict versus unfinished check. - Safety cases: `runtime-host.test.ts` and `codex-runtime-adapter.test.ts` forward the callback. The existing startup retry, provider capacity, recovery classification, legacy reconciliation, retry scheduling and execution configuration suites pass. Commands: - `cd server && npx vitest run src/__tests__/heartbeat-provider-limit-wait.test.ts src/__tests__/ai-connections.test.ts src/services/native-runtime/ src/services/recovery/ src/services/execution-projection.test.ts src/services/legacy-execution-recovery.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/__tests__/heartbeat-execution-configuration.test.ts` - `cd packages/paperclip-runner && npx vitest run src/drivers/provider-failure.test.ts src/drivers/acpx/runtime-host.test.ts src/drivers/acpx/codex-runtime-adapter.test.ts src/cli/acpx-runtime-sidecar.test.ts` - `tsc --noEmit` for server and paperclip-runner; the ACPX sidecar and provider-failure contract checks pass. I did not run a live provider limit on staging. The live evidence is the staging run records for the affected agent, which I read only. ## Risks - The limit evidence is per agent, not per account. If the board changes the agent's AI connection, automatic wakes can still wait until the old reset (at most one hour when no reset was reported, and at most one day). A manual run is not held, and one successful run clears the evidence. - A new wake that arrives during a limit is now cancelled and retried at the reset, instead of failing. The task stays in its status, and the run shows the reset time. - The Runner change takes effect when the provider pack is rebuilt and deployed. Until then, the server still holds new work using the existing `rate_limit` evidence from `session.open` rejections. - A turn terminal with a typed rate limit or quota is now `native_provider_usage_limit` instead of `native_provider_terminal_failed`. Both are terminal and board-owned. The recovery text now asks the board to restore provider usage capacity. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser reads of staging run records) 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> |
||
|
|
d0afeacdd5 |
fix(remote-workspaces): keep retained runner state when replacing a retired root (#19251)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote workspaces save a task's files as snapshots. Runtime state under `.paperclip-runtime` is never in a snapshot, because it belongs to the sandbox > - #19231 lets a parent task on a retained sandbox restore newer child work over its own retired workspace root > - That replace removed the whole retired root, including the parent's retained runner session state under `.paperclip-runtime/paperclip-runner/` > - The parent's native continuation then reads that runner state on the resumed sandbox, cannot find it, and fails with `runner_remote_state_unavailable` > - This pull request moves the retired root's runtime state into the restored root before it removes the retired root > - The benefit is that delegating parents on retained sandboxes continue their session with the children's saved work ## Linked Issues or Issue Description Refs #19231 **What happened?** After #19231, a parent task on an agent with a retained sandbox delegates two subtasks. Both subtasks finish. The parent's wake run now restores the newer saved work successfully, but the run fails right after with `runner_remote_state_unavailable: exit=1 timedOut=false`. The run error code is `native_continuation_requires_reconciliation` and the task becomes `blocked`. In one staging wave, 4 of 4 such parents failed. Root cause: the native continuation on a resumed sandbox reads `<remoteCwd>/.paperclip-runtime/paperclip-runner/sessions/<session>/runner/runner-state.json`. Snapshots exclude `.paperclip-runtime`, so the restored root has only the new restore receipt. #19231 parked the retired root and then removed it, so the runner state was removed too. **Expected behavior** The restored root has the newest saved files and the sandbox's retained runtime state. The parent's continuation reads its runner state and continues. **Steps to reproduce** 1. Prepare a remote workspace on sandbox A. Write files and runner state under `.paperclip-runtime/paperclip-runner/`. Save. 2. Make a second run the writer on sandbox B. It restores, writes new files, and saves. 3. Make a third run the writer on sandbox A again and prepare. The restore replaces the retired root. 4. Read the runner state with `readRemoteRunnerState`. Before this change, it fails with `runner_remote_state_unavailable: exit=1 timedOut=false`. **Paperclip version or commit** `master` at `26391707fb`, which includes #19231. ## What Changed - `server/src/services/remote-workspace-transport.ts`: before it removes the retired root, the restore script moves each entry of the retired `.paperclip-runtime` into the restored root's `.paperclip-runtime`. It does not move `workspace-ready.json`, `workspace-restored.json`, or `workspace-snapshots`, and it does not replace an entry that already exists. The move is a rename, so it keeps sockets and large session state. The script also finishes an interrupted carry when the same restore repeats. Only an authorized replace with that restore identity creates the `<root>.retired-<identity>` path. - `server/src/services/remote-workspaces.integration.test.ts`: the retained-sandbox regression now writes parent runner state, restores newer child work, and reads the state with the same `readRemoteRunnerState` call that the continuation uses. It also covers an interrupted carry on a repeated restore. - `doc/execution-semantics.md`: document that runtime state moves with the restored root. ## Verification - Red before the fix: `npx vitest run src/services/remote-workspaces.integration.test.ts -t "retired root"` failed in the `own retired root`, `own restored root`, and `read-only folder` cases with `Error: runner_remote_state_unavailable: exit=1 timedOut=false`. This is the same error as the staging failure. - Green after the fix: all nine cases pass. The continuation read returns the parent's runner state. The old `workspace-ready.json` is gone and the new restore receipt is present. A repeated restore finishes an interrupted carry and removes the retired root and its stale files. - `npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts`: 250 passed. - Staging evidence (read-only): the failed parent wake run recorded a succeeded `remote_workspace_restore` at the parent's own provider lease, and then failed in the runner-state read. - I did not run a live staging run. This PR does not deploy. ## Risks - Runtime state from the retired root now persists into the restored root. The state is the sandbox's own state for this workspace path, which is the same state that a same-allocation reuse keeps. - If the restored root already has an entry with the same name, the old entry is not moved and is removed with the retired root. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. It used tool use for code search, shell, test runs, and read-only staging inspection in the browser. ## 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> |
||
|
|
21640b6d51 |
feat(connections): gate the built-in Google Workspace connector behind an experimental toggle (#19247)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Apps catalog lets operators connect external tools, including Google services. > - Paperclip has a built-in Google Workspace connector, but Google has not yet verified Paperclip's Google app. > - The built-in connector also hides the Composio and Arcade Google apps, because native Google definitions claim those app names. > - Users therefore see an unverified connector and cannot find the working aggregator Google apps. > - This pull request puts the built-in connector behind a default-off experimental setting. When the setting is off, the aggregator Google apps appear. > - The benefit is that users get working Google connectors now, and the team can still test the built-in connector on demand. ## Linked Issues or Issue Description **Problem** Google has not verified Paperclip's Google app yet. The Apps page still offers the built-in Google Workspace connector. The built-in Google definitions also suppress the Composio and Arcade Google apps (Gmail, Google Calendar, Google Drive, Docs, Sheets, Slides, Chat), so users cannot find them. **Proposed solution** Add the `enableGoogleWorkspaceConnector` experimental setting. The default is off. When it is off, hide and block the built-in connector, and show the aggregator Google apps. When it is on, keep the current behavior. **Alternatives considered** Delete the built-in connector until verification is complete. We rejected this option because the team must continue to test the connector. **Additional context** Composio documents Gmail (63 tools), Google Calendar (50 tools), and Google Drive toolkits that use Composio-managed OAuth. These toolkits do not need Paperclip's Google app verification. ## What Changed - Added the default-off `enableGoogleWorkspaceConnector` experimental setting to the shared types, validator, feature catalog, server defaults, and the managed-key list for Paperclip Cloud staging. - Added `experimentalFeatureKey: "enableGoogleWorkspaceConnector"` to the `google-workspace` app definition. The existing generic connector gate then hides the connector in the gallery and blocks its setup, discovery, execution, and agent suggestions when the setting is off. - Added a "Google Workspace connector (Paperclip)" toggle card to **Settings → Experimental**. - Apps page (`Browse.tsx`): when the setting is off, built-in Google rows without saved accounts no longer claim aggregator app identities. The Composio and Arcade Google apps then show as aggregator rows. The legacy Google slug filter now applies only to native rows, so the Composio `gmail` row is not dropped. - Saved per-service Google connections (Gmail, Drive, and others) are not gated. They stay visible and usable. - Updated `doc/connections/GOOGLE-WORKSPACE.md`. - Tests: added tests for the hidden and visible Apps page states, the settings toggle, and the definition gate. Existing built-in Google Workspace tests now enable the setting. ## Verification - `cd packages/shared && npx vitest run` passes (1055 tests). - `cd server && npx vitest run src/__tests__/connection-management.integration.test.ts src/__tests__/tool-access-service.test.ts src/__tests__/tool-gateway-service.test.ts src/services/gmail-tool-governance.test.ts src/services/google-chat-tool-policy.test.ts src/__tests__/instance-settings-service.test.ts src/__tests__/health.test.ts src/__tests__/instance-settings-managed-overlay.test.ts` passes (864 tests). - `cd ui && npx vitest run src/pages/apps src/pages/InstanceExperimentalSettings.test.tsx src/features/connections` passes. - `tsc --noEmit` passes for `packages/shared`, `server`, and `ui`. The `ui` run shows errors only in the untouched `storybook/support/feature-map` files. - `pnpm check:token-gates` passes. - Manual check: open **Apps** with the setting off. Search for "Gmail". The Composio Gmail row appears, and the built-in Google Workspace card does not. Turn on the setting in **Settings → Experimental**. The built-in card appears again. ## Risks - Existing built-in `google-workspace` connections stop running while the setting is off, because the generic gate blocks execution. This is intentional, so that the unverified Google app is not used. Turn on the setting on instances that must keep these connections. - Saved per-service Google connections are not affected. - When the setting is on, the behavior does not change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, running in Claude Code with tool use (shell, file edits, web fetch). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
0c9962a9bc |
fix(plugins): replace a retired worker with a fresh process (#19245)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox providers such as Daytona run as plugin workers that the host manages in `server/src/services/plugin-worker-manager.ts` > - The host retires a worker when it cannot prove that a login pseudo-terminal or a duplex channel route is gone (fail closed) > - Retirement used `killProcess()`, which marks a deliberate stop, so the handle stayed `stopped` and no restart followed > - Every later call then failed with `Cannot call "<method>" — worker for "<id>" is stopped` until the server process restarted > - This pull request makes a retirement replace the worker with a fresh process through the existing backoff timer, with a bound on repeated retirements > - The benefit is that one unconfirmed close no longer makes a sandbox provider unavailable for the whole server ## Linked Issues or Issue Description Refs #19150 **What happened?** A Daytona plugin worker was retired after an unacknowledged duplex close. After that, every run that used the Daytona environment failed setup with `Cannot call "environmentExecute" — worker for <plugin> is stopped`. 15 runs failed in about 12 minutes. The plugin stayed unavailable until the server restarted. #19150 removed the specific trigger. It keeps fail-closed retirement for close replies that the host cannot confirm, so the same outage can still occur. **Expected behavior** The retired process, its routes, and its tombstones are never reused. The plugin comes back on a fresh process after a short, bounded backoff. A deliberate stop (plugin disable, upgrade, shutdown) stays stopped. **Steps to reproduce** 1. Start a plugin worker and open a duplex channel whose close acknowledgement is wrong or missing. 2. Close the channel. The host retires the worker. 3. Call any worker method. Before this change, the call fails with `is stopped` forever. ## What Changed - Add `retireWorkerProcess()`. It kills the process but does not mark a deliberate stop. The three fail-closed retirement paths use it instead of `killProcess()`. - In the exit handler, a retired exit settles every route as before, then counts the retirement and schedules a fresh process through the existing backoff timer (`crashed` → `backoff` → `running`). It emits the existing `crash` event with `willRestart`. - A deliberate stop wins. If `stop()` runs before the exit, the exit ends `stopped`. If `stop()` runs during backoff, it cancels the restart. - Bound repeated retirements: at most 5 retirements in the 10-minute crash window. A successful restart does not reset this count (it does reset the crash count), so a worker that starts cleanly and is retired again cannot loop forever. After the bound, the worker stays `crashed`. - `scheduleRestart()` / `computeBackoffMs()` take the attempt number, so the crash path and the retirement path each use their own counter. - Add an optional `crashRecovery` start option (`minBackoffMs`, `maxConsecutiveRetirements`) so tests can use short waits. - Update the Daytona worker close test: a stalled close now shows `backoff`, then a fresh `running` process with a new pid. ## Verification - Red before the fix: the three new tests in `server/src/__tests__/plugin-worker-manager-duplex.test.ts` (`plugin worker replacement after a fail-closed retirement`) failed with `expected 'stopped' to be 'running'` / `'backoff'`. - Green after the fix. The tests prove: - the retired route and a sibling route on the same worker both settle with `{ exitCode: null }`; - the fresh process has a new pid, a write to a retired route does not revive it, and a new route on the fresh process echoes data; - `stop()` during backoff stays `stopped` with no process; - with `maxConsecutiveRetirements: 2`, the third retirement leaves the worker `crashed` and `call()` rejects. - The real Daytona plugin worker test (`plugin-worker-manager-daytona-close.test.ts`, `stalled-close`) now shows the replacement with the actual provider bundle. The closed lease still rejects on the fresh process. - `cd server && npx vitest run src/__tests__/plugin src/__tests__/managed-plugin src/services/plugin` → 48 files, 649 tests pass. The new tests passed 5 runs in a row. - `npx tsc --noEmit -p server` shows no errors in the changed files. - Not run: a live staging run. This PR does not deploy. ## Risks - A plugin that keeps causing retirements now restarts up to 5 times in 10 minutes (1s, 2s, 4s, 8s, 16s backoff) instead of staying dead. After that, it stays `crashed`, as a crash loop does now. - A retirement now emits `plugin.worker.crashed`, so host-service cleanup runs for the plugin, the same as for a crash. A restart emits `plugin.worker.restarted`. A retirement also counts in `totalCrashes`, so idle sleep treats the worker as it treats a crashed worker. - A call made during backoff fails with `is backoff` rather than `is stopped`. The outage is bounded to the backoff, not to a server restart. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, 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> |
||
|
|
5498db1072 |
fix(interactions): relay a verified reply to a self-requested human-only confirmation (#19243)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents ask users to approve proposals and plans with confirmation cards, and users often answer in the conversation > - `resolve-from-comment` lets the responding agent record that verified reply on the card > - Native agents often create their own proposal or plan confirmation as `human_only`, because the `request_human_input` schema offers only that value > - `resolve-from-comment` then refuses with `interaction_human_only`, and the plan and onboarding gates keep the task in `in_review` or `blocked`, although the user explicitly approved > - This pull request adds one narrow relay for that case and keeps every other human-only gate on its card > - The benefit is that an explicit conversational approval works again, and governed, company-capped and quality-review approvals keep their existing controls ## Linked Issues or Issue Description Refs #19073 **What happened?** TestOps launch https://paperclip.testops.cloud/launch/2694 (GitHub run https://github.com/paperclipai/paperclip/actions/runs/41915965118) failed three Product regression cases for the same reason: `continuation/revision-preserves-approval`, `first-task/clarify-propose-accept` and `first-task/revise-accept`. In each case the agent created a `request_confirmation` with `resolverPolicy: human_only` (`effectiveResolverPolicySource: requested`). The user replied "I approve…" or "Yes, I accept that proposal". The agent's `resolve-from-comment` call returned 403 `interaction_human_only`. The task stayed `in_review` (plan gate) or became `blocked` (onboarding decision gate). **Expected behavior** When the agent itself asked for a human-only answer, the verified latest reply of the user it serves is that human answer. The agent can record it. Company-capped, governed-action, issue-review and quality-review gates still require the human to use the card. **Steps to reproduce** 1. A native agent calls `request_human_input` with `interactionKind: confirmation` and `resolverPolicy: human_only`. 2. The conversation user posts an explicit approval comment. 3. The agent calls `POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment`. Before this change it gets 403. ## What Changed - `issue-thread-interaction-resolution.ts`: the agent actor gets an optional `relayedHumanUserId`. A human-only decision is allowed as `allow_relayed_human` only when all of these are true: - the stored policy is `human_only`, its source is `requested` and its provenance is `explicit` - the resolving agent created the card (`createdByAgentId` matches) - no governed action applies - no additional restriction (issue review) applies - any named addressee user is the relayed user Company caps, the `governed_action` source and legacy rows with an unknown source are still denied. - `confirmation-comment-resolution.ts`: the service sets `relayedHumanUserId` only after its existing comment checks pass: user-authored, not agent-attributed, posted after the card, the latest user message, the same conversation session. It also requires that the author is the card's addressee, the conversation user or the run's responsible user, that the card is not a quality review, and that the company's current cap for the kind is not `human_only` (a tied cap keeps the source `requested`). The same actor goes into the locked accept/reject re-check. The activity entry adds `resolverPolicy: "human_only"` and `resolution: "relayed_human_decision"`. - `routes/issues.ts`: the route pre-check passes the candidate comment author. The service still re-verifies everything under its locks. - `issue-thread-interactions.ts`: carries the relay field into the service's own audience re-check. - Docs: the first-task skill and `api-reference.md` describe the narrow relay. The guidance still says not to add `human_only` to ordinary proposals. ## Verification - `server/src/__tests__/confirmation-comment-resolution.test.ts` (embedded Postgres): 72 pass. - Green: relay through a conversation user and through a run's responsible user. The card is `accepted`, `resolvedByAgentId` and `resolvedByRunId` are set, `resolvedByUserId` stays null, and the audit records `relayed_human_decision`. - Negative controls, each with the card still pending and no audit: agent-authored comment, earlier comment, wrong user, unbound issue/run, `company_cap` source, `governed_action` source, `qualityReview`, `toolAction`, `connectionAuthorization`, issue-review `human_only` restriction, another addressed user, human-created card, another agent's card, inherited company default, and a real capped card created through `svc.create` with a tied cap. The tied-cap test fails as expected when the cap is removed. - Existing negative controls are unchanged, including `human_only` without a bound user and the "rechecks narrowed permissions" retry. - Red check: with the three service files reverted to `master`, the two new green tests fail and the negative controls pass. - `server/src/services/issue-thread-interaction-resolution.test.ts`: 21 pass, including the pure evaluator relay rules. - `server/src/services/native-runtime/confirmation-comment-api.integration.test.ts`: 11 pass. The real route accepts the relayed reply and refuses another user's comment with 403. - Also pass: `issue-thread-interaction-routes`, `onboarding-first-task-admission`, `paperclip-runner-tool-authority`, `runner-api`, `onboarding-first-task-assets` and `company-skills-service` (2,474 tests in total). - Changed files typecheck cleanly. - Live verification: Product regression single-case runs of the three cases are pending after merge. ## Risks - This touches an approval gate. The relay is limited to cards whose creator chose `human_only`. All existing reply-evidence checks still apply, plus a bound-user requirement. Company caps, governed actions, quality reviews, issue-review restrictions and named addressees still deny the relay. - Attribution does not change: the resolver is still the agent and run. The human's comment is kept as `result.commentId` and in the audit. - Legacy rows with a null policy source are not relayed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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> |
||
|
|
26391707fb |
fix(runner): never settle an OpenCode reply truncated at the output limit (#19242)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native OpenCode runs turn the provider's last assistant text into
the task's visible reply
> - A model can loop until it reaches the output limit. The provider
then reports `finish: "length"`
> - The driver used that cut-off text as the final answer, and the run
succeeded with a broken reply
> - This pull request treats a response that stopped at the output limit
as incomplete
> - The benefit is that users get the accepted completion summary or one
corrected answer, never a truncated wall of text
## Linked Issues or Issue Description
**What happened?**
In a QA staging run, an OpenCode agent on
`openrouter/deepseek/deepseek-v4.1-flash-1017` called `paperclip_finish`
correctly. Then it produced about 32,000 output tokens of repeated text
(`E2E_QUIP_QUIP_...`) until the output limit stopped it. The driver
selected this truncated text as the settled final answer. The run
succeeded, and the task reply was wrong.
**Expected behavior**
A response that the provider stopped at the output limit is not a
complete answer. If an accepted completion report exists, its summary is
the reply. If not, the driver asks for one concise final answer. If that
also stops at the limit, the turn fails.
**Steps to reproduce**
1. Run a native OpenCode turn where the model calls `paperclip_finish`.
2. Let the next assistant message run to the output limit (OpenCode
reports `finish: "length"`).
3. Before this change, that message text is emitted as the final
`agentMessage`.
**Paperclip version or commit**
master at
|
||
|
|
e8163fc860 |
fix(remote-workspaces): restore newer saved work over a retained sandbox's own retired root (#19231)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote workspaces save a task's files as snapshots and restore them on the sandbox that runs the next turn > - A parent task can delegate subtasks. The subtasks inherit the parent's workspace, run on their own sandboxes, and save newer snapshots > - When the parent wakes on its retained (warm, reused) sandbox, its earlier workspace root is still on that sandbox at the same published path > - The restore refused that root with `reason=occupied_root`, so the parent run failed at setup and the task became blocked with a board recovery action > - This pull request lets the restore replace that root only when the server proves it is the workspace's own earlier, fully saved allocation on this sandbox > - The benefit is that a delegating parent on a retained sandbox continues with the children's saved work, and unknown occupants are still refused ## Linked Issues or Issue Description **What happened?** A parent task on an agent with a retained sandbox delegates two subtasks to other agents. Both subtasks finish. The parent's wake run fails before the provider starts with `remote_workspace_restore_failed: step=sandbox_restore; ... stage=inspect_root reason=occupied_root`. The task becomes `blocked` with a stranded-work recovery action. Parents on agents with ephemeral sandboxes complete in the same wave. Root cause: the subtasks inherit the parent's execution workspace. Each subtask becomes the writer at a new epoch on its own sandbox, restores at the same published directory name, and saves a newer snapshot. The workspace location then points at the last subtask's sandbox. The parent's wake run gets a lease on its retained sandbox, so preparation is not "same allocation" and must restore the newer snapshot. The target path on the retained sandbox still holds the parent's epoch-1 root, which has a preparation receipt (`workspace-ready.json`) and no restore receipt. The restore script refuses any occupied root that is not the same restore, so it fails with `occupied_root`. A warm reuse cannot just keep the old root, because the snapshot is newer. **Expected behavior** The parent restores the newest saved work (its own files plus the children's results) into its published root and continues. A foreign or unknown occupant is still refused. **Steps to reproduce** 1. Prepare a remote workspace on sandbox A, write files, and save (final boundary). 2. Make a second run the writer on sandbox B. It restores, writes new files, and saves. 3. Make a third run the writer on sandbox A again (same provider lease id, new lease row) and prepare. 4. Before this change, step 3 fails with `stage=inspect_root reason=occupied_root`. **Paperclip version or commit** `master` at `47027e7ba5`. ## What Changed - `server/src/services/remote-workspaces.ts`: add `isRetiredOwnAllocation`. It reads this workspace's snapshot and restore operations recorded at the same environment, provider lease id, and root path. It returns true only when every such use is from an earlier epoch than the current writer, the last use ended in a succeeded `final` save with quiesced save work (`unknown=false`, no active invocations), no other save in that epoch is newer or unsettled, and the saved snapshot is committed or later. Preparation passes the result to the restore as `replaceRetiredRoot`, and only when there is no migration and the authority is clean. - `server/src/services/remote-workspace-transport.ts`: the restore script accepts `replaceRetiredRoot`. In that mode it replaces an occupied root only when the root carries a well-formed Paperclip receipt (`workspace-ready.json` or `workspace-restored.json` with a 64-hex identity). It parks the old root at `<root>.retired-<identity>`, publishes the restored folder, rolls the old root back if the publish rename fails, and then removes the parked copy. A repeated restore with the same identity adopts the published folder and removes any parked copy left by an interrupted attempt. Without a receipt, the script still fails with `reason=occupied_root`. `replaceRetiredRoot` and `allowEmptyRoot` cannot be combined. - `doc/execution-semantics.md`: document the rule. - `server/src/services/remote-workspaces.integration.test.ts`: add a red/green regression and the safety cases. ## Verification - Red before the fix: `npx vitest run src/services/remote-workspaces.integration.test.ts -t "retired root"` failed in the `own retired root` case with `remote_workspace_restore_failed: step=sandbox_restore; remote_workspace_operation_failed: operation=restore exit=1; remote_workspace_restore_failed: stage=inspect_root reason=occupied_root`. This is the same cause text as the staging failure. - Green after the fix: all six cases pass: - `own retired root`: the parent restores on its retained sandbox. The root has the parent's file and the child's file, the parked and staging folders are removed, and the authority records the parent's lease. A repeated preparation (lost reply) keeps the parent's new unsaved edits. - `own restored root`: an occupant with a restore receipt from a different preparation is also replaced. - `receipt missing` and `forged receipt`: the occupant is refused with `occupied_root`, and its files stay. - `later allocation`: a later use of that root on that sandbox without a final save is refused. - `never saved here`: a different sandbox with an unknown folder at that path is refused. - `npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts`: 247 passed. This includes the existing `occupied root` cold restore case, which is still refused. - `tsc --noEmit` shows no errors in the changed files. The local tree has unrelated errors from unbuilt workspace packages. - I did not run a live staging run. Staging is read-only for this work, and this PR does not deploy. ## Risks - The replaced root is removed after the publish. The server proof states that its content is in the saved lineage. If a process wrote to that root after its final save and outside a Paperclip run, those writes are lost. The same assumption applies when authority moves to another allocation today. - The proof uses operation records that have `exportCleanup` location metadata. Older records without that metadata do not qualify, and those cases still fail closed with `occupied_root`. - A warm runner process that kept an open working directory in the old root sees a removed folder. This is not verified live. The new turn is given the published path, which holds the restored files. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. It used tool use for code search, shell, test runs, and read-only staging inspection in the browser. ## 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> |
||
|
|
5d8d9bf83e |
fix: share native lifecycle policy with instruction checkpoints (#19230)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Warm native Codex turns save registered agent files at each turn boundary. > - Native execution can inherit warm mode from the agent when the environment has no explicit policy. > - File preparation checked only the environment policy and could miss that inheritance. > - The new copy then had no warm checkpoint callback, even though the native session stayed warm. > - This pull request resolves one admitted policy before both owners use it. > - The benefit is a terminal file checkpoint under the selected lifecycle, with the existing ownership and permission fences. ## Linked Issues or Issue Description Refs #14735 and #19225. The first defines warm file checkpoints. The second retains coordinator ownership while cleanup joins. This change fixes the earlier lifecycle selection mismatch. **What happened?** With no explicit environment policy and an agent warm default, native selection chose warm mode with a 900,000 ms idle window. Actual file preparation registered an agent_files copy without its warm flag or checkpoint callback. An unchanged turn could have no save marker. Later retirement of a retained lease could reach the stopped-file retrieval guard and preserve pending collection debt. **Expected behavior** File preparation and native execution use the same admitted lifecycle policy. Warm native Codex copies receive their terminal checkpoint callback. Explicit per-turn overrides and pinned admissions keep their existing precedence. A pending historical copy does not become saved because policy resolution changed. **Steps to reproduce** 1. Configure a native Codex agent with a warm default and a reusable sandbox without an explicit runner lifecycle policy. 2. Prepare its managed agent-file copy through the normal instruction owner. 3. Select native execution through the normal runtime owner. 4. Compare the selected warm policy with the copy's missing warm flag and checkpoint callback. **Paperclip version or commit** Base: `3a090b815b`. The owning policy code also exists on its parent revision. **Deployment mode** Native Codex with registered agent files and a reusable sandbox that inherits its lifecycle policy. ## What Changed - Move the existing lifecycle precedence and backend gates into a narrow shared resolver. - Resolve one value before instruction preparation and pass it into runtime selection. - Select warm file ownership from that value and the existing file capability gate. - Add real-Postgres unchanged and changed terminal checkpoint cases, policy precedence cases and updated preparation coverage. - Exercise the same resolve, timed preparation and runtime-selection orchestration used by heartbeat; preserve preparation error identity and skip selection on preparation failure. - Document new-admission overrides and pinned-policy behavior. ## Verification - Red: actual preparation and runtime-selection producers disagree. The selected native mode is warm, but the real persisted copy has no warm flag. The fixture uses real PostgreSQL, managed-root authorization and a synthetic loopback transport. - Green: 85 owner tests pass. They include actual unchanged and changed terminal checkpoints plus 19 lifecycle hierarchy/capability cases. Changed synthetic bytes reach canonical storage with a saved receipt; unchanged bytes record unchanged. - Safety: 25 existing warm retirement and stopped/destroyed/foreign-owner cases pass. Six authorization, stale-adoption and adopted-owner preservation cases pass. The lease guard and native executor are unchanged. - Workspace typecheck, build and map checks pass. The final candidate's code and test blobs equal the tested source; its exact build stamp is recorded locally. - No new repository-wide local run was repeated. A separate earlier broad command on an older head failed a reproducible GitHub enrollment fixture; it is not a full local pass, established flake or proven caused by this change. Fresh full hosted CI is green on the reviewed final head; both current-head Greptile and native substantive reviews are 5/5 with no actionable findings. - Review regression: removing actual policy propagation to preparation in the shared production orchestration fails both real-Postgres cases while runtime selection still chooses warm mode. Restoring it passes. These cases invoke the production helper used by heartbeat, not the entire outer heartbeat service. - The provider-facing transport is a local loopback fixture. No model turn, actual remote platform, physical stop receipt or deployed recovery is proved by these tests. ## Risks - New Codex copies that inherit agent warm mode now use the existing terminal checkpoint path. This can add the expected file checkpoint where only an unchanged probe ran before. - The resolver preserves explicit environment per-turn overrides, pinned admissions and backend/credential gates. It does not create a timer, provider retry or new permission. - Existing pending copies remain pending. This change does not authorize retrieval from stopped or replaced leases and does not establish recovery of historical files. - No database migration, public API field, agent setting, provider change or manual rollout is included. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, source inspection and local code execution. Context window and reasoning effort are host-managed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
47027e7ba5 |
fix(routines): retain authenticated agent responsibility on creation (#19222)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Routines save work for later execution under a responsible person's authority. > - An agent can create a routine through its normal REST or native API tools. > - The create route kept the agent creator but dropped the authenticated responsible user. > - The service then selected a parent task's user or a company default, even when the source run had a different user. > - This pull request passes the authenticated responsible user separately from creator attribution. > - Future automatic runs then use the authenticated responsible user saved in the routine revision. ## Linked Issues or Issue Description **What happened?** An authenticated native agent created a routine without a parent task. The stored responsible user became a different company owner. An unrelated parent task could also replace the authenticated user. The defect changes the identity saved for future automatic work. **Expected behavior** Creation must use the server-resolved responsible user from the authenticated run or API key. The agent remains the creator. A person who clicks Run now still supplies that manual run's identity. **Steps to reproduce** 1. Create an active company member and a different company owner. 2. Initialize a native agent run with the member's identity. 3. Use `call_api` to create a paused, self-assigned routine without a parent. 4. Read the routine and its first revision. Before this fix, both use the company owner. **Paperclip version or commit** Reproduced on master `56517dda6c5b32bc901b7dbf1605eaaa041c5fba`. **Deployment mode** Local source integration with real HTTP routes, agent authentication, native tool authority, and embedded PostgreSQL. No provider turn is needed for the reproduction. Related work: [#19128](https://github.com/paperclipai/paperclip/pull/19128) adds board controls for saved responsibility. [#15207](https://github.com/paperclipai/paperclip/pull/15207) repairs synthetic identities from built-in installers. Neither fixes this authenticated create path. ## What Changed - Pass the authenticated agent's responsible user as a separate internal actor field. - Select the board user first, then the agent’s server-resolved responsible user, before the existing parent and company fallback. - Keep creator attribution, initial revision attribution, manual dispatch identity, and existing records intact. - Add real native API, HTTP, and PostgreSQL regressions for identity selection, replay, client spoofing, stale claims, revoked membership, cancellation, cross-company calls, API keys, and fallback behavior. - Document the creation and manual dispatch identity rules in the execution contract. Correct the obsolete responsibility sentence in the agent access map. - Make the existing live-retry route fixture wait for its committed comment’s durable wake before draining. Keep all process and saved-owner assertions. ## Verification - Captured a failing native `call_api` test before changing production code. The persisted responsible user was the unrelated company owner. - The same real route and database boundary passes after the fix. - Initial affected routine and identity checks: 131 passed. The existing pending-AI-card fence also passed. - Follow-up routine checks: 118 passed. The owning scheduled-retry suite passed all 31 tests. - Initial CI serialized shard 7/9 failed with a `23503` wake-to-agent foreign key error during test cleanup. A controlled real-route/PostgreSQL pause reproduced that exact error on unchanged base `56517dda6c`. The drain returned before the route registered its deferred comment wake. The same pause passed after the fixture waited for the actual commentId’s durable wake receipt. The original failure is preserved. - Affected server typecheck after the fixture query change: passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - `pnpm test:run`: still running as supporting coverage. The worktree changed during the run: map cleanup, then the six-line fixture wait. Production and routine-test files stayed unchanged. This is mixed-working-tree coverage, not an immutable full-suite pass for a commit. Exact next-head CI remains required. - `node scripts/check-maps.mjs` and `git diff --check`: passed. - Exact head `a12e3f6c42b8ad5826fd58faa3cd9553ea251e09`: all CI gates passed. Greptile and the native substantive review both gave 5/5 with no open findings. The native reviewer did not run tests or exercise shadow-mode authorization. - Live provider execution remains unverified. This change does not grant access to another person's AI account. ## Risks New agent-created routines now use their authenticated responsible user before a parent or company default. This is the intended behavior change. The service keeps its fallback when no bound identity exists. Missing agent-key identity still fails authentication. Missing personal AI configuration still blocks execution. Existing routines, revisions, tasks, grants, and manual dispatch behavior do not change. The fixture waits for a persisted wake record before draining; its original live-process and saved-owner assertions remain. ## Model Used OpenAI `gpt-6.1-sol` with reasoning, tool use, and code execution. The host did not expose the context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
3a090b815b |
fix: retain native checkpoint ownership through warm close (#19225)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native warm session must save its checkpoint under its exact coordinator owner. > - A failed turn can return an accepted result before that checkpoint is saved. > - The executor stopped renewal and released the owner before the managed session closed. > - The backup owner guard then refused the uncommitted, released owner. A failed close could remain in the warm session map. > - This pull request keeps the claim and its existing renewal through checkpoint, close and instruction collection. > - The benefit is a valid checkpoint owner and one fenced release after the work joins. ## Linked Issues or Issue Description Refs #19208, #19206 and #14695. Those fixes cover a destroyed backup source, Stop before a provider session opens, and retained agent-file ownership. They do not change this checkpoint ownership order. **What happened?** A failed warm turn accepted its result. Its coordinator reached `workspace_finalizing`, with a result ID and no lease owner. Managed close then called the durable checkpoint writer. The writer correctly refused this state with `native_harness_storage_owner_missing`. The failed close could remain for the next fresh session to retire. **Expected behavior** The current claim and renewal remain valid until the required checkpoint, close and instruction collection join. Cleanup stops and joins renewal, then releases only that exact claim. A changed owner is not cleared. An earlier error keeps its identity. **Steps to reproduce** 1. Run the new real-Postgres regression against the base commit. 2. Complete a structured failed warm turn through the normal control-plane result producer. 3. Let the actual managed-session close call its production checkpoint callback. 4. Observe the missing-owner refusal after coordinator release. **Paperclip version or commit** Base: `56517dda6c`. The native ownership order also exists on the earlier failing revision. **Deployment mode** Native warm execution with durable harness storage and a managed remote workspace. ## What Changed - Join warm checkpoint, managed close and instruction collection before coordinator release. - Keep the existing lease renewal active through that work. Stop and join it before release. - Run the same fenced release after a checkpoint or collection error. Preserve the initiating error unless restart detachment has won. Keep the typed restart outcome and changed-owner fences. Attach only closed cleanup-failure flags. - Add real-Postgres ownership, delayed-renewal, error, retry and denial cases. Route the managed-session regression through the existing artifact-equipped native test lane. Update the continuity test for the retained claim. Document the active renewal lifetime. ## Verification - Red: the producing coordinator had an accepted result, `phase=workspace_finalizing` and `leaseOwner=null` at managed close. The production checkpoint callback refused it with `native_harness_storage_owner_missing`. - Green: all 9 real-Postgres regression cases pass. They cover a real 20-second renewal tick, a failed checkpoint, one force-fresh successor, and wrong-boot and changed-owner denial. Actual restart detachment wins even when checkpoint and joined renewal-stop both reject. The replacement claim remains unchanged. No new provider turn or cancellation receipt is created. A private canary does not enter the closed diagnostic flags or an error cause. - The executor, restart-recovery and heartbeat compatibility suites pass: 1,259 tests, with 20 gated skips. The container-enabled continuity case was not run locally. Its 21-second timer check remains. It now requires the exact claim during `hasChanges()` and checks release after the joined work. - All 39 shard-routing and combined-coverage tests pass. The new regression runs in the existing native lane with the required Runner artifact. - Final local head `0edcc37f35` passes `pnpm -r typecheck`, `pnpm build`, `node scripts/check-maps.mjs` and the 9-case regression. The earlier broader compatibility run used identical source bytes. A separate earlier repo-wide local suite on `6cdd68e915` ended with exit 1. Its general-server stage had one GitHub enrollment acknowledgement failure, 31,469 passing tests and 215 skipped tests. That acknowledgement failure reproduced in one narrow run on the same old tree. It is not an established flake or proven caused by this fix. Later repository stages did not run. No duplicate broad suite or full local pass is claimed. - The first published head failed review for detach-error precedence and the old continuity assertion. Its server shard also failed because the new test lacked a Runner daemon in the general lane. Those failures remain recorded. This candidate fixes their causes. Both exact-head reviews are now 5/5. The hosted native-server lane ran all 9 new ownership tests successfully. One unchanged tool-gateway rate-rule test failed in the first current-head CI run. It passed previously and passed locally, both narrowly and in its full 183-test file. The single same-head failed-job rerun passed. All current-head hosted gates are clean. The initial failure remains recorded; no test assertion was weakened. - The provider transport is an in-memory fixture. The managed wrapper, production checkpoint callback, control-plane result and Postgres ownership are real. No paid provider or live remote process is exercised. This does not prove a deployed recovery or a remote stop receipt. ## Risks - A slow required checkpoint extends the lifetime of the existing active renewal. Its cadence stays at 20 seconds for remote claims and 5 minutes for local claims. It runs only while the admitted checkpoint, close and collection remain active, then stops and joins on completion or unwinding. The tests observe a real remote renewal tick. No idle timer, new retry budget or new worker is added. - The release still checks company, task, run, owner, attempt, boot, PID, process start and expiry. Restart detachment retains its recovery owner. - The storage owner guard is unchanged. No migration, permission, provider setting or deployment change is included. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, source inspection, local code execution and read-only staging metadata. Context window and reasoning effort are host-managed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
b8e8ab4641 |
fix(recovery): continue saved input after a recovered sandbox stops, not after its destroy (#19224)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can stop or interrupt a native run and send a message for the next turn > - When the stop leaves sandbox files unsaved, automatic recovery saves them, re-stops the sandbox, and the saved message continues > - For agents whose sandbox is destroyed after each turn (an ephemeral lease), recovery then queues the destroy. That step removed the lease's stop receipt > - The continuation gate waits for a stop receipt, so the message waited for the destroy and its cleanup retry, about 5 to 8 minutes > - Agents with retained sandboxes continued within seconds, so the delay looked agent specific > - This pull request keeps the verified stop on the cleanup lease and lets the continuation and the next remote writer accept it > - The benefit is that a redirect, Retry or interrupt continues right after its files are saved, for every lease policy ## Linked Issues or Issue Description Refs #19206 **What happened?** On hosted staging, a Stop or `interrupt: true` comment on a native run left a follow-up turn that started 5 to 8 minutes later for some agents, and 25 to 40 seconds later for others. The slow agents use ephemeral sandbox leases. In each slow case, the stopped run's lease was released by `pending_cleanup_retry` about 5 minutes after recovery, and the saved message started one second later. The fast agents use retained leases, which recovery releases at once. **Expected behavior** The saved message continues right after recovery saves the files and the sandbox is stopped. The destroy of a stopped sandbox is resource cleanup and must not hold the next turn. **Steps to reproduce** 1. Use an agent whose sandbox lease is ephemeral (destroy after each turn). 2. Interrupt or stop its native run so that recovery must save the sandbox files. 3. Post a message. See the follow-up start only after the sandbox destroy finishes. **Paperclip version or commit** Hosted, at `56517dda`. ## What Changed - `server/src/services/workspace-recovery.ts`: `scheduleRecoveredWorkspaceCleanup` keeps the verified stop receipt as `workspaceRecoveryCleanup.stopReceipt` when it queues the destroy. - `server/src/services/remote-execution-termination.ts`: new `hasRecoveredCleanupStopReceipt(lease)`. It is true only when the lease has recovery cleanup for its own run, was released, and the carried receipt matches the lease, run, provider and provider lease id. - `server/src/services/explicit-native-continuation.ts`: the `remote_cleanup` gate accepts that receipt. - `server/src/services/execution-blocker.ts`: `hasPendingRunCleanup` ignores a lease with that valid receipt, so a saved native interrupt send (and Retry or fresh-workspace checks) does not wait for the destroy. Other cleanup holds are unchanged. - `server/src/services/remote-workspaces.ts`: the next remote writer accepts that receipt for the ended writer's lease. - `server/src/services/execution-blocker.ts` (from #19206 review, not in the merged head): pre-turn evidence uses the exact startup lifecycle event list, so turn-scoped events such as `workspace.diff.recorded` count as provider work. - `doc/execution-semantics.md`: describes the rule. ## Verification - Red before the fix, green after: `cd server && npx vitest run src/services/workspace-recovery.test.ts -t "admits the interrupt redirect"`. An interrupted native run with recovered files and a queued destroy blocked the user message with `remote_cleanup` before the fix. After the fix the message is admitted. A lease without the stop receipt, or with a receipt for another run, still blocks with `remote_cleanup`. The test runs the same input as a plain message and as a saved native interrupt send (`queuedCommentInterruptId`); `hasPendingRunCleanup` was true before the fix and blocked the interrupt send with `cleanup_pending`. - `cd server && npx vitest run src/services/remote-workspace-reservation.test.ts -t "ended writer"`: the next writer reserves the workspace from an ended writer whose recovered lease awaits destroy; a missing or foreign receipt still blocks. Removing the new branch fails the positive case. - Updated `saves the authoritative remote workspace after native failed ...`: Retry now continues before the destroy, and the cleanup lease carries the stop receipt for its run and lease. - `workspace-recovery.test.ts -t "provider work cannot be excluded"` includes `workspace.diff.recorded` as provider work. - Related suites pass: `workspace-recovery.test.ts`, `explicit-native-continuation.test.ts`, `remote-workspace-reservation.test.ts`, `remote-workspaces.integration.test.ts`, `remote-workspace-lifecycle.test.ts`, `task-retry*.test.ts`, `native-cancelled-workspace-preservation.test.ts`, `workspace-recovery-source.test.ts`. `cd server && npx tsc --noEmit` passes. - Hosted evidence (read only): slow cases have ephemeral leases released by `pending_cleanup_retry` about 5 minutes after `issue.workspace_recovered`, with settlement one second later. Fast cases have retained leases released by recovery. ## Risks - Low to medium. The next turn can start while the old stopped sandbox is still being destroyed. The receipt proves that the sandbox was stopped after the files were saved, and an ephemeral successor uses a new sandbox. A destroy failure stays with the existing cleanup retry. - Leases without the carried receipt behave as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
beb73381ca |
fix(native): let a Stop before the provider session opens continue once (#19206)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A remote task restores its saved workspace into a sandbox before the agent starts. > - When that restore fails, the run records only `remote_workspace_operation_failed: operation=restore exit=1`. > - The run then fails as `setup_failed`. No retry starts, and a `blocked` task gets no recovery action. > - An operator cannot tell which step failed, and nobody owns the next step. > - This pull request makes the sandbox scripts name their failed step in a closed grammar, and it gives a restore failure a bounded retry or a named board recovery action. > - The benefit is a visible cause and a clear owner. Saved work stays protected. ## Linked Issues or Issue Description Refs #19152 and #19187. Those changes name copy-back (save) failures. Refs #18852 and #19195. Those changes added the bounded bootstrap retry that this change reuses. **What happened?** A board reply woke an agent on a `blocked` parent task. The run failed during setup 13 seconds later. The only run event was `remote_workspace_operation_failed: operation=restore exit=1`. The run had no workspace operation row. The task stayed `blocked`, and its recovery actions showed `active: null`. The same opaque family also appeared for workspace preparation (`exit=1` only). **Expected behavior** The run names which restore step failed, without command output or paths. A failure before provider work uses a bounded retry when a retry can help. Otherwise, and when the retry budget is spent, the board gets a named recovery action. The saved version is never changed or overwritten. **Steps to reproduce** 1. Save a remote workspace, then start a new run on a different allocation. 2. Put a folder at the published workspace path in the sandbox before restore. 3. Observe that the base code records only `operation=restore exit=1`, then `setup_failed`, with no recovery action for a `blocked` task. **Paperclip version or commit** Base: `87dfba6818`. **Deployment mode** Legacy runtime with sandbox (Daytona) remote workspace storage. ## What Changed - The prepare, repository, and restore sandbox scripts track their current stage. On failure they print `remote_workspace_<op>_failed: stage=<stage>` plus closed fields: `reason=` (the script's own snake_case refusal), `archive=` (the archive helper's fixed reason), `code=` (errno), `disk=full`, `git=` (a fixed label chosen from Git output), and the existing `exit`/`signal`/`program` fields. - The server accepts that text only when it matches a strict grammar for the same operation. Other output is dropped, as before. - Provider refusals during the restore upload and restore command are recorded by closed labels, like copy-back. - A failed saved-workspace restore raises `RemoteWorkspaceRestoreError` with the server step (`snapshot_lookup`, `snapshot_download`, `sandbox_restore`). It is retryable only when the owner re-reads the same writer, epoch, and saved version with no unsaved work, and the sandbox did not name a refusal reason. - Setup failure handling classifies a pre-dispatch legacy restore failure as `remote_workspace_restore_failed`. A retryable failure uses the existing bounded bootstrap retry. A non-retryable or exhausted failure records a board-owned recovery action with the redacted cause, when stranded-work recovery does not scan the task status (for example `blocked`). - Stranded-work recovery gets a named notice for this code. Generic continuation recovery does not grant it a second budget, and legacy reconciliation does not invent provider actions for it. - `doc/execution-semantics.md` documents the contract. ## Verification - Red: the new transport tests failed on the base code with `remote_workspace_operation_failed: operation=restore exit=1` (and the same for prepare and repository). The new completion tests failed because the run became `setup_failed` with no retry and no recovery action. - Green: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts src/services/remote-workspaces.integration.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/services/legacy-execution-recovery.test.ts src/services/recovery src/services/remote-workspace-save-concurrency.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/__tests__/native-workspace-finalization-work.test.ts` passes (563 tests). - The transport tests use real sandbox commands. They prove an occupied folder reports `stage=inspect_root reason=occupied_root` and keeps its files. They prove a wrong manifest reports `stage=extract archive=manifest_mismatch`. They prove a missing branch reports `stage=checkout_branch`, and a changed origin reports `stage=verify_origin`. Forged output with a path is dropped. - The real-PostgreSQL cold-restore test proves an occupied root gives a non-retryable `sandbox_restore` failure and keeps the unknown files. - The completion tests cover retried, exhausted, refused, post-dispatch, and `in_progress` cases. - `tsc --noEmit` for the server reports no errors in the changed files. ## Risks - A retry keeps the same restore path. If the cause is deterministic but unnamed, the run uses its two retries before the board action. Named refusals skip the retries. - The board action is recorded only for statuses that stranded-work recovery does not scan, and only when no other action is active. This prevents a second recovery identity for one failure. - Script stderr grammar is new. Old sandboxes run the script that the server sends, so no runner update is necessary. - No migration, deployment, or credential change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use, code execution, and browser tools for read-only staging evidence. Context window and reasoning mode are host-managed. ## 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> |
||
|
|
833d068741 |
fix(goals): enforce company boundaries for parent goals (#19203)
Validate parent goals within the child's company and retain parent references through each write transaction. Validate raw plugin patches with the shared goal PATCH contract and keep goal identity immutable. Add real PostgreSQL and plugin RPC regressions for denied writes, valid edits and concurrent reference checks. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
f5891564ec |
fix(recovery): continue from a retired sandbox's backup and hand spent export repairs to the board (#19208)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native tasks keep a warm sandbox and a verified per-turn backup of the provider session. > - Recovery holds stop a task when continuation is not safe. They must have a next action and an owner. > - In a staging soak, two kinds of held tasks could not move forward. Users followed the product's own instruction, and nothing changed. > - First, a reply to a task whose warm sandbox Paperclip had already destroyed failed every time. The label then said "Send a message to continue" again. > - Second, an exhausted system workspace repair stayed active and system-owned for hours. The board saw no owner, and a reply started nothing. > - This pull request lets a reply continue from the backup when the source sandbox is provably gone, and hands a spent system repair to the board with a named Retry. > - The benefit is that productive work always has a next action that can succeed, and nothing replays or runs twice. ## Linked Issues or Issue Description **What happened?** (a) A native warm task ran one successful turn. A later legacy follow-up retired the warm sandbox lease. The next user reply got a new sandbox (`created` lease). The executor found a verified backup but no reusable lease, so it threw `runner_harness_state_mismatch: backup_without_reusable_lease`. The run retried on the restart-recovery path, failed `native_remote_recovery_lease_mismatch`, and ended as `native_continuation_requires_reconciliation`. Workspace recovery then said "Saved work recovered. Send a message to continue." Each new reply repeated the same failure. (b) A run failed `native_workspace_sync_out_retry_exhausted`. System workspace recovery used its 6 bounded attempts and became `exhausted`. The recovery action stayed `active` with `ownerType: system`. The UI hides system-owned recovery actions, so the board saw no action. Nothing ran for more than two hours. **Expected behavior** (a) When Paperclip destroyed the source sandbox and no provider work followed the backup, the reply continues once from the backup. Ambiguous states still fail closed. (b) When the bounded budget is spent, the action names a board owner and a Retry action that can progress it. **Steps to reproduce** (a) Finish a warm native turn. Retire its reusable lease through obsolete-lease cleanup. Send a reply. The new run gets a `created` lease and fails `backup_without_reusable_lease`. (b) Let system workspace recovery reach its deadline or retry limit. Read the recovery action. It is still `ownerType: system`. Related merged work checked first: #19145, #19184, #19187, #19189, #19190, #18943 (added the guard for migrated sessions). Open recovery PRs (`gh pr list --search recovery --state open`) do not touch these paths. ## What Changed - `native-retired-backup-source.ts` (new): `retiredSourceAuthorizesNativeBackupRestore` decides whether a created lease may restore the backup. All must be true: the backup's source lease is `expired` or `failed`; it has a provider termination receipt with state `destroyed`; its `nativeHarnessBackup` stamp names this exact manifest SHA-256, session and runner instance; exactly one host lease maps to the provider allocation; and no provider durable event for the session was recorded after the backup completed. The provider event type set moved here and the executor imports it. - `native-session-executor.ts`: `shouldRestoreNativeHarnessBackupIntoSandbox` accepts `retiredSourceContinuation`. A created lease with a configured reusable lease evaluates it, beside the existing migrated-session path. The restore uses the existing verified restore and stamps the new lease. - `system-recovery.ts`: `updateSystemRecovery` moves an `exhausted` action to `ownerType: board` and writes a next action and workspace recovery message that name the attempt count, the last failure code, and **Retry recovery**. Retry (`requestWorkspaceRecovery`) already starts a new bounded generation and returns ownership to the system. - `doc/execution-semantics.md`: documents both rules. ## Verification - Red before the fix, green after: - `npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "restores a warm backup into a created lease only when"`. Before: the destroyed-source case rejected with `runner_harness_state_mismatch: backup_without_reusable_lease`. After: it restores the runner state and provider home into the new sandbox and stamps the new lease. - `npx vitest run src/services/workspace-recovery.test.ts -t "hands an exhausted system export repair"`. Before: `exhausted` with `ownerType: "system"`. After: `ownerType: "board"`, the blocker says Retry recovery and never says "send a message", no provider run or lease resume happens, and Retry starts one new system generation. - Safety cases (fail closed): a stopped (not destroyed) source, a stamp for another checkpoint, provider work after the backup, a still-reusable `released` lease, a missing receipt, and two host leases for one allocation. These run in the executor test and in `native-retired-backup-source.integration.test.ts` against embedded Postgres. - Related suites pass: `workspace-recovery.test.ts`, `system-recovery-state.test.ts`, `native-session-executor.test.ts`, `task-retry.test.ts`, `explicit-native-continuation.test.ts`, `workspace-recovery-source.test.ts`, `native-workspace-export-retry.test.ts`, `native-remote-retry-admission.test.ts`, `native-workspace-warm-reuse.test.ts`, `native-workspace-sync-retention.test.ts`, `heartbeat-observed-workspace-preservation.test.ts`, `execution-control-work.test.ts` (2,224 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - Not done: a live run against staging. Staging access was read-only. The staging evidence is the run event timeline described above. ## Risks - (a) A backup restore after a lost sandbox could rewind a session if newer provider state existed. The predicate requires a provider `destroyed` receipt and no provider event after the checkpoint. A stopped sandbox, a pending cleanup, or a still-reusable lease still fails closed with the old error. - (a) The event-time check can fail closed if a late event from the backup turn is written after the checkpoint time. That keeps the old behavior; it never authorizes a restore. - (b) The action's next action text stays as the exhausted message while a later Retry generation runs. The task blocker shows the live workspace recovery message first, so users see "Recovering saved work…" during that time. - Not changed: when the predicate fails closed, the run still follows the existing reconciliation path. That path can still end with the "Send a message to continue" receipt. That residual is narrower now and needs a separate change to the workspace recovery receipt. > 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. Agentic coding with tool use (shell, file edits, read-only browser inspection of staging). Default context window. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
2ce5f03cd8 |
fix(workspaces): name remote restore failures and leave a board recovery action (#19207)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A remote task restores its saved workspace into a sandbox before the agent starts. > - When that restore fails, the run records only `remote_workspace_operation_failed: operation=restore exit=1`. > - The run then fails as `setup_failed`. No retry starts, and a `blocked` task gets no recovery action. > - An operator cannot tell which step failed, and nobody owns the next step. > - This pull request makes the sandbox scripts name their failed step in a closed grammar, and it gives a restore failure a bounded retry or a named board recovery action. > - The benefit is a visible cause and a clear owner. Saved work stays protected. ## Linked Issues or Issue Description Refs #19152 and #19187. Those changes name copy-back (save) failures. Refs #18852 and #19195. Those changes added the bounded bootstrap retry that this change reuses. **What happened?** A board reply woke an agent on a `blocked` parent task. The run failed during setup 13 seconds later. The only run event was `remote_workspace_operation_failed: operation=restore exit=1`. The run had no workspace operation row. The task stayed `blocked`, and its recovery actions showed `active: null`. The same opaque family also appeared for workspace preparation (`exit=1` only). **Expected behavior** The run names which restore step failed, without command output or paths. A failure before provider work uses a bounded retry when a retry can help. Otherwise, and when the retry budget is spent, the board gets a named recovery action. The saved version is never changed or overwritten. **Steps to reproduce** 1. Save a remote workspace, then start a new run on a different allocation. 2. Put a folder at the published workspace path in the sandbox before restore. 3. Observe that the base code records only `operation=restore exit=1`, then `setup_failed`, with no recovery action for a `blocked` task. **Paperclip version or commit** Base: `87dfba6818`. **Deployment mode** Legacy runtime with sandbox (Daytona) remote workspace storage. ## What Changed - The prepare, repository, and restore sandbox scripts track their current stage. On failure they print `remote_workspace_<op>_failed: stage=<stage>` plus closed fields: `reason=` (the script's own snake_case refusal), `archive=` (the archive helper's fixed reason), `code=` (errno), `disk=full`, `git=` (a fixed label chosen from Git output), and the existing `exit`/`signal`/`program` fields. - The server accepts that text only when it matches a strict grammar for the same operation. Other output is dropped, as before. - Provider refusals during the restore upload and restore command are recorded by closed labels, like copy-back. - A failed saved-workspace restore raises `RemoteWorkspaceRestoreError` with the server step (`snapshot_lookup`, `snapshot_download`, `sandbox_restore`). It is retryable only when the owner re-reads the same writer, epoch, and saved version with no unsaved work, and the sandbox did not name a refusal reason. - Setup failure handling classifies a pre-dispatch legacy restore failure as `remote_workspace_restore_failed`. A retryable failure uses the existing bounded bootstrap retry. A non-retryable or exhausted failure records a board-owned recovery action with the redacted cause, when stranded-work recovery does not scan the task status (for example `blocked`). - Stranded-work recovery gets a named notice for this code. Generic continuation recovery does not grant it a second budget, and legacy reconciliation does not invent provider actions for it. - `doc/execution-semantics.md` documents the contract. ## Verification - Red: the new transport tests failed on the base code with `remote_workspace_operation_failed: operation=restore exit=1` (and the same for prepare and repository). The new completion tests failed because the run became `setup_failed` with no retry and no recovery action. - Green: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts src/services/remote-workspaces.integration.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/services/legacy-execution-recovery.test.ts src/services/recovery src/services/remote-workspace-save-concurrency.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/__tests__/native-workspace-finalization-work.test.ts` passes (563 tests). - The transport tests use real sandbox commands. They prove an occupied folder reports `stage=inspect_root reason=occupied_root` and keeps its files. They prove a wrong manifest reports `stage=extract archive=manifest_mismatch`. They prove a missing branch reports `stage=checkout_branch`, and a changed origin reports `stage=verify_origin`. Forged output with a path is dropped. - The real-PostgreSQL cold-restore test proves an occupied root gives a non-retryable `sandbox_restore` failure and keeps the unknown files. - The completion tests cover retried, exhausted, refused, post-dispatch, and `in_progress` cases. - `tsc --noEmit` for the server reports no errors in the changed files. ## Risks - A retry keeps the same restore path. If the cause is deterministic but unnamed, the run uses its two retries before the board action. Named refusals skip the retries. - The board action is recorded only for statuses that stranded-work recovery does not scan, and only when no other action is active. This prevents a second recovery identity for one failure. - Script stderr grammar is new. Old sandboxes run the script that the server sends, so no runner update is necessary. - No migration, deployment, or credential change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use, code execution, and browser tools for read-only staging evidence. Context window and reasoning mode are host-managed. ## 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> |
||
|
|
9e9a628a57 |
feat(ui): turn Artifacts V1 on for every instance and hide its toggle (#19164)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Artifacts show the output of agent work on the Artifacts page, in the panel Artifacts tab, and in the task thread > - #17228 added the Artifacts V1 presentation behind an experimental setting that is off by default > - The presentation is now ready for all users, so an opt-in toggle is no longer necessary > - A new default alone does not reach all instances, because each experimental write persists every flag > - This pull request removes the toggle and turns V1 on for every instance > - The benefit is one artifact presentation for all users, with an off switch for hosting operators ## Linked Issues or Issue Description Refs #17228 Refs #17263 **Is your feature request related to a problem? Please describe.** Artifacts V1 shows only on instances where an admin turned on an experimental setting. Other users see the previous cards. **Describe the solution you would like** Show Artifacts V1 on all instances. Remove the toggle from Experimental settings. **Describe alternatives you have considered** A new default without a change to the hook. This does not work, because many instances have a stored false that nobody chose, and no toggle would remain to change it. ## What Changed - Removed the Artifacts V1 card from Instance Settings, Experimental. - Changed `enableArtifactsV1` to default on in the schema, the feature catalog, and the settings service. - Changed `resolveArtifactsV1Enabled` to ignore a stored false. It honours false only when the value comes from managed config. - Kept the flag and the previous cards, so a hosting operator can hold V1 off. - Kept the key in the Cloud staging list, so a managed false does not hold V1 off on staging. - Updated unit tests, the Artifacts V1 plan, and the directions-lab Today reference. - Moved the attachment and upload e2e specs and the runner download helper to the V1 rows. - Added V1 tests for a Markdown review: expansion, deep link, and open, raw and download actions. ## Verification - `cd ui && npx vitest run`: one test file failed before its fix. That file tests the previous cards, and it now mocks the hook to off. I reran it with the other changed UI tests, and they pass. I did not repeat the full UI run after the fix. - `npx playwright test --config tests/e2e/playwright.config.ts tests/e2e/text-attachment-tabs.spec.ts tests/e2e/artifact-uploads.spec.ts tests/e2e/artifact-tab-arrival.spec.ts`: 10 tests pass on V1. - `tests/e2e/cloud-auth.spec.ts` failed once in CI on a task heading. It does not touch artifacts, and 10 local runs pass. - `cd server && npx vitest run src/__tests__/instance-settings`: 183 tests pass. - `cd packages/shared && npx vitest run src/feature-catalog.test.ts src/validators/instance.test.ts`: 35 tests pass. - `pnpm run check:token-gates` and the `ui` and `server` typechecks pass. I did not run the full repository check. - In the artifact directions lab, with no stored settings, the Today right panel renders the V1 rows. - I did not test this on a live instance. After merge, confirm on staging that the Experimental page has no Artifacts V1 card and that all three surfaces show V1. ## Risks - All users see V1 after the next release, not only staging users. To revert, revert this pull request. - Users cannot turn V1 off. Only a hosting operator can, through managed config, and not on Cloud staging. - An instance with a managed `enableArtifactsV1: false` stays on the previous cards. The cloud harness source has no reference to this key, but I cannot read the live fleet configuration. - V1 surfaces render before settings load. An operator-held instance can briefly show V1 before the previous cards. - Admins who use the API can still write the flag. The write has no effect on unmanaged instances. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use 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> |
||
|
|
ea42a2e417 |
fix(recovery): coalesce duplicate legacy no-replay holds (#17055)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery records keep uncertain execution outcomes from being replayed. > - A settled record can still block replay while it waits for a verified decision. > - Repeated scans can create more settled records for the same failed execution. > - Resolving only one duplicate leaves the task behind another hold. > - This pull request retains the settled hold and applies one verified decision to its exact duplicates. > - The task can continue through the existing delivery path without creating another delivery owner for each duplicate. ## Linked Issues or Issue Description Refs #14600. This change addresses duplicate settled holds and their same-source resolution. The visibility and other deferred-wake paths remain outside this change. Related: #10011 handles unchanged active incidents. This patch handles settled no-replay records. #15285 covers missing reconciliation input. #17031 covers a handoff to a new task owner. External-adapter continuation is covered separately by #15982. ## What Changed - Retain a resolved legacy no-replay decision when a later scan finds the same company, task, cause, fingerprint, and source run. - After a verified reconciliation, resolve duplicate records that also match the selected action's kind. Keep the selected action as the delivery owner for this group. Record the merged action IDs in the activity log. - Preserve independent workspace and system recovery records, existing reconciliations, and records for other sources. - Add PostgreSQL regression tests for repeated scans, historical duplicates, delivery count, repeated resolution, scope boundaries, simultaneous resolutions, and rejected reconciliation rollback. - Update the execution semantics and recovery maps. This patch has no schema migration or adapter-specific code. ## Verification Current head: `3c84bc89325d40bfdda5d28f24ed2fb67d48756c`. It merges `master` at `043f2514e67c50320433a0013fb228a75a51325c` without rewriting the existing commits. The only conflict was in `markExecutionReconciliation`. The resolution retains the upstream transaction and all three queue notifications. Source, sibling, and audit writes use the same transaction. The final merged tree passed: ```sh pnpm exec vitest run server/src/__tests__/issue-recovery-actions.test.ts server/src/services/legacy-execution-recovery.test.ts server/src/__tests__/heartbeat-process-recovery.test.ts server/src/__tests__/execution-control-work.test.ts --maxWorkers=1 # 549 passed; zero skipped pnpm --filter @paperclipai/server exec tsc --noEmit # exit 0 node scripts/check-maps.mjs # passed: 35 areas, 191 capability IDs git diff origin/master --check # passed ``` Three new cases add real PostgreSQL and HTTP-route evidence. Two resolution requests target different duplicate holds while both wait on database locks. They leave one continuation owner, one coalescing audit record, and one queued successor. A live-provider rejection leaves all records unchanged. A checkout rejection after reconciliation writes rolls back the source, siblings, and audit row. The fixture occupies the dispatch slot, so no external model provider runs. The new contention case was also run with only `execution-recovery-resolution.ts` replaced by its parent revision at `65f59e339a9fb6e0635bcbd8c36791bd610326a6`. It failed with two continuation owners instead of one. The source was restored byte-for-byte, and all three new cases passed again before the merge. The final four-suite run above validates the merge with the upstream notification changes. The original positive regressions also failed on that parent revision: three scan-and-settle cycles left four records instead of one, and resolving one historical hold left its two duplicates unreconciled. The scope matrix covers company, task, kind, cause, fingerprint, source run, replay state, protected recovery records, and prior reconciliation. Fresh-checkout collection initially failed because plugin SDK build outputs were absent. After building them, PostgreSQL startup exposed native library symlinks missing from an installation with scripts disabled. The package's symlink setup resolved that failure. The standard server typecheck attempt started a Rust vendor prerequisite build and was stopped; direct TypeScript checking passed. These failed or incomplete attempts are not counted as passes. Full repository build, recursive typecheck, and CI were successful on the earlier `08e5c0285` revision ([CI run](https://github.com/paperclipai/paperclip/actions/runs/39541949702)). They are historical evidence, not results for this merge. The current local proof is the four suites and checks listed above. Full upstream CI for the merge is pending. Greptile reviewed `3c84bc89325d40bfdda5d28f24ed2fb67d48756c` with 5/5 confidence and no actionable findings; that source review is separate from test execution. ## Risks - A match that is too broad could clear an independent hold. Tests cover company, task, kind, cause, fingerprint, source run, replay state, workspace recovery, system recovery, and prior reconciliation boundaries. - The existing issue-lock transaction covers the selected action and its duplicates. The patch does not create a new lock or an automatic retry path. - The patch does not infer that a network or provider failure means no action occurred. Existing reconciliation validation still checks the source, owner, stopped processes, leases, and observed action outcome. - Previously reconciled records and their delivery state remain unchanged. This patch prevents new duplicate delivery owners when it resolves unreconciled duplicates. It does not migrate every historical delivery record. ## Model Used OpenAI Codex, based on GPT-6, assisted with the implementation, tests, and review. It used repository tools and code execution. The exact runtime model ID, context window size, and reasoning setting are not exposed in this session. An independent AGY review also ran; its provider model ID was not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with the available 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 linked the existing issue with `Refs #14600` - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal task IDs or instance details - [x] I have run the focused tests locally and they pass; full upstream CI is pending for the final merge - [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 risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Rockiez <7697772+Rockiez@users.noreply.github.com> Co-authored-by: Devin Foley <devin@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
87dfba6818 |
fix(native): retry an unpublished accepted task reply durably instead of dropping it (#19200)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run commits its status decision and its final reply comment
in separate steps
> - Background presentation recovery publishes an accepted reply when
the live path does not record one
> - That worker dropped the run after one refused attempt and only threw
on an error, with no record on the run
> - A task could then show done while the user never saw the answer,
until an unrelated wake reloaded the queue
> - This pull request makes the publication of an accepted ordinary task
reply durable, bounded, and recorded
> - The benefit is that a task cannot close silently without its answer
## Linked Issues or Issue Description
Refs #19130 (verify requested publication at finalization, for
documents), #19183 (deliver messages saved while a native run
finalizes), #19186 (stale owner done). This change does not duplicate
them. It covers the final reply comment.
**What happened?**
A native run accepted a result with disposition `done`. The status
decision set the task to done. The accepted final reply was not
published as a task comment. The run had no publication record (no
`run.presentation.resolved` event and no `presentationDecision`). The
reply was published by background recovery 58 minutes later, after an
unrelated wake reloaded the presentation queue.
**Expected behavior**
The accepted reply is published promptly. A refused or failed attempt is
retried with backoff and never posts twice. If publication fails
permanently, the run and the task show a named failure with an owner.
**Steps to reproduce**
1. Commit an accepted native task result with disposition `done`, with
the final reply not yet visible to presentation recovery.
2. Run the native response presentation worker check once. The attempt
is refused.
3. On `master`, the worker returns `null` and drops the run. Nothing
records the attempt. Nothing retries it until another notification for
that scope.
**Paperclip version or commit**
`master` at
|
||
|
|
07a2941f0c |
fix(recovery): reclaim verified inactive provider caches on full disks (#19192)
Recover from a full-disk workspace save by reclaiming only a verified inactive provider installation under the existing source and maintenance authority. Commit the preparation inventory in Core before changing cache contents. Bind interrupted cleanup to stable filesystem identities and keep small staged generation markers when directory birth times are unavailable. Skip read-only or permission-denied assets and stop freeing blocks once marker space is available. Preserve task files, Git, session history and uncertain action receipts. Only the normal saved-workspace and provider-stop receipts permit continuation. Validated with 446 complete recovery regressions on the repair revision, subsequent focused recovery and transport suites, typecheck/build, and all exact-head hosted CI. Greptile reviewed the final head at 5/5 with no unresolved findings. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
037c58bb22 |
fix(daytona): keep the provider auto-stop strictly after the warm runner idle window (#19199)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A warm remote agent keeps its sandbox running after a turn. The host keeps a warm runner for an idle window (`runnerIdleTimeoutMs`, or the agent's `idleTimeoutMs`) > - The Daytona provider also stops an idle sandbox after `autoStopInterval` minutes. The default is 15 minutes > - On staging, the warm idle window was 900,000 ms (15 minutes). The two windows were equal > - A follow-up turn at the end of the warm window resumed a sandbox that Daytona was stopping. Daytona refused the resume in about 4 seconds, and the run failed at setup > - #19195 added a bounded retry for that refusal. It did not remove the cause > - This pull request makes the two windows non-overlapping by construction. Daytona derives its auto-stop for a warm reusable sandbox from the same warm idle value that the host uses > - The benefit is that a follow-up inside the warm window always finds a running sandbox, and idle sandboxes still stop ## Linked Issues or Issue Description Refs #19195 (bounded retry for a refused resume; this change removes the common cause). **What happened?** On a staging soak, three follow-up runs failed at setup with a refused sandbox resume. Each resume began 15 minutes 8 to 25 seconds after the previous turn released the sandbox. The warm idle timeout was 900,000 ms. Daytona's default `autoStopInterval` is 15 minutes. Nothing in the code kept these two values apart. **Expected behavior** A reusable sandbox that keeps a warm runner must not auto-stop inside the warm idle window or at its edge. The provider backstop must fire after the host's own idle handling, with a margin. Idle sandboxes must still stop. **Steps to reproduce** 1. Configure a Daytona sandbox environment with `reuseLease` and a warm runner with `runnerIdleTimeoutMs: 900000` (or an agent with `lifecycleMode: warm`, `idleTimeoutMs: 900000`). 2. Run a turn. Do not set `autoStopInterval`. 3. Before this change: the sandbox is created with `autoStopInterval: 15`, equal to the warm window. A follow-up near minute 15 races the provider stop. **Paperclip version or commit** `c656b95b6b` (current `master`). ## What Changed - Daytona plugin (`plugin.ts`): add `effectiveAutoStopIntervalMinutes`. For a reusable lease that keeps a warm runner, the effective auto-stop is at least `ceil(warm idle / 1 min) + 5` minutes. The plugin applies the floor only when the host forwards `warmRunnerIdleTimeoutMs`, or when the environment itself selects `runnerLifecycleMode: warm`. An explicit `0` (disabled), a longer configured value, `inherit` with a per-turn agent, per-turn, and non-reusable sandboxes keep the configured value. - Daytona plugin: use the effective value at sandbox creation. On resume, raise a shorter auto-stop on an existing sandbox. This update is best effort and bounded to 5 seconds; the #19195 retry still covers a refusal. The stale-handle refresh threshold keeps the configured interval, so a cached handle is not trusted for longer before the provider confirms the new interval. The warm window is not written to the saved normalized config. - Host (`runner-environment-lifecycle.ts`): add `resolveWarmRunnerIdleTimeoutMsForRun`. It returns the run's effective warm idle window (environment `runnerIdleTimeoutMs`, else the agent `idleTimeoutMs`) for a warm `paperclip_runner` sandbox run. It shares the lifecycle-mode resolution with `resolveRunnerEnvironmentForRun`. - Host (`environment-run-orchestrator.ts`, `environment-runtime.ts`): pass that value as `warmRunnerIdleTimeoutMs` to the sandbox-provider driver on acquisition and on remote recovery resume. The driver adds it to the provider worker config only. Lease identity, reuse matching, and lease metadata are unchanged. - Daytona manifest: describe the warm floor on `autoStopInterval`. - `doc/execution-semantics.md`: document the contract. ## Verification - Red (before the fix, same tests, with the floor disabled): 5 failures in `plugin.test.ts` "warm idle window versus Daytona auto-stop". The staging case failed with `expected 900000 to be greater than or equal to 1200000`. The resume case failed because `setAutostopInterval` was not called. - Red (host): the new `environment-runtime.test.ts` case first failed because the provider worker config had no `runnerIdleTimeoutMs`. The service wrapper dropped the field. The fix forwards it. - Green: `npx vitest run --config vitest.config.ts` in `packages/plugins/sandbox-providers/daytona`: 563 passed. This includes a setter that never settles (resume completes after the 5-second bound) and `inherit` with no forwarded window (explicit `autoStopInterval: 2` stays 2). - Green: `npx vitest run src/services/runner-environment-lifecycle.test.ts src/__tests__/environment-runtime.test.ts src/__tests__/environment-run-orchestrator.test.ts` in `server/`: 242 passed. This includes recovery resume forwarding the admitted warm window. - The regression tests assert the relationship `autoStop >= warm idle + margin` for several warm windows (125 s, 300 s default, 900 s, 1,800 s) and for an explicit `autoStopInterval: 15`. They are not constant checks. - `node scripts/typecheck-low-memory.mjs` passes. `pnpm typecheck` in the Daytona package passes. ## Risks - A warm reusable Daytona sandbox now stays running up to the warm window plus 5 minutes after its last activity, instead of 15 minutes. With a 15-minute window this is 20 minutes. Cost is bounded and idle sandboxes still stop. - The resume-time `setAutostopInterval` call is best effort. If it fails, the old interval stays and the #19195 retry still applies. - Other providers are unchanged. They receive one extra host-owned config key (`warmRunnerIdleTimeoutMs`) only for warm `paperclip_runner` runs, and they ignore unknown keys. - No lease identity change, so existing retained sandboxes are still reused. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |