mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
db0d523bc84bb92428e4939389cbff166c613633
3765
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
db0d523bc8 |
fix(recovery): start a Retry after a restart cut off a relaunch whose earlier Runners were proven stopped (#20537)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a run fails, a person presses Retry and expects a new run to start > - Before a native Retry starts, Paperclip must prove that the previous run's Runner and provider stopped, so two providers never work on the same task > - When the server is killed after it asked for a Runner launch but before it recorded the Runner's process, there is no PID to check. Paperclip then proves the stop from two facts: no Runner process for the run exists on this host, and the run never saved Runner state > - A run that already crossed restarts did save Runner state, from its earlier Runners. Those Runners were proven stopped, with stop receipts. The proof still refused, because it only asked whether any state belonged to the run > - The Retry returned 202, but the task then waited for a board operator to confirm the stop (`confirm_run_stopped`, `process_identity_missing`), until the task timed out > - This pull request lets the proof accept Runner state that only the earlier, proven-stopped Runners could have written > - The benefit is that a Retry after a restart-interrupted relaunch starts right away, with no operator step ## Linked Issues or Issue Description **Subsystem affected** Server native-run stop proof for a Retry or a new message (`server/src/services/native-local-process-stop.ts`, `unidentifiedLocalRunnerProvenAbsent` in `server/src/services/native-runtime/native-session-executor.ts`). **Problem or motivation** Restart-chaos wave `w-20261110T111245-s701`, task t03 (local, codex, C1, K1/K2, `--storage s3 --storage-faults ST7`): run `f1ffeed3` crossed three restarts. Its event history is: 1. Launch request, process identity, stop receipt (first Runner, killed by K1) 2. Launch request, process identity, stop receipt (second Runner, killed by K1) 3. Launch request at 11:13:54.999, then a K1 kill before the Runner's identity was recorded 4. A fourth controller resumed the run, a K2 drain interrupted its preparation, and the run failed (`adapter_failed`, fixed separately by #20514) The person's Retry at 11:14:37 returned 202. Admission reached `recordUnidentifiedLocalRunnerStop`. No Runner process for the run existed, but the Runner state root held `control-plane-state.json` with this run's id, written by Runners 1 and 2. The proof refused, the issue was saved as `waiting_for_user` / `confirm_run_stopped` / `process_identity_missing`, and the wake stayed deferred until the task timed out about 15 minutes later. The harness graded `R3:retry_no_new_run`. **Proposed solution** A Runner persists its command journal (`runner/runner-state.json`) before any command takes effect, and a provider starts only from a command. Its `runner/process-lineage/` writer marker is written before any supervised process starts. So a launch whose Runner never wrote either file after the launch request cannot own a provider. - The launch request event now records `requestedAt`, taken on the launching host just before any backend may spawn. - `recordUnidentifiedLocalRunnerStop` passes that time on only when the latest process event before the request is a stop receipt (every earlier Runner of the run was proven stopped) or there is none. - `unidentifiedLocalRunnerProvenAbsent` then accepts saved state of this run only when `runner/runner-state.json` (if present) and every `runner/process-lineage/` entry were last modified before `requestedAt`. A file time that falls on a whole second (a coarse-timestamp filesystem) must predate the request by 2 s. The existing checks stay: no `--run-id` Runner process on this host, the launching host matches, and no remote lease. **Previous-release story** A launch request saved by the previous release has no `requestedAt`. It keeps today's proof: it is accepted only without saved Runner state of the run, otherwise the board shows the operator confirmation. Nothing is migrated or rewritten. **Alternatives considered** - Treating a proven-gone controller as proof that its Runner stopped. Local Runners outlive a controller restart (graceful drains reattach them), so a gone controller says nothing about its Runner or provider. - Running `paperclip-runnerd reap-lineage` for each writer marker. It needs a Runner PID and binary on the hot Retry path, and a Runner whose journal write failed has no marker to find. ## What Changed - `native-session-executor.ts`: the `native.process_start_requested` payload adds `requestedAt`. `unidentifiedLocalRunnerProvenAbsent` takes `earlierRunnersStoppedBefore` and accepts this run's saved state only when the Runner journal and lineage entries predate it (`runnerStateUnchangedSince`). - `native-local-process-stop.ts`: `recordUnidentifiedLocalRunnerStop` reads the last two process events and passes the request time only after a stop receipt (or no earlier launch). - `explicit-native-continuation.test.ts`: new embedded-Postgres case. A native run with a proven-stopped Runner, saved Runner state, and a later unidentified relaunch: the Retry is admitted and the stop is recorded. It stays blocked (`process_identity_missing`) when the Runner journal or a lineage entry changed after the request, when the request has no `requestedAt` (previous release), when the earlier Runner has no stop receipt, when a whole-second file time is within 2 s of the request, or when a `--run-id` Runner process is alive. A whole-second time 3.5 s before the request is admitted. ## Verification - Red/green: `cd server && npx vitest run src/services/explicit-native-continuation.test.ts -t "proven stopped"`. On master the `proven` case fails (admission returns null); with the fix all 8 cases pass. - Related suites: `explicit-native-continuation.test.ts`, `issue-confirm-run-stopped-routes.test.ts`, and the unidentified-Runner and stop cases of `native-session-executor.test.ts`. - Live, the s701 seed with the original wave's flags (`--env local --adapters codex,claude --workloads A1,C1 --faults K1,K2 --triggers time,result --concurrency 4 --tasks 4 --seed 701 --storage s3 --storage-faults ST7 --keep`): - `w-20261110T131243-s701` (final head) and `w-20261110T125259-s701`, master plus this fix plus #20514 merged locally: 4/4 pass each, every task crossed a restart, no R3 signature, 0 speed breaches. - `w-20261110T125616-s701`, master plus this fix without #20514: t01 and t02 pass. t03 and t04 hit the drain-during-preparation failure that #20514 fixes (`R3:retry_needed`). The Retry started a new run each time and both tasks finished (`retry_no_new_run` is gone). New launch requests carry `requestedAt`. - The restart timings of these replays did not land in the exact gap between a launch request and its process identity. The red/green test covers that window. ## Risks - Low. The change only widens one fail-closed proof, and only for launch requests saved by this release, after a stop receipt, with Runner files older than the request. The proof compares file times with a time taken on the same host. A backwards wall-clock step between the request and a Runner's first write could hide that write. The `--run-id` process check still has to pass, so that Runner must also be gone. - The Retry path adds one `lstat` per file in a small directory. It adds no wait and no work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
faae6b919f |
fix(runner): explain task scope on missing tool inputs (#19774)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents read saved task documents through native tools.
> - Active-task tools use the task bound to the run. Other task tools
require a task UUID.
> - Agents sometimes call the second set with only a document key.
> - The validator rejects the call, but gives no instruction about the
active-task tool.
> - This pull request adds scope instructions to the tool schemas and
returns them for missing required fields.
> - Agents can correct a rejected call. Task selection and permission
checks stay in place.
## Linked Issues or Issue Description
**What happened?**
Two observed document reads sent only `{"key":"sentry-case"}` to
`read_task_document`. Each returned `Invalid tool input: / must have
required property 'taskId'`. Each later retry with a task UUID returned
without an error.
Pattern: [Task reads fail when agents omit
taskId](https://www.braintrust.dev/app/Paperclip/p/Paperclip%20Staging/patterns/703d8c29-6a40-4aa2-b2c6-e2c5c36b5657).
Related PR: #18824 changed document key validation. It does not address
task scope.
**Expected behavior**
The tool contract must state which task it reads. A missing-field error
must include the schema owner's recovery instructions. A retry must
still pass validation and task permission checks.
**Steps to reproduce**
1. Call `read_task_document` through the MCP bridge with only a document
key.
2. Inspect the error sent to the caller.
3. Retry with an authorized task UUID. Confirm the returned document
belongs to that task.
### Hypothesis and thesis for a fix
Hypothesis: generic tool descriptions contribute to confusion between
active-task and explicit-task tools. An agent can also ignore a correct
schema. These tests do not measure the rate of model mistakes.
Thesis: Paperclip owns task scope instructions. Its five explicit-task
tools now describe the required UUID and the active-task alternative.
The two MCP transports return direct missing-property descriptions from
the admitted schema. The transports do not infer task IDs or apply
Paperclip task policy.
Proof: the same HTTP MCP regression fails on baseline
`f673bd3cb5b5b65ec51781e37355e168c3fe8ae4` and passes with the fix. It
still rejects before handler dispatch. An explicit retry reaches the
requested task. An isolated real API and PostgreSQL test checks the
saved revision, active-task selection, malformed UUIDs, and
cross-company denial through both transports.
## What Changed
- Add UUID and active-task guidance to `get_task`, `comment_on_task`,
and the three explicit-task document tools.
- Return bounded, distinct descriptions for up to three direct missing
required fields. Keep ordinary validation errors unchanged.
- Share this formatter between the Runner and OpenCode MCP bridges.
- Add transport regressions, real route and database checks, and tool
documentation.
## Verification
- Baseline red: both transport regressions return only the
required-property error and fail the recovery instruction assertion.
- Candidate green: 58 Runner and OpenCode transport and error formatter
tests pass on Linux with Node 24.22.2.
- Final-head server verification passed all 115 tests: 113 document-key
checks and two real API integration cases.
- The two local integration cases use real MCP HTTP, agent authority,
API routes, and disposable PostgreSQL. They read a selected other task
and its saved revision, read the active task separately, reject missing
IDs for all five tools, reject malformed UUIDs, and return 404 for a
foreign company task.
- Runner TypeScript typecheck and build pass.
- The full server typecheck passed on head
`03a37e9e50896416cfd45be33bf6dbe3e13169df` with
`PAPERCLIP_TYPECHECK_LOW_MEMORY=1 node scripts/typecheck-layers.mjs`.
The first run ended without a compiler diagnostic. Final-head CI passes
and Greptile is 5/5.
- Full preflight stops because cargo is absent. Rust checks were not run
locally.
- One existing delayed-body fetch test failed with a Node 25 fetch
runtime error. All transport tests pass on Node 24. The first
integration attempt used the wrong fixture return shape; the test was
corrected before the passing run.
- CI follow-up: the Runner test program reported four TS18046 errors
because `Response.json()` returns `unknown` under CI typings. Both tests
now declare the response type. `pnpm exec tsc -p tsconfig.test.json
--noEmit` passes, and both transport suites pass again (56 tests). Final
head `ffccebca19fe9e21a097829adc020679ffa7d86c` passes CI and Greptile
5/5.
- Review follow-up: the foreign task now has a saved document with a
distinct body. Both integration cases passed again after this test-only
change. The read must return 404 and must not return that body.
Current-head CI passes and Greptile is 5/5.
- Final CI:
https://github.com/paperclipai/paperclip/actions/runs/42352822739 . All
required jobs passed for `ffccebca19fe9e21a097829adc020679ffa7d86c`.
Greptile is 5/5 with no unresolved review thread.
- Real local API behavior is verified. External model behavior and
deployed recovery are not verified. No merge or deployment is included.
## Risks
- Missing-field errors now contain schema descriptions. The text is
limited to direct properties in the admitted schema, at most three
distinct descriptions and 1,000 characters each.
- Nested schemas and references do not get recovery text. Their
validation errors remain intact.
- Clearer instructions can help callers recover. They do not guarantee
that a model will choose the correct tool.
## Model Used
- OpenAI GPT-6 via Codex, with tool use and code execution. The exact
deployment model ID and context window are not exposed to 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>
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
|
902f376796 |
fix(recovery): keep a remote restart claim's lease through slow preparation (#20540)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, a remote run resumes on the same heartbeat run under a restart claim: a lease on its native coordinator row, 60 s for a remote claim (`REMOTE_CONTROLLER_LEASE_MS`) > - Preparation then runs before the native executor takes its own lease renewal. It includes the run-log begin and the workspace restore, which can each wait out a storage outage for minutes (#20299) > - Preparation renewed the claim only for `reattach_remote_runner`. A `bootstrap_incomplete` or resume claim kept its 60 s lease with nothing renewing it > - So a preparation wait longer than a minute let the lease lapse. The run failed at its next writer-authority check (`run_log_writer_authority_lost`) the moment storage came back > - This PR renews every remote restart claim during preparation. When the same owner's executor claims at the next attempt, it takes over that renewal > - The benefit is that a resumed remote run survives a long storage outage during preparation instead of failing as soon as storage returns ## Linked Issues or Issue Description **What happened?** In restart-chaos waves on Daytona R1 with `--storage s3 --storage-faults ST1` and K2 at +110 s, a resumed `bootstrap_incomplete` run's `heartbeat.prepare.run_output` ended `failed` right after the outage ended: - `w-20261110T101517-s11936` t01, run `9d39550a`. The claim was at 10:19:55 and the outage ended at 10:22:18.15. `run_output` failed at 10:22:18.17, after 141 s. - `w-20261110T081647-s11936` t02, run `9da10424`. The claim was at 08:21:29 and the outage ended at 08:23:30. During the outage the journal claim fails on its head `GET` before it checks authority. The first attempt after the outage reaches `claimFrom`'s `assertAuthority` (`run-log-authority.ts`), which requires a live coordinator lease. That lease expired 60 s after the claim. **Why not just widen the condition?** For a `bootstrap_incomplete` claim, the executor's own claim moves the coordinator attempt to N+1 (`nextNativeProviderAttempt`). `startNativeSessionExecutionLeaseRenewal` shares one renewal per lease owner and threw `native_session_renewal_binding_changed` when the attempt differed. That is presumably why preparation renewal was limited to reattach claims. **Related:** #20460 (live status while a run waits out a storage outage). #20514 settles a failure that this lapse used to leave silent. ## What Changed - `heartbeat/recovery.ts`: start the preparation lease renewal for every remote restart claim (`scope.remote`), not only `reattach_remote_runner`. - `native-runtime/native-execution-lease.ts`: - When the same owner's executor starts its renewal at a higher attempt, it takes over the shared renewal, rebinding the attempt and `onLost`. A lower attempt or any other binding change still throws. - A renewal that failed under the old binding is not counted as a loss. - A failed renewal is not a loss when the coordinator shows the same owner already at a higher attempt (the executor's claim committed while the renewal was in flight). The renewal deadline still ends ownership if no executor takes over. - Tests: one recovery case and three lease-renewal cases (below). ## Verification - `cd server && npx vitest run src/services/native-runtime/native-execution-lease.test.ts src/services/heartbeat/recovery.test.ts src/services/native-runtime/native-execution-lease-reacquire.test.ts src/services/native-runtime/native-runner-restart-lease-renewal.integration.test.ts src/services/native-runtime/native-runner-restart-preparation.integration.test.ts`: 143 pass. `npx tsc --noEmit`: pass. - Red/green, recovery test "keeps a remote bootstrap restart claim through preparation longer than its first lease, then hands it to the next attempt": the claim's first lease ends 4 s in. Preparation runs past it and the run-log writer authority still passes. Then the executor claims attempt 2 and its renewal keeps extending the lease with no detach. - Red with master's `recovery.ts`: the lease is never extended. - Red with only the `recovery.ts` change: `native_session_renewal_binding_changed`. - Green with both. - Red/green, lease unit tests: handover to the next attempt (and refusal of a lower one); a failure under the old binding is not a loss; a failure while the coordinator shows the same owner at a higher attempt is not a loss. All three fail on master. - Live wave: results in a comment below. ## Risks - Medium-low. - The preparation renewal now runs for remote `bootstrap_incomplete` and `resume_dead_runner` claims too. It renews the same coordinator row under the same CAS conditions reattach claims already use. - If preparation truly loses the lease (another owner takes it), its `onLost` still runs (`detachNativeSessionsForRestart`) and authority checks still fence writes. - The new non-loss case applies only when the coordinator names this exact owner at a higher attempt. The renewal deadline still bounds it. - Previous-release state: this persists nothing new. A claim saved by the previous release resumes unchanged; its preparation is simply renewed now. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
f4337bb5fb |
test(evals): land the chat catalog eval harness on master (#19614)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The chat-feature evals measure whether agents complete real chat requests through the product > - The chat catalog eval harness and its repairs live on the `chat-eval/master-candidate` branch, not on master > - Today's work made 139 of 242 chat evals pass in verified single-case live runs. Each product fix landed on master as its own pull request > - The eval-side changes (setups, graders, fault seams, evidence capture) still exist only on the candidate > - This pull request lands the harness on master. Master stays the source of truth for product behavior > - The benefit is one tree: evals run against master commits, and product fixes reach the evals as soon as they merge ## Linked Issues or Issue Description Refs #19441, Refs #19599, Refs #20270, Refs #20334, Refs #20342, Refs #20344, Refs #20354, Refs #20360, Refs #20373, Refs #20382, Refs #20409 **What existing behavior does this improve?** The chat catalog eval harness and the product it evaluates live on a long-running branch outside master. Eval fixes do not reach master, so master-based eval runs use old setups and graders. **Subsystem affected** `tests/runner-e2e` chat catalog harness, plus a small set of product changes that exist only on the candidate. **Proposed change** Land the harness on master. Every product fix from this eval work already merged to master as its own pull request. Conflicts keep master's product behavior. ## What Changed - Most of the diff (808 of 851 files) is under `tests/runner-e2e/`: chat-catalog domains, scenarios, descriptors, the TestOps manifest, private evidence tooling, and their tests. - Graders now check saved state, setup literals and rendered or clicked links. Many keyword and regex checks on free-form replies are removed. Each changed grader has a bumped `oracleVersion` and calibration tests (a valid reply, a reworded valid reply, and broken variants that must fail). - The shared link helper `hasLink` no longer treats a list link (for example `/COM/issues?view=mine`) as a link to every task without a URL key. - Setup fixes build real state through supported product routes: routine history, agent grants through the board grant route, budget holds with a real cap, and a storage-layer archive fault for `inbox-search.archive-partial-failure`. - Browser boundary journeys (sign-in, chat-disabled, onboarding) now write the standard pass evidence files. - Retargeted cases, approved by the maintainer: `agent-chat.chat-disabled` (Conference Room gate), `agents.restore-instructions` (no saved instruction history), `access.manage-membership` (member permission edits stay board-only). - The private evidence trusted files (`manual-private-handoff.ts`, `manual-private-retention.ts`, `manual-private-report-assets.ts`) are byte-identical to master. - Product files outside `tests/` (43 files, +1,328 −49) carry candidate-only changes that master does not yet have. Most are tests. The main code changes are in `request-human-input.ts`, `heartbeat-run-summary.ts`, `native-completion-feedback.ts`, `native-run-finalizer.ts`, `openapi.ts`, `secrets.ts` and `IssueChatLongThreadPerf.tsx`. ## Verification - `pnpm vitest run chat-catalog-domains chat-catalog-contract.test.ts chat-catalog-consistency.test.ts unit-launcher.test.mjs --testTimeout=180000` in `tests/runner-e2e`: 269 files pass, 3,550 tests pass. - `node --test chat-full-catalog-testops-manifest.test.mjs`: 10 pass. - `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`: no change. - `tsc` for `server` and `tests/runner-e2e`: clean. - Live single-case verification: 139 of 242 cases passed on paid runs, and a person read the saved evidence for each. These are single-case runs, not one full-cohort run. ## Risks - Large diff. Most of it is test-only, but the 43 non-test files change product code paths. Review those files first. - Some graders are now less strict on reply wording. A person must read replies for meaning on those cases. - 42 cases need disposable outside accounts (Notion, Slack, GitHub, Daytona and others) and stay blocked. `access.invite-teammate` stays failing until local agent isolation lands, because the runner's durable state keeps a live invite token. - The paid workflow compares the trusted private-evidence files with master. They are unchanged, so this risk is low. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution, in Claude Code. Subagents used the same model. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
60b9147e7c |
fix(native): accept harness diagnostics in a remote session-opened bootstrap proof (#20543)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs on a remote sandbox (Daytona) keep their controller
state in a Postgres checkpoint. The controller writes every command to
that checkpoint before it sends the command.
> - After a restart, recovery uses that record to prove that a drained
remote bootstrap reached no provider turn. If so, it bootstraps the run
again on the same heartbeat run.
> - A graceful restart seconds after native start often catches a runner
that opened its session and emitted ACPX `harness.diagnostic` events,
but was never sent a turn.
> - The remote no-turn proof did not accept `harness.diagnostic`, so
recovery blocked these runs as `ambiguous_remote_recovery_evidence` and
the task needed a manual Retry.
> - This pull request accepts `harness.diagnostic` in that proof. The
proof's command check still requires that no turn command was ever
issued.
> - The benefit is that a restart just after native start re-bootstraps
the run automatically.
## Linked Issues or Issue Description
Refs #20538 (the harness trigger that reproduced it)
**What happened?**
Restart-chaos wave `w-20261110T091639-s11936` t02, run `c4fc7f03`
(Daytona R1, K2): a K2 landed 13 s after native start. Recovery returned
`ambiguous_remote_recovery_evidence`, and the run blocked with
`native_restart_recovery_blocked` (R3 retry_needed). I reproduced it
with a kept database in `w-20261110T132656-s11936` t02, run `ecd98d55`,
using `--native-trigger native-start+5` from #20538. Decoded controller
checkpoint:
- commands: `run.prepare`, `session.open`, `session.goal.get` (no turn
command);
- committed events: `session.started`, `session.capabilities.updated`,
`session.goal.snapshot`, `harness.diagnostic`, `harness.diagnostic`,
`session.goal.snapshot`;
- `ackedSourceSeq` 6, no `sessionCheckpoint`, no projected provider
events.
`nativeRunnerCheckpointRemoteBootstrapProof` returned `null` only
because `harness.diagnostic` was not in `SESSION_OPEN_EVENT_TYPES`.
**Expected behavior**
Recovery classifies the run `bootstrap_incomplete`
(`remote_session_opened_without_turn`) and bootstraps it again.
**Steps to reproduce**
1. Run a Daytona R1 codex wave with `--faults K2 --native-trigger
native-start+5`.
2. The suspended run is blocked with
`ambiguous_remote_recovery_evidence`.
**Paperclip version or commit**
`master` at
|
||
|
|
c9226de30b |
fix(heartbeat): leave a preparing run to the next controller after a graceful drain (#20514)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The heartbeat service runs each agent task. A graceful shutdown
(SIGTERM) drains running native runs and hands them to the next server.
> - The drain detaches the native runner session and releases the run's
lease with the recovery state `awaiting_runner_reattach`.
> - An in-process execution can still be in run preparation at that
moment, for example while the run-log journal claim waits on a slow or
unavailable object store.
> - When that execution then lost writer authority
(`run_log_writer_authority_lost`), the generic failure path settled the
run as `adapter_failed`. The next server then saw
`native_continuation_requires_reconciliation`, and replay stayed
blocked.
> - This pull request makes a preparing execution that a restart
detached stop quietly, the same way a detaching native session already
does.
> - The benefit is that a slow preparation step plus a graceful restart
no longer turns into a failed run that needs a manual retry.
## Linked Issues or Issue Description
**What happened?**
In restart-chaos wave `w-20261110T092411-s701` (local, A1/C1, K1/K2, ST7
read-after-write lag), three of four tasks failed with R3
`retry_needed`. For run `218a0425`, the K2 SIGTERM drain wrote
`native.recovery.transition` with `awaiting_runner_reattach` and
released the lease. About 3 s later, the old server's in-process
execution, which was still in `runOutput.begin`, got
`run_log_writer_authority_lost`. `runCompletion.failRun` then settled
the run `adapter_failed` with a durable settlement. The next server
found `native_continuation_requires_reconciliation` and did not replay
the run.
**Expected behavior**
The drain hands the run to the next server. The old execution must not
settle it. A real authority loss outside a drain must still fail the
run.
**Steps to reproduce**
1. Start a native run recovery, and hold it in preparation (slow store,
ST1 outage wait, or ST7 lag).
2. Send SIGTERM so `drainRunningRunsForShutdown` suspends the run.
3. Let preparation continue and check writer authority. The run settles
`adapter_failed`.
**Paperclip version or commit**
`master` at
|
||
|
|
15a734d068 |
fix(heartbeat): show a storage outage wait as live status, then record it once (#20460)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In cloud mode a run reads and writes file storage on its critical path: it starts its run log (`s3_segments`), restores its saved workspace, and saves it again > - Since #20299 a run waits out a storage outage there for up to 5 minutes instead of failing > - That wait was silent. The board showed "running" with no sign of life, so a storage wait looked like a dead run (restart-chaos R5 `silent_dead_run`) > - The first version of this PR wrote a run event on every wait. That writes the run row (`next_event_seq`, `updated_at`) while the run-log claim is still in progress, and it covered only the run-log begin > - This PR shows the wait as live run status instead, writes nothing during the wait, and records one lifecycle event when the wait ends. It covers every critical-path wait in the run's storage scope > - The benefit is that a storage outage shows on the board as waiting, the run's history keeps one honest line about it, and the claim path is left alone ## Linked Issues or Issue Description Refs #20299 (wait out a store outage on a run's critical path). **What happened?** - `w-20261110T043533-s11936` (Daytona R1, ST1 194 s, K2): run `d49f58d8` waited 186 s in `heartbeat.prepare.run_output` with no event. The grader failed it as `R5:silent_dead_run`. - `w-20261110T105903-s11936`: the restore download (`storage.snapshot.download`) waited 182 s with no sign of life. **Expected:** the board shows the run waiting for file storage, and nothing about the notice can disturb run-log ownership. **On the failed `run_output` seen on builds of the first version** (`w-20261110T101517-s11936` t01 run `9d39550a`, `w-20261110T081647-s11936` t02 run `9da10424`): the evidence points to an expired restart lease, not to the notice's row writes. Neither the writer-authority check (`run-log-authority.ts`) nor the journal claim compares `next_event_seq` or `updated_at`. A remote `bootstrap_incomplete` restart claim gets a 60 s lease (`REMOTE_CONTROLLER_LEASE_MS`), and preparation renews it only for `reattach_remote_runner` claims. In t01 the claim was at 10:19:55, the outage ended at 10:22:18.15, and `run_output` failed at 10:22:18.17, on the first authority check after the store came back. That fix ships separately as its own PR. ## What Changed - `storage/storage-unavailable.ts`: - The run's storage scope can carry a wait observer (`reportStorageOutageWaitsTo`). - `retryThroughStorageOutage` tells it about each backoff (`waiting`). The operation that owns the wait reports one `ended` (`recovered` true or false), so nested waits do not double-report. - `storageOutageWaitFor(purpose, work)` names the step. - An observer that throws never changes the wait. `endStorageOutageWaitOn` keeps the observer. - `heartbeat/storage-wait-notice.ts` (new): - Each wait sets live runtime status (phase `waiting_for_storage`, "Waiting for file storage before starting the run log (45s so far)") and publishes `heartbeat.run.progress`. Nothing goes to the database. - When the wait ends, it writes one `lifecycle` warn event: "Waited Ns for file storage before …", with payload `reason: "storage_outage_wait"`, `purpose`, `waitedMs` and `recovered`. - `heartbeat.ts`: installs the notice next to `endStorageOutageWaitOn` (one line). - Purposes: - run-log begin: "starting the run log"; - `loadWorkspaceSnapshotArchive`: "restoring the workspace"; - `uploadWorkspaceSnapshot`: "saving the workspace". - `packages/shared`: adds `waiting_for_storage` to `HeartbeatRunStatusPhase`. - `tests/restart-chaos/live-status-evidence.ts` (new): R5 also counts the board's live run status (`/api/issues/:id/live-runs` `currentStatusUpdatedAt` / `lastEventAt`) as evidence that a running run is alive. Wiring it into `observer.ts` takes one import and one changed line. - `doc/run-log-events.md`: documents the live status and the one event. - Removed: the first version's per-wait run event. ## Verification - `cd server && npx vitest run src/services/heartbeat/run-output.test.ts src/__tests__/storage-outage-wait.test.ts src/services/remote-workspace-snapshot-storage.test.ts src/services/object-byte-journal.test.ts src/__tests__/storage-s3-credential-rotation.test.ts`: 103 pass. - Red/green: with the observer disabled (master behaviour), these new tests fail. With this change they pass: - run-output "shows the wait as live status and writes nothing to the run while its log claim waits": no run-row write and no event during the claim's waits; the live status is `waiting_for_storage`; the claim's writer-authority check still passes; one lifecycle event afterwards; - the helper's observer tests (one end per wait; gave-up reporting; a throwing observer; the Stop signal keeps the observer); - snapshot storage "tells the run it is waiting for storage while a save or a restore waits out an outage". - `PAPERCLIP_IN_WORKTREE=false npx vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/live-status-evidence.test.ts tests/restart-chaos/observer.test.ts tests/restart-chaos/triggers.test.ts`: 65 pass. `node scripts/typecheck-test-projects.mjs tests/restart-chaos`: pass. `cd server && npx tsc --noEmit`: pass. - Live restart-chaos waves: results in a comment below. ## Risks - Low. - The retry policy, its budget and Stop handling are unchanged. - During a wait, the only new effect is an in-memory status and a live event. - Afterwards, one event per wait, written without blocking the run. - Runtime status is per server process. A board on another instance sees the waiting line only after the closing event. - Shared journal work (`runSharedStorageWork`) keeps its own scope without the observer. Its waits surface through the enclosing run-scoped wait, as before. - Previous-release state: this persists no new run or session state. A run saved by the previous release resumes unchanged. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
bf514759f9 |
fix(run-log): answer 404 for an object run log that was never written (#20539)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Each heartbeat run keeps a run log, and the board reads it through
`GET /api/heartbeat-runs/:id/log`
> - A run can store its log in object storage (`s3_segments`)
> - When a run fails before its log is written (for example in
`runOutput.begin`), the object store throws a plain `Error("Object run
log not found")`
> - The route turns that plain error into a 500, so the board shows a
server failure for a log that simply does not exist
> - The local file store already answers 404 `Run log not found` for the
same case, and the UI treats a 404 as an unavailable log
> - This pull request makes the object store throw the same 404
> - The benefit is that a missing object run log looks like a missing
log, and a real storage failure still answers 500
## Linked Issues or Issue Description
**What happened?**
`GET /api/heartbeat-runs/:id/log` answered 500 `Internal server error`
for an `s3_segments` run whose log was never written. The server error
was `Object run log not found` from the object run-log store.
**Expected behavior**
The route answers 404 `Run log not found`, the same as the local file
store. The transcript view already treats a 404 for a finished run as a
missing log, and the log viewer already treats it as an unavailable log.
A real storage failure still answers 500.
**Steps to reproduce**
1. Use S3 storage for run logs.
2. Make a run fail before its run log is written (for example in
`runOutput.begin`).
3. Read `GET /api/heartbeat-runs/<run id>/log`. It answers 500.
**Paperclip version or commit**
`master` at
|
||
|
|
fea4530a30 |
fix(recovery): retry a preparation killed by a restart in the uncharged lane (#20534)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, an interrupted run must continue on its own. A user must not have to press Retry because a deploy or a crash happened > - A preparation that a restart stops before the provider starts spent no provider work. It is retried in the uncharged `interrupted_preparation` lane (12 retries), not the charged `transient_failure` lane (2 retries) > - #20496 made that work for a graceful drain (SIGTERM). A hard kill (SIGKILL) during workspace preparation still landed in the charged lane > - Two killed preparations in one chain then spent the failure budget, and the next interruption could not continue: the task needed a manual Retry > - This pull request lets restart recovery hand a killed preparation to the uncharged lane once the killed controller is proven gone > - The benefit is that a task survives any number of restarts during workspace preparation (up to the crash-loop cap) without a Retry click ## Linked Issues or Issue Description **Subsystem affected** Heartbeat restart recovery: the process-loss reaper in `server/src/services/heartbeat/recovery.ts`. **Problem or motivation** The restart-chaos harness on Daytona with `--storage s3 --storage-trigger snapshot-download` (seed 603, task t02, a codex C1 follow-up) kills the server while a workspace snapshot downloads. On builds that contain #20496, two replays still ended in `R3:retry_needed`. The run chain in replay `w-20261110T061107-s603-replay-t02-1794309789008`: 1. K1 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 1/2 … transient_failure". 2. K2 drain during preparation: `interrupted_preparation` 1 (the #20496 path, correct). 3. K4 SIGKILL during preparation: `process_lost`, then "Scheduled bounded retry 2/2 … transient_failure". 4. K2 drain during preparation: the failure budget was spent (`failureRetries` 2), so the hold could not continue. The harness had to click Retry. **Root cause** The reaper does record the no-provider-work certificate (`executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`) for a preparing run from another boot. But before it fails the run, `revokeExpiredLegacyControllerWithState` swaps in a random boot id with a 60 s lease, a tombstone that no controller holds. `scheduleBoundedRetryForRun` then asks `isInterruptedNativePreparation`, whose `preparerMayStillUnwind` check sees that live lease and says the old preparer may still be unwinding. The lane falls back to `transient_failure` and charges the failure budget. Every killed preparation is charged, whether or not its controller is provably gone. **Proposed solution** When the reaper has already proven the old controller's recorded process gone (`legacyControllerGone`, the shared owner prover that #20496 also uses) and the revoked state shows the run was still preparing with no process recorded (`restartInterruptedBeforeDispatch`), expire that tombstone lease in the same `process_lost` write. The lane decision then sees an expired lease and applies its existing checks (no coordinator, result or execution events; leases released with stop receipts; workspace repair when needed). Any evidence of provider work, or a controller that cannot be proven gone, keeps today's charged behaviour. **Alternatives considered** - Revoking with an already expired lease: the 60 s revocation lease fences other servers while this pass fails the run, so the expiry belongs in the terminal write. - Teaching `preparerMayStillUnwind` to ignore unstamped leases: that would also skip the wait for a lease of a live controller saved by an older release. ## What Changed - `server/src/services/heartbeat/recovery.ts`: the `process_lost` write sets `controllerLeaseExpiresAt` to now when `restartInterruptedBeforeDispatch && legacyControllerGone`. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: - New: a chain whose preparation is killed twice (controllers proven gone) and then drained. Each kill schedules `interrupted_preparation` (attempts 1 and 2, `failureRetries` 0), and after the drain the restart settlement schedules `interrupted_preparation` 3 without a Retry. - New old-saved/new-resumed case: a preparing run saved by an older release without a controller process stamp keeps the charged `transient_failure` lane. - `doc/execution-semantics.md`: one additive sentence saying the lane also covers a hard kill once the killed controller is proven gone. ## Verification - Red/green: `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "killed twice"` fails on master (the first kill schedules `transient_failure`) and passes with the fix. - Related suites pass: `heartbeat-process-recovery`, `interrupted-preparation-hold-settle`, `execution-recovery-dead-controller`, `legacy-controller-lease` and `ownership-reconciliation-report` (5 files, 521 tests). - Live (Daytona, `--storage s3 --storage-trigger snapshot-download`, replay of `w-20261110T061107-s603/t02` with K1/K2/K4/K2, image `paperclip-daytona-runner@sha256:05a4a092…` whose runner content matches this branch): running; the result will be posted as a comment. ## Risks - Low. The change applies only when the shared owner prover returns `gone` for the exact recorded controller and the run was still preparing with no process recorded. The lease's boot id stays the random revocation id, so the old controller can still never renew it. Old rows without a controller stamp keep today's behaviour. - Previous-release story (upgrade-safe-run-state): no persisted shape changes. A run saved mid-preparation by an older release has no `controllerProcess` stamp, cannot be proven gone, and keeps the charged lane (covered by the new old-saved test). Retries an older release already scheduled in the charged lane keep their counters. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
adb0e77a4c |
fix(run-log): read a resumed run's own log while the store still hides its new head (#20523)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In cloud mode, run logs use the `s3_segments` store: an object byte journal whose `head.json` names the current writer and the committed bytes > - When a run resumes after a restart, `runOutput.begin` claims the journal (this publishes a new `head.json`) and then reads the existing log prefix to find where native log export continues (`nativeRunLogPrefixCursor`) > - On an S3-compatible store with read-after-write lag, the `head.json` that the claim just published can read as `NoSuchKey` for some seconds > - The read then throws "Object run log not found", and the resumed run settles as `adapter_failed` and needs a Retry > - This pull request makes a read through a live writer's own handle use that writer's head when the store does not show the head yet, and makes the one-time source-prefix proof step treat a hidden head as "proof unavailable" instead of "another writer" > - The benefit is that a run that resumes right after a restart survives a store that shows new objects late, with no added wait ## Linked Issues or Issue Description Refs #20509 (the same lag on the journal claim). **What happened?** In restart-chaos wave w-20261110T095205-s701 (local, `--storage s3 --storage-faults ST7`, K1/K2, on master with #20504, #20509 and #20514), two resumed runs (t03 run 4f4fc7bc, t04 run c92c53d8) failed in `runOutput.begin`. The error was "Object run log not found" from `object-run-log-store.ts` `read`, called by `nativeRunLogPrefixCursor` (`native-run-log-export.ts:314`). Both runs settled `adapter_failed`, and the grade was R3 `retry_needed`. ST7 makes a GET or HEAD of a just-written key answer 404 `NoSuchKey` for 5–30 s. **Expected behavior** A resumed run reads the log prefix that it just claimed and continues. A store that shows a new object late must not look like a missing log. **Steps to reproduce** Write a run log, begin a successor writer on a new store instance, then make `head.json` read as `NoSuchKey`. `read(successor)` throws "Object run log not found". ## What Changed - `server/src/services/object-run-log-store.ts`: `read()` reads the head from the store as before. When the store says the head does not exist and this process has a live writer for that handle, it uses the writer's head. The writer published that head, so it proves the log exists. A handle without a live writer still fails at once with "Object run log not found". - `server/src/services/object-byte-journal.ts`: `recordVerifiedSourcePrefix` reads the head again to confirm ownership before it publishes the optional source-prefix proof. When that head reads as missing, it now throws `ObjectJournalSourcePrefixProofUnavailableError` (the existing "proof is optional" result). The store then continues without the proof, and the next resume scans the prefix again. A visible head with a different etag, a closed head or a deleted head still fails as `ObjectJournalConflictError`. Later appends still publish the head with a conditional write against the writer's etag, so another writer is still detected. - `server/src/services/object-run-log-store.test.ts`: a test hides the successor's just-published `head.json` for the full resume sequence that `nativeRunLogPrefixCursor` runs against the store (`read`, then `recordVerifiedSourcePrefix`). Before this change, `read` fails with "Object run log not found"; with only the read fix, the proof step fails with "The output journal is owned by another writer". After both, the read returns records 1–3, the proof is marked unavailable, and a later append and read return records 1–4. A second test checks that a read of a log that was never written still fails at once. ## Verification - `cd server && npx vitest run src/services/object-byte-journal.test.ts src/services/object-run-log-store.test.ts src/services/object-run-log-store-tail-read.test.ts src/services/run-log-store.test.ts src/services/run-log-store-cancellation.test.ts`: 87 pass. The new resume test fails with "Object run log not found" without the read fix, and with "The output journal is owned by another writer" without the proof fix. - `cd server && npx vitest run src/services/native-runtime/native-run-log-recovery.test.ts src/services/native-runtime/native-run-log-watermark.test.ts`: 27 pass. - `cd server && npx tsc --noEmit -p tsconfig.json` passes. - Live: a local ST7 rerun (seed 701) on a build with this fix follows in a comment. ## Risks - Low risk. The read change only affects a read whose store head is missing and whose handle has a live writer in this process. All other reads behave as before, including fetch counts and the cache. - When the head is hidden during the proof step, the proof is skipped for this attempt. The next resume of the same run scans the log prefix once more. This is the existing degraded path for a proof outage. - The fix does not wait for lagged segment objects. A segment that the previous server wrote less than the lag window before a restart can still read as `NoSuchKey` during the prefix scan. The wave evidence shows only the head lag. A broad segment wait broke existing failure tests, so it is not part of this change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
bf45700ca3 |
fix(native): make Retry work after a turn start cut off once Codex recorded the turn (#20516)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run sends `turn/start` through its runner. A control-plane restart can kill the runner and Codex while that request is in flight > - #20507 lets the runner resume by itself when Codex's thread history proves the turn was never created. When Codex did record the turn, the turn may have done work, so the runner still fails closed and nothing replays it > - That branch must still end in a Retry that works. The next run on the same conversation is refused at the runner-state fence before it asks for a runner. #20507 let Retry continue after that run by matching the refusal's error text > - Driving that branch end to end with the real runner exposed a second dead end. When the owner's runner dies, its provider-session close cannot prove suspension, so this server process can keep an operator-required cleanup quarantine for the task. Retry was then refused outright ("Send a new message") > - This pull request replaces the text match with a server-authored run-log event, lets Retry take the same verified-stop path as a new message, and adds a real-runner test of the whole branch: the cut-off owner fails closed, the follow-up is refused with structured proof, Retry is admitted, and the fresh Retry run completes > - The benefit is that this restart window cannot strand a task, and the Retry gate no longer depends on error wording ## Linked Issues or Issue Description **What happened?** Follow-up to #20507 (restart-chaos wave `w-20261110T033426-s108`, t03). #20507 left one item open: prove that a person's Retry produces a working run when the restart cut off `turn.start` after Codex recorded the turn. It also matched the fence refusal by its error text. **Expected behavior** After that cut-off, the owner stays failed closed and the turn is never started again by itself. A person's Retry starts a new run that completes. It never fails with `quarantine_owner_unverified` and never returns 202 with no run. **Steps to reproduce** 1. Run `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-cut-turn-start-retry.integration.test.ts`. 2. Without the new event, the test fails: the refused follow-up has no proof that it never launched, so Retry after it would wait on `process_identity_missing`. ## What Changed - `native-session-executor.ts`: when the runner-state fence refuses a run before launch (`prior_owner_*` or `legacy_prior_owner_*`), the executor records the server-authored event `native.prior_owner_fence_refused` with the closed refusal `reason`, then fails the run as before. A failed write leaves the run held (the safe side). - `native-local-process-stop.ts`: `nativeRunNeverLaunched` accepts a failed run that has this event (and still no launch, identity, `adapter.invoke` or runner-sourced event). It no longer reads the error text. - `explicit-native-continuation.ts`: when the task's in-process cleanup quarantine is operator-required, a Retry no longer returns `cleanup_quarantined` at once. Like a new message, it must first pass `verifyStoppedNativeSessionForContinuation`, which releases the quarantine and records `native.stopped_conversation_verified`. A native Retry always starts a fresh conversation (`forceFreshSession`), so it never resumes the interrupted turn. If the proof fails, Retry still returns `cleanup_quarantined`. - `verifyStoppedNativeSessionForContinuation` (`native-session-executor.ts`), three narrow additions: - a run with `native.prior_owner_fence_refused` may stand in for the saved state's owner, like a run refused by the cleanup quarantine (both were refused before launch); - when the session root is gone, it reads the fence's single `identity_indeterminate` quarantined copy of the owner's state (several copies stay ambiguous); - a provider startup attempt in phase `initialization_failed` whose direct child exited and whose process and process group are gone no longer blocks the proof. A same-run restore that fails closed leaves exactly this record. Those PIDs are also re-checked before the quarantine is released. - New real-runnerd integration test `native-cut-turn-start-retry.integration.test.ts` (real runnerd and fake Codex with `--hang-turn-start after`): 1. The owner run's `turn/start` hangs after Codex recorded the turn; the runner and Codex are killed. The owner's provider state keeps the ambiguous start, and the thread history keeps the turn. 2. The same-run resume restores the runner and fails closed with "cannot safely recover an ambiguous Codex turn start". Codex saw exactly one `turn/start`. 3. A follow-up on the same conversation is refused with `prior_owner_*`, never `quarantine_owner_unverified`. It records `native.prior_owner_fence_refused`, launches nothing, and `nativeRunNeverLaunched` is true for it. 4. The owner's operator-required in-process cleanup quarantine is pinned (whether the real close lands there is timing-dependent; about half of the local runs did). 5. Retry of the refused follow-up passes `admitExplicitNativeContinuation` (not a dry run) with no block reason, releases the quarantine and records `native.stopped_conversation_verified`. 6. The Retry run, prepared with a fresh conversation as task Retry does (`forceFreshSession`), uses a different runner-state root and completes with exit code 0. Codex saw one new `thread/start` and one new `turn/start`; the cut-off turn was never started again. - `explicit-native-continuation.test.ts`: the fence rows now insert the event. A new row shows that the refusal text alone keeps the run held. The "worker loss (retry)" case now expects a verified Retry to be admitted like a new message. - `native-recovered-owner-followup.test.ts`: a fixture event moves to a free sequence number, because the refused follow-up now writes its own event. - Docs: `doc/run-log-events.md` (new event) and `packages/paperclip-runner/docs/durable-recovery.md`. ## Verification - Red/green: - Without the event write, the integration test fails (no fence event, so the follow-up is not proven unlaunched). - Without the gate and proof changes, it fails with `expected [ 'cleanup_quarantined' ] to deeply equal []`: Retry is refused while the owner's quarantine is held. - With all changes, it passes. - `vitest run` of `native-recovered-owner-followup`, `explicit-native-continuation` (427 tests), `quarantined-task-retry`, `issue-confirm-run-stopped-routes`, `native-finalization-recovery`, `task-interruption-query`, `execution-projection`, `chat-run-publications`, `closed-native-receipt-accounting`, `agent-live-run-routes` and the new integration test pass. - Server `tsc --noEmit` passes. - Runner code and the pack `contractDigest` do not change. ## Risks - Low-medium risk. The new event is written only on the fence refusal path, which already fails the run. It is local run-log data (no Telemetry or OpenTelemetry). - Retry after an operator-required cleanup quarantine is a behaviour change: it was always refused, and now it is admitted only after the same exact stop proof a new message needs. That proof checks the owner's runner stop record, every recorded provider and startup PID and process group, and the unchanged state fingerprint before it releases the quarantine. - A run refused by a build between #20507 and this change has only the error text. It stays held on `process_identity_missing` until a person sends a new message or the board confirms the stop. #20507 has not shipped in a release yet. - The test models the restart in-process: it kills the runner and Codex and calls the executor again. It does not drive the HTTP Retry route; the route's own behaviour (fresh conversation, admission gate) is covered by existing tests and by `nativeRunNeverLaunched` here. - Open decision, unchanged: a turn Codex recorded before the restart is never replayed or adopted. Continuing is a person's decision. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 Refs #20507 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
93faca77ef |
fix(recovery): settle a drained run's hold once its controller is proven gone (#20496)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, an interrupted run must continue on its own. A user must not have to press Retry because a deploy happened > - A graceful drain (SIGTERM) marks a preparing legacy run `interrupted` and exits, but the run keeps its legacy controller lease for up to 60 s more > - After the restart, `settleUnrecoverableExecutions` skips the run's recovery hold while that lease is live, in case a living controller is still unwinding a Stop > - The controller is the drained process, which is gone. For up to a minute, the task shows "Couldn't finish this attempt" with a Retry button, before the automatic continuation starts > - This pull request lets the disposition proceed when the shared owner prover proves the lease's recorded controller process gone, and expires that exact lease, so the held-preparation continuation also starts at once > - The benefit is that a run drained during workspace preparation continues automatically right after the restart, without a Retry click ## Linked Issues or Issue Description **Subsystem affected** Server execution recovery: the disposition of `legacy_execution_requires_reconciliation` holds (`server/src/services/execution-recovery-resolution.ts`). **Problem or motivation** The restart-chaos harness can now restart the server while it is in an S3 transfer (#20451). The wave ran on Daytona with `--storage s3 --storage-trigger snapshot-download`. A K2 drain interrupted a C1 follow-up run while its workspace snapshot was downloading, then a second K2 drain interrupted the automatic retry during its restore. The task then showed the board-owned "Couldn't finish this attempt. Your message is saved. Retry" state, and the harness had to click Retry (`R3:retry_needed:C1:K2`). A `--keep` replay reproduced it. The kept database shows a run drained at 07:08:05 with `controller_lease_expires_at` 07:09:04.98. The server was ready again at 07:08:45. Its disposition did not settle the hold until 07:09:14.8. The automatic continuation was then delegated, because the user's Retry had already started a run. **Proposed solution** Before the disposition transaction, ask the shared owner prover (`legacyControllerProvenGone`) whether the live lease's recorded controller process is gone. For that exact controller boot id only, skip the wait and expire the lease in the same transaction (compare-and-swap on run, company and boot id). The later checks (`isInterruptedNativePreparation`, `isFailedNativePreparation`, the held-preparation settlement) then see an expired lease and schedule the `interrupted_preparation` successor. A lease held by a live process, including this one, still blocks as before. **Alternatives considered** Expiring the lease in the drain itself. The drain runs before the process exits, so the owner proof after the restart is the safer evidence. ## What Changed - `legacy-controller-lease.ts`: new `provenGoneLegacyControllerBootId(db, companyId, runId)`. It returns the boot id of a legacy run's unexpired controller lease when the shared prover proves that controller's recorded process gone, else null. - `execution-recovery-resolution.ts`: `settleUnrecoverableExecutions` computes that boot id before its transaction. When the live lease's boot id matches, it expires that lease and settles the hold instead of waiting. - New `execution-recovery-dead-controller.test.ts` (embedded Postgres): a drained run whose controller process has exited is settled at once, its lease is expired, and `isInterruptedNativePreparation` turns true (continuation eligible) while the original lease would still run for about 55 s. A lease held by the current process still waits and keeps its expiry. ## Verification - Red/green: `npx vitest run src/services/execution-recovery-dead-controller.test.ts` fails on master (the hold stays `active`) and passes with the fix. - Related suites pass: `task-retry`, `execution-control-work`, `heartbeat-process-recovery` and `native-startup-retry.integration` with the new file (5 files, 573 tests). - Live: the Daytona `--storage s3 --storage-trigger snapshot-download` rerun on a build with this fix will be posted as a comment. ## Risks - Low. The change only removes a wait when the shared owner prover returns `gone` for the exact recorded controller. An unprovable or alive controller keeps the old behaviour. The prover runs outside the row-locked transaction, and only for candidates whose lease is still live. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
c0593b69f1 |
fix(accounting): close a remote-Runner turn completed across a session restore as a lower bound (#20524)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs settle their charge once the final usage receipt is complete. > - #20517 fixed one case for the local Runner. If a restart stops the provider before a turn's final usage report, and the restored Runner then reports that turn completed, the turn now closes as a lower bound instead of waiting 10 minutes. > - Remote Runners deliver usage through a separate producer-owned receiver. That receiver dropped `session.resumed`, so the same turn there still waited for the 10-minute closure timeout. > - This pull request applies the #20517 rule to that receiver. > - The benefit is that remote ACPX and OpenCode runs settle promptly with an honest lower-bound label after a mid-turn restore. ## Linked Issues or Issue Description Refs #20517. **What happened?** After #20517, `session.resumed` reached turn accounting only through the local Runner's fallback recorder. The remote producer receiver (`native-usage-delivery.ts`) and the session-authority filter (`isNativeUsageAccountingEvent`) both dropped it. A remote ACPX or OpenCode turn that was open across a session restore, and then reported terminal without a complete report, left the receipt incomplete until the closure timeout. **Expected behavior** Same rule as the local Runner. Such a turn closes as a lower bound right away. A complete report that arrives after the restore is still used, and a turn that starts after the restore stays pending until its own report is complete. **Steps to reproduce** `native-usage-delivery.integration.test.ts > a turn restored mid-turn on a remote Runner > closes the turn as a lower bound, also after the controller restarts between restore and terminal` fails on master: the receipt stays `accountingReceiptReady: false`. ## What Changed - **Receiver (`createNativeUsageReceiver`).** It now accepts `session.resumed` for turn-reporting providers (ACPX, OpenCode) while a turn is open. It journals the restore as a receipt like any other producer event, so turn accounting marks the open turn restored. A replacement controller rebuilds the same state from that journal. - **Restores never fail delivery.** A restore from another run or source, or one with no open turn, is ignored, not rejected. An unacknowledged lifecycle event would otherwise replay without end. - **Session-authority filter.** `acceptAccountingEvent` now uses a new `isNativeUsageDeliveryEvent`: the usage events plus `session.resumed`. A restore for another run is ignored there too. - **Recovery routing is unchanged.** `acceptRecoveryAccountingEvent` and `onRecoveryAccountingEvent` keep the usage-only `isNativeUsageAccountingEvent`, so a source run's restore never requires a proven recovery source. - **Turn accounting** gets a `hasOpenTurn()` helper. - **Docs:** `doc/SPEC-implementation.md` now covers both Runners. ## Verification - `native-usage-delivery.integration.test.ts`, describe `a turn restored mid-turn on a remote Runner` (real PostgreSQL): | Test | On master | Here | |---|---|---| | `closes the turn as a lower bound, also after the controller restarts between restore and terminal`: the restore is journaled by one receiver and the terminal arrives at a rebuilt one; replaying everything again is idempotent | fails | passes | | `keeps the turn's own final report when it arrives after the restore` (guard) | passes | passes | | `keeps a turn started after the restore pending, and journals nothing for a restore with no open turn` (guard) | passes | passes | - `native-session-executor.test.ts > binds retained warm transport usage to the successor run authority` covers routing through the session-authority filter: - a current-run restore reaches the successor's capture; - a draining-run restore reaches the drained capture; - an unrelated-run restore resolves with no capture; - recovery routing ignores a source restore. It fails when the filter is reverted to `isNativeUsageAccountingEvent`. - Related suites pass: `native-usage-delivery.integration` and `native-usage-accounting` (46 tests), and the usage, accounting, receipt and authority cases in `native-session-executor.test.ts` (224 tests). Server typecheck passes. - **The live remote path is unproven.** No Daytona wave ran because the Daytona Runner image has not been rebuilt yet. The local Runner counterpart (#20517) passed restart-chaos L1, seed 20386, 4/4 (`w-20261110T104332-s20386`). ## Risks - **Upgrade safety.** Usage state saved by current master has no journaled restore and rebuilds as before. A receiver that predates this change replays a journaled restore as an ignored event, which is the behaviour on master. A restore that an older controller already acknowledged is not replayed, so that run waits for the closure timeout as it does today. - **One extra receipt per mid-turn restore**, only for ACPX and OpenCode and only while a turn is open. Nothing is added before `turn.start`: a restore with no open turn returns immediately. - **Codex and other providers are unaffected.** They do not use turn accounting. - No schema or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
23fa37f5bb |
fix(recovery): treat a retired prepared remote epoch as nothing to recover (#20491)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A restart must never leave a task stuck: an interrupted run must resume on its own or have a working Retry > - When a graceful restart interrupts a remote-workspace run that is still preparing, the run keeps a `prepared` reference to its workspace epoch, and a preserve-without-replay hold blocks the task > - The unsaved-files reconciler then retires that epoch with `abandonUnverifiedRemoteWorkspaceEpoch` and writes a `paperclip.remote-workspace-abandoned-epoch.v1` receipt. The workspace content is the last committed save again > - Recovery never read that receipt. The held-preparation reconciler waited for a repair receipt that could never exist, and restore recovery refused the source as `source_binding_invalid`. The task stayed blocked, and a UI Retry returned 202 and started nothing > - This pull request adds one strict shared rule: an abandoned-epoch receipt for the run's exact prepared epoch and allocation means there is nothing to recover. Every recovery path now uses it > - The benefit is that the task resumes automatically from the last committed save after the restart, whichever recovery path owns the hold ## Linked Issues or Issue Description Refs #20072 (replay a preparation a restart interrupted), #20299 (storage outage waits). **What existing behavior does this improve?** Recovery of a remote-workspace run that a graceful restart interrupted during preparation, after its unsaved epoch was retired. **Subsystem affected** Server: recovery (held-preparation reconciler, workspace restore recovery, the workspace repair check). **Current behavior** These are Daytona R1 restart-chaos waves (Paperclip-sized workspace, `--storage s3`, K2 graceful restart at +110 s, during the ~730 MiB first save): - `w-20261110T052533-s8805` and `w-20261110T062703-s8805` (ST2 slow store), held-preparation path: both runs ended `interrupted` in `executionStage: preparing`. The reference was `{ state: "prepared", epoch: 1 }`, and the workspace had moved to `{ epoch: 2, location: null, dirty: false, writerRunId: null }` with an abandoned-epoch receipt for epoch 1. The `legacy_interrupted_preparation_hold` reconciler never settled the hold (`I2_settled` violations), because `isInterruptedNativePreparation` requires `hasVerifiedWorkspaceRepair`, and that check accepts only a `saved` reference. Grader: `R3:retry_no_new_run`. - `w-20261110T091639-s11936` (ST1 outage), restore-recovery path: the run also had `workspaceRestoreFailure: "restore_failed"` and `workspaceRestoreRecovery`. Restore recovery owns that hold. `inspectWorkspaceRecoverySource` threw `source_binding_invalid` / `workspace_reference_mismatch` with retry class `service_required`, and the hold says Retry "succeeds only after Paperclip is updated". Grader: `R3:retry_no_new_run`. **Proposed behavior** `isAbandonedPreparedRemoteWorkspace(db, run)` is true only when all of these hold: - The run has a `prepared` remote reference and no native workspace sync reference. - The workspace authority is at a later epoch, and the run is not its writer. - The workspace has an abandoned-epoch receipt for the same run, the same epoch, and the same lease, provider lease and remote cwd. That rule is used in three places: - `hasVerifiedWorkspaceRepair` (`workspace-recovery-receipt.ts`): the retired epoch counts as repaired. This covers held-preparation replay, the restore-failure check in `settleInterruptedNativeBootstrap`, the execution blocker, and explicit Retry admission. `verifiedWorkspaceRepairReceipt`, which returns a receipt id, is unchanged. - `inspectWorkspaceRecoverySource` (`workspace-recovery-source.ts`): the retired source is reported as `source_unavailable` / `use_alternative_strategy`, not as a binding failure that needs a service update. - `heldInterruptedPreparationWhere`: a restore-recovery hold is listed for the held-preparation reconciler only when the matching receipt exists (SQL pre-filter). The settle then re-checks the full rule in JS before it acts. **Reason and benefit** The retired epoch held only preparation files: the hold proof requires that no provider work started. Its unsaved files were already given up by the reconciler that wrote the receipt. Waiting for a repair, or for a service update, cannot finish. ## What Changed - New `server/src/services/remote-workspace-abandoned-preparation.ts`: `isAbandonedPreparedRemoteWorkspace` (the strict rule) and `abandonedPreparedRemoteWorkspaceExists` (its SQL pre-filter). - `workspace-recovery-receipt.ts`: `hasVerifiedWorkspaceRepair` also accepts the rule. - `workspace-recovery-source.ts`: a retired prepared source is `source_unavailable` / `use_alternative_strategy`. - `interrupted-preparation-hold-settlement.ts`: restore-recovery holds are listed only with a matching receipt. - `interrupted-preparation-hold-settle.test.ts`: held-preparation replay (before and after the first save committed); a restore-recovery hold in the exact shape saved by the previous build (`restore_failed`, `workspaceRestoreRecovery`, exhausted `source_binding_invalid`) that now settles and replays; inspection reporting `source_unavailable`; and the rule refusing a workspace still at the run's epoch, a run still the writer, a native sync reference, another run's receipt, or another allocation's receipt. Not changed: `run-completion.ts` and the graceful-drain path in `recovery.ts`. ## Verification - `cd server && npx vitest run src/__tests__/interrupted-preparation-hold-settle.test.ts src/services/task-retry.test.ts`: 101 pass. - Red before: with the three recovery files at master, 4 tests fail (held-preparation replay ×2, the restore-recovery replay, and the inspection result). Green after. - Neighbouring recovery suites: see the comment below. - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Chaos rerun on a build with this change: see the comment below. ## Risks - Previous-release story (upgrade-safe run state): no persisted shape changes. Holds, run references and receipts written by the previous build are read as they are. The restore-recovery test uses the exact shape the previous build saved. A task already stuck in that state settles on the next reconciler pass after deploy. - The rule needs an exact receipt (run, epoch, lease, provider lease, cwd) and a later workspace epoch, so a real recovery is never skipped. Any mismatch keeps today's behavior. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
37808f2a4e |
fix(accounting): close a local-Runner turn completed across a session restore as a lower bound (#20517)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs record usage receipts, and accounting settles a run's charge once its final receipt is complete. > - ACPX Claude reports provisional zero usage until a turn ends, then one final report. > - A restart can stop the provider after the agent's finish call and before that final report. The restored Runner then resumes the session and reports the same turn completed with no complete report. > - The receipt stayed incomplete, so the accepted run waited for the 10-minute closure timeout and then settled as unpriced. Its spend was missing from company totals in the meantime. > - This pull request closes such a turn as a lower bound, the same way a turn the Runner fails as lost on restore is already closed. > - The benefit is that an accepted run settles right away with its observed usage and an honest lower-bound label. ## Linked Issues or Issue Description Refs #20484 (restart-chaos L1 contention follow-up). **What happened?** After #20484 fixed the database contention, restart-chaos L1 (`--seed 20386 --db-pressure 4`, codex and claude, K1) reached 4/4 tasks done. Each task still failed the T-oracle `company_spend_settled` with `pending_runs=1`. A replay on master with `--keep` (`w-20261110T100747-s20386`) reproduced it, and I read the kept database: - The pending run (`71f71d10`, ACPX Claude) succeeded and had its result accepted. Its usage had `accountingReceiptReady: false`, zero tokens, `costStatus: unpriced`, and no cost event. - Its only turn started before K1, and every usage report in it was provisional (`runDeltaComplete: false`). K1 struck right after the agent's `paperclip_finish` call (source seq 118), before the turn's final report. - On restart, recovery verified the local process stopped and resumed the session (`session.resumed`, seq 119). The restored Runner then reported the same turn `turn.completed` (seq 124) with no usage report. - The other Claude run in that wave lost its turn as `provider_turn_lost_on_restore`, ran a new turn live with full usage, and settled at once. **Expected behavior** An accepted run whose open turn's final report was lost with the provider process settles promptly. It records its observed usage as a lower bound, like a turn lost on restore. A complete report that does arrive after the restore is still used. **Steps to reproduce** `server/src/services/native-runtime/native-usage-accounting.test.ts > closes a turn that completed across a session restore without its final report, as a lower bound` uses the event sequence from the kept database. It fails on master because the snapshot stays incomplete. ## What Changed - `createNativeTurnAccounting` observes `session.resumed` and marks every open turn as restored. A terminal event for a restored turn without a complete report marks it lost. The snapshot then closes it as a lower bound (`accountingUsageLost`), through the existing lost-on-restore path. A new `turn.started` clears the mark. - A lost terminal turn keeps its identity in the snapshot, so the executor's terminal-turn match certifies the final checkpoint as complete. This came from review. - The native session executor includes `session.resumed` when it rebuilds turn state from the run's event log after a controller restart. - `doc/SPEC-implementation.md` documents the case next to the existing lost-on-restore rule. ## Verification - Red on master, green here: `native-usage-accounting.test.ts > closes a turn that completed across a session restore without its final report, as a lower bound`. It also checks that replaying the history twice gives the same snapshot. - Red on the first revision, green after the review fix: `native-session-executor.test.ts > certifies a turn completed across a session restore as a lower bound (history replay: false|true)`. It drives the restored sequence through executor completion, both live and from the event history. - New guards that pass before and after: - `keeps a restored turn's own final report when it arrives after the restore`: a complete report after resume is used, not marked lost. - `keeps an incomplete turn pending when the session resumed before it started`. - The existing `keeps an incomplete provider report pending instead of trusting placeholder zeroes` still passes. - Related suites pass: `native-usage-accounting`, `native-usage-delivery.integration`, `closed-native-receipt-accounting` (107 tests), and the usage, accounting and receipt cases in `native-session-executor.test.ts` (151 tests). Server typecheck passes. - Live, restart-chaos L1, seed 20386, `--db-pressure 4`, codex and claude, K1, head `a6e521f06c`, clean worktree: `w-20261110T104332-s20386` passed 4/4. R1–R6 and the T-oracle passed for every task. `company_spend_settled` reported `pending_runs=0` (summary 85.30823 = cost events 85.30823), with zero serialization failures under about 397k pressure updates. On master the same seed failed every task on `company_spend_settled pending_runs=1` (`w-20261110T100747-s20386`, among others). ## Risks - A restored turn without a complete report now settles at its observed usage, flagged as a lower bound, instead of after 10 minutes as "closure timed out". In both cases the final report never arrived. The money recorded is the same observed usage; only the timing and the provenance label differ. - Scope: local Runner only. The executor fallback-recorder path sees `session.resumed`. The producer-owned delivery path for remote runners (`native-usage-delivery.ts`, `isNativeUsageAccountingEvent`) still drops it, so a remote ACPX/OpenCode turn restored mid-turn still waits for the closure timeout. That is a follow-up. - No schema or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
8b3348c6ba |
fix(runner): classify Claude's rejected selected model as an unavailable model (#20513)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runner agents on the Claude (ACPX) provider run with the
model the user saves on the agent
> - When the provider cannot run that model, Paperclip should classify
the run as `native_provider_model_rejected`, ask the user to choose a
model, and let Retry finish the task
> - Claude Code accepts an unknown model at session start and reports it
only on the first turn, as an AIR `request` failure with a fixed title.
The runner reported it as an unknown provider failure
> - So the run failed as `native_provider_terminal_failed`, and the task
stopped with `provider_effect_inventory_unavailable` ("Automatic
recovery stopped. Recorded work is preserved…") without naming the model
> - This pull request classifies that Claude title as an unavailable
model, the category the native runtime already turns into
`native_provider_model_rejected`
> - The benefit is that an unknown Claude model asks for a model choice
and names it, as Codex does after #20488
## Linked Issues or Issue Description
**What happened?**
Config wave `w-20261110T094430-s207-cf` on `master` (
|
||
|
|
57def75d01 |
fix(run-log): wait for a just-written journal head instead of failing the claim (#20509)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In cloud mode, run logs use the `s3_segments` store: an object byte journal whose `head.json` names the current writer > - Each run start (and each resume after a restart) claims the journal: it reads the head and publishes the next generation with a conditional write > - On an S3-compatible store with read-after-write lag, the head that the previous generation just published can read as `NoSuchKey` > - The claim then tries to create a new head, the store rejects the create because the head exists, and the run fails with `object_journal_writer_conflict` > - This pull request makes the claim wait for the head to become readable when its read said "missing" but the store says it exists > - The benefit is that a run that resumes right after a restart survives a store that shows new objects a few seconds late ## Linked Issues or Issue Description **What happened?** In restart-chaos wave w-20261110T085328-s701 (local, A1/C1, K1/K2, `--storage s3 --storage-faults ST7`), 3 of 4 tasks failed R3 with `retry_needed (adapter_failed)`. Every failed run has `failureCause: object_journal_writer_conflict` and the error "The output journal is owned by another writer" at `object-byte-journal.ts` `publish` from `object-run-log-store.ts` `begin`. All six failures happened in the ST7 window (GET/HEAD of a just-written key answers 404 `NoSuchKey` for 24 s), right after back-to-back K1 restarts. The proxy lagged 34 `GetObject` reads. **Expected behavior** A resuming run claims the run log and continues. A store that is slow to show a new object must not look like another writer. **Steps to reproduce** Claim a journal, then claim it again while the store answers `NoSuchKey` for `head.json` for a short time. The second claim fails with `ObjectJournalConflictError`. ## What Changed - `server/src/services/object-byte-journal.ts`: `claim()` reads the head and claims from it. When the head read as missing and the create fails as a conflict, it waits with backoff (250 ms doubling to 4 s, up to 60 s) and claims again from the head it then reads. A conflict on a head it could read, or a deletion tombstone, still fails at once. - `server/src/services/object-byte-journal.test.ts`: a new test hides a just-written head for three reads. Before this change the second claim fails with "The output journal is owned by another writer". After it, the claim gets the next generation and keeps the committed bytes. ## Verification - `cd server && npx vitest run src/services/object-byte-journal.test.ts src/services/object-run-log-store-tail-read.test.ts src/services/object-run-log-store.test.ts src/services/run-log-store-cancellation.test.ts src/services/run-log-store.test.ts`: 83 pass. The new test fails before the fix and passes after. - `cd server && npx tsc --noEmit -p tsconfig.json` passes. - Live: an ST7 rerun on a build with this fix follows in a comment. ## Risks - A claim on a journal that a truly concurrent first writer just created waits up to 60 s for that head and then claims after it. The caller already asserted execution authority, so it is the owner; it would claim after that head anyway once the head is readable. - A store that never shows the head makes the claim fail after 60 s instead of at once. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
dc9971bdb8 |
fix(native): keep another server's orphan sweep off a lease its owner took back (#20508)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an executor lease on its `native_run_finalizations` row. #20474 lets the same owner take back its own lease when the lease lapsed only because the host slept. > - That take-back is safe only if every other actor that acts on a lapsed lease records its action under the coordinator row lock first. The plan `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md` lists the actors that do not. Gap 2 is another server's orphan sweep. > - The sweep reads the lease as lapsed. Later it writes the ownership-unverified marker on the run, without the coordinator lock and without a new check of the lease. If the owner wakes and takes its lease back between the read and the write, a healthy run gets the marker. > - The board then shows "automatic recovery is blocked" on a run that works. After the next host sleep, the owner cannot take its lease back, so the turn is interrupted and the user must click Retry. The owner's rule is that a task interrupted by sleep must resume by itself. > - This pull request makes the sweep write its marker under the coordinator row lock, and only while no executor lease on the run is live. > - The benefit is that exactly one side wins: the sweep marks a run whose owner did not come back, and it leaves alone a run whose owner took its lease back. ## Linked Issues or Issue Description Refs #20474 **What happened?** Two servers share one database. Server A's host sleeps, so the executor lease of its native run lapses. Server B's orphan sweep reads the lapsed lease. Server A wakes and takes its lease back (#20474). Server B then writes `native_execution_ownership_unverified` on the run that A still owns. **What did you expect?** Server B writes the marker only if the lease is still lapsed when it writes. One side wins. **Steps to reproduce** `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` holds the owner's take-back open, runs the real `heartbeatService(db).reapOrphanedRuns()` until its write waits on a lock, then commits the take-back. Before this change the run ends with `errorCode: native_execution_ownership_unverified` and the owner's next take-back fails with `native_session_lease_lost`. ## What Changed - `setRunStatusFromLive` (`server/src/services/heartbeat/run-lifecycle.ts`) has a new option `onlyWhileNativeLeaseLapsed`. It locks the coordinator row first. This is the same order that `lockNativeExecutionLeaseForReacquire` uses. The UPDATE then requires that no executor lease on the run is live. - `markNativeOwnershipUnverified` (`server/src/services/heartbeat/recovery.ts`) takes `leaseLapsed`. The orphan sweep in `recoverActiveExecutions` sets it when it read the lease as lapsed or unset. Other callers do not change. They keep the same call shape. - A new regression test, red before the fix: `server/src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts`. It also checks that the sweep still marks a lapsed run whose owner did not come back, and that the late owner stays fenced out. - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: gap 2 is marked closed without the epoch design. ## Verification - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts` — red on `origin/master` (the run keeps the marker), green with this change. - `cd server && pnpm exec vitest run src/__tests__/heartbeat-native-orphan-sweep-lease-race.test.ts src/services/heartbeat/recovery.test.ts src/services/native-runtime/native-execution-lease-reacquire.test.ts src/__tests__/heartbeat-status-preserving-telemetry.test.ts` — 129 passed. - `cd server && pnpm exec tsc --noEmit -p tsconfig.json` — clean. - Restart-chaos time-fault wave on this branch: see the wave comment below. ## Risks - Low risk. Only the orphan sweep's marker write changes, and only for a run whose lease it read as lapsed. The sweep now takes the coordinator row lock before the run row. The owner's take-back uses the same order, so the two cannot deadlock. - If the owner took the lease back, the sweep now writes nothing and appends no event. The next sweep sees a live lease and defers, as before. - The other gaps in the plan (workspace finalization, shared-workspace holder detection, board copyback confirmation) do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
a09093a5c5 |
fix(native): resume a run whose turn.start a restart cut off before Codex created the turn (#20507)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native Codex run sends `turn.start` to its runner, and the runner sends `turn/start` to Codex. The runner marks the start "ambiguous" before it sends the request > - A control-plane restart can kill the runner and Codex while that request is in flight. After the restart, neither journal says whether Codex created the turn, and the resumed thread has no active turn > - The restored runner then failed closed on every same-run resume with "cannot safely recover an ambiguous Codex turn start without an active replacement turn". The answer never reached the agent > - The next run on the task was refused at the runner-state fence (`prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(turn.start.failed)`) before it asked for a runner. Retry after that run then waited forever on `process_identity_missing` for a process that never existed > - This pull request lets the runner prove from Codex's own thread history that the start never created a turn, and then resume and start the turn once. It also lets Retry continue after a run that the runner-state fence refused before launch > - The benefit is that a turn start cut off by a restart continues by itself when Codex never saw it, and a person's Retry is no longer a dead end ## Linked Issues or Issue Description **What happened?** Local restart-chaos wave `w-20261110T033426-s108`. Command: `--env local --adapters codex --workloads Q1,Q2 --faults K1 --triggers answered --tasks 4 --concurrency 2 --seed 108`. Task t03 (Q2): the answer run `92a57f50` resumed after the first restart. A second K1 restart landed 0.25 s after `session.resumed`, while its `turn.start` was in flight. Each same-run resume then failed with `PRP command session.snapshot failed: cannot safely recover an ambiguous Codex turn start without an active replacement turn`. When the prerequisite finished, the next run `0e6aa9d1` failed before launch with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(turn.start.failed)`. The accepted Retry (202) never created a run: the task stayed on `retryWait.reason=process_identity_missing` until the wave timed out (`R3:retry_no_new_run`). **Expected behavior** If Codex never created the turn, the run resumes and starts it once by itself. If that cannot be proven, a person's Retry starts a new run instead of waiting forever. **Steps to reproduce** 1. Run the wave command above on master. 2. Or run `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-cut-turn-start.test.ts` on master (case "before" fails with the wave error). ## What Changed - Runner (`codex_provider.rs`, `provider_backend.rs`): when the restored provider state has an ambiguous turn start, no active turn before the loss, and the resumed thread has no active turn, the runner reads the newest turn of the thread (`thread/turns/list`, limit 1, metadata only, the same request `read_thread` already sends). If there is no turn, or the newest turn is one Paperclip already settled, Codex never created the turn. The runner clears the ambiguity, emits a `harness.diagnostic` with code `ambiguous_turn_start_not_created`, and restores an idle session. The old `turn.start` stays `execution_indeterminate` and is never executed again. The same-run resume then starts the run's turn once, through the existing "recovered session without an active turn" path. - Any other history (a turn Paperclip never settled, an in-progress turn, an incomplete page, a non-Codex provider) keeps today's fail-closed error. The probe runs only on this recovery path. The normal turn path does no new work before `turn.start`. - Server (`native-local-process-stop.ts`): `nativeRunNeverLaunched` also accepts a failed native run whose error is the runner-state fence refusal `runner_state_identity_mismatch: prior_owner_*` (or `legacy_prior_owner_*`). The executor throws it only while it migrates another run's saved state, before it requests a launch. The existing evidence check still applies: any launch request, identity record, `adapter.invoke` or runner event keeps the run held. Other runner-state errors (for example `authority_indeterminate`) stay held. - Fake Codex (`fake-codex-app-server.rs`): keeps the thread's turn history in a `<state-file>.turns` sidecar, so `thread/turns/list` returns history like real Codex (newest first, honours `limit`). New `--hang-turn-start <file>` holds `turn/start` "before" or "after" the thread records the turn, to model a kill in flight. - Tests: - New real-runnerd test `runnerd-codex-transport-cut-turn-start.test.ts`. A real runner and fake Codex start a turn; both processes are killed while `turn/start` is in flight. "before": the resumed run completes, the journal holds the old `turn.start` (`execution_indeterminate`) and one new completed `turn.start` with the same task text, and Codex saw exactly one more `turn/start`. "after": recovery still fails closed and no new `turn/start` is sent. - Rust: `ambiguous_turn_start_that_never_created_a_turn_recovers_and_starts_once` and `ambiguous_turn_start_that_created_an_unsettled_turn_still_fails_closed`. Both rewrite saved provider state the way a master runner wrote it, so they also cover state saved before this change. - `explicit-native-continuation.test.ts`: five new rows for the runner-state fence. Admitted: refused before launch (with and without a coordinator). Held on `process_identity_missing`: refused after a launch request, refused after a runner event, and a run that failed its own `authority_indeterminate` check. - `docs/durable-recovery.md`: describes both behaviours. ## Verification - Red/green, real runnerd: on master (the staged master runnerd), `runnerd-codex-transport-cut-turn-start.test.ts` "before" fails with `PRP command runner.drain failed: ... cannot safely recover an ambiguous Codex turn start without an active replacement turn`, the wave error. With this change, both cases pass. - Red/green, Rust: without the `provider_backend.rs` change, `cargo test -p paperclip-runner-core --test codex_provider ambiguous_turn_start_that` fails the "never created" test. With it, both pass. The full `cargo test -p paperclip-runner-core` passes. - Red/green, server: without the `native-local-process-stop.ts` change, the two "refused at the prior-owner runner-state fence" rows fail with `process_identity_missing`. With it, all 14 rows pass. - Related suites pass: 11 Runner transport files, 419 tests (`runnerd-codex-transport`, `-attachment-recovery`, `-cold-restore`, `-warm-attachment`, `-release-resume`, `-cut-attach`, `-cut-turn-start`, `-settlement`, `-suspension`, `runnerd-restart-continuation`, `runnerd-saved-continuation`); server `explicit-native-continuation`, `quarantined-task-retry` and `issue-confirm-run-stopped-routes`, 3 files, 470 tests; full `cargo test -p paperclip-runner-core`. - `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit` and server `tsc --noEmit` pass. - Pack `contractDigest` is unchanged: `sha256:3164357d…` before and after (`scripts/contracts/contract-fingerprints.mjs`). - Live proof: the wave command above on this branch. Results follow in a comment. **No duplicate provider work.** The runner starts a new turn only when Codex's own history proves the cut-off start created nothing. The old command is never executed again (the runner refuses an `indeterminate` command). A recorded turn that Paperclip never settled keeps the run failed closed. ## Risks - Low-medium risk. The runner change runs only while it restores a session whose saved state has an ambiguous turn start and no active turn. Before this change, that run could never continue. - The proof relies on Codex recording a turn in the thread before it runs anything. If Codex accepted the request but died before it wrote the turn, the runner treats the start as not created and starts the turn once. Codex writes the user turn to the thread when it starts the turn, before it asks the model for output, so no model output or tool call is expected without a recorded turn. - If the thread's newest turn is not in the runner's settled set (for example after a provider identity epoch rollover forgot older identities), the runner keeps failing closed, as today. - **Open decision (not changed here):** when Codex did record the cut-off turn, it may have done work. This pull request keeps the run failed closed and does not replay or adopt that turn. Continuing is a person's decision through Retry or a new message. - The server change trusts the executor's own error text `runner_state_identity_mismatch: prior_owner_*`, together with the absence of every launch and runner event. A run that requested a launch keeps every stop proof. - **Runner release:** the pack `contractDigest` does not change. Local runners use the runnerd built with the app, so the fix applies with this release. - Not covered: the related `authority_indeterminate` variant in the same wave (t04, Q1) is a separate cause; its Retry already worked in that wave. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 Refs #20477, #20482 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
0b20ea6fe9 |
fix(accounting): keep budget admission and usage handoff alive under run-row updates (#20484)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Before a run starts provider work, Paperclip reserves its budget and binds a usage recorder. Both write the run's `heartbeat_runs` row. > - Run-log appends, output progress and lifecycle events update that same row while the run prepares and streams. > - #20385 fixed this race for the standalone usage-receipt write only. Budget admission and the recorder handoff still take a repeatable-read snapshot before they lock the run row. > - Each concurrent row update makes that snapshot stale. Four retries run out with SQLSTATE `40001`, and the run ends `adapter_failed` with raw SQL as its error. > - This pull request applies the #20385 pattern (READ COMMITTED under the existing locks) to those writers and to the final dispatch gate. It also gives exhausted retries a bounded cause. > - The benefit is that busy runs start and account normally, and a remaining failure names its cause. ## Linked Issues or Issue Description Refs #20385. Builds on #20483 (bounded failure cause and `withoutSqlStatementText`). **What happened?** The restart-chaos L1 workload with `--db-pressure 4` (harness writers that update each running run row, one row per statement, like a fast stream) failed on master. In one wave, 0 of 4 tasks passed. Every first run failed in `reserveRunBudget` before spawn. In an earlier wave, a run failed in `createRunUsageRecorder`. The stack in both cases is `could not serialize access due to concurrent update` (`40001`) on the run-row `FOR UPDATE`. The board `runError` showed the full `select ... from heartbeat_runs ... for update` statement. The server log line `heartbeat execution failed` dropped the database cause. Realism: the harness writers are synthetic, but the product has the same writers on the same row. `appendHeartbeatRunEvent` increments `next_event_seq`, output progress updates `last_output_*`, and lifecycle events land while a run prepares. On a hosted database the window between snapshot and row lock is wider than locally, so a busy run reaches the same race without the harness. **Expected behavior** Budget admission and the recorder handoff commit while other writers update the run row. Budget invariants stay intact: one reservation per run, a reached hard stop still refuses work, and committed holds count against later admissions. Usage receipts are journaled once. If retries still run out, the run shows a bounded cause without SQL, and the server log records the SQLSTATE. **Steps to reproduce** Run `server/src/services/run-accounting-contention.test.ts` against the parent commit. The deterministic cases commit a real `appendHeartbeatRunEvent` after the writer's transaction checks the company and before it locks the run. Six of the eight cases fail with `40001`. ## What Changed - `reserveRunBudget` runs at READ COMMITTED. All mutable reads follow the shared admission fences, the agent `FOR SHARE` lock and the run-row `FOR UPDATE`. Exposure stays one SQL statement (hold-to-ledger atomic). The `budget_reservations_run_idx` unique index and the run lock still prevent a second hold. - The final dispatch gate's accounting phase (`dispatchResolvedInteractionIfCurrent` through `withIssueThenRunLocks` in `run-dispatch/adapters/postgres.ts`) runs at READ COMMITTED. Its ownership phase already read the same staleness facts at READ COMMITTED under the same issue and run locks. Retry promotion and stale-run cancellation keep their repeatable-read snapshot. The first L1 proof wave found this writer exhausting `40001` once the other three were fixed. - `createRunUsageRecorder` runs its receipt-source handoff and its capture-failure fence at READ COMMITTED. Both lock the run row first, the same as `persistUsageReceipt` after #20385. - `withOperationAccountingTransaction` raises `AccountingContentionError` when retries run out. It has a bounded message, `code` keeps the SQLSTATE (`40001`/`40P01`), `cause` keeps the database error, and a warning logs both. - `failRun` shows that bounded message. The code stays `adapter_failed`, so chat and recovery retry paths still apply. #20483's `failureCause` records `40001`. All other SQL text handling comes from #20483 (`withoutSqlStatementText`), unchanged here. - `doc/DATABASE.md` documents the isolation choice and the failure contract. ### Inventory of REPEATABLE READ / SERIALIZABLE transactions that write run-scoped rows | Writer | Status | Notes | | --- | --- | --- | | `persistUsageReceipt` (`usage-receipts.ts`) | Fixed in #20385 | READ COMMITTED under the run lock. | | `reserveRunBudget` (`budget-reservations.ts`) | Fixed here | Seen in waves (`budget-reservations.ts:90`). | | `createRunUsageRecorder` handoff (`usage-receipts.ts`) | Fixed here | Seen in waves (`usage-receipts.ts:572`). | | `createRunUsageRecorder` capture-failure fence (`persistFailure`) | Fixed here | Same shape: run lock first, runs mid-stream. | | Native finalizer `claimCoordinator` (`native-run-finalizer.ts`) | Safe | A default READ COMMITTED transaction with `FOR UPDATE` on the coordinator row. It cannot raise `40001`. The wave errors there were `native_finalization_missing`, a symptom of the failed reservation (no coordinator existed yet). | | Final dispatch gate, accounting phase (`dispatchResolvedInteractionIfCurrent`) | Fixed here | Seen in proof wave `w-20261110T075857-s20386` (runs `140ddb10`, `c028c6c4`, `fea0ee5a`). Its ownership phase already used READ COMMITTED under the same locks with the same staleness reads. | | Retry promotion and stale-run cancellation (`promoteOrCancelDueRetry`, `cancelStaleQueuedRun`, same lock helper) | Follow-up | Kept at repeatable read: `loadGateFacts` combines wakes, comments, dependencies, tree holds and repair state in one decision. They act on queued or scheduled runs, not streaming ones. Not seen failing in waves. If retries run out, they now fail with the bounded cause. | | Run settlement `accountRunCost` (`run-cost-accounting.ts`) | Follow-up | Seen with `40001` in a wave log. Leaves the run pending; `reconcileRunCosts` retries it durably. Multi-aggregate (coordinator, receipts, images, closure). | | Tool cost events (`paperclip-runner-tool-authority.ts`, `costs.ts`) | Safe | Insert `cost_events` only; the run row is referenced, not locked or updated. | | Budget enforcement and evaluation (`budgets.ts`), billing reconciliation, accounting-integrity repair | Safe | Lock policy, invoice or company rows, not a streaming run row. Run reads are plain selects. | | Browser use, decisions, prompt-injection, fast responses | Safe | Write their own invocation/reservation rows, not `heartbeat_runs`. | | Read-only RR snapshots (`withAccountingReadSnapshot`, idle-sleep, tool gateway, customer success, `routes/decisions.ts`) | Safe | Read only, or no run-scoped writes. | ## Verification - Red on the parent commit, green on this branch: `server/src/services/run-accounting-contention.test.ts` (real embedded PostgreSQL). - `reserveRunBudget > reserves once when run events commit between its company check and its run lock` (red: `40001`) - `reserveRunBudget > reserves every run while back-to-back writers update the run row` (red: `40001`) - `reserveRunBudget > counts every committed hold, so successive admissions never exceed the budget` (red) - `reserveRunBudget > still refuses a run once the hard stop is reached` (red: `40001` instead of `409`) - `createRunUsageRecorder > binds its receipt source when run events commit between its company check and its run lock` (red: `40001`; green asserts the spooled receipt is journaled exactly once) - `createRunUsageRecorder > binds while back-to-back writers update the run row` (red: `40001`) - `reserveRunBudget > never creates a second reservation for one run when two dispatches race` (invariant; passes before and after) - `final dispatch gate > admits and hands off a reserved run when run events commit between its company check and its run lock` (red: `AccountingContentionError` after 4 × `40001`) - `createRunUsageRecorder > persists its capture-failure fence once when run events commit between its company check and its run lock` - `final dispatch gate > admits while back-to-back writers update the run row` (red: `40001`) - `exhausted retries > fail with a bounded, SQL-free cause that keeps the SQLSTATE` (new contract) - `run-completion.test.ts > saves a bounded cause and keeps the retryable generic code when accounting retries are exhausted` (saved code, message and `failureCause`, and log fields, through `failRun`) - Related suites pass: `usage-receipts-read`, `admission-independent-of-cleanup`, `budget-live-attempts`, `idle-accounting-work`, `budget-enforcement-scoping`, `closed-native-receipt-accounting`, `budget-evaluation-work`, `run-dispatch/adapters/postgres`, `heartbeat/run-completion` (all 126 run-completion cases pass; its `afterAll` embedded-PostgreSQL teardown exceeded 30 s on this loaded host, the known local flake). - `pnpm --filter @paperclipai/server typecheck` passes. - Latency of `reserveRunBudget` on the normal path (no contention, embedded PostgreSQL, 350 measured reservations per run, 3 runs each): before p50 14.7 / 15.0 / 18.6 ms, p95 25.6 / 31.4 / 39.4 ms; after p50 14.2 / 14.0 / 16.6 ms, p95 29.7 / 24.0 / 29.3 ms. No added latency: the statements are the same, only the isolation level changes. Under contention it removes the retry backoff. - Live restart-chaos L1 waves (`--seed 20386 --db-pressure 4`, codex and claude, K1): - Master before this change: `w-20261110T050818-s20386`, 0/4. Every first run failed in `reserveRunBudget` with `40001`. - Head `90530789b0` (first three writers fixed): `w-20261110T075857-s20386`, 2/4. Both codex tasks passed. Both claude failures (t02, t03) trace to the dispatch gate, now fixed. Those runs already showed the bounded cause, with no SQL on the board. The failure was logged with `sqlState` 40001. No `reserveRunBudget` or recorder `40001` remained. - Head `d5ed21926f` (dispatch gate fixed): `w-20261110T090531-s20386`. All 4 tasks reached done with no retry and R1–R6 passed. There were no failed runs and zero `40001` in the server log, under 310k pressure updates. Each task still failed the T-oracle `company_spend_settled` with `pending_runs=1`. That pending run is t02's continuation run `601ae80a`, cancelled before provider work as `issue_terminal_status`, whose accounting debt was not yet settled when graded about 15 s later. It is not a contention failure. Caveat: I edited the worktree during this wave (`worktree_changed_mid_wave`), so a clean wave on the final head follows. - Final head `9f1c480717`, clean worktree: `w-20261110T094417-s20386`. All 4 tasks reached done with no retry, and R1–R6 passed for each. There were no failed runs and zero serialization failures in the server log, under about 523k pressure updates. Each task still fails only the T-oracle `company_spend_settled` (`pending_runs=1`). It is the same cancelled pre-provider continuation run (`issue_terminal_status`), whose accounting debt was still unsettled at grading. That is a separate settlement-timing follow-up, not database contention. Master was 0/4 on the same seed. ## Risks - READ COMMITTED gives each statement its own snapshot. Admission reads policy, scope and exposure in separate statements. The shared company/project fences and the agent share lock exclude concurrent pause and policy edits, and exposure stays one statement, so each read is at least as fresh as the old snapshot. - Concurrent admissions of different runs stay best-effort, as `doc/SPEC-implementation.md` already states ("concurrent starts can exceed the same observed balance"). This change does not widen that window: each statement snapshot is newer than the old transaction snapshot. It does not add a company mutex. - A run that fails after exhausted retries now records `accounting_contention` instead of `adapter_failed`. Retry rules that key on `adapter_failed` spawn-like messages did not match this failure before, so no automatic retry changes. - No schema, migration or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
82abb3ea04 |
fix(native): classify a Codex API-key unknown model as a model rejection (#20488)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runner agents run on a provider (here Codex) with a model the
user saves on the agent
> - When the provider rejects that model, Paperclip should classify the
run as `native_provider_model_rejected`, ask the user to choose a model,
and let Retry finish the task
> - Paperclip recognized only the ChatGPT-account rejection (a 400 JSON
envelope). With API-key auth, Codex returns a different terminal error
for an unknown model
> - So the run failed as a generic `adapter_failed` ("Native session
failed"), and recovery opened a reconciliation incident ("Recovery
needed") for a turn that never did any work
> - This pull request recognizes the real API-key shape, bound to the
configured model, and names the rejected model on the task
> - The benefit is that the task asks for a model choice instead of a
reconciliation, and Retry completes the task after the user picks a
valid model
## Linked Issues or Issue Description
**What happened?**
The restart-chaos config fault CF3 `model_id` saves a Codex agent with
the model `chaos-missing-model-<nonce>`. The run stays "running" for
about 60 seconds, then fails with `errorCode: adapter_failed`, "Native
session failed". The task shows "Recovery needed", cause
`native_continuation_requires_reconciliation`. The grader fails as
`CF3/model_id:actionable_message` (waves `w-20261110T061914-s206-cf` t01
and `w-20261110T053831-s205-cf` t02).
Root cause: the wave runs Codex with an OpenAI API key. I drove the
bundled `codex app-server` (0.160.0) directly with an unknown model:
- With API-key auth, Codex retries the Responses API 404 ten times
(about 59 s), then fails the turn with `{"message":"unexpected status
404 Not Found: The model \`<id>\` does not exist or you do not have
access to it., url: https://api.openai.com/v1/responses, cf-ray: …,
request id:
req_…","codexErrorInfo":{"httpConnectionFailed":{"httpStatusCode":404}}}`.
- With ChatGPT auth, the turn fails in about 3 s with `message` =
`{"type":"error","status":400,"error":{"type":"invalid_request_error","message":"The
'<id>' model is not supported when using Codex with a ChatGPT
account.","code":"model_not_found"}}`. Paperclip already classified this
one.
`createNativeProviderFailureObservation` parsed only the JSON envelope,
and the runner's provider-failure category for an HTTP 404 is `unknown`.
The terminal therefore stayed unclassified.
**Expected behavior**
The run is `native_provider_model_rejected`, the task shows `userAction:
"choose_model"` and names the model, no reconciliation incident opens,
and Retry completes the task after the user picks a valid model.
**Steps to reproduce**
`node cli/node_modules/tsx/dist/cli.mjs
tests/restart-chaos/config-wave.ts --faults CF3 --adapters codex
--references model_id --tasks 1 --concurrency 1 --restarts K2 --seed
206` on `master` (with `OPENAI_API_KEY` set).
**Paperclip version or commit**
`master` at
|
||
|
|
8a8abdb7b9 |
fix(recovery): retire the ledger row of an abandoned workspace save (#20502)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Remote workspaces save their files to object storage. Each save
writes a row in the `execution_workspace_snapshots` ledger. The row
stays `pending` until the upload publishes it.
> - A restart that kills the server during a save leaves the save
unfinished. The ownership reconciler settles the save operation as
`remote_workspace_save_abandoned`. It does not touch the ledger row.
> - The row then stays `pending` for good, and its `updated_at` never
moves. The `remote_snapshot_ledger` ownership kind has no settle path,
so every later ownership report flags it as an `I2_settled` violation.
> - This pull request retires that row to a terminal state once no retry
can reopen its save, and gives `remote_snapshot_ledger` a settle path.
> - The benefit is a clean ownership report after a restart during a
save, and a ledger that tells the truth about uploads that will never
finish.
## Linked Issues or Issue Description
**What happened?**
A Daytona R1 × K1 restart-chaos wave (`w-20261110T071047-s7101`,
`--storage s3`) killed the server during the initial workspace save of 3
runs. The save operations ended `failed` /
`remote_workspace_save_abandoned`. Their `execution_workspace_snapshots`
rows stayed `pending` permanently, with no object key and no upload
attempt. The workspaces later committed newer snapshots (authority epoch
3, row epoch 1). Every ownership report afterwards flagged 3
`I2_settled` violations of kind `remote_snapshot_ledger`.
**Expected behavior**
When a save is abandoned and its run and writer epoch have ended, its
pending ledger row reaches a terminal state, and the ownership report
shows no `I2_settled` violation for it.
**Steps to reproduce**
1. Run a Daytona R1 × K1 wave with `--storage s3` and K1 at about +65 s,
so the restart lands inside the initial remote workspace save.
2. Let the runs finish or fail and the workspace commit a newer
snapshot.
3. Read `execution_workspace_snapshots`: the abandoned rows are still
`pending`. The server log shows `I2_settled` violations for
`remote_snapshot_ledger` on each report.
**Paperclip version or commit**
master at
|
||
|
|
1402e92d1f |
fix(native): take back an executor lease that lapsed only because the host slept (#20474)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run holds an execution lease on its `native_run_finalizations` row. The lease lasts 20 minutes, and the owner renews it every 5 minutes. > - When a laptop sleeps for longer than the lease, the wall clock and the database clock move past `lease_expires_at`. The server's monotonic timers barely move. > - After the wake, the next renewal finds its own lease expired. It interrupts a healthy turn with `native_session_lease_lost`, and the user must click Retry. Nobody else took the lease. > - Several background consumers act on an expired lease without changing its owner. So expiry alone cannot simply be ignored for the owner. > - This pull request makes takeover explicit. The same owner can take back its own lapsed lease only when no other actor has recorded anything on the row or the run. > - The benefit is that a run interrupted only by host sleep continues with no Retry. A real takeover still fences the late owner out. ## Linked Issues or Issue Description **What happened?** The restart-chaos time fault TM2 stops the server and runner tree and ages every database time by one hour, as a stand-in for a laptop that slept. After the thaw, the next 5-minute renewal of the native execution lease found the lease expired. It interrupted the turn (`turn.interrupted` at 300 s, `native_session_lease_lost`). The failure settlement was then refused with the same code, and the UI showed "Couldn't finish this attempt" with Retry. The owner, attempt, boot and pid were unchanged. Evidence: wave `w-20261110T051535-s304` t01 (TM2, 1 h) failed R3. t02 in the same wave (TM1, a 113 s server-only freeze with no clock jump) passed, so the failure follows the clock jump and not the pause. **Is this a harness artifact?** No. TM2 ages the stored rows, and a real sleep moves the clock forward. Both produce the same comparison: a stored `lease_expires_at` is earlier than `clock_timestamp()` after the wake. The server's renewal timer and local deadline use the monotonic clock. On macOS and Linux that clock does not count suspended time, so they do not notice the sleep in either case. The first database check after the wake is the renewal predicate `lease_expires_at > clock_timestamp()`, and it fails in both cases. **Expected behavior** A run whose lease lapsed only because time jumped, and that no other actor claimed, keeps running and finishes without Retry. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --adapters codex --workloads B5 --faults TM2 --time-age-hours 1 --concurrency 1 --tasks 2 --seed 304` with the harness from #20458. Refs #20458, #20500 ## What Changed - `native-execution-lease.ts`: add `nativeExecutionLeaseHeldCondition` and `lockNativeExecutionLeaseForReacquire`. A lease is held when it is live. For a local run, it is also held when it lapsed and is untouched. "Untouched" means all of these are true: - the claimed generation matches; - the phase is `observed` and no result is accepted; - `recovery_state`, `recovery_request_id`, `failure_code` and `next_attempt_at` are null; - `controller_host_hash` is not the board's attested-stopped stamp; - the run is still `running`, without the ownership-unverified error code and without a `nativeStartupCleanup` marker. - The caller's WHERE already matches the owner token, attempt and controller boot/pid/start. - The lock helper locks the coordinator row. If the lease lapsed, it also takes a share lock on the run row. The predicate then reads the latest committed run, and an in-flight run write cannot slip past it. - Renewal (`startNativeSessionExecutionLeaseRenewal` / `renewNativeSessionExecutionLease`) takes back a lapsed lease when the executor passes its claimed generation. Remote targets never do. A deadlock (40P01) on that rare path retries at the next tick, as a statement timeout already does. - `native-session-executor.ts`: the executor passes its generation to renewal. Failure settlement and the final release use the same lock and condition. After a result is accepted, a lapsed lease can only be relinquished (set to null), never renewed, because workspace finalization may act on it. - New `native-execution-lease-reacquire.test.ts` (embedded Postgres, 24 cases). - `doc/plans/2026-11-09-explicit-native-lease-claim-tokens.md`: step 2, which adds explicit claim tokens for the consumers this PR does not yet move. Consumer inventory (readers of `native_run_finalizations.lease_expires_at` that can act on a lapsed lease): - Restart recovery and takeover (`claimNativeRestartRecoveries`): claims first. It writes `recovery_state`, `lease_owner` or `controller_*` under the coordinator lock. - Finalizer and cleanup claims: claim first (`lease_owner`). - Failed-run owner release, environment settlement, observed-workspace preservation, retained-session cleanup: require a terminal run status, `committed`, or a recovery state. The rule refuses all of these. - Finalization reconciler, live recovery: require `retryable_failure`, a field the rule checks. - Board controller-stopped confirmation: writes `controller_host_hash`, which the rule checks. - Startup cleanup promotion: writes the run's `nativeStartupCleanup` marker, which the rule checks. It also applies only to runner-task leases. - Another server's orphan sweep: writes the run's ownership-unverified code. The rule checks it when the marker lands first. The reverse order is deferred (see Risks). - Workspace finalization copyback: acts on a lapsed post-result lease without a coordinator write. Post-result leases are therefore never renewed (deferred to step 2). - Accepted-result and token lease settlement, checkpoints, cancellation delivery, admission and the other readers: act on live leases only, or are read-only. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-execution-lease-reacquire.test.ts` passes (24/24) on this branch. With the source files from `origin/master`, 5 fail, including "lets the same owner take back a lease that lapsed untouched" and "renews from the shared timer after the sleep instead of interrupting the turn". - (a) The owner takes back its lapsed lease when no other actor claimed it. Also covered: the shared 5-minute timer, rows saved by the previous release, and the post-result relinquish. - (b) 14 cases where a consumer claimed first fence the owner out and leave the row unchanged. A release over a recorded recovery claim is refused. A different boot (a deploy mid-run), a different generation, a missing generation and a remote target are refused. - (c) Races have exactly one winner: a claimant and the owner lock the coordinator in each order, an in-flight run status write races the owner, and 8 simultaneous starts each have one winner. - Existing suites pass: `native-execution-lease.test.ts`, `native-session-executor.test.ts`, `native-runner-restart-recovery`, `native-runner-restart-lease-renewal`, `native-runner-restart-preparation` and `native-runner-restart-checkpoint-cost` integration tests. - Live proof: `B5 × TM2 --time-age-hours 1 --tasks 2 --seed 304` with `--server-from` this branch. Wave `w-20261110T083444-s304` passed 2/2, with no Retry and no `native_session_lease_lost`. - Both runs ran past the 300 s renewal after the one-hour aging, at the exact point where `origin/master` fails. t01 ran 310 s and t02 ran 319 s. Each was a single run that succeeded. - The harness used the aging exclusion from #20500 (merged): process start identity is not aged. Without that exclusion, TM2 also moves `controller_process_started_at`. The live owner then fails its identity check, which a real sleep never causes. An earlier wave on the unpatched harness, `w-20261110T072106-s304`, failed 0/2 for that reason only. ## Risks - Previous release: no persisted shape changes and no migration. Rows saved by the previous release have the same columns, and a null `controller_host_hash` is accepted. A deploy that lands mid-run cannot take back the old server's lease, because the boot and pid differ. - On the normal path, a live lease takes the same single coordinator lock as before. The run share lock is taken only after a lapse, and nothing is added before `turn.start`. - Deferred to step 2 (see the plan): - The orphan sweep of another server process can read the row before the owner takes the lease back and write after. That run would carry an ownership-unverified error while it is live. - Shared-workspace holder detection reads a silent run with lapsed leases as gone. - Post-result leases are not renewed. - Remote leases are unchanged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used in Claude Code with tool use (shell, file editing, sub-agent code audit). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
a14ae9b5c9 |
fix(workspaces): name a failed workspace setup command on the task page (#20478)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before an agent starts, Paperclip prepares the task workspace and runs the project's configured setup commands (for example the workspace provision command) > - When that command fails, the run fails as `setup_failed`, and the task stops with a Retry > - The task page shows only "Failed" and a generic recovery notice. The real error (the command, its exit status and output) is visible only on the run page > - The user cannot see from the task which setting is broken or what to change > - This pull request records a bounded, named cause for a failed setup command and shows it on the task page > - The benefit is that the task names the broken setting and what to change, and classification, retry and recovery stay the same ## Linked Issues or Issue Description **What happened?** The restart-chaos config fault wave CF1 sets a project `workspaceStrategy.provisionCommand` to `bash ./scripts/provision-worktree.sh`, a script that does not exist. The run fails as `setup_failed` with `Execution workspace provision command "bash ./scripts/provision-worktree.sh" failed: bash: ./scripts/provision-worktree.sh: No such file or directory`. The task page shows "Failed" and "Paperclip automatically retried continuation … still has no live execution path". The grader fails the task as `CF1/provision_command:message_only_in_run_detail`. **Expected behavior** The task page names the failed setup step, the configured command, its exit status and a short tail of its output, and says what to change. Retry behavior stays the same. **Steps to reproduce** `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/config-wave.ts --faults CF1 --adapters codex --tasks 2 --concurrency 2 --restarts K1,K2 --seed 204` on `master`. **Paperclip version or commit** `master` at |
||
|
|
52b906b112 |
fix(native): let Retry continue after a run that failed before native selection (#20482)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before Paperclip starts a new message or a Retry after a failed native run, it proves that the run's local process stopped > - A native run can fail before native selection, for example when a budget reservation is refused or a preparation query fails. That run never had a runner, a native session or a process ID > - The stop gate still waited for a stop record of that process. Retry returned 202, then the task stayed on `retryWait.reason=process_identity_missing` and no run started > - Native selection writes the run's native issue, native session, runner instance and coordinator in one transaction. A runner launch needs all of them, and the executor records `native.process_start_requested` before any spawn > - This pull request treats a settled run with none of that evidence as having nothing to stop > - The benefit is that Retry and new messages continue after a failure that happened before the agent started ## Linked Issues or Issue Description **What happened?** In local restart-chaos database-load wave `w-20261110T050818-s20386`, all 4 tasks failed before any agent process started. One example is t01, run `f7e2ee55`. A preparation query failed, and the run ended `adapter_failed` with no native session, no runner instance, no process ID and no launch event. Retry returned 202. For 16 or more minutes, `retryWait` then stayed on `process_identity_missing` and no run started. **Expected behavior** If the previous run never had a process, there is nothing to verify, so Retry and new input continue. **Steps to reproduce** 1. Make a native run fail before native selection (for example, a refused budget reservation or a failed query). 2. Click Retry, or send a message. Before this change, the input waits on `process_identity_missing` and does not continue. ## What Changed - `native-local-process-stop.ts`: `nativeRunRefusedBeforeLaunch` (renamed `nativeRunNeverLaunched`) now also proves "never launched" for a settled native run in this state: - no process ID and no process group; - no native issue, no native session and no runner instance (native selection writes them together). A coordinator row requires the native issue by foreign key, so the native-issue check also rules out a coordinator; - no launch request, no identity record, no `adapter.invoke` and no runner-authored event (any event with a source event ID). The existing case, a run that the cleanup quarantine refused before launch, does not change. - `explicit-native-continuation.test.ts`: the guard "failed for another reason without a process record" now separates real pre-selection failures from the shapes that must stay held. - Admitted: a run that failed before it requested a launch. This case is red on `master`. - Still held on `process_identity_missing`: a run that has a runner event, a run with a prepared native session, and a run with a coordinator, for `failed`, `timed_out` and `interrupted` runs. A `timed_out` run that failed before launch is admitted. The chat suite's `unreconciled` case (a native issue and phase, no session) also stays held. ## Verification - Red/green: on `master`, `routes input after a run failed before it requested a launch` fails with `process_identity_missing`. It passes with this change. - `vitest run src/services/explicit-native-continuation.test.ts src/services/quarantined-task-retry.test.ts src/__tests__/issue-confirm-run-stopped-routes.test.ts`: 3 files, 462 tests pass. This includes every existing "keeps the hold for missing_process" and historical-run case. - Server `tsc --noEmit` passes. ## Risks - Low risk. The new proof needs structural evidence that no runner could exist: no native selection and no launch or runner event. A run that reached native selection, or that has any launch evidence, keeps every existing stop proof. - The proof does not depend on the build: a runner from any build needs the native session and the runner instance. Saved runs from older builds that were selected keep those fields, so they stay held exactly as before. - Relates to #20477, which fixes the restart window inside an answer run's attachment. This pull request is independent of it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 Refs #19829 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
1e13c58a4e |
fix(heartbeat): name the cause of generic adapter failures and keep SQL text off the board (#20483)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Heartbeat run settlement records why a run failed, and the board shows that record. > - Runs that fail without a named code settle as `adapter_failed` (or `setup_failed`) with no cause. > - A user, an operator, and the restart chaos harness cannot tell what went wrong from such a run. > - Some run errors also show raw SQL statement text to users. > - This pull request records a bounded `failureCause` code for generic failures, shows it on the board, and keeps SQL text out of run errors. > - The benefit is that a generic failure now names its cause, for example "adapter failed (40001)", without leaking messages, SQL, or secrets. ## Linked Issues or Issue Description Refs #20385 (the nested-cause extraction this change reuses). Related: #20421 (restart chaos universal checks U1-U4). **What happened?** The restart chaos audit of 79 saved waves found 30 failed runs with `errorCode: "adapter_failed"` and a null or generic error. Every U3 ("diagnosable") warning was `adapter_failed`. Nobody could tell what failed. Some board run errors also showed raw SQL statement text from wrapped database errors (Drizzle `Failed query: ... params: ...`). **Expected behavior** A run that fails with a generic code names a short, safe cause. The board shows the cause next to the stop reason. Run error text never shows SQL statements or their parameters. **Steps to reproduce** Make an adapter throw a wrapped database error (an `Error` whose nested `cause` has `code: "40001"` and whose message has a Drizzle `Failed query:` statement). Before this change, the run settles as `adapter_failed` with no cause, and `heartbeat_runs.error` contains the SQL statement and parameters. The new tests in `server/src/services/heartbeat/run-completion.test.ts` reproduce this. ## What Changed - Runner: move the #20385 event-commit cause extraction into `safeErrorCauseCode` (`packages/paperclip-runner/src/control-plane/safe-error-cause.ts`) and export it. The Runner's own diagnostics use the same function, unchanged in behavior. - Shared: add the `failureCause` contract (`packages/shared/src/run-failure-cause.ts`): one `[A-Za-z0-9_.:-]` token of at most 80 characters, `normalizeRunFailureCause`, `readRunFailureCause`, and `withoutSqlStatementText`. - Server: add `server/src/services/heartbeat/run-failure-cause.ts`. Settlement records `resultJson.failureCause` only when the code is generic (`adapter_failed`, `setup_failed`). `errorCode` stays the same. - Thrown errors (`failRun`, `failRunSetup`): `provider_http_<status>`, else `provider_<category>`, else the first safe code in the error and up to three nested causes (`40001`, `ECONNRESET`), else the error class name, else `unknown_error`. - Adapter results without a code (`completeRun`): the error family, `signal_<name>`, `process_cancellation_failed`, `exit_code_<n>`, `adapter_reported_error`, or `unknown_error`. - The failure log lines (`heartbeat execution failed`, `heartbeat execution setup failed`, and a new `heartbeat run failed` warning for generic adapter results) include `failureCause` and `errorCode`. - Server: replace SQL statement text in run error text with "Database query failed (<cause>)" at all three settlement paths, including the cancelled branch. The execution blocker's `runError`, the attention inbox (failed-run excerpt and metadata), and continuation summaries apply the same replacement on read, so older rows also stop showing SQL there. - Server: Retry admission and failed-preparation replay accept only a closed set of `resultJson` keys. `failureCause` is added to each of those lists (`task-retry.ts`, `chat-channels/failed-run-retry.ts`, `cancelled-native-startup.ts`, `fresh-workspace-continuation.ts`). It is a diagnostic code with no execution or replay authority, so a failed-preparation run stays retryable. - Server: `runsForIssue` returns a validated top-level `failureCause`. It is null for older rows, non-string JSON values, and malformed values. - UI: `RunForIssue.failureCause`; `IssueRunLedger` shows "adapter failed (40001)". Older runs keep "adapter failed". The Storybook run-ledger fixture has a failed run with a cause. - Restart chaos harness: run samples keep `failureCause`. U3 treats a generic code with a specific `failureCause` as diagnosable. A bare `Error`, `unknown_error`, or the no-code fallback `adapter_reported_error` still counts as vague. U3 stays a warning. - Docs: `doc/run-log-events.md` and the harness README describe the field and the SQL replacement. ## Verification - Red before the fix: the four new `run-completion.test.ts` cases failed against `origin/master`'s `run-completion.ts` (no `failureCause`, SQL text in `error`). - Green: - `server/src/services/heartbeat/run-completion.test.ts`: all tests passed (real embedded PostgreSQL). New cases cover a nested `40001` three levels deep for execution and setup failures, SQL and a secret parameter absent from `error` and `resultJson`, the bound (an 81-character code and a SQL "code" fall back to the class name), `exit_code_137` for a code-less adapter result, a cancelled result with SQL text, no cause for a named code, and the log line. On this loaded host the suite's `afterAll` database teardown hit its 30 s hook limit after every test passed (the known local teardown limit, also seen in #20385). CI runs the suite. - `server/src/services/heartbeat/run-failure-cause.test.ts` (4), `server/src/__tests__/activity-service.test.ts` (recorded, legacy, malformed, overlong, boolean, and number cause projections), `server/src/__tests__/issue-continuation-summary.test.ts` (legacy SQL row). - `packages/shared/src/run-failure-cause.test.ts` (6), `ui/src/components/IssueRunLedger.test.tsx` (21), restart chaos `universal.test.ts` and `observer.test.ts`. - `packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts`: 207 passed (the stall-diagnostic cases still read `40001` and `ECONNRESET`). - Typecheck: shared, runner, server (all layers), UI, restart chaos harness. `pnpm check:token-gates`: all gates clean. - `pnpm verify` returned exit 2 (global doc input changed); required CI supplies broad coverage. - CI on `6295f8d` found that the new key made failed-preparation runs ineligible for Retry (`chat-channels.integration.test.ts` saved board-message Retry got 409; `workspace-recovery-preparation-dispatch.test.ts` dispatched 0 Retries). Both reproduced locally and pass after the allowlist fix, with the other 459 Retry cases in `chat-channels.integration.test.ts` and all 25 in `workspace-recovery-preparation-dispatch.test.ts`. - No chaos waves were run. Storybook was not opened in a browser; the fixture typechecks and the label is covered by the UI test. ## Risks - Low risk. `errorCode` values do not change, so retry, recovery, and comparison logic is unaffected. - No schema change and no migration. `failureCause` is a new optional `resultJson` key. Rows saved by the previous release have no key and render as before; a new row read by an old server is ignored. - The SQL replacement only acts on a Drizzle `Failed query:` marker, or on `$n` placeholder text with a statement shape. Ordinary error text does not change. When it acts, it also drops the text after the statement. - Code that checks `resultJson` against a closed key list must allow `failureCause`. I found and updated the four lists on the Retry path; a list I missed would refuse a Retry for a run with a generic failure. - This is the run log / instance database path. It adds no Telemetry event. - The server failure log line still logs the raw error message (unchanged behavior); only the board-facing text is changed. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file editing) and code execution for tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
53c9be8d41 |
Reuse identical captured workspace archives (#20476)
Reuse the current committed snapshot when a fresh capture matches its manifest and plaintext archive hash and size. Preserve the original encryption binding and verify each new run receipt for recovery, concurrent replay and storage request statistics. Verified with 338 focused save and recovery tests, server typecheck and build, all final-head CI checks, and Greptile 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
233f615644 |
fix(heartbeat): resume legacy runs a graceful shutdown stopped before dispatch (#20461)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The heartbeat service runs agent turns and recovers them after a
server restart
> - A graceful restart can stop a legacy run (for example a `process`
adapter agent) while the run is still in setup, before the adapter
starts
> - That run was parked with `legacy_execution_requires_reconciliation`,
so automatic recovery stopped
> - A `process` or `http` adapter cannot continue that hold through
Retry, so the accepted Retry waited forever and the task stayed blocked
> - This pull request proves "no adapter work started" for that case and
resumes the run automatically
> - The benefit is that a restart during setup no longer strands the
task, and Retry starts a run for holds that an older server saved
## Linked Issues or Issue Description
**What happened?**
The restart-chaos harness ran workload Q3 (a board approves an agent's
app action late) with a `process` adapter agent. A graceful restart
(fault K2) fired right after the approval. The agent's follow-up run was
interrupted with `server_shutdown_interrupted` 0.2 s after it started,
while it was still in setup. Automatic recovery stopped with
`legacy_execution_requires_reconciliation`. The board clicked Retry. The
server accepted the Retry (HTTP 202), but no run started. The task
showed "Retry is waiting for execution checks" (`retryWait.reason =
execution_recovery`) forever.
**Expected behavior**
A run that a graceful shutdown stops before any adapter work must resume
automatically. An accepted Retry must start a run.
**Steps to reproduce**
1. Run a `process` adapter agent on a task.
2. Send SIGTERM to the server while the run is still in setup (before
the adapter starts).
3. Restart the server. The task is blocked with
`legacy_execution_requires_reconciliation`.
4. Click Retry. The request is accepted, but no run starts.
**Paperclip version or commit**
master at
|
||
|
|
693de27c90 |
fix(storage): wait out expired or rotated S3 credentials and re-resolve the key (#20456)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In cloud mode, run logs, provider traces, attachments and remote workspace snapshots go to S3 > - The server uses the default AWS credential chain, and real keys expire or rotate while runs are in flight > - #20299 made runs wait out a storage outage, but a 403 `ExpiredToken`, `InvalidAccessKeyId` or `SignatureDoesNotMatch` was a permanent `adapter_failed` > - The SDK also caches a key until its stated expiration (forever for a key with none), so a rotated key was never picked up > - This pull request treats credential rejections as a storage wait and re-resolves the key from the credential chain after a rejection > - The benefit is that runs survive key expiry and rotation without failed attempts or a manual Retry ## Linked Issues or Issue Description Refs #20299 (storage outage wait). Harness that found this: #20455. **What existing behavior does this improve?** Runs wait out S3 outages (5xx, timeouts, resets) but fail at once when the store rejects the server's credentials. **Subsystem affected** server storage (S3 provider) and heartbeat run completion. **Current behavior** Restart-chaos wave w-20261110T041556-s501 (`--storage s3 --storage-faults ST5`): one 41 s window of 403 `ExpiredToken` failed 4 of 6 tasks. Each run ended `adapter_failed` with "The provided token has expired.", and the automatic retries failed within one second, so the tasks needed a board Retry (R3 `retry_did_not_complete`). **Proposed behavior** A credential rejection is a transient infrastructure condition. Critical-path storage calls wait it out like an outage, and a run that stops on it gets the `storage_unavailable` wait retry. After each rejection the S3 client re-resolves its key with `forceRefresh`, so the next attempt signs with a rotated key. The run error names the credentials. If they never come back, the exhausted-wait message tells the operator to check the storage service and its credentials. **Reason and benefit** Expiring or rotated keys are normal in cloud deployments. They must not fail work. ## What Changed - `storage-unavailable.ts`: `ExpiredToken`, `ExpiredTokenException`, `TokenRefreshRequired`, `InvalidToken`, `InvalidAccessKeyId`, `InvalidClientTokenId` and `SignatureDoesNotMatch` answered by the store with 400/403 count as storage unavailable. `AccessDenied` and a credential chain that cannot load any key (`CredentialsProviderError`) stay permanent. New credential-specific run message; the exhausted message also names credentials. - `s3-credential-refresh.ts` (new): S3 client middleware that calls the client's credential provider with `forceRefresh` after a rejection, once per credential generation (requests that signed with an already-replaced key do not refresh again), then rethrows so the existing storage wait owns the retry. - `s3-provider.ts`: installs the middleware on both S3 clients. - `run-completion.ts`: uses the credential-aware message. ## Verification - `cd server && npx vitest run src/__tests__/storage-s3-credential-rotation.test.ts src/storage/ src/__tests__/storage-outage-wait.test.ts src/__tests__/storage-s3-transient-retry.test.ts src/__tests__/storage-s3-failure-diagnostics.test.ts src/services/heartbeat/run-completion.test.ts` (186 pass). - The new `storage-s3-credential-rotation.test.ts` fails 5 of 6 cases on master (red) and passes with this change (green). It covers a real key rotation through the default credential chain, 403 windows for each code, the message, and `AccessDenied` staying permanent. - `cd server && npx tsc --noEmit` passes. - `s3-credential-refresh.test.ts`: two parallel requests signed with the old key, rejected 50 ms and 400 ms apart, cause one forced lookup (3 before the generation check, 2 after). - Restart-chaos rerun on a build with this fix and #20455: w-20261110T050044-s503 (`--env local --workloads A1,C1 --faults K1,K2 --tasks 8 --storage s3 --storage-faults ST5,ST5R --storage-fault-seconds 30-90`) passed 8/8 tasks (R1–R6 and T) with a 39 s `ExpiredToken` window and an 85 s real key rotation (76 credential 403s). Before the fix, w-20261110T041556-s501 failed 4/6 tasks. ## Risks - Scope: calls with a Readable body (streamed `putObject`, buffered multipart) cannot be resent, so they still fail on the first rejection, as they do for outages. The middleware has already re-resolved the key, so the caller's next attempt (the run-level wait retry for run work) uses the new key. - A key that the store rejects for good (for example a wrong secret) now waits up to the storage wait budget (5 min in the call, then up to 10 run retries) before the board shows the exhausted message, instead of failing at once. - `forceRefresh` re-runs the credential chain after a rejection. Calls are coalesced, so a burst of rejections causes one refresh. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
8809f44b5e | Connect Codex and Claude login to agent computers (#20417) | ||
|
|
120017e39d |
fix(workspaces): read saved workspace files in place instead of extracting the whole archive (#20442)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a cloud task finishes, its workspace is saved as an encrypted
snapshot and its sandbox is released. The board's Files view then reads
that saved copy
> - Saved Files access downloaded, decrypted and extracted the whole
archive for every request, and refused archives over 512 MiB with
`too_large`
> - A finished task in a Paperclip-sized repository saves a snapshot of
about 730 MiB, so its files could not be listed or opened at all
> - The archive format already records each file's size and SHA-256, and
the snapshot records the manifest digest, so single files can be read
and verified without the rest of the archive
> - This pull request reads the saved archive in place, one verified
file at a time
> - The benefit is that users can open any file of a large finished
workspace quickly, with bounded memory and no whole-archive transfer
## Linked Issues or Issue Description
**What happened?**
In restart-chaos waves on Daytona with a real Paperclip clone, the
committed workspace snapshots were 763,944,535 bytes. After the sandbox
was released, every saved Files read returned 422 `too_large` (506 reads
in total), because `createSavedWorkspaceLoader` refuses archives over
`maxArchiveBytes` (512 MiB). The board Files view uses the same loader,
so it fails the same way.
**Expected behavior**
A user can list and open any single file of a saved workspace of
Paperclip-repository size without the server extracting or buffering the
whole archive. Memory and time stay bounded per request, and integrity
is still verified.
**Steps to reproduce**
1. Run a cloud task whose workspace is a large repository, so the final
snapshot exceeds 512 MiB.
2. Let the task finish and its sandbox be released.
3. Open the task's Files view, or call `GET
/api/issues/:id/file-resources/content?workspace=execution&path=…`: it
returns 422 `too_large`.
**Paperclip version or commit**
`master` at
|
||
|
|
d064bd21e4 |
perf(plugins): pick each lifecycle resource's head in one pass (#20406)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins receive resource lifecycle events (agent, project and agent-runtime changes) through a durable inbox > - The inbox lists an event only when it is the first unacknowledged event of its resource, so each resource delivers in order > - The list query checked that rule with a `NOT EXISTS` for every unacknowledged event. Each check walked the resource's whole acknowledged history > - A resource with a long acknowledged history and a backlog behind a deferred head costs history × backlog probes. On the hosted staging instance three copies of this statement ran for more than 23 minutes each, on a database that was at 6–8 of 8 vCPU all day > - This pull request picks the deliverable head of each resource in one pass > - The benefit is a list call that is linear in the company's events, and database CPU that other requests can use ## Linked Issues or Issue Description **What existing behavior does this improve?** `pluginLifecycleInbox(...).list()` in `server/src/services/plugin-lifecycle-inbox.ts`. The delivery worker calls it in a loop and when it checks for idle work. **Subsystem affected** Plugin lifecycle delivery (server service, PostgreSQL query). **Current behavior** Neon "Active queries" for the staging database on 2026-11-09 showed three copies of this statement at 1,381–1,401 s each. In the same view, trivial primary-key reads waited 100–280 ms. Locally, one resource with 2,000 acknowledged events and a 2,000-event backlog takes 3.5 s for one `list()` call. The cost grows with acknowledged history × backlog. **Proposed behavior** Only the head of a resource is deliverable: its unacknowledged creation if it has one, otherwise its lowest unacknowledged event. This is exactly the set that the old `firstForResource` predicate admits. The query now selects these heads with `DISTINCT ON (resource_type, resource_id)` ordered by `(action <> 'create'), id`, and then applies the page cursor, order and limit. **Reason and benefit** The same fixture lists in 8–24 ms instead of 3.5 s. Long-running copies of this statement no longer hold database CPU that every request needs. **Breaking changes** None. `acknowledge()` keeps its per-event eligibility check. ## What Changed - `pluginLifecycleInbox.list()` uses a `lifecycle_heads` CTE with `DISTINCT ON` to pick one head per resource, then filters by the `afterId` cursor and orders by id with the limit. - New test in `server/src/__tests__/resource-lifecycle-events.test.ts`: one resource with 2,000 acknowledged events and a 2,000-event backlog, plus a second resource. It asserts the exact heads in id order and a list time below 1 s (3.5 s before, 8–24 ms after). ## Verification - `cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts -t "acknowledged backlog"` — red on `origin/master` (3,546 ms), green with the change (24 ms, 8 ms). - `cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/agent-runtime-assignments.test.ts src/__tests__/agent-runtime-hooks.test.ts src/__tests__/project-instance-secrets.test.ts` — the existing ordering, paging, per-plugin isolation, backfilled-creation and late-commit tests pass. (One `agent-runtime-hooks` case is timing-sensitive on a loaded machine and also fails on `origin/master` there. It passed 22/22 on rerun.) - `pnpm verify` selection and CI. - After deploy: Neon "Active queries" no longer shows long-running `resource_lifecycle_events … NOT EXISTS` statements. ## Risks - Low risk. The result set is the same: per resource, the unacknowledged creation, otherwise the lowest unacknowledged id. The cursor still applies after the head is chosen, as before, so paging past failed heads does not skip later events. - Each call still reads the company's events of the requested types once. A company with millions of acknowledged events still pays one linear pass per call. A per-plugin acknowledgment watermark would remove that and is out of scope. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code execution). Extended reasoning on. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
0934b834c4 |
perf(workspaces): release retained cohort parent scratch early (#20447)
## Thinking Path > - Paperclip manages agent work and preserves its workspace files. > - Online migration captures local files and publishes remote workspace authority. > - Each capture verifies encrypted readback, restores the archive and compares the current source. > - A cohort kept every completed parent payload until the whole cohort published. > - A larger cohort could retain many complete copies while later members still worked. > - This pull request releases only finished parent payloads before a capture worker starts another member. > - Nested-child proof, final fresh checks and atomic publication keep their existing owners. ## Linked Issues or Issue Description Refs: #20418, #20437, #20441. **What happened?** A cohort retained each member's capture archive, verified archive, comparison archive and restored tree until atomic publication. The capture worker limit did not limit that retained parent data. **Expected behavior** Release completed parent payloads once their consumers finish. Keep files that a later publication check still needs. **Steps to reproduce** Run the added held-peer regression against the previous owner. It completes one parent, holds a later member and finds all four completed parent paths still present. ## What Changed - Release three named parent archives and the restored tree after a successful online cohort capture, before that worker takes another member. - Keep parent scratch roots and all nested-child verification areas. Keep every final fresh child and publication check. - Retain payloads for archived repository intents. Use the existing writable-tree cleanup for read-only restored directories. - Add real filesystem regressions for early release, retained child proof, late child mutation and the final archived-intent path check. Update the source-fence guide. This branch incorporates the final #20441 owner and both distinct saved-relative-path regressions. Its scratch-specific production change is nine added lines and one removed line. Single imports and saved journal/archive formats stay unchanged. ## Verification - Canonical Node24 preflight passed. The Runner build came from the content cache. After the current-master merge, the preflight checked 1,674 inputs and 1,817 outputs and found unchanged build inputs. Plugin SDK prerequisites passed. - Genuine RED on the unchanged #20441 owner: one held-peer case failed at four retained payload paths; the archived-intent control passed. The run took21.14s, with221 unrelated cases intentionally unselected. The first sandbox-restricted setup attempt hit DNS errors and was stopped; the normal network-qualified preflight then passed. - Initial treatment: four selected PostgreSQL/filesystem cases passed in38.61s, with219 intentionally unselected. The held peer observes released parent payloads and retained child areas. A later child mutation still refuses the whole cohort and publishes no authority or activity. - The archived-intent control observes all parent payloads at the actual final saved-relative-path check. The changed path refuses publication and keeps task/repository state intact. This exercises its actual singleton owner; archived-only sources do not enter the online cohort API. - After a normal merge of current master and the final #20441 test, the five affected cases and server TypeScript passed. No full local matrix was repeated. Initial CI failed the inherited recovered-upload assertion because the new retention test leaked its archive and filesystem spies. That case passed alone but failed in the adjacent three-case sequence (two pass, one fail); scoped `finally` cleanup made the same sequence pass three of three in17.15s, with191 unrelated cases intentionally unselected. No upstream assertion or production code changed. Server TypeScript passed again. All required [CI](https://github.com/paperclipai/paperclip/actions/runs/42746586511) passed on final normal-merge head `604c0f67bf7282ecd3f6e5e5da1c9d8637ef70cc` (56 success, four skipped and one neutral; no pending or failing checks). The previously failing server4/12 passed in10m23s. Fresh Greptile is5/5 on that exact head with no open threads. The merge resolves the duplicate #20441 squash insertion; both tested owner and test files are byte-for-byte identical to repair head `b5c8eed82b6a676985e1a8af785e5df2ae3be2ce`. The prior head received [Greptile5/5](https://github.com/paperclipai/paperclip/pull/20447#issuecomment-6757435885); its archived-cohort coverage warning was [answered using the actual public admission boundary](https://github.com/paperclipai/paperclip/pull/20447#discussion_r4520641017). - `git diff --check` passed. No hosted cleanup, migration, provider action or deployment was performed. ```sh node scripts/agent-preflight.mjs --skip-rust-debug pnpm --filter @paperclipai/plugin-sdk ensure-build-deps node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts server/src/services/remote-workspace-migration.test.ts -t 'releases completed parent payload|retains archived intent payload|shares the merged child verifier|rechecks the current archived repository intent path at publication' pnpm --filter @paperclipai/server exec tsc --noEmit ``` ## Risks - This reduces retained parent data. It does not establish a total disk bound. Active capture/encryption/readback files and retained child areas still need measured capacity. - Cleanup failure uses the existing worker abort and peer-join path before publication. Original source files, uploaded objects and journals remain intact. - Archived repository intents still need their restored payload at publication. Those payloads stay retained. No capture limit, budget or integrity guard changes. - Saved operations and archive identities carry forward unchanged. Resume still uses the existing full source and authenticated object checks. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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> |
||
|
|
9f00f2b83e |
fix(workspaces): recognize committed imports in linked source scope (#20444)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration fences every local Git source that a capture can reach. > - A linked worktree can remain on disk after its workspace is migrated to cloud storage. > - The source inspector lists local workspace rows, so it then treats that linked worktree as unregistered. > - This pull request uses the existing canonical import journal and snapshot proof to recognize its exact original physical root. > - The benefit is that a later linked workspace can migrate without weakening source ownership or stopping agents. ## Linked Issues or Issue Description **What happened?** After one linked Git workspace commits its canonical migration, its local files and Git registration can remain. The workspace row now uses `cloud_sandbox`. A later migration of a related local workspace fails with `online_workspace_source_scope_unproven` because the original linked root is no longer in the local-row list. **Expected behavior** The inspector can recognize the exact original physical root from the same-company successful migration journal and its committed or superseded snapshot. Unknown or changed provenance must still fail. **Steps to reproduce** 1. Create two local Git workspaces with a shared worktree registration. 2. Complete canonical migration for one workspace while retaining its original local files. 3. Inspect or migrate the other workspace. Before this fix, the inspector returns no source scope. **Paperclip version or commit** Reproduced on `f73ff4ace9` with real PostgreSQL, Git, filesystem, and canonical snapshot storage. **Deployment mode** Self-hosted server. This is a core workspace migration bug. **Relevant logs or output** `remote_workspace_migration_blocked:online_workspace_source_scope_unproven` Refs #20418, #20436, #20441. This fix changes source registration only. ## What Changed - Let the existing imported-source owner match optional exact linked physical roots in addition to its existing nested-child paths. - Require each added linked root to have a finished successful same-company journal, an exact committed or superseded snapshot binding, and the same manifest digest. - Include those proven physical roots in the online scope and its source fingerprint checks. - Retain Git root, backlink, pointer, ownership, company, foreign writer, source lock, and publication reinspection guards. - Keep existing nested-child behavior and all persisted schemas unchanged. Previous-release canonical imports remain readable. No runtime authority comes from an arbitrary provider path or remote authority. ## Verification - Genuine baseline RED: both committed and superseded imported sibling cases return a null source scope after a real successful import. - GREEN: 17 focused real-PostgreSQL/FS/Git tests pass. They cover both imported states, the original issue source fence, changed journal/snapshot/manifest/unfinished provenance, a foreign local alias, unregistered links, backlink/pointer corruption, and a listing change during upload. - Command: `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'imported linked Git source|foreign local writer|unproven linked|revalidates the linked Git listing|nested Git wrapper.*shared|nested Git wrapper blocked'`. - Server layered typecheck, module boundaries, and `git diff --check` pass. - Actual controlled staging canary: the canonical API committed both targeted workspace snapshots in 310.249 seconds. The owned job closed with exit 0 and all owned processes joined. A separate read-only inventory confirmed that both workspaces left the legacy provider set. This used a reviewed private administrative artifact; the serving app image and normal release path stayed unchanged. - Final head `1759ac6f40a1b35f79587b246de8b36e7a371537`: 56 successful checks, 4 intentional skips, and 1 neutral status. Greptile and Ponytail both report 5/5 with no findings or unresolved threads. - Initial CI had unchanged Runner-support and workspace-recovery timeouts, plus an unrelated heartbeat plugin-environment timing/fixture failure. The same owning inputs passed on the sibling PR. Each failed job then passed one targeted retry on unchanged product bytes. The original failures remain recorded in the CI history. ## Risks - This recognizes a retained original root only from the existing committed import ledger. It does not treat arbitrary cloud workspace paths as local ownership. - Each physical root still passes the existing Git identity and foreign-scope checks. Publication re-inspects the source scope under the existing source locks. - This adds no object reads, provider calls, recurring work, capture algorithm changes, or data exclusion. The existing import query keeps its current 10,240-row limit and runs during explicit migration inspection only. ## Model Used OpenAI GPT-6.1 Codex, with reasoning, repository tools, and code execution. The host does not expose an exact 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> |
||
|
|
0991d9bda1 |
fix(workspaces): protect migration sources and add guarded recovery service (#20443)
## Thinking Path > - Paperclip manages agents and their task workspaces. > - Online migration saves files before it publishes remote workspace authority. > - A saved migration journal can survive the importing process. > - Git cache expiration did not count that journal as a consumer. > - Expiration could remove a source and change its fingerprint while its import was incomplete. > - This PR protects unfinished imports and exposes a guarded internal service transition for an already affected cohort. > - New imports can preserve the current sources while all old recovery data stays retained. ## Linked Issues or Issue Description **What happened?** Git cache expiration could archive and remove a workspace after an import process ended but before its saved cohort finished. The saved source fingerprint then differed from the current archived workspace. Exact cohort resume correctly refused that changed source. Reopening the workspace cannot recreate the old metadata or fingerprint. **Expected behavior** An unfinished migration remains a durable consumer after process loss. If an older server already expired some sources, one canonical transition can fail the exact unpublished cohort without losing its snapshots, objects, source history, or recovery obligations. New attempts use the current verified sources. **Steps to reproduce** 1. Start a cohort import of eight Git worktrees with local changes. 2. Interrupt it after four snapshot uploads. 3. Let an older server expire two idle source worktrees. 4. Resume the saved cohort. It refuses the changed archived sources. Related: #20429 preserves exact cohort identity when independent companion imports change only the scope proof. #20424 preserves prepared recovery state for archived sources. This PR handles completed source expiration, not a scope-only change. Public searches found related expiration work and no matching compensation owner. ## What Changed - Count saved migration markers and unfinished migration journals as Git expiration consumers. Keep failed migration retention holds as consumers too. - Expose `supersedeExpiredRemoteWorkspaceMigrationCohort` as an internal service API for a privileged operator with a real database handle and reviewed company/cohort inputs. It is called directly by the PostgreSQL/Git test and reviewed one-time administrative qualification. It adds no CLI mode or HTTP route. The existing CLI still refuses a damaged cohort; this PR does not claim automatic CLI recovery. - Require an exact unpublished cohort of up to eight members. It requires completed expiration and an unexpired restore recipe for each missing member. The other members must retain their saved source fingerprints. - Recheck current source and recovery state under source, lifecycle, finalizer, and row locks. One transaction fails all old journals, writes audit records, and removes only their exact matched active markers. - Keep old cohort proofs, operations, snapshots, and object references. Genuine failure receipts use the existing failed-save retention hold. This transition does not report migration success. - Reuse the existing bounded source-lock queue when `sourceLockWaitMs` is set. Keep fast refusal by default. Cancellation targets only this waiter, and current checks still run after admission. - Record predecessor IDs on new attempts and reserve a snapshot generation above the existing ledger maximum. New success does not retire the old hold. - Document the owning transition and retention rules. ## Verification - The two new journal-consumer cases failed under the old expiration predicate and passed under the new predicate. - The full new recovery case passed in 21.12 seconds with real PostgreSQL locks, Git repositories, encrypted local object storage, and archive restores. It covers four retained uploads, two real expirations, unchanged old resume refusal, authority change refusal, transaction rollback, a held native finalizer, idempotence, eight new imports, dirty/untracked file restores, and a retention pass 30 days later with no object deletion. - Server TypeScript and module-boundary checks passed. - On `7cec8c915acfad2d08874eaf26abaa852f560219`, the queued-admission regression failed before forwarding the option (24.07 seconds), then passed (22.25 seconds). It verifies a real held project lock, cancellation that preserves the holder and all journals, release into full compensation, retention, and eight restored fresh imports. Current server TypeScript and module/diff checks passed. The only production follow-up changes are the optional queue input and its existing-owner call. - The earlier `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c` CI run had one Runner support failure: the unchanged credential-free Python/Pi calibration exceeded its outer 25-second subprocess deadline. That job passed 6,926 other tests and skipped four. The timeout is retained as a nonpass. The changed logical head requires fresh CI and review; no test deadline was raised. - The affected cohort, Git expiration, and eligibility suites passed all 148 cases in 228.41 seconds on `aa589f3f447e36d0277c32f8b44cbe5e84a673be`. Merged current master `a9d0946e64`; the only conflict retained both independent added test blocks. The complete merged cohort suite passed all 30 cases in 70.00 seconds on `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c`, including upstream larger-cohort controls. Final-head server TypeScript, incremental preflight, module-boundary, and diff checks passed. Unchanged expiration/eligibility evidence remains valid. Fresh `809ab704e619812d8f1bf38cf5de9c65893861cc` CI and review are pending. - On `809ab704e619812d8f1bf38cf5de9c65893861cc`, the complete recovery fixture passed in 21.38 seconds after adding one valid archived recipe with no restore point. It refuses with `migration_supersession_expiration_unverified` and keeps all workspaces, journals, and snapshots unchanged. The same case then verifies queue cancellation/release, full compensation, retained history, fresh imports, and restored bytes. Server TypeScript passed. This follow-up changes only the test and service-scope documentation; product bytes remain identical to qualified `7cec8c915acfad2d08874eaf26abaa852f560219`. - Earlier fixture attempts did not pass. They exposed the selection of a held source and an incorrect expected refusal string. The fixture now selects eligible sources and asserts the canonical authority refusal. Production guards were not relaxed. - Live qualification of the exact owning product bytes completed in 230.419 seconds. All eight old operations became failed with auditable supersession lineage. All eight old snapshot rows remained byte-for-byte identical, and their object references remain retained. The existing source-lock queue waited 1.151 seconds for a normal finalizer handoff. The three complete source/recovery passes took about 218 seconds including their transaction, which explains why the earlier 180-second total budget was insufficient for that measured workload. No source proof or publication guard was skipped. Compensation made no provider or object-storage calls. - Earlier live qualifications remain nonpasses: the 180-second attempt did not finish; its instrumented successor completed source checks and refused a busy source lock before mutation. The successful bounded-queue attempt retained those failure records. Fresh archived/online imports are a separate subsequent step and are not claimed complete here. - The local checks do not claim browser acceptance, app deployment, fleet enrollment, or a completed zero-downtime cutover. ## Risks - This owner mutates migration bookkeeping. It preserves all-or-nothing row comparisons and current source/recovery locks. A changed journal, authority, writer, finalizer, expired recipe, or partial transaction refuses the transition. - Historical snapshots and objects remain pinned after new success. This intentionally retains storage. Release requires a separate reviewed action. - An older importer cannot use the successor generation contract. Use the containing canonical owner for recovery. Normal app delivery must also carry the expiration guard; a private maintenance graph does not patch a running server's cleanup worker. - The migration owner crosses 1,000 lines. The bounded compensation transition remains beside its private source inspection, fingerprint, fence, reservation, and publication contracts. Moving it now would require exporting or duplicating those safety owners. No unrelated refactor or new framework is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source editing, Git, and local execution tools. The exact serving 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 - [ ] 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> |
||
|
|
29a66d679e |
fix(workspaces): verify archived repository intents before publication (#20441)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Workspace migration moves local files into durable, authenticated snapshots. > - Archived Git workspaces can retain an unmaterialized repository request after their local tree is removed. > - Admission cannot prove filesystem absence from the removed tree. > - This pull request checks strict request candidates against the private restored Git tree and the authenticated snapshot readback. > - Publication rechecks current repository rows without changing the request or task history. ## Linked Issues or Issue Description **What happened?** An archived workspace with an authenticated restoration recipe and an unresolved, unmaterialized repository request fails migration with `repository_preparation_unresolved`. Its original directory no longer exists. **Expected behavior** Migration can preserve the request when the actual private restored tree proves its target absent and every parent safe. It must refuse partial materialization, symlinks, or changed request state. **Steps to reproduce** 1. Archive a Git workspace with a valid saved recipe and a pending repository request. 2. Remove its original local workspace directory. 3. Run canonical workspace migration. The previous implementation refuses before it can inspect the restored tree. **Paperclip version or commit** Reproduced on `1ca3e8ea2a759e511ab2d2659339c6c770a7a2ba`. **Deployment mode** Self-hosted server. This is a core workspace migration bug. **Relevant logs or output** `remote_workspace_migration_blocked:repository_preparation_unresolved` Refs #20416, #20424, #20436. These fixes retain their source, recovery, and ownership guards. ## What Changed - Separate strict repository request metadata from filesystem absence proof. - Admit only strict candidates backed by a canonical archived Git recipe to private materialization. - Verify target absence and safe parents before a new upload and after authenticated archive restore. - Lock and recheck current repository rows before authority publication, including recovered uploaded archives. - Preserve previous-release recipes, request rows, task selection, history, provider references, and unresolved recovery obligations. No persisted schema or public option changes. ## Verification - Genuine baseline RED: both pending and needs-repair archived requests fail on unchanged master. - GREEN: 27 focused real-PostgreSQL/FS/Git tests pass. Includes partial/symlink refusal before upload, request mutation during upload, recovered-upload readback, and existing native F/source checks. - Command: `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'archived repository intent|unmaterialized repository request|unresolved repository request|pending repository|unselected archived recipe|archived prepared source|competing native F owner'`. - After rebasing onto current master, all six new cases pass on `0b6854789acc067a44e697acebdc04e51b9afe88`. Server layered typecheck, module boundaries, and `git diff --check` pass. A review follow-up adds one pending-path mutation regression (1/1 real-PG pass) without changing product code. Fresh required CI runs on final head `2a30a4b444108488ac8b51d0c46c340ee15227b9`; fresh Greptile is 5/5. Server7 had an unchanged trace-parent assertion failure; that same shard passed on the previous product-identical head and the linked-source sibling PR. The original failure remains recorded. The single targeted server7 retry passed. Final head has 56 successful checks, 4 intentional skips, and 1 neutral review status. ## Risks - Metadata admission is only a candidate check. A candidate still fails unless the private authenticated filesystem proves absence. - Publication keeps repository rows locked while it checks their current paths against the owned restored tree. A real pending-relativePath mutation now tests this final check. Existing workspace/source/native locks remain in place. - Absence is proved only in SAVED data reconstructed from the retained recipe and snapshot. This does not prove that a request never materialized before archival. Existing Git restore points can exclude ignored `.paperclip-repositories` paths outside recorded checkouts. This PR does not change that preexisting coverage or recover bytes that were never saved. - The original recipe, restore point and checkout references remain unchanged. No files are recovered into or removed from the original directory. No request is marked ready or restored. Partial target files present in reconstructed saved data remain blocked and retain their state. ## Model Used OpenAI GPT-6.1 Codex, with reasoning, repository tools, and code execution. The host does not expose an exact 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> |
||
|
|
5bad074baa |
feat: add individual agent interaction permissions (#20411)
Add eleven sparse incoming and outgoing restrictions, human CLI/API administration, shared mutation enforcement, and pending-delivery checks. Preserve existing defaults, agent settings, and existing authorization boundaries. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
99a6daf5bc |
perf(auth): resolve a Cloud tenant actor in one database round trip (#20390)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On Paperclip Cloud, each request resolves its board actor from trusted Cloud headers in `actorMiddleware` > - This runs for every request after the liveness probe, also for static files such as `favicon.ico` and `index.html` > - The resolution ran three database statements one after another: the stale `instance_admin` purge, the membership read and the owner-elevation read > - On a loaded database, each statement waits. Three sequential waits make every request slow, also a request that needs no data > - This pull request keeps the purge and the tenant sync in order and runs the two reads at the same time as them > - The benefit is one database wait per request instead of three ## Linked Issues or Issue Description **What existing behavior does this improve?** Cloud tenant actor resolution in `server/src/middleware/auth.ts` (`resolveCloudTenantActorOnce`). **Subsystem affected** Server authentication middleware (Cloud trusted-header path). **Current behavior** Measured on the hosted staging instance on 2026-11-09 from a signed-in browser, 45 paired samples in 40 s: - `/api/liveness` (mounted before actor resolution): client median 92 ms, p90 255 ms, server time ≤ 1 ms. - `/favicon.ico` with a session (after actor resolution): client median 413 ms, p90 1,751 ms, max 4.6 s. - `/api/cli-auth/me`: server median 346 ms, p90 1,528 ms. The same Node process answers liveness quickly, so the event loop is not blocked. The database is busy (6–8 of 8 vCPU for the whole day), and trivial primary-key reads waited 100–280 ms each. Three sequential reads explain the ~320 ms median gap between liveness and favicon. **Proposed behavior** The purge and the debounced tenant sync stay ordered. The membership read and the owner-elevation read run at the same time as them. The sync writes only the primary company's membership, and the membership read excludes that company, so the result does not change. **Reason and benefit** A synced request costs one database wait instead of three. With 150 ms of injected database latency, a synced resolution goes from 914 ms to 310 ms (3 runs each, same machine). **Breaking changes** None. ## What Changed - `resolveCloudTenantActorOnce` starts the purge-and-sync, the additional-membership read and `resolveOwnerInstanceAdmin` together and awaits them with `Promise.all`. A membership read failure still degrades to the primary company. The owner-elevation read still fails closed. - New regression test `server/src/__tests__/cloud-tenant-actor-round-trips.test.ts`. It connects through a TCP proxy that delays every client write by 150 ms and asserts that a synced resolution finishes in less than 4 × the delay. Before the change it takes about 6 × (914 ms); after the change about 2 × (310 ms). - `auth-session-route.test.ts`: the fake database binds each `returning()` to its own insert, and the test matches inserts by content instead of position, because the instance-settings insert can now happen in parallel with the tenant sync. ## Verification - `cd server && npx vitest run src/__tests__/cloud-tenant-actor-round-trips.test.ts` — red on `origin/master` (914 ms, 913 ms, 916 ms), green with the change (310 ms). - `cd server && npx vitest run src/__tests__/cloud-tenant-company-provisioning.test.ts src/__tests__/auth-session-route.test.ts src/__tests__/liveness.test.ts src/__tests__/cloud-warm-standby.test.ts src/middleware/cloud-tenant-actor.test.ts` - `pnpm verify` (selected tests), and CI. - After deploy: compare `/favicon.ico` and `/api/cli-auth/me` client time with `/api/liveness` on staging. Expect the median gap to drop from about 3 database waits to about 1. ## Risks - Low risk. Authorization does not change: the same statements run and the request waits for all of them before it continues. - Each request now uses up to three pooled connections at the same time instead of one. The staging pool limit is 500, so this is not a concern there. Smaller pools reach their limit sooner under a burst. - The diff re-indents the purge-and-sync block. The statements in it do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code execution). Extended reasoning on. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
f73ff4ace9 |
fix(native): reprove stale filesystem preservation receipts (#20438)
## Thinking Path > - Paperclip manages agent work and preserves saved native sessions across releases. > - Filesystem independence receipts bind database facts to preserved controller and skill evidence. > - A workspace migration can add run metadata without changing those preserved bytes. > - That change makes the saved facts digest stale. Full native preservation then refuses the existing receipt, even after fresh source checks. > - WEB preservation already supports a full current proof and guarded receipt refresh. > - This pull request applies that pattern to full native receipts, reuses the authenticated archive, and keeps the existing ownership and destruction checks. > - The benefit is a current receipt without a new archive or provider replay. ## Linked Issues or Issue Description Refs: #20322. **What happened?** Adding the canonical `legacyWorkspaceRecoveryMigration` marker to a settled native run makes its saved receipt stale. The full native preservation owner checks current evidence but then refuses the old facts digest. **Expected behavior** The owner can refresh the proof metadata after it proves all current facts and preserved bindings again. Changed evidence or authority must leave the prior receipt intact. **Steps to reproduce** Use the added PostgreSQL regression. It saves a valid receipt for a succeeded source with 453 checkpoint parts, adds the producer-shaped migration marker, and calls preservation again. The unchanged owner fails with `native_harness_filesystem_independence_unverified`. ## What Changed - Reuse the prior controller archive through the existing authenticated readback path. - Require the old receipt to match every current immutable binding before a facts refresh. Reconstruct its exact digest from unchanged bindings and the prior proof fields. - Recheck current database facts, assets, controller and local files under the existing finalizer, selection, task and run fences. - Publish with an exact prior-receipt comparison. Keep a valid concurrent winner and refuse changed receipts or new owners. - Audit the prior receipt digest and four proof fields. Add focused source, integrity, race and DB-only validation tests, and update the database guide. ## Verification - Local Node24 canonical preflight and plugin SDK prerequisites passed. The Runner build came from the content cache. After a normal merge of fresh master, preflight confirmed unchanged build inputs. - Genuine RED on `3040448da5`: the saved receipt is valid before the migration marker and stale afterward. Reproof fails at the old receipt guard (one failed, 102 intentionally not selected). Both retained RED attempts reached that same guard. - GREEN on final local merge `00f4b5d678083106afc88fb2f7e0588961b46d6d`: 19 PostgreSQL controls passed, 91 intentionally not selected (17.75s). The wrapper completed cleanup. - The positive preserves the same archive, controller bytes, assets, local proof and all 453 checkpoint parts. It performs authenticated object reads and zero archive PUTs. Runs, results, finalizers and leases remain unchanged except for the receipt. The audit reconstructs the exact prior receipt. - DB-only validation passes with the original home, state directory and volume absent, while storage reads throw. - Refusal and race controls cover changed asset bindings, raw controller bytes, missing destruction evidence, unknown receipt fields, a valid concurrent winner, changed receipts and a late active owner. Existing WEB refresh, physical finalization-lock and saved-binding controls also pass. - Server TypeScript passed, including the adjacent integration test. Fresh Greptile is 5/5 on exact head `00f4b5d678083106afc88fb2f7e0588961b46d6d`, with no review threads ([review](https://github.com/paperclipai/paperclip/pull/20438#issuecomment-6756548830)). Final-head CI is terminal: 56 success, 4 skipped and 1 neutral. The aggregate `ci / verify` passed ([run](https://github.com/paperclipai/paperclip/actions/runs/42737704110)). - The first Runner-support job had one unrelated process-inspection nonpass: `server-stop.test.ts` expected a successful wrapper close, but its bounded process inspector returned no table. Its cause remains unknown. The unchanged test, process helpers, workflow and dependency inputs match the passed master control. One authorized same-head job retry passed all 328 files / 6,927 tests (4 skipped); the exact previously failed case passed in 665ms ([retry](https://github.com/paperclipai/paperclip/actions/runs/42737704110/job/129576420893)). No timeout, assertion or source change was made for the retry. ```sh node scripts/agent-preflight.mjs --skip-rust-debug pnpm --filter @paperclipai/plugin-sdk ensure-build-deps node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts -t "reproves stale native facts|retains stale native proof|guards stale native reproof|invalidates a published receipt after changed|actual physical-finalization lock|refreshes stale WEB facts|refuses a WEB facts refresh|retains or refuses the existing receipt when an earlier capture finishes late" pnpm --filter @paperclipai/server exec tsc --noEmit ``` ## Risks - Old receipt schemas and immutable bindings remain required. Full fresh proof carries compatible saved receipts forward; unknown fields or changed evidence refuse refresh and preserve the old receipt. - Full native preservation still requires canonical destroyed-provider receipts. The change retains the existing source capability, cancellation, finalizer and publication checks. - Only proof metadata changes. No provider operation, archive replacement, hosted recovery or deployment was performed for this PR. Live qualification remains with the operator after delivery. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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 |
||
|
|
07f6e8ff75 |
perf(workspaces): group same-root cohorts with bounded workers (#20437)
## Thinking Path > - Paperclip preserves agent work when its control-plane server is replaced. > - Online migration imports local workspaces into authenticated snapshots. > - Several workspace records can point to one physical directory. > - Shared child verification avoids repeated work inside one held cohort fence. > - The eight-member limit splits 30 identical-root aliases into four cohorts and repeats the child passes. > - This pull request admits up to 32 members only for one canonical physical root, while keeping each work stage bounded to eight peers. > - The benefit is fewer repeated child checks while capture and publication guards stay in place. ## Linked Issues or Issue Description Refs: #19873, #19882, #20418, #20424, #20429. Related PR search found shared verification and restart fixes, but no larger same-root cohort owner. The shared verifier and archived-source changes are merged into this branch from current master. **Current behavior** Thirty fresh aliases of one physical root require cohorts of 8, 8, 8 and 6. Each cohort performs an initial and final local child-validation pass. The same children are checked eight times in total. **Proposed behavior** Admit one cohort with nine to 32 members only when every canonical resolved physical root is exactly equal. Keep the existing complete source-scope equality and admission guards. Keep mixed-leaf cohorts limited to eight members. Bound source reads, reservations, recovery checks, final readers, replay and cleanup to eight joined peers. Capture concurrency stays at most eight. **Reason and benefit** For N distinct canonical covered children, the 30-alias case changes 8N local child checks to 2N, a 75% reduction in repeated child validation. If N is 68, that is 544 checks versus 136. Thirty parent captures, uploads and restores remain required. No total wall-clock improvement is claimed. **Breaking changes** Saved cohort fields and exact membership remain unchanged. Older tools that enforce the eight-member bound refuse larger cohorts. Use the newer tool to resume them; never split or regroup their saved membership. ## What Changed - Raise the canonical service, saved-cohort and maintenance-request member bounds to 32. Keep the capture-worker bound at eight and single-workspace modes at one. - Require one exact canonical physical root for more than eight members. Keep the existing full source-scope comparison, leaf rule and P/I/L/F guards. - Use one private stage mapper with batches of at most eight. Join every started peer before an error can leave the stage. Stop future batches after a failure or cancellation. - Preserve the existing cancellation and pending-snapshot ordering for stages of at most eight members. Check cancellation around extended batches. - Keep all saved cohort, member, operation and snapshot identities, proof refresh and atomic publication checks. - Bound cleanup without skipping later scratch cleanup after an earlier cleanup error. - Extend the existing PostgreSQL fixture with ten real aliases, partial-upload resume, cold restoration, replay and mixed-root refusal. Extend the request boundary test. - Document old-tool refusal, work counts and the retained-scratch capacity bound. ## Verification - Canonical Node24 worktree preflight and Plugin SDK prerequisites passed. After merging current master, preflight passed again with unchanged cached build inputs. - Genuine RED on the old bound: both real ten-alias import/resume cases and the 32-member request reject the eight-member limit (3 failed; 47 not selected; 18.23 seconds). - First focused GREEN: 4 passed in 24.20 seconds. The fresh ten-alias case performs exactly four local child captures for two children: one initial and one final pass. Actual reader, transaction and upload maxima are eight. All ten archives cold-restore after the original source is parked; replay retains the same cohort. - The partial-upload case keeps all ten saved operation IDs and the existing snapshot IDs. More than eight members across different canonical leaf roots refuse before any journal or storage work. The request parser accepts 32 members and eight workers, and refuses 33 members or nine workers. - The first expanded corpus had 55 passes and one real compatibility failure: a new eager cancellation check skipped the old pending snapshot boundary. The mapper now retains existing cancellation ordering for at most eight peers. That failure remains recorded as a nonpass. - Final sources include current master at `18697b8d21`, with the verifier, archived-source and UI repairs. The selected four-file corpus passes 47 cases in 78.88 seconds, with 191 intentionally not selected. The full maintenance suite separately passes 23 cases in 10.21 seconds. Nine cases overlap; the final union is 61 distinct passing checks. - Final coverage includes all 27 cohort cases, all 23 maintenance cases, all six actual old-format compatibility cases and five archived prepared-source controls. Existing source mutation, finalizer contention, actual fence loss, cancellation peer joins, journal compare-and-swap and outer-commit rollback controls pass. - Final server TypeScript, module boundaries and whitespace checks pass. Only the resource-risk documentation changed after that test run; runtime and fixture bytes stayed unchanged. - Commands: `pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts server/src/maintenance/online-workspace-migration.test.ts server/src/__tests__/online-workspace-migration-compatibility.test.ts`; archived controls use the existing `remote-workspace-migration.test.ts` cases named `unselected archived recipe` and `archived prepared source`; `pnpm --filter @paperclipai/server exec tsc --noEmit`; `node scripts/check-module-boundaries.mjs`. - Initial source review was 4/5 and requested a control for the extended reader-batch stop boundary. The test-only follow-up `ca80ae9cc8791a72dab632090488093c5d0916f3` adds actual ten-member failure and cancellation cases. They hold a first-batch peer, verify that the source fence stays held, and prove members nine and ten never start preflight. Both pass in 10.87 seconds with no new journal, snapshot or upload effects. Runtime source hashes are unchanged; the local union is now 63 distinct checks. - The first stop-test attempt observed the two controlled readers before all SQL-backed peers had entered the inspection callback. Its assertion was too early. The corrected test checks the held first-batch readers at that point, then requires all eight to join before completion. Both cases pass; the earlier attempt remains a nonpass. - Final head `ca80ae9cc8791a72dab632090488093c5d0916f3`: [required CI](https://github.com/paperclipai/paperclip/actions/runs/42737676664) and [aggregate verify](https://github.com/paperclipai/paperclip/actions/runs/42737676664/job/129575813765) passed. Full rollup: 56 successes, four intentional skips and one advisory neutral result. Classic Snyk passed. [Greptile](https://github.com/paperclipai/paperclip/pull/20437#issuecomment-6756419093) and [Ponytail](https://github.com/paperclipai/paperclip/pull/20437#issuecomment-6756388693) both reviewed that exact head at 5/5. The one review finding is addressed and resolved; no open findings remain. Older partial CI and the initial 4/5 review remain nonfinal evidence. Hosted acceptance and measured wall-clock savings remain pending. ## Risks - Completed parent captures retain their scratch until the atomic publication completes. A 32-member cohort can retain up to four times the parent scratch of an eight-member cohort. Check actual parent capture, verification and restore sizes against current target filesystem capacity before selecting a larger cohort. Keep the existing resource guards. - Larger member arrays must not raise execution fanout. Every reader/reservation/final stage is bounded to eight and joins current peers before failure. Capture workers remain at most eight; child-verification workers remain two. - More than eight members require the exact same canonical physical root and complete source scope. A different root, new writer, changed source, recovery owner or journal still refuses through the existing owners. - Shared verification is limited to the held invocation fence. The separate fresh final pass remains required after all captures join. Different child sets and restart attempts do not share a local-byte proof. - The old saved membership and archive identities remain authoritative. Older tools fail closed on the larger bound. No schema, archive or provider format changes are included. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The runtime does not expose a more specific deployment identifier 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> |
||
|
|
1ca3e8ea2a |
fix(native): let a resume claimed after a restart write the run's existing log (#20427)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native run is interrupted, Paperclip can persist a same-run resume: a later attempt continues the same run from its exact checkpoint > - If the server restarts before that resume is dispatched, the new server process claims it. The resume then reopens the run's existing log, and only the current controller may write it > - The resume claim took the lease owner but left the dead process's controller identity on the lease. The log writer check compares the boot id, so it refused, and the exact checkpoint resume failed as `run_log_writer_authority_lost` > - This pull request stamps the claiming process's identity on the resume claim, as restart recovery's own claim already does > - The benefit is that a resume persisted before a restart continues after it, instead of failing and needing a manual Retry ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261110T003423-s9903` (local ST2, seed 9903), task t01 (codex C1, K1). The wave went 7/8; t01 was the one task that needed a Retry. - Run 279772b3 was SIGKILLed during its first turn. The restarted server resumed it, and the Codex snapshot failed ("cannot safely recover an ambiguous Codex turn start"). A same-run resume was persisted. - A second SIGKILL hit (00:35:37), and the next process dispatched the scheduled resume at 00:35:56. - The resume's run-log claim failed in `createRunLogWriterAuthority`: the coordinator lease still carried the killed process's `controllerBootId`. The exact checkpoint resume was refused (`run_log_writer_authority_lost`), and the run ended `adapter_failed`, needing a Retry. The wave had S3 latency injected, but the failure does not depend on it. The new test reproduces it on master with no storage faults. ## What Changed - `server/src/services/native-runtime/native-finalization-reconciler.ts`: `dispatchNativeSessionResumptions` writes the claiming process's controller identity (boot id, pid, host hash, process start) along with its lease owner and expiry. This matches the restart-recovery claim, so ownership proofs and the run-log writer check see the process that actually owns the resume. - `server/src/__tests__/native-session-resumption.test.ts`: a regression test for this case. ## Verification - `native-session-resumption.test.ts`: - A coordinator persisted for resume by a previous process (stale boot id), on a run with an existing object-store log, is dispatched. - The run-log writer check accepts the new claim, and the lease carries this process's identity. - The test fails on master with `run_log_writer_authority_lost`. - Regression: the native resumption, binding, recovery-deadline, restart-recovery, scheduled-resume, owner-release, runner-ownership and recovery suites pass locally. `tsc --noEmit` passes for `server`. ## Risks - The resume claim now also refreshes the controller pid, host hash and process start on the coordinator row. These describe the process that owns the lease, exactly as after a restart-recovery claim. A claim by the same process writes the same values as before. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
232480d18f |
perf(runner): write preparation span rows to the run log in the background (#20426)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a native run starts its provider, the executor copies the preparation spans (queue, prepare phases, environment) into the run trace and the run log > - It records about 25 spans and waits for one run-log insert per span, one after another > - On a hosted test deployment, that step (`native.controller.span_persistence`, added in #20396) took a median of 0.28 s, and every task waited for it before the Runner started > - The run trace already has a way to write diagnostics without blocking the caller: `defer`, which `finish` drains before the root span closes > - This pull request writes those run-log rows through that path, in order, while the spans themselves are still recorded at once > - The benefit is about 0.25 s less before the provider starts, on every native run, cold or warm ## Linked Issues or Issue Description **What existing behavior does this improve?** The time between the end of heartbeat preparation and `native.session.execute` for native Runner tasks. **Subsystem affected** Server: native runtime tracing (`server/src/services/native-runtime/native-run-trace.ts`) and the native session executor. **Current behavior** `executeNativeSession` calls `trace.record` for each preparation span and awaits it. `trace.end` awaits `emitCompletedSpanEvent`, which inserts one `run.performance.span` row through `onEvent`. On a hosted test deployment, measured with the #20396 spans across 10 runs, `native.controller.span_persistence` took a median of 0.28 s: - 0.30 to 0.36 s for runs right after a computer wake - 0.17 to 0.25 s for runs on an awake computer It is the second largest step between preparation and the provider start (the median of that whole gap is 1.0 s). **Proposed behavior** The spans are still started and ended at once, with the same parents. The speed-budget bookkeeping still runs at once too. Only the run-log row is written later: one row at a time, in the order the spans ended, before the trace finishes. **Reason and benefit** The rows are diagnostics, and they already carry their own start time (`wallStartedAtMs`, `startOffsetMs`). Readers do not depend on the row order: the only reader in the repository, `summarizeStartupReuse`, looks for `runner.process.launch`, which this change does not defer. The provider can start without waiting for these inserts. **Breaking changes** None. ## What Changed - `native-run-trace.ts`: `end` and `record` take an optional `deferRunLog`. With it, `end` queues the run-log row on a per-trace chain registered with `defer`, so rows are written one at a time in order and `finish` waits for them. A row requested after the trace finished is dropped, as for other deferred diagnostics. - `native-session-executor.ts`: the preparation spans and the `environment.startup` scope use `deferRunLog: true`. - `doc/run-log-events.md`: note that native preparation span rows are written late, in order. - `native-run-trace.test.ts`: two tests. - Spans are recorded under their parent while every row write is blocked, and the rows are written in order before `task.provider_session.measured`. - A deferred row requested after `finish` is not written. ## Verification - `npx vitest run src/services/native-runtime/native-run-trace.test.ts src/services/native-runtime/native-run-trace-context.test.ts` (34 passed). Both new tests fail without the source change: the first times out waiting on the blocked write, and the second sees the row written. - `npx vitest run src/__tests__/heartbeat-preparation-gap-spans.test.ts src/services/native-runtime/native-session-executor.test.ts` (1321 passed) - `pnpm --filter @paperclipai/server typecheck` ## Risks - Low risk. The change is diagnostic only: OpenTelemetry spans and the speed-budget checks are unchanged. - The preparation rows can now land in the run log after the first Runner events. Readers already use the payload times, not the row order. - If the server process dies between the provider start and `finish`, the preparation rows of that run can be missing from the run log. The OpenTelemetry spans are not affected. - The deferred rows are written while the run continues, as budget-breach rows already are, so the sink sees concurrent writes from one run. This was already the case for deferred diagnostics. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs, Honeycomb queries) and extended thinking. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
4608744977 |
fix(workspaces): preserve recovery for verified archived Git sources (#20436)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Durable workspace migration must retain saved Git files and pending native recovery. > - The archived-Git owner validates saved recipes, exact cleanup receipts, and surviving owned branches. > - Online recovery preservation currently accepts only the recipe variant. > - A valid cleanup receipt or owned branch therefore cannot migrate while an old native reference is still prepared. > - This pull request admits every verified archived-Git variant through the same recovery and finalization locks. > - Tasks stay done and keep their selected workspace, history, and pending provider obligations. ## Linked Issues or Issue Description Refs #20424, #20298, and #20418. The related cleanup work in #19812 does not cover this admission gap. **What happened?** Canonical inspection accepts the exact old commit of an archived Git workspace through a valid branch-deletion receipt or an owned surviving branch. Migration still reports `remote_copyback_unresolved` when a terminal native run retains a prepared reference. A read-only staging inventory reproduced this sole blocker for four such workspaces. Their original Git objects and canonical source proofs already exist. **Expected behavior** Preserve the verified Git baseline and every typed native/provider recovery obligation through the existing migration owner. Keep the task status, selection, history, archive state, and recovery records unchanged. Do not reopen the task or substitute the current default branch. **Steps to reproduce** 1. Save a clean archived Git workspace with an authentic cleanup receipt or a surviving owned branch, without a restore recipe. 2. Retain a terminal prepared native reference and its exact provider allocation and recovery action. 3. Remove the old task worktree. 4. Call `migrateRemoteWorkspace` online. **Paperclip version or commit** Reproduced on merged #20424, `915edcff49bb2cbacfbaee604b41e082e0397109`. **Deployment mode** Self-hosted server with PostgreSQL and configured object storage. ## What Changed - Change three conditions to accept a non-null canonical archived-Git source instead of requiring its recipe field. - Use the same typed recovery inspector, sorted native finalization locks, fresh reinspection, and source-task publication locks for all accepted variants. - Reuse the existing Git source validator, private scratch materializer, encrypted snapshot restore, journal, and recovery markers. No source-proof schema or provider protocol changes. - Extend the existing real fixture for cleanup receipts and retained branches. Verify exact old Git commits and unchanged task/recovery state. Refuse receipt revocation and branch movement during upload. ## Verification - Genuine RED on merged #20424: both new source-kind cases fail with `remote_workspace_migration_blocked:remote_copyback_unresolved,workspace_recovery_unresolved`. - GREEN: nine focused PostgreSQL/Git/encrypted-storage cases pass. They include both new variants, two actual upload-time source mutations, the existing recipe interrupted-journal resume, reference/provider mutations, and a competing native finalization owner. - Command: `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'no-recipe archived|unselected archived recipe|archived prepared source|competing native F owner'`. - All nine focused cases, server typecheck layers, module boundaries, and whitespace checks pass on exact published head `03065f97b31846a405a8911c30640ef62374e4a4`, after the clean rebase onto merged #20418. Required CI is now clean on this exact head:56 SUCCESS and4 expected SKIPPED checks, with no pending or failed checks. Fresh Greptile is5/5 with no actionable findings or unresolved threads. - Previous-release state: existing cleanup receipts, owned branches, prepared v3 references, and journal shapes carry forward unchanged. Saved snapshots and source identities retain their existing owners. Unrecognized or changed sources still refuse by the existing named errors. Migration does not declare remote files recovered or provider allocations stopped. - Hosted work in this PR was read-only source inspection and inventory. No staging import, source mutation, lifecycle reopen, or provider call was performed by this PR author. ## Risks The existing archived-Git validator now admits its other verified variants to recovery preservation. Incorrect source proof could publish incomplete authority. This PR keeps its exact commit/receipt/branch checks and full authenticated capture/readback. It also keeps all native ownership, reference, allocation, and publication checks. Missing or changed sources remain blocked, and pending remote data retains its original recovery obligation. ## Model Used OpenAI Codex, GPT-6, with reasoning, repository tools, and code execution. The runtime does not expose a deployment 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 #` / `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> |
||
|
|
18697b8d21 |
perf(workspaces): share cohort child-source verification (#20418)
## Thinking Path > - Paperclip must preserve agent workspace bytes during online migration. > - A cohort can contain several aliases of the same physical source. > - The importer verifies imported children before capture and again before publication. > - The immutable-object cache avoids repeated downloads but still recaptures local child bytes for every alias. > - This pull request shares the initial proof within the held cohort fence and checks fresh bytes once more after every capture joins. > - The benefit is less repeated local archive work while child mutations still block the whole cohort. > - Required browser CI twice reproduced an independent stale composer-mode bug. A focused regression proves that refreshed persisted task mode was ignored after mount. The state repair follows task data when there is no local override. Explicit local mode choices retain their own generation so a completed send cannot clear the next draft’s choice. ## Linked Issues or Issue Description Refs: #20311, #20387, #20401, #20404. **Current behavior** Two aliases recapture the same imported child's local bytes four times, even with one immutable object read. A larger alias cohort repeats this multi-GB work for each member. **Proposed behavior** Share the successful initial verification for the same physical root, Git roots and exact recorded child set or discovery mode. After all captures join, run a separate fresh final verification for each root and recorded child set. Keep every member's database and publication checks. **Reason and benefit** The local PostgreSQL fixture proves that two aliases now perform two child recaptures instead of four. Hosted timing improvement has not been measured. ## What Changed - Add invocation-local initial and final proof maps to the existing cohort child verifier. - Retain the existing canonical source-owner check before and after shared results. - Defer cohort final nested checks until all captures finish, and join every final reader before publication or fence release. - Keep singleton verification, saved journal schemas, snapshot IDs, archive bytes and per-member publication checks unchanged. - Extend the existing real PostgreSQL alias fixture with a child mutation during the last parent upload. - Repair the required CI planning-mode race with a nullable local override: absent override follows task data; a user selection belongs to the draft. A successful send or confirmed resend/receipt clears it only if no newer choice exists. Failed saves preserve the mode and restored message. Queued-edit backup retains whether the mode was explicit. Leave the browser assertion and timeouts unchanged. ## Verification - Canonical Node24 worktree preflight and plugin SDK prerequisites passed. Server TypeScript passed. - Genuine unchanged-source PostgreSQL RED: two aliases performed four local child recaptures instead of two (one failed, 22 intentionally not selected). GREEN: five source-sharing, last-upload child mutation, capture/cancellation join and actual outer-commit rollback controls passed (18 intentionally not selected; 66.53s). The actual fence-loss control also passed (one passed, 22 intentionally not selected; 38.13s). Both wrappers completed cleanup. - Migration source and tree bytes remain identical to locally qualified `466837cfe997924c3794fd5d0e96134cab1cd04c`; later commits only repair the composer and its tests. - Required planning browser CI originally failed, then failed its one unchanged-head retry, because the planning chip remained absent after opening-question dismissal. Those failures remain recorded ([original](https://github.com/paperclipai/paperclip/actions/runs/42723622165/job/129523785985), [retry](https://github.com/paperclipai/paperclip/actions/runs/42723622165/job/129534415590)). The final-head identical case passes in 30.5s, with all 34 shard cases passing ([final CI](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129550549775)). Assertions and timeouts were not relaxed. - Composer regressions reproduced stale loaded mode, optimistic-save rollback, overwritten next-draft choices, and stale mode after confirmed resend/receipt. All 145 composer tests now pass (2.88s). Canonical UI TypeScript and token gates pass. The original planning browser case also passed locally (15.5s; 26.9s including setup). - The first final-head serialized-suite attempt was automatically cancelled at its unchanged 20-minute job limit. Grouped Turbo output did not expose an individual failing assertion; cause remains unknown ([retained attempt](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129550550373)). One same-head diagnostic retry was a cache miss and executed all 419 heartbeat recovery tests successfully in 376.94s on the identical CI checkout ([retry](https://github.com/paperclipai/paperclip/actions/runs/42731309796/job/129561196792)). It is not classified as infrastructure or a known flake. - Final exact head `ba1f85f18085dd0e84c594ed0cb68d5bf817c93c`: all 61 checks terminal (56 success, four skipped, one neutral); required CI is green ([workflow](https://github.com/paperclipai/paperclip/actions/runs/42731309796)). Fresh Greptile is 5/5 with every review thread resolved ([review](https://github.com/paperclipai/paperclip/pull/20418#issuecomment-6754705323)). Focused commands: ```sh node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts -t "shares the merged child verifier|joins active capture peers|keeps every authority and activity unpublished" pnpm exec vitest run ui/src/components/task-chat/TaskChatComposer.test.tsx pnpm --filter @paperclipai/server exec tsc --noEmit pnpm --filter @paperclipai/ui typecheck pnpm check:token-gates ``` ## Risks - Reuse is limited to one invocation under the same complete source and finalizer fences. Different physical roots, Git roots, recorded child sets and discovery mode have different keys. No local-byte proof is stored durably or reused after restart. - Initial and final maps are separate. A child change during any parent upload must fail the fresh final verification before the atomic publication transaction starts. - Final readers all finish before an error can release the source fence. Per-member database child rows, source fingerprints, journals, keys and publication locks retain their existing checks. - This reduces repeated verification for aliases. It does not change capture concurrency, upload policy or immutable checkpoint validation. Hosted speedup remains unverified. - The CI repair must distinguish task data from explicit draft intent. Only a successful send with the same choice generation clears its override; a later draft selection and failed-save rollback retain it. Regression tests cover these transitions. It adds no styling, request, or persistence changes. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model identifier 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> |
||
|
|
915edcff49 |
fix(workspaces): preserve archived native recovery during migration (#20424)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Durable workspace migration must retain saved files and pending recovery work. > - An archived Git workspace can have an authenticated restore recipe and a terminal native run with pending remote files. > - Its task can select a different workspace. Reopening the archived workspace would change that task. > - The current migration owner refuses this source because it has no live local directory. > - This pull request uses the existing online recovery preservation owner for this exact archived recipe case. > - Migration retains the old task, provider allocation, recovery action, and native sync reference. ## Linked Issues or Issue Description Refs #20298, #20394, and #20418. Related cleanup work in #19812 does not cover this migration case. **What happened?** A historical archived Git workspace with a saved restore recipe and a terminal prepared native sync reference cannot migrate online. Admission reports `remote_copyback_unresolved` and `workspace_recovery_unresolved`. The old task can already select another workspace, so reopening it is not a safe migration path. **Expected behavior** Authenticate the archived files into private scratch. Preserve all typed native and provider recovery obligations in the existing migration journal and markers. Keep the original task selection, history, archive state, and restore flags unchanged. **Steps to reproduce** 1. Archive a Git worktree with staged, unstaged, and untracked files in a saved recipe. 2. Retain a terminal failed native run, its prepared sync reference, and its matching remote allocation and recovery action. 3. Remove the old local worktree and select a different workspace on the task. 4. Call `migrateRemoteWorkspace` without `writersStopped`. **Paperclip version or commit** Reproduced on `b2a7d322ed0bafda04a0b78ce14e4f25c8d5d170`. **Deployment mode** Self-hosted server with PostgreSQL and configured object storage. ## What Changed - Permit the existing online recovery owner to inspect recipe-backed archived Git sources. - Acquire the existing native finalization locks for those sources. Reinspect their exact proofs after lock acquisition and before publication. - Lock related source-task ancestry before publication. - Reuse the existing private Git restore, authenticated archive, journal, marker, and recovery-source contracts. Add no schema or provider calls. - Leave active sources and archived sources without a valid recipe on their existing refusal paths. - Include the shared UI correction from #20418. A nullable draft override follows fresh task data when the user has no local choice. Explicit choices increment a generation. Normal and recovered successful sends clear only their own choice; failed sends and newer choices remain intact. ## Verification - Unchanged master RED: the new historical-source fixture fails with `remote_workspace_migration_blocked:remote_copyback_unresolved,workspace_recovery_unresolved`. - Five focused real PostgreSQL, filesystem, Git, and encrypted-storage tests pass on Node 24.22.2. The success case loses an upload reply, resumes the same operation and snapshot, restores the staged/unstaged/untracked bytes, and verifies unchanged task history, lease, recovery action, finalizer, and recipe flags. - Recipe, prepared reference, and provider allocation changes during the actual upload each refuse publication. A competing native finalization owner refuses before capture. - Migration command: `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'unselected archived recipe|archived prepared source|competing native F owner'`. - All server typecheck layers, module boundary checks, and whitespace checks pass for the migration revision. The migration source bytes remain unchanged through the shared UI follow-ups. - Initial CI run 42727051535 exposed an existing planning-mode browser failure: shard 3/8 passed 33 cases but the refreshed task mode did not appear. The same issue occurred twice on #20418. A real local regression proved that initial cached data could leave the composer in its old mode. This path does not invoke migration. - Review then exposed genuine defects in the initial shared UI repair: an optimistic save rollback could reset a failed draft, and an earlier send could replace a newer draft choice. Recovered delivery also left a consumed choice pinned. All three have real RED→GREEN controls. The final shared fix uses an explicit nullable override, a choice generation, and one attempt-bound settlement owner for normal and recovered success, with no value-equality inference or durable payload changes. - Exact shared UI source now passes all 145 composer tests (2.88s), the original desktop/mobile planning browser case (15.5s; assertions and timeouts unchanged), token gates, and the canonical UI typecheck. The test harness uses the full persisted `IssueWorkMode` type, including `skill_test`, to fix the observed CI typing failure. - These UI results are reused from the identical shared source in #20418 through `ba1f85f18085dd0e84c594ed0cb68d5bf817c93c`. The unchanged planning browser pass is on the explicit-state model before the later settlement-helper extraction. The final receipt/resend controls, full composer suite, typecheck, and token gates include that extraction. They do not claim a local browser run on this entire combined branch. Fresh required CI and review qualify this PR head. - Previous-release state: existing recipe, v3 prepared reference, and journal shapes carry forward unchanged. A saved partial operation resumes with the same IDs. Migration does not certify remote file recovery or provider termination. Live provider recovery and hosted migration remain separate acceptance steps. ## Risks The change admits one more source class into an existing preservation path. An incorrect proof could publish incomplete file authority. The canonical recipe validation, full authenticated restore, native finalization locks, source reinspection, and publication checks remain mandatory. Pending remote data stays pending and retains its original allocation and reference. The shared UI repair updates only an unchanged draft mode; its regression checks preserve a local user choice. ## Model Used OpenAI Codex, GPT-6, with reasoning, repository tools, and code execution. The runtime does not expose a deployment 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 #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
3040448da5 |
fix(workspaces): refresh saved cohort source proofs on resume (#20429)
## Thinking Path > - Paperclip preserves agent work when a control-plane server is replaced. > - Online workspace imports save exact cohort, operation and snapshot identities for restart. > - A cohort proof also includes the complete current companion source scope. > - An independent companion import removes that companion from the local scope. > - The saved cohort then fails resume even when every saved member is unchanged. > - This pull request rechecks the current scope under fresh locks and refreshes its proof for all members in one transaction. > - The benefit is restartable online migration while independent work progresses. ## Linked Issues or Issue Description Refs: #19873, #20394, #20418. Related PR search found the original cohort owner, recovery-proof refresh and separate child-verification reuse. This fix addresses cohort scope freshness. **What happened?** An interrupted cohort rejects resume with `workspace_cohort_membership_changed` after a nonmember companion is migrated. The saved source proof includes companion workspace and task IDs. The exact saved members and their fingerprints can remain unchanged. **Expected behavior** The same saved cohort must resume after fresh admission. It must keep all member, operation and snapshot IDs. Changed members, active owners, changed journals and lost source locks must still refuse publication. **Steps to reproduce** 1. Start a two-member cohort and interrupt the second upload after the first archive is saved. 2. Import an archived nonmember companion through the existing canonical migration owner. 3. Resume the same two-member cohort. The original implementation refuses before it acquires the source fence. **Paperclip version or commit** Reproduced on master `3a5572c9e5`. **Deployment mode** Online administrative workspace migration with PostgreSQL and authenticated snapshot storage. ## What Changed - Admit a changed aggregate scope proof only when every saved member has a running journal and its current fingerprint matches both saved fingerprints. - Keep partial reservations on the original strict aggregate-proof path. Keep exact cohort and operation membership checks. - Run the existing source, lifecycle, recovery and finalizer admission before refresh. Lock member rows, inspect the current scope again, and compare every journal in one transaction. - Change only `cohortMigration.sourceProof`. Preserve all other metadata, identities, archive and recovery records. Abort the complete refresh on cancellation, a changed journal or a lost fence. - Extend the existing real PostgreSQL fixture with partial upload, canonical companion migration, refusal controls, transaction rollback, cold archive restore and replay. - Document the proof lifetime and resume boundary. ## Verification - Canonical Node24 worktree preflight passed. Plugin SDK build prerequisites passed. - Genuine RED on unchanged product: the real partial-upload and companion-import sequence fails with `workspace_cohort_membership_changed` (1 failed; 22 intentionally not selected; 10.44 seconds). - The identical positive passes after the fix (1 passed; 22 intentionally not selected; 9.87 seconds). - The complete focused cohort file passes: 23 tests in 33.33 seconds. Existing controls include changed member fingerprints, foreign and completed journals, active operations, peer cancellation, actual source-fence connection loss and outer-commit failure. - The new sequence also checks a held source lock, a held finalizer lock, cancellation before refresh, and an injected second-member update failure. The failed update rolls back every proof change. Both saved archives restore tracked and untracked work, and replay uses the same cohort. - After one final read-only preflight assertion, that complete new sequence passes again in 14.52 seconds with 22 existing cases intentionally not selected. Server TypeScript passes on the final files. Module boundaries and `git diff --check` pass. - Commands: `pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts`; final case selection: `-t 'same uploaded cohort'`; `pnpm --filter @paperclipai/server exec tsc --noEmit`; `node scripts/check-module-boundaries.mjs`. - Initial local setup attempts remain nonpasses: sandbox PostgreSQL could not bind localhost, then the new archived companion fixture used a noncanonical repository folder. The corrected fixture uses the existing managed repository path. Neither setup failure is counted as the product RED. - Fresh [Greptile review](https://github.com/paperclipai/paperclip/pull/20429#issuecomment-6755862789) is 5/5 on exact head `6c640c4667b136e5416c1375f491729cf4bb9b49`, with zero inline threads. Required final-head CI is running. Hosted acceptance is not claimed by this PR. ## Risks - The old aggregate proof contains only a hash. Refresh therefore requires durable fingerprints for every saved member and repeats all current admission guards. A partially reserved cohort cannot use this path. - A stale proof must never hide a changed member or a new writer. Current source inspection, P/I/L/F locks, locked member rows, complete journal compare-and-swap and the existing capture and publication checks remain authoritative. - The all-member update must stay atomic. A database fault or cancellation rolls it back. The original cohort, operation and snapshot IDs remain fixed. - The change adds one bounded transaction only when a fully saved cohort's scope proof changes. It adds no recurring work, schema, provider, archive format, concurrency or deployment changes. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The runtime does not expose a more specific deployment identifier or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
3a5572c9e5 |
Extend login PTY contracts for existing computers (#20410)
## Thinking Path
> - Paperclip manages agent execution across environments.
> - Environment providers own execution resources.
> - Codex and Claude already share the login PTY transport.
> - A login on an existing computer needs its own allocation and
deadline.
> - Input retries must retain their identity.
> - Codex also needs a fixed credential-file read; Claude already
returns its token through PTY output.
> - This change extends the shared contract and keeps both existing
provider parsers.
## Linked Issues or Issue Description
**Problem**
The existing login contract assumes a temporary sandbox. It cannot
describe a login allocation on an existing computer or export its fixed
credential result without generic execution.
**Proposed solution**
Add shared allocation purpose, command identity, PTY deadline, and input
identity fields for both Codex and Claude. Add one optional
credential-result hook for Codex's auth.json file.
**Alternatives considered**
A separate Claude login implementation would duplicate the current
setup-token parser and owner-bound credential writer. Claude continues
to use that existing path. The new artifact hook is Codex-specific
because Claude's setup-token result comes through the existing shared
PTY output stream.
## What Changed
- Export the login contract types from the main SDK entry point.
- Forward login purpose and command through plugin allocation and retain
them in lease metadata.
- Forward the PTY deadline and give each input write a stable identity.
- End a login route when the worker cannot confirm input delivery.
- Preserve the opening company scope for input, stop, and close
callbacks.
- Advertise and dispatch the optional fixed Codex credential-result
hook.
The allocation, deadline, input, output, and cleanup contracts serve
both providers. This PR does not enable either provider on an existing
computer. The dependent consumer PR connects admission, services,
credential completion, and UI for both providers.
## Verification
- Close-ordering regression: all 91 host worker-manager tests passed,
including concurrent close callers waiting behind a held plugin
acknowledgement after input failure. Server TypeScript check passed. All
current CI gates, ordinary review, and the selected follow-up review
pass on
|
||
|
|
347d85fef3 |
perf(chat): index GitHub review lookups and share review-wait reads (#20400)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub chat endpoints let agents review pull requests. The server reads review state when it lists issues and when it updates GitHub checks. > - On a hosted instance, two GitHub review queries used the most total database time. One query ran about 660,000 times at about 33 ms. The other query ran about 10,000 times at about 1 s. > - The first query finds a source comment's chat link. No index covers `chat_message_links.comment_id`, so each call reads the full table. Issue lists also read the same review waits two times per request. > - The second query joins deliveries, links and runs on a JSON key. The planner walks the agent's run history in creation order and reads each run's context. > - This pull request adds four concurrent indexes, splits the run lookup into two small reads, and shares review-wait reads in one issue-list request. > - The benefit is less background database load, so request reads wait less. The results stay the same. ## Linked Issues or Issue Description **What existing behavior does this improve?** GitHub review state reads. These are the inherited review-wait lookup by source comment, and the review-check lookup of the run that a delivery's comment woke. **Subsystem affected** Server chat channels (GitHub review waits and checks), issue list attention maps, and the database schema. **Current behavior** - `readInheritedGitHubReviewWait` filters `chat_message_links` by `company_id`, `comment_id` and `direction`. No index has `comment_id`, so PostgreSQL reads the full table on each call. The run-origin check does the same lookup first. - An issue list calls the external review-wait read from the review attention map and again from the blocked-inbox attention map. The two maps read the same in-review issues. - `githubReviewCheckState` joins deliveries, links and runs with `context_snapshot->>'wakeCommentId' = comment_id::text`, sorts by `created_at desc` and takes one row. The planner reads the agent's runs in creation order and reads each run context. If no run matches yet, it reads all of them. The delivery filter on `normalized_event` and the link join on `delivery_id` also have no index. **Proposed behavior** The same rows come back from small index reads. Each issue's review wait is read one time per issue-list request. **Reason and benefit** These two statement families used about 33,000 s of database time in one cumulative statistics window. The database had about 50 concurrent active queries, and request reads waited behind this background work. **Breaking changes** None. The migration adds indexes only. ## What Changed - Migration `0456_chat_review_lookup_indexes` builds four indexes with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. It runs through the existing top-level statement path, which repairs an invalid index on retry: - `chat_message_links_company_comment_idx` on `(company_id, comment_id)` where `comment_id is not null`. - `chat_message_links_delivery_idx` on `(delivery_id)` where `delivery_id is not null`. - `chat_deliveries_github_automatic_delivery_idx` on `(endpoint_id, normalized_event->'githubAutomatic'->'context'->>'deliveryId')` where that value is not null. - `heartbeat_runs_company_wake_comment_idx` on `(company_id, context_snapshot->>'wakeCommentId')`. This index is not partial. The planner does not use expression statistics from a partial index. Without these statistics, it estimates many matches and keeps the creation-order walk. - `githubReviewCheckState` reads the delivery's comment IDs first. If there is a comment, it reads the newest same-agent run with one of these `wakeCommentId` values. If there is no comment, it skips the run read. The comparison stays text equality on `comment_id::text`. - `createExternalReviewWaitReader` shares one read per issue between the review and blocked-inbox attention maps of one issue-list request. The reader exists only for that request. - `doc/DATABASE.md` describes the indexes and the new read order. - `scripts/benchmark-review-lookup-indexes.ts` compares the pre-fix and new code with and without the indexes. `scripts/benchmark-review-lookup-index-migration.ts` times the migration on a large synthetic database. ## Verification Local SQL measurements on a synthetic fixture. They are not production timings. Environment: embedded PostgreSQL 18.1, baseline revision `02bfe71a6b`. Fixture: 50,000 links, 5,000 deliveries with about 2 KiB events, and 4,000 runs with about 12 KiB incompressible contexts. The wake run is in the middle of the run history. Each value is the median server execution time from `EXPLAIN ANALYZE` (2 warmups, 9 samples). The order was: no indexes, indexes, then no indexes again. | Caller and statement | Before (no indexes, two passes) | After | Shared buffers before → after | | --- | --- | --- | --- | | Inherited review wait, link by source comment (same SQL) | 2.72 / 2.36 ms | 0.076 ms | 929 → 12 | | Review check, wake run found | 31.9 / 21.9 ms | 0.29 ms (two statements) | 25,776 → 62 | | Review check, no wake run yet | 71.0 / 47.5 ms | 0.06 ms (two statements) | 34,861 → 12 | - Indexes alone do not fix the review check. The old SQL with the new indexes took 8.6 ms (run found) and 29.7 ms (no run), because it still walked the run history. The new SQL without the new indexes was neutral (23.5 and 35.1 ms). - Service wall times on this shared machine were noisy, so the table reports server execution time. - Correctness: the benchmark fails unless the inherited statement returns the same positional rows in each index state. The check service result and the final wake-run rows of the new code must also match the pre-fix module for both cases. - Migration timing, normal migration runner, synthetic data: 200,000 runs (1.8 GB with 8 KiB incompressible contexts), 500,000 links and 100,000 deliveries. First run 4.2 s; rerun 0.38 s. Separate builds: links by comment 1.5 s, run wake comment 1.1 s, deliveries 0.7 s, links by delivery 0.3 s. All indexes were valid. The run index was 8.3 MB. - New tests: - `server/src/__tests__/github-review-check-state.test.ts` checks that the newest same-agent run is selected. It also checks that other agents, other companies, uppercase IDs, non-string values, list-only `wakeCommentIds`, comment-less links and unknown deliveries do not match. It checks that the generated statements can use the new indexes. - `server/src/services/native-runtime/external-review-wait.test.ts` checks that the reader reads each issue one time, keeps concurrent callers correct, and rejects all waiting callers when a read fails. - Red/green: with the pre-fix `review-check-state.ts`, the behavior test passes and the index test fails. With this change, both pass. - Focused runs pass: `issue-blocker-attention`, `heartbeat-issue-liveness-escalation`, review-wait, review-check-state, external-review-wait, review-repair (106 tests), and `external-chat-wait.integration` (285 tests). `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-indexes.ts --baseline-ref=02bfe71a6b pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-index-migration.ts ``` ## Risks - The migration builds four indexes concurrently. It does not block writes. On a large database, the run index reads every run context one time. The local run took 1.1 s for 1.8 GB from a warm cache. Cold storage on smaller compute can be slower. - The run index adds one entry for each run insert and for each run update that is not a heap-only update. Many runs have no `wakeCommentId`, so these entries hold a null key. - The review check now sends two statements instead of one when a delivery has a comment. Each statement reads only a few index entries. - When two matching runs have the same `created_at`, either can be selected. The previous query had the same tie behavior. - This change does not change the number of in-review issues that one issue list reads. The blocked-inbox map still reads all in-review issues of the company. A smaller scope needs a separate change to the liveness classifier. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) by Anthropic, in Claude Code with tool use (shell, file edits, local PostgreSQL benchmarks). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
326df52673 |
perf(api): rank Mine inbox candidates from the participation set (#20413)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The Mine inbox reads `GET /api/companies/:companyId/inbox/mine`. Its
main query is `issueService.list` with "touched by me", "not archived by
me", six statuses including `done`, and `LIMIT 100` by canonical last
activity.
> - That query evaluates `touchedByUserCondition` (a five-way OR of
EXISTS) on every status-matching issue in the company. Because `done` is
included, the scan grows with company history, not with the user's work.
> - The unread badge already solved the same problem with a UNION of
indexed participation sources (#17973). The list path did not.
> - This pull request makes the Mine list rank page ids from that same
participation UNION, then load full rows (with the expensive select-list
subqueries) only for the 100 selected ids.
> - The benefit is that the main Mine query scales with the user's
participation instead of the company's issue count. Results stay
identical, and a new test proves it against the original scan.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The latency of the Mine inbox snapshot, `GET
/api/companies/:companyId/inbox/mine`.
**Subsystem affected**
Server: issue list service (`server/src/services/issues.ts`) and the
Mine route.
**Current behavior**
Measured on 2026-11-09: 280–570 ms server time on a dev instance and
1.9–3.2 s on staging. The list query scans every status-matching issue
(28k of 30k in the fixture below) to test participation, then sorts the
touched ones.
**Proposed behavior**
The list query reads only the user's participation set and computes the
row projection only for the page.
**Reason and benefit**
Inbox opens are frequent. The old cost grows with every completed task
in the company.
Refs #17973 (same UNION for the unread badge).
## What Changed
- Extracted the participation UNION from `unreadInboxIssueIds` into
`mineParticipationIssueIds()`. The unread badge uses it unchanged.
- New `IssueFilters.touchedCandidates: "participation"`. When it is set
with `touchedByUserId` (and there is no search and no
`oneChildPerParent`), `list`:
1. replaces `touchedByUserCondition` with `issues.id IN (<participation
UNION>)`. Each UNION member is one OR branch of the old condition, so
the set is the same;
2. runs a narrow `candidates` query (`SELECT id … WHERE <all other
filters> ORDER BY <same order> LIMIT/OFFSET`);
3. loads the full projection for only those ids (re-checking company,
visibility and the read condition), and restores the phase 1 order.
- Only the Mine route sets the flag. Every other `svc.list` caller keeps
the old SQL.
- A new Server-Timing phase, `inbox_issues_candidates`.
- `server/src/__tests__/inbox-mine-candidates.test.ts`: a
result-equivalence test.
- `scripts/benchmark-inbox-mine.ts`: a local benchmark that seeds a
large fixture, compares full payloads, and records `EXPLAIN (ANALYZE,
BUFFERS)` per query.
### Not changed, with evidence
- **Recomputed comment/activity maxima** (`lastActivityStatsForIssues`):
0.9 + 0.2 ms per call in the plans. Not worth a second code path.
- **Touched condition inside `userCommentStatsForIssues`**: 1.4–2.1 ms.
Not changed.
- **`liveDescendantCountMapForIssues`**: 12 ms at 30k issues and 40 ms
at 90k. It now costs about as much as the list query. It scans all live
runs and hashes all company issues. It is company-wide work, outside the
candidate selection, so I left it for a separate change.
## Verification
### Correctness
`inbox-mine-candidates.test.ts` runs the old scan and the new path on
the same data. It asserts equal ordered ids and equal full JSON
payloads. Fixture: 170+ touched issues (more than one page, with tied
timestamps) plus these edge cases:
- included: created by me, assigned to me only, user activity, my direct
comment, `done`, pending request addressed to me, archived then
re-activated by a newer outside comment, read state with a later agent
comment, a live child (live descendant count);
- excluded: archived with no newer activity, mention-only, read audit
only (`issue.read_marked`), uppercase entity id, channel-sourced
comment, agent-created with my user id, `cancelled`, hidden, harness,
plugin operation, private to another user (`issueReadSqlCondition`),
answered request, untouched issues, another company.
It also compares other shapes: no archive filter, `status=done` with the
full projection, `offset`/`limit`, ascending order, unrestricted read
condition, a second user, and a user with no participation. Mutation
check: removing the comment source from the UNION makes the test fail.
The benchmark also asserts identical full payloads for every sample of
both arms.
Focused suites (local): `inbox-mine-candidates`, `inbox-mine-routes`,
`authorization-privacy-query`, `issues-unread-inbox`,
`inbox-archive-routes`, `issues-assignee-unread`,
`issues-creation-visibility` — all pass.
### Performance (local SQL measurements)
Command: `pnpm --filter @paperclipai/server exec tsx
../scripts/benchmark-inbox-mine.ts --baseline-ref=18c3f4ef7b
--issues=<N> [--touch-density=<d>]`. Baseline =
|
||
|
|
4a2eb920cf |
fix(storage): wait out a store outage on a run's critical path instead of failing it (#20299)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A cloud instance keeps run logs and evidence in object storage, so a run touches the store as soon as it starts > - In storage chaos wave `w-20261109T145628-s9902` (ST1: 75–176 s windows where every S3 call returns 503), only 1 of 8 tasks finished. A run starting or resuming during an outage failed within a second as `adapter_failed`, generic recovery burned its attempts seconds apart, the task was escalated to the board, and the user saw the raw S3 error > - This pull request treats a transient storage error as an infrastructure wait: the run is retried on a minute-scale schedule, rapid recovery is suppressed, the board shows it as working, and the run error says storage was unavailable > - The benefit is that a store outage of several minutes delays work instead of failing and stranding it ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261109T145628-s9902` (master `610b54f551`, `--storage s3`, ST1). **Where the run fails.** At run start or resume, `run-output` `begin` opens the run log: `getRunLogStore().begin` → object run log store → object byte journal `claim` → S3 conditional PUT. The journal retries only 3 times within about 300 ms, so a 503 escapes and fails the run as `adapter_failed`. The async stack ends in the SDK middleware with no app frame. `log_store` stays null on the failed run, which pins the claim. **What followed.** - `failRun` released the issue into generic immediate recovery. Recovery retried 1–2 s apart inside the outage (t01: 14:58:33, :35, :59, 14:59:01), then stopped and opened a board-owned `stranded_assigned_issue` action. These are the open recovery actions on CHA-1, CHA-4, CHA-7 and CHA-8; a later Retry hit the outage again and escalated the same way. - The run error was the raw S3 text ("Injected storage outage"). **Slow store (ST2, `w-20261109T151946-s9903`).** With about 9.7 s of added latency per call, the same claim hit the journal's 15 s per-operation deadline (`AbortSignal.timeout(15_000)`). The run failed after about 36 s with `AbortError: Request aborted` and burned 3 runs the same way; 4 of 8 tasks needed a Retry. t02/t03 (Retry parked with `process_identity_missing`) are a separate native-recovery class and are handled in a follow-up. **Daytona storage chaos ST1 (`w-20261109T161202-s9912`, 1/8):** with S3 returning 503 for 72 to 154 s at a time: - Workspace restore failed at once (`remote_workspace_restore_failed: step=snapshot_download; storage=provider_error`), three times in 40 s, then `setup_failed`. It left an active recovery action behind. - Runs reattached after a restart died within 2 s on the run-log claim, then became reconciliation-required. - Setup failed on other storage reads. - 6 of 7 failures needed a UI Retry. So the rule here is one class rule: a transient store error on a run's critical path waits, it never fails the run. ## What Changed - New `server/src/storage/storage-unavailable.ts`: - `isStorageUnavailableError` reads the recorded storage failure diagnostics first, then the SDK error shape. Unavailable means 429/500/502/503/504, the S3 throttling and timeout names, network codes (`ECONNRESET`, `ECONNREFUSED`, `ETIMEDOUT`, `EPIPE`, `EAI_AGAIN`, `ENOTFOUND`), or an `AbortError` whose cause is a `TimeoutError` (a per-operation deadline). 403, 404 and 412 are not outages, and neither is an abort caused by the run's own Stop. - It defines the `storage_unavailable` code, the `storage_unavailable_wait` retry reason, the retry policy (60 s × 10 attempts, about 10 minutes) and the user-facing message. - `server/src/services/heartbeat/run-completion.ts`: - `failRun` classifies such a failure as `storage_unavailable` with the plain message. - Unless native recovery owns the run, it schedules the storage wait with `scheduleBoundedRetryForRun`, skips the interaction-continuation retry, and releases the issue with `suppressImmediateRecovery`. - `failRunSetup` uses the same policy for this code. - `server/src/services/heartbeat/retries.ts`: `storage_unavailable` joins the transient infrastructure codes. It gets its own retry budget and policy, like `runner_revision_mismatch`. - `server/src/services/task-interruption-presentation.ts`: `storage_unavailable_wait` is a resource wait, so the board shows the task as working with no Retry while it waits. - **One shared outage wait for the run's critical path** (`server/src/storage/storage-unavailable.ts`): - `retryThroughStorageOutage` retries a transient store failure with backoff (2 s doubling to 30 s) until that storage operation's outage has lasted 5 minutes, then gives the error back. The outermost wait of an operation records its first failure, and nested and parallel waits inside it share that start; no other request's success extends it. Shared probes and shared reads run under their own scope, so one run's Stop never cancels work another run waits on. A stop signal ends the wait, and non-outage errors are never retried. - Inside `waitOutStorageOutages`, every replayable S3 request waits instead of failing: get, head, delete, buffered and file puts, and multipart create, file parts and complete. A consumed upload stream is never resent, and requests outside the scope keep failing fast. - The scope covers run execution (`heartbeat.ts` `executeRun`: setup, workspace restore, run-log claim and the run itself), plus `loadWorkspaceSnapshotArchive` and `uploadWorkspaceSnapshot`, which copy-back and recovery workers use too. - The run-log claim retries as a whole, because the journal bounds each operation with a 15 s deadline. - Past the budget, the run takes the bounded `storage_unavailable` wait below. - The classifier follows the error's `cause` chain, so a restore failure wrapping an outage counts. `failRunSetup` checks storage before restore failures, so that case takes the storage wait instead of two quick retries and an escalation (the R6 leak). - `object-byte-journal.ts`: when its deadline expires, the journal rethrows the recorded storage error instead of a bare `TimeoutError`. ## Verification Red/green (each new test fails against master and passes with this PR): - `run-completion.test.ts` "waits a minute and suppresses rapid recovery when a run fails because storage is unavailable (storage chaos ST1)". An S3-shaped 503 records `storage_unavailable` with the plain message (no raw S3 text), schedules `STORAGE_UNAVAILABLE_RETRY`, releases with `suppressImmediateRecovery: true`, and skips the continuation retry. "keeps an ordinary adapter failure on the normal recovery path" guards the default. - `task-interruption-query.test.ts` "does not present an overdue storage_unavailable_wait wait as a failed attempt": working, `canRetry: false`. - `storage-unavailable.test.ts`: which errors count as an outage and which do not, including recorded diagnostics, a deadline-aborted call (ST2), and a Stop-aborted call that must not count. Static checks: - `tsc --noEmit` passes for `server`. Live verification: Phase 3 will rerun ST1 (seed 9902) with the MinIO fault proxy after merge. - `server/src/__tests__/storage-outage-wait.test.ts` runs against a local S3 that answers 503 for 3 s: - Inside the scope, a snapshot `getObject` and a run-log conditional PUT succeed once the store is back. - Outside it, a request fails fast. - A stop ends the wait. - Unit cases cover the budget, non-outage errors, and a `RemoteWorkspaceRestoreError` wrapping an outage. - `run-completion.test.ts`: a setup failure from a restore that wrapped an outage takes the storage wait (red before). - `object-byte-journal.test.ts`: a real `claim` whose head PUT hits its deadline is classified as a storage outage (red before). ## Risks - A storage outage now holds a task for up to about 10 minutes of automatic waits instead of escalating within seconds. A longer outage still ends in the existing board recovery. - A run whose failure is misclassified as storage-unavailable waits instead of using fast recovery. The classifier accepts only server-side or network failures from the storage SDK, never 4xx answers. - No schema change. - A run can now wait until a storage outage has lasted 5 minutes before it fails over to the bounded wait. It shows as working throughout, and Stop ends the wait. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |