mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
bf67e2fbd909fc62e9efbfe70147fd83935d3e6f
3611
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicky LeachandClaude Opus 5.5
|
990de1e0bf |
perf(environments): release a stopped plugin task once (#20203)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugin environment drivers with `supportsTasks` run agents as provider tasks, and warm runs keep a parked task for the next run > - A follow-up run can find that its parked task already stopped. Core then retires that task before it acquires a new one > - Core sent `stop`, `status`, and `complete` in the settle step. Then the plugin release hook sent `stop`, `status`, and `complete` again for the same task. Every call was on the critical path of the new run > - This pull request removes the repeated calls and keeps every call that does work no other call does > - The benefit is a faster start for a run whose parked task already stopped ## Linked Issues or Issue Description No issue exists. This follows the enhancement template. **What existing behavior does this improve?** The release of a plugin task lease that has already stopped. This occurs when a follow-up run finds that its parked warm task stopped. **Subsystem affected** Server: plugin environment drivers and the plugin Runner task lifecycle. **Current behavior** Core sends `stop`, `status`, and `complete` in the settle step. Then the plugin release hook sends `stop`, `status`, and `complete` again for the same task. Core also sends `stop` after resume already saw that the task stopped. On a hosted test deployment, 8 of 40 follow-up runs took this path. From the resume to the new acquire, these runs spent P50 1014 ms and P90 1080 ms. These 8 runs are the full overall P90 (1275 ms). **Proposed behavior** Core does not call the release hook when settle confirmed the stop and completed the same task. After a resume with no lease, Core sends `status` first, and sends `stop` only when `status` does not confirm the stop. **Reason and benefit** The repeated calls do no new work. Removing them saves about 355 ms, plus the `stop` share of about 187 ms, for each such run. These numbers are modeled from trace timings. **Breaking changes** None for the plugin API. A task driver must release task-scoped resources in its `status` or `complete` handling. The doc now says this. **Additional context** The settle `status` stays. Providers release task storage there, and it records the stop that gates the credential writeback and `complete`. ## What Changed - `settleDispatchedEnvironmentTask` now returns `"completed"` when it confirmed the stop and completed the task in this call, `"stopped"` when the lease was already settled, and `false` otherwise. - The plugin driver release does not call `environmentReleaseLease` when settle returned `"completed"`. In all other cases it calls the hook as before. - When `environmentResumeLease` returns no provider lease, the release sends `status` first. It sends `stop` (and a second `status`) only when that `status` does not confirm `executionStopped: true`. Startup and startup-cleanup leases keep the old `stop` first order. - The cleanup sweep (`retryPendingSandboxTeardown`) and `destroyRunLease` do not change. A settle failure still leaves the lease in `pending_cleanup`, and the sweep runs `stop`, `status`, `complete`, and the release hook. - `doc/plugins/PLUGIN_RUNNER_EXECUTION.md` tells drivers about the new order and that task-scoped resources must be released in `status` or `complete`. ### Dispatch count for a stopped parked task (resume to new acquire) | | Calls | | --- | --- | | Before | resume, stop, status, (credentials), complete, release hook (stop, status, complete), acquire: 9 provider calls (10 with Codex writeback) | | After | resume, status, (credentials), complete, acquire: 4 provider calls (5 with Codex writeback) | The modeled saving is about 540 ms for each such run: about 355 ms for the repeated release, and about 187 ms for the settle `stop` and `status`. This pull request keeps the settle `status`, so the real saving is about 355 ms plus the `stop` share of the 187 ms. These numbers are modeled from trace timings. They are not measured on this branch. ### What covers each removed call - Release hook `stop`, `status`, `complete`: settle already sent `stop` and `status` and checked `executionStopped: true` for the same task id (`lease.providerLeaseId`), then sent `complete`. The skip uses settle's success as its condition, not the code path. - Settle `stop` after a resume with no lease: the next `status` must report `executionStopped: true`. If it does not, Core sends `stop` and `status` as before. ### What each kept call protects - Settle `status`: providers release task storage and cancel preparations here. It also writes `runnerTaskExecutionStopped`, which the credential writeback and the `complete` bookkeeping need. - Credential writeback: Codex can rotate its token. The writeback must land before the new task installs credentials. It still runs inside `complete`, before the new acquire. - Settle `complete`: it retires every run lease that owned the task and clears the dispatch marker. - Release hook when settle did not complete a dispatched task in this call: the provider may still hold the lease. ## Verification - `pnpm --filter @paperclipai/shared --filter @paperclipai/plugin-sdk build` - `pnpm --filter @paperclipai/server typecheck` - `npx vitest run src/__tests__/plugin-warm-tasks.integration.test.ts src/__tests__/plugin-task-credential-writeback.test.ts src/__tests__/environment-task-runtime.test.ts` - Related suites: `environment-runtime`, `plugin-runner-task.integration`, `task-creation-recovery.integration`, `heartbeat-plugin-environment`, `native-startup-cleanup-recovery`, `plugin-environment-driver-ready-recovery`, `environment-task-authority.integration`, `native-workspace-warm-reuse`, `remote-workspaces.integration`, `legacy-workspace-restore-recovery`, `plugin-task-source-recovery.integration`, `managed-plugin-environments`. Locally, all of these pass except `heartbeat-plugin-environment` "inherits the instance default environment across companies…". That test also fails locally with the `origin/master` service files, so this change does not cause it. The existing `plugin-runner-task.integration` same-run replacement test now expects no release hook after a completed settle. New tests: - The stopped parked path sends exactly resume, `status`, `complete`, acquire. The `status` goes to the old task. - A resume with no lease for a task that still runs sends `status`, `stop`, `status`, `complete`. - A failure at `status`, `complete`, or `stop` leaves the lease in `pending_cleanup`, and the sweep then sends `stop`, `status`, `complete`, and the release hook. - With an observed stop, the Codex writeback runs before `complete`. A writeback failure leaves the lease for cleanup, and the retry writes back and completes. - A startup-cleanup lease still sends `stop` first. ## Risks - A third-party task driver that releases resources only in `onEnvironmentReleaseLease` would not get that call after a completed settle. The doc now states the requirement. The cleanup sweep still calls the hook after any failure. - A driver that returns no lease from resume while the task still runs costs one extra `status`. The behavior stays correct. - No migration. No API change for plugins. ## 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 - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky LeachandClaude Opus 5.5
|
9046884811 |
perf(native-runtime): claim a parked Runner while the follow-up run prepares (#20195)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Paperclip Runner executes native agent runs. A host can park an idle
Runner (release it and freeze it) between runs, and resume it for the
next run on the same task.
> - A follow-up run that reuses a parked Runner must claim it first. The
claim is a bootstrap ticket, a checkpoint flush, and the plugin resume
that thaws the Runner.
> - Core made the claim only inside `attachRun`, after all of its
per-run preparation. The claim does not depend on that preparation, so
the two ran in series.
> - This pull request starts the claim while Core still prepares the
run. `attachRun` then waits for the claim that is already in progress.
> - The benefit is a shorter start for every follow-up run on a parked
Runner. The model below gives about 285 ms per run at P50.
## Linked Issues or Issue Description
No public issue exists. The issue is described inline.
**What happened?**
A follow-up run that reuses a parked Runner claims the Runner only in
`attachRun`. Core calls `attachRun` after backend setup: runner
environment, assigned MCP tools, tool authority and the plugin task
owner. The claim waits for all of this, but it uses none of it.
**Expected behavior**
The claim runs at the same time as backend setup. Everything that needs
the new run's tool authority stays at attach.
**Steps to reproduce**
1. Run a Codex or Claude task on a host that parks Runners between runs.
2. Let the run finish and the Runner park.
3. Send a follow-up. Before this change, the plugin resume starts only
after backend setup ends.
**Paperclip version or commit**
`master` at
|
||
|
Nicky LeachandClaude Opus 5.5
|
b4a77b8915 |
perf(plugins): read runtime identity in one statement without a row lock (#20192)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Lifecycle plugins read the agent runtime identity through the plugin
host
> - The identity read used the full-state path. That path locks the
agent row and runs 11 statements
> - Plugins call the identity read many times as a freshness fence, so
each round trip adds to the run latency
> - This pull request makes the identity read one statement on the
request pool, without a row lock, and loads instance secrets with their
versions in one statement
> - The benefit is fewer round trips per read (11 to 4) and no wait
behind writers that hold the agent row
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The plugin host read of the agent runtime identity
(`pluginAgentRuntimeIdentity`).
**Subsystem affected**
Server: plugin agent runtime state and instance secrets
(`server/src/services/plugin-agent-runtime-state.ts`,
`server/src/services/secrets.ts`).
**Current behavior**
The identity read shares the full-state transaction. It opens a
transaction on the file-storage pool and locks the agent row with `FOR
UPDATE`. It reads the plugin, the company settings and the latest
lifecycle event in separate statements. Then it reloads each instance
secret and its version one at a time. One read costs 11 statements. It
also waits behind any writer that holds the agent row, for example
managed file publication.
**Proposed behavior**
The identity read is one statement on the request pool. The statement
joins the plugin and the company settings, and reads the latest
lifecycle event id in a subquery. The agent fields, the authorization
inputs and the revision come from one snapshot. Instance secrets load
together with their latest versions in one statement. One read costs 4
statements.
**Reason and benefit**
A resumed agent run makes about 7 identity reads on its critical path in
a hosted deployment. With a 4 ms round trip, the modeled saving is about
330 ms per run at P50.
## What Changed
- `pluginAgentRuntimeIdentity` reads the agent, plugin, company settings
and latest lifecycle event id in one statement on the request pool. It
does not take a row lock.
- The authorization check moves into `assertOwningLifecyclePlugin`. The
identity path and the full-state path both use it, so both fail with the
same forbidden error.
- A missing agent still returns `null` before authorization runs.
- The full-state path (`readState`) keeps its transaction and its `FOR
UPDATE` lock. It no longer has an identity-only branch.
- `instanceSecrets.read` loads each secret with its latest version
through a left join in one statement. `resolveSecretValueInternal`
accepts these preloaded rows and skips the reload when the secret and
the version match. It still reloads when they do not match.
- The two audit writes per resolved secret run concurrently, as before.
- Tests:
- `agent-runtime-hooks.test.ts` compares the identity read with the full
read across revisions, lifecycle changes and secret changes, and across
every forbidden case. It confirms that forbidden reads record no secret
access. It confirms that the identity read does not wait for a held
agent row lock.
- `agent-instance-secrets.test.ts` confirms that a read of rotated
secrets uses one select and returns the latest values. It confirms that
each read records an access event. It confirms that a revoked version
fails with `version_inactive` and a missing version fails with
`version_missing`.
### Why dropping the lock is safe
The lock exists for the full-state path. That path projects instruction
files under the agent lock. The identity path does no projection. It
also released the lock before it returned, so no caller could rely on
the lock.
### Unchanged behavior
- Authorization: the same plugin, settings, capability and
required-plugin checks, with the same forbidden error.
- Decryption: the same provider resolution from the version material.
- Scope checks and version status checks: the same checks on the same
rows.
- Audit: the same `lastResolvedAt` update and access event for each
resolved secret.
## Verification
- `pnpm --filter @paperclipai/shared --filter @paperclipai/plugin-sdk
build`
- `pnpm --dir server run typecheck`: pass.
- Targeted tests: pass (6 files, 174 tests).
```
pnpm --dir server exec vitest run \
src/__tests__/agent-runtime-hooks.test.ts \
src/__tests__/agent-instance-secrets.test.ts \
src/__tests__/project-instance-secrets.test.ts \
src/__tests__/secrets-service.test.ts \
src/__tests__/agent-runtime-assignments.test.ts \
src/__tests__/agent-runtime-skills.test.ts
```
- CI runs the full suite.
### Benchmark
A local benchmark ran against the embedded test Postgres. A delay proxy
simulated network round trips.
| Condition | Before | After |
|---|---|---|
| Statements per read | 11 | 4 |
| P50, no added delay | 2.38 ms | 1.47 ms |
| P50, 4 ms round trip | 104.0 ms | 32.2 ms |
| P50, 4 ms round trip, prepared statements | 51.9 ms | 16.3 ms |
The new read costs about 0.3× the old read in round trips. In a hosted
deployment, a resumed agent run makes about 7 of these reads on its
critical path. The modeled saving is about 330 ms per run at P50.
## Risks
- Low risk.
- The identity read no longer runs in the same snapshot as a concurrent
writer that holds the agent row. It can return the state from before
that write commits. The `revision` field still comes from the same
snapshot as the agent fields, so callers that use it as a freshness
fence see a consistent pair.
- The identity read now runs on the request pool, not the file-storage
pool. It adds one short query to the request pool and removes one
transaction from the file-storage pool.
- The preloaded secret path applies only when the secret and the version
match the rows from the join. Any other case falls back to the existing
reload.
- No migration. No API 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 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>
|
||
|
DottaandClaude Opus 5.5
|
3964ee1fca |
fix(native): a governance pause Stop with no turn result stays a cancellation (#20186)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop pauses the company; the pause's lifecycle Stop cancels the agents' running native turns > - When a Codex turn is interrupted this way, it ends without a semantic result > - The native executor treated that missing result as a retryable interruption, so it rewrote the cancelled run to `failed` and scheduled an exact resume > - The pause held that recovery, and Raise budget and resume continues only runs that are still cancelled, so the task stayed blocked with no new run > - This pull request makes the executor treat an acknowledged governance pause Stop like a person's Stop: the run stays cancelled > - The benefit is that Raise budget and resume continues the task (#20144), with no manual Retry ## Linked Issues or Issue Description Refs #20144 (resume continues budget-stopped work), #20143 (governance pause stops are not failures), #20113 **What happened?** Restart-chaos wave w-20261109T051242-s9802 (H1, seed 9802), task t03 (Codex, fault K2). The company budget crossed its hard limit at 05:19:39.9. The pause's lifecycle Stop ("Agent lifecycle stop requested") cancelled run 59e976bc at 05:19:40.05, and Codex acknowledged the interrupt. The turn ended without a semantic result, and at 05:19:41.27 the executor rewrote the run to `failed` / `native_session_interrupted`, with an agent-owned `active_run_watchdog` recovery (exact checkpoint resume). The task went to `blocked`. After `raise_budget_and_resume` at 05:20:20, no run started for 14 minutes. **Expected behavior** A pause's Stop is intentional. The run stays cancelled with its pause record, and resuming continues the task once. **Steps to reproduce** Pause a company for budget while a Codex native turn runs, so the Stop interrupts the turn before it records a result. Before this change, the run becomes `failed` with a scheduled exact resume instead of staying `cancelled`. ## What Changed - `native-session-executor.ts`: the no-semantic-result branch now also accepts an acknowledged governance pause Stop (`isGovernancePauseStop` with an acknowledged native Stop receipt) as an intentional stop. It settles the coordinator as `native_retry_cancelled` and raises `NativeCancellationPendingRecoveryError`, as it already does for a person's Stop and for a reassignment. The run is not rewritten to `failed`, and no recovery action is created. - The Stop's cancellation record is written by its fence before dispatch, so the check does not depend on whether the terminal write has landed. ## Verification - `native-session-executor.test.ts`: new cases for "Agent lifecycle stop requested" with a `pauseStop`, and for "Cancelled due to budget pause". Each was red before the fix (it reached the retryable-failure write) and is green now. A control case shows that a system lifecycle Stop without `pauseStop` (termination) still takes the retry path. - Full `native-session-executor.test.ts`: 1309 passed. Server typecheck (`typecheck-layers`, low-memory): no errors. - Not run: the H1 chaos rerun. Phase 3 runs it after merge. ## Risks - A governance pause Stop that lands during a turn no longer schedules an exact resume of that turn. The task continues on resume through a new Retry (#20144) instead. - Termination and cleanup use the same lifecycle Stop reason. They are excluded because only the pausing path records `pauseStop`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
7c65a2f479 |
fix(authz): link plugin settings and say reply now on board-only denials (#20182)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents run with agent credentials, and plugin management routes are board-only > - A board-only denial tells the agent the denial is final and links the board page that holds the control, when the page is known (#20151) > - Plugin routes had no known page, so the denial said "link a page only if you already know its route" > - In a chat eval, an agent asked to finish configuring a plugin got this denial and kept searching the API for about two minutes, so it did not answer in time > - This pull request names the plugin manager, or that plugin's settings page, in the denial > - The benefit is that the agent can hand the step to the person with a working link right away ## Linked Issues or Issue Description Refs #20151 **What happened** An agent that called a board-only plugin route (for example `GET /api/plugins` or `POST /api/plugins/:pluginId/config`) got the generic remediation with no page. In the `plugins.configure-plugin` chat eval, the agent kept calling `search_api` for another plugin route after the 403 and did not reply within the 150-second window. **Expected behavior** The denial names the page where a board user does the step: the plugin manager (`/<prefix>/company/settings/instance/plugins`) or the plugin's settings page (`/<prefix>/company/settings/instance/plugins/<id or key>`). No link is given when the operator hides plugin management (`PAPERCLIP_HIDDEN_SETTINGS=instance.plugins`). **Steps to reproduce** 1. Install a plugin. 2. As an agent, call `POST /api/plugins/<pluginId>/config`. 3. See a 403 `board_access_required` remediation without a plugin page. ## What Changed - `server/src/routes/authz.ts`: board-only denials on `/api/plugins` routes link the plugin manager, or the plugin's settings page when the path names a plugin UUID or key. The plugin settings page accepts either. No link when plugin management is hidden. - `server/src/routes/authz.ts`: every agent board-only denial now says to stop working on the denied step and reply now. With the plugin link alone, the eval agent still searched the API for two more minutes. - `server/src/routes/authz.ts`: any manifest-format plugin key (including dot-free keys such as `my-plugin`) gets a direct link; plugin manager endpoints (`examples`, `install`, `tools`, `ui-contributions`) link the plugin list. - `ui/src/pages/PluginSettings.tsx`: custom settings slots match by plugin key as well as plugin id, so a page opened by key shows the plugin's own settings. - `server/src/__tests__/board-access-denial.test.ts`: cases for list, install, config by id, disable by key, a dot-free key, manager endpoints, and the hidden-settings case. `ui/src/pages/PluginSettings.test.tsx`: opening the page by key mounts that plugin's settings slot. - Merged `master`, keeping its project access-members denial cases. ## Verification - `cd server && npx vitest run src/__tests__/board-access-denial.test.ts src/__tests__/agent-permissions-routes.test.ts src/__tests__/environment-routes.test.ts src/__tests__/execution-workspace-runtime-lease-route.test.ts`: 241 passed after merging master. - `cd ui && npx vitest run src/pages/PluginSettings.test.tsx`: 9 passed; the new key-route case fails without the `PluginSettings` change. - `cd server && pnpm typecheck`: passes. ## Risks - Low risk. Only the remediation text of agent board-only denials on `/api/plugins` routes changes. Board users and authorization do not change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
492316e576 |
fix(plugins): accept secret_ref bindings in string secret-ref config fields (#20181)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Plugins declare their settings in an `instanceConfigSchema`, and the
server validates each save against that schema
> - Plugin credential fields use `type: "string", format: "secret-ref"`
> - The settings form, the plugin secrets handler and the plugin SDK all
use the `{ type: "secret_ref", secretId, version }` binding object for
these fields, and the secrets handler rejects a bare secret id
> - Schema validation ran on the raw config, so the binding object
failed with "must be string" and no value could be saved
> - This pull request validates the binding's secret id in place of the
binding object
> - The benefit is that an operator can bind a company secret to a
plugin credential field from the settings page or the API
## Linked Issues or Issue Description
**What happened**
`POST /api/plugins/:pluginId/config` returned 400 "Configuration does
not match the plugin's instanceConfigSchema" with
`{"field":"/apiKey","message":"must be string"}` when the config held `{
"apiKey": { "type": "secret_ref", "secretId": "<uuid>", "version":
"latest" } }` for a field declared `{ "type": "string", "format":
"secret-ref" }`. A bare UUID string passes the schema but the secrets
handler then rejects it ("must use { type: \"secret_ref\", secretId,
version? }"). So no value could be saved for a required secret-ref
field.
**Expected behavior**
The binding object that the settings form submits and that
`ctx.secrets.resolve()` accepts is saved. Other schema rules still
apply.
**Steps to reproduce**
1. Install a plugin whose manifest has
`instanceConfigSchema.properties.apiKey = { "type": "string", "format":
"secret-ref" }`.
2. Create a company secret.
3. On the plugin settings page, pick that secret for the field and click
Save Configuration (or POST the binding object to
`/api/plugins/:pluginId/config`).
4. See the 400 "must be string" response.
Found while getting the chat eval cases `plugins.configure-plugin` and
`plugins.disable-plugin` to run. Their setup and the human's
settings-page save both hit this error.
## What Changed
- `validateInstanceConfig` now replaces each valid `secret_ref` binding
object at a schema `format: "secret-ref"` path with its secret id before
Ajv validates. The stored config and the config sent to the worker do
not change.
- The walk follows array `items` (single and tuple) and objects inside
arrays as well as object properties and `allOf`/`anyOf`/`oneOf`, so
secret pickers inside array fields can save too.
- A binding is accepted only when `envBindingSecretRefSchema` (the
secrets handler's own parser) accepts it, so validation and the secrets
handler agree on what is a valid binding.
- Malformed bindings and objects in ordinary string fields still fail
validation.
- Added `server/src/__tests__/plugin-config-validator.test.ts`.
## Verification
- `cd server && npx vitest run
src/__tests__/plugin-config-validator.test.ts
src/__tests__/plugin-routes-authz.test.ts
src/__tests__/plugin-sandbox-provider-config-validation.test.ts`: 48
passed.
- The new test fails on `master` without the fix (3 of 3 fail) and
passes with it.
- `cd server && npx vitest run
src/__tests__/plugin-config-validator.test.ts`: 6 passed after the
array/parser follow-up; the 3 follow-up tests fail on the previous
commit.
- `cd server && pnpm typecheck`: passes.
## Risks
- Low risk. Only configs that held a valid binding object at a declared
secret-ref path change result, from a 400 to accepted. The secrets
handler still checks that the secret belongs to the company.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
7d92b6a7cd |
fix(api): use a neutral catalog pointer in the call_api protocol refusal (#20191)
## Thinking Path
- Paperclip agents reach REST routes through the `call_api` tool;
protocol endpoints (MCP, OAuth, WebSocket) are refused there with
guidance.
- #20170 changed that refusal to tell the agent the endpoint's catalog
description says who can use it.
- Review on #20170 found that some protocol endpoints' descriptions (for
example the WebSocket entries) do not say who may call them, so the
refusal could send the agent to look for information that is not there.
- The fix (
|
||
|
DottaandClaude Opus 5.5
|
7086019d4b |
fix(api-catalog): say Agent Chat is always on instead of offering a retired toggle (#20169)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents learn what the API can do from the API catalog at `/api/openapi.json` > - PR #19360 made Agent Chat standard. The `enableAgentChat` key is now retired: it always reads true, and the app ignores written values > - The catalog still lists `enableAgentChat` as an ordinary boolean on the experimental settings operations, with no note that it is retired > - An agent that was asked to "Enable Agent Chat on this instance" read the catalog and told the person to turn on a toggle in Instance Settings → Experimental. That toggle no longer exists > - This pull request adds a note to both experimental settings operations: Agent Chat is always on, the key is retired, and agents must tell people it is already on > - The benefit is that agents give a correct answer and do not send people to a control that is not there ## Linked Issues or Issue Description Bug report (no public issue exists). **What happened** An agent was asked "Enable Agent Chat on this instance." It read `PATCH /api/instance/settings/experimental` in the API catalog and found `enableAgentChat`. It replied that an admin must open Instance Settings → Experimental and turn on Enable Agent Chat. That toggle was removed in #19360, and Agent Chat is already on for everyone. **Expected behavior** The agent says that Agent Chat is already on (the person is using it), and that nothing must be enabled. **Steps to reproduce** 1. Open a chat with an agent on any current instance. 2. Send "Enable Agent Chat on this instance." 3. The agent reads the experimental settings operation in the API catalog and sends you to a settings toggle that does not exist. Refs #19360 ## What Changed - `server/src/routes/openapi.ts`: the GET and PATCH `/api/instance/settings/experimental` operations now say that Agent Chat is always on, that `enableAgentChat` is a retired compatibility key that always reads true and ignores written values, and that agents must tell people it is already on instead of sending them to a toggle. - `server/src/__tests__/openapi-routes.test.ts`: a regression test checks this note on both operations. - The note says Agent Chat "needs no setup" instead of "there is nothing to enable", so it does not match unrelated keyword searches. The runner API no-results test (`nothing-zzzzzzzzzz`) had matched the word "nothing". ## Verification - `cd server && npx vitest run src/__tests__/openapi-routes.test.ts src/services/native-runtime/runner-api.test.ts` passes (2108 tests), and the server `tsc --noEmit` passes. - The chat eval `instance-operations.instance-settings` found this defect. Its run log shows that the agent reasoned from the catalog schema ("The setting is `enableAgentChat` on PATCH /api/instance/settings/experimental"). ## Risks - Low risk. This changes only catalog description text. It does not change behavior, validation or the schema. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution, used through 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> |
||
|
DottaandClaude Opus 5.5
|
b07399c241 |
fix(authz): link the project's access settings when an agent cannot share a project (#20176)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Some actions are board-only. When an agent calls one, the 403 denial tells it that the denial is final and, when known, links the board page that holds the control > - Project sharing (`/api/projects/:id/access-members`) had no known page, so the denial told agents to link a page only if they already knew its route > - An agent asked to "Let Riley see the acquisition project" got this denial. It told the person to look for "access/sharing settings" and gave no link, so the person had to search for the control > - This pull request maps project access-member routes to the project's Configuration page, where the Manage access control is > - The benefit is that agents give people a direct link to the control that shares a private project ## Linked Issues or Issue Description Bug report (no public issue exists). **What happened** An agent tried to add a person to a private project. `POST /api/projects/:id/access-members` returned 403 "Board access required" without a page link. The agent told the person to find the project's access or sharing settings in the web app and gave no link. **Expected behavior** The denial links the project's Configuration page (`/<prefix>/projects/<id>/configuration`), where Manage access is. The agent gives that link to the person. **Steps to reproduce** 1. Create a private project and give an agent access to it. 2. In chat, ask the agent to let another company member see the project. 3. The agent's access-member request is denied, and its reply has no link to the sharing control. Refs #20151 ## What Changed - `server/src/routes/authz.ts`: board-only denials for `/projects/:id/access-members` (and its member subpaths) link "the project's access settings" at `/<prefix>/projects/<id>/configuration`. The link uses the resolved route project ID, so short references such as `/api/projects/acquisition/access-members` also get the link. Without a resolved UUID, the denial falls back to the generic instruction. - `server/src/__tests__/board-access-denial.test.ts`: cases for add and remove access-member denials, plus a short-reference case that goes through a route param resolver. ## Verification - `cd server && npx vitest run src/__tests__/board-access-denial.test.ts` passes (17 tests). The short-reference test fails on the previous matcher. The server `tsc --noEmit` passes. - The chat eval `projects.share-project` found this gap: in one run the agent linked the project, and in the next run it gave no link. ## Risks - Low risk. This changes only the remediation text in an agent's 403 response. It does not change authorization. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution, used through 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> |
||
|
DottaandClaude Opus 5.5
|
2fc0293586 |
fix(api): tell agents only the person can revoke assistant connections, and where (#20170)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - People connect external assistants to Paperclip, and each person
manages their own assistant connections in the browser
> - Chat agents find API routes through `search_api` and call them with
`call_api`
> - The catalog entries for `/api/mcp/connections` did not say who can
use them or which page holds the control
> - `call_api` refuses these routes as protocol endpoints, and its
message did not say the refusal is final
> - A chat agent asked to "disconnect my old assistant" searched for an
agent route for its whole turn and did not reply
> - This pull request names the person-only boundary and the page in the
catalog, and makes the refusal final
> - The benefit is that the agent replies at once with a link to the
page where the person revokes the connection
## Linked Issues or Issue Description
No public issue exists. This is a bug report.
**What happened**
A person asked the chat agent: "Disconnect my old assistant, but leave
the website task running." The agent found `DELETE
/api/mcp/connections/{id}`. `call_api` refused the list route with "This
endpoint requires its existing protocol client; call_api supports REST
only". The agent then searched the API for other routes (dot bindings,
agents, "mcp", "assistant") for two minutes. The turn timed out with no
reply. The agent wrote in its reasoning that there "must be a supported
path".
**Expected behavior**
The agent tells the person that only they can revoke the connection,
links the Assistant connections page, and says the delegated task keeps
running.
**Steps to reproduce**
1. Connect two assistants to a company as one person. Let the first
assistant create a task for an agent.
2. In chat, ask the CEO: "Disconnect my old assistant, but leave the
website task running."
3. See that the agent searches the API until the turn ends and does not
reply.
## What Changed
- `server/src/routes/openapi.ts`: the `GET /api/mcp/connections` and
`DELETE /api/mcp/connections/{id}` entries now have a description. It
says only the signed-in person can list or revoke their assistant
connections, links `[Assistant
connections](/<prefix>/apps/assistant-connection)`, notes that the
connected assistant itself can revoke its own token through OAuth token
revocation, says not to search for another route, and says that tasks
the assistant already created keep running.
- `server/src/services/native-runtime/runner-api-client.ts`: the
`call_api` protocol refusal now says no other API route substitutes for
the endpoint. It points to the catalog description only when that
operation has one, so undocumented protocol routes such as `POST
/api/mcp/project-tools` do not send the agent to an empty description.
- `server/src/__tests__/assistant-connections-guidance.test.ts`: new
regression test. It uses the exact search queries from the failed run
and covers the refusal with and without a description.
- `server/src/__tests__/public-mcp.test.ts`: agent and non-session board
actors get 401 on listing and revoking assistant connections.
## Verification
- `cd server && pnpm exec vitest run
src/__tests__/assistant-connections-guidance.test.ts
src/__tests__/primary-agent-guidance.test.ts
src/__tests__/openapi-routes.test.ts
src/services/native-runtime/runner-api.test.ts` passes (2111 tests).
After the review fixes,
`src/__tests__/assistant-connections-guidance.test.ts
src/__tests__/public-mcp.test.ts
src/services/native-runtime/runner-api.test.ts` passes (2157 tests), and
the server `tsc --noEmit` passes.
- The new test fails on `master` without the change (2 failures) and
passes with it.
- The end-to-end check is the
`assistant-connections.revoke-personal-grant` chat eval with this commit
cherry-picked.
## Risks
- Low risk. The change is catalog text and one error message. No route,
permission or data behavior changes.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Devin FoleyandPaperclip
|
9b71d03215 |
fix: recover interrupted native preparations and unblock upgrades (#20177)
## Thinking Path > - Paperclip manages AI-agent companies and their work. > - A native run prepares its workspace before a runner PID or checkpoint exists. > - A server crash can interrupt that stage and leave its controller on another host. > - Recovery needs a durable owner proof, a delivered recovery wake, and the saved execution contract. > - The old paths missed controller lease registration and boot proof, skipped checkpoint-free preparations, and counted retained work as live upgrade blockers. > - This PR connects preparation to the existing controller lease and recovery paths, and separates retained work from the upgrade census. > - The result is bounded recovery without a second executor or loss of saved work. ## Linked Issues or Issue Description **What happened?** A native preparation interrupted by a server crash could remain running indefinitely with no runner PID or checkpoint. Native controller stamps did not wake the shared instance lease. That lease was disabled on volume-backed Railway services, and native takeover omitted the boot binding needed for its cross-host proof. Scoped recovery selected checkpoint-backed runs but skipped these preparations. The upgrade probe then counted the retained row as current execution and could block every later upgrade. **Expected behavior** Track the preparation's controller before a runner starts. Recover it through the normal classifier after proving its owner stopped or fencing that boot. Keep live or unprovable owners held. Deliver recovery wakes to preparations without checkpoints. Preserve unresolved historical work and report it separately from current execution during upgrade observations. **Steps to reproduce** 1. Save an observed native preparation with no runner process, result, provider-start evidence or checkpoint. 2. Replace its controller with a process in another PID space. Let its owner lease expire and send a scoped recovery wake. 3. Before this fix, recovery skips the checkpoint-free preparation or cannot prove its foreign controller stopped. Upgrade readiness continues to count that old row as a live blocker. Related work: #18037, #19651, #19868, and #20168. The foreign-PID guard from #19868 remains. The queued first-turn input fix from #20168 is included through the current base. ## What Changed - Register the existing instance lease when native execution and restart claims are written. Keep passive identity reads free of ownership effects. Keep that lease enabled on volume-backed Railway services unless explicitly disabled. - Preserve the existing PID/start-time proof within the same readable PID space. Bind foreign or unknown-space takeover to its saved boot and last write. A live lease or grace period keeps the run held with a scheduled check. Repeated evidence waits preserve their timestamp. - Include explicitly fenced, checkpoint-free preparations in the existing finite recovery candidate inventory. Normal launch, checkpoint, provider evidence and admission checks still decide whether work may resume. - Expose the existing board stop-confirmation operation for historical foreign-host owners. Bind confirmation to the exact run, boot, PID, process start and host. Reject changed ownership. - Report narrowly defined old, unowned preparations as `retainedBlockedRuns` outside the live upgrade census. Local executions, pending admission, drain state, truncation and positive checkpoint proof retain their barriers. - Update the execution contract and add PostgreSQL, real-route, race and preservation regressions. ## Verification - Genuine red/green regressions cover the old upgrade census, missing native lease registration, the volume-backed default, foreign-host confirmation, and scoped recovery skipping checkpoint-free preparation. Test bodies ran against real PostgreSQL before their corresponding production changes. - Combined verification before the final idle correction (head `055be6c84d9`): **459 passed, 22 skipped, 11 files passed**. Covers controller lease/identity, native restart classification and integration, host confirmation, heartbeat recovery, instance route authorization, upgrade readiness, and queued first-turn input. The focused command used `pnpm exec vitest run` with those 11 suites and `--maxWorkers=1 --no-file-parallelism`. The 22 skipped cases require the Rust debug fake-Codex fixture, which was not built locally; the real-process CI coverage must pass. - `pnpm --filter @paperclipai/server typecheck`: passed on head `055be6c84d9b058064daad0af71bb5d893ab9529`. - Actual route proof uses the real heartbeat service and PostgreSQL. Unauthorized access returns 403. The retained run and owner records remain unchanged. Recovery integration admits one bootstrap claim across two replacement controllers, keeps a live owner held, and preserves local execution. - `pnpm verify`: exit 2, requests broad CI because the execution contract changed. Full local suite not run. Current-head CI must supply broad coverage. - Local source checkout on macOS ARM64, Node v25.6.1, pnpm 9.15.4. Tests use isolated PostgreSQL. No live provider restart is claimed by those fixtures. - CI found an idle-query regression in the first version of the lease wiring. Two focused PostgreSQL regressions reproduced the misplaced identity-read notification and the missing claim notification. The fix moves registration to actual ownership writes. `env -u npm_execpath pnpm exec vitest run server/src/__tests__/maintenance-server-routes.test.ts --maxWorkers=1 --no-file-parallelism`: **17 passed**, including 90 real seconds and two qualified hourly windows with no extra idle queries. An initial local setup attempt inherited npm metadata and did not run tests; clearing that metadata selected the correct pnpm helper. Three focused controller/native suites also passed **168 tests, 22 fixture-gated skips**. Server typecheck passed after the correction on head `4a527326dcd5b3a9e286f0e4c3e761d3056af4d9`. - The older CI server shard also exposed a same-space graceful-restart regression. The new boot fallback had overridden local process-exit proof. Seven suspension cases and a focused unit regression reproduced it; the fix limits boot fallback to foreign or unknown PID spaces. Fresh verification on `acf1065d98150210c1e227c7f3b05a1eb489730b`: **251 passed, 22 fixture-gated skips, 4 files passed**, including all **40** graceful-restart suspension tests. Server typecheck passed. The local regression failed before this correction, and the same-space and foreign-space paths pass together after it. - Current-head CI completed successfully on `acf1065d98150210c1e227c7f3b05a1eb489730b` (run 42568109784). Greptile reviewed that exact head at 5/5 with no unresolved review threads. - Secret and private-data scans passed. No workflow, lockfile or migration changes. ## Risks - The instance lease now applies to volume-backed Railway services. Its existing 30-second renewal, quiescence, grace and self-fence behavior applies while work is held. Idle servers retain the existing no-query behavior. An explicit `PAPERCLIP_CONTROLLER_INSTANCE_LEASE=off` remains available. - A controller lease proves only the server owner's state. It does not prove an agent process stopped or authorize replay, resource release or volume removal. The existing classifier and provider/runner checks remain responsible for those decisions. - Previous-release records keep their shape and saved state. A missing boot lease stays unprovable and requires the scoped stop-confirmation path. A changed confirmation target is rejected. - The upgrade observation does not recover a held task by itself. Source tests do not qualify a hosted replacement. Deployment and the affected task's useful result must be verified separately. - This repairs recovery after a crash; it does not claim to identify or fix every source of heap exhaustion. ## Model Used OpenAI `gpt-6.1-sol`, xhigh reasoning effort, through Codex in T3 Code, with repository, shell, and GitHub tools. Context window size is not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Nicky LeachandClaude Opus 5.5
|
2d93413085 |
perf(runner): prepare the next Claude provider generation at park (#20145)
## Thinking Path
> - Paperclip runs agents through Paperclip Runner, and a host can park
an idle Runner between runs and resume it for the next run on the same
task.
> - For Claude (ACPX), a follow-up run on a parked Runner pays about
1.5–2.3 s at `run.attach` before the provider can start the turn. The
attachment closes the Claude sidecar and spawns a new one: initialize,
`session.open` with `session/load`, `run.attach`, then a dormant verify.
> - The respawn is required. The tool-bridge MCP endpoint lasts for the
whole session and can't tell which turn a late callback came from, so a
new turn must not run in a process that ran an earlier one.
> - Nothing requires the respawn to happen at the *start* of the next
run. A turn that ends 2 s later is a better experience than one that
starts 2 s later.
> - This pull request does the respawn while parking. The next run's
`run.attach` then only rebinds the run id on a fresh, never-used
generation.
> - Follow-up Claude runs on a parked Runner reach the provider about
1.5–2 s sooner, and the late-callback guarantees are unchanged.
## Linked Issues or Issue Description
No public issue exists. The issue is described inline.
### What happened?
On a hosted deployment with parked Runners, Claude follow-up runs
reached the provider about 1.8 s later than Codex follow-ups.
`runner.session.resume` medians were 5.2 s for Claude and 3.4 s for
Codex. The extra time is the ACPX respawn inside `run.attach`. Codex
keeps its process in place.
### Expected behavior
The respawn happens while the Runner parks, so the next run starts on a
prepared generation.
### Steps to reproduce
1. Run a Claude (ACPX) task on a host that parks Runners between runs.
2. Let the run finish and the Runner park.
3. Send a follow-up. `run.attach` closes the provider and starts,
initializes, opens and attaches a new sidecar before `turn.start`. The
Claude CLI itself starts at the first prompt on either path.
### Paperclip version or commit
`master` at
|
||
|
DottaandClaude Opus 5.5
|
95bf90bd2c |
fix(chat): post a conversation turn's answer while an approval holds the task (#20179)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in chat. The agent's final message becomes the chat reply. > - A chat turn can end while the task stays held. For example, the agent resubmits a hire approval and the approval is pending. > - The finalizer keeps the task in review for the approval (`governed_response_waiting`). That decision is not `conversation_turn_finished`. > - Run completion then treats the yielded turn as a control-plane wait. It records `presentationDecision: none` and posts no comment. > - The person never sees the agent's answer. The chat shows nothing after the request. > - This pull request counts a committed conversation turn as answered unless its own pending question or confirmation card presents it. > - The benefit is that the person always gets the reply, whatever state the task keeps. ## Linked Issues or Issue Description Refs #19971 (post the full final reply). That fix covers the reply text. This fix covers replies that were never posted. **What happened** In a chat, the person asked the CEO agent to change a proposed hire to a cheaper model and send it back for approval. The agent resubmitted the approval correctly. It called `paperclip_finish`, and Paperclip accepted the report with "task is still waiting for approval". The run succeeded and the task moved to `in_review`. The run event `run.presentation.resolved` recorded `commentAction: none` (reason `yielded_control_plane_wait` on master). The agent's final message never reached the chat. **Expected behavior** A chat turn's accepted final answer is always posted to the conversation. An approval, an execution stage or a dependency blocker can hold the task. It must not hide the reply. Only a question or confirmation card that the turn itself left pending presents the turn instead of its prose. **Steps to reproduce** 1. Open an Agent Chat with an agent. Link a hire approval in `revision_requested` to the chat task. 2. Ask the agent to change the hire and resubmit it. 3. The agent resubmits the approval and ends the turn with a yielded result and a response wake. 4. See that the chat shows no reply. The run is `succeeded` and its presentation decision is `none`. ## What Changed - `server/src/services/native-runtime/native-governance.ts`: move the pending-card query of `pendingNativeGovernance()` into `pendingNativeInteraction()`, with no change in behavior. Add `conversationTurnAnswered()`. It returns false only when a pending thread interaction (the same predicate as status arbitration) gates the turn. It checks cards on their own, so a pending execution stage does not hide a card. - `server/src/services/heartbeat/run-completion.ts`: a successful conversation run whose status decision committed now passes `conversationTurnFinished: true` to the presentation resolver, unless `conversationTurnAnswered()` is false. The existing `conversation_turn_finished` path is unchanged. - `server/src/services/heartbeat/run-completion.test.ts`: add a regression table with a real database. A pending approval, a revision-requested approval, a pending execution stage and a dependency blocker now post the answer. A finished turn still posts. The turn's own question card still owns the response, also when an execution stage is pending. - `doc/execution-semantics.md`: state the rule. Other finish outcomes I checked: - `done` or `needs_review` with a gate (`governed_gate_pending`): the result is not yielded, so the reply was already posted. - `blocked` (`paperclip_block`, waiting on a person or another task): not yielded, so the reply was already posted. - Yielded with a dependency blocker (`durable_dependency_blocker_bound`), or with `record_finalization_error` (`prior_status_preserved_no_live_path`, `monitor_wait_authority_lost`): the reply was also dropped. This fix covers them, because the rule does not depend on the reason code. ## Verification - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts`: 118 passed. Before the fix, 4 new cases failed (both approval states, execution stage, dependency blocker). The finished-turn and question-card cases passed both before and after. - `cd server && npx vitest run src/__tests__/heartbeat-run-summary.test.ts src/services/heartbeat/run-comment-selection.test.ts src/__tests__/slack-conversation-lifecycle.test.ts src/services/native-runtime/agent-chat-response-recovery.test.ts src/__tests__/chat-run-milestone-governed-work.test.ts src/__tests__/agent-conversations.test.ts`: all passed. - `tsc --noEmit -p server`: no errors in the changed files. Unrelated errors in this worktree come from an unbuilt local Runner package. - An end-to-end chat eval run on the case branch is in progress. I will add its result here. ## Risks - Low risk. The change only affects successful Agent Chat runs with a committed status decision. Ordinary tasks and external chat (Slack, GitHub) keep their own paths. External chat returns before this check. - A turn that yields while an approval is pending now posts its prose. The approval stays pending and the task status does not change. - One extra small read (the task row and pending gates) runs after a conversation turn that is not `conversation_turn_finished`. It runs after the provider turn ends, so it does not add time before the first token. - Previous-release state: no persisted shape changes. The fix reads `finalizationPhase` and `finalizationReasonCode`, which the previous release already writes. A run in flight at the deploy completes on the new code and posts its reply. A run that completed before the deploy keeps its recorded decision; nothing re-presents it. - Known gap, unchanged: background presentation recovery for conversations still requires `conversation_turn_finished` with a `done` result. If the server restarts between the status commit and the live presentation, a held yielded turn is not re-posted. That is a separate follow-up. > 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`), with tool use (shell, file edits, test runs) 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> |
||
|
DottaandClaude Opus 5.5
|
c5c587f162 |
fix(issues): start a deferred children-completed wake the release drain missed (#20180)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A parent that delegates waits for its children, and is woken when
they finish (`issue_children_completed`)
> - Restart chaos t06 ended with both children done and the parent
blocked forever
> - The children finished while the parent's run was still going, so the
wake was deferred as a follow-up. That run then ended with the agent
setting the parent blocked, and the deferred wake was never started
> - The durable re-drivers for deferred wakes cover only
`issue_blockers_resolved`, so nothing else picks up a missed
`issue_children_completed`
> - This pull request makes both re-drivers cover children-completed
wakes too
> - The benefit is that a parent always hears that its children
finished, even when the release drain misses the deferred wake
## Linked Issues or Issue Description
Refs #19831 (durable ownership plan). Restart chaos wave
`w-20261109T041248-s9501`, task t06 (claude, M1, K2), Daytona per-turn,
master
|
||
|
DottaandClaude Opus 5.5
|
5a51a7685d |
fix(budgets): raise_budget_and_resume continues the work its budget stopped, once (#20144)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop cancels the scope's running work; the board resolves it with Raise budget and resume > - In the restart-chaos H1 scenario, the cancelled tasks did not continue after resume and each needed a manual Retry > - Resume is the board's explicit decision to continue, so the work it stopped should continue once, as if the board clicked Retry > - This pull request files that Retry automatically, keyed by the cancelled run, once the run's stop has settled > - The benefit is that a budget raise restores the work without manual Retries, without repeating completed work, and without charging the run's attempt budget ## Linked Issues or Issue Description Refs #20143 (bug B; this PR builds on it), #20113 (bug A), #20101 (superseded: split into #20143 and this PR) **What happened?** Restart-chaos waves w-20261108T195944-s42 and w-20261108T210118-s9802 (H1). After `raise_budget_and_resume`, CHA-3's budget-cancelled run (enforcement stop) and CHA-1's run (agent lifecycle pause from the budget hold) stayed cancelled until someone clicked Retry. **Expected behavior** Resuming continues each task its budget stopped, exactly once, with no attempt charged, an activity entry, and no repeat of work that already continued. A run a person stopped stays stopped. **Steps to reproduce** Set a budget below spend while a task runs; let the stop cancel the run; resolve with Raise budget and resume. Before this change, no new run is queued. ## What Changed - New table `budget_resume_claims` (migration 0451). One row per cancelled run, outside the run's result JSON, so a Stop that settles during the resume cannot erase it. - `markBudgetCancelledWorkForResume` (called by `resolveIncident` before the incident closes): claims every task run its budget stopped since the incident opened. That covers runs budget enforcement fenced, including stops still in flight, and runs the agent lifecycle pause stopped for this budget (`pauseStop`, carried from the lifecycle operation's `pauseCause` so a resume before the Stop keeps the link). Runs a person stopped are excluded. Two indexed passes led by `(company_id, status)`. - `deliverBudgetResumeContinuations`: once a claimed run's stop settles, files the resolving board member's Retry (`requestTaskRetry`, idempotency key `budget-resume:<run>`, with the member's session source and the chat Retry callback). The Retry path checks, under the task lock, that the run is still the task's latest and the task is open, so continued or completed work is never repeated. Claim settlement and its activity entry commit together; retryable refusals and errors retry. - Delivery runs in the always-on budget-enforcement worker after each pass's Stop delivery. Its schedule includes deliverable claims and the earliest stale-claim expiry. The execution-continuation worker is a backstop. - `agent.paused` / `agent.resumed` logging is in #20113. `budget.cancelled_work_resume_requested` (incident) and `budget.cancelled_work_continued` / `_not_continued` (task) are recorded here. ## Verification - New `budget-resume-continuation.test.ts` (Postgres, real `resolveIncident` → claims → `requestTaskRetry` → real `resumeTaskRetry` into a queued run): - each task continues once, including a blocked task; repeat delivery or resolve saves nothing new; - a stop still in flight at resume, whose terminal write replaces the result JSON, is still continued; - a 30-task scope is fully continued; - a lifecycle-pause stop caused by the budget is continued, and one caused by a manual pause is not; - continued work, done tasks, a run that finished before its stop, and person-stopped runs (including ones enforcement also fenced) are not continued. - `lifecycle.test.ts`: the pause records its cause on the operation and on stopped runs, including when the budget is resumed before the pause's Stop. `budget-enforcement-worker.test.ts`: delivery after Stop delivery, and a wake at claim expiry. - 287 tests across the affected suites passed. Server typecheck (`typecheck-layers`, low-memory): no errors. - Not run: the H1 chaos rerun (Phase 3 runs it after merge). ## Risks - Resume now continues budget-stopped tasks automatically. A board that wants a task stopped can Stop it again; person-stopped runs are never continued. - A new table and one index (migration 0451). > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
b54317d3fb |
fix(native): give a preparing run's first turn the user's queued messages instead of interrupting it (#20168)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A Paperclip Runner (native) run prepares its workspace and
environment before it starts its first provider turn. This can take
about a minute.
> - A user's message that arrives during that time waits as a deferred
wake behind the run. The Interrupt control then cancelled the run before
any provider turn.
> - The cancel threw away the whole preparation. A new run had to
prepare again, so the user waited longer for the first token.
> - This pull request gives the run's first turn the queued messages.
The run seals its first-turn input immediately before `turn.start`, so
the messages are part of the prompt that the turn starts with.
> - The benefit is that the user sees the message handled in the same
run, with no cancel and no second preparation.
## Linked Issues or Issue Description
**What happened?**
A Paperclip Runner run took about 54 s to prepare (environment startup,
workspace, runtime selection). The user sent a second message 32 s into
that time and clicked Interrupt 21 s later. Interrupt cancelled the run
as `operator_interrupted` ("Interrupted to send queued messages") before
any provider turn. A new run then prepared again from the start. The
runtime-selection transaction that froze the run's input completed about
5 s before the click, and the coordinator claim came 4 s after it, so
the message could not be given to the first turn at any point in the old
code.
**Expected behavior**
When a user message arrives for a task whose current run has not started
its first provider turn, the run does not stop. The message goes into
that run's first turn. A run that is in a turn is interrupted as before.
**Steps to reproduce**
1. Assign a task to a Paperclip Runner agent with a sandbox environment,
so preparation takes a long time.
2. While the run prepares, post a second comment on the task.
3. Click Interrupt on the queued message before the run starts its
provider turn.
4. See that the run is cancelled and a new run prepares again.
**Paperclip version or commit**
Staging revision
|
||
|
Devin FoleyandClaude Opus 5.5
|
32b3648758 |
fix(storage): back off and reduce parallel parts when S3 asks to slow down (#20165)
A workspace snapshot upload sends up to eight file-backed parts at once. The SDK never retries a part because its body is a stream, so a part that received 503 SlowDown had only the uploader's three fixed-delay attempts (0.5 s and 1 s). Under a request-rate limit the save failed and the run failed in setup. A rate-limited part now retries with exponential full-jitter backoff (up to eight attempts, at most 20 s per wait), and the upload halves the parts it admits, once per round of throttled requests. Other part failures keep the existing three attempts. Tests cover a real-client SlowDown recovery, the single halving, the attempt bound, cancellation during a wait, and the backoff schedule through its ceiling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
098f7d370b |
fix(workspaces): release an unchanged remote writer without a snapshot and wake its waiters (#20171)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A remote (sandbox) task workspace has one writer run at a time. The
writer holds the workspace until its files are saved as a snapshot
> - A run that a user comment interrupts right after its claim did no
work. No provider turn started and no command ran in the sandbox
> - Paperclip still kept its sandbox, then took a full snapshot of the
unchanged workspace. On a 2 GB repository this took 3 minutes
> - The next runs failed setup with `workspace_busy` ("ended, but its
workspace save has not settled"). The retry then waited for the
one-minute settling re-check after the save. The user's message waited
more than 4 minutes
> - This pull request releases such a writer at once, at the saved
version it started from, when durable evidence proves that it changed
nothing. It also wakes parked `workspace_busy` waiters when a hold
settles
> - The benefit is that an interrupted run no longer delays the next
message by a full workspace save
## Linked Issues or Issue Description
**What happened?**
A remote-workspace run was claimed (`native.coordinator.claim`). One
second later a board comment interrupted it (`operator_interrupted`). It
made no provider turn and no tool calls. Cancelled-workspace
preservation still retained its sandbox, and workspace recovery took a
full `remote_workspace_snapshot` of the unchanged workspace (3 minutes
for a 2 GB repository). Two follow-up runs were cancelled with
`workspace_busy`. Their retry was due during the save, and it parked for
the one-minute settling re-check after the save finished.
**Expected behavior**
A writer that provably changed nothing releases the workspace at once
and keeps its existing `snapshotId`. The next run is admitted without
`workspace_busy`. When a save does run, waiters start as soon as it
commits.
**Steps to reproduce**
1. Run a task on a remote sandbox workspace that already has a saved
version.
2. Interrupt the run with a board comment right after the claim, before
the provider turn.
3. See the full snapshot operation on the interrupted run and
`workspace_busy` on the follow-up runs.
**Paperclip version or commit**
Staging served revision
|
||
|
DottaandClaude Opus 5.5
|
8e5959c759 |
fix(agents): keep pause board-only and resume grant-gated on the PATCH status route (#20164)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board governs the agent lifecycle: pause, resume and terminate > - The V1 permission matrix says agents may never pause an agent, and may resume one only with a direct `agents:configure` change grant > - `POST /api/agents/:id/pause` and `POST /api/agents/:id/resume` enforce this, but `PATCH /api/agents/:id` with a `status` ran the same lifecycle change after only the agent-configuration check > - The `agents.pause-agent` chat eval caught it: the CEO agent paused Morgan itself through `PATCH`, although the case (and the spec) expect a board user to do it > - This pull request applies the dedicated routes' checks to `PATCH` status changes, as #19955 did for termination > - The benefit is one lifecycle authority, with no second route around it ## Linked Issues or Issue Description Refs #19955 **What happened** An agent that may configure another agent could pause it with `PATCH /api/agents/{id}` `{ "status": "paused" }`, although `POST /api/agents/{id}/pause` is board-only. The same `PATCH` with `{ "status": "idle" }` resumed an agent without the change-grant decision and without the invalid-org-chain guard that `POST /resume` applies. In a chat eval run, the CEO agent paused Morgan this way and reported "Morgan is paused". **Expected behavior** Pause is board-only on every route (`doc/SPEC-implementation.md` section 9.3: "Pause/resume agent ... pause: no; resume: direct `agents:configure` grant only"; "Pause, clear-error, terminate, approval, and key-management routes remain board-only"). An agent gets the standard final "Board access required" denial with a link to the agent's profile. Resume through `PATCH` needs the same grant and org-chain checks as `POST /resume`. **Steps to reproduce** 1. As an agent with agent-configuration permission over another agent, call `POST /api/agents/{id}/pause`. The result is 403. 2. Call `PATCH /api/agents/{id}` with `{ "status": "paused" }`. Before this change the result is 200 and the agent is paused. ## What Changed - `server/src/routes/agents.ts` (`PATCH /agents/:id`): `status: "paused"` now calls `assertBoard`, the same check as `POST /pause`. Agents get the structured `board_access_required` denial that says the denial is final and links `/<prefix>/agents/<id>`. - `status: "idle"` now calls `assertCanResumeAgent` (the `agent_config:update` decision with `requiresChangeGrant`) and returns 409 with the repair guidance when the org chain is invalid, the same as `POST /resume`. - `server/src/__tests__/agent-permissions-routes.test.ts`: pause is denied through both routes for an agent allowed to configure the target, with no lifecycle change, run cancel or activity. A board user can still pause through `PATCH`. `PATCH` idle applies the change-grant decision, allows a granted agent, and keeps the org-chain guard. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-permissions-routes.test.ts src/__tests__/agent-cross-tenant-authz-routes.test.ts src/__tests__/board-access-denial.test.ts`: 147 passed. - Without the route change, three new tests fail (PATCH pause by an agent, PATCH idle without the change grant, PATCH idle with an invalid org chain). - Policy evidence: `doc/SPEC-implementation.md` section 9.3 and the `agents.pause-agent` scenario ("Pausing an agent requires board control"). ## Risks - Non-board automation that paused agents through `PATCH` status now gets 403. That is the intended governance boundary. Board users are not affected. - Agents that resumed agents through `PATCH` status now need the same change grant as `POST /resume`. Standard agents get `agents:configure` on creation, so ordinary behavior does not change. - `PATCH` idle now returns 409 for an agent with an invalid org chain, the same as `POST /resume`. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f89813d005 |
fix(api-catalog): describe the real backup and restore paths for agents (#20159)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Instance admins and operators own database backups; a chat agent cannot create one > - When a person asks an agent to back up the instance and check a restore, the agent must say who can do it and how > - The agent reads the API catalog to find the operator path, but the backup entry had only the summary "Trigger a database backup" > - With no real path in the catalog, the agent told the person to use an "Instance settings > Backups" page that does not exist > - This pull request describes the real paths in that catalog entry: the endpoint, the CLI, the retention setting, the result file, and how to check a restore safely > - The benefit is that the agent sends the person to paths that exist and keeps a restore check off the live database ## Linked Issues or Issue Description **What happened** A person asked the CEO chat agent: "Back up this test instance and check that we can restore it." The agent got 403 on `POST /api/instance/database-backups`, which is correct. It searched the catalog for backup and restore operations and found only that endpoint with no description. Its reply then said: "Create a backup in Instance settings → Backups". The web UI has no such page or control. **Expected behavior** The agent says that an instance admin or operator must do this. It names real paths: the endpoint for an instance admin, `paperclipai db:backup` on the server host, and the Backup retention setting for automatic backups. For the restore check, it says to restore into a separate empty database, not over the live one. **Steps to reproduce** 1. Start a local instance and open a chat with the CEO agent. 2. Ask: "Back up this test instance and check that we can restore it." 3. Read the reply: it names a backup page in Instance settings. Look for it in the UI: there is none. ## What Changed - `server/src/routes/openapi.ts`: add a description to `POST /api/instance/database-backups`. It says that only instance admins can call it (agents get 403, cloud-managed instances run backups for you), that automatic backups run about hourly with retention set on the company-prefixed /company/settings page, that the UI has no create or restore control, that an operator can run `paperclipai db:backup`, what the result file is, and that a restore check uses psql into a separate empty database, never over the live one. - `server/src/routes/openapi.ts`: register the success status as `201`, which is what the route returns, instead of `200`. The published document already showed `201` through the created-operations fixup, so this makes the source match it. - `server/src/__tests__/database-backup-catalog-guidance.test.ts`: new regression test. It runs the agent's catalog searches from the failed run and checks the description. It also checks that the published document shows `201` and not `200` for a successful backup. ## Verification - `pnpm exec vitest run src/__tests__/database-backup-catalog-guidance.test.ts src/__tests__/openapi-routes.test.ts` (from `server/`): 40 tests pass. Without the change, the 2 search tests fail. The status test guards the published document, which the fixup already kept correct. - `pnpm exec vitest run src/__tests__/instance-database-backups-routes.test.ts`: 7 tests pass. The server typecheck passes. - Chat eval `instance-operations.backup-and-restore` (runner-opencode), with this commit cherry-picked onto the eval branch: before the change, the reply named an "Instance settings → Backups" page that does not exist. After the change (GitHub Actions run 42552777052), the eval passed. The first reply said the agent got 403, named `POST /api/instance/database-backups` for an instance admin and `paperclipai db:backup --json` for an operator, linked Backup retention on the company settings page, and said to restore with psql into a separate, empty database, never over the live one. The second reply reported the operator's restore receipt and its limits correctly. ## Risks - Low risk. The change adds catalog text and makes the registered success status match the published 201. It does not change any API or permission behavior. - If a backup page or restore command is added later, this text must change. The regression test names these facts, so such a change fails the test and shows the text to update. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
3f12046576 |
fix(tool-access): an unscoped tools:use grant does not turn on actions a connection left off (#20160)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - People connect apps and choose which actions of each app agents may
call; the choice is saved as the connection's tool profile
> - Every new standard agent also gets an unscoped `tools:use` grant,
which means "this agent may use tools"
> - The access policy checked explicit grants before profiles, so this
default grant allowed every action of every installed connection,
including actions the person left off
> - The agent's effective profile showed the action off, but the gateway
and the test-as-agent summary allowed it
> - This pull request makes an unscoped grant stop at a connection
profile: when a profile governs the tool's connection, only a grant
scoped to the action, connection or application allows an action the
profile leaves off
> - The benefit is that the actions a person turns off for a connection
stay off for new agents
## Linked Issues or Issue Description
**What happened**
A person connected an app with one read action and one destructive write
action. When they finished the connection, they enabled only the read
action. They installed the connection for a new agent, Morgan. Morgan's
effective profile (`GET
/api/companies/{companyId}/tools/profiles/effective/agents/{agentId}`)
allowed only the read action. But `GET
/api/tool-connections/{connectionId}/test-agents/{agentId}/access`
reported both actions as `allowed` with reason `allow_explicit_grant`,
and a chat agent told the person that Morgan could also run the
destructive write. The same policy decision admits gateway calls, so
Morgan could call the write.
**Expected behavior**
An action that the connection's profile leaves off is off for the agent.
The access summary, the effective profile and the gateway agree. A grant
that a person scopes to that action, connection or application still
allows it.
**Steps to reproduce**
1. Connect an MCP app that has a read tool and a destructive tool. In
the finish step, enable only the read tool.
2. Create a new agent through the API (it gets the default unscoped
`tools:use` grant) and install the connection for it.
3. Call `GET
/api/tool-connections/{connectionId}/test-agents/{agentId}/access`: the
destructive tool shows `allowed`, reason `allow_explicit_grant`.
## What Changed
- `server/src/services/tool-access-policy.ts`: for an agent, a tool's
connection is governed by a connection profile when the agent's
effective profiles include the connection's own access profile (the app
wizard's `app:<connectionId>` profile or the agent's connection-intent
profile, even with no entries), or any entry that names this connection
or selects this action (including application, tool-name and risk-level
selectors). For a governed connection, `explicitGrant` counts only
grants whose scope names an action, connection or application (`allow`,
`toolName`, `catalogEntryId`, `connectionId`, `applicationId`,
`applicationKey`). A grant scoped only by context (for example an agent
or project) still covers every tool, so it does not override the
profile. Board users keep their operator grant.
- `server/src/services/tool-profile-binding-precedence.ts`: export the
existing connection-profile identity check as
`isConnectionAccessProfile`, so the policy and binding precedence use
one definition.
- `server/src/__tests__/generic-mcp-connection.test.ts`: regression
tests through the gateway's access summary and the direct policy check:
- only the read action enabled: the write stays off with the unscoped
grant, and a grant scoped to the write allows it
- every action turned off (both lists empty): both stay off; a
context-only scope stays off; a connection-scoped or application-scoped
grant allows them
- an agent the connection's profile does not cover keeps the unscoped
grant
- a general profile with an application-selector exclude (no connection
id) is not overridden by the unscoped grant
- with company-wide access, the direct policy check denies the off
action for an agent and still allows it for a board user with the
unscoped grant
## Verification
- New tests: all pass with the change. On `master`, 4 of 5 fail. On the
first revision of this PR, the all-off, application-exclude and
board-user tests fail.
- Regression suites, all pass: `generic-mcp-connection`,
`tool-access-policy-service`, `tool-profile-matching`,
`tool-profile-entry-order`, `connection-management.integration` (238
tests), and `tool-access-service`, `tool-gateway-service`,
`tool-gateway`, `tool-profile-review-service` (799 tests).
- Every enforcement path uses the one `decide` function in
`tool-access-policy.ts`, and `explicitGrant` is called only there. That
covers the gateway tool listing and calls, the access summary, delegated
and Test tab calls, the connection-intent access check, the GitHub
review bot tool, and the example smoke check.
- Chat eval `app-permissions.test-as-agent` (runner-opencode). Before
the change (GitHub Actions run 42550085273), the access summary for
Morgan reported the write as `allowed` by `allow_explicit_grant`. With
the first commit cherry-picked onto the eval branch, GitHub Actions run
42552819589 passed. The summary reported `launch_read` allowed
(`allow_profile`) and `launch_update` off (`deny_default`), which
matches Morgan's effective profile, and the agent's reply said the same.
## Risks
- Medium risk. This changes an access decision. An agent that used a
connection action only through its default unscoped grant, while the
connection's profile or an explicit exclude left that action off, loses
that action. That is the intended fix. An operator who relied on it must
enable the action in the connection or give the agent a grant that names
the action, connection or application.
- A grant scoped only by context (for example `{ projectId }`) no longer
overrides a connection profile. Risk-level and provider-type scopes are
not treated as naming an action.
- Tools that no connection profile governs (for example Paperclip's own
tools), agents without a connection profile, and board users keep the
current behavior.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
Related: #17884 (scoped `tools:use` grants at hire time).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
28e070f8dc |
feat(skills): show a line diff for a pending tracked-source update (#20155)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Skills can come from a tracked GitHub source, and an update moves
the installed copy to a newer upstream revision
> - Before an update, a person asks the agent "what changed in the new
version?"
> - The product has no diff for a pending update. The agent previews two
full SKILL.md snapshots and compares them by eye
> - In two eval runs the agent reversed the same changed line. In a
third run it fetched the files with curl and ran `diff` itself, and the
turn took too long
> - This pull request adds a read-only endpoint that returns the line
diff between the installed and the latest tracked revision
> - The benefit is that review answers come from an exact diff, and they
come faster
## Linked Issues or Issue Description
No public issue exists. Feature description:
**What happened**
An agent asked to show what changed in a pending skill update had only
`POST /skill-sources/preview`, which returns one file at one commit. It
previewed both commits and compared about 9 KB of text by eye. It
reported one changed line in the wrong direction ("every new build" ->
"every build or release" instead of the reverse), and later retracted a
real change.
**Expected behavior**
The agent can request the exact changes of a pending tracked-source
update and report them with the correct direction.
**Steps to reproduce**
1. Install a skill from a GitHub source at an old commit and set the
tracking reference to a newer commit (automatic updates off).
2. Ask the chat agent: "Show me what changed in the new accessibility
skill version before we update it."
3. See that the agent can only preview full files at each commit and
must compare them itself.
## What Changed
- Add `GET /api/companies/{companyId}/skills/{skillId}/update-diff`. For
a skill from a tracked GitHub source, it compares the installed source
revision (`currentRef`) with the latest tracked revision (`latestRef`).
For each changed package file it returns `change` (added, removed,
modified, or `not_compared`), `binary`, `executableChanged`, and a
unified line diff (`-` installed, `+` latest), or `diffUnavailable`
(`binary`, `too_large`, `omitted`, `budget`) that says why there is no
diff. The endpoint is read-only. The `skillId` parameter handler checks
company access and skill visibility, the same checks other skill reads
use.
- `hasUpdate` is false and `files` is empty when the installed revision
is already the latest one. That case reads only the latest commit and
scans nothing.
- Return `supported: false` with a reason for: a skill without a tracked
source, a disconnected source, an unknown installed revision, and a
repository package that a scan could include only in part (a subset, or
skill folders only). In the last case, files that a scan limit left out
would otherwise look removed.
- A path the scan left out of a revision (too large, a symlink, or
rejected by the audit) is listed as `not_compared` instead of added or
removed.
- One response shares a work budget (24M LCS cells), a diff-text budget
(1M characters) and a file-count budget (1,000 files), and sets
`truncated` when it reaches one. The GitHub read and both scans stop
when the caller disconnects.
- Add `server/src/services/skill-update-diff.ts` with a bounded line
diff (shared head and tail trimmed, LCS size capped). An absent side has
no lines, a final newline is not a line, and a missing final newline
shows the unified-diff marker. A replaced line reads `-old` then `+new`.
- Add the shared types `SkillSourceUpdateDiff` and
`SkillSourceUpdateDiffFile`.
- Document the endpoint in the API catalog (`openapi.ts`) so agents find
it with `search_api`.
## Verification
- `cd server && npx vitest run src/__tests__/skill-update-diff.test.ts`
(10 tests):
- direction of `-`/`+` lines, and separate hunks with bounded context
- exact diffs for added and removed files, and final-newline handling
- a change to only the executable bit, and paths left out of a scan
- the response budgets
- the snapshot comparison for the selected skill path, with the abort
signal passed to both scans
- a commit mismatch
- refusal of partial repository scans
- `cd server && npx vitest run
src/__tests__/skill-update-diff-routes.test.ts` (6 tests):
- the diff response
- no pending update (no scan)
- a caller from another company (board user and agent) gets 403 before
any lookup
- a skill the caller cannot read gets 404 before any source read
- every unsupported case
- a client disconnect aborts the scans
- `company-skills-routes.test.ts` and `openapi-routes.test.ts` pass (119
tests across the four files).
- `npx tsc --noEmit -p server`: no errors from this change.
- End to end: the `skills.review-update` chat eval exercises this path.
Its grader checks the quoted hunks against the real pinned diff.
## Risks
- Low risk. The endpoint is new and read-only. Existing routes do not
change.
- When an update is pending, each call scans the source twice (installed
and latest commit). It uses the same GitHub reader, scan limits and scan
slots as preview and refresh. The diff work and response size are capped
per response.
- `/update-status` and this route both read the latest commit of the
tracking ref. They are kept separate because `/update-status` compares
with the source's last scan, while this route compares with the
installed revision.
- No UI uses the endpoint yet.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
4afdf0493d |
fix(api-catalog): name the Connections page for company model service settings (#20152)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A company can turn on model services such as Fast response, Decision model and Prompt Injection Prevention > - Only people who manage connections can change these services, so a chat agent must send the person to the right page > - The agent finds these operations through the API catalog search, but their catalog entries named no page > - The agent searched for the settings page, found nothing, and linked the general /company/settings page, which does not have the control > - This pull request adds a description to these catalog entries that names the section and the company-prefixed /company/settings/connections page > - The benefit is that the agent links the page that actually holds the control ## Linked Issues or Issue Description **What happened** A person asked the CEO chat agent: "Help me enable the company's Fast response feature so chat acknowledges my requests faster." The agent correctly got 403 on the fast-response API. It then searched the API catalog for "company settings page fast responses web UI" and found no page. It linked the general company settings page (`/<prefix>/company/settings`) and told the person to find Fast response there. That page has no Fast response section. The section is on /company/settings/connections. **Expected behavior** The agent links /<prefix>/company/settings/connections and tells the person to use the Fast response section there. If the section is hidden, the agent can say that an instance admin turns the experiment on. **Steps to reproduce** 1. Turn on the fast responses experiment. Create a shared OpenRouter API connection. 2. Open a chat with the CEO agent and ask it to enable Fast response. 3. Read the reply: it links /<prefix>/company/settings. Open that link: the page has no Fast response control. ## What Changed - `server/src/routes/openapi.ts`: add one shared description to the fast-response, decision-model and prompt-injection-prevention catalog entries. It names the section, the company-prefixed /company/settings/connections page, the Markdown link form, and that agent credentials cannot change or test the service. For Fast response, it also names the instance experiment that hides the section. - `server/src/__tests__/company-model-service-settings-guidance.test.ts`: new regression test. It runs the agent's catalog search (including the query from the failed run) and checks that the results name the Connections page and section. It also requires every page the descriptions name to be a route that `ui/src/App.tsx` mounts, so renaming a settings route fails CI. ## Verification - `pnpm exec vitest run src/__tests__/company-model-service-settings-guidance.test.ts` (from `server/`): 6 tests pass. Without the catalog change, the 5 search tests fail. If the Connections route in `ui/src/App.tsx` is renamed, the route test fails. - `pnpm exec vitest run src/__tests__/openapi-routes.test.ts`: 37 tests pass. - Chat eval `companies.fast-responses` (runner-opencode), with this commit cherry-picked onto the eval branch: before the change, the reply linked the general settings page, which has no Fast response control. After the change (GitHub Actions run 42549422681), the eval passed. The reply linked the Experimental settings page and the Connections page and said to use the Fast response section there. The scripted person clicked the reply's links and found the Fast response control on the Connections page, then saved and tested it. ## Risks - Low risk. The change adds catalog text only. It does not change any API or permission behavior. - If the settings page moves, these descriptions become wrong. The regression test compares every page the descriptions name with the routes in `ui/src/App.tsx`, so a route change that does not also update the description fails CI. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
43fe87795a |
fix(authz): make board-only denials final and actionable for agents (#20151)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People talk to their agents in Agent Chat, and the agents act through the Paperclip API > - Many API routes are board-only. The shared `assertBoard` check returns 403 `"Board access required"` to an agent, with no other detail > - The chat guidance also told the agent to "use permitted discovery to identify the supported operator path" after such a denial > - So the agent keeps searching the API, `call_api` and the shell for a second route. On a slow model, this search uses the whole reply window and the person gets no answer > - Earlier single-route fixes (#19990, #20041, #20008) showed the remedy: a denial that says it is final and names where the person acts. Reply time fell from 10-24 minutes to about 2 minutes > - This pull request applies that remedy once, in the shared board-only check and in the chat guidance > - The benefit is a prompt, correct handoff for every board-only action that an agent cannot do ## Linked Issues or Issue Description Refs #19990, #20008, #20041 (same remedy for single routes). **What happened** In about 30 Agent Chat evaluation cases on a slower model, the agent got 403 "Board access required" (or read `authorization.actor: "board"` in the API catalog). Then it searched for another route until the attempt deadline. Examples: archive a company, set up a managed agent profile, add a company secret. In one recorded run, the agent got the 403 at minute 1, then tried PATCH, environment variables, the CLI and UI paths for more than 3 more minutes, and never replied. **Expected behavior** A board-only denial is final. The agent stops, tells the person that a board user must do the action, links the page in the Paperclip web app where they can do it, and ends its reply. **Steps to reproduce** 1. Open Agent Chat with a CEO agent. 2. Ask it to archive the company (or to add a company secret). 3. The agent calls `POST /api/companies/{id}/archive` and gets `{ "error": "Board access required" }`. 4. The body says nothing about finality or where the person acts. The agent searches for other routes and replies late or not at all. ## What Changed - `server/src/routes/authz.ts`: `assertBoard` now throws `boardAccessRequired(req)`. For an agent actor, the 403 body adds `code: "board_access_required"` and a `remediation`. The remediation says the action is board-only by design, the denial is final, and no other route can do it. When the API path maps to a known board page, it links that page with a `<prefix>` placeholder: company settings, company secrets, budgets, members, tool gateways, connection settings (decision model, prompt injection prevention), an approval, or the agent profile tab that holds the control (API keys, permissions, budget, identity, harness settings). Otherwise it says to link a page only when the route is already known, and not to search for one. Board and anonymous actors get the same body as before. - `environments.ts`, `instance-settings.ts` and the issue force-release route used their own copy of the check. They now call `assertBoard`, so they return the same denial. Status and error text do not change. - `server/src/middleware/error-handler.ts`: for this code, the handler does not repeat `code` and `remediation` inside `details`. They are already top-level, and the copy only made the agent's context longer. - `AGENT_CHAT_DIRECTIVE`: the board-identity paragraph now says that a board-only operation (catalog actor `board`, or a 403 "Board access required") is final. The agent must stop searching, tell the person which control in the web app to use, and link it when it knows the route. The old instruction to "discover the operator path" is removed. - Claude Managed Agent setup: the Paperclip Runner configuration doc and the `POST /api/companies/{companyId}/managed-agent-profiles` catalog entry now say that a board operator creates and qualifies the profile (limited environment, retention acknowledgement) and that agents cannot. Before, an agent asked for an "Anthropic managed agent" explored hiring and adapter docs for its whole reply window. - Tests: new `board-access-denial.test.ts` covers the agent body for each mapped page, the agent profile tabs, the general fallback, the unmapped fast-response route, the org-access guard, the unchanged non-agent body, the guidance and the managed profile catalog entry. Two exact-body tests now also expect the new code. ## Verification - `cd server && npx vitest run src/__tests__/board-access-denial.test.ts src/__tests__/costs-service.test.ts src/__tests__/browser-use-route-scope.test.ts src/__tests__/primary-agent-guidance.test.ts src/__tests__/agent-cross-tenant-authz-routes.test.ts src/__tests__/company-portability-routes.test.ts src/__tests__/issue-comment-cancel-routes.test.ts src/__tests__/error-handler.test.ts src/__tests__/managed-agent-profile-routes-authz.test.ts src/__tests__/companies-route-cross-company-authz.test.ts src/__tests__/environment-routes.test.ts src/__tests__/environment-instance-routes.test.ts`: all pass. - Real chat journeys on the chat evaluation branch (runner-opencode, `deepseek-v4.1-flash`). Before this change, each case below got no reply before its deadline: - `companies.archive-company`: passed. Reply 47 s after the request (before: none in 270 s). The agent read the remediation and linked `/COM/company/settings`. - `companies.configure-decision-model`: passed. Reply 68 s after the request (before: none in 389 s). It linked `/COM/company/settings/connections`. - `companies.prompt-injection-guard`: reply 85 s after the request (before: none in 390 s), with the correct handoff and link. The case still fails later, in the scripted settings step: the saved model is the default, not the model the script typed. That is a separate defect. - `navigation-preferences.sidebar-order`: reply 99 s after the request (before: none in 150 s). The case still fails in its scripted sidebar step (a locator was not found). - `secrets.configure-company-secret`: reply 6.5 min after the request (before: none in 7.5 min). The reply is correct, but a grader keyword check reads "chat is not a safe place to send it" as a request for the secret. - `agents.managed-profile`: still no reply in its 150 s window. After the catalog change, the agent found the board-only profile setup within 30 s, but then spent about 90 s deciding what "Anthropic managed agent" means. ## Risks - Low risk. The HTTP status and the `error` text stay the same for every actor. Board users and anonymous callers get the same body as before. - Agent actors now get two more top-level fields (`code`, `remediation`). A client that compares the whole 403 body of an agent call exactly will see a change. Two server tests did this and are updated. - The page map uses the API path. A wrong map entry gives a less useful link but does not change authorization. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
fa7950121d |
fix(native): move an interrupted bootstrap's session aside on the run's own created sandbox lease (#20154)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When restart recovery proves a remote bootstrap reached no provider work, it bootstraps the same run again in the same sandbox. Before relaunching, it stops the interrupted runner (#20103) > - The relaunch starts a fresh controller record, whose command ids restart at `command_prp_00000001`. The sandbox still held the interrupted runner's journal, which had already recorded that command id with different data > - The relaunched runnerd rejected it ("commandId was reused with different command data") and exited while waiting for `run.prepare`. Every same-run retry collided the same way until the retry budget ran out and the task blocked > - The proven interrupted-bootstrap session move (#20103) ran only for a resumed lease. A same-run retry on a per-turn sandbox reuses the run's own created lease, so nothing moved the interrupted session aside > - This pull request applies that same proof-gated session move on the run's own created lease > - The benefit is that a same-run bootstrap retry starts its runner on a clean journal, and the old journal is kept as evidence ## Linked Issues or Issue Description Refs #20103 **What happened?** Daytona per-turn restart-chaos M1 rerun w-20261109T013733-s9501 (master |
||
|
DottaandClaude Opus 5.5
|
510746c682 |
fix(native): restart a remote run whose controller never sent its runner a command (#20153)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A follow-up run on a remote sandbox inherits the previous run's session checkpoint at admission, rebound to the new run's identity > - A remote runner receives work only through the controller's Postgres checkpoint, which the controller writes before every command. A run with no checkpoint rows, no runner events and no recorded runner process never sent its runner anything > - Restart recovery still read the rebound admission checkpoint's provider session id as provider evidence, and classified such a run `reattach_remote_runner`. The executor then found the root still owned by the previous run and failed with `native_remote_recovery_lease_mismatch: stage=runner_verification field=identity_run_id` > - This pull request classifies that run as an incomplete bootstrap, and lets the executor start it from the previous owner's settled root > - The benefit is that a follow-up interrupted before its runner ever got a command restarts on its own ## Linked Issues or Issue Description Refs #20137 **What happened?** Daytona per-turn restart-chaos M1 rerun w-20261109T013733-s9501 (master |
||
|
DottaandClaude Opus 5.5
|
763adbea09 |
fix(execution): a pause's stop never blocks the task behind an incident with no control (#20143)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop pauses the agent, which stops its running work > - In the restart-chaos H1 scenario, the stopped task went `blocked` behind a board action that said "inspect and reconcile" and offered no button, and it stayed there after the board resumed the budget > - A pause is an intentional stop, not a failure, so it must not leave the task with no way forward > - This pull request treats a pause's stop as intentional: the task keeps its status and no incident opens, so the normal Retry works > - The benefit is that a budget or agent pause never strands a task behind a control-less incident ## Linked Issues or Issue Description Refs #20113, #20101 **What happened?** Restart-chaos wave w-20261108T210118-s9802 (H1). At the budget pause, task CHA-1 went `blocked` (21:02:27.870) with an active board `active_run_watchdog` action, cause `native_continuation_requires_reconciliation`, next action "Inspect the original failure and reconcile the previous execution before continuing." After raise_budget_and_resume it stayed blocked, `canRetry=false`, with no Retry. The run was cancelled by the agent lifecycle pause (`cancellation: { reason: "Agent lifecycle stop requested", initiator: system, expected: true }`). The wake-queue release (`recordNativeTerminalRecoveryIfNeeded`) blocks every cancelled native run that is not an acknowledged user Stop. **Expected behavior** A pause's stop keeps the task's status and opens no incident; the task can be continued. **Steps to reproduce** Pause an agent (budget hold or company budget pause) while its native run is active. Before this change, the task is blocked behind a board action with no control. ## What Changed - New `server/src/services/governance-pause-stop.ts`: `isGovernancePauseStop(run)` matches an expected system cancellation whose reason is the budget pause stop, or the agent lifecycle stop with a `pauseStop` stamp. - `wake-queue/adapters/postgres.ts` `recordNativeTerminalRecoveryIfNeeded`: returns early for such a run, so the release keeps the task's status and opens no reconciliation incident. - `agent-lifecycle-driver.ts`: the `pausing` host step stamps `resultJson.pauseStop = { holds, companyPauseReason }` on the agent's active runs before stopping them (one update, company pause reason by subquery). Termination and cleanup share the lifecycle stop reason but write no stamp, so they still open the incident. ## Verification - `wake-queue/adapters/postgres.test.ts`: new cases. A run stopped by the lifecycle pause or by the budget pause keeps the task `in_progress` with no incident (both red on master). Agent termination, and another system stop (company archive), still open the incident. - `agent-lifecycle/__tests__/lifecycle.test.ts`: "records what a pause stopped each active run for". `driver-scope.test.ts` updated for the stamp's single update. - `wake-queue` (107), `lifecycle` (68+), `driver-scope`, `companies-service`: 199 passed. Server typecheck (`typecheck-layers`, low-memory): no errors. - Not run: the H1 chaos rerun (after the follow-up continuation PR). ## Risks - A native run that a pause stopped no longer opens a reconciliation incident. Resuming it is the normal Retry, which continues the conversation. Termination, archive and failures still open one. - One extra update per pause attempt on the agent's active runs. It is idempotent (skips runs that already have the stamp) and not on any turn path. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
3eea8b87bd |
fix(native): settle a finished owner whose journal holds a definitively answered command (#20148)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Before a per-turn follow-up starts a new runner, it retires the
previous turn's finished owner. Retirement requires that owner's
controller journal to be settled: every command answered, then a run
terminal
> - A restart can cut off a continuation turn. The recovered runner
answers that `turn.start` with `failed` and the nested code
`execution_indeterminate` ("it will not execute twice"), and the run
continues on a later turn to an accepted result
> - Retirement counted only `completed` commands as answered. A finished
run whose journal held that answered `turn.start` never counted as
settled, so the follow-up failed with
`retirement_refused=journal_commands_unsettled(turn.start.failed)` and
needed a Retry
> - This pull request counts definitive `failed` and `rejected` answers
as settled, except for runner lifecycle commands. An unknown-outcome
`turn.start` counts as settled only once a later `turn.start` completed,
which shows the run moved past it. Every other unknown or pending
command stays unsettled
> - The benefit is that a follow-up after a restart-interrupted, then
finished, run continues on its own
## Linked Issues or Issue Description
Refs #20140
**What happened?**
Local restart-chaos M1 K4/K6 rerun w-20261109T003126-s9701 (master
|
||
|
DottaandClaude Opus 5.5
|
cedf335c98 |
fix(native): a failed terminal followed by an active restart continuation is not a permanent failure (#20149)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a restart loses a native run's active turn, the run records that turn as failed with `provider_turn_lost_on_restore` and continues on a same-conversation continuation turn > - Restart recovery blocks a run as `provider_checkpoint_permanently_failed` when its checkpoint carries a failed run terminal and no pending restart interruption > - While a continuation is running, the checkpoint keeps the lost turn's failed run terminal and names the continuation as the active turn. The pending-interruption check returns nothing once a turn is active > - So a second restart during the continuation read the run as permanently failed, blocked replay, and needed the board > - This pull request treats that failed terminal as the restart loss it records whenever an active turn follows a restart-lost terminal turn > - The benefit is that runs survive repeated restarts during their continuations, including M1 parents and their subtasks ## Linked Issues or Issue Description Refs #20148 **What happened?** Local restart-chaos M1 K4/K6 rerun w-20261109T003126-s9701 (master |
||
|
DottaandClaude Opus 5.5
|
c0fc7608ee |
fix(agents): honor an explicit hiring denial on CEO agents (#20041)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The board controls which agents can hire, with the "Can create
agents" permission on each agent
> - A CEO agent with that permission turned off could still hire,
because the CEO role allowed hiring before the server read the saved
permission
> - In chat, a CEO agent asked to hire then created the agent through
the `hire_agent` tool, against the board's policy
> - This pull request makes the saved permission decide, and uses the
CEO role only for old rows that never saved a value
> - It also tells a denied agent that the denial is final and where a
person can act
> - The benefit is that the permissions page and the server now agree,
and the board can trust a hiring denial
## Linked Issues or Issue Description
**What happened**
A board user turned off "Can create agents" for a CEO agent. Then the
user asked the CEO in chat to hire a QA engineer. The CEO called
`hire_agent` (`POST /api/companies/{companyId}/agent-hires`). The server
returned 201 and created the agent. The legacy creator rule allowed any
agent with `role: "ceo"` before it read `permissions.canCreateAgents`.
**Expected behavior**
An explicit `canCreateAgents: false` denies hiring for every role,
including CEO. The agent gets a 403 that says the denial is final, and
it sends the person to the hire form or to its permissions page.
**Steps to reproduce**
1. Create a company with a CEO agent.
2. On the CEO's permissions page, turn off "Can create agents".
3. In chat with the CEO, send "Hire a QA engineer named Morgan."
4. See that a new agent named Morgan exists.
Found by the chat eval case `agents.hire-denied`. Related: #19614 (chat
eval harness).
## What Changed
- `canCreateAgentsLegacy` (authorization) uses the role-aware permission
normalization. A CEO keeps hiring unless the board recorded a hiring
denial.
- New `ceoHiringDeniedBy` record in `permissions` (`{ actorType,
actorId, at }`). `PATCH /agents/:id/permissions` writes it when an
authorized actor saves a CEO's hiring as off (an earlier record is
kept). The page shows the effective value, so a saved off is the board's
choice. Turning the toggle off also removes an explicit `agents:create`
grant. A saved `canCreateAgents: false` restricts a CEO only together
with this record.
- Reads report the effective value. A CEO row with no value, or with an
unrecorded false, shows hiring on. A CEO with a recorded denial shows it
off.
- Create and hire inputs cannot supply the record. Configuration updates
keep an existing record unless they turn hiring on. Company export keeps
a CEO's recorded denial, and import restores it, stamped by the
importing user.
- `PATCH /agents/:id/permissions`: an agent actor needs `agents:create`
to change `canCreateAgents`. A denied CEO cannot restore hiring for
itself or grant it to another agent.
- `assertCanCreateAgentsForCompany` adds a final, actionable sentence to
an agent's `agents:create` denial on hiring routes only (`POST /agents`,
`POST /agent-hires`, the hiring toggle on `PATCH /permissions`).
- `POST /api/companies/:companyId/imports/apply` (CEO-safe import): an
agent import that would create agents needs an `agents:create` decision
before any write. Before, any CEO agent could create agents this way.
Import also removes an existing `agents:create` grant when it restores a
CEO's denial.
- Chat guidance tells the agent to link `/<prefix>/agents/new` and its
own `/permissions` page when it cannot hire.
- Regression tests: authorization (a recorded denial is honored; an
unrecorded false keeps CEO hiring), normalization, route tests (denial
text, self-escalation, the record is written only when a CEO is turned
off), service tests for the permission and configuration write paths,
chat guidance text.
### Why a recorded denial and not a plain false
These normal paths save `canCreateAgents: false` on a CEO without a
board choice:
- The permissions page posts the full object. Before this change, a
legacy CEO row with `{}` was read as false, so saving any other toggle
stored false.
- A company import that updates an existing agent normalizes the
manifest permissions in the "stored" context. A manifest without the
field stores false. Export drops false values, so this is common.
- Low-trust creation stores false. If the trust preset later changes to
standard, the false remains.
These paths store true or nothing for a CEO: onboarding seed
(`requestHire`, create context: true), teams-catalog and import creation
(create context: true; the bundled catalog sets true only for a CTO),
built-in agents (no CEO roles), migrations and seed data (no writes to
this field).
A plain-false rule would remove hiring from those CEOs. With the record,
existing data keeps today's behavior, and no data migration is
necessary.
## Verification
- `cd server && npx vitest run
src/__tests__/agent-permissions-service.test.ts
src/__tests__/authorization-service.test.ts
src/__tests__/agent-permissions-routes.test.ts
src/__tests__/agent-conversations.test.ts` passes (244 tests).
- `teams-catalog-service`, `company-portability` and `built-in-agents`
tests pass (189 tests).
- Red check: the new authorization test fails without the
`authorization.ts` change.
- `npx tsc --noEmit -p server` passes.
- End to end: the chat eval `agents.hire-denied` on the OpenCode runner.
The baseline run (Actions run 42478870236) created Morgan through
`hire_agent`. With this fix (Actions run 42480936881), the agent got the
403, created no agent, and linked the hire form and its permissions
page. The eval clicked the rendered hire-form link.
## Risks
- Behavior change: a CEO whose hiring the board turns off after this
change can no longer hire.
- A CEO that already has a saved false keeps hiring until a board user
turns it off again on the permissions page. The page now shows such a
CEO as on, so the board can see and change it.
- A request that creates a CEO with `canCreateAgents: false` does not
restrict it. The board must turn hiring off on the permissions page.
- A CEO agent whose hiring is denied can no longer import a package that
creates agents. A board user can still import it.
- Other CEO special cases are not hiring routes: `issues.ts` lets a CEO
link approvals to issues, and `agents.ts` gives a CEO task assignment.
`teams-catalog.ts` and `llms.ts` read the normalized permissions, so
they follow the same effective value.
- The denial text is longer. No existing test compares this route's
exact error text.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
a0b9895bbd |
fix(native): roll back an undelivered warm attach rotation once its runner is proven dead (#20146)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A follow-up turn can attach to the previous turn's warm local runner. The transport first rotates authority: it archives the previous epoch (suspended runner state and settled controller) under `authority-epochs/` and writes a new control plane that holds only the `run.attach` command > - A hard kill in that window, before the runner connects, leaves the root with the new control plane and no `runner/runner-state.json`. Restart recovery proves the warm runner dead and classifies the run `resume_dead_runner` > - The executor saw a root with the run's own identity and no runner state, called its authority indeterminate, and quarantined it. The run failed with `runner_state_identity_mismatch: authority_indeterminate` and needed a Retry > - The attach was never delivered, and the runner that would have received it is proven dead, so it cannot have taken effect. The root's real authority is still the archived previous epoch > - This pull request rolls back such an undelivered rotation, so the follow-up continues from the previous turn > - The benefit is that a restart in the first second of a warm follow-up no longer needs a Retry ## Linked Issues or Issue Description Refs #20140 **What happened?** Local restart-chaos K13 regression storm w-20261108T235957-s9401 (master |
||
|
DottaandClaude Opus 5.5
|
bd97a3e6f7 |
fix(native): retire a finished per-turn owner past its own quarantined interrupted bootstrap (#20140)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A follow-up turn on a per-turn session must retire the previous turn's runner state before it starts a new runner. Retirement proves the previous owner finished, and it also proves that every quarantined copy of that session root is settled > - If restart recovery interrupts a run's own start (for example a hard kill during `session.open`), recovery proves the half-started bootstrap held no provider work, quarantines that root, and bootstraps the same run again. The quarantined copy keeps the run's identity > - After that run finished, retirement checked the old bootstrap copy as if it were a finished owner. An unfinished `session.open` (or an opened session without a turn) is never "settled", so the follow-up failed with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate` and needed a Retry > - This pull request skips a quarantined copy of the same owner when the same bootstrap proof that let recovery quarantine it still holds > - The benefit is that a follow-up after a restart-interrupted start continues on its own ## Linked Issues or Issue Description Refs #20137 **What happened?** Local restart-chaos K13 regression storm w-20261108T235957-s9401 (master |
||
|
Devin FoleyandPaperclip
|
0e49576fd9 |
fix(github): default repository owner to the only installed organization (#20141)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents use a managed GitHub connection to create repositories and to fork repositories > - The Paperclip GitHub App can be installed only on an organization, and not on the user's personal account > - Then `create_repository` and `fork_repository` fail when `organization` is empty, because GitHub sends them to the personal account > - Agents, approval cards, and connection tests all fail on their first attempt with `403 Resource not accessible by integration` > - This pull request fills an empty `organization` with the only organization where the App is installed > - The benefit is that repository creation works on the first attempt for these connections ## Linked Issues or Issue Description No public issue exists. Refs #19655 (GitHub repair guidance for the same error). **What happened?** A user connected GitHub through the managed connection. The Paperclip GitHub App was installed on one organization and not on the user's personal account. An agent called `create_repository` without `organization`. GitHub returned `POST https://api.github.com/user/repos: 403 Resource not accessible by integration`. The test button on the connection page gave the same error. The agent then stopped and asked the user to create the repository manually. **Expected behavior** When the App can create repositories only in one organization, an empty `organization` uses that organization. The approval card and the test result show that owner. **Steps to reproduce** 1. Install the Paperclip GitHub App on one organization only. Do not install it on the personal account. 2. Connect GitHub with the managed connection. 3. Run `create_repository` with only `name`, from an agent or from the connection test dialog. 4. GitHub returns 403. ## What Changed - Added `server/src/services/tool-access/github-default-owner.ts`. It finds the default organization from the grant's access snapshot. It returns an organization only when there is exactly one installation, and the snapshot marks that owner's repositories as organization repositories. Older snapshots without account types get no default until access is refreshed. - `governedToolArguments` in `tool-gateway.ts` fills an empty `organization` for `create_repository` and `fork_repository` on managed GitHub connections. The change occurs before policy, approval signing, previews, and audit. Thus agent calls, approval cards, and connection tests use the same owner. - The grant lookup runs only for these two tools, and only when `organization` is empty. If the credential is unavailable, the call does not change and fails with its own error at dispatch. - An explicit owner does not change. A fork into its own owner does not change. Token (PAT) connections, personal installations, and multiple organizations keep GitHub's behavior. - `projectedConnectionToolInputSchema` describes the default on the `organization` argument. Agents and the connection test form see where the repository goes. - Updated `doc/connections/GITHUB.md`. ## Verification - `pnpm --filter @paperclipai/server typecheck` passes. - `npx vitest run src/services/tool-access/github-default-owner.test.ts`: new unit tests for the owner rules (single organization, personal installation, several organizations, another person's account, older snapshots, blank values, forks into the same owner, schema text). - New gateway test in `src/__tests__/tool-gateway.test.ts`: "defaults GitHub repository creation to the only installed organization for agents and connection tests". It covers an agent call, an explicit owner, a connection test call, and a personal installation. It fails without the gateway change. - Related suites pass, 393 tests in 9 files: `tool-gateway.test.ts`, `tool-gateway-service.test.ts`, `connection-instructions.test.ts`, `github-access-errors.test.ts`, `github-default-owner.test.ts`, `tool-gateway-discovery-http.test.ts`, `github-operation-credentials.test.ts`, `project-git-authorization.test.ts`, `prompt-injection-gateway.test.ts`. - Not tested against a live GitHub App installation. ## Risks - Low risk. The default applies only when the grant snapshot shows one organization installation and no personal installation. Without the change, these calls fail with 403. - A stale access snapshot can choose the organization after a user installs the App on the personal account. Installation events and **Refresh access** update the snapshot. - An approval that was created before this change, without `organization`, no longer matches the projected arguments. That approval cannot run and must be requested again. Without the change, it fails with 403. - `gh repo create` and `gh repo fork` in agent shells do not use the gateway, so they keep GitHub's personal default. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (code reading, shell commands, and test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
8e3b64a257 |
fix(runner): an unknown agent id says which id and how to find the right one (#20135)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents delegate by creating subtasks through Paperclip's tools. A tool error must tell the agent how to recover, or it improvises > - Restart chaos t06 ended a delegation with four children instead of two > - Evidence showed no replay: the model built an assignee id from two real agents' halves and got a bare `paperclip_runner_agent_not_found` four times > - To debug the error it created "diagnostic test" subtasks without an assignee. Those succeeded and were left behind as duplicates > - This pull request makes the error say which id was not found, which real agents it most resembles, and to call `list_agents` > - The benefit is that an agent corrects the id instead of creating throwaway tasks ## Linked Issues or Issue Description Refs #19831 (durable ownership plan). Restart chaos wave `w-20261108T225520-s9501`, task t06 (claude, M1, K2 drain). **Problem:** parent run 2751fe3c created Part 1 correctly. For Part 2 it passed `assigneeActorId: 0cb62ac2-742d-442c-acc9-d21dd936e716`: Chaos claude's prefix with Chaos codex's suffix. - Four `create_task` calls (keys `part2`, `part2` again, `-v2`, `-backlog`) each returned only `paperclip_runner_agent_not_found`. One of them crossed the K2 drain and failed the same way as the one before it. - The model then created "Part 2 06f119cf test" (`-noassignee`) and "Part 2 06f119cf" (`-title-test`), both described as "diagnostic test - will be removed/ignored". With no assignee they defaulted to the caller. They stayed in backlog. - Only after `list_agents` did it create the real Part 2. - Same-key replays were never the issue: `create_task` stores the `idempotencyKey` as the child's `origin_id`. ## What Changed - `server/src/services/native-runtime/paperclip-runner-tool-authority.ts`: - `create_task` (assignee) and `get_agent` now fail with `paperclip_runner_agent_not_found: no agent in this company has id "<id>". Closest ids: <name> (<id>), …. Call list_agents and copy a valid id exactly; nothing was created or changed.` - Suggestions are up to three agents of the caller's company whose ids share at least eight leading or trailing characters with the missing id. Another company's agents are never named. - The recoverable-read check matches the error code before the `:` details, so a recovered `get_agent` read still returns the error as a tool result. ## Verification Red/green: - `paperclip-runner-tool-authority.test.ts` "names a missing agent id and its closest real agent instead of a bare code (restart chaos t06)". It uses t06's spliced id, with a second agent "Chaos claude" in the company. - On master: the error is the bare code (red). - With this PR: both `create_task` and `get_agent` name the id, `Chaos claude (0cb62ac2-…)` and `list_agents`, and no task is created. - The file passes (42 tests), including the existing foreign-company assignee rejection. `tsc --noEmit -p server` is clean. ## Risks - One extra read of the company's agents, only on the failure path. - The message names agents of the caller's own company, which `list_agents` already returns to it. - No schema or persisted state change. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above (#20069, #20108) - [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 (no user docs affected) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending) - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
c49ab6b9ee |
feat(issues): wake the parent once when a child or blocker stalls on the board (#20098)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A parent task delegates to children and waits on blockers. Its agent learns of progress only through wakes > - Today only completion (`issue_children_completed`) and resolution (`issue_blockers_resolved`) wake a waiting issue > - In restart chaos `w-20261108T181313-s9701` t05, a blocker stopped on a recovery action that needs the board, and its parent sat silently blocked for 50+ minutes > - This pull request wakes the waiting agent once per such stall, tells it what it can do, and shows the stall on the parent's page with the child's Retry > - The benefit is that a stuck child or blocker no longer silently strands its parent ## Linked Issues or Issue Description Refs #19831 (durable ownership plan; restart chaos campaign). Restart chaos wave `w-20261108T181313-s9701`, task t05 (M1, claude). **Problem** From the kept database: - CHA-8 `blocks` CHA-5. - CHA-8's board-owned recovery action (`native_session_retry_exhausted`) was created at 18:44:02.909 and resolved with outcome `blocked` at 18:44:08, when CHA-8 moved to `blocked` for the board. - CHA-5's only wake in the whole wave was its `issue_assigned` at 18:20:48. Its agent never heard about CHA-8. **Expected behavior** - When a child or blocker stalls on the board, the waiting agent is woken once. It can then re-delegate, escalate, or block with a reason. - The board sees the stall on the waiting task, with the child's Retry. ## What Changed - **Shared:** - `ISSUE_BLOCKER_STALLED_WAKE_REASON` (`issue_blocker_stalled`), `ISSUE_BLOCKER_STALL_RELATIONS`, and the types `IssueBlockerStallRelation` and `IssueBlockerStalledWakeContext`; - `Issue.stalledDependencies`. - **Server, `services/recovery/blocker-stall-wakeups.ts`:** - A stall is an issue that is not done or cancelled, whose latest recovery action needs the board. That means either open (`active`/`escalated`) and board-owned or escalated, or board-owned and resolved as `blocked` while the issue is still `blocked`. The second case is what t05 needed: recovery resolves its action that way when it hands the issue to the board. - Waiters are the parent, and every issue the stalled one `blocks`, when they are open, visible and agent-assigned. - Each waiter gets one wake per stall. The idempotency key `issue_blocker_stalled:{waiter}:{stalled}:{recoveryActionId}` is checked through the indexed `(company_id, payload->>'issueId')` path. A later stall has a new action id, so it wakes again. - Each wake writes activity `issue.blocker_stalled_wake`. - The pass runs from the recovery-action sweep (`reconcileActiveRecoveryActions`) and from the per-issue blocked sweep (`reconcileResolvedDependencyWakeBackstop`, which visits each blocked issue). No new worker. - A waiter's agent that cannot read the stalled issue (`issue:read`, which honors private visibility) is woken without its title, identifier or recovery details; the activity entry withholds them too. - **Database:** migration `0450` adds the partial unique index `agent_wakeup_requests_issue_blocker_stalled_uq` on `(company_id, idempotency_key)` for `issue_blocker_stalled:` keys in delivered or pending statuses, so two concurrent sweeps cannot both deliver a stall. It is built `CONCURRENTLY IF NOT EXISTS` outside a transaction (listed in `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS`). - **Queue:** `allowsDependencyBlockedWake` (in `run-dispatch/domain/wake-context.ts`) lets a stall wake run while the waiter's blockers are unresolved, at both dependency gates. Without it the wake was parked until the stalled blocker resolved, which is never. Pause holds are unchanged. - **Agent guidance:** the wake payload carries `blockerStall`, and `renderPaperclipWakePrompt` adds a "Stalled dependency" section. It names the task and its recovery action and the board's next action, then the three choices: re-delegate, escalate with a comment, or block with a reason naming the stalled task. - **Other reason lists:** `issue_blocker_stalled` joins the responsible-user wake reasons, the conversation execution-wake reasons (it never drives a chat turn) and the wake diagnostic's known reasons. - **Board surface:** - `GET /issues/:id` returns `stalledDependencies`, readable issues only (`readableIssueEdgeIds`). - `StalledDependencyNotice` shows on the issue's chat notices, next to `IssueBlockedNotice`: each stalled child or blocker, its board next action, a link, and the child's own Retry (`useTaskRetry`), offered only when the child's interruption allows it. ## Verification Red/green: - `server/src/__tests__/issue-blocker-stall-wakeups.test.ts` (14 tests). - On master: no `issue_blocker_stalled` wake is raised. - With this PR: the parent is woken once per stall, and re-woken after a new stall. A blocker's dependent is woken. - The t05 shape (an action resolved as `blocked`) wakes once, and nothing more after a Retry. - No wake for an agent-owned action, a done, unassigned or hidden waiter, or an action resolved another way. - `listStalledDependencies` lists children and blockers. - `heartbeat-dependency-scheduling.test.ts` "admits an issue_blocker_stalled wake while a blocker is unresolved". On master the wake was parked (`null`); once admitted, the run-start gate cancelled it. With this PR it runs with `dependencyBlockedInteraction`. The file: 14 passed. - `packages/adapter-utils` `server-utils.test.ts`: the new prompt test failed on master (no `blockerStall`). The file: 141 passed. - `ui/src/components/StalledDependencyNotice.test.tsx`: 3 passed. `IssueChatThread.test.tsx`: 109 passed. Regression and static checks: - Nearby server suites (recovery durable wait, dependency wake routes, recovery actions, run-preparation): 149 passed. - `tsc --noEmit` passes for `packages/shared`, `server` and `ui/src`. - `pnpm check:token-gates` is clean. Review round 1 (Greptile, Superagent), each with a regression: - A backlog waiter is never woken. The waiter's status and assignee are guarded at enqueue. - One stall per run: no coalescing, so a merged run cannot keep only the last stall's context. - Only a delivered or pending wake ends a stall. A refused one (wake-on-demand off) is retried after 10 minutes, and an agent that takes no wakes is skipped without writing a row. - Company-wide passes resume after the last stall a capped pass reached. The per-issue sweep also checks the issue's own children and blockers, which covers an unassigned stalled blocker. - The dependency gates no longer trust the reason string. `verifyBlockerStallWake` requires a system request, the stalled issue's current stall action, and a real child or blocker edge. An agent-requested wake with the same context is parked (tested). - The notice shows a refused Retry's reason, and a lost response as "Taking longer than usual…", with Retry still available (tested). Review round 2 (Greptile), each with a regression: - Race: two passes claiming one stall leave one delivered row (unique index; the loser's insert is skipped). - The company recovery sweep selects companies whose only stall is a resolved-`blocked` board action, so an unassigned stalled blocker with a todo waiter is reached. - Private stalled issue: the waiter's wake and activity carry no title, identifier or recovery details. - `GET /issues/:id` always returns `stalledDependencies`, so a cleared stall replaces the cached list. - The notice shows a failed child read with a Reload button (tested). - `pnpm --filter @paperclipai/db check:migrations` passes. Review round 3 (Greptile), each with a regression: - A stall wake that fails to enqueue is retried: the recovery-action pass reports `blockerStallWakeFailures` and the dependency pass counts it in `enqueueFailed`, so the worker throws and retries at its backoff (`issue-recovery-work-lifecycle.test.ts`). - The notice drops a stall once the child's refreshed status has moved on (`StalledDependencyNotice.test.tsx`, 7 tests). ## Risks - A deploy wakes the waiters of every issue that is stalled today, once each. That is intended (they are stranded now), and it's bounded at 200 stalls per pass. - A stall wake may run while blockers are unresolved, like a comment wake. It is bounded to one per stall, and pause holds still apply. - The agent skill (`skills/paperclip`, CODEOWNER-owned) does not yet list the new reason. The wake prompt carries the guidance. - One additive index, built concurrently; no persisted run state change. Existing rows cannot violate it before deploy, since no `issue_blocker_stalled:` keys exist yet. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
e194e1930b |
fix(native): prove a drained remote start had no turn when its controller only read the goal (#20137)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a graceful drain suspends a remote native run during its start, restart recovery may bootstrap it again only if the run's Postgres controller checkpoint proves that no provider turn exists > - That proof accepted `run.prepare`, `session.open` and session-open events > - A codex controller reads the session goal (`session.goal.get`, a read-only query) right after it opens the session and before it sends the first turn. A drain that lands in that window leaves a third command in the checkpoint > - The proof refused any checkpoint with a third command. Recovery called it `ambiguous_remote_recovery_evidence` and blocked replay, so the task waited for the board > - This pull request lets the proof accept the runner maintenance and read-only session commands that already count as maintenance in the local proof, and reads every recorded command > - The benefit is that a run drained in the first seconds of a per-turn Daytona start bootstraps again on its own ## Linked Issues or Issue Description Refs #20136 **What happened?** Daytona per-turn restart-chaos wave w-20261108T225520-s9501 (master |
||
|
DottaandClaude Opus 5.5
|
6451d2bfbf |
fix(native): verify a staged attachment restart on the claimed sandbox lease (#20136)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A follow-up codex run on a per-turn Daytona sandbox gets a fresh sandbox, with harness state restored from the previous turn's checkpoint, and attaches the previous runner session to the new run through a staged attachment > - A graceful restart during that start suspends the run. The next controller reattaches to the surviving remote runner, after proving it is on the sandbox lease the restart claim names > - The staged-attachment path required the lease to have been acquired as `resumed`. A per-turn follow-up's lease is `created`, so every such restart was refused with `native_runner_staged_attachment_recovery_unproven` > - The run then needed a Retry > - This pull request binds the staged restart to the claimed lease the same way the ordinary remote reattach does: same provider lease, same remote directory, and not a replacement sandbox > - The benefit is that a per-turn follow-up interrupted by a restart continues on its own ## Linked Issues or Issue Description Refs #20103 **What happened?** Daytona per-turn restart-chaos wave w-20261108T225520-s9501 (master |
||
|
DottaandClaude Opus 5.5
|
4420a5adcf |
fix(issues): a Retry held by a blocked subtask names it instead of failing silently (#20109)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board's Retry is the one action a person takes when a task stops; it must either start work or say why it can't yet > - Restart chaos t05 left a parent task blocked on a blocked subtask. The board clicked Retry twice; each was accepted and nothing happened > - The Retry was saved and the queue deferred it behind the unresolved blocker, which is correct. But the task still looked failed with Retry offered, so the wait was invisible > - This pull request presents a saved Retry that waits only on unfinished blockers as waiting, names the blocked subtask and links it > - The benefit is that the board sees what to act on (the subtask's own Retry) instead of clicking a Retry that cannot run ## Linked Issues or Issue Description Refs #19831 (durable ownership plan). Restart chaos wave `w-20261108T200620-s9701`, task t05 (claude, M1, K4). **Problem:** parent CHA-5 was blocked by subtask CHA-10, which was itself blocked on a board recovery action. The board pressed Retry on CHA-5 at 20:27:46 and 20:40:46. Each `POST /issues/:id/retry` returned 202, and the saved Retry stayed `deferred_issue_execution` behind CHA-10, so no run started. The task's interruption presentation kept saying `failed` with `canRetry: true` and gave no reason. ## What Changed - `server/src/services/task-interruption-presentation.ts`: - For tasks with a saved (deferred, user-requested) Retry, the loader reads the queue's own dependency readiness (`listDependencyReadiness`). That counts cancelled blockers and done blockers whose workspace has not finalized, like the queue does. - When the saved Retry for the latest run is held only by unfinished blockers (no execution wait, no preparation), the task is presented as `working` with `canRetry: false` and `retryWait: { reason: "dependency_blocked", message }`. - The message has three forms: - a blocked blocker: "Retry or unblock it"; - a cancelled blocker: "Remove or replace that dependency to continue", because the queue keeps it unresolved; - otherwise: waiting for the blockers to finish. - The presentation carries no blocker names or ids. Issue reads return it to restricted agents too. - The Retry response (`POST /issues/:id/retry`) already returns this presentation, so the API says the Retry is waiting on dependencies as soon as it is saved. The blockers themselves are in the issue's `blockedBy`, which issue reads already filter per viewer. - `ui/src/components/ExecutionBlockerNotice.tsx` takes `blockers` (the issue's `blockedBy`, from `IssueDetail`). For a `dependency_blocked` wait it lists each readable unfinished blocker with a link and what it needs: "Blocked: open it to Retry" or "Cancelled: remove or replace it". Tokens only; `pnpm check:token-gates` is clean. - No shared type change, and no code comments added. ## Verification Red/green: - `server/src/__tests__/task-interruption-query.test.ts` "explains a saved Retry held by unfinished blockers instead of offering Retry again (restart chaos t05)". It sets up a parent with a failed run, a saved Retry deferred, and a blocked subtask that blocks it. - On master: `{ state: "failed", canRetry: true }`, no reason. - With this PR: `working`, `canRetry: false`, `dependency_blocked`. The presentation has no blocker title or identifier. - A cancelled blocker gets the remove-or-replace message. Once the blocker is done, the dependency wait is gone. - `ui/src/components/ExecutionBlockerNotice.test.tsx`: the notice links the blocked and the cancelled blockers with their actions, skips the done one, and offers no second Retry. Regression: `task-interruption-query` (25 tests) and `ExecutionBlockerNotice` (16 tests) pass, as do the presentation unit tests, workspace-recovery admission, gates and copyback tests, and `heartbeat-dependency-scheduling`. `tsc --noEmit` is clean for `server` and `ui`. ## Risks - One more batched read (dependency readiness plus blocker statuses), only for tasks in the page that have a saved Retry. - Presentation only. Admission is unchanged: the queue still runs the saved Retry when the blockers are done. - A Retry saved before a blocker was added shows the same wait, which is accurate, because the queue holds it too. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above (#20069) - [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 (code comments; no user docs affected) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending) - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
77168d3367 |
fix(recovery): settle a dead controller's result lease for a run a pause cancelled (#20113)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop pauses the agent, which cancels its running work and waits until every execution has provably stopped > - The shared ownership reconciler (#19933) settles an accepted result's executor lease when its controller is proven dead, so finalization does not wait out the 20-minute lease > - In the restart-chaos H1 wave, the pause cancelled a run 0.36 s after its result was accepted, and a hard kill then took the controller > - The settle only acted on runs that were still `running`, so the dead lease held finalization, the agent stayed in `pausing`, and every later task for it was stranded > - This pull request settles that lease for a cancelled run too, and records the budget pause with its reason > - The benefit is that a pause crossed by a restart completes at once and resume continues normally ## Linked Issues or Issue Description Refs #19933, #20101 **What happened?** Restart-chaos wave w-20261108T210118-s9802 (scenario H1). An agent-scope budget hard stop paused agent "Chaos claude". The pause cancelled run `dfc22298` at 21:02:27.818, 0.36 s after its result was accepted (finalization phase `workspace_finalizing`, executor lease until 21:21:55, controller PID 78157). A K4 SIGKILL followed. After the board's `raise_budget_and_resume`, the agent stayed `status=paused`, `lifecycle_state=pausing`, `lifecycle_holds=[]`, with its lifecycle operation at `attempts: 90`, `hostComplete: false` and no error. The pause host step (`stopInvocationsForAgents` → `agentExecutionsHaveStopped`) returns `pending` while any coordinator holds an owner lease with an accepted result. `settleNativeAcceptedResultLease` required `heartbeat_runs.status = 'running'`, so it returned `lease_no_longer_held` for the cancelled run. The pause also wrote no activity entry. **Expected behavior** A proven-dead controller's accepted-result lease is settled whatever stopped the run, so finalization finishes and the pause completes. The budget pause is logged with its reason. **Steps to reproduce** Pause an agent right after its run's result is accepted, then SIGKILL the server. After restart, the agent stays in `pausing` until the lease expires (up to 20 minutes). ## What Changed - `native-accepted-result-lease-settlement.ts`: settle the lease for a `running` or `cancelled` run. All other guards are unchanged: exact owner compare-and-set, owner proven gone by `proveOwnerGone`, remote copyback excluded, another host never proof. - `agent-lifecycle.ts` `applyAgentBudgetHoldInTransaction`: when the budget hold is added or removed, records `agent.paused` / `agent.resumed` with `reason: "budget"`, the lifecycle state and the holds, in the transaction that applies the hold. Applying the same hold again records nothing. - `budgets-service.test.ts`: its activity-log mock now also stubs `persistActivity`. Not changed, by design: resume still waits for an incomplete pause to finish before resuming. The pause's host step proves the agent's executions stopped before new work starts. In H1 it was blocked only by the dead lease, which is now settled at restart. ## Verification - `heartbeat-native-restart-suspension.test.ts`: new "finalizes an accepted result whose run a pause cancelled before a hard kill took its controller". It fails on master (lease still 20 minutes out) and passes with the fix (lease owner cleared, finalization past `workspace_finalizing`). Whole file: 41 passed. - New `agent-budget-hold-activity.test.ts`: the pause and release entries, with the reason, written once each. - `budgets-service.test.ts` (19), `budgets-history.test.ts`, `company-portability-routes.test.ts`, `agent-lifecycle/__tests__/lifecycle.test.ts`: passed. - `server` typecheck (`scripts/typecheck-layers.mjs`, low-memory): no errors. - Not run: the H1 chaos rerun (Phase 3 schedules it after merge). ## Risks - A cancelled run's accepted result is now finalized promptly after a hard kill instead of after the lease term. The finalizer already handles a run cancelled during finalization; the settle only shortens the wait. - New activity entries on budget pause and release. Run-log and activity only; no Telemetry. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
776f4cb602 |
fix(native): restart an interrupted remote bootstrap on a resumed reusable lease (t06) (#20103)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run on a remote sandbox must survive a control-plane restart; a bootstrap interrupted before its first turn is restarted on the same run > - With a reusable (warm) sandbox lease, the restart always resumes the same sandbox. The interrupted runner's state is still on disk there: never suspended, and with no provider turn > - The resumed check accepted only a suspended checkpoint or no state at all, so it failed closed with `resumed_not_suspended`, and the task needed a Retry (restart chaos t06) > - The pre-phase had already proven no turn was sent and had stopped the stale runner, but that proof never reached the resumed check > - This pull request carries that proof forward, for this run only, and starts the bootstrap again on the resumed sandbox > - The benefit is that a warm task interrupted during startup continues on its own after the restart ## Linked Issues or Issue Description **What happened?** In restart-chaos wave w-20261108T192459-s9501 (Daytona warm, claude C1), run 9bf86b59's runner reached transport-ready at 19:37:42 UTC. The control plane was then down (another task's K4, then this task's K1). Restart recovery classified the run `bootstrap_incomplete` and restarted it on the same run at 19:41:51. The reusable lease resumed the same sandbox. `inspectRemoteHarnessState` found this run's runner state with `lifecycle` other than `suspended`, and the run failed at 19:41:59 with `runner_ingress_unavailable: runnerd process failed: runner_harness_state_mismatch: resumed_not_suspended`. The task needed a manual Retry. Per-turn and non-reusable leases do not resume, so they take another branch and were unaffected. **Expected behavior** A same-run restart of a bootstrap that provably sent no turn starts the bootstrap again on the resumed sandbox, with no Retry. It must never take over a live provider session or another run's warm session. **Steps to reproduce** 1. Run a task on a reusable (warm) Daytona lease. 2. Kill the control plane after the runner connected and before the first turn, so the run is `bootstrap_incomplete`. 3. Before this change, the restart fails with `resumed_not_suspended` on the resumed lease. **Paperclip version or commit** master at |
||
|
DottaandClaude Opus 5.5
|
c4ba3974a7 |
fix(heartbeat): close run admission before a graceful shutdown closes its listener (t09) (#20102)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A graceful restart (K2) drains the control plane: it closes HTTP admission, suspends running runs, and the next boot resumes them > - Run admission stayed open while the HTTP listener closed. Heartbeat's shutdown flag was set only later, inside hot-restart preparation > - A run that finished in that window released its issue and promoted the queued comment wake. That started a new run that had no callback listener > - A remote warm continuation then failed with `local_runtime_api_listener_unavailable`, and the task needed a Retry (restart chaos t09) > - This pull request closes run admission before the listener closes, so the promoted wake stays queued for the next boot > - The benefit is that a comment sent during a deploy continues on its own after the restart ## Linked Issues or Issue Description Refs #19596 **What happened?** In restart-chaos wave w-20261108T192459-s9501 (Daytona warm, Codex C1, K2), a K2 SIGTERM started a graceful drain at 19:42:52 UTC, and the HTTP listener closed first. At 19:43:03 the running turn finished and released its issue. That promoted the queued comment wake, and `startNextQueuedRunForAgent` started a new run about 11 s into the drain. Backend setup resolved the local runtime API URL, found the listener closed, and threw `local_runtime_api_listener_unavailable`. The remote target is excluded from the closed-listener handoff, so the run failed, and the task waited for a manual Retry. **Expected behavior** No queued run starts once a graceful shutdown has begun. A wake promoted during the drain stays queued, and the next boot starts it with no Retry. **Steps to reproduce** 1. Run a remote warm task and post a comment while its turn runs, so the comment wake queues behind it. 2. Send SIGTERM to the control plane just before the turn finishes. 3. Before this change, a new run starts during the drain and fails with `local_runtime_api_listener_unavailable`. **Paperclip version or commit** master at |
||
|
DottaandClaude Opus 5.5
|
f789f3e2af |
fix(environments): a consented environment delete releases restore-source protection so it never stays blocked (#20099)
## Thinking Path - Deleting an environment with consent destroys its kept sandboxes. #20085 added failed runs' restore-source sandboxes (`retain_on_failure`) to that teardown, and the delete dialog now tells the operator that the consented delete destroys those files. - But every teardown path refuses a lease whose sandbox carries a `workspaceRestoreRecovery` marker (`legacyShutdownWorkspaceResourceProtected`). That is correct for ordinary cleanup, since the files may be the only copy. - So the consented delete claimed the restore sources for cleanup, every teardown returned without destroying, and the delete returned 409 `pending_sandbox_cleanup` forever: a refusal the user cannot act on. - The operator's consent is exactly the decision the protection waits for, so the delete releases it explicitly and audits who did, then proceeds. ## Linked Issues or Issue Description Refs #20048, follow-up to #20085 **What happened?** In Daytona warm wave w-20261108T192459-s9501, the harness's consented environment delete returned 409 `pending_sandbox_cleanup` for minutes (it retried every 3 s). Two restore-source leases (8b26e044, bcb897c9) had been claimed for cleanup with `environment_deleted`. The activity log shows a cleanup attempt on every retry, each returning without destroying. On the kept database copy, `legacyShutdownWorkspaceResourceProtected` returns `true` for both, because of their `workspaceRestoreRecovery` marker. Their restore recovery had already succeeded (workspace sync finalized). Phase 3 deleted the two sandboxes by hand. **Steps to reproduce** 1. A failed run leaves a `retain_on_failure` lease with `workspaceRestoreRecovery` metadata (sandbox stopped). 2. `DELETE /api/environments/:id?destroyReusableSandboxLeases=true`. 3. The lease is claimed (`pending_cleanup`), the teardown refuses it, and every retry of the delete returns 409. **Expected behavior** The consented delete destroys the restore source (the operator was told the files go with it) and the environment is deleted. No 409 the user can't act on. ## What Changed - `server/src/services/environment-runtime.ts`: new `releaseRestoreSourcesForEnvironmentDelete({ environmentId, consentedBy })`. For each of the environment's restore-source sandboxes (`retain_on_failure` with `workspaceRestoreRecovery`, any non-terminal status including an earlier `pending_cleanup` claim), it removes the marker from every lease row on that sandbox and records `restoreSourceReleasedByEnvironmentDelete: { consentedBy, releasedAt }`. `destroyReusableSandboxLeasesForEnvironment` takes `consentedBy` and releases first. - `server/src/routes/environments.ts`: a consented delete whose only blockers are pending cleanup or kept sandboxes releases restore protection before the pending-cleanup retry, so leases an earlier attempt left claimed are freed too. It then passes `consentedBy` to the teardown. - Tests: - A restore source now carries the real `workspaceRestoreRecovery` marker, and the consented teardown destroys it. This failed before the fix (`{ destroyed: 0, failed: 1 }`), matching the wave. - The route test asserts the release runs before the pending-cleanup retry. ## Verification - `cd server && npx vitest run src/__tests__/environment-runtime.test.ts src/__tests__/environment-service.test.ts src/__tests__/environment-routes.test.ts`: 347/347 pass. - The restore-source test fails on master with `failed: 1`. ## Risks - **Losing restore files without consent.** The release runs only on the explicit consented delete path (`destroyReusableSandboxLeases=true`), and only when the remaining blockers are pending cleanup or kept sandboxes. It is recorded on every released row. Leases with a pending native export are still skipped by the teardown. - **Ordinary cleanup.** Unchanged: restore protection still holds everywhere else. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
1b7254206f |
fix(recovery): replay a preparation a restart interrupted once proof shows no provider work started (#20072)
## Thinking Path - Restarts are product failures: a task interrupted by a restart must resume on its own, never wait for a user Retry. - A run interrupted during preparation, before any provider work, is safe to replay; the agent never touched the workspace. Paperclip already has the proof (`isInterruptedNativePreparation`) and the settle (`settleInterruptedNativeBootstrap`) that replay such a run in the uncharged interrupted-preparation lane. - But a graceful shutdown terminalizes that run inside the draining process, where the proof cannot hold yet: the owning controller is still that process, and its sandbox lease may still be cleaning up. So recovery records `preserve_without_replay_v1` with replay blocked. - When the proof becomes available (the next process boots, and the lease is released with a confirmed stop), nothing re-judges the hold. The only re-evaluation point runs after a pending-cleanup sweep, so the task waits for a manual Retry. - The coordinator set the rule: replay automatically with durable proof of no provider work; anything uncertain stays held. This PR adds the re-evaluation point and no new proof logic. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T142906-s9501, task t05: scheduled retry c99b3a2e was 2 s into preparation (legacy, stage `preparing`, no runtime mode, no events, no finalization) when the K2 graceful shutdown interrupted it. Terminalization in the draining process recorded `legacy_execution_requires_reconciliation` with `preserve_without_replay_v1` and replay blocked, while its lease was still `pending_cleanup`. The lease was released with a stop receipt 2 s later, and the next process started, but the hold was never re-judged. The harness clicked Retry at 14:44:27. **Steps to reproduce** 1. Start a run that is still in preparation (legacy, `preparing`, adapter `paperclip_runner`). 2. Gracefully shut down the server; terminalization records a preserve-without-replay hold. 3. Let its lease release with a confirmed stop, and restart the server. 4. The hold stays `blocked`; the task needs a manual Retry. **Expected behavior** Once the owning controller is proven gone and every lease is released with a confirmed stop, the ownership reconciler re-judges the hold with the existing proof. With no provider work and every cleanup confirmed, the hold is settled and the preparation is replayed with no attempt charged. Possible provider work, a live controller, or an unfinished cleanup keeps the hold. ## What Changed - New ownership kind `legacy_interrupted_preparation_hold`, in `server/src/services/interrupted-preparation-hold-settlement.ts` and registered in `recovery/ownership-registry.ts`. - **Listing:** legacy runs that are `interrupted` or `failed`, in stage `preparing` with no `runtimeModeResolvedAt`. Each must have a system-recorded `legacy-execution:<run>` hold: resolved/blocked, `preserve_without_replay_v1`, replay blocked, no owning user, and none of `ownerGoneAttestation`, `explicitUserContinuation` or `executionReconciliation`. Every lease must be released with no failed cleanup, so a lease that settles later is listed on the next pass. - **Owner:** the run's recorded controller (`legacyControllerOwnerStamp`). The shared reconciler settles only after `proveOwnerGone` proves it gone. A missing stamp, or one from another host or PID space, is unprovable and stays held. There is no age fallback. - **Settle:** re-checks the existing durable proof (`isInterruptedNativePreparation`: no coordinator, result or execution events; leases released with stop receipts; workspace repair when needed). It then resolves the hold through `settleInterruptedNativeBootstrap` as `false_positive` with replay allowed and continuation pending, delivered through the existing workspace-recovery queue in the uncharged interrupted-preparation lane. Its row locks make the transition happen once; the settle reports settled only when it resolved holds. - `server/src/services/legacy-execution-recovery.ts`: `settleInterruptedNativeBootstrap` takes an optional `proof`, recorded in the `issue.recovery_action_resolved` activity details as `ownerProof`, and an `onHoldsResolved` callback. - `server/src/__tests__/interrupted-preparation-hold-settle.test.ts`, K2-during-preparation shape, run through the reconciliation pass: - settled with a dead controller in this PID space, with the audit entry naming `pid_dead`; - kept on a `turn.started` event, a stamp from another process space, a missing stamp, the current process, a lease still cleaning up, an operator owner-gone attestation, and a user continuation; - two concurrent settles produce one resolution. - `ownership-reconciliation-report.test.ts`: adds the kind to the list of settling kinds. **Previous-release state:** holds written by the previous release (`preserve_without_replay_v1`, replay `blocked`) are what this re-judges, through the same proof. A run without a recorded controller stamp stays held. No saved state changes shape. ## Verification - `cd server && npx vitest run src/__tests__/interrupted-preparation-hold-settle.test.ts`: 9/9 pass. Removing the kind registration makes the settle case fail. - The ownership, retained-orphan and recovery suites (19 files) pass, after adding the kind to the report's settling-kinds list. - Not yet covered: the Daytona warm wave rerun, on measure/remote-warm with this change. ## Risks - **Replaying work that already ran.** Settlement requires both the shared owner proof and the existing durable no-provider-work proof. Anything unprovable stays held. - **Overriding an operator.** Only holds the system recorded are listed; attested, continued or owned holds are never touched. - **Load.** The listing is a filtered query on the 30 s reconciliation pass, gated so a run with an unsettled lease is not polled. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
d007afbf86 |
fix(native): let a follow-up wait out the previous turn's warm release instead of failing busy (#20068)
## Thinking Path - With a warm lifecycle, one session serves consecutive turns on a task. A follow-up comment wakes the next run, which claims the warm session's instruction copy and reuses the session. - #20048 makes warm the default on remote sandboxes. Its merge condition includes "no restart or follow-up ever leaves a task needing Retry". - In the #20048 Daytona chaos wave, task t12's follow-up run was created 43 ms after the previous run settled. The previous run was still releasing its warm session, so `claimWarmNativeInstructionCopy` saw it busy and failed the run with `native_session_supervisor_busy`. The task then needed a Retry. - The release takes seconds and always finishes, so a follow-up should wait for it rather than fail. A session busy because another run is preparing or executing a turn is a genuine conflict and must still fail at once, as existing tests require. ## Linked Issues or Issue Description Refs #20048 **What happened?** Wave w-20261108T142906-s9501, t12 (claude, warm): run 5b839da4 succeeded at 14:46:36.224. The follow-up run f2bf2f05 was created at 36.267 and failed immediately with `adapter_failed` / `native_session_supervisor_busy`, from `claimWarmNativeInstructionCopy` called by `instruction-working-copy.prepare`. Its automatic retry also failed, and only a user Retry completed the task. **Steps to reproduce** 1. Run a warm native session turn to completion. 2. While that run is still releasing its warm session (post-turn instruction check, release, hand-back), admit the next run on the same session. 3. The next run's instruction-copy claim throws `native_session_supervisor_busy`. **Expected behavior** The follow-up waits for the previous run's release (bounded) and reuses the session. A session held by a run that is still preparing or executing fails admission at once, as before. ## What Changed - `server/src/services/native-runtime/native-session-executor.ts`: - When the provider turn ends, the executor records the session scope as settling (`settlingNativeSessionScopes`). The outer `finally` clears the mark once the run has handed every scope back. - `claimWarmNativeInstructionCopy` waits, polling every 25 ms for at most 30 s (`warmSessionReleaseWait`), while the warm entry is busy and its scope is settling. It then re-reads the entry and runs the existing checks. If the entry is still busy at the deadline, or busy for any other reason, it throws `native_session_supervisor_busy` as before. - No wait happens on the normal path (entry idle), so turn admission adds no latency. - `server/src/services/native-runtime/native-session-executor.test.ts`: two cases. - A follow-up claimed while the previous turn's post-turn check is held: it waits, then succeeds once the release finishes. It fails on master with the wave's error. - The release outlasts the bound: it fails busy. **Previous-release state:** the change touches only in-memory admission state, so no saved run or session state changes. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/heartbeat/instruction-working-copy.test.ts`: 1301/1301 pass. - With the wait removed (behaviour on master), the "release finishes" case fails with `native_session_supervisor_busy`. - The existing concurrency tests ("fences owned instruction preparation: concurrent", and the settled-release ownership tests) still fail admission at once. - Not yet covered: the Daytona warm wave rerun for #20048. ## Risks - **A wait that never ends.** The wait is bounded at 30 s and happens only while the owner is past its turn, which always reaches the outer `finally`. Every caller of the inner executor runs inside that `try`, and attachment staging keeps the same session scope. - **Hiding a real conflict.** Only a settling owner is waited for. A preparing or executing owner fails at once, as the existing tests assert. - **Scope.** I tried a similar wait at execution admission, but it conflicted with deliberate fences that reject a run during a settled release (the existing "releases settled codex only under its current owner" tests). It is not part of this PR. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
b32ef9f111 |
perf(budgets): judge time to first visible agent activity; keep time to first text as a reported metric (#20077)
## Thinking Path - The native speed budgets (plan invariant 8) judge how fast an agent responds. They judged `task.time_to_first_token`: time to the first deliverable **text**. - In the clean local restart storm (w-20261108T153612-s9401), Claude breached its TTFT budget (cold p50/p95 17.4/34.7 s against 16.5/30). The per-run spans showed this was not a control-plane wait: in all 49 cold runs (and 76% of warm), Claude started a tool call before writing any text. Its first visible activity came at 7.4 s p50 from run creation; first text came at 19.6 s. Codex never calls a tool first. - So first text charges model tool work as latency for tool-first adapters. What the user experiences as "the agent responded" is the first visible activity: a text delta, a tool-call start, or a thinking indicator. - The coordinator decided: budgets judge time to first visible agent activity from run creation, with the budget numbers unchanged; first text stays a reported metric with no budget for tool-first adapters. ## Linked Issues or Issue Description **What happened?** Local storm s9401: the Claude TTFT budget breached on every aggregate, while the control-plane steps (queue 0.05 s, prepare 0.3 s, runner startup ~4 s, first provider event 1.3–1.8 s) were well within budget. The rest of the first-text wait was Claude streaming tool input (about 5.5 s per call) and running tools before it wrote text. **Steps to reproduce** 1. Run a Claude task whose first action is a tool call (most real tasks). 2. Read the run log: `provider.time_to_first_agent_event` ends at the tool call; `task.time_to_first_token` ends many seconds later at the first text. 3. The speed budget judges the later one and reports a breach. **Expected behavior** The budget judges the first visible agent activity from run creation (`task.time_to_first_agent_activity`). Time to first text is still recorded and reported. ## What Changed - `server/src/services/native-runtime/native-session-executor.ts`: when the first meaningful agent event commits (the existing `firstMeaningfulAgentEventKind`: text delta, tool-call start, or reasoning), record `task.time_to_first_agent_activity`. It uses the same detached anchor resolution as TTFT (accepted comment, answered question, durable wake request, or run creation), so it adds nothing to the turn path. - `server/src/services/native-runtime/native-speed-budgets.ts`: the metric is now named `time_to_first_activity` (was `ttft`), with identical budget numbers. The type comment documents the definition and why first text is unbudgeted. - `server/src/services/native-runtime/native-run-trace.ts`: the budget check judges `task.time_to_first_agent_activity` from run creation, keeping the existing input-wait handling. `task.time_to_first_token` is no longer budgeted. - `tests/restart-chaos/ttft-baseline-stats.ts` and `ttft-baseline.ts`: report first activity from run creation (the budgeted value) next to TTFT. - Docs: `doc/observability.md` (new span) and `doc/plans/2026-11-07-ttft-baseline.md` (metric table). - Tests: - A tool-first turn records first activity at the tool call (5 s) while TTFT records first text (16 s). - A tool-first trace with slow first text but fast first activity is within budget. - The existing budget tests now judge the activity span. Observability path (AGENTS.md section 5.7): the new run-log span uses only the existing `eventKind` and `anchor` attributes. Its name survives run-log redaction intact; checked with `redactEventPayload`. The operator OTLP gate is unchanged. It is not Telemetry. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-speed-budgets.test.ts src/services/native-runtime/native-run-trace.test.ts`: 44/44 pass. - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "first visible activity|first assistant text once|first-token timing"`: 7/7 pass. - `npx vitest run --root tests/restart-chaos`: 159/159 pass; `tsc` on `tests/restart-chaos` is clean. ## Risks - **A metric rename.** Consumers of `run.performance.budget_exceeded` see `metric: "time_to_first_activity"` instead of `"ttft"`. In-repo there are none besides the tests; Phase 3's chaos speed grade switches to the new span. - **A looser budget for slow first text.** Intentional: first text is reported, not judged. A regression in control-plane latency still shows up in first activity. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
015d3d0a7c |
fix(environments): tear down retain-on-failure sandboxes when their environment is deleted (#20085)
## Thinking Path - Remote sandboxes cost money while they exist, so every sandbox Paperclip keeps must have an owner and an end. - Two lease policies keep a sandbox after its run. `reuse_by_environment` keeps a warm sandbox for the next turn. `retain_on_failure` keeps a failed run's sandbox, stopped, as a workspace-restore source. - Environment deletion is the last chance to end both: with the operator's consent it destroys the environment's kept sandboxes, otherwise it refuses. - It only looked at `reuse_by_environment`. A `retain_on_failure` sandbox neither blocked the delete nor got destroyed, so it was orphaned the moment its environment was deleted. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T170423-s9501, the harness deleted the environment after grading. Three stopped sandboxes (4e20c8b3, abf0980b, 5e49c561) remained afterwards and had to be deleted by hand through the Daytona API. All three belonged to `retain_on_failure` leases: failed runs' sandboxes kept as workspace-restore sources. **This leaks on master today, with per-turn sandboxes too.** The four leases in that wave carried `workspaceRestoreRecovery` from `legacy-workspace-restore-recovery.ts` (no `nativeSourceRetention` marker). That writer turns any failed run's lease with a workspace-restore failure into a `retain_on_failure` restore source, whether the environment is warm or per-turn. Every such sandbox has been surviving environment deletion and costing money until deleted by hand. **Steps to reproduce** 1. A run in a sandbox environment fails, and its lease is released as a `retain_on_failure` restore source (sandbox stopped, not destroyed). 2. Delete the environment with `destroyReusableSandboxLeases=true`. 3. The delete succeeds; the restore-source sandbox is never destroyed and has no lease or environment left to find it by. **Expected behavior** The delete blast radius counts kept restore-source sandboxes like reusable ones. Without consent, the delete is refused; with consent, they are destroyed through the same claimed teardown before the environment row is removed. ## What Changed - `server/src/services/environments.ts`: `getDeleteBlastRadius` counts leases with policy `reuse_by_environment` **or `retain_on_failure`** (status `active`, `released`, `retained` or `failed`) as kept-sandbox holders. The blocked reason is still `reusable_sandbox_lease`. - `server/src/services/environment-runtime.ts`: `destroyReusableSandboxLeasesForEnvironment` selects both policies. It claims through `destroyClaimedSandboxLease` with `anyLeasePolicy` plus a scope condition limited to those two policies, keeping its existing guards: live-run skip, pending export, legacy shutdown protection, and the claim compare-and-swap. - `server/src/__tests__/environment-runtime.test.ts`: a released `retain_on_failure` lease on a failed run now blocks the delete and is destroyed by the consented teardown (`expired`, `environment_deleted`). This fails on master. ## Verification - `cd server && npx vitest run src/__tests__/environment-runtime.test.ts src/__tests__/environment-service.test.ts src/__tests__/environment-routes.test.ts`: 347/347 pass. - The new test fails on master: the blast radius reports no kept sandbox. ## Risks - **A delete now refused that used to succeed.** An environment holding restore-source sandboxes is refused until the operator consents to destroying them. That is the intended behaviour; it was silently leaking them before. - **Losing restore files.** The consented delete destroys restore sources, which is what the operator agreed to by deleting the environment. Leases with a pending export or legacy shutdown protection are still skipped and keep blocking. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
4f7eee10c2 |
feat(recovery): let the board confirm an unrecorded owner stopped for every settling process kind (#20084)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Recovery releases in-progress records (leases, claims, intents) only
when it can prove their owner is gone
> - A record saved before its server process was fully recorded is
`unprovable`, and it is never released on its own
> - #20067 added a board-only action to confirm that such an owner
stopped, but only the environment run lease used it
> - Other settling kinds (for example the legacy controller lease, which
shows `missing_identity` rows in the Phase 2 reconciler data) stay stuck
with no way forward
> - This pull request opens the same confirmation to every settling kind
whose record names a partly recorded server process
> - The benefit is that an operator can release these stuck records
safely, and recovery settles them at once
## Linked Issues or Issue Description
Refs #20067, #20054
**What happened?**
The Phase 2 reconciler data shows `legacy_controller_lease` records that
stay `unprovable` with `missing_identity`: they were saved before
Paperclip stored the controller's PID space. Recovery never releases an
unprovable owner, and only the environment run lease offered the board's
"confirm the previous server stopped" action. These records, and records
of other settling kinds saved the same way, had no path forward.
Also, a board confirmation did not wake the reconciler. Unprovable
records are not watched, so the release waited for some unrelated pass.
**Expected behavior**
Recovery asks the board on the record's task. After the board confirms,
recovery releases the record through the kind's own compare-and-swap
settle, at once. A confirmation never releases a record that changed
since the board was asked.
**Steps to reproduce**
1. Have a legacy run whose `controller_process` was recorded without a
host hash (an older server).
2. Run the ownership reconciliation pass. The record is `unprovable`
(`missing_identity`) and no board action appears.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
e8a3f1f6f2 |
feat(recovery): prove a replaced container's records gone through a fenced controller epoch (#20054)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane settles in-progress records (leases, claims, intents) only once it proves their owner process is gone > - The proof compares host and PID space, so a stamp from a replaced container or a rebooted machine stays unprovable forever, and its records never settle > - Self-hosted users on Docker, Kubernetes and Fly hit this on every redeploy; a Railway deployment with an exclusive volume is covered by `deployment_replaced` (#20026) > - This pull request adds a fenced controller epoch: each boot holds a database lease, and an expired boot is fenced so its records can be proven gone > - The benefit is that work stranded by a replaced container is recovered, without keeping an idle database awake ## Linked Issues or Issue Description Follow-up to #20026 and the Phase 2 durable-ownership data. Related: #19965 (owner stamps), #19883 (process space). No duplicate PR was found. **What happened?** A record stamped by a server process in another container reports `different_process_space`. A dead PID elsewhere is never proof, so the record stays `unprovable` and is never settled. Every redeploy of a self-hosted container leaves such records behind. **Expected behavior** A record owned by a server boot that was replaced (stopped cleanly, or lost its lease and was fenced) is proven gone and settled exactly once. A live boot's records stay with it. An idle server issues no recurring queries. **Steps to reproduce** 1. Run Paperclip in a container and start work, so an environment lease is stamped by that boot. 2. Replace the container. The new container has a new PID namespace. 3. The ownership reconciler reports `different_process_space` (unprovable) for the old lease, forever. **Paperclip version or commit** `master` before this PR. ## What Changed - New `controller_instances` table (migration 0449): boot id, epoch (bigserial), process space, pid, a lease on the database clock, `fenced_at` / `fenced_by_epoch`, `stopped_at`. - `controller-instance-lease.ts`, registered with the shared delivery-work coordinator (`controller-instance-lease` queue, runs during drain): - **Holds work → renews; idle → silent.** One upsert creates the row on first use and extends the lease only if the boot holds work: a stamp within the last 90 s, or a stamp whose ownership write may still be pending (held until the server's tracked work settles, capped at 10 minutes; a renewal that sees some other held record does not clear it, so a slow ownership write cannot be outlived), or an in-progress record held by its boot id. Those records are active environment leases, in-flight cleanup claims, sandbox creation intents, workspace finalize attempts, agent lifecycle steps, native finalizer and cleanup leases (`lease_holder` bound to the token), open native coordinator claims, and running legacy runs. Renewal runs every 30 s while the boot holds work. When it finds none, the lease lapses and the worker sleeps until the next stamp (`onControllerStamp`). - **No idle trailing statements.** When the server's last tracked work settles, the worker re-checks holdings at once, at most every 5 s (the rate limit also absorbs the worker's own run). So the last check happens with the work, not 30 s later. - **Quiesce, don't crash.** Each renewal has a 10 s bound. A failed or timed-out renewal applies an internal task drain, so no new work is dispatched. A recovered renewal resumes. The boot exits only after 60 s of failure (counted from the first failed attempt's start) while it still holds work, or at once when it finds itself fenced. - **Fence on demand, by a newer epoch.** For a stamp from another host or PID space, the prover reads that boot's row: - `stopped` or `fenced`: gone. - live lease: alive. - otherwise, the boot's last sign of life is the later of its lease expiry and the record's last write plus the TTL. Within 30 s of that, the owner stays `alive` (`controller_lease_grace`), so the settle watch keeps rechecking. After that, the prover fences the boot, but only with a lease row of its own (created on demand) and only an older epoch. - A graceful shutdown records `stopped_at`. - A reconciliation pass that settles nothing (`record: false` or `settle: false`) never fences and never creates the boot's row. - `owner-proof.ts`: these proofs apply only to server-held records. A stamp with a recorded process group stays unprovable, because agent processes can outlive their server. The reconciler passes each record's `updatedAt` to the proof. - `task-admission.ts`: - the quiesce drain never replaces an operator's drain, and has no idle owner, so HTTP stays open; - a `subscribeIdleWorkSettled` hook fires when the last tracked work finishes. - Off on a Railway deployment with an exclusive volume (it has its own replacement proof). `PAPERCLIP_CONTROLLER_INSTANCE_LEASE=on|off` overrides this. A stateless Railway service keeps the lease. A server without its own lease row never fences. - `maintenance-server-routes.test.ts`: in the post-run quiet window, the lease may check once more that the run's records were released. The test now excludes those statements, then asserts that the lease issues nothing in a further window. - Docs: `doc/execution-semantics.md`; the Phase 2 data doc records the two known gaps. ## Verification - `controller-instance-lease.test.ts` (15 cases): - a replaced container with all Railway variables empty is fenced and settled exactly once over two passes; - a live lease is alive; - an owner in grace, and work written after an idle lapse, stay alive and watched; - an older epoch never fences a newer one; - no fencing without its own lease row (off or unregistered), and an idle prover creates its row on demand; - a stopped boot is gone; - process-group stamps stay unprovable; - idle issues zero statements, and one upsert renews only while records are held; - **a 45 s outage:** quiesced, no exit, resumes; - **a renewal that never answers:** bounded, and the boot exits on time; - **a 70 s outage:** exits at 60–65 s, and the successor recovers the work exactly once; - fenced: stops; - the settle re-check; - graceful stop; off on a volume-exclusive Railway deployment, on for a stateless one, and the on/off override; - a running legacy run keeps the lease live. - Red, with the proof's instance check removed: the replaced-container, live, stopped and successor cases fail. - Wider: task-drain, idle-sleep, idle-local-work, owner-proof and ownership suites, plus the maintenance quiet-window tests. Server low-memory typecheck is clean. ## Risks - **Clock skew.** The proof compares a record's `updatedAt` (often the writer's clock) with the lease (database clock). The 30 s grace absorbs ordinary skew. - **A new exit path exists.** A server whose database is unreachable for more than 60 s while it holds work now exits, after first quiescing. This is the intended safety backstop. - **Bounded trailing activity.** A lease that holds work issues one statement every 30 s, plus at most one check when its work settles. An idle server issues none. - **Boots from before this release** have no row, and their records stay unprovable. The board "mark owner gone" action for those follows in a separate PR. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
0a64c9b4ab |
fix(recovery): treat a warm sandbox as abandoned only on positive evidence its task moved (#20070)
## Thinking Path - #20065 made idle warm sandboxes owned records on the shared retained-orphan kind. A warm sandbox no turn can resume is stopped if running, then destroyed. - One branch of that predicate decides that a task "moved" off a workspace. As merged, it was true whenever no task was bound to the lease's workspace. That includes a task whose binding is null: finished, or between bindings. - A null binding is not proof of a move. The warm sandbox may still be resumable, and for a remote workspace it can hold the files. Phase 2's review flagged this as blocking, so this follow-up requires positive evidence. ## Linked Issues or Issue Description Refs #20048, follow-up to #20065 **What happened?** `taskMovedToAnotherWorkspace()` listed a warm lease as abandoned when its task had no bound workspace. The pending-export check was then the only thing preventing the destroy of a sandbox that could still be resumed. **Steps to reproduce** 1. Create a warm (`reuse_by_environment`) lease, released with a confirmed stop, bound to workspace W. 2. Clear the task's `execution_workspace_id` (task finished, or between bindings). 3. Run an ownership reconciliation pass: on master the lease is settled and queued for destruction. **Expected behavior** Only positive evidence counts as a move: the lease's own task is bound to a different, non-null workspace, and no task is bound to W. An unbound task's warm sandbox is kept; a done or cancelled task's sandbox goes through the existing retention paths. ## What Changed - `server/src/services/environment-retained-orphan-settlement.ts`: - `taskMovedToAnotherWorkspace()` now requires that the lease's own task is bound to a non-null workspace different from the lease's, and that no task in the company is bound to the lease's workspace. - `newestRowForSandbox()` breaks `created_at` ties by id, so two rows written in the same millisecond can't both count as newest. - `server/src/__tests__/environment-retained-orphan-sandbox.test.ts`, two keep cases: - the task has no bound workspace, for both a released and a retained warm lease; - two rows share the newest `created_at`. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts`: 32/32 pass. - With master's settlement file, the null-binding case fails, because the lease is settled. The tie case also passes on master, since the settle's existing shared-resource hold protects it; it stays as a regression test for the listing. - The full chain (retained, stop confirmed, listed, handed to destruction) is covered in one test from #20065. ## Risks - **Fewer sandboxes cleaned.** A warm sandbox whose task finished unbound now waits for the existing retention paths instead of this branch. That is the conservative direction, so files are never lost. - **Scope.** No change to the holds or the settle contract. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
a9cf4ee731 |
fix(outbox): deliver a user comment whose wake was lost on a task its agent then closed (#20066)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user's comment on a task with a live run must reach the agent: it is steered into the run or delivered in a follow-up run. Per-comment exactly-once delivery is plan invariant 4 > - A comment commits before its wake is written. A control-plane restart in between loses the wake, and the reconciler's lost-wake rule (chaos s9401/t69) saves the missing outbox entry > - That rule only looked at `todo` and `in_progress` tasks. In the restart chaos Daytona wave s9501, task t06 got a comment 10 ms before a K1 kill. The live run then finished, its agent closed the task, and the comment was never read > - This pull request extends the rule to a task its agent closed, and reopens that task for the user's message, as the release drain already does > - The benefit is that a message sent while the agent works is never silently dropped by a restart ## Linked Issues or Issue Description Refs #19831 (plan, section E: work-intent outbox, invariant 4), and #19992 (the s9401/t69 and t72 rules this extends). Found by the restart chaos campaign, wave w-20261108T142906-s9501, task t06: - The comment was saved at 14:40:17.048. - The K1 kill came at .058. - No wake request ever carried the comment, and no run event shows it steered in. - The run that started before the comment finished at 14:44:22 and closed the task. ## What Changed - `recovery/ownership-invariant-report.ts` (`checkUserCommentsWithoutWake`): also reports a comment on a `done` task its agent closed (`taskClosedByAgentSql`), when no wake carries it, no run started after it, and no run of the task logged it as a steered `userMessage` item. - `work-intent-outbox.ts` (`saveUndeliveredUserComments`): a new `reopenAgentClosedDone` option. In the same transaction that locks the issue, re-checks the agent close and saves the receipt, it reopens the task to `todo` through the issues service. It also logs `issue.updated` (reopened from done, source `undelivered_comment_backstop`). - `native-finalization-reconciler.ts` (`applyOwnershipReportBackstops`): the lost-wake backstop allows `done` and passes `reopenAgentClosedDone`. The receipt keeps its `wake_lost` / `reconciler_backstop` wait, so the existing release path delivers it once. ## Verification - `heartbeat-held-comment-wake.test.ts`, new test "delivers once a comment whose wake a restart lost mid-run on a task the run then closed". It fails on `master` and passes here. The violation is found, the backstop saves and reopens, the comment is delivered exactly once, and a second pass and backstop deliver nothing more. All 25 tests in the file pass. - `ownership-reconciliation-report.test.ts`, new test "finds a user comment that a restart left without a wake on a task its agent then closed". It fails on `master` and passes here: only the lost comment is reported, not one steered into the run and not one on a task a person closed. All 26 pass. - The server package typecheck passes. ## Risks - **Reopening a done task.** This only happens when the latest recorded close was not made by a person, the comment is a direct human comment, and no run received it or had it steered in. That's the same rule the t72 stale-continuation path and the release drain already apply. - **The steer check** joins the task's runs to their `item.started` events by comment ID. It runs only for candidate comments within the lookback window. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |