mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
ba75b44c3ca812d62ab891b38c96a7cf7faada3c
2618
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
ba75b44c3c |
fix(chat): name the addressee on human-only card denials and list team requirements (#20245)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent Chat answers a person's requests with a chat agent that reads API results and the chat guidance > - A decision card can be human-only and addressed to one person, and the teams catalog lists ready-made teams with their requirements > - When an agent tried to accept another person's card, the denial said only "human-only", so the agent told the asker to accept the card themselves > - When an agent recommended a catalog team, the guidance asked only for roles, so the agent left out the required skills > - This pull request makes the denial name the addressee as the only resolver, and asks agents to list a team's roles, required skills and configuration inputs > - The benefit is that chat answers send people to the right person and show what a team needs before install ## Linked Issues or Issue Description Refs #20206, #20220, #20239 **What happened** Alex wrote "I approve Riley's request." in Agent Chat. The pending card is human-only and addressed to Riley. The agent tried to record the decision and got `403 interaction_human_only` with only "This issue-thread interaction is human-only". The reply correctly said the card stays pending, but it then told Alex to "accept or decline that request" on the task. Alex cannot do that; only Riley can. In a second chat, a person asked "Do we have a ready-made team for building a website?". The agent recommended Product Engineering and its roles. It did not name the four required skills that the catalog returned in `requiredSkills`. **Expected behavior** When an agent cannot resolve a card that is addressed to another person, the reply names that person as the only one who can resolve it. The reply does not tell the asker to act on the card. When an agent recommends a catalog team, the reply names the team's roles, every required skill and every configuration input. It says that a catalog listing does not mean the accounts are configured. **Steps to reproduce** 1. As user Alex, create a `request_confirmation` card with `resolverPolicy: "human_only"` and `addresseeUserId` set to user Riley on Alex's chat task. 2. In that chat, write "I approve Riley's request." 3. Read the agent run. `resolve-from-comment` returns 403 `interaction_human_only` without the addressee. The reply tells Alex to accept or decline the card. 4. In a new chat, ask "Do we have a ready-made team for building a website?". 5. Read the reply. It names Product Engineering and its roles, but not its required skills. ## What Changed - `server/src/services/issue-thread-interaction-resolution.ts`: when an agent is denied on a human-only card that is addressed to a person other than the verified replying user, the denial says the card is addressed to that person, that only they can accept or decline it, that the denial is final, and that the agent must not ask the asker to act on the card. The details include `addresseeUserId`. Cards without an addressee keep the old message. - `server/src/routes/issues.ts`: the route adds the addressee's display name to that denial (`details.addresseeName` and the message). If the name lookup fails, the generic addressee text stays. - `server/src/services/agent-conversations.ts`: the teams catalog paragraph asks agents to name a recommended team's roles (`agentSlugs`), every required skill (`requiredSkills`, by ref) and every configuration input (`envInputs`), and to say that a listing does not mean accounts are configured. - Tests: evaluator unit tests, a route-level test for the named denial, and a directive assertion. ## Verification - `cd server && npx vitest run src/services/issue-thread-interaction-resolution.test.ts src/__tests__/agent-conversations.test.ts src/__tests__/issue-thread-interaction-routes.test.ts src/__tests__/confirmation-comment-resolution.test.ts`: 4 files, all pass. - `tsc --noEmit -p server/tsconfig.json`: passes. - The change is cherry-picked onto the eval branch `chat-eval/case-reply-completeness`. The live Runner E2E cells `questions-and-approvals.wrong-reviewer` and `teams.find-team` run there. ## Risks - Low risk. The resolver decision does not change: the same requests are allowed and denied. Only the denial text and details change for agents on addressed human-only cards. - The denial reads one user row for the display name. A failed read falls back to the generic text. - The chat directive is slightly longer, so each chat prompt uses a few more tokens. ## 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
|
b3bf12a5db |
fix(workspaces): close a retained local task directory as archived, not cleanup_failed (#20248)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task runs in an execution workspace. A person or an agent can close that workspace and keep its files for later > - A local workspace directory that the runtime did not create is never removed on close. Examples are an operator directory and an isolated task directory that retention keeps > - The cleanup result still said `cleaned: false`, because that directory still exists > - So every close of such a workspace got the status `cleanup_failed`, with no cleanup reason > - This pull request treats that directory as retained by design, so the close records `archived` > - The benefit is that a normal close no longer shows a failure that did not happen ## Linked Issues or Issue Description Refs #16776 (related: records-only archive of shared non-git sessions; that PR changes close readiness, this PR changes the recorded close outcome) **What happened** A chat agent closed a completed task's execution workspace with `PATCH /api/execution-workspaces/:id` and `status: "archived"`. The workspace is a `local_fs` isolated task directory with `createdByRuntime: false`. The workspace got `status: "cleanup_failed"`, a `closedAt`, and `cleanupReason: null`. The files stayed, as intended. The agent then had to tell the person that cleanup failed. **Expected behavior** The close records `status: "archived"`. The retained directory is the intended outcome, so it is not a failed cleanup. A runtime-created directory is still removed. **Steps to reproduce** 1. Create a task workspace whose `local_fs` directory exists and has `metadata.createdByRuntime: false` (for example an isolated task directory). 2. Close it with `PATCH /api/execution-workspaces/:id` and body `{"status":"archived"}`. 3. Read the workspace. Its status is `cleanup_failed` and `cleanupReason` is `null`. ## What Changed - `cleanupExecutionWorkspaceArtifacts` reports `cleaned: true` for a `local_fs` directory that the runtime did not create. It never removes that directory, so nothing is left to clean. - Added `server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts`: a retained directory reports `cleaned: true` and keeps its files; a runtime-created directory is still removed. ## Verification - `pnpm exec vitest run server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts` fails before the change (`cleaned: false`) and passes after it. - `server/src/__tests__/workspace-runtime.test.ts` (179), `execution-workspaces-routes.test.ts` (17), `execution-workspaces-service.test.ts` (85) and `workspace-delivery-cleanup.integration.test.ts` (19) pass. - End to end: the chat eval `workspaces.close-and-reopen` (runner-opencode) failed only on `status: cleanup_failed` after a correct agent close in GitHub Actions run 42606822088. This fix is cherry-picked onto the eval branch for a rerun. ## Risks - Low risk. Only `local_fs` workspaces with `createdByRuntime` not `true` change. Git worktrees, runtime-created directories, plugin and remote workspaces keep their behavior. - The terminal reaper uses the same cleanup function. It now records `archived` instead of `cleanup_failed` for such a retained directory. Isolated task directory retention treats both statuses as closed, so retention is not changed. ## 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
|
2ea1a4f4eb |
fix(chat): tell chat agents what a status card is and to reply now after queuing one (#20230)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent Chat lets a person talk to an agent in a standing conversation
> - Status Cards is an experimental feature: a saved card on the
company's Status page that a summarizer agent keeps up to date
> - A chat agent cannot read the experimental settings, so nothing it
reads says that status cards exist, how to create one, or where a card
lives
> - When a person asks for a status card, the agent writes a Markdown
task document instead. When a person asks for a refresh, the agent
queues it and then sleeps and polls the card for minutes
> - This pull request tells the chat agent about Status Cards only while
the experiment is on, and makes the create and refresh catalog entries
say that the work is queued and the agent replies now with the card link
> - The benefit is that the agent creates a real card, links it, and
does not hold the chat open while the summarizer works
## Linked Issues or Issue Description
**What happened**
With Status Cards enabled, a person asked the CEO agent "Make me a
status card showing the launch's biggest blockers." The agent's
reasoning treated "status card" as a format. It never searched for the
feature and saved a Markdown task document. In a second chat, a person
asked "Refresh the launch status card now." The agent searched for about
54 s to learn what a status card is, called the refresh route, and then
slept 20, 30 and 45 s to poll the card, although its guidance says not
to sleep or poll. The refresh response also did not include the card's
page link. Separately, a teammate assigned as the card's summarizer
spent over two minutes looking for the undefined summary sentinel format
named in its update task, so a failed generation never reached its
blocked state in time.
**Expected behavior**
While the experiment is on, the agent knows that a status card is a
saved feature at `/<prefix>/status/<cardId>`. It creates one with `POST
/api/companies/{companyId}/status-cards`, or refreshes one with `POST
/api/status-cards/{id}/refresh`. It replies at once with the card as a
Markdown link and says that the summarizer updates the card. When the
experiment is off, the agent sees no status card guidance.
**Steps to reproduce**
1. Enable Status Cards in Instance Settings > Experimental.
2. Open a chat with the CEO agent.
3. Send "Make me a status card showing the launch's biggest blockers."
4. See that the agent writes a task document and no card appears on the
Status page.
## What Changed
- Add `STATUS_CARDS_CHAT_DIRECTIVE` in
`server/src/services/agent-conversations.ts`. It says what a status card
is, names the Status page and card routes, names the create and refresh
routes, and tells the agent to reply now with the card's `href` as a
Markdown link instead of polling.
- Render the directive in standing-chat task markdown only when
`statusCardsEnabled` is true
(`server/src/services/heartbeat/task-markdown.ts`). Set the flag from
the experimental settings that the heartbeat already loads
(`server/src/services/heartbeat/task-context.ts`). No new query runs
before the turn starts.
- Add descriptions to the create and refresh entries in the API catalog
(`server/src/routes/openapi.ts`). They say that the summarizer does the
work in its own run, and that the agent replies now with the returned
link.
- Return the card's `href` in the refresh response and in the card list,
as create, get, update and archive already do
(`server/src/routes/status-cards.ts`). An agent that reuses a listed
card can then link it.
- Tell the agent to report the outcome the refresh response gives:
queued, already running, or no run started (deferred by policy, or a
concurrent refresh already finished).
- Define the summary sentinels in the update task prompt
(`server/src/services/status-cards.ts`). The prompt told any assigned
summarizer to "preserve" the built-in Summarizer's `STATUS:` and
`<<<SUMMARY-DRAFT>>>` sentinels without defining them. A non-built-in
summarizer searched for the format for minutes. The prompt now gives the
exact lines, says the PUT body is plain Markdown, and says to comment
and block the task when no summary can be produced, so the card keeps
its previous summary and shows the failure.
- Add tests: the directive appears only in standing chats with the
experiment on, and its page paths are real routes in `ui/src/App.tsx`;
the catalog descriptions; the refresh response `href`.
## Verification
- `cd server && npx vitest run
src/services/heartbeat/task-markdown.test.ts
src/__tests__/openapi-routes.test.ts src/__tests__/status-cards.test.ts
src/__tests__/agent-conversations.test.ts` passes (all pass;
`status-cards.test.ts` also checks the update task prompt).
- `npx tsc --noEmit -p server` reports no errors in the changed files.
- End to end: the `status-cards.create-card` and
`status-cards.refresh-card` chat evals run with this commit
cherry-picked onto their case branch.
## Risks
- Low risk. The text is added only to standing-chat context and only
while the experiment is on.
- The refresh response gains one field (`card.href`). Existing clients
ignore unknown fields.
## 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
|
9db7da444f |
fix(chat): point agents at the teams catalog for ready-made teams (#20239)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent Chat answers a person's requests with a chat agent that reads the API catalog and the chat guidance > - Paperclip ships a teams catalog of ready-made teams that a company can preview and install > - The chat guidance does not name this catalog, and its API catalog entries have only short summaries > - So agents asked for a "ready-made team" check only the existing agents and skills, or search npm and GitHub > - This pull request names the teams catalog where the agent already looks: the chat guidance and the API catalog entries > - The benefit is that agents find, preview and link ready-made teams instead of saying none exist ## Linked Issues or Issue Description **What happened** An agent was asked "Do we have a ready-made team for building a website?". It read only the company's agents, projects and skills. It replied that the company has no website team and offered to hire agents. It did not read the teams catalog, which has a Product Engineering team. An agent was asked "Show me what installing the website team would add.". It found `GET /api/teams/catalog`, but found no team named "website". It then searched local files, the skills catalog, npm and GitHub until the turn timed out. It did not preview the install. **Expected behavior** The agent reads the teams catalog and chooses a team by its description and roles. It previews an install with the preview route, which creates no resources. It does not link a teams catalog page, because the web app does not route one. It does not search outside Paperclip for other teams. It knows who can install a team, and it names the `paperclipai teams install` command for a board user. Preview also works in Ask and Plan modes, because it saves no data. **Steps to reproduce** 1. Create a company with only a CEO agent. 2. In Agent Chat, ask "Do we have a ready-made team for building a website?". 3. Read the run. The agent does not call `GET /api/teams/catalog`, and the reply says no team exists. 4. In a new chat, ask "Show me what installing the website team would add.". 5. Read the run. The agent searches npm and GitHub and does not call the preview route. ## What Changed - `server/src/services/agent-conversations.ts`: the chat directive has a new short paragraph. It says that ready-made teams are packages in the teams catalog. It tells the agent to list the catalog, to choose by description and roles, and not to search package registries, repositories or the web. It names the preview route and says that preview creates no resources. It says who can install. It says that the web app has no teams catalog page, so the agent must not link one, and it names the `paperclipai teams install <team id> --company-id <companyId>` command for a board user. - `server/src/routes/openapi.ts`: `GET /api/teams/catalog` now has a description and documents its `q`, `kind` and `category` query parameters. The preview route says that it creates no resources. The install route says who can install and names the CLI command. - `server/src/services/native-runtime/runner-api-catalog.ts`: the team preview route is allowed in Ask and Plan modes, like other reads. It computes a plan and saves no data. Install stays in standard mode only. - Tests in `agent-conversations.test.ts` and `runner-api.test.ts`. The API search for "ready-made team" now returns `GET /api/teams/catalog` first. A test validates a preview call in Ask mode and rejects install in Ask mode. ## Verification - `pnpm exec vitest run --root server src/services/native-runtime/runner-api.test.ts src/__tests__/agent-conversations.test.ts src/__tests__/openapi-routes.test.ts src/__tests__/teams-catalog-routes.test.ts`: all tests pass. - The change is also cherry-picked onto the eval branch `chat-eval/case-teams`. The live Runner E2E cells for `teams.find-team`, `teams.preview-install` and `teams.install-team` run there. ## Risks - Low risk. Most of the change is text in the API catalog and in the chat guidance, plus documented query parameters that the route already reads. - One behavior change: agents in Ask and Plan modes can now call the team preview route. The route only computes a plan, and it applies the same company access check. - The chat directive is slightly longer, so each chat prompt uses a few more tokens. - Related open PR #18970 adds request body schemas to the same team routes. The two changes touch nearby lines and can need a small merge. ## 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
|
780f378f60 |
fix(recovery): settle a local workspace command whose server process died mid-command (#20233)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A run prepares its execution workspace with local commands
(`workspace_provision`, `worktree_prepare`, teardown, cleanup). The
server records each command as a `workspace_operations` row.
> - A restart can stop the server while such a command runs. The row
then stays `running` forever, because no process or recovery path owns
it.
> - The restart-chaos harness found this. A K2 drain interrupted a run,
then the server started `bash ./scripts/provision-worktree.sh` for the
run and exited before it finished. The retry succeeded, but the old row
stayed `running`.
> - The ownership reconciler already settles finalize attempts whose
process died. Local command operations had no owner stamp and no
ownership kind.
> - This pull request stamps each local command operation with the
process that runs it and adds a `workspace_command_operation` ownership
kind that fails an abandoned one.
> - The benefit is that a restart never leaves a stuck workspace
operation behind.
## Linked Issues or Issue Description
**What happened?**
In restart-chaos wave seed 9807 (local, R1, faults K1/K2), tasks t03 and
t04 graded R6 with `running_workspace_operations`. A K2 drain
interrupted the run with `server_shutdown_interrupted`. A
`workspace_provision` operation started 0.3 s later in the same process
and never finished. The retry run succeeded. The orphan operation stayed
`running` forever.
**Expected behavior**
When the process that ran a local workspace command is gone, recovery
closes the operation. It does not change the run or its outcome.
**Steps to reproduce**
1. Start a run that provisions a worktree with a provision command.
2. Drain or restart the server while the command runs.
3. After the restart, see that the `workspace_operations` row for the
command stays `running`.
**Paperclip version or commit**
master at
|
||
|
DottaandClaude Opus 5.5
|
267f54ef72 |
fix(chat): make task-not-found, unreachable-MCP and board denials final for agents (#20208)
## 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 > - A board-only denial now tells the agent that it is final and where the person acts (#20151, #20182). Other refusals and errors still give no sign that they are final > - So the agent keeps trying until the reply deadline, and the person gets no answer. In chat evaluations this happened on a task lookup that returned 404, on an MCP server that Paperclip could not reach, on a model question, and after a final budget denial > - This pull request makes those answers say that they are final and what the person must do, and it tightens the chat guidance after a board-only denial > - The benefit is a prompt, correct reply instead of a long search or forbidden workarounds ## Linked Issues or Issue Description Refs #20151, #20182 (same remedy for board-only denials). **What happened** In four Agent Chat evaluation cases on a slower model, the agent did not reply before its deadline: - Asked to open a task from an old link, the agent got `404 {"error":"Issue not found"}`. It quoted the private-task guidance, then searched archived projects, activity and the old host for 3 more minutes. - Asked to connect an MCP server at an unreachable URL, the agent got `502 {"error":"fetch failed","code":"runtime_error"}` from the health check. It retried, then used `curl` on the host and gateway IP addresses to look for the server. - Asked which coding models an agent can use, the agent got the runtime's model list in its first minute. The catalog did not say how to list OpenRouter models or that these lists answer the question, so it enumerated OpenRouter with the shell until the deadline. - Asked to raise a budget, the agent got the final board-only denial. The chat guidance said "Complete other permitted work", so it probed approvals and other routes and replied too late. **Expected behavior** The answer the agent receives says whether it is final and what the person must do, with a link when the page is known. The agent replies at once. It does not search other routes, retry, or contact other network addresses. **Steps to reproduce** 1. Open Agent Chat with a CEO agent. 2. Ask it to open a deleted task by its old link, or to connect an MCP server at a closed local port. 3. The agent calls `GET /api/issues/{id}` (404) or `POST /api/tool-connections/{id}/health-check` (502 "fetch failed"). 4. The body says nothing about finality. The agent keeps searching or probing and replies late or not at all. ## What Changed - `GET /api/issues/:id`: for an agent, the 404 body adds `code: "issue_not_visible"` and a `remediation`. It says that the lookup is final, that the agent must not search other routes or open a similarly named task, and that it should reply now and offer sharing or pasting. A missing task, a private task and another company's task get the same body, so the body reveals nothing. People and other routes keep the plain body. - MCP health check and catalog refresh: when Paperclip cannot reach the server (connection refused, DNS, network, reset, timeout or TLS), an agent's error adds a `remediation`. It says the person must correct the URL or start the server, that the agent must not contact other hosts, ports or network addresses, and it links Connectors (`/<prefix>/apps`). Status, `error` and `code` do not change. - `AGENT_CHAT_DIRECTIVE`: after a board-only denial, the agent replies now instead of making more calls. "Complete other permitted work" is removed. - API catalog: `POST /api/companies/{companyId}/tools/connections` says it saves without a check and points MCP-by-URL setup at `tools/apps/connect`, which checks the server first and saves nothing for a fresh setup that cannot reach it. `GET /api/companies/{companyId}/adapters/{type}/models` says which lists answer a model question (the runtime's list with `provider`, `acpxAgent` for ACPX agents and `environmentId`, and `opencode_local` with `provider=openrouter` for OpenRouter keys), that the OpenRouter list is not a qualification, and that with those inputs these calls answer the question. `openrouter` is added to the documented `provider` values. - API catalog authorization: budget policy upsert, budget incident resolution, company and agent budget updates, and approval approve/reject are now listed as board-only. Their routes already call `assertBoard`, but the catalog said `board_or_agent`, so an agent explored them as its own action before it got the 403. - Tests: new `agent-refusal-finality.test.ts` (including the catalog actor for the budget and approval routes); the private-task access test now checks that a private and a missing task give an agent the same 404 body; the guidance test checks the new wording. ## Verification - `cd server && npx vitest run src/__tests__/agent-refusal-finality.test.ts src/__tests__/board-access-denial.test.ts src/__tests__/issue-access-grants-routes.test.ts src/__tests__/authz-existence-oracle-guard.test.ts`: all pass (embedded Postgres included). - `cd server && npx vitest run src/__tests__/error-handler.test.ts src/__tests__/generic-mcp-connection.test.ts src/__tests__/openapi-routes.test.ts src/__tests__/remote-http-rebinding.test.ts src/__tests__/cloud-portfolio-error.test.ts src/__tests__/database-backup-catalog-guidance.test.ts`: 183 tests pass. - `npx tsc --noEmit -p server`: passes. - Real chat journeys on the chat evaluation branch (runner-opencode): the results will follow in a comment. ## Risks - Low risk. HTTP status, `error` and `code` stay the same for each changed error. Agents get extra top-level fields (`code` on the task 404, `remediation`). A client that compares a whole agent 404 body for `GET /api/issues/:id` exactly will see a change. - The task 404 body is the same for missing, private and other-company tasks, so it adds no existence oracle. Other issue routes keep their plain body. - The guidance change can make an agent stop after a board-only denial in a request that had other, permitted parts. The agent still reports what it finished. ## 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
|
d2fe85e57c |
fix(secrets): delete a secret that a binding proposal referenced (#20193)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents can propose a secret binding. A person approves or rejects it on a review card. > - Each binding proposal row refers to one source: a secret or a pending secret proposal. A check constraint enforces this. > - The secret reference used `ON DELETE SET NULL`. When a person deletes the secret, Postgres sets the reference to null. The check constraint then rejects the row. > - So a person cannot delete any secret that a binding proposal ever referenced. The API returns 500. > - This pull request deletes the proposal rows together with the secret, as the other source reference already does. Before the delete, it rejects pending proposals so their cards close with a reason. > - The benefit is that secret deletion works again and no review card points at a removed proposal. ## Linked Issues or Issue Description Refs #15644 (related: block secret deletion while bindings still exist). **What happened** `DELETE /api/secrets/:id` returned 500 for a secret that a binding proposal referenced. The server log showed `new row for relation "company_secret_proposals" violates check constraint "company_secret_proposals_shape_check"`. This happened for pending, rejected and approved proposals. It also happened after the person removed the agent's binding. **Expected behavior** The person can delete the secret. Proposals that referenced it go with it. A pending review card closes as rejected and tells why. **Steps to reproduce** 1. An agent proposes a binding for an existing company secret (`POST /api/agents/me/secret-proposals` with `secretId`). 2. Approve or reject the proposal, or leave it pending. 3. Delete the secret with `DELETE /api/secrets/:id`. 4. The response is 500 and the secret stays. ## What Changed - `packages/db`: `company_secret_proposals.secret_id` now uses `ON DELETE CASCADE`, the same as `secret_proposal_id`. Migration `0452_secret_proposal_secret_cascade.sql` replaces the foreign key (`DROP CONSTRAINT IF EXISTS`, then `ADD CONSTRAINT`). - `server/src/services/secrets.ts`: `remove` accepts an optional `beforeDelete` hook. The hook runs after the secret is marked deleted and before its row is removed. - `server/src/services/secret-proposals.ts`: new `rejectPendingBindingsForSecret`. It rejects pending binding proposals for a secret with a reason and returns them. This closes their review cards and writes the normal activity entries. `createBinding` now holds the secret row `FOR SHARE` in its insert transaction and refuses a deleted secret. So no new pending proposal can start after the delete marks the secret deleted. - `server/src/routes/secrets.ts`: the delete route rejects pending proposals in `beforeDelete`. For each one it sends the normal rejection notice and wake (`notifySecretProposalResolution`), so the proposing agent continues. - `server/src/__tests__/secret-proposals-routes.test.ts`: a regression test deletes a secret with a pending, rejected and approved binding proposal through the route. It checks the secret and proposal are gone, the card status is correct, and only the pending case sends one rejection notice and one wake. A second test checks that a binding proposal for a secret that the delete already marked deleted fails with 404. ## Verification - Red first: the new test failed on `origin/master` with the shape-check violation for all three cases. - `cd server && vitest run --config vitest.config.ts src/__tests__/secret-proposals-routes.test.ts`: 47 passed. - `vitest run src/__tests__/chat-run-milestone-governed-work.test.ts src/__tests__/agent-secrets-routes.test.ts src/__tests__/secrets-service.test.ts`: 112 passed. - `tsc --noEmit` for `server` and `packages/db`: pass. `pnpm --filter @paperclipai/db run check:migrations`: pass. - Found by the `secrets.secret-proposal` chat evaluation. Its cleanup deletes the fixture secret and failed on this error in runs 42563093640 and 42571127911. ## Risks - The migration only replaces one foreign key. It does not rewrite rows. Existing rows already have a non-null `secret_id` or `secret_proposal_id`, so nothing else changes. - Deleting a secret now also deletes its binding proposal rows. The activity log keeps the proposal history (`secret.proposal.*` and `secret.deleted`). Secret-kind proposal rows that created the secret keep `created_secret_id ON DELETE SET NULL`, so their history stays. - Other paths that delete secrets (company deletion, user secret cleanup) now cascade instead of failing on the same constraint. They do not reject pending cards first. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used 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
|
98e0f1fc0d |
fix(native-runtime): a stopped run cannot close a task another run continues; deliver a pre-restart comment at once (#20216)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the board stops a run and then posts a comment asking the agent to continue, that comment starts a new run, and only that new run should decide when the task is done > - In restart chaos wave `w-20261109T081456-s9901`, t16 (claude S1, K1) closed the task as done without the line the user asked for, and t10 (codex S1, K6) never woke a run for the continue comment, so it needed a Retry > - In t16, the stopped run's saved "done" result was accepted 10 s after the continue run started. In t10, a restart killed the server 21 ms after the comment committed, before its wake was written, and the comment backstop then waited out its 30 s grace > - This pull request stops a run from closing a task that another run now holds, and makes the backstop deliver at once a comment that predates the server process > - The benefit is that a user's latest direction after a Stop is neither overridden by the stopped run nor lost to a restart ## Linked Issues or Issue Description Refs #19831. Restart chaos wave `w-20261109T081456-s9901` (master `09e24dd12c`), tasks t16 and t10. **t16, from the kept database (CHA-20):** - 08:29:19: the board stopped run 1 (`6e33d123`). Its saved result reported done, but finalization failed on the stop (`finalization_failed_claim_preserved`, a retryable failure). - 08:29:41: the user's continue comment started run 2 (`2cb71c5b`). Run 2 held the task's execution and saw the comment. - 08:29:51: the finalization reconciler retried run 1's saved result and committed `completion_claim_policy_accepted` for run 1. The task closed as done. - From then on, every Paperclip tool call from run 2 was refused (`paperclip_runner_tool_binding_not_authorized`), so it could not add the `CONTINUE-…` line. Its result (blocked) was then preserved as `terminal_status_preserved`. - Run 2 ending `succeeded` with `native_session_lease_lost` follows from this: its result was accepted before the lease was lost. The comment was delivered; the model saw it. **t10 (CHA-14):** - The Stop moved the task to blocked. The user's comment reopened it to todo at 08:26:28.633, and K6 killed the server at 08:26:28.654, before the route wrote the comment's wake. No wake request exists for that comment. - This is not caused by the Stop, #20066 or #20180: the wake is written after the comment commits, and the kill landed in between. - The I4 backstop ("a user comment without a wake") matched the comment, but treated it as possibly still getting its normal wake for 30 s. The pass interval then pushed the check to 02:27:16, and the harness clicked Retry at 02:26:59. ## What Changed - `status-arbiter.ts`: - New `heldByAnotherRun(issue, runId)`. - `supersedeStaleOwnerCompletion` also keeps status, with reason `completion_run_superseded`, when the issue's `executionRunId` is a different run. A run that was reassigned away still gets `completion_owner_superseded`. - `native-run-finalizer.ts` and `native-status-reassessment.ts` pass the run id to that guard. - `status-decision-committer.ts` rechecks the same rule under the issue lock: a done decision for a non-conversation issue held by another run raises `NativeStatusRaceError`. The finalizer then re-arbitrates and keeps status. - `ownership-invariant-report.ts`: the comment-without-wake grace now only applies to comments created after this server process started (`getServerProcessStartedAt`). Separate live server replicas are not supported, and the wake is written by the process that accepted the comment, so an older comment is owed at once. The startup pass now delivers it. `runOwnershipReconciliationPass` takes an optional `processStartedAt`. ## Verification Red/green: - `native-stale-owner-completion.test.ts`: three new tests. - The unit policy: another run holding the task keeps status; the same run, or none, still closes it. - "does not let a stopped run's saved done result close the task a newer run is continuing" (the t16 shape). - "does not close the task when a newer run takes it after arbitration but before the status commit" (only the committer recheck covers this). - On master: 3 failed. With this PR: 13 passed. - `ownership-reconciliation-report.test.ts`: "finds at once a young user comment whose wake a restart lost after the board stopped the run" (the t10 shape). It fails on master and passes with this PR; the same comment in a process that accepted it still waits for its grace deadline. The file: 27 passed. Regression and static checks: - 16 nearby native finalization, status projection, workspace finalizer and arbiter suites: 439 passed. - `tsc --noEmit` passes for `server`, and for the two test files. ## Risks - A done result now waits when a different run holds the task. That run reaches its own decision; the earlier result stays as history. If the holder is stale, the existing execution-lock recovery clears it, and a later reassessment can then close the task. - After a restart, a user comment newer than the task's last run, with no wake, is delivered at the startup pass rather than 30 s later. A replica that is still live and writing the same wake is not a supported topology. - No schema change and no change to persisted run state. ## 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
|
6383eb8033 |
fix(authz): require board authority for agent budget changes on every route (#20213)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budgets stop agent spending. The budget hard stop is a control-plane invariant > - The budget routes (`PATCH /api/agents/:id/budgets`, `PATCH /api/companies/:id/budgets`) are board-only > - `PATCH /api/agents/:id` also accepts `budgetMonthlyCents`, and the configuration service writes the agent budget policy from it. That route had no board check for the field > - So an agent, such as a CEO, a manager or the agent itself, can change a monthly budget without a board user. This is a second route around a board-only control, like the pause bypass that #20164 fixed > - This pull request applies the budget route authority to every route that writes an agent budget, and gives agents the final board-only denial with a link to the budget page (#20151) > - The benefit is that only the board changes budgets, and an agent that asks gets a final answer that it can pass to the person ## Linked Issues or Issue Description Refs #17857 (earlier fix, now in conflict with master; this PR rebases it and adds the budget page link). Refs #20164 and #20151 (same pattern for PATCH status and board-only denials). **What happened** An agent can send `PATCH /api/agents/{id}` with `budgetMonthlyCents`. The route accepts it, and the configuration service writes the agent budget policy. This works for the agent's own budget and, with update permission, for other agents. The same field also reaches budget policy through a configuration rollback, direct agent creation, an immediate hire and a built-in agent provision. A chat eval (`budgets-costs.resolve-budget`) found the route. **Expected behavior** Only a board user can change an active budget, as on the dedicated budget routes. An agent gets `403` with code `board_access_required`. The denial says it is final and links the agent's budget tab (`/<prefix>/agents/<id>/budgets`) or the company budgets page (`/<prefix>/activity/budgets`). An agent can still propose a budget in a hire that waits for board approval. The approving board user activates it. **Steps to reproduce** 1. On master, authenticate as an agent of a company (for example the CEO). 2. Send `PATCH /api/agents/{agentId}` with body `{"budgetMonthlyCents": 5000}`, for its own id or for another agent. 3. See `200`. The agent budget policy changes with no board action. 4. The new route tests reproduce this: on master source they fail with `expected 200 to be 403`. ## What Changed - `PATCH /api/agents/:id`: a changed `budgetMonthlyCents` requires a board actor. An unchanged amount with other edits still works. - Configuration rollback: a rollback that changes the budget requires a board actor. - `POST /api/companies/:id/agents` and an immediate `agent-hires`: a positive budget requires a board actor. A hire that needs board approval keeps the proposed budget and does not activate it before approval. - Built-in agent provision: a budget override requires a board actor. - `prepareAgentConfiguration` rejects an agent-originated budget change again (defense in depth). An agent rollback with an unchanged amount does not rewrite the policy. - Board writes keep the actual operator identity. The initial hire budget is saved in the hire transaction. - `assertBoard` and `boardAccessRequired` take an optional board page. Budget denials link the agent's budget tab or the company budgets page, not the general agent profile. - Agent access map updated. - This includes the four commits of #17857 ( |
||
|
DottaandClaude Opus 5.5
|
fc5c79a327 |
fix(guidance): tell agents who acts and which page to link for person-only steps (#20206)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents in chat read the API catalog (`search_api`) and `connections_search` to learn what they can do and who must act > - Some requests need a step that only a person can take: connect an assistant, allow a quarantined app action, turn on an experiment, enter an API key, or finish their own connection request > - For these steps, no catalog entry or search result said who acts or which page to link, and none of them returns a board-only 403, so the denial guidance from #20151 and #20182 does not reach them > - The agent knew a person had to act, searched for the page until its turn ran out, and never replied > - This pull request puts the fact and the exact page link where the agent already looks, following #20152 and #20170 > - The benefit is that the agent replies at once with the right page instead of timing out ## Linked Issues or Issue Description Refs #20151, #20152, #20170, #20182 **What happened** In chat, five requests ended with no reply. The agent searched the API until the turn deadline: - "Connect my assistant so it can see our tasks." `GET /api/mcp/setup` is refused for agents. Its catalog entry had no description, so the refusal text "Its catalog description says who can use it" pointed at nothing. - "Use the app's new export feature." The new action was quarantined. No catalog entry said where a person reviews it. - "Let me chat with Morgan over iMessage." `connections_search` hides chat channels while the Chat connectors experiment is off, so the agent could not find iMessage or the setting that hides it. `POST /api/companies/{companyId}/chat-endpoints` also accepted a draft endpoint for the turned-off provider. - "Connect my account using an API key." The service was the person's own MCP server. `connections_search` returned no result and did not name the secure "Connect your own MCP server" page. - "I will finish Alex's connection request for them." No catalog entry said that only the addressee can resolve a connection request card. **Expected behavior** The agent says who must act, links the exact page, and replies in the same turn. It does not search further, does not ask for a key in chat, and does not create another request. **Steps to reproduce** 1. Ask a chat agent one of the five requests above, in the matching state (for example, Chat connectors off, or a quarantined catalog entry). 2. Watch the run events. The agent calls `search_api` and `call_api` until the turn deadline and posts no reply. ## What Changed - `server/src/routes/openapi.ts`: add catalog descriptions that say who acts, link the page, and say not to search further: - `GET /api/mcp/setup` and `POST /api/mcp/requests/{id}/consent` link [Assistant connections](/<prefix>/apps/assistant-connection) and say the consent screen offers only access within the person's own role. - `GET /api/tool-connections/{connectionId}/catalog`, `.../review-counts` and `GET /api/companies/{companyId}/tools/apps/attention` say a quarantined action waits for a person on `/<prefix>/apps/<connectionId>/permissions`. - The four `/api/connection-intents/{interactionId}/...` routes say only the addressee can resolve the card, and link the task that holds it. - `server/src/services/connection-intents.ts`: - While Chat connectors is off, a search that names a chat-only channel (for example iMessage) returns it as `unavailable`, with a reason that links `/<prefix>/company/settings/instance/experimental`. Channels that also offer tools, such as Slack, keep their current result. - A single `unavailable` result now always gets the "explain the reason, do not bypass" instruction. - When no listed service matches and the person gave the service's own server URL, the instruction links `/<prefix>/apps/byo`, the secure "Connect your own MCP server" page, and says never to ask for a key in chat. - `server/src/routes/chat-channels.ts`: `POST /api/companies/{companyId}/chat-endpoints` returns 403 with the Experimental settings page while the provider's experiment is off (Chat connectors, or GitHub review bots for GitHub). AgentMail is unchanged. The catalog entry says the same. The UI already hides the setup pages in this state. - `GET /api/issues/{id}/interactions` says only the addressee resolves a connection request card. - A hidden channel that an aggregator can serve (for example Discord) keeps its aggregator routes instead of the unavailable result. - Tests: `person-only-page-guidance.test.ts` checks the catalog text and that every named page is a route that `ui/src/App.tsx` mounts. `connection-intents-service.test.ts` covers the hidden channel (off and on) and the own-server page. ## Verification - `cd server && npx vitest run src/__tests__/person-only-page-guidance.test.ts src/__tests__/connection-intents-service.test.ts` passes. `chat-channels.integration.test.ts` passes with `-t` for every test that posts to the create route. - These related suites pass: `openapi-routes`, `connection-aggregator-fallback`, `generic-mcp-connection`, `assistant-connections-guidance`, `company-model-service-settings-guidance`, `board-access-denial`, `email-channels.integration`, and the eight suites that import `runner-api-catalog`. - `npx tsc --noEmit -p server` shows no errors in the changed files. - End to end: the full-stack chat evals for the five requests ran on a branch with this change. The run results are in the comments below. ## Risks - Low risk. The change is text in the API catalog and in `connections_search` results, plus one result for a channel that was hidden before. - A hidden channel now shows in search as `unavailable` with no methods. It cannot be set up from the result, so the gate stays in place. - Chat endpoint creation is now refused while the experiment is off. Callers that created endpoints with the setting off now get 403. Three integration tests that relied on that now turn the setting on first, and a new test covers the refusal. - The single-`unavailable` instruction now also applies in suggestion mode. Before, that mode told the agent to "respect unavailable states" with no reason. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
09e24dd12c |
fix(authz): say every plugin management step is board-only in plugin denials (#20207)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People ask a chat agent to do work, and some of that work is board-only, such as plugin management > - When an agent gets a board-only denial, the denial tells it to stop and reply with a link to the board page > - The denial only spoke about the one route that was denied. An agent that read the plugin list first was not told that plugin configuration and secret binding are also board-only > - The agent then searched the API catalog for an agent route to configure the plugin. It searched for 110 seconds, and its reply came after the chat window closed > - This pull request adds one fact to every board-only denial on a plugin route: all plugin management steps have no agent route > - The benefit is that the agent replies at once with the plugin settings link, and does not search for a route that does not exist ## Linked Issues or Issue Description Refs #20182 (the plugin denial link and "reply now" text that this builds on). **What happened** In the chat eval `plugins.configure-plugin`, the person asked the agent to finish configuring the preview plugin. The agent called `GET /api/plugins` and got the board-only denial. Its reasoning said: "Denial final. But ... perhaps the preview plugin is configured through a different route accessible to agents". It then made 14 more catalog searches for plugin config, credential and secret-binding routes. The run did not reply before the 150 second chat window closed. **Expected behavior** After a board-only denial on any plugin route, the agent replies now. It says that a board user must configure the plugin, and it links the plugin settings page. **Steps to reproduce** 1. Install a plugin that has a required secret setting, and do not configure it. 2. In chat, ask an agent: "Finish configuring the preview plugin." 3. The agent calls `GET /api/plugins` and gets 403 "Board access required". 4. Before this change, the denial does not say that configuration and secret binding are also board-only, and the agent searches for another route. ## What Changed - `server/src/routes/authz.ts`: a board-only denial for an agent on any `/api/plugins` route adds one sentence. The sentence says that listing, installing, configuring (including choosing a secret for a plugin setting), enabling, disabling, upgrading and removing plugins have no agent route. - Other board-only denials do not change. Plugin tools that are assigned to an agent are not mentioned, so tool use is not discouraged. - `server/src/__tests__/board-access-denial.test.ts`: a regression test for the plugin list and plugin config denials, and a negative check for a non-plugin denial. ## Verification - `cd server && npx vitest run src/__tests__/board-access-denial.test.ts` passes (26 tests). - Without the `authz.ts` change, the new test fails. - The chat eval `plugins.configure-plugin` is rerun on a case branch that includes this commit. The result is in the PR comments. ## Risks - Low risk. The change only adds text to the 403 body that agents get. People and board actors get the same body as before. - The sentence is a fact about the current routes. Every plugin management route uses `assertBoard`, `assertBoardOrgAccess` or `assertInstanceAdmin`. If a plugin management route becomes agent-accessible later, this sentence must change too. ## 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> |
||
|
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
|
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
|
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> |
||
|
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
|
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
|
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>
|
||
|
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
|
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
|
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
|
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
|
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> |
||
|
DottaandClaude Opus 5.5
|
3448c7f9ba |
fix(recovery): own abandoned warm sandboxes on the retained-orphan path (#20065)
## Thinking Path - Paperclip runs agent tasks in remote sandboxes (Daytona). With a warm lifecycle, a sandbox outlives its turn so the next turn on the same workspace resumes it instead of provisioning and uploading again. - #20048 makes warm the default on remote sandboxes. Its merge condition: an idle warm sandbox must be an owned record, adopted by its successor or destroyed or suspended once its owner is proven gone, with zero orphans in a Daytona chaos wave. - That wave (warm environment, K1/K2/K4 faults) left sandboxes alive behind dead lease rows. Reading the wave's database shows three warm-specific shapes that nothing ever tears down: 1. **A Retry moved the task to a new workspace.** `retainedIssueWorkspaceForRetry` reuses only local leases, so a remote task always gets a fresh workspace. Reuse matches only the exact workspace, so the old sandbox is unreachable: `released` and stopped (wave t05), or still `retained` and running. 2. **A failed warm lease.** The plugin stopped the sandbox for reuse, but reuse never resumes a `failed` row (wave t12). 3. **A warm sandbox kept running (`retained`).** Its idle timer lived only in the process that kept it. If that task moves on, nothing stops the sandbox, even after a restart. - The shared ownership path already has a kind for exactly this lifecycle: `environment_retained_orphan_sandbox` destroys a stopped, retained sandbox once nothing references it, but it skipped warm leases on purpose. This PR extends that kind instead of adding a new sweep. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T142906-s9501: - t05: run 8758207a's sandbox was resumed by c99b3a2e and released stopped in workspace 35a678f0. The Retry kept a fresh workspace (8ac860c3) and provisioned a new sandbox, and the old one stayed forever. - t12: run f2bf2f05's lease on sandbox 6d4c91fb was released as `failed` with the sandbox stopped, and the next run on the same workspace provisioned a new one. Both sandboxes were still alive when the wave ended. **Steps to reproduce** 1. Use a sandbox environment with warm reuse (`reuseLease: true`). 2. Complete a turn so the warm lease is kept, retained or released and stopped. 3. Make the next run fail so the Retry keeps a fresh workspace (always the case for a remote task), or so the lease is released as `failed`. 4. The next run provisions a new sandbox; the old one is never resumed or destroyed. **Expected behavior** A warm sandbox that no turn can resume is an owned record. Once its owning run is terminal and nothing references it, the ownership reconciler stops it if it is still running, then destroys it. A sandbox its task can still resume on the same workspace is left for reuse. ## What Changed - `server/src/services/environment-retained-orphan-settlement.ts`: the listing (`retainedOrphanSandboxCondition`) now has two warm branches, each on the newest lease row for its sandbox: - `abandonedWarmSandboxCondition`: a `reuse_by_environment` lease with a confirmed `stopped` receipt for this lease and run, either `failed`, or `released` while the task's bound workspace differs from the lease's. The settle hands it to the cleanup sweep through the existing stopped-cleanup contract, so the old run's successors are never blocked on the destroy. - `supersededRunningWarmSandboxCondition`: a `retained` (still running) warm lease whose task moved workspace. The settle queues a stop-only intent. When the provider confirms the stop, the row becomes `released` with a receipt, and the first branch destroys it on a later pass. - The settle re-checks the same predicate under the row lock. Every existing hold still applies: the run still active, open runs or recovery actions on the issue, pending exports, stop or restore intents, or another live lease on the same sandbox. - `server/src/services/sandbox-stop-and-retain.ts`: new `prepareSupersededWarmSandboxStop`, which prepares the existing stop-only intent from `retained`. The intent carries the recovery marker, so the sweep dispatches its first attempt at once. - `server/src/__tests__/environment-retained-orphan-sandbox.test.ts`, new tests: - a superseded released lease is destroyed; - a failed warm lease is destroyed; - a running superseded lease is stopped, then destroyed; - keep cases: the task can still resume it (released, and retained after a restart), no stop receipt, a newer row on the same sandbox, a run in flight on the task. The positive tests fail on master and pass with this change. **Previous-release state:** the conditions read existing columns only (status, lease policy, workspace binding, stop receipt). Rows the previous release saved are judged as-is. The tests build them in exactly the shape that release writes (no new marker), so old-saved state is handled by the new code without migration. Rows without a confirmed stop are never listed. ## Verification - `cd server && npx vitest run src/__tests__/environment-retained-orphan-sandbox.test.ts`: 29/29 pass. With the two service files reverted to master, the 3 positive tests fail and the other 26 pass. - Neighbouring suites `environment-run-lease-ownership`, `heartbeat-pending-cleanup-sweep`, `ownership-reconciliation-report`, `heartbeat-orphaned-active-lease-sweep`, `environment-pending-cleanup-queue-owner` and `retained-native-session-cleanup-work`: 150/150 pass. - The wave's lease rows were read from the run's kept database copy to confirm the shapes above. - Not yet covered: the same Daytona warm wave, rerun with this fix. #20048 merges only at ≥11/12 with zero orphans. ## Risks - **Destroying a sandbox someone can still use.** Only the newest row for a sandbox is judged. `released` and `retained` rows qualify only when the task's bound workspace differs from the lease's, which is the exact rule reuse matches on. All existing holds still apply, and a stop must be provider-confirmed before any destroy. - **Unsaved work.** Pending native exports and stop or restore intents hold the sandbox. A stopped sandbox is destroyed only after its owning run is terminal and nothing on the task is still running. - **Blocking a successor.** The destroy uses the stopped-cleanup contract (`workspaceRecoveryCleanup` with the stop receipt), which `hasPendingRunCleanup` treats as resource cleanup. This is covered by a test. - **Out of scope.** A `retain_on_failure` lease kept for workspace restore (wave t12, a095e0b5) stays with restore recovery, which owns its own cleanup. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
0746d1203d |
feat(recovery): let the board confirm that an unrecorded owner stopped (#20067)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery releases an in-progress record only once its owner is proven gone, never by age > - A run lease saved before owners were stamped has no owner Paperclip can judge on an ephemeral (multi-host) server, so it stays stuck and holds its run's sandbox forever > - Only a person can tell that the previous server stopped (for example, its container was replaced) > - This pull request asks the board on the stuck task and lets an owner confirm the stop, with an activity-log entry; recovery then releases the record on its normal path > - The benefit is that legacy stuck records have a way out that never guesses ## Linked Issues or Issue Description Follow-up to the Phase 2 durable-ownership work: #20054 (controller instance lease) and #19883 (process-space recording). It generalizes the existing "confirm the previous server stopped" action for native controllers (`native_controller_host_unverified`). No duplicate PR was found. **What happened?** On an ephemeral server, an active run lease of a finished run that was saved before holders were stamped has no owner stamp. The ownership reconciler reports it `unprovable (no_owner_stamp)` and never settles it. No operator action exists, so the lease and its sandbox stay reserved. **Expected behavior** The task shows the stuck resource, and a board owner can confirm that the previous server stopped. Recovery then releases the lease. Agents and operators without the owner-stop permission cannot. **Steps to reproduce** 1. Run with `PAPERCLIP_EPHEMERAL_SERVER=true`. 2. Have an active environment lease with no `leaseController` stamp, whose run is terminal. 3. Run an ownership reconciliation pass. The lease stays `unprovable` with no action on the task. **Paperclip version or commit** `master` before this PR. ## What Changed - `packages/shared`: `confirmOwnerGoneSchema` (`actionId`, `recordId`, `stopEvidence` of 20 to 12000 characters). - `recovery/owner-gone-attestation.ts`: - `requestOwnerGoneConfirmation(kind)` is an `onUnprovable` hook. For a legacy unprovable owner (`no_owner_stamp`, `process_space_unrecorded`, `missing_identity`) on a record that has a task, it opens one idempotent board recovery action on that task (cause `ownership_owner_unprovable`). - `confirmOwnerGone(...)` verifies the exact action and record. It asks the kind to write the attestation (compare-and-swap), resolves the action, and writes an `issue.ownership_owner_gone_confirmed` activity-log entry with the actor and the stated evidence. - Ownership registry: - `OwnableKind.attestOwnerGone` is a new optional hook. - `environment_run_lease_active` implements it: only an unstamped active lease of a terminal run, with the notification for the cleanup path. - The lease's owner then reads `operator_attested`, which proves gone, so the existing settle releases it. A holder stamped since the board was asked owns the lease, and its stamp decides. - Route: `POST /api/issues/<built-in function id>/recovery-actions/confirm-owner-gone`. It uses `assertBoard` plus `runtime:confirm_owner_stop`, the same board-only permission as confirm-controller-stopped. It is registered in OpenAPI. - UI: `issuesApi.confirmOwnerGone`. The task's recovery card shows the existing `PreviousServerStopConfirmation` for this cause, with resource-specific wording; owners confirm and operators see the wait. `isPreviousServerStopCause` includes the cause, so the generic card does not offer other resolutions. ## Verification - `owner-gone-attestation.test.ts`: - the stuck lease opens one board action on the task; - confirmation resolves the action, writes the activity log, and the next pass settles the lease (`operator_attested`) exactly once; - a mismatched record is refused with 409; - a holder stamped after the ask is refused and the lease stays active; - no action for a judgeable owner or for a record with no task. - Red, with the `onUnprovable` hook removed: 2 of 3 fail. - UI: `WorkspaceExportRecovery.control.test.tsx` checks that the owner confirms with the right payload and that an operator sees the wait without the control (47/47). - Server suites: openapi-routes, ownership and recovery suites, plus the existing native-controller-host recovery. Low-memory server typecheck is clean; `check:token-gates` is clean. ## Risks - The board can attest a server stopped that is in fact running. The action and the UI copy ask for the evidence, and the activity log records who attested what. This is the same trust model as confirm-controller-stopped. - Only run leases opt in so far. Records without a task (login/probe leases) keep the legacy idle sweep. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
17ad23aafe | fix(workspaces): back off failed terminal cleanup and retire abandoned reservations through remove (#20020) | ||
|
DottaandClaude Opus 5.5
|
cf9c81d9a7 |
perf(run-log): record named preparation phases before dispatch as run-log spans (#20046)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every native run records performance spans in its run log, the instance-local `heartbeat_run_events` table, so slow turns can be diagnosed without any external service > - The steps between environment acquisition and dispatch are timed by named preparation phases, but those timings reach only OpenTelemetry, which is off unless an operator configures an endpoint > - On a Daytona follow-up in a paperclip-sized workspace, 17–30 s sat in one opaque span, `heartbeat.prepare_after_environment` > - This pull request also records the named preparation phases, and their nested steps, as run-log spans > - The benefit is that the next slow step shows up by name in the run log; this change found a full workspace re-upload on every remote turn ## Linked Issues or Issue Description **What happened?** The run log showed `heartbeat.prepare_after_environment` at 17–30 s on Daytona follow-ups, with nothing inside it. The phases that would explain it (`heartbeat.prepare.*`, `runtime_selection.*`, `workspace_sync.*`) were recorded only as OpenTelemetry spans, and those are a no-op without an OTLP endpoint. **Steps to reproduce** 1. Run a Codex task on a Daytona environment with a large repository as the project workspace. 2. Post a follow-up comment. 3. The run log shows one large `heartbeat.prepare_after_environment` span with no children. **Expected behavior** The run log names the steps that make up the wait. ## What Changed - `execution-preparation-timing.ts`: every named preparation phase (`measure`) and nested run phase (`measureRunPhase`) records its start, end and outcome in a per-run in-memory list. - Bounds: phases under 50 ms are skipped, at most 200 per run, at most 256 runs held. - Without a tracer, the run scope is still set, so nested phases reach the run. - OpenTelemetry behavior is unchanged. - `runtime-execution.ts`: at native dispatch, the recorded phases are drained into the run's preparation spans under `task.prepare`. - `runtime-selection.ts`: the remote Codex CLI version read and the runtime context build get their own phases, `runtime_selection.codex_cli_version` and `runtime_selection.runtime_context`. ## Verification - `execution-preparation-phases.test.ts`: a slow phase is recorded and a fast one skipped, a failure is marked, draining clears the run, runs are kept apart, and a nested `measureRunPhase` step is recorded without a tracer. - `database-operation-attribution`, `company-skills-observability` and `github-catalog-timing`: pass (15 tests in total). - Daytona Codex run in a fresh paperclip clone: the follow-up's 17.8 s gap now reads `heartbeat.prepare.runtime_selection` 15.7 s, then `runtime_selection.workspace_sync` 15.4 s, then `workspace_sync.transfer` 13.3 s. ## Risks - This is a run-log change only, not Telemetry: the data stays in the instance database. It adds roughly 10–40 spans per run, bounded as above. - The recorder is in memory and per process. A run that never dispatches is evicted when the cap of 256 runs is reached. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution, Daytona measurements). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
96badeaa42 |
fix(lifecycle): never repeat a passed harness verification; settle a dead verifier's lease at once (#20030)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A new or reconfigured agent runs a harness verification (a hello probe in a test sandbox) before it is ready, and its runs wait until it passes > - In the final Daytona restart chaos wave, a Codex agent's runs waited about 417 s for their first sandbox, while boot took only about 32 s > - Each control-plane restart killed the verifier. The next process waited out the 120 s lifecycle lease and then booted a fresh sandbox. Nothing recorded a configuration that had already passed. > - This pull request records a passed verification by what makes it valid and reuses it, and settles a dead verifier's lease on the shared ownership path at once > - The benefit is that a restart never repeats a passed verification, and never leaves runs waiting out a dead process's lease ## Linked Issues or Issue Description **What happened?** In the final Daytona chaos wave, the Codex agent was created at 09:55:46 UTC and its three runs were queued. Verification ran four times, in four sandboxes, because faults K1, K4 and K2 each killed the verifying server. After K2 the server was ready at 10:00:20, but verification restarted only at 10:02:13: it waited out the 120 s lifecycle lease. The first run sandboxes were created at 10:02:43, 417 s after launch. **Steps to reproduce** 1. Create a Codex agent and assign it a task at once, so the task waits for harness verification. 2. Kill the control plane (SIGKILL) while the agent is verifying. 3. The restarted server waits out the dead verifier's 120 s lifecycle lease, then verifies again in a new test target. The task's first token arrives about 130 s after the fault. **Expected behavior** - A passed verification of the same effective configuration in the same runner image is never repeated. - A verification is never restarted from scratch while its holder process is alive. When the holder is proven gone, its lease is settled at once. - Runs gate only on a configuration that has never passed. **Root cause** `verify()` (`server/src/services/agent-harness-verification.ts`) leased a fresh sandbox on every attempt and recorded nothing on a pass. The lifecycle lease (`modules/agent-lifecycle/adapters/postgres.ts`) named no holder process, so a restart could only wait for it to expire. ## What Changed - Restart chaos TTFT driver: a `verify` phase, reported in its own rows, and a `--verify-fault K1|K2|K6` option. The fault fires 1 s after `verifying` is observed, so it interrupts a held lease. The seed keeps each agent's creation body, so the driver can hire an identical agent. - **Persisted pass:** new columns `agents.harness_verified_digest` and `harness_verified_at` (migration 0448). The digest is an HMAC, keyed with the instance's decision-signing secret, over: - the fully resolved effective configuration: every value the test used, including `cwd`, resolved credentials, and agent-over-environment precedence; - the selected AI account and its credential generation (connection and grant ids and `updatedAt`, never the credential); - the environment and the runner image. The key means the stored value cannot confirm a guessed credential offline. For a sandbox, the runner image is an immutable image digest or a qualified Daytona snapshot ID (`sandboxBinding`, now exported). For local runs it is the server version. A mutable image or snapshot name, or a local environment redirected to the managed sandbox, never reuses a pass. - **Server-only:** `normalizeAgentBaseRow` and `redactAgentRowForResponse` strip the digest and its timestamp from agent API responses. - `verify()` still runs every account, policy and secret check, then returns complete on a matching digest without booting a test target. It records the digest on a pass before the lifecycle step completes, so a restart in between reuses it. - **Lease holder:** the lifecycle `claim()` stamps the claiming controller process on the lease (`lifecycleOperation.leaseHolder`, supplied through the lifecycle effects port) and clears the stamp with the lease. The compare-and-swap release lives in the lifecycle module (`releaseAgentLifecycleLeaseOfHolderInTransaction`). - **Shared ownership path:** a new `agent_lifecycle_lease` kind with `settleAgentLifecycleLease`. - When the reconciler proves the holder gone, settle clears exactly that lease (compare-and-swap on the holder), notifies lifecycle work, and logs `agent.lifecycle_lease_reconciled`. - A live holder is never taken over. The lifecycle state, operation and progress are untouched. ## Verification - `restart-verification.test.ts` (embedded Postgres, real lifecycle store and worker, real reconciler pass): - A verification held by a dead process is claimed and completed at once after the startup pass, with the agent ready in under 10 s instead of waiting out the 120 s lease. It fails without the new kind. - A live holder is never taken over. - The claim stamps the holder, and `defer` clears it. - `agent-harness-verification-preflight.test.ts`: - A second `verify()` of the same configuration allocates no target. A revoked account still fails before reuse. A changed model, `cwd` or literal credential tests again. It fails without the reuse. - A failed verification records nothing. - Ownership report, all lifecycle module tests, remote, provider-API and unit harness verification tests, and crash-point call sites: 13 files, 220 tests pass. - Targeted local run: the TTFT driver's new `verify` phase (`--phases verify --verify-fault K1`, Codex, 2 samples) hires a new agent, assigns it a task at once, kills the control plane (K1) while it verifies, and times fault to first agent token: - Before (master server code): 129.8 s and 129.8 s. The restarted server waited out the dead verifier's 120 s lease. Run-log TTFT from run creation was only 4.1 s. - After (this branch): 15.8 s and 17.2 s. The startup reconciler pass settled `agent_lifecycle_lease` and verification re-ran at once. ## Risks - **A stale pass:** the digest covers every resolved value, the selected account generation, the environment and an immutable runner image, so any change tests again. Rotating the instance signing secret invalidates every digest, so each agent verifies once more. - **Migration:** two nullable columns, no backfill. Existing agents verify once more, then reuse. - **Settle:** it runs only on a `gone` proof, compare-and-swap on the exact holder. It never changes the lifecycle state. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
d5cbe70636 |
fix(workspaces): create git worktrees one at a time per repository; retry another process's config lock (#20039)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Tasks in a project with git_worktree isolation each get their own
worktree, created with `git worktree add -b <branch> <path> <base>`
> - That command writes the repository's shared `.git/config` to record
the new branch's upstream
> - When two tasks start at once on one repository, both write
`.git/config`, and one fails with "could not lock config file"
> - The failed attempt already created its branch, so the retry attached
that branch without upstream tracking and without runtime ownership
> - This pull request serializes worktree creation per repository and
retries another process's brief config lock cleanly
> - The benefit is that simultaneous tasks on one repository start
without a failed attempt or a degraded branch
## Linked Issues or Issue Description
**What happened?**
In a TTFT measurement with a fresh clone of the paperclip repository as
the project workspace, two agents started tasks at the same time. The
Codex task's `git worktree add -b` failed twice ("error: could not lock
config file .git/config: File exists"). It then succeeded by attaching
the branch the failed attempt had created.
**Steps to reproduce**
1. Use a project whose primary workspace is a git clone, with
git_worktree isolation and a remote-tracking base ref.
2. Start several tasks in that project at the same moment.
3. Some worktree creations fail with "could not lock config file". The
task falls back to attaching the existing branch, without upstream or
branch ownership.
**Expected behavior**
Every simultaneous task gets a fresh worktree on its own new branch,
with upstream tracking, owned by the runtime.
**Root cause**
`git worktree add -b` writes `.git/config`, which git locks with
`config.lock`. Concurrent creates on one repository contend for that
lock. On a lock failure, git has already created the branch but no
worktree.
## What Changed
- `executeGitCommand` (in `workspace-runtime.ts`) runs every `git
worktree add` from `recordGitOperation`. The recorded and unrecorded
paths both use it.
- Creates in this process run one at a time per repository, keyed by the
realpath of `git rev-parse --git-common-dir`. Linked worktrees and the
primary checkout share one key.
- A create that still meets a config lock held by another process
retries after 100, 250, 500, 1000 and 2000 ms. Before each retry it
removes the branch that same `-b` command created. Git refuses an
existing branch with "already exists" before it touches config, so a
branch present after a config-lock failure is one this command made. The
retry then creates the branch again with its upstream.
## Verification
- `workspace-worktree-concurrency.test.ts`:
- Three rounds of ten simultaneous `realizeExecutionWorkspace` calls on
a fresh clone with base `origin/main`. Every workspace is `created:
true` and `branchCreatedByRuntime: true`, on its own branch. Without the
serialization it failed 3 of 3 runs.
- A create that meets `.git/config.lock`, held for 300 ms as another
process would, succeeds with the branch's upstream recorded.
- `workspace-runtime.test.ts`: 179 tests pass.
## Risks
- Worktree creation on one repository is now serial within a process.
Each create takes about 0.1–1 s, so ten simultaneous tasks add a few
seconds at most. Creates on different repositories still run in
parallel.
- The branch deletion runs only after a config-lock failure of a `-b`
create, when the branch is provably new.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file editing, test execution).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
ba867dea39 |
fix(recovery): keep the orphan marker for a stop intent saved before #19976 (#20043)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane stops and keeps the sandbox of an orphaned run lease, then destroys it once nothing references it > - #19964 marked such a sandbox only inside its stop intent; #19976 added the top-level marker the destroy looks for > - A stop still pending when #19976 shipped lost the intent on success, so the destroy never saw that sandbox > - This pull request carries the orphan marker forward when a stop intent that says `recoveredOrphanedRunLease` is confirmed > - The benefit is that those sandboxes are destroyed like every other retained orphan ## Linked Issues or Issue Description Refs #19964 and #19976. This is a follow-up gap from the Phase 2 ownership migration. No duplicate PR was found. **What happened?** Between #19964 and #19976, the shipped builds were `canary/v2026.1108.0-canary.6` and `canary.7`. In that window the orphaned-run-lease settle wrote `recoveredOrphanedRunLease: true` only on the `sandboxStopAndRetain` intent. `settleStopOnlyCleanup` removes the intent when the stop is confirmed, and the retained-orphan kind lists only leases with the top-level `orphanedRunLeaseRecovery` marker. So an orphan whose stop was still pending at upgrade stays `released` with a stopped sandbox forever. **Expected behavior** Once its stop is confirmed and nothing references it, the orphan is destroyed through the retained-orphan settle, whichever build wrote the intent. **Steps to reproduce** 1. Write a `pending_cleanup` lease with a `sandboxStopAndRetain` intent that has `recoveredOrphanedRunLease: true` and no top-level marker (what canary.6/.7 wrote). 2. Confirm the stop through `settleStopOnlyCleanup`. 3. Run an ownership reconciliation pass. Before this PR the retained-orphan kind never lists the lease. **Paperclip version or commit** `master` before this PR. Rows written by `canary/v2026.1108.0-canary.6` and `canary.7`. ## What Changed - `settleStopOnlyCleanup` writes the top-level `orphanedRunLeaseRecovery` marker (`fromStopIntent: true`) when a confirmed stop's intent carries `recoveredOrphanedRunLease: true` and the lease has no marker yet. - An ordinary stop-and-retain (no recovery flag) is not marked. ## Verification - Red, before the change: "keeps the orphan marker for a stop intent saved before the top-level marker existed" failed. - Green: `environment-retained-orphan-sandbox` and `environment-run-lease-ownership` 36/36; `environment-runtime` and `heartbeat-pending-cleanup-sweep` 246/246. - Counter-case: an ordinary stop-and-retain intent stays unmarked, and the retained-orphan pass does not settle it. ## Risks - Not covered: an orphan whose stop was confirmed *while* canary.6/.7 ran kept no durable marker at all. Its intent was removed and the receipt never carried the flag, so it cannot be told apart from an intentional stop-and-retain. Those rows exist only on instances that ran those two canaries during about three hours, and the provider's own auto-delete bounds them. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
cefd17a82b |
fix(environments): renew the stop-only cleanup claim while the provider stop runs (#20042)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a run ends with a "stop and retain" disposition, the control plane stops the agent's sandbox and keeps it for later recovery > - The release claims the lease with a 15-minute in-flight fence, so no cleanup sweep acts on it at the same time > - The stop-only path never renewed that fence; a provider stop that outlives 15 minutes lets a sweep take the lease over and stop it again mid-flight > - This pull request renews the stop-only claim every 30 s while the provider stop runs, like the destroy claim and the pending-cleanup sweep > - The benefit is one owner per cleanup attempt, however slow the provider is ## Linked Issues or Issue Description Follow-up gap from the Phase 2 ownership migration (durable ownership and reconciliation plan). Related: #19966 (teardown claim on the shared path), #19980 (pending-cleanup queue owner). No duplicate PR was found. **What happened?** `releaseRunLeases` with `providerResourceDisposition: "stop_and_retain"` claims the lease (`pendingCleanupInFlight: true`, `pendingCleanupLeaseExpiresAtMs: now + 15 min`) and calls the provider stop. Nothing renewed the fence while the stop ran. The destroy claim and the pending-cleanup sweep both renew theirs every 30 s. **Expected behavior** While this process is still running the provider stop, its claim stays fenced, so no other sweep starts a second attempt. **Steps to reproduce** 1. Release a run lease with `stop_and_retain` against a provider whose stop RPC blocks. 2. Let more than 15 minutes pass. 3. See that the fence expired while the first stop is still in flight. **Paperclip version or commit** `master` before this PR. ## What Changed - New `renewCleanupClaim(leaseId, attemptId, failure)` in `environment-runtime.ts`. It renews `pendingCleanupLeaseExpiresAtMs` every 30 s, only while the row still names this attempt and is in flight. - The destroy claim now uses the helper; its behaviour is unchanged. - The stop-only claim starts the renewal right after the claim and stops it before it settles the attempt, on success, on failure and in `finally`. ## Verification - Red, before the change: `environment-runtime.test.ts` "renews the stop-only cleanup claim while the provider stop is still running" failed. The fence stayed at its aged value. - Green: `pnpm exec vitest run src/__tests__/environment-runtime.test.ts src/__tests__/heartbeat-pending-cleanup-sweep.test.ts`, 247/247. ## Risks - Low. One extra metadata write every 30 s per in-flight stop, the same as for the destroy claim. The renewal is fenced on the attempt id, so it never extends another attempt's claim. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |