Commit Graph
27 Commits
Author SHA1 Message Date
aaeff418cf fix(acpx): add a positive control to stalled-tool permission evidence (#16769)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The ACPX engine runs Claude agents. A watchdog ends a turn when a
pending tool call gets no update for 30 minutes.
> - #16688 added a field to stalled runs. The field records if the
stalled call's permission request reached Paperclip.
> - Recent stalled runs show `acpStalledToolPermissionRequested: false`.
We can read this as "the call stopped before its permission request".
That reading is correct only if calls of the same kind usually send a
request.
> - We have no data that shows if finished calls of that kind sent
requests. Thus we cannot tell if the missing request is a real signal.
> - This pull request records a positive control: the number of finished
calls of the stalled call's kind, and how many of them sent a permission
request.
> - The benefit: the next stall event tells us if the missing request
locates the stall, or if it gives no information.

## Linked Issues or Issue Description

Refs #16688

**What happened?**
Runs stop with the stalled-tool watchdog error. The diagnostics show
that the stalled call sent no permission request. The diagnostics do not
show if finished calls of the same kind sent requests.

**Expected behavior**
The stalled-run diagnostics contain enough data to show if a missing
permission request is a real signal.

**Steps to reproduce**
A pending tool call gets no ACP update for 30 minutes. The watchdog ends
the turn. The run result contains the stalled-tool fields.

## What Changed

- `packages/adapter-utils/src/acpx-engine/execute.ts`: when the watchdog
fires, count the finished calls of the stalled call's kind, and count
how many of them sent a permission request. Write the counts to the run
result as `acpSettledSameKindToolCount` and
`acpSettledSameKindToolPermissionRequestedCount`.
- `server/src/services/run-failure-diagnostics.ts`: allow the two new
fields as non-negative safe integers. Other values are dropped.
- `doc/observability.md`: explain how to read the two fields with the
existing permission-request field.
- Tests: extend the existing stalled-tool tests in `execute.test.ts` and
`run-failure-diagnostics.test.ts`.

## Verification

- `pnpm exec vitest run src/acpx-engine/execute.test.ts` in
`packages/adapter-utils`: 244/244 pass.
- `pnpm exec vitest run
src/services/__tests__/run-failure-diagnostics.test.ts` in `server`:
115/115 pass.
- Mutation check: remove the kind filter, or count every finished call
as "requested". Each mutation makes the stalled-tool test fail.
- `tsc --noEmit` passes in `packages/adapter-utils` and in `server`.
Note: the full `server` `typecheck` script did not run locally, because
the runner vendor build step failed in this sandbox. I ran `tsc
--noEmit` directly.

## Risks

- Low risk. The change only adds two counts to the run result after the
watchdog fires. It does not change the decision to stop the turn, or any
permission decision.
- The fields hold only counts. They contain no tool titles, inputs, or
identifiers. This is the run-log and run-result path. It is not
Telemetry. The fields reach Sentry through the existing allowlist in
`run-failure-diagnostics.ts`.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-18 13:59:16 -07:00
63306f6660 fix(acpx): record whether a stalled tool call's permission request arrived (#16688)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Hosted `claude_local` runs use the ACPX engine. Claude can start
background subagents, and those subagents call the runtime MCP tools
that Paperclip attaches.
> - #16671 added a stalled-tool watchdog. It ends a Claude turn when a
pending tool call gets no ACP update for 30 minutes. That PR said the
most likely cause of the stall is a lost ACP permission round trip, but
it could not prove it.
> - A staging run tripped the watchdog. Two background subagents started
12 MCP calls. ACP reported each call as `pending`, and no call got
another update. The MCP gateway logs show the 9 earlier calls from the
parent agent, but no request from the 12 subagent calls.
`MCP_TOOL_TIMEOUT` (15 minutes) did not end them. So the calls stalled
before Claude sent them to the MCP server.
> - The run record cannot say if the permission request for a stalled
call reached Paperclip. Without that fact, the next stall gives no new
evidence about the cause.
> - This pull request records, for a stalled run, whether the stalled
call's permission request reached Paperclip, and how many pending calls
had a permission request.
> - The benefit is that the next stall tells us which side of the
permission round trip stopped, so the real fix can go to the correct
layer.

## Linked Issues or Issue Description

- Refs #16671 (the stalled-tool watchdog; this PR tests the cause that
PR suspected)
- Refs #16360 (the MCP client deadline that cannot reach a call that was
never sent)

**What happened?**

A hosted staging `claude_local` run started two background subagents.
Each subagent started six `execute-sentry-tool` MCP calls. ACP reported
all 12 calls as `pending`. None got a result, an update, or an error.
The MCP gateway received no request for them. The parent turn finished
its answer but stayed open for the subagents. After 31 minutes the
watchdog ended the turn with `acpx_tool_stalled`. A single background
subagent with six parallel calls to the same tool did not stall in a
later test.

**Expected behavior**

When a tool call stalls, the run record shows if the call's permission
request reached Paperclip. Then the stall can be put on the agent side
or on the relay side of the round trip.

**Steps to reproduce**

1. Start an ACP `claude_local` run in a sandbox that uses two background
subagents to call runtime MCP tools.
2. Wait until a subagent's tool calls stay `pending` with no further ACP
update (in staging this happened without a known trigger).
3. See that the watchdog fails the run with `acpx_tool_stalled`, but the
record has no evidence about the permission round trip.

**Paperclip version or commit**

`master` at 1f27427.

## What Changed

- The ACPX engine passes an `onPermissionRequest` hook to the ACPX
runtime. The hook records the tool call ID of each permission request
and returns `undefined`. ACPX then uses its permission mode to decide
the request, the same as before.
- When the watchdog fires, it records `permissionRequested` for the
stalled call and counts the pending calls that had a permission request.
- A stalled run's result has two new fields:
`acpStalledToolPermissionRequested` (boolean) and
`acpPendingToolPermissionRequestedCount` (non-negative integer). The
`[paperclip]` stderr line also says if the request reached Paperclip.
- The server run-failure diagnostics allowlist the two fields. They
accept only a boolean and a safe non-negative integer.
- `doc/observability.md` describes the fields. It also says that a
background subagent's Task call completes when the subagent starts, so
the watchdog watches the subagent's own calls.

## Verification

- `npx vitest run src/acpx-engine` in `packages/adapter-utils`: 21
files, 423 tests pass.
- Engine tests:
- The existing stall test now sends a permission request for the Task
call only. The run records `acpStalledToolPermissionRequested: false`
and a count of 1. The hook returns `undefined`. The log line says the
request did not reach Paperclip. No tool title or ID appears in the
diagnostics.
- A new test sends the permission request after the relay saw the
stalled call. The run records `acpStalledToolPermissionRequested: true`.
- Mutation check: when the hook does not record the ID, both tests fail.
- `npx vitest run
src/services/__tests__/run-failure-diagnostics.test.ts` in `server`: 115
tests pass. Invalid values for the new fields are dropped.
- `npx tsc --noEmit` passes in `packages/adapter-utils` and in `server`.
- Runtime dependency assumption: the hook must not change permission
decisions. I called the real patched `acpx@0.12.0`
`AcpClient.handlePermissionRequest` with and without the hook in
`approve-all`, `approve-reads` and `deny-all`. The responses and the
permission stats were the same in each mode, and the hook saw each
request. The hook only adds an ID to an in-memory set for the run, so it
adds no remote call and no blocking work.
- Limits: the tests mock the runtime, so they do not prove the hook runs
in a real sandbox relay. The acpx source passes the hook to the client
on every runtime path (`runtime.js`), and the real-client check above
covers the decision path.

## Risks

- Low risk. The hook only observes. If ACPX ever treats a host hook as
the decision owner, the check above would show a different decision.
That is not the case in the pinned `acpx@0.12.0`.
- The set holds one string per permission request for the length of one
run. That is bounded by the number of tool calls in the run.
- This PR does not fix the stall. It gives the evidence for the next
occurrence.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use
and code execution. It read the run log, the gateway logs, and the ACP
agent and client code, ran reproduction probes, edited the code, and ran
the tests.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-17 21:05:43 -07:00
938a441944 fix(acpx): end Claude turns whose pending tool call stops receiving updates (#16671)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Hosted `claude_local` runs use the ACPX engine. Paperclip sends
runtime MCP servers to Claude, and Claude can start subagents (the Task
tool) that call those MCP tools.
> - #16360 set `MCP_TOOL_TIMEOUT` for Claude. That bound works: in a
local test with the same Claude binary, a direct MCP call and a subagent
MCP call to a server that never answers both ended at the bound.
> - A hosted run still hit the 4h sandbox timeout on a release that has
#16360. Its run log shows a subagent that started four MCP calls. ACP
reported the four calls as `pending`, and they never got another update.
About 40 other MCP calls in the same minutes finished in 3–25 seconds.
So the four calls most likely never reached the MCP server, and
`MCP_TOOL_TIMEOUT` did not apply to them.
> - The subagent's Task call got an `in_progress` heartbeat every 30
seconds for the full 4 hours. The turn looked active, so no existing
bound stopped it.
> - This pull request adds a stalled-tool watchdog to the ACPX engine.
It ends the turn when a pending tool call gets no update for 30 minutes
and no other tool call finishes in that time.
> - The benefit is that a stalled run fails in about 30 minutes with a
clear error code, not after 4 hours with a general timeout.

## Linked Issues or Issue Description

- Refs #16360 (MCP client deadline; this PR covers the stall that the
deadline cannot reach)
- Refs #15962 (stop-time diagnostics that showed the pending tools and
their age)

**What happened?**

A hosted `claude_local` run gave lookups to two subagents. One subagent
started four MCP tool calls. ACP reported each call as `pending`. No
call got a result, an update, or an error. The other subagent finished
in 3 minutes. The stalled subagent's Task call got a heartbeat every 30
seconds, so the last-event age at stop was only 28 seconds. The oldest
pending tool was 3h58m old. The sandbox execution timeout ended the run
after 4 hours.

**Expected behavior**

A tool call that never gets a result ends the turn after a bounded time.
The run fails with an error that names the stall.

**Steps to reproduce**

1. Start an ACP `claude_local` run that uses a subagent to call runtime
MCP tools.
2. Make one of the subagent's tool calls stay `pending` with no further
ACP update (in production this happened without a known trigger).
3. See that the run continues until the adapter execution timeout,
because the Task heartbeats keep the turn active.

**Paperclip version or commit**

`master` at 4415665.

## What Changed

- Add `ACPX_STALLED_TOOL_TIMEOUT_MS` (30 minutes),
`ACPX_STALLED_TOOL_MARGIN_MS` (15 minutes), the check interval, and the
`acpx_tool_stalled` error code in `acpx-engine/constants.ts`.
- Add `resolveStalledToolTimeoutMs()`. Claude turns use 30 minutes, or
`MCP_TOOL_TIMEOUT` / `BASH_MAX_TIMEOUT_MS` plus 15 minutes if that is
longer. Other agents are not watched. The adapter config field
`stalledToolTimeoutSec` overrides the bound for any agent, and 0
disables it. The bound is not part of the session fingerprint.
- The engine records the time of the last update for each tool call, and
the time a tool call last reached a final status.
- During a turn, the engine checks the pending tool calls every 60
seconds. If a pending call has had no update for the bound, and no tool
call reached a final status in that time, the engine:
- records the stop-time activity (the same fields as a timeout or an
operator stop),
- writes a `[paperclip]` stderr line with the closed tool kind and the
silent time,
- cancels the turn, and starts the existing fail-fast cancel deadline
that the duplex-loss path uses. The deadline helper is now shared by
both paths.
- Finalization reports the stall as a failure: `errorCode:
"acpx_tool_stalled"`, `resultJson.status: "failed"`,
`acpStalledToolKind` (one closed ACP `ToolKind` label or `unknown`), and
`acpStalledToolSilentMs`. A timeout or a duplex loss still takes
priority. A turn that completed anyway keeps its success.
- The server Sentry diagnostics allowlist the two new fields. It drops
the kind if it is not one closed label.
- Document the watchdog in `doc/observability.md` and the
`stalledToolTimeoutSec` field in the `claude_local` config notes.

## Verification

- `npx vitest run src/acpx-engine` in `packages/adapter-utils`: 21
files, 422 tests pass.
- New engine tests:
- A silent pending call next to a Task call that keeps getting
heartbeats ends the turn with `acpx_tool_stalled`, the closed kind, and
the silent time. No tool title or ID appears in the diagnostics.
- An agent that ignores the cancel: the fail-fast deadline closes the
stream and the run fails with `acpx_tool_stalled`.
  - A long tool that keeps getting updates is not cancelled.
- A queued call that waits behind slow calls that keep finishing is not
cancelled.
- `stalledToolTimeoutSec: 0` disables the watchdog, and only the
wall-clock timeout ends the turn.
- `resolveStalledToolTimeoutMs()` defaults, env bounds, other agents,
and config override.
- Mutation checks: with the watchdog disabled, the two stall tests time
out (the same hang as production). Without the "no tool finished" guard,
the queued-call test fails.
- `npx vitest run
src/services/__tests__/run-failure-diagnostics.test.ts` in `server`: 115
tests pass.
- `npx tsc --noEmit -p packages/adapter-utils`: passes.

## Risks

- A legitimate tool call that gets no ACP update for 30 minutes while no
other tool call finishes now fails the run. Claude's own tool bounds
(MCP 15 minutes, Bash 10 minutes by default) end healthy calls before
that. If an operator raises `MCP_TOOL_TIMEOUT` or `BASH_MAX_TIMEOUT_MS`
in the env, the bound moves up with it. Operators can also set
`stalledToolTimeoutSec` or disable the watchdog with 0.
- A long sync subagent stays safe while its Task call gets heartbeats.
Only a call that is itself silent can trip the watchdog.
- The run fails instead of continuing. The failure uses the normal
failed-run recovery, and it reports to Sentry under its own error code,
not under the generic timeout.
- This PR does not find why the four calls never got a result. Claude
runs with `--permission-mode default`, so each MCP call makes an ACP
permission round trip, and the ACP stream goes through the remote
process-session relay. A lost request or response on that path is the
most likely cause. The new diagnostics show how often the stall happens
and for which tool kind.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use
and code execution. It read the run log and the ACP agent and client
code, edited the code, and ran the tests.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-17 14:51:49 -07:00
Devin FoleyandClaude Opus 5.5 11d69d04c7 fix(native-runtime): report remote recovery labels thrown before coordinator save (#16406)
Restart reattachment can fail in the executor before the native coordinator saves its result. The run failure report now reads the closed remote recovery stage and reason from the caught error when the saved result lacks them. The saved result keeps priority and the same closed-vocabulary validation applies.

Add a regression for thrown, wrapped-transport, saved-priority, and out-of-vocabulary cases. Full current-head CI and Greptile 5/5 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-15 02:30:58 -07:00
275189d8d6 fix(native): report why an unclassified Codex turn failed (#16354)
<!-- Write all pull request text in Simplified Technical English
(ASD-STE100): short sentences, one instruction per sentence, simple
approved vocabulary, and the active voice. -->

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs use the Paperclip runner to drive a provider session,
for example the Codex app server
> - When a Codex turn fails, two classifiers look for known failures: a
model rejection and a server capacity failure
> - When no classifier matches, the run fails with the generic message
`Native session failed` and the error code `adapter_failed`
> - The run failure report to Sentry then has no data about the cause,
because the server drops the Codex `codexErrorInfo` value
> - This pull request records a closed-vocabulary diagnostic for that
failed turn and sends it to the Sentry `provider_failure` context
> - The benefit is that operators can see which Codex failure kind
causes these generic failures, and then write a targeted fix

## Linked Issues or Issue Description

**What happened?**
Hosted staging reports many run failures with the title `Error: Native
session failed`. The events have `agent_adapter=paperclip_runner`,
`error_code=adapter_failed`, `driverKind=codex_app_server`, and
`exitCode=1`. The events do not show why the Codex turn failed.

**Expected behavior**
A run failure report for a failed Codex turn shows the failure kind, so
that an operator can find the cause. The report must not contain
provider response text, prompts, or model names.

**Steps to reproduce**
1. Start a native Codex run.
2. Make the Codex app server end the turn with `status: "failed"` and an
error that is not a model rejection or `serverOverloaded`. For example,
use `codexErrorInfo: "contextWindowExceeded"`.
3. Look at the Sentry event for the run failure. Before this change, the
`provider_failure` context is empty.

**Agent adapter(s) involved**
Codex (native runner, `codex_app_server` driver).

## What Changed

- `native-provider-failure.ts`: The provider failure observation now
records a diagnostic for each failed Codex turn. The diagnostic has only
closed values: `provider: "codex"`, `category: "turn_failed"`, a known
`CodexErrorInfo` kind (or `unrecognized` / `absent`), an HTTP status
from 100 to 599 for the HTTP variants, and a runner failure code from an
explicit allowlist of the codes the runner itself writes on a failed
Codex turn (any other string becomes `unrecognized`, because a provider
error object can carry its own `code`). It never copies the provider
message.
- New `unclassifiedTurnFailure()` gives this diagnostic only for a
failed terminal turn that `forTerminal()` does not classify.
`forTerminal()`, error codes, error messages, retries, and finalizer
decisions do not change.
- New `readNativeTurnFailureDiagnostic()` rebuilds a persisted value
from the closed vocabulary and rejects all other values.
- `native-session-executor.ts`: The adapter result keeps the generic
message. It adds `resultJson.nativeTurnFailure` when the turn failure is
unclassified.
- `run-failure-diagnostics.ts`: The run failure report copies the
validated diagnostic into the `provider` context. Sentry sends this as
`provider_failure`.
- Tests: unit tests for the vocabulary, HTTP status bounds, unknown
kinds, runner codes, and privacy; a diagnostics test; and an update to
the end-to-end executor test for the `unknown-error` scenario.

## Verification

- `pnpm exec vitest run
src/services/native-runtime/native-provider-failure
src/services/__tests__/run-failure-diagnostics.test.ts
src/services/__tests__/run-failure-report*.test.ts
src/__tests__/sentry*.test.ts` (in `server/`): 6 files, 432 passed, 5
skipped.
- `pnpm exec vitest run
src/services/native-runtime/native-session-executor.test.ts` (in
`server/`): 698 passed.
- I could not run the full `tsc --noEmit` in my sandbox: the memory
limit stopped the process, and the runner vendor build needs `cargo`. CI
runs the full typecheck.
- To check by hand: make a Codex turn fail with an unclassified error.
Then make sure that the Sentry event `provider_failure` context shows
`category: turn_failed` and the `codexErrorInfo` kind, and no provider
text.

## Risks

- Low risk. The change adds diagnostic data only. Run status, error
code, error message, retry, and finalizer logic do not change.
- Privacy: the diagnostic uses an allowlist of Codex enum names, a
bounded integer, and an allowlist of runner-issued failure codes. Tests
make sure that provider text does not get through.
- If Codex adds a new `CodexErrorInfo` kind, the diagnostic shows
`unrecognized` until the allowlist gets the new kind.
- This change does not fix the root cause. It gives the data that is
necessary to find the cause.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude
Code with tool use (shell, file edit, test runs).

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Related: #15347 (capacity classifier), #15304 (model rejection
diagnostics).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 20:14:10 -07:00
ccc77c5b5e fix(server): record legacy controller lease-loss timings (#16359)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The heartbeat runner executes legacy agent runs, and a controller
lease watchdog keeps durable ownership of each run
> - The watchdog renews the lease every 10 seconds and stops the run
when a renewal does not finish within the 60-second lease
> - A staging run failed in setup with `LegacyControllerLeaseLostError`
and the `renewal_deadline` cause, after about eight minutes of
preparation and with no server restart
> - The setup failure path did not record the cause, and no evidence
showed why the renewal did not finish: a stalled database query or a
blocked event loop
> - This pull request records bounded watchdog timings with each lease
loss, on the setup failure path and on the cancellation path
> - The benefit is that the next event shows which of the two causes
stops renewal, which is the missing fact to fix the root cause

## Linked Issues or Issue Description

Refs #16084 (added the closed lease-loss cause for cancelled runs)
Refs #16355 (retries a run that a restart stopped before dispatch; a
different path)

**What happened?**
A legacy `paperclip_runner` run on staging failed with `setup_failed`
and the message "Legacy controller lease lost". The stack shows the
watchdog deadline timer (`renewal_deadline`). The run was in
`executionStage: "preparing"` for about 467 seconds. The server started
about 11 minutes before the run, so this was not a restart. The run
record and the Sentry event do not show the lease-loss cause, because
the setup failure path does not read it. Nothing shows if a renewal
query was in flight at the deadline, or if the deadline callback ran
late.

**Expected behavior**
When the watchdog loses the lease, the run records the closed cause and
the watchdog timings. This applies when the loss fails setup and when it
stops the adapter. The failure report contains only the closed cause and
bounded numbers.

**Steps to reproduce**
1. Start a legacy run with a controller lease.
2. During setup, make a lease renewal query hang for more than 60
seconds, or block the event loop past the deadline.
3. Let the watchdog abort the run.
4. Read `resultJson` on the failed run. Before this change, it has no
lease-loss evidence.

## What Changed

- `legacy-controller-lease.ts`: The watchdog tracks the last successful
renewal, the renewals in flight, the scheduled deadline, and the renewal
count. `LegacyControllerLeaseLostError` now carries `leaseLossTiming`:
`sinceRenewalMs`, `renewalPendingMs` (or `null`), `deadlineLateMs` (only
for `renewal_deadline`), and `renewalCount`. Ownership behavior does not
change.
- `run-cancellation.ts`: Add the `LegacyControllerLeaseLossTiming` type
and `readControllerLeaseLoss()`. The reader accepts only a closed cause
and non-negative finite numbers, and clamps each value to 24 hours.
- `heartbeat/run-completion.ts`: Persist
`resultJson.controllerLeaseLoss` with the cause and timings. This
applies to a lease loss that stops the adapter and to a lease loss that
fails setup (the thrown abort reason).
- `run-failure-diagnostics.ts`: Report `controllerLeaseLossCause` and
the four bounded timing fields from `controllerLeaseLoss`. A setup
failure now gets the cause label too.
- `doc/run-log-events.md`: Document the new record and how to read the
timings.
- Tests: watchdog timings for a stalled query, a blocked event loop, and
a failing query; completion records the evidence for an adapter stop and
for a setup failure; the failure report clamps values, drops unknown
keys, and drops a record without a closed cause.

## Verification

- `pnpm --filter @paperclipai/server exec vitest run
src/services/legacy-controller-lease.test.ts
src/services/heartbeat/run-completion.test.ts
src/services/__tests__/run-failure-report.test.ts
src/services/run-cancellation.test.ts`
- I could not run these tests locally. The sandbox has a 4 GB memory
limit, and `pnpm install` was stopped by the out-of-memory killer during
lockfile resolution. CI is the test run for this change.
- After deployment, a new lease loss shows
`controllerLeaseRenewalPendingMs` and `controllerLeaseDeadlineLateMs` in
the Sentry execution context. A pending time near 50 seconds or more
points at the database. A large deadline lateness points at the event
loop.

## Risks

- Low risk. This change adds diagnostics. It does not change when the
watchdog stops a run, and it does not prevent the lease loss.
- The new `resultJson.controllerLeaseLoss` field is server-written.
Readers ignore unknown values. Old rows do not have it.
- The failure report gets five closed or numeric fields. It gets no
reason text, IDs, or user content.
- The event-loop test blocks the loop for about 80 ms to make the
deadline late. It uses real timers, so a very slow CI host makes the
lateness larger, not smaller.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with
tool use (shell, file edits, Sentry and GitHub connectors). The exact
context-window size is not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [ ] I have run tests locally and they pass (not possible in this
sandbox; see Verification)
- [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: Devin Foley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 19:52:23 -07:00
DottaandPaperclip ff0f976de1 fix(native-runtime): retain remote recovery failure reasons (#16341)
Preserve closed marker-read and process-verification reasons through native recovery failures and operator reports. Keep existing transport errors and reattachment ownership checks.

Add shell, process-probe, persistence and privacy regression coverage. All current-head CI checks pass and Greptile is 5/5.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-14 21:24:02 -05:00
Devin FoleyandPaperclip 5b29b9062e fix(server): compact finished task worktrees and shorten the reaper cooldown (#16304)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Each task can run in its own isolated Git worktree, and each
worktree installs its own dependencies
> - The terminal-workspace reaper removes a worktree only when the task
tree is finished, the delivery is merged, the files are clean, and a
7-day cooldown has passed
> - A recurring task that runs every hour keeps a week of worktrees, and
each worktree holds gigabytes of `node_modules`. A finished task with no
merged pull request, or with leftover agent files, keeps its worktree
forever
> - On a busy instance this fills the disk in days, and a larger volume
only delays the next failure
> - This pull request adds compaction: one hour after a task tree
finishes, the reaper removes regenerable directories from the worktree
and keeps the workspace, the branch, and all source changes
> - The benefit is that finished tasks stop holding gigabytes of disk,
and no work is lost because a resumed task regenerates the files when
its workspace is provisioned again

## Linked Issues or Issue Description

Refs #2022 (retention for workspace cleanup). Related: #11936 and #14608
add external reclaim scripts (a scheduled tool and a disk-guard step).
This pull request does the reclaim inside the server's
terminal-workspace reaper instead. That way it uses the reaper's
lifecycle lock, issue-tree checks, and live-run checks, and needs no
extra scheduler.

**What existing behavior does this improve?**
Disk retention of execution workspaces after their tasks finish.

**Current behavior**
A finished task keeps its full worktree, including installed
dependencies and build output, for at least 7 days. If the delivery is
not merged, or the worktree has uncommitted or untracked files, the
reaper never removes it. On one instance, an hourly recurring task
created about 22 worktrees per day at about 3 GB each (about 2.8 GB of
that was `node_modules`). Together with finished tasks that had no
merged pull request, they filled a 100 GB volume in about two days.

**Proposed behavior**
One hour after a task tree becomes terminal, the reaper removes
regenerable directories (installed dependencies, build output, tool
caches) from the worktree. It keeps the worktree, the branch, the
commits, and every tracked, untracked, or other ignored file. When the
task resumes, the existing reuse path runs the provision step, which
reinstalls the dependencies. Clean, merged worktrees are archived after
1 day instead of 7.

**Reason and benefit**
Most of a finished worktree's size is regenerable. Compaction removes
that part without the risk of deleting work, so it can apply to
workspaces that the reaper must keep (no merged pull request, leftover
files). The shorter cooldown limits the cost of the source checkouts
that stay.

**Breaking changes**
None. The default archive cooldown changes from 7 days to 1 day.
`PAPERCLIP_WORKSPACE_REAPER_COOLDOWN_DAYS` still overrides it.
`PAPERCLIP_WORKSPACE_COMPACTION_ENABLED=false` turns compaction off.

## What Changed

- New `server/src/services/workspace-compaction.ts`:
- Finds directories with a regenerable name (`node_modules`, `dist`,
`build`, `coverage`, `storybook-static`, `target`, `.cache`, `.next`,
`.nuxt`, `.parcel-cache`, `.svelte-kit`, `.turbo`, `.vite`).
- Keeps only the directories that the repository's own `.gitignore`
files ignore and that hold no tracked file. A negated pattern
(`!build/`), a global excludes file, and `.git/info/exclude` do not
count. Each candidate is checked per path, so dependencies below an
ignored parent (`vendor/tool/node_modules`) are found.
- Searches inside a regenerable-named directory that does not qualify
(for example a tracked `build/`), so its ignored dependencies still go.
- Never enters `.git` or `.paperclip` (the worktree instance data), and
never follows a symlink.
- Each Git read has a 60-second deadline and stops when the sweep is
aborted.
- Moves the directories into `.paperclip-compaction-trash` beside the
worktree with a same-filesystem rename, and can restore them.
- `sweepTerminalWorkspaces` compacts each terminal workspace once per
terminal cycle, after the compaction delay and before the archive
cooldown check:
- Only isolated linked Git worktrees qualify. A shared workspace, a
project's main checkout, or a row whose `.git` is a directory is never
compacted.
- The move runs inside a transaction that holds the workspace lifecycle
lock. The guard checks a fresh row and the same lifecycle generation. It
also checks that the issue tree is terminal, that no queued or running
run works on the tree or the workspace (by checkout or execution run, or
by run context), and that no runtime service runs from the workspace.
- The guard runs before the move and again after it. If the second check
fails (for example, the task reopened or a run started), the directories
move back. Otherwise the trash is deleted.
- The worker wakes at the compaction deadline, not only at the archive
deadline.
- Stale trash from an interrupted compaction or a failed delete is
purged by later sweeps. The sweep checks each worktree parent at most
every 30 minutes, whether or not anything is left to compact.
- New counters `compacted` and `compactionFailed` in the sweep result.
The activity log gets an `execution_workspace.regenerable_files_removed`
entry with the removed paths.
- Config: new `PAPERCLIP_WORKSPACE_COMPACTION_DELAY_HOURS` (default `1`)
and `PAPERCLIP_WORKSPACE_COMPACTION_ENABLED`. The default for
`PAPERCLIP_WORKSPACE_REAPER_COOLDOWN_DAYS` changes from `7` to `1`. Both
new keys are added to the run-failure diagnostics allowlists.
- Docs: `doc/DEVELOPING.md` documents the new variables and has a new
"Terminal workspace compaction" section. `doc/SPEC-implementation.md`
describes compaction.
- `execution-workspaces.ts` was already above 1,000 lines. The
file-system logic is in the new module. Only the guarded orchestration,
which uses the existing lifecycle lock and run predicates, is in the
service.

## Verification

- `pnpm exec vitest run src/__tests__/workspace-compaction.test.ts
src/__tests__/execution-workspace-compaction.test.ts` (in `server/`): 27
passed. `workspace-reaper-cooldown-config.test.ts`: passed.
- Unit tests: directory selection (ignored, untracked, regenerable name
only, `.env` and local data kept, tracked `coverage` kept, `.paperclip`
skipped, symlinks not followed), nested candidates under a source
`build/` and under an ignored `vendor/`, a negated `!build/` kept,
global and `info/exclude` rules ignored, glob characters in names,
abort, main checkout vs linked worktree, move and restore, restore that
keeps a recreated directory, and stale-trash purge.
  - Embedded Postgres tests:
- A no-PR, unmerged, dirty workspace is compacted. HEAD, the branch ref,
`git status --porcelain --untracked-files=all`, untracked notes, `.env`,
local data, and `.paperclip` data stay the same, and the row stays
active.
    - A delivered workspace inside the cooldown is compacted.
    - The delay is respected, and the worker wakes at the delay.
- An active checkout run, a queued run found through its context, an
unfinished child issue, and a running runtime service all block
compaction.
- The task reopens during the move: the files are restored and
unchanged.
    - A run starts during the move: the files are restored.
    - A shared workspace and a main checkout are never compacted.
- Compaction runs once per terminal cycle and again after a later cycle.
    - Disabled compaction does nothing.
- End to end: `realizeExecutionWorkspace` creates a worktree, the sweep
compacts it, and a second `realizeExecutionWorkspace` reuses the same
worktree. The provision command regenerates `node_modules`, and the
untracked notes and the unmerged commit are still present.
- Leftover trash is purged when the workspace has nothing left to
compact.
- Mutation checks: when the post-move guard is disabled, both race tests
fail. Without the negation fix or the repository-only source fix, their
tests fail.
- `pnpm exec vitest run` for `execution-workspaces-service.test.ts`,
`terminal-workspace-work.test.ts`, `execution-workspace-reopen.test.ts`,
`server-startup-feedback-export.test.ts`, and
`workspace-runtime.test.ts`: 335 passed. Two `workspace-runtime.test.ts`
cases fail locally with and without this change (`does not diagnose a
missing remote branch as a rejected credential`, `rejects explicit main
on a master-only remote ...`). They also fail on unmodified `master` on
the same machine, so the cause is the local Git environment.
- `pnpm typecheck` in `server/`: passed. `pnpm check:module-boundaries`:
passed.

## Risks

- A repository can keep something that is not regenerable in an ignored
`dist`, `build`, `target`, or `.cache` directory. Compaction removes it.
The fixed name list, the ignore check, and the tracked-file check limit
this. `PAPERCLIP_WORKSPACE_COMPACTION_ENABLED=false` turns compaction
off.
- A repository without a provision command does not get dependencies
back automatically. This is the same state as a new worktree for that
repository, where the agent already installs them.
- A resumed task pays for a dependency install again. This happens only
for tasks that finished more than an hour earlier.
- The shorter default cooldown removes clean, merged worktrees after 1
day instead of 7. An operator who wants the old hold can set
`PAPERCLIP_WORKSPACE_REAPER_COOLDOWN_DAYS=7`.
- Self-hosted instances get the same behavior. There is no schema
change.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code
with tool use (shell, file edits, test runs). A separate Claude agent
read the reaper code during the diagnosis.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-14 09:49:04 -07:00
DottaandPaperclip 21ee5c9c85 feat(sentry): add opt-in run failure reporting with saved run references (#16300)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agent runs retain execution results and output in the instance.
> - Operators can report run failures to Sentry for investigation.
> - A backend Sentry DSN currently enables those reports with no
separate choice.
> - A repair bot also needs the exact instance, company, agent, and
saved run.
> - This pull request adds an explicit reporting mode and authenticated
run references.
> - The benefit is that operators can investigate every failure on
selected deployments while keeping other deployments quiet.

## Linked Issues or Issue Description

**What existing behavior does this improve?**

Optional server reporting of terminal agent run failures to Sentry.

**Current behavior**

A backend DSN enables run failure reports. Known account, quota, and
connection failures are excluded. Reports omit company and agent
references. Runs without a task are skipped. Tool-name collision errors
omit the conflicting bindings.

**Proposed behavior**

Set `PAPERCLIP_SENTRY_RUN_FAILURES=unexpected` to retain the existing
filters. Set it to `all` to report every failed or timed-out run,
including known setup and quota failures. The default is `off`. Expected
stops stay excluded. Each report supplies identifiers and API paths that
a bot can use with its own authorized Paperclip connection.

**Reason and benefit**

Operators can enable broad failure reporting on one staging deployment.
A bot can fetch the saved run, events, and logs instead of relying on a
short error label or copying complete transcripts into Sentry.

**Breaking changes**

A backend DSN alone no longer enables run failure reports. Existing
operators must set `PAPERCLIP_SENTRY_RUN_FAILURES=unexpected` to retain
the previous policy. Ordinary server exception capture remains enabled
by the DSN. Failure grouping gains the phase and original cause code
when available.

Refs #16281, #16297, #16298. These cover related tool discovery and
failure diagnostics. This change retains their existing boundaries and
does not depend on their open changes.

## What Changed

- Add `off`, `unexpected`, and `all` modes. Missing and invalid values
keep reporting off before diagnostic reads.
- Keep the Sentry backend DSN and audited optional SDK requirements.
- Include company, agent, run, and optional task identifiers. Include
versioned paths for the saved run, events, log, and task. Add absolute
run URLs only for a valid public HTTP(S) origin.
- Report company runs without a task. Prefer the first-class issue
binding when selecting the task reference.
- Select saved native provider/model settings, timestamps, and original
failure codes. Preserve gateway reason codes through redacted exception
projection.
- Group by error code, adapter, and available phase/cause. Keep run IDs
and error text out of grouping.
- Include the tool names and connection bindings in collision errors
without copying tool schemas or provider metadata.
- Document configuration, upgrade behavior, bot access, redaction,
retention, and best-effort delivery.

## Verification

- All 56 checks passed for `cccb47a721eaa1cc36e63278fc177b9101477541`.
Two unrelated Storybook jobs were skipped by their path filters. No
checks remain pending.
- Complete [Paperclip CI
run](https://github.com/paperclipai/paperclip/actions/runs/38591422284):
passed. This covers general and serialized tests, Runner checks, browser
E2E shards, typecheck, build, release-registry checks, canary dry run,
and Docker checks.
- Greptile reviewed that exact head: 5/5, with no unresolved threads or
actionable findings. Superagent, Socket, and Snyk checks passed.
- Local focused reporting, diagnostics, collision, and real SDK contract
tests: 410 passed across five suites. This includes the external-wake
filter in both reporting modes.
- The audited `@sentry/node@10.71.0` tests use an in-memory transport.
No events were sent to Sentry.
- `pnpm -r typecheck` and `pnpm build`: passed locally after the final
rebase.
- `node scripts/check-module-boundaries.mjs` and `git diff --check`:
passed.
- The full local `pnpm test:run` command was stopped to avoid
duplicating the complete CI shards. It did not finish locally. The full
suite passed in CI.

## Risks

- The new default disables existing run failure exports until the
operator selects a mode. The upgrade setting is documented.
- `all` can increase event volume and include known user-action
failures. Secret redaction and diagnostic limits still apply.
- Reporting remains asynchronous and best effort. This PR adds no
durable delivery queue and does not install a bot or Sentry alert rule.
- API references grant no authority. Existing company, private-task, and
run-output checks still apply. Complete transcripts stay in Paperclip.
- This changes optional Sentry error reporting. It changes neither
first-party Telemetry nor OpenTelemetry span exports. It adds no
database migration.

## Model Used

- OpenAI GPT-6 through Codex, with reasoning, shell, file editing, and
test execution. The exact backend deployment ID and context window are
not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-14 10:56:40 -05:00
03fcd3c1fb fix(native-runtime): report which close barrier failed (#16298)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native agent runs use the Paperclip runner. When a run ends, the
controller closes the session. Close must prove that the runner durably
suspended before the controller releases its checkpoint.
> - If close cannot prove this, the run fails with
`provider_transport_failed: runner did not durably suspend before
checkpoint`.
> - The runner knows which barrier was not proven. It writes that
settlement only to the instance run log.
> - The Sentry report for the failed run has no settlement evidence.
Operators cannot tell a stuck suspend command from an undrained
provider, an unreadable provider state, or unsettled tool results.
> - This pull request sends that settlement to the run failure report as
closed labels and counts.
> - The benefit is that the next occurrence shows which barrier failed,
so the root cause can be fixed instead of guessed.

## Linked Issues or Issue Description

No public issue exists. This describes the bug.

**What happened?**

Hosted staging native Codex runs failed with `provider_transport_failed:
runner did not durably suspend before checkpoint`. This happened on two
runs, on 2026-09-30 and 2026-10-14. The second run used a release from
2026-10-14. The Sentry events contain only the error code, run phase,
driver kind, and duration. They contain no stack and no settlement data.
The native executor calls `reportRunFailure(db, run)` without the caught
error, and the runner settlement is never read.

**Expected behavior**

A failed close report should tell an operator which barrier was not
proven: runner suspension, provider drain, provider state, or semantic
tool settlement. It must not expose tool identities, digests, provider
turn IDs, or unredacted error text.

**Steps to reproduce**

1. Run a native Codex session whose close cannot prove durable runner
suspension. For example, the runner suspend command does not complete
before the close deadline.
2. Observe the Sentry run failure event. Before this change,
`run_execution` has no close settlement fields.

## What Changed

- `collectRunFailureDiagnostics` accepts a new `nativeCloseFailure`
option. It reads only a `native_session_close_unrecoverable` error,
directly or up to four `cause` levels deep.
- It adds closed labels and counts to `run_execution`:
`closeRunnerSuspended`, `closeProviderDrained`,
`closeSuspendCommandStatus`, `closeRunnerLifecycle`,
`closeRunnerIdentityMatches`, `closeProviderState`,
`closeProviderPendingEventCount`, `closeProviderTurnActive`,
`closeSemanticResultPersistenceFailed`, `closeSemanticToolsPending`,
`closeSemanticToolDeliveriesOpen`, and `closeSemanticResultFailures`.
- Values outside the known vocabularies are dropped. Getters that throw
are contained.
- The native session executor passes the caught error as
`nativeCloseFailure` when it reports a terminal run failure. It does not
pass the error as `error`, because the raw error text and stack bypass
the executor's identity redaction.
- `doc/observability.md` documents the new fields and the privacy
boundary.

## Verification

- `cd server && npx vitest run
src/services/__tests__/run-failure-diagnostics.test.ts
src/__tests__/run-failure-sentry.test.ts`: 130 tests pass.
- New tests check the full label set, a wrapped cause, null suspension
and provider states, unrelated error codes, hostile and unknown values,
and a missing settlement. They also check that no tool identity, digest,
turn ID, or error text leaves the module.
- Negative control: with the reader disabled, the new main test fails.
- A scoped `tsc` run on `run-failure-diagnostics.ts`, its test, and
`run-failure-report.ts` passes. The full server `tsc` and `pnpm build`
were not run locally. The sandbox has a 4 GiB memory limit and no
`cargo` for the runner binary. CI runs the full checks.

## Risks

- Low risk. The change only adds Sentry context fields for one error
code. Close, quarantine, retry, and recovery behavior do not change.
- Sentry grouping does not change. The fingerprint stays `[errorCode,
agentAdapter]`, and no exception chain is added.
- The data path is Observability (Sentry error reporting, gated by the
operator DSN). It does not change Telemetry. The fields contain only
booleans, fixed labels, and counts. The DSN gate is unchanged.
- These fields are observations. They do not prove a root cause by
themselves.

> 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`), run through Claude Code
as an agent with tool use (shell, file edits, test execution, Sentry and
GitHub connectors). Context window and reasoning mode are not exposed in
this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] 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: Devin Foley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 07:57:21 -07:00
a64969b288 fix(server): attribute legacy runs stopped by controller lease loss (#16084)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The heartbeat runner executes legacy agent runs. A controller lease
watchdog keeps durable ownership of each legacy run.
> - When the watchdog loses the lease, it aborts the adapter. The abort
reason was a plain `Error`, and nothing recorded why the run stopped.
> - Completion then saved the cancellation as source `unknown`. Sentry
got an unexpected cancel that it could not tell apart from other
unrecorded stops.
> - We saw this on a sandboxed Grok run. The system cancelled it after
about 22 minutes with `cancellationSource: unknown`. Workspace copy-back
then failed. Every `cancelRun` path records a stop request, so an
unrecorded stop this late points to the lease watchdog.
> - This pull request records a lease loss as its own cancellation
source, with a closed cause label.
> - The benefit is that the next event shows whether the lease loss
caused the stop, and which renewal failure caused it. That is the
missing fact to fix the root cause.

## Linked Issues or Issue Description

Refs #15790
Refs #15920

**What happened?**
A legacy sandbox run was stopped by the system while it was working. The
run record and the Sentry diagnostics show `cancellationSource:
unknown`, with no initiator. The controller lease watchdog aborts the
adapter with a plain `Error("Legacy controller lease lost")`. Completion
does not read the abort reason, so it records the stop as `unknown`.

**Expected behavior**
When the host stops its own adapter because it lost the controller
lease, the run records that cause. Diagnostics name the cause with a
closed label. A stop that a user or the control plane requested earlier
keeps its recorded source.

**Steps to reproduce**
1. Start a legacy run that has a controller lease.
2. Make lease renewal fail: the renewal query errors, the update matches
no row, or the lease deadline passes.
3. Let the adapter return its cancelled result.
4. Read `resultJson.cancellation` on the run. Before this change,
`source` is `unknown`.

**Agent adapter(s) involved**
Not adapter-specific (core bug). We saw it with `grok_local` in a
sandbox.

**Additional context**
This change adds diagnostics. It does not prevent the lease loss. #15920
retries a transient copy-back failure. It does not help here, because
the stopped sandbox rejects every retry.

## What Changed

- `legacy-controller-lease.ts`: The watchdog aborts with a new
`LegacyControllerLeaseLostError`. The error carries a closed cause:
`renewal_rejected`, `renewal_deadline`, or `renewal_error`.
- `run-cancellation.ts`: Add the `controller_lease` cancellation source.
Add `controllerLeaseLossCancellation()` and
`readControllerLeaseLossCause()`. The reader accepts only the
server-written reason text.
- `heartbeat/run-completion.ts`: When the outcome is `cancelled` and the
abort reason is a lease loss, record a `controller_lease` cancellation
(`expected: false`). This covers both an adapter that returns a
cancelled result (`completeRun`) and one that throws after the abort,
for example through `signal.throwIfAborted()` (`failRun`). A stop
request recorded earlier still takes precedence.
- `run-failure-diagnostics.ts`: Add `controllerLeaseLossCause` to the
execution diagnostics. Only the closed label goes to Sentry, never the
reason text.
- `heartbeat/run-state.ts`: preserve `controller_lease` in the bounded
SQL projection when saved results exceed 64 KB. A database regression
verifies the default run-detail read.
- `canContinueCancelledRun` stays unchanged. It does not accept
`controller_lease`, so a lease loss does not authorize automatic
continuation.
- `doc/run-log-events.md`: Document the new source and cause labels.
- Tests: unit tests for the cause on each watchdog path, completion
tests that record the source when the adapter returns and when it throws
the abort reason (both fail without the change), tests that a recorded
stop request takes precedence, and a test that the Sentry report
contains the closed label and not the reason text.

## Verification

- `cd server && npx vitest run src/services/run-cancellation.test.ts
src/services/legacy-controller-lease.test.ts
src/services/__tests__/run-failure-report.test.ts
src/services/heartbeat/run-completion.test.ts`: 238 passed, including
the embedded Postgres suites.
- I reverted only `run-completion.ts` and ran the new completion test.
It failed with `source: "unknown"`. With the change, it records
`controller_lease`.
- I removed only the `failRun` attribution and ran the new thrown-error
test. It failed without the `controller_lease` record, and passes with
it.
- Final full repository `pnpm -r typecheck` and `pnpm build` pass.

- Final local regression verification: all 247 tests pass across
run-state, cancellation, watchdog, failure reporting, and completion
suites. The large-result test fails before the SQL allowlist change. All
latest-head CI checks pass (54 successes and two intentional skips).
Fresh Apex is 5/5 on `708856c568d07d7b446a4a4af2edc6d9eec7d155` with no
unresolved findings.

## Risks

- Low risk. The change adds a new source value to server-owned JSON. Old
rows keep their values. The readers accept the new value, and no UI code
switches on this field.
- A lease-loss cancel stays `expected: false`, so it is still reported
as unexpected, as before. Now it has a more specific source.
- The Sentry payload gets one new closed label
(`controllerLeaseLossCause`). It does not include reason text or IDs.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with
tool use (shell, file edits, test runs).

OpenAI GPT-6 (`gpt-6`) through Codex added the large-result regression
and SQL projection fix, with shell, file-edit, and test tools. The exact
context-window size is not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-13 14:50:40 -07:00
4b766b8729 fix(acpx): record turn activity at the moment a stop fires (#15962)
Record stop-time ACP turn diagnostics (last-event age, oldest pending tool age, pending tool kinds) so wall-clock timeout events show whether the turn was stalled. Diagnostics only.

Co-Authored-By: Paperclip <Paperclip-Paperclip@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-12 12:33:59 -07:00
657e71584a fix(daytona): record provider state when lease creation hits its deadline (#15946)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents can run in Daytona sandboxes. The Daytona plugin acquires a
lease before each run.
> - Some acquisitions time out. The report then says
`environmentAcquisitionPhase=create` and that the whole 300 s budget was
used.
> - The Daytona SDK create timeout is equal to the acquisition budget.
The outer timer always fires first. Thus the report never shows the
state that Daytona left the sandbox in.
> - Without that state, we cannot choose a fix. A stuck snapshot pull, a
slow start, and a missing allocation need different fixes.
> - This pull request adds one bounded, read-only provider lookup at the
create deadline. It records a fixed-enum provider state in the existing
acquisition diagnostic.
> - The benefit is that the next timeout identifies where the provider
stopped. Retry, budget, and cleanup behavior do not change.

## Linked Issues or Issue Description

Refs #15736 (bounded Daytona acquisition failure observations). This
pull request extends those observations.

**What happened?**

A Daytona lease acquisition timed out with `Daytona lease acquisition
timed out; allocation cleanup is pending`. The run-failure report gave
`environmentAcquisitionPhase=create`,
`environmentAcquisitionElapsedMs=300004`, and
`environmentAcquisitionBudgetMs=300000`. The report did not show the
Daytona sandbox state. The SDK create timeout is equal to the outer
budget, so the outer timer always fires first. The SDK never reports the
stuck state.

**Expected behavior**

When creation is still pending at the deadline, the report should show
where the provider left the allocation. Examples are `pulling_snapshot`,
`starting`, `error`, or `absent`.

**Steps to reproduce**

1. Configure a Daytona environment.
2. Start a run when Daytona does not finish create and start within the
acquisition budget.
3. Inspect the setup-failure report. Only the phase, the elapsed time,
and the budget are present.

**Paperclip version or commit**

`master` at ba04968.

## What Changed

- `packages/plugins/sdk`: Add an optional `providerState` field to
`PluginEnvironmentAcquisitionDiagnostic`. The values are a fixed enum:
`absent`, `creating`, `pending_build`, `building_snapshot`,
`pulling_snapshot`, `starting`, `started`, `error`, `build_failed`,
`other`, `unavailable`. The reader omits an unknown value and keeps the
other fields. Old envelopes stay valid.
- `packages/plugins/sandbox-providers/daytona`: When the acquisition
deadline fires while `client.create` is still pending, make one
`client.get` of the provisional `paperclip-create-<attempt>` name. The
lookup has a 5 s bound, inside the host's 30 s RPC margin. Report the
state only when the name and all ownership labels match. Otherwise
report `unavailable`. A not-found error reports `absent`.
- The lookup does not delete. It does not set `observedProviderLeaseId`.
It does not change retries, budgets, or the cleanup envelope.
- After the deadline, only the timer settles the call. A late create
result cannot replace the timeout error before the lookup completes. A
test proves this guard.
- `server`: Project the value to
`run_execution.environmentAcquisitionProviderState`. It uses the same
private, run-scoped receipt as the other acquisition fields.
- `doc/observability.md`: Document the new field and its limits.

## Verification

- `cd packages/plugins/sandbox-providers/daytona && npx vitest run
--config vitest.config.ts`: 392 passed, 6 skipped.
- `npx vitest run
packages/plugins/sdk/tests/environment-creation-cleanup.test.ts
server/src/services/__tests__/run-failure-diagnostics.test.ts
server/src/__tests__/environment-acquisition-diagnostics.test.ts
server/src/__tests__/run-failure-sentry-real-sdk.test.ts
server/src/__tests__/environment-runtime.test.ts`: 302 passed, 11
skipped.
- `tsc --noEmit` passes for `packages/plugins/sdk` and
`packages/plugins/sandbox-providers/daytona`.
- New tests cover state mapping, `other` for unlisted states, `absent`,
`unavailable` for lookup failure and label mismatch, a hung lookup
bounded to 5 s, no deletion, an unchanged cleanup envelope, and a late
create that settles during the lookup.
- A mutation check removed the late-settlement guard. The late-create
test then failed.
- The local server `tsc` was killed by the sandbox memory limit. CI must
confirm the server typecheck. The server change is one line.

## Risks

- Low risk. The timeout rejection can come up to 5 s later, and only for
a create-phase deadline. The host RPC margin is 30 s.
- The lookup is one extra read-only Daytona API call for each
create-phase timeout.
- This is a diagnostic. It does not fix the upstream cause. The next
timeout report must show the state before a fix is chosen.
- This is a Sentry run-failure report change. It is not a Telemetry
change. The new value is a fixed enum. It does not contain provider IDs,
causes, URLs, or paths.

> 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 edit, test runs). Context window and
reasoning configuration are set 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
- [ ] 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: Devin Foley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 21:22:39 -07:00
Devin FoleyandPaperclip 9befc66a44 Retain bounded Daytona acquisition failure observations (#15736)
## Thinking Path

> - Paperclip manages AI-agent work and reports failed runs.
> - Some runs acquire a Daytona sandbox before model execution.
> - Acquisition has one budget for creation, setup, and inline cleanup.
> - A timeout currently carries cleanup ownership but loses the step
that failed.
> - This pull request carries a bounded observation through the existing
error envelope.
> - Operators can identify the first failed step without exposing
provider responses or changing recovery.

## Linked Issues or Issue Description

**What happened?**

An acquisition timeout reports pending allocation cleanup without the
first failed step. The provider already knows its current step, but that
evidence is discarded at the worker boundary. A setup failure followed
by slow cleanup is also indistinguishable from slow creation.

**Expected behavior**

The immediate setup-failure report should contain a fixed phase and
bounded elapsed/budget values. Cleanup ownership, original messages,
deadlines, retry policy, and task behavior should remain unchanged.

**Steps to reproduce**

Use the provider regression fixtures to stall creation, workspace setup,
shell detection, expiry configuration, or sentinel upload. Also fail
setup early and let cleanup consume the remaining budget. The previous
source emits no phase diagnostic.

**Paperclip version or commit**

Base commit: `98e3cec78e8461774022a5ba0a777b09d4d16b96`.

**Deployment mode**

Hosted and self-hosted installations that use the Daytona sandbox
provider and enable Sentry.

Related public changes: #13979 established failed-allocation cleanup
ownership; #14097 preserves the provider acquisition budget; #15637
carries bounded workspace-transfer evidence. Duplicate searches found no
existing acquisition-phase diagnostic patch.

## What Changed

- Capture the first acquisition failure before inline cleanup. Keep the
observation if cleanup later reaches the deadline.
- Add an optional, validated observation to the existing
creation-cleanup envelope for acquire calls. Old ownership envelopes
remain valid. Malformed observations do not alter cleanup.
- Bind immediate reporting to an actual RPC error with matching company,
environment, and run ownership. Project only three bounded fields from
private host evidence.
- Add provider, SDK, actual worker/client, PostgreSQL cleanup,
collector, and real-Sentry regressions. Document the evidence limits.

## Verification

- Passed 404 focused tests across the SDK cleanup envelope, complete
Daytona provider suite, diagnostic collector, actual built worker/client
subprocess, and mandatory real-Sentry SDK suite.
- Passed 10 focused PostgreSQL cleanup-boundary cases. The filter
excluded 152 unrelated cases.
- Passed `pnpm -r typecheck` and `pnpm build` on the committed source.
- Full `pnpm test:run` is running. Final outcomes will be recorded
before merge.
- Independent source review passed. Replacing the provider with the
original source fails all 8 new phase/timing assertions. Replacing the
host boundary with the original source fails all 3 owned diagnostic
cases while 7 negative cases remain clean.
- CI passed: 54 successful checks and 2 expected Storybook skips. This
includes all server, workspace, serialized-server, Runner, browser,
build, canary, and real-Sentry SDK lanes.
- Greptile scored the committed source 5/5 with no review threads.
- No live provider operation, task replay, runtime configuration change,
or deployment was performed.

## Risks

Low risk: this only adds optional diagnostic evidence. The first
observed failing phase does not prove an upstream cause. Elapsed time
ends at that observation, not at completion of later cleanup. Evidence
is process-local and accompanies immediate failure reporting;
post-restart reports cannot reconstruct it. No historical run or lease
is backfilled. The wire schema remains v1 and cleanup ownership parsing
is unchanged.

## Model Used

OpenAI Codex, GPT-6, with repository inspection, code editing, local
execution, and independent agent review. The exact model snapshot and
context-window size are not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-09 14:25:14 -07:00
Devin FoleyandPaperclip 7333e4eba8 Capture bounded workspace base-ref failure diagnostics (#15719)
## Thinking Path

> - Paperclip manages AI agents and their work.
> - A run can prepare a Git worktree before an adapter starts.
> - That preparation must resolve the configured base ref.
> - The same unresolved-ref error can follow a missing ref, failed
fetch, or failed credential lookup.
> - Current failure reports do not distinguish these paths.
> - This PR records bounded outcomes from the operations that already
run.
> - Maintainers can use this evidence without changing task recovery or
hiding failures.

## Linked Issues or Issue Description

Related: #11892 established remote-only base-ref resolution and
pre-adapter recovery. I found no open PR for these diagnostics.

**What happened?**

A fresh worktree can fail with `UnresolvedWorkspaceBaseRefError`. Its
failure report has no closed fields that distinguish remote lookup,
credential lookup, fetch, and local ref-probe outcomes. An empty warning
list does not prove that a fetch ran or that a remote ref is absent.

**Expected behavior**

Keep the task-visible failure and owner recovery action. Report bounded
observations from the existing operations. Keep unknown causes
reportable.

**Steps to reproduce**

Use a local Git repository with a configured base branch that is absent.
Repeat with no origin, an inaccessible local origin, or a rejected
credential resolver. Start fresh workspace preparation. Before this
change, each path lacks the new typed observations in the failure
report. Tests use local repositories and intercepted fetch commands;
they require no external Git service.

**Paperclip version or commit**

Reproduced against source commit `1602512820`, built from source. This
affects core workspace preparation before adapter dispatch.

## What Changed

- Record closed remote lookup, credential lookup, fetch, and local
ref-probe outcomes. Limit new output parsing to 8 KiB.
- Attach evidence privately to the resolver's own error. Reject copied,
forged, inherited, accessor, and malformed evidence.
- Persist allowed values in the existing setup result and export them
through run-failure diagnostics.
- Preserve Git commands, credential fallback, warning text, recovery
actions, and Sentry reporting.
- Add real Git, heartbeat, reporter, and real Sentry envelope tests.
Document the fields and their limits.

## Verification

- Current head: `015b2737df004e971eeeb134ebc9a1024e244494`, based on
`be1bc5d494d`. The only rebase conflict was in heartbeat test imports.
All three imports were retained; `git range-diff` confirms no other
patch change.
- Full `pnpm -r typecheck` and `pnpm build` passed on the current head.
- Fresh focused tests passed: 13 real-Git producer cases, 279 complete
collector/reporter cases, two actual heartbeat recovery cases, and all
nine real Sentry SDK cases. The required-SDK gate used version 10.71.0.
Actual envelopes contain only allowed fields, with no private fixture
text or cross-event leakage.
- Independent review passed all 16 combined heartbeat cases from both
integrated changes and five real-Git probes with identical baseline
command sequences and outcomes. The original producer negative control
failed on the absent receipt; the candidate passed. No review findings
remain.
- Current-head CI is complete: 54 successful checks and two intentional
Storybook skips. Greptile is 5/5 on this exact head, with no unresolved
threads or merge conflicts.
- Local aggregate limitation: the earlier full run reported one failure
in an unchanged tool-access case and was stopped with SIGINT (exit 130)
to isolate it. Vitest did not print the deferred assertion summary. The
unchanged complete file passed 420/420 alone. That aggregate is not
claimed as a pass. Before the mechanical rebase, the complete
workspace-runtime file also passed 172/172. The fresh full CI result
covers the integrated head.
- An initial independent combined reporter run hit its database setup
timeout; the unchanged complete reporter file then passed 186/186 alone.
Earlier-head CI lost four runners and recovered on one verification
retry; current-head CI passed separately.

## Risks

- These observations do not prove that a remote ref is absent. All
affected failures remain reportable, including resumed and
provider-started runs.
- The new fields include no repository URL, ref text, customer path, raw
command output, or credential. Existing task-visible details are
unchanged.
- No migration, dependency, authorization, retry, or deployment change.

## Model Used

OpenAI GPT-6 through Codex, with tool execution and parallel independent
review. The exact serving variant and context-window size are not
exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-09 12:50:03 -07:00
Devin FoleyandPaperclip 1c4ce44e13 fix: retain bounded workspace transfer failure evidence (#15637)
## Thinking Path

> - Paperclip manages AI agents and the work they produce.
> - Sandbox work must return to the host when an agent run ends.
> - A failed transfer must keep its source available for recovery.
> - Some transfer errors lose their useful fields before restore
diagnostics reach Sentry.
> - This change records fixed transfer stages, failure kinds, and
numeric RPC codes.
> - The next failure can identify the operation that failed without
exposing workspace contents.

## Linked Issues or Issue Description

**What happened?**

A workspace transfer can fail after model execution succeeds. Several
command, archive validation, and download errors become plain errors.
The existing RPC envelope then reports `unknown`, with no transfer stage
or command exit code. A host RPC timeout also loses its code from
restore diagnostics.

**Expected behavior**

Keep bounded evidence through the provider, worker RPC, restore result,
and Sentry context. Keep the same exception, error message, retry rules,
and source retention policy. A transfer timeout must remain separate
from the model execution timeout flag.

**Steps to reproduce**

Return a nonzero exit code from the sandbox archive command, return a
per-file download error, exceed a tar listing limit, or time out the
sync-out RPC. Observe the missing fields in the saved restore
diagnostic. The added tests use local fixtures for these cases.

Related public work: #15479 preserves the source after restore failure.
#15481 carries bounded sync-out diagnostics through RPC. This change
supplies missing producer evidence and extends that same envelope. I
checked the roadmap and searched open PRs for duplicate transfer
diagnostic work.

## What Changed

- Add optional transfer stage and failure kind fields to the existing
diagnostic envelope. Capture command exits and listing deadlines at
their producers.
- Record only known codes from typed RPC errors at the sync-out
boundary. Revalidate every field before persistence and Sentry
projection.
- Keep annotations private to each outbound request and restore
settlement. Preserve frozen error identity and prevent evidence from
leaking across concurrent or later calls.
- Document the fields. Cover producer failures, quota limits, old
workers, concurrent error reuse, and redaction through the real Sentry
SDK.

## Verification

- Focused SDK, provider, host, persistence, and real Sentry tests: 449
passed, no skips. Independent review also ran the focused contracts and
all provider tests. The optional Sentry SDK is required for this check,
so the contract tests cannot skip.
- A compiled Daytona transfer and compiled SDK worker pass a local RPC
round trip. This uses the development TypeScript loader for workspace
dependency exports. The transfer and capture modules are compiled
JavaScript.
- `pnpm -r typecheck` and `pnpm build` passed. The excluded Daytona
package also passed `tsc --noEmit`.
- The initial local `pnpm test:run` stopped in the general-server group:
60 suites could not initialize embedded PostgreSQL because the isolated
install skipped its Darwin library-link setup. The existing package
postinstall repairs this; `initdb --version` now passes. All 60 affected
suites then passed: 1,342 tests, no skips. No local full aggregate pass
is claimed.
- Independent source review covers privacy, concurrency, packaging, and
recovery behavior.

## Risks

- These diagnostics help identify future failures. They do not establish
or repair the cause of a past transfer failure.
- New fields are optional. Older workers remain compatible. Unknown
values are omitted.
- The change adds a small request-local diagnostic scope. It keeps the
original thrown errors, archive confinement, cleanup order, timeout
values, retry count, and source retention policy.
- No schema or deployment changes are required. No live provider actions
are part of validation.

## Model Used

OpenAI Codex, based on GPT-6, with repository inspection, code
execution, and independent agent review. The exact deployed model ID and
context-window size are not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] 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>
2026-10-08 19:27:39 -07:00
Devin FoleyandPaperclip 5717523b9e fix: retain the original repository for reused task workspaces (#15528)
Apply the reviewed change for fix: retain the original repository for reused task workspaces.

Validation: required local typecheck and tests, passing CI, Greptile 5/5, and independent review of the exact source head.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-07 19:52:30 -07:00
Devin FoleyandPaperclip 1d19f9b562 Distinguish failed Git inspection from missing worktree registration (#15525)
Apply the reviewed change for Distinguish failed Git inspection from missing worktree registration.

Validation: required local typecheck and tests, passing CI, Greptile 5/5, and independent review of the exact source head.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-07 19:05:04 -07:00
Devin FoleyandPaperclip 128c95837b Retain bounded orphan process-loss diagnostics (#15475)
Record bounded observer uptime, run and output ages, process-check observations and retry eligibility before orphan cleanup changes the evidence. Preserve existing recovery and reporting behavior and omit process IDs, raw paths and credentials.

Verified focused helper, actual reaper and real SDK regressions, full typecheck/build, exact-head CI and independent review. An unchanged Cursor timeout passed its isolated retry; unrelated local Slack timing failure is documented separately. Greptile 5/5 with no unresolved comments.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-07 12:05:21 -07:00
Devin FoleyandPaperclip 1d5a439e2b Diagnose native model rejections and preserve repairable reviews (#15304)
## Thinking Path

> - Paperclip manages agent execution and recovery.
> - A native provider can return a failed turn with an accepted result.
> - This path replaced a structured model rejection with a generic
failure.
> - Finalization could also queue the same failed request for recovery.
> - This change retains a bounded model and authentication diagnosis.
> - Workers wait for a configuration fix; reviews retain their
assignment.

## Linked Issues or Issue Description

**Describe the bug**

A native Codex turn can report HTTP 400 because the selected model is
not supported by its ChatGPT connection. When the turn also returns a
failed semantic result, the run shows `Native session failed` and
`adapter_failed`. The native failed-result finalizer can create a retry
intent before the adapter error is saved.

**To Reproduce**

Use a native Codex session that returns a structured
`invalid_request_error` model/ChatGPT rejection and an accepted failed
result. The regression tests reproduce this with a stub provider and a
real isolated PostgreSQL database. No provider account or live request
is needed.

**Expected behavior**

Show an actionable error and stop automatic retries of the rejected
request. Block the current worker for a configuration fix. Preserve a
pending review so its reviewer can explicitly retry after repair and
resolve the original assignment. A stale run must not block a new task
owner or override a completed review.

**Version and deployment mode**

The affected source path is present at `858094ba81`. This change applies
to native Codex execution in local and remote environments.

Related work: #13853 covers CLI protocol compatibility, #14942 refreshes
runtime/model pins, and #12767 proposes legacy adapter model validation.
This change concerns accepted native failed results and does not replace
those changes.

## What Changed

- Recognize only a bounded structured HTTP 400 provider error for the
exact selected model and failed turn.
- Save a fixed actionable error and four fixed diagnostic fields.
Exclude raw provider text and model names from the new diagnostic.
- Re-derive finalization evidence from the pinned execution and
committed runner event, including its canonical hash and identity
bindings.
- Block only the current worker. For a current pending reviewer,
preserve the original review decision and status version, release
execution, and create no automatic recovery action. Recheck execution
ownership under the issue lock. Preserve superseded and completed
reviews.
- Keep unknown failures, accepted results, cleanup, and ownership fences
intact. Advance the status policy version to `phase6-v8`.
- Route this exact failure code through the existing configuration
recovery path. Permit an authenticated Board retry of this native
failure only for its saved, still-pending review; discard caller review
markers and revalidate at dispatch.

## Verification

- Node 24: `pnpm -r typecheck` and `pnpm build` passed.
- Independent focused validation: 222 tests passed across the new
observer and PostgreSQL integration suites, status arbiter, diagnostic
redaction, recovery classification, wake policy, and wake use cases.
- The integration tests use actual result persistence, finalization, and
the wake dispatcher. They cover zero retry dispatch for an authorized
rejection, repeated reconciliation, pending/superseded/resolved reviews,
reassignment, and a same-agent successor at an unchanged status version.
- Final-commit verification: full typecheck and build passed;
independent route/integration/arbiter/review validation passed 136 tests
with no remaining source findings. This includes authenticated retry
refusals, actual heartbeat retry admission, atomic queued-review claim,
and acceptance of the original review.
- The local aggregate test run was stopped to avoid duplicating the
canonical Linux CI aggregate on this development host. Its partial log
showed no failures; no full local-suite pass is claimed. The full hosted
Linux CI aggregate passed on the final commit.
- No live provider calls, runtime changes, or schema changes were made.

## Risks

The classifier is intentionally narrow. A changed provider response
format remains an unknown failure. No global model restriction is
introduced. The worker blocker requires current task authority and does
not replace newer assignments or completed review decisions. A pending
review remains in review and requires an explicit retry after
configuration repair; it is not rebound to a new status decision.

The retry decision is durable. There remains a small existing gap
between status/effect commit and diagnostic metadata persistence; a
crash there can leave a generic diagnostic while preserving the blocked
decision. No schema migration is required.

## Model Used

OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The
exact serving model suffix and context-window size are not exposed in
this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#123` / `Refs #123` 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 references)
- [x] My branch name describes the change and contains no internal
ticket id or instance-derived details
- [x] I have run focused 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>
2026-10-05 20:57:33 -07:00
Devin FoleyandPaperclip efac8ff2f4 Add bounded Git integration restore diagnostics (#15291)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Sandbox runs must restore their workspace before finalization can
succeed.
> - Restore diagnostics identify the failed phase and operation.
> - A Git integration exit code can still describe several different
failures.
> - This pull request adds fixed command labels and supported failure
classes.
> - Operators can distinguish these failures without collecting private
Git output.

## Linked Issues or Issue Description

Refs #15005. This branch includes merged #15268. It labels that change's
locked ref transaction and nested branch probe without changing their
behavior.

**What happened?**

A failed Git integration can report only `git_integration`, `unknown`,
and an exit code. That evidence does not identify the failed command.
Some Git versions also return exit 1 for both a merge conflict and an
invalid object.

**Expected behavior**

Record a fixed command family and a supported failure class. Keep
unknown cases as `unknown`. Exclude command arguments, process output,
paths, repository URLs, filenames, and ref names.

**Steps to reproduce**

The tests create local repositories with conflicting commits, a missing
object, and an expected-old ref mismatch. They call the real Git
operations and inspect the resulting diagnostic. No hosted workspace or
external provider is used.

**Paperclip version or commit**

Base: `e99854249c`.

**Deployment mode**

Built from source. The diagnostic applies to sandbox workspace restore.

## What Changed

- Label Git integration calls with a closed command enum. Preserve
arguments, options, errors, and retry behavior.
- Recognize supported object and ref errors and OS permission codes.
Require both exit 1 and completed tree output for a merge conflict.
- Carry the closed fields through the existing restore receipt, saved
adapter result, and Sentry projection. Revalidate saved metadata before
projection.
- Test nested wrappers, parallel failures, reused errors, handled
probes, result settlement, and privacy with the real Sentry SDK.
- Document the field contract and its limits.

## Verification

- Eight focused suites pass: 295 tests. These cover Git sync, restore
diagnostics, result settlement, teardown, sandbox runtime, failure
projection, the real Sentry SDK, and native warm-workspace Git history.
- `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1 pnpm exec vitest run
packages/adapter-utils/src/workspace-restore-diagnostics.test.ts
packages/adapter-utils/src/workspace-restore-result.test.ts
packages/adapter-utils/src/git-workspace-sync.test.ts
packages/adapter-utils/src/workspace-restore-teardown.test.ts
packages/adapter-utils/src/sandbox-managed-runtime.test.ts
server/src/services/__tests__/run-failure-diagnostics.test.ts
server/src/__tests__/run-failure-sentry-real-sdk.test.ts
server/src/__tests__/native-workspace-sync-history.test.ts`
- Local checks use Node 24.21.0 and the pinned pnpm 9.15.4. The optional
Sentry SDK is pinned to the declared 10.71.0 and uses an in-memory
transport.
- `pnpm -r typecheck` and `pnpm build`: pass on the rebased head.
- The full local `pnpm test:run` was stopped before source changes when
#15268 merged and required a rebase. It reported three pre-existing
company-skill cache test failures on macOS. The same three cases fail on
clean bases `2c43b39167` and `e99854249c` and the earlier head with
`EACCES` when publishing a read-only cache directory. The relevant test,
service, and cache source blobs are identical. No cache changes are
included. Later local test groups were not reached.
- All 54 checks pass on exact head `8448896bc6`, including the
post-ready security scan, with two intentional Storybook skips. Greptile
scores this head 5/5 with no review threads. Full Linux CI covers the
local groups that were not reached.
- Independent review found no blocker. Its diagnostics, Git workspace,
and native history suites pass 115/115 on this head.
- A source comparison confirms that removing only diagnostic wrappers
yields the merged upstream Git integration code exactly, including ref
locks, transaction protocol, arguments, options, and retry decisions.
- The clean base has stale dependency overrides in its lockfile. Local
installation resolved them as the existing PR CI fallback does. No
manifest or lockfile change is included.
- `git diff --check` and a local secrets/PII review pass.

## Risks

A Git version or localized message may not match a known failure form.
Such cases remain `unknown`. Up to 16 KiB of stderr and the bounded
tree-ID prefix of stdout are inspected only in memory. The saved fields
contain enum values only. These diagnostics do not establish workspace
recovery or authorize retries. Restore decisions and Git mutations are
unchanged. No schema migration is required.

## Model Used

OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The
service does not expose the exact model deployment ID or context-window
size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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>
2026-10-05 17:10:05 -07:00
Devin FoleyandPaperclip 3b47a6befd Record bounded workspace restore failure stages (#15005)
Capture allowlisted restore substep and failure metadata for future diagnosis while preserving workspace recovery behavior, error identity, cleanup ordering, and privacy.

Validation: exact-head Greptile 5/5, passing CI, no unresolved review threads, and a clean merge. Detailed verification and limitations are recorded in the pull request.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-05 10:58:32 -07:00
DottaandPaperclip 7a52dcdc74 fix: repair MCP validation and cancelled execution recovery (#14951)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The tool gateway gives agents access to connected services. Recovery
controls what happens when a run stops.
> - Generated tool names can exceed the provider limit after the MCP
client adds its prefix.
> - The same invalid definition can fail each automatic retry. A
cancelled run can also hold saved messages without showing its cause.
> - This pull request bounds tool names, stops configuration retries,
and retains cancellation evidence.
> - It shows the stopped run and admits saved input only after the
existing safety checks pass.
> - The benefit is a clear recovery path that preserves operator Stop
and prevents duplicate message delivery.

## Linked Issues or Issue Description

**What happened?**

A long connected MCP tool name makes the provider reject the entire
request. Automatic recovery repeats the invalid request. Separately,
unexpected legacy cancellations can leave saved input behind a recovery
hold. The notice does not identify the stopped run or its cause.

**Expected behavior**

Complete MCP names fit the provider limit. Tool-definition errors
require configuration repair. Cancelled runs retain their source and
reason. The recovery notice shows the cause and saved-message count.
Verified unexpected cancellations can start a fresh turn through the
existing admission checks.

**Steps to reproduce**

1. Assign an App gallery connection with a long application key and tool
name to a Claude agent.
2. Start a run. The provider rejects a name over 128 characters,
including its MCP prefix.
3. For cancellation recovery, stop a legacy provider turn without an
operator Stop request and send a user message while the recovery hold is
active.
4. Inspect the recovery notice and the deferred message queue.

**Paperclip version or commit**

Rebased onto master at `cf8ad63c806685bfd7c48e3ed4a919d61a7c55f1`.

**Deployment mode**

Hosted or self-hosted server with legacy Claude or Codex execution.

Related public work:

- Refs #14017. That PR caps name segments. This PR preserves existing
short names and uses stable hash aliases for long complete names. It
also covers classification and recovery.
- Refs #4510. That PR adds a cancellation-source column. This PR records
bounded evidence in the existing run result, without a migration.
- Refs #12552 and #4506. Those PRs suppress recovery after operator
cancellation. This PR preserves operator intent and uses the existing
continuation gates.

## What Changed

- Bound gateway names with the full provider prefix in the 128-character
budget. Retain the original upstream tool name for dispatch and
permissions.
- Classify invalid tool definitions as configuration failures before
diagnostic redaction. Stop automatic retries and continuation attempts
for that error code.
- Persist cancellation source, expectedness, initiator, reason, and
time. Preserve recorded Stop intent when adapter results arrive. Report
unexpected started cancellations with closed diagnostic labels.
- Show the run cause, saved-message count, and Inspect run link. Offer
Continue for eligible unexpected cancellations. Require verified
provider stop, empty tool inventory, ownership, and the existing pause,
budget, approval, and dependency gates. Use the existing queue for
single delivery.
- Add regression coverage and update the execution, MCP gateway, and
run-log documentation.

## Verification

- `pnpm -r typecheck` and `pnpm build` passed.
- `pnpm check:token-gates` passed.
- Ran `pnpm test:run` and completed its workspace and serialized groups.
Initial resource and timing failures passed on isolated reruns. All 149
serialized route suites passed.
- Reran the changed server, adapter, and UI suites after the rebase.
Coverage includes long-name upstream dispatch, configuration retry
suppression, cancellation evidence retention, privacy labels, oversized
run projection, and concurrent saved-message delivery.
- `pnpm test:e2e tests/e2e/legacy-failure-continuation.spec.ts` passed
all six browser scenarios. The recovery notice shows the run cause and
inspection link, and each recovery entry point reaches one new response.
- Added database-backed checks for active, removed, paused, unavailable,
and disabled chat connections. The final continuation and
recovery-notice suites passed 167 tests. Externally bound chats hide
board Continue and show a usable next action.
- All 55 GitHub checks passed on
`42afbf1371dcaeb72646e3d8f65c19ff7cddf8de`. Two unrelated Storybook jobs
were skipped by their normal conditions. Greptile reviewed that commit
at 5/5 with no findings and no open review threads.

## Risks

- Long tool names change to aliases. Existing short names stay
compatible. The original connection and upstream name remain the
dispatch authority.
- Invalid tool definitions no longer get automatic retries. An operator
must repair the configuration before a new attempt.
- Continuation changes apply only to positively identified unexpected
legacy cancellations with complete empty tool inventory. Operator Stop,
unknown historical cancellations, outstanding tools, and unverified
provider termination keep their holds.
- No database migration. The added projection fields are optional.
Cancellation reason and initiator IDs remain local run evidence; Sentry
receives only closed source and initiator-type labels and expectedness.

## Model Used

- OpenAI GPT-6 through Codex, with reasoning, repository editing, shell
execution, and GitHub tool use. The runtime does not expose the exact
model variant or context-window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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>
2026-10-02 13:47:59 -05:00
Devin FoleyandPaperclip f2e0f19630 Defer agent directory cleanup until stop proof is available (#14866)
## Thinking Path

> - Paperclip manages agents and their persistent files.
> - Each run owns a temporary agent directory and a save receipt.
> - Cleanup needs independent proof that the owning process stopped.
> - A cleanup call without that proof currently waits for the directory
lock anyway.
> - A second lock failure can prevent environment release after the run
already reported a failed save.
> - This change skips cleanup that has no authority and retries
unavailable remote copies after exact destruction proof.
> - The save failure stays visible. Existing lock owners remain
protected.

## Linked Issues or Issue Description

Related work: Refs #14787 (lock diagnostics), #14695 (warm instruction
ownership), #9667 (stale lock proposal), and #9872 (control-plane
ownership proposal). I checked open PRs and issues. This change leaves
the shared filesystem lock protocol in place and does not duplicate the
warm-retention work in #14695.

**What happened?**

Heartbeat cleanup records an explicit unavailable instruction-save
warning, then calls directory release before releasing the environment
lease. Release can wait for a lock even though the copy has no
process-stop proof and cannot be removed. That secondary timeout
prevents the following lease-release step. If destruction proof arrives
later, the unavailable copy is excluded from both recovery queries.

**Expected behavior**

Skip a release that cannot remove anything. Preserve the failed-save
receipt and candidate fields. Once exact remote destruction is recorded,
recover remote cleanup without running a provider command. Unavailable
local copies retain their potentially uncollected edits even if local
stop proof arrives later. A blocked cleanup must not prevent cleanup for
other agents.

**Steps to reproduce**

1. Prepare an agent directory, report its save unavailable, and leave
process-stop proof absent.
2. Hold the shared directory lock and call release. Before this change,
release waits and fails although removal is not authorized.
3. Record destruction of the copy's exact remote lease. Before this
change, neither recovery sweep selects the unavailable copy.

**Paperclip version or commit**

Reproduced against `efc2e6810e9bc0dc8cb412b0e7647c0db9821caa`.

**Deployment mode**

Local and remote execution with persistent agent directories. Tests use
an isolated embedded PostgreSQL database and fixture transports.

## What Changed

- Re-read receipts and skip release before lock acquisition when stop
proof is absent, the copy is superseded, or cleanup is complete. Keep
the same checks inside the lock.
- Recover unavailable remote copies only after exact destruction proof.
Preserve their unavailable state, errors, candidate hash, and candidate
bytes. Keep unavailable local copies and their uncollected edits
unchanged.
- Store destruction-only cleanup authority with the stop proof. Later
cleanup honors it after a lost database response or restart, including
when a transport remains cached.
- Defer failed or unproven cleanup with bounded batches and a retry
delay. Keep failed cleanup visible in logs and its receipt.
- Serialize preparation of an existing run with cleanup. Fresh run
preparation keeps its existing admission path.
- Cover held locks, receipt scope, delayed proof, batch fairness, lost
update responses, cached transports, and concurrent same-run preparation
with database regressions.

## Verification

- Focused directory, legacy instruction-copy, shared lock, and bounded
diagnostic suites: 169 tests passed across four files.
- `pnpm -r typecheck`: passed on the final source.
- `pnpm build`: passed on the final source.
- Completed all selected local `pnpm test:run` groups: 733 general
server suites, 149 serialized suites, and 14 workspace projects. There
are 13 known macOS `EACCES` failures in the unchanged runtime skill
cache tests. Their exact signatures match earlier clean-base results,
and the cache source and test blobs match both that base and this PR
base (existing fix: #14290). One CLI import test timed out under
concurrent load; its full file passed separately (17 tests). Broad
coverage began before the review corrections; the final source has the
focused 169-test run, typecheck, and build. This is a local verification
limit, not a passing full local suite.
- `git diff --check` and local Gitleaks plus private-identifier/PII diff
scans passed.
- Independent review of the final source found no remaining actionable
issue. Its 17 targeted tests cover crash recovery, cached transports,
same-run preparation, real local edit preservation, proof scope, and
batch fairness. The main focused run also covers contained scheduling
failures.
- Final commit `35a24085f7`: Greptile 5/5 with no recommendations and
zero unresolved review threads.
- Final commit `35a24085f7`: all 53 checks passed, including Canary Dry
Run and the security scan; two visual checks were intentionally skipped.
The workspace shard passed on retry after GitHub reported that its first
runner lost communication. An earlier Canary runner shut down after the
release dry run passed. Neither interruption recorded an application
assertion failure; the exact final-head checks are now green.

## Risks

- This repairs cleanup ordering and recovery eligibility. It does not
repair an ambiguous legacy lock owner or restore unsaved files. Actual
collection still fails visibly when its lock cannot be acquired.
- An unavailable remote copy is recovered only after exact destruction
proof. A stopped but retained environment stays protected; recovery does
not execute a command that could restart it.
- Unavailable local copies with later stop proof still retain
potentially uncollected edits. A general local recollection or
reclamation policy remains outside this change.
- Existing-run preparation now waits for the same lock as cleanup. The
fresh-run path is unchanged.
- The cleanup mode is stored in the existing private receipt JSON. No
schema migration or public API change is required.
- No deployment, task replay, or runtime lock deletion was performed.

## Model Used

OpenAI GPT-6 (Codex), with reasoning, repository tools, and test
execution. The runtime does not expose a more specific model suffix or
context-window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` 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 references)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id
- [ ] 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>
2026-10-01 14:28:47 -07:00
Devin FoleyandPaperclip 4b9a6000f7 Add bounded evidence for directory lock timeouts (#14787)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agent files use directory locks during collection and cleanup.
> - A lock timeout can fail finalization after the model turn completes.
> - The timeout currently identifies no owner state or waiting
operation.
> - This pull request adds bounded evidence to the existing run failure
report.
> - Operators can distinguish a known local holder from a possible old
lock without changing lock safety.

## Linked Issues or Issue Description

**What happened?**

A directory lock timeout does not distinguish active local work from an
owner record left by an earlier process. The stored execution stage can
also precede the cleanup operation that failed.

**Expected behavior**

The failure report should identify the waiting operation and expose
bounded ownership clues. It must preserve the timeout and keep unknown
ownership protected.

**Steps to reproduce**

Hold a directory merge lock while a second caller reaches its
acquisition deadline. The regression tests exercise a live holder and an
older owner record with a live PID.

Related: #9667 proposes stale-lock recovery under a single-server
assumption. This change only adds evidence and does not adopt that
assumption. #14575 and #14665 add other run failure diagnostics.

## What Changed

- Record lock owner state, capped age and wait duration, same-process
and process-age comparisons, and whether this module holds the lock.
- Label agent-directory release, collection, checkpoint, and warm
handoff timeouts with a fixed operation code.
- Validate each field before the existing event-local Sentry report
accepts it. Exclude owner records, PIDs, paths, and absolute timestamps.
- Limit the extra diagnostic owner read to 100 ms with best-effort
abort; malformed JSON is `invalid` and unreadable owner records remain
`unknown`.
- Document the diagnostic limits and verify that contenders never
reclaim protected locks.

## Verification

- Focused lock, diagnostic, real Sentry SDK, and database-backed
agent-directory tests: 126 passed, including stalled-read and
malformed/missing/unreadable-owner regression coverage.
- Final revision `0691613dcc`: all 54 reported checks successful, with
two intentionally skipped Storybook checks. Greptile: 5/5, zero
unresolved review threads; no merge conflicts.
- `pnpm -r typecheck`: passed.
- `pnpm build`: passed.
- `pnpm test:run`: complete suite coverage ran with the existing
repository shard flags: four general-server shards, four serialized
shards, two general-workspaces-a shards, and general-workspaces-b. The
full run is not green because of the base failures below.
- The broad run found 13 failures in the unchanged macOS skill-cache
tests. All 13 reproduce on the clean base revision. Open PR #14290
covers that existing failure.
- Two unchanged CLI archive tests hit their five-second limits during
the broad run; all 17 tests in that file pass on recheck. A CLI auth
socket error also cleared on recheck (19 tests), and its full serialized
shard passed on rerun.

## Risks

This is a diagnostic change, not a stale-lock fix. Owner observations
can race with release. Wall-clock shifts can affect the age comparison.
A local-holder flag covers only this module instance. None of these
fields authorizes reclamation or proves a file save. Lock acquisition,
release, retries, task status, and recovery guards retain their current
behavior. No schema change or deployment action is required.

## Model Used

OpenAI Codex, based on GPT-6, with code execution and repository tools.
The exact model build and context window were not exposed to this agent.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [ ] I have run tests locally and they pass (focused checks pass;
existing base failures are documented above)
- [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>
2026-09-30 18:35:12 -07:00
Devin FoleyandPaperclip 0ea6b10967 Record ACP activity and workspace restore failure evidence (#14665)
## Thinking Path

Paperclip records terminal run failures for operators. A timeout's own
log and cleanup output update the run's last-output timestamp, so that
timestamp can make a long-silent provider look active. Snapshot runtime
activity before finalization and include the saved workspace restore
classification to make the next failure actionable without copying tool
payloads.

## Linked Issues or Issue Description

**What existing behavior does this improve?**
Terminal run diagnostics in the existing opt-in Sentry integration.

**Current behavior**
Reports cannot distinguish runtime events from finalization logging and
omit the already-persisted workspace restore code. A later successful
run also does not establish that earlier workspace files were restored.

**Proposed behavior**
Record runtime-event age/count and pending-tool inventory at
finalization, before status reads and cleanup. Forward only finite
counts, a completeness boolean, and known restore codes through the
existing reporter.

**Reason and benefit**
Operators can distinguish a silent turn with unfinished tools from
recent runtime activity and see restore failures without retrieving
private run output. Neither signal certifies productive work or
successful recovery.

**Breaking changes**
None. Error grouping, execution deadlines, cancellation, recovery
policy, and the Sentry opt-in remain unchanged.

Related diagnostic work: #14573, #14575, #14639.

## What Changed

- Snapshot ACP activity before success/failure finalization, including
thrown relay failures.
- Forward bounded numeric/boolean evidence and shared workspace restore
codes; exclude commands, tool IDs, paths, and arbitrary result data.
- Document limitations and test silence, empty streams, timeout, cleanup
delay, incomplete tool inventory, and privacy.

## Verification

- `pnpm -r typecheck` passed after the final implementation.
- Changed suites: 252 tests passed; all 29 database reporter tests
subsequently passed after restoring the embedded-Postgres package
library symlinks. The migration test also passed (30 database cases
total).
- `pnpm build` passed during implementation. Final head
`fe78dba6f592b1abccac7cdbf341bd2e0b0d30cb` passed all 53 CI checks,
including complete test coverage, typecheck, build, and browser/runner
gates; two unrelated checks intentionally skipped.
- Full local test attempts initially hit missing embedded-Postgres
library symlinks; the dependency setup was repaired and database tests
passed. Duplicate local full-suite runs were stopped after full CI
completed. This PR does not claim a completed full local suite.
- Review regression: completed, failed, and cancelled tools are excluded
from the pending count; focused activity/timeout tests and adapter-utils
typecheck passed.
- Reviewed the diff for secrets, customer data, and internal references.

## Risks

Low risk, diagnostic-only. The existing tool inventory is incomplete for
some runtime events, so the report carries its completeness flag. Event
age is measured at finalization and does not prove useful work or
identify the underlying provider failure. No schema changes or new
capture gate.

## Model Used

OpenAI GPT-6 via Codex, with repository inspection, code execution, and
tests. Exact model build identifier is not exposed by this session.

## Checklist

- [x] Thinking path and model are specified
- [x] Checked ROADMAP.md; this is a maintenance correction, not planned
feature work
- [x] Searched for duplicate and related PRs
- [x] Described the issue using the enhancement template
- [x] No internal issue references, customer data, or private instance
links
- [x] Descriptive branch name
- [x] Focused regression tests pass
- [x] Added tests and updated documentation
- [x] Risks documented
- [x] Required validation and CI gates are green (full suite validated
in CI; local scope documented above)
- [x] Greptile is 5/5 with no unresolved findings
- [x] I will address review comments before requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-09-30 14:27:53 -07:00
DottaandPaperclip b3eb03fcba fix(sentry): preserve run failure stacks and diagnostic context (#14585)
Builds on merged #14575 and targets `master`.

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Operators use optional Sentry reports to diagnose failed agent runs.
> - The shared reporter currently converts each saved error message into
a new exception.
> - This loses the original stack and cause chain, and omits available
adapter and provider diagnostics.
> - This pull request selects, redacts, and bounds useful diagnostics
before it sends them to Sentry.
> - Operators can locate the failing operation and correlate upstream
failures without copying arbitrary run data.

## Linked Issues or Issue Description

Refs #14573. That change preserves ACP provider failure details in the
run record.
Refs #14575. That merged PR adds recorded exit codes and signals. This
PR covers stacks, causes, and structured diagnostics without duplicating
those fields.

A thrown setup or adapter error currently appears in Sentry with the
reporter's stack. A saved provider failure can contain useful details
that never reach the Sentry event. Both cases use the same shared
reporting path.

## What Changed

- Pass caught setup and execution exceptions, and structured adapter
error metadata, through successful terminal status transitions.
- Snapshot a fixed set of execution, adapter, provider, and exception
fields. Preserve up to four exceptions in the cause chain.
- Remove registered secret values, declared runtime environment
credentials, unknown inherited environment values and encoded credential
forms, and credential patterns before truncation. Mark truncated fields
and bound provider details and stacks.
- Rebuild sanitized Sentry exceptions with the original stacks and
causes. Use an adapter stack preview when available. Omit a fabricated
reporting stack when the source has no stack.
- Keep contexts local to each event. Preserve the optional DSN gate and
existing error-code/adapter fingerprint.
- Document the fields, limits, and omitted data.
- Settle leftover chat fixture outbox rows only after assertions and
worker shutdown, so subsequent tests cannot claim earlier cases’ pending
actions or provider I/O. This fixes the CI shard contamination exposed
during verification; production chat behavior and test timeouts are
unchanged.

## Verification

- `pnpm -r typecheck` passed.
- `pnpm build` passed.
- 86 focused diagnostic, Sentry, and startup tests passed after the
rebase, including the real `@sentry/node@10.71.0` SDK with an in-memory
transport.
- Tests cover cause chains, HTTP status and request IDs, long provider
details, secret redaction, failed secret resolution, cyclic causes, size
limits, and context isolation.
- Five targeted heartbeat integration cases passed, including thrown and
returned errors containing an opaque environment-bound credential. The
earlier full heartbeat integration run also passed its integration
cases.
- The focused suite also passed with an unknown inherited environment
value set to `1`; reporting tests use a controlled environment and
separately verify short-secret redaction.
- Before the fixture cleanup, the full chat shard reproduced the CI
Telegram timeout at `pending recovery before restart` (354 passed, 1
failed). After the cleanup, the same shard passed all 355 tests. No
timeout or production behavior changed.
- Latest-head CI (`0ae9e70df321d66dda025c3a7ba4787e169e0a4f`) passed
typecheck, build, all 12 server shards, all 3 chat shards, workspace and
serialized suites, browser shards, Runner checks, canary validation, the
real Sentry SDK contract, and security checks. Required `ci / verify`
and `ci / e2e` passed.
- The branch has been rebased onto `master` after #14575 merged. All 55
applicable checks passed on this head (2 unrelated Storybook checks
skipped). Greptile re-reviewed the current 11-file diff at 5/5 with no
findings or unresolved review threads.
- Local monolithic full-suite attempts were interrupted to apply fixes;
full-suite success is not claimed from those runs.

## Risks

- Error messages and stacks can contain credentials. The reporter uses
existing redactors and the run's encrypted secret registry, reads only
known fields, and skips capture if registered-secret resolution fails.
- Unknown environment values remain private by default. Unrecognized
short values can mask benign matches; known public settings are
explicitly allowed.
- Diagnostic text is bounded and can be truncated. Truncation is
explicit. Data discarded upstream cannot be recovered.
- These additional fields go to the operator's configured Sentry
endpoint. Arbitrary request/response objects, headers, configuration,
prompts, and stdout/stderr are not copied.

## Model Used

OpenAI GPT-6 through Codex, with tool use and code execution. The exact
serving model ID, context window, and configured reasoning level are not
exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-09-29 12:54:12 -05:00