mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
d33b3ac2632442acb816a1ebe551258561e79d27
649
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
d33b3ac263 |
fix(wake-queue): save temporarily refused work intents and promote them through one path (#19956)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Users send work to agents as assignments, comments and Retries. The wake queue delivers each one as a run > - When an admission check refused a wake for a temporary reason, the wake often ended as a terminal skip, so the work was lost > - Restart-chaos waves found dropped comments and Retries (RC-37, s7532) and double delivery (RC-53). Each fix covered one hold type > - The durable ownership plan (section E) asks for one outbox rule: a closed set of terminal outcomes, a saved deferral for every temporary refusal, and one promotion path > - This pull request adds that rule. It saves messages refused by a pause, promotes deferred intents from every hold-release event through one function, and adds a reconciler backstop > - The benefit is that an accepted message is delivered exactly once after a pause lifts, and the reconciler reports any intent that is stuck or dropped ## Linked Issues or Issue Description Refs #19831 (plan, section E), #19838 (inventory, wake-queue rows and section 1b), #19927 (Phase 2 data: the I4 false positive), #19702, #19741, #19829, #19826. Builds on #19944 (section D, merged). **What happened?** - A user commented while the agent was paused, or while the company was paused. Admission refused the wake. Nothing saved the message as pending, so when the pause lifted, nothing delivered it. - Hold-release events woke different subsets of workers. A saved intent then waited out a 30-second refusal backoff after its hold cleared. - The I4 reconciler check matched free text with a regular expression. Phase 2 found a false positive: a delivered retry whose error said "retrying once". **Expected behavior** An accepted work intent ends only as delivered, consumed by a later run, task cancelled, or hard stop. Every temporary refusal saves the intent as `deferred` with its reason and its release trigger. Every hold-release event promotes deferred intents through one function (invariant 4). **Steps to reproduce** Pause an agent. Post a user comment on its task. Resume the agent. On `master` the comment is never delivered as a wake. ## What Changed - New `server/src/services/work-intent-outbox.ts`: - `WORK_INTENT_TERMINAL_OUTCOMES`: the closed set of terminal outcomes. - `TEMPORARY_REFUSAL_TRIGGERS`: maps each temporary refusal code to its release trigger. - `promoteDeferredWorkIntents`: the one promotion function. It removes the 30-second refusal backoff from the matching deferred intents. It then wakes run dispatch, the saved-receipt worker and stranded-work recovery, and hints each saved intent of the issue. - `heartbeat/queue.ts`: the #19829 save-as-deferred block is now a shared helper. A user's message refused by a paused agent, a paused company, or a budget pause that is not a reached hard stop is saved with `executionWait: { reason, trigger, message }`. The caller still gets a 409. The message is saved only when the task's latest run is terminal, because the release drain promotes a saved message behind that run. A task that never ran keeps the message in the task, and stranded-work recovery starts the task once the pause lifts. A `pending_approval` agent and a task-tree hold are not changed here: the release drain still fails a promoted wake for a pending approval, and tree holds have their own resume path. - Hold-release events call `promoteDeferredWorkIntents`: - accounting settled (`run-cost-accounting.ts`); - environment lease released (`environments.ts`, two sites); - native run projected terminal (`native-run-projection.ts`); - budget pause lifted (`budgets.ts`); - company reactivated (`companies.ts`); - task-tree hold released (`issue-tree-control.ts`); - agent lifecycle ready (`agent-lifecycle-driver.ts`). - Reconciler (`recovery/ownership-registry.ts`, `ownership-invariant-report.ts`, `native-finalization-reconciler.ts`): - registers three new kinds: `wake_request_deferred`, `queued_run` and `scheduled_retry_run`; - I4 now reads the closed refusal code (the wake `reason`) of accepted intents, not free text; - a new I4 check reports a deferred pause refusal whose pause has lifted for more than 5 minutes. The worker then promotes it as a backstop; - a lifted pause inside its 5-minute grace window returns its deadline through `nextDueAt`, and the worker also wakes on agent-lifecycle signals (company and agent pause changes). So a missed release event is still checked. - `doc/SPEC-implementation.md`: adds the rule "Accepted work intents are delivered exactly once". ## Verification Each focused file ran alone in one vitest process (`--maxWorkers=1`) on embedded Postgres. Results at `2875c85fd2`: | Question | Loop | Elapsed | Result | |---|---|---|---| | Is a comment refused by a pause saved, then delivered once after the pause lifts? | `heartbeat-held-comment-wake.test.ts -t "pause lifts"` | 8 s | 2 pass. With `queue.ts` from `master`, both fail: no intent is saved | | Does I4 use the closed code, and does it catch a stuck deferral? | `ownership-reconciliation-report.test.ts` | 45 s | 20 pass. This includes the Phase 2 "retrying once" false positive and the case "paused, then lifted" | | No regression on the paths that changed | held-comment (15), archived-company guard (8), undispatched execution wait (32), deferred receipt work (45), recovery pause durability (16), native run projection work (55), tree-control routes and services (19, 1, 7) | 8–334 s each | All pass | - Exactly-once: each pause test runs two promotions (a release event, then the reconciler backstop) and many dispatch passes. It asserts one delivery. The existing consumption tests (RC-53) still pass. - Server package typecheck passes. - Speed: nothing new runs before `turn.start`. The new admission work happens only on a refusal. Promotion runs in the release transaction and in the background reconciler. - Not run here: chaos waves. The coordinating session schedules them on the shared harness. ## Risks - `promoteDeferredWorkIntents` moves `updated_at` back for deferred intents that were refused in the last 30 seconds. A busy company with many release events can retry refused intents more often. Each retry re-runs normal admission, and the refusal resets the backoff. - More release sites now wake run dispatch and stranded-work recovery. These notifications are coalesced per company. - Lifting a pause is now the operator's explicit authorization to send what the pause refused, as Send is after a Stop. When the pause stopped the task's run, the lifted pause issues the task's next explicit wake, which adopts the saved message once. Other messages queued behind an acknowledged Stop keep the existing rule. If Stop recovery still holds the task, the message waits for that recovery. - An agent pause's stop sweep keeps the messages that the pause deferred (`preservePauseDeferred`). Terminate and cleanup stops still end them. - The backstop check reads only pause refusals (`company.inactive`, `agent.not_invokable`), because those holds can be checked exactly. Other deferred kinds keep their existing promotion paths. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f755361ffe |
fix(recovery): deliver a steered message the restart lost before the model read it (#19962)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user can steer a running Codex or Pi turn: a comment goes into the live turn instead of waiting for the next run > - The provider acknowledges the steer at once, but the model reads it only at its next step, after the in-flight tool call > - A control-plane restart in that window kills the provider with the message in memory. Restart recovery starts a continuation turn that does not carry the message. The message is lost and no error shows > - The restart chaos harness showed this in 6 of 6 steered runs (Codex, K1, K2 and K6, 2 samples each): steer acknowledged, run succeeded, no reply to the steered message > - This pull request makes restart recovery deliver every acknowledged steer that the model did not read, exactly once > - The benefit is that manual Steer no longer loses messages on restart, and the planned automatic steering can ship on top of it ## Linked Issues or Issue Description **What happened?** A comment steered into a running Codex turn got no reply when a control-plane restart (SIGKILL, SIGTERM or crash loop) happened during the agent's tool call. The run succeeded, and the steered comment had no effect. **Steps to reproduce** 1. Start a Codex task that runs a 40-second command. 2. During the command, post a comment and click Steer. The provider acknowledges it. 3. Restart the control plane before the command ends. 4. Recovery resumes the turn with a continuation. The agent finishes the task and ignores the comment. **Expected behavior** The agent reads and handles the steered comment once, after recovery. **Root cause** The provider keeps acknowledged steering input in memory until its next model step. The restart continuation (`paperclip.native-continuation.v1`) only told the model to continue the current request, so the acknowledged input was gone. ## What Changed - `unreadSteeredMessageIds` (runner package): derives acknowledged-but-unread steers from run-log events in order. A `steering_acknowledgement` item adds the message id (once per id). A `userMessage` item in the same turn reads the oldest unread steer. Codex records each steered input as one `userMessage` item when it enters the conversation. A `userMessage` item in a later turn reads all earlier steers, because the continuation carries them. `turn.completed` reads all steers. - `readUnreadSteeredMessages` (server): reads those facts from `heartbeat_run_events` (only transport-validated events with a `sourceEventId`). It also reads the comment or interaction text for the same company and issue. The run log is the durable record of both "acknowledged" and "read", so no new column is needed. - The restart continuation now includes `steeredMessages` and one instruction to handle them, oldest first. The runtime reads them through a lazy `getUnreadSteeredMessages` option, only when a restart continuation starts. Native recovery wires it for every run. - `queuedInteractionSteeringBody` is extracted, so recovery rebuilds a steered interaction response with the same text the steer sent. - Fake Codex gets `--consume-steer`, which emits the `userMessage` item for an accepted steer, like real Codex. ## Verification - Red/green live test with real runnerd and fake Codex (`runnerd-restart-continuation.test.ts`, "delivers a steered message the restart lost exactly once"): start a held turn, steer it, SIGKILL runner and provider, restore and recover. - "unread": the continuation carries the message exactly once. Without the runtime change this test fails. - "read" (`--consume-steer`): the continuation does not carry it again. - Both: after the continuation completes, a later restart has nothing to deliver. - `steering-delivery.test.ts`: ordering, dedupe, initial-input, later-turn and completed-turn rules. - `unread-steered-messages.test.ts` (embedded Postgres): ignores events without a source id and messages from other issues. It reads interaction text, ignores model-output items as read evidence, and returns nothing after the continuation input. - `pnpm vitest run src/steering-delivery.test.ts src/live/runnerd-restart-continuation.test.ts` (runner) and `pnpm vitest run src/services/native-runtime/unread-steered-messages.test.ts` (server): pass on the rebased revision. - `issue-queued-comments-routes.test.ts` and `native-session-runtime.test.ts -t restart`: pass. `native-session-resume.test.ts` "archives a damaged prior epoch…" fails locally on plain `origin/master` too (local runnerd binary); CI runs it with a fresh build. - Restart chaos (Codex steer mode, K1/K2/K6) runs after this lands in the harness slot. Before: 0 of 6 steered messages answered. ## Risks - Duplicate delivery: a provider that records steered input without a `userMessage` item, or a `userMessage` event lost in the crash window before persistence, makes recovery send the message again. The model then sees it twice. That is safer than losing it, and the instruction asks the model to handle each one once. - Pi (ACPX) steering has no `userMessage` read evidence today, so a Pi steer that the model already read before a restart may arrive again in the continuation. - Only the restart continuation carries the messages. Other recovery paths start after a completed turn, which already read every steer. - No schema or migration change. A session saved by an earlier release resumes normally; its run log already holds the acknowledgement events. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
bf3a067076 |
feat(recovery): settle the finalizer and cleanup leases on the shared ownership path (#19950)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs keep their ownership in `native_run_finalizations`, and the shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The finalizer lease (`native-finalizer:*`, 5 min) and the retained-session cleanup lease (`native-cleanup:*`, 60 s) recorded only a random token. They named no holder process > - A server killed while it finalized or cleaned up kept the run waiting for the whole lease. The lease was released by age alone. A graceful shutdown also keeps an in-progress finalizer lease for the next boot to wait out > - This pull request records the holder process with each claim and moves both kinds onto the shared prover and a compare-and-swap settle > - The benefit is that a dead holder's run continues in the first pass after boot, and a live or unprovable holder is never released ## Linked Issues or Issue Description Refs #19933 (the settle hook and the first moved kind, merged). **What happened?** A finalizer or cleanup lease whose server process died stayed held until the lease expired (5 min or 60 s). The run stayed `running` and its follow-up work waited. **What did you expect?** The next boot proves the dead holder gone and frees the lease at once. A live holder, or one that cannot be proven gone, keeps its lease. **Steps to reproduce** 1. Accept a native result and let the finalizer claim it. 2. Kill the server (SIGKILL) before the finalizer commits. 3. Start the server. The run stays `running` for up to 5 minutes. ## What Changed - `native_run_finalizations.lease_holder` (jsonb, migration 0446): `{token, bootId, pid, hostHash, processStartedAt}` of the process that took a token-only lease. It names the holder only while `token` equals `lease_owner`, so renewals keep it and a later lease never inherits it. The executor's `controller_*` columns are not changed. - The finalizer claim (`native-run-finalizer.ts`) and the cleanup claim (`native-session-executor.ts`) write it in the same UPDATE as the lease, from `currentControllerProcessStamp()` (no I/O). - New `native-token-lease-settlement.ts`: compare-and-swap settle that expires exactly the proven-gone holder's outstanding lease, after it notifies the finalization or cleanup work that the expiry frees. - Both claims also wake the ownership worker queue in their transaction, so a quiet worker watches the new holder (Greptile round 1). - `ownership-registry.ts`: `native_finalizer_lease` and `native_cleanup_lease` register `settle`. Their owner is the holder process; rows without a holder keep the token fence. Listings cover outstanding leases only. - `heartbeat/recovery.ts`: restart recovery settles both kinds before it finalizes and schedules cleanup. A failure is logged and the reconciler pass retries it. - Two sparse partial indexes on `lease_expires_at` for the two listings, so the settle watch re-check stays indexed. Bespoke code deleted: none. These kinds had no liveness check; their only release was lease age (the takeover in the claim), which stays as the fence for unprovable holders. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t "finalizer|cleanup|holder|exact lease"`: 12 passed (rerun after the rebase on master). The crash-point test also asserts the ownership wake for its run; it fails without the signal. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red on the base branch (server code at `bea3bfb20b`, tests and schema from this branch): 7 new tests fail, 5 pass (the 5 are existing tests). Red: the crash-point restart test, the two reconciler settle tests, the three live/unprovable counter-cases (owner was a token fence), and the exact-lease CAS test. The tests were written after the code. - New tests: - crash point: a finalizer claim was written and its holder killed before commit; the next restart finalizes the run, and this boot's claim stamps itself as holder; - a dead finalizer or cleanup holder is settled by a recording pass, never by a read-only pass (`record: false`); the accepted result and phase are untouched; - live counter-cases: a live earlier-boot PID (`pid_alive`), the current boot (`current_process`), a dead PID in another PID space (`different_process_space`, unprovable) keep the lease through a pass and a restart; - a holder written for another token, and a holderless row of the previous release, stay token fences, also with `PAPERCLIP_EPHEMERAL_SERVER=true`; - settle refuses a lease taken over after the proof (`owner_changed_since_proof`). - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `lease_holder`. They stay token fences and the existing claim takes them over at expiry. Neither lease outlives 5 min, so this covers at most 5 min after the upgrade. A rollback ignores the new column. - Unprovable holders (other host or PID space, unknown space): no operator action. The lease is short and self-expiring; at expiry the next claim takes it over and the old holder's next token compare-and-swap fails. That fence is the handoff. A different host is never proof. - The settle only expires a lease. Finalization and cleanup still run under their own checks (cleanup keeps its maintenance-history proofs). Settling ownership does not settle the accepted result, workspace preservation, or accounting. - Migration adds a nullable column and two partial indexes on a sparse predicate. Low risk. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, local test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
ca34bed257 |
test(native): exited-remote-runner budget case follows provider-work attempt charging (#19963)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Two restart-recovery PRs merged independently: #19951 lets a remote runner that exited before provider work bootstrap again, and #19948 charges a restart attempt only when the provider accepted a turn. > - Together they change one of #19951's test expectations: at the last attempt, the exited runner is no longer charged, so it bootstraps again instead of hitting the budget. > - That test now fails on master in `Verify Paperclip Runner (vitest native server)`, which blocks other PRs. > - This pull request updates the test to the combined behavior. Product code does not change. > - The benefit is a green master, with the generation-cap bound on repeated exits still under test. ## Linked Issues or Issue Description Refs #19951, #19948, #19950 (blocked by this failure). **What happened?** On master, the test "keeps an exited remote runner's retry held when the bootstrap budget is spent" receives `bootstrap_incomplete` with `chargesProviderAttempt: false` instead of `blocked / execution_recovery_budget_exhausted`. **What did you expect?** The test should match the combined behavior: uncharged at the last attempt, and blocked once the controller generation cap is reached. **Steps to reproduce** 1. `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner"` on master fails. ## What Changed - Removed the stale budget row from the exited-runner counter-cases. - Added two cases. An exited runner at attempt 3 bootstraps without being charged. At `NATIVE_PROGRESS_CREDIT_GENERATION_LIMIT` it blocks with `execution_recovery_budget_exhausted`. These mirror #19948's own cases. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner|relaunched remote runner|generation cap"`: 11 pass. ## Risks - Low. Test-only change. Repeated runner exits before provider work stay bounded by the generation cap, which the new case covers. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f6cb3e43f4 |
fix(admission): admit new work independent of other runs' cleanup and accounting (#19944)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission decides if new agent work can start in a company > - Five admission gates waited on the cleanup or accounting of other runs > - The restart-chaos campaign found that these gates froze companies for up to 10 minutes, caused false budget pauses, denied in-run writes, and dropped wakes, comments and Retries > - The durable ownership plan (Decision 1) removes the company-wide accounting gate. It does not narrow it again > - This pull request removes the five gates. Unfinished accounting now adds its bounded estimate to exposure > - The benefit is that one run's unsettled accounting never stops the other work in its company ## Linked Issues or Issue Description Refs #19831 (plan, section D), #19838 (inventory, finding 3), #19927 (Phase 2 reconciler data), #19497, #19724, #19826. **What happened?** Admission refused new work in these cases, with no reached hard stop and no pause: - a terminal run awaited accounting (`waitingRunCount`); - a native run recovered its accounting (`recoveringRunCount`); - a budget-owned scope hold awaited reconciliation, after the hard stop no longer applied; - a budget input was older than 15 s (`budget_evaluation_work`); - the predecessor run row of a task was missing (`hasPendingRunCleanup` failed closed). **Expected behavior** Admission never waits on the cleanup or accounting of another run. Only a reached hard stop, an explicit pause or an inactive company refuses new work (invariant 3). **Steps to reproduce** Make a company with an active budget policy. Leave one failed run with `cost_accounting_pending = true`. Request work for another agent. On `master` admission refuses with "Company cannot start work while completed runs await accounting." ## What Changed - `budgets.ts`: admission refuses only for a reached hard stop or an explicit pause. The code removes `observedBlocks`, the `recoveringRunCount` check, the stale `budgetHeld` refusal and the 15 s input-age gate. It also removes the unused `inspectInvocationEligibility`. `reconcileScope` no longer returns accounting holds. - `budget-exposure.ts`: removes the `waiting` and `recovering` aggregates and the own-task subquery. The admission query is smaller. Unfinished accounting stays exposure: the larger of the held reservation and the observed cost. - `budget-evaluation-work.ts`: removes `oldestBudgetInputAt`, which nothing reads now. - `execution-blocker.ts`: `hasPendingRunCleanup` returns false for a missing run row. A run row that is deleted removes its finalization coordinator and nulls its lease references, so the row owns nothing. A predecessor that owns a lease or a coordinator still holds the successor of its own task. - `recovery/ownership-invariant-report.ts`: the I3 check no longer mirrors the removed gate. It reports a wake request or a run that was refused with an accounting-hold reason. Any such report is a regression. - `doc/SPEC-implementation.md`: adds the product rule "Admission is independent of cleanup and accounting". `doc/execution-semantics.md` is aligned. ## Verification - New `server/src/__tests__/admission-independent-of-cleanup.test.ts`, on embedded Postgres. It has one case per gate. All six cases fail on `master` and pass here. A seventh case (an explicit pause still refuses) passes on both. One case also asserts invariant 3 together with exposure: two failed runs that each hold a 400-cent reservation fill a 1000-cent budget, and a hard stop still refuses. - Existing tests that asserted the old gates now assert admission (invariants 3 and 4): `closed-native-receipt-accounting`, `budgets-service`, `budget-evaluation-work`, `heartbeat-held-comment-wake`, `ownership-reconciliation-report`. In `heartbeat-held-comment-wake`, each saved comment, Retry, assignment and continuation is delivered exactly once while another run's accounting is pending. Settling accounting later does not deliver it again. - All six files pass locally, one file per process. The server package typecheck passes. - Speed: admission is on the turn path. This change only removes work from it: two correlated aggregates, one own-task subquery and one `budget_evaluation_work` read per policy. No wait or I/O is added before `turn.start`. - Not run here: the chaos waves (K13 storm, K6 crash loop, K4 long holds). The coordinating session schedules them on the shared harness. ## Risks - Behavior change: work starts while accounting for other runs settles. Spend stays bounded by the reservation capacity check and the hard stop, which read the ledger plus observed cost. A run that has no reservation and no observed cost adds zero exposure until its charge commits. This is the gap that Decision 1 accepts. Budget enforcement was already best effort (SPEC 11.6). - A budget-owned pause with no reached hard stop no longer refuses work in budget admission. The paused company or agent is still refused by the existing pause checks until the budget worker lifts the pause. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
a4a3e342ec |
fix(recovery): charge a restart attempt only when the provider accepted a turn and made no progress (#19948)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run has an execution recovery budget: three provider attempts for native runs, and two failure retries for legacy runs > - In the restart chaos campaign, control-plane restarts used up that budget for runs that were healthy (RC-22, 22b, 22c, 40 and 60) > - #19720 and #19747 fixed two variants with watermark heuristics, and legacy interrupted preparation still spent the failure budget > - The durable-ownership plan (section F, invariant 5) asks for one rule: charge an attempt only when a recorded outcome proves that provider work started and no progress followed > - This pull request adds that rule and moves the native restart charge and the legacy preparation retry onto it > - The benefit is that a restart never spends budget for work the provider never started, while real failure loops and crash loops stay bounded ## Linked Issues or Issue Description Refs #19720, #19747, #19856 **What happened?** A control-plane restart charges a provider attempt by heuristics. The first crash restart always charges. A later restart charges unless the previous resume had no start evidence (#19747) or completed work (#19720). A runner that spawned but whose provider never accepted a turn counted as started, so a K6 crash loop could still spend the budget. Legacy runs whose preparation a restart interrupted (`providerWorkStarted: false`) spent `failureRetries`, the same counter as real failures (`execution-recovery-attempt.ts`). **Expected behavior** An attempt is charged only when the provider accepted a turn (`turn.accepted` or `turn.started`) and no accepted turn then completed work. A startup before spawn, a runner that spawned but was never admitted, and a `turn.start` the provider did not accept are not charged. The generation-12 cap still bounds uncharged restarts. **Steps to reproduce** Start a native run at its last attempt. Kill the controller after the runner spawns but before the provider accepts a turn. Before this change, the next restart claim blocks the run with `execution_recovery_budget_exhausted`. ## What Changed - New `server/src/services/native-runtime/provider-attempt-outcome.ts`: the one rule. `readProviderAttemptOutcome()` returns `provider_not_started`, `provider_started_without_progress` or `provider_progressed` from run events. `providerAttemptChargesBudget()` charges only the second. `providerAttemptWindowStart()` starts the attempt window at the latest `native.process_start_requested`, so earlier work never counts for a later attempt. `PROVIDER_ATTEMPT_CREDIT_GENERATION_LIMIT` (12) is the shared cap. - `restartResumeKeepsAttempt()` (`native-restart-recovery.ts`) now applies that rule to the attempt the restart interrupted, including the first restart. It keeps the unstarted-resume carry from #19747, the reattachment rule (an adopted provider counts as started), the drain exemption from #19625, and the generation-12 cap. A history entry from an older release without a recorded position still charges. - Legacy runs: `interrupted_preparation` is now an uncharged retry lane (`execution-recovery-attempt.ts`). It keeps its own counter, bounded at the same cap (12), and never changes `failureRetries`. `retries.ts` uses the longest transient delay after the second lane attempt. - `doc/execution-semantics.md` states the rule. - Accounting has its own "provider work started" readers (inventory Summary finding 5). Phase 3 owns those; this PR exposes the rule for them to adopt and does not change them. ## Verification - `native-runner-restart-recovery.integration.test.ts` (embedded Postgres, real `claimNativeRestartRecoveries`): 129 passed. New cases, red on master: - not charged at the last attempt for a claimed remote bootstrap, a remote bootstrap retry, and a claimed unlaunched remote run; - not charged when the runner spawned but was never admitted, and when the provider did not accept `turn.start`; - not charged on the first restart when the original attempt already made progress; - a normal retry that spawned but was never admitted is not charged, and earlier work still does not count for it. - Counter-cases, green on master and with the change: an accepted `turn.accepted` or `turn.started` with no progress is charged; the generation cap blocks at the last attempt; crash loops with and without progress stay bounded; the #19720 and #19747 cases still pass. - `execution-recovery-attempt.test.ts`: new lane tests (red on master). - `workspace-recovery-preparation-dispatch.test.ts` (25 passed), `heartbeat-process-recovery.test.ts` (interrupted-run cases), `run-dispatch/adapters/postgres.test.ts` (interrupted cases). - `server` typecheck (`scripts/typecheck-layers.mjs`, low-memory mode): no errors. - Not run: chaos waves (the coordinator schedules them on this host), and the full server suite (CI covers it). ## Risks - Behavior change: the first crash restart no longer charges when no accepted turn is recorded. If a crash lands before the controller projects the provider's `turn.accepted`, that attempt is not charged. This fails toward more retries, and the generation-12 cap bounds it. - A legacy preparation crash loop now gets up to 12 retries instead of 2. Each retry still passes the normal issue gates and keeps the transient delay. - The new queries use the `(run_id, seq)` index, once per restart claim. Nothing new runs before `turn.start`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
c6e4ed1547 |
feat(recovery): settle a claimed wake whose run finished without settling it (#19934)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A wake request is claimed by the run that handles it, and that run marks it finished. > - In a K4 chaos wave (#19927), native restart recovery blocked a run and ended it, but left its assignment wake `claimed`. Nothing ever released it. > - The durable-ownership plan settles such records on one shared path, by proof, never by age. > - This pull request registers a settle for `wake_request_claimed`. It is Phase 2's second kind and is stacked on the accepted-result lease PR. > - The benefit is that no run-ending path can strand a claimed wake. ## Linked Issues or Issue Description Refs #19831 (plan), #19861 (registry), #19927 (the data that found this). Stacked on #19933 (it adds the `settle` hook). **What happened?** Run `7c45860b…` failed with `native_restart_recovery_blocked` (`execution_recovery_budget_exhausted`) at 23:19:41. Its wake `6b3c4cce…` stayed `claimed` until the end of the wave (23:44 and later). Rule I2 flagged it. **What did you expect?** A claimed wake ends when its run ends, with the run's outcome. **Steps to reproduce** 1. `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --faults K4 --k4-hold-max-seconds 600 --tasks 6 --concurrency 3 --seed 9203 --keep` 2. Look for `agent_wakeup_requests` rows with status `claimed` whose run is terminal. ## What Changed - New `recovery/wake-request-claimed-settlement.ts`: when the owner run is terminal, and for a native run its coordinator is `committed` or `terminal_failure`, the claim gets the run's outcome. That is `completed` for a succeeded run and otherwise the run's status, as run completion and native terminal projection already do. It is compare-and-swap on the claim (id, run, status `claimed`). - `ownership-registry.ts`: `wake_request_claimed` registers that settle. - `native-finalization-reconciler.ts`: the pass log line includes the settled count per kind. ## Verification - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 pass, including the new case. It covers a blocked native run, a native run whose projection is still pending (kept), a succeeded legacy run, a live run (kept), and a read-only pass (no change). - Red/green (test written after the code): the new test fails on master and passes here. - Server typecheck passes. ## Risks - No persisted shape changes, so there is no previous-release story beyond this: claims saved by an older release are settled the same way. - Race with the run's own settle: it waits for a native coordinator to be final. For a legacy run, both write the same status, so the race is idempotent. - No change adds work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
83dec71a8f |
fix(runner): return the edited agent's link and effect in instruction save receipts (#19940)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - In chat, a person can ask the CEO agent to change another agent's saved instructions > - The native instruction tools save the change, but their receipts contain no link to the edited agent > - Completion feedback tells the agent to use only hrefs that tools returned and not to guess routes > - So the agent can not link the agent it changed, and it has no grounded fact about when the change applies > - This pull request adds the edited agent's real profile href and the save's effect to the save and restore receipts > - The benefit is a chat reply that lets the person open the changed agent and know which runs use the new rule ## Linked Issues or Issue Description Refs #19344 **What happened** A person asked in chat: "Add a rule for Morgan to run the relevant tests before handing work back." The agent saved the rule correctly and kept concurrent human guidance. The reply did not link Morgan and did not say when the rule applies. The run's tool events show the cause: `update_agent_instructions` returned the revision but no href, and the accepted `paperclip_finish` feedback said "Do not guess routes or invent links when no resource href was returned." **Expected behavior** After an instruction save or restore, the agent has a real profile href for the edited agent and a statement of when the saved instructions take effect. The reply links the agent and explains that later runs use the change. **Steps to reproduce** 1. In a chat with the CEO agent, ask it to add a rule to another agent's instructions. 2. Let the agent call `update_agent_instructions`. 3. Read the tool receipt. It contains no href for the edited agent and no effect statement. ## What Changed - `update_agent_instructions` and `restore_agent_instructions` receipts now include `resourceLinks` with the edited agent's company-scoped profile href (`/<prefix>/agents/<agentId>`) and name. - The same receipts include `appliesTo`. It states that runs that start after the save use the new instructions and that runs in progress keep their snapshot. This matches `materializeInstructionBundle`, which reads the committed head when it builds the run context. - The link uses the agent id, which is unique. It does not use the name-derived URL key. ## Verification - `cd server && pnpm exec vitest run src/services/native-runtime/agent-instruction-tools.integration.test.ts src/services/native-runtime/agent-instruction-tools.test.ts`: 14 passed. - Without the source change, the new assertions fail in 3 tests (update receipt and both lost-acknowledgement recovery receipts). - The recovery test confirms that the persisted receipt, which recovery replays, also carries the link. - The end-to-end chat eval `agents.edit-instructions` runs this change on its case branch. ## Risks - Low risk. The change adds fields to a tool receipt. Existing fields do not change. - Receipts that were saved before this change replay without the new fields. That is acceptable: the agent then has no link, as before. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
196108acb9 |
fix(native): bootstrap again when a relaunched remote runner exited before provider work (#19951)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Restart recovery decides whether an interrupted native run can resume, bootstrap again, or must wait for a person. > - A remote run is blocked as "ambiguous" when recovery cannot prove what reached the sandbox runner. > - In a Daytona chaos wave (#19947), two runs were blocked even though the controller had recorded that the relaunched runner exited before any provider work. > - This pull request accepts that recorded runner exit as proof and bootstraps the run again, under the existing attempt budget. > - The benefit is that a restart no longer turns a run with no provider work into a failed, blocked run. ## Linked Issues or Issue Description Refs #19947 (Daytona data), #19831 (durable-ownership plan), #19934 (the stuck claimed wake from the same runs). **What happened?** Wave `w-20261108T012041-s9301`, runs `450f277d…` and `9b93d40c…`, went through this sequence: 1. Restart recovery proved the remote bootstrap incomplete and relaunched it. 2. The new runner exited: "runnerd exited while waiting for run.prepare". The executor settled `bootstrap_retry` with `providerSessionEstablished: false`, `providerEventsExist: false` and `checkpointExists: false`. 3. The next restart recovery saw the recorded runner PID and blocked the run as `ambiguous_remote_recovery_evidence`. **What did you expect?** Bootstrap again, because the runner is proven gone and no provider work happened. **Steps to reproduce** 1. `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "relaunched remote runner"` fails on master with `blocked / ambiguous_remote_recovery_evidence`. ## What Changed - `native-restart-recovery.ts`: new proof `remoteRunnerExitedBeforeProviderWork`. It applies to a remote sandbox with a prepared workspace and a settled `bootstrap_retry` failure whose original code is `native_runner_process_exited`, with no provider session, events or checkpoint and no lease or successor. Such a run is classified as `bootstrap_incomplete` (`remote_runner_exited_before_provider_work`). It passes the same retry admission check as `remote_runner_never_launched`, and the same three-attempt budget. - Tests: the s9301-shaped retry plus five counter-cases. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "exited remote runner|relaunched remote runner|remote bootstrap retry|never requested a runner|exact-checkpoint retry"`: 14 pass. - Red/green: the new bootstrap test and the budget counter-case failed on master (both returned `blocked / ambiguous_remote_recovery_evidence`, the same as the s9301 rows) and pass here. The other four counter-cases (not a runner exit, session established, provider events, checkpoint) block on both. - Loop: focused vitest with one embedded Postgres, 26–50 s. - Not run locally: the server typecheck and the full suite (memory limits on this host). Required CI covers them. ## Risks - Fail-open risk: the proof needs the executor's own settled record of a runner exit with no provider work, plus a fresh check that no checkpoint or provider evidence exists. A recorded PID without the exit record still blocks, as before. - Previous-release story (rules/upgrade-safe-run-state.mdc): no persisted shape changes. Rows without these failure fields keep today's outcome. - Nothing runs before `turn.start`. This is restart recovery only. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f5400e2594 |
feat(recovery): settle the accepted-result lease on the shared ownership path (#19933)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A control-plane restart can kill the controller after an agent's result is accepted. The result then stays leased to a dead process. > - Today a private sweep (`native-dead-controller-result-lease.ts`, #19713) proves that controller dead with its own liveness rules. > - The durable-ownership plan (#19831) replaces private proofs with one prover (`proveOwnerGone`, #19853) and one registry (#19861). > - This pull request moves the accepted-result executor lease to that shared path and deletes the private sweep. It is Phase 2's first kind. > - The benefit is one tested rule for when a dead owner's result lease is released, and the reconciler now repairs a lease that restart recovery missed. ## Linked Issues or Issue Description Refs #19831 (plan), #19853 (prover), #19861 (registry), #19713 (old sweep), #19883 (legacy host fallback), #19927 (Phase 2 data). **What happened?** The accepted-result lease had its own liveness proof, separate from the shared prover. The reconciler could only report a dead owner, not settle it. When restart recovery missed a lease, it waited out the 20-minute lease. **What did you expect?** One prover decides, and both restart recovery and the periodic reconciler pass settle a lease whose controller is proven gone. **Steps to reproduce** 1. Accept a native run result. 2. Kill the controller (SIGKILL) and start a new server. 3. If the restart sweep does not see the row, the result stays leased until the lease expires. ## What Changed - `ownership-registry.ts`: a kind may register `settle` and `onUnprovable`. `native_accepted_result_lease` registers both. Other kinds stay report-only. - New `native-accepted-result-lease-settlement.ts`: a compare-and-swap settle that expires exactly the proven owner's lease. It keeps the old guards: the run must still be `running`, remote copyback stays fenced, and a lease taken over since the proof is left alone. - `ownership-invariant-report.ts`: the pass calls `settle` on `gone` and `onUnprovable` on `unprovable`. A read-only pass (`record: false`) never settles. New `settleOwnableKind()` for restart recovery. - `heartbeat/recovery.ts`: restart recovery calls `settleOwnableKind("native_accepted_result_lease")` instead of the old sweep. - Deleted `native-dead-controller-result-lease.ts`. - `owner-proof.ts`: accepts a board attestation (`operator_attested`). It also keeps the one-release local fallback for rows saved before their PID space was recorded, on single-host servers only, and flags each use so it is logged (`legacy_host_unknown_dead_pid_proof`, #19883). An ephemeral server returns `unprovable` (`process_space_unrecorded`) and the existing "confirm the previous server stopped" board action is raised. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts`: 27 pass, 7 of them new. - `npx vitest run src/services/owner-proof.test.ts src/__tests__/ownership-reconciliation-report.test.ts`: pass. - Red/green (tests written after the code): 5 new behaviour tests fail on master and pass here. They cover the ephemeral board action, the reconciler-pass settle, the live-owner counter-case in that pass, the legacy marker, and operator attestation. The stamped-dead-controller and foreign-PID-space cases pass on both, as the old path behaved the same. - Server typecheck passes (`npx tsc --noEmit -p server/tsconfig.json`). - Not run locally: the full server suite (`pnpm verify` asks for broad coverage because a file was deleted). Required CI covers it. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): NRF rows saved before #19868 have no `controller_host_hash`. They keep the same one-release local proof as before on single-host servers, and each use is logged. On ephemeral servers they stay unprovable and ask the board, as before. Rows on another PID space keep their lease (the fence), as before. - Fail-open risk: settle is compare-and-swap on the lease token, boot, PID, host hash and start time of the proven owner. An `unprovable` owner is never released. Live-owner and foreign-space cases are tested. - No change adds work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
18644a3263 |
feat(recovery): instrument 20 durable write boundaries with crash points (#19938)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A control-plane restart between two durable writes leaves an in-progress record owned by a dead process > - #19937 added `crashPoint(name)` and a registry of 20 named boundaries from the Phase 0 inventory > - A name without a call site cannot fire, so the crash-point suite cannot test it > - This pull request puts one call at each registered boundary and adds a test that keeps the registry and the call sites in step > - The benefit is that the on-demand suite (next pull request) can kill the control plane at every known write-ahead gap ## Linked Issues or Issue Description Refs #19831 (plan, Phase 5). Refs #19846 (inventory, "Write-ahead coverage"). Stacked on #19937 (helper and registry). Refs #19861 (ownable kind ids). **Problem** The registered crash points have no call sites. **Expected behavior** Each registered point is called at its gap: after the first durable write or outside effect returns, before the next one starts. Disarmed (always in production) each call is one comparison. **Scope** Call sites and a coverage test only. The suite runner and CI workflow follow. ## What Changed - Server call sites (`@paperclipai/shared/crash-points`): - `sandbox-creation-operations.ts`: around the provider `create` call (intent → provider → lease). - `native-session-executor.ts`: after `native.process_start_requested`, before spawn. - `paperclip-control-plane-port.ts`: after the result-accept transaction, before finalization starts (only when a result was inserted). - `native-run-finalizer.ts`: after `commitNativeStatusDecision`, before the run projection. - `workspace-operations.ts`: `workspace_finalize` only — after the running row, and after `settle()` before the done row. - `environment-runtime.ts`: provider release → lease row; teardown claim → destroy; reusable destroy → lease release; stop-and-retain claim → stop. - `heartbeat.ts`: lease release → runtime services; budget reservation hold → dispatch. - `heartbeat/queue.ts`: run claimed `running` → wake request `claimed`. - `run-cost-accounting.ts`, `usage-receipts.ts`: accounting commit → spool unlink; receipt spool → DB row → unlink. - `packages/paperclip-runner/src/crash-point.ts`: a small copy of the helper for controller code in this package, which does not depend on `@paperclipai/shared` (adding that dependency would change the runner content ID). It arms only `runner.*` names. runnerd never sees the variable: its environment is an allowlist. - Runner call sites: after the runnerd spawn, before its identity is published (both spawn paths); `turn.start` result received → controller journal saved → next send. - `server/src/__tests__/crash-point-call-sites.test.ts`: every registered point has a call site, every call names a registered point, and every `kinds` entry is an ownable kind id of the Phase 1 registry. ## Verification - `cd server && npx vitest run src/__tests__/crash-point-call-sites.test.ts` → 3 passed. - `pnpm --filter @paperclipai/server typecheck` → clean. `pnpm --filter @paperclipai/paperclip-runner exec tsc --noEmit -p .` → clean. - Behavior with nothing armed is unchanged: each call returns at its first comparison (measured in #19937: about 3.7 ns). ## Risks - Low risk. No call changes control flow unless `PAPERCLIP_CRASH_POINT` names it and `NODE_ENV` is not `production`. - Points on the path before `turn.start` (process start, runner spawn) cost nanoseconds, within plan invariant 8. - `usage-receipts.ts`: the persist-then-unlink one-liner became a block with the same statements in the same order. - The runner package change gives the runner a new content ID, as any runner source change does. No persisted run state changes (rules/upgrade-safe-run-state.mdc). ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits) and a read-only exploration subagent that mapped the boundaries. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f35eb1e5ef |
fix(observability): judge the TTFT budget from run creation; report the wait behind a live turn (#19941)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Invariant 8 of the durable-ownership plan says turns must start fast. #19863 added per-run speed budgets that write `run.performance.budget_exceeded` to the run log. > - The Phase 2 chaos data (#19927) showed follow-up TTFT about 3x over the claude budget. > - The cause is measurement scope. A follow-up comment posted during a live turn waits for that turn to end, and TTFT is anchored at the comment. The budgets came from idle follow-ups. > - This pull request judges the TTFT budget from run creation and reports the wait separately. > - The benefit is that budget breaches point at real start-up slowness, not at time spent behind a turn the user is already watching. ## Linked Issues or Issue Description Refs #19863 (speed budgets), #19927 (Phase 2 data), #19852 (startup sub-spans), #19888 (warm default). **What happened?** In two local storm waves, C1 follow-up runs breached the claude warm TTFT budget (p50 27.6–29.9 s against 9.5 s). Their spans show `comment.to_run_created` at 6–62 s (waiting for the live turn), then session resume at 1.4–3.8 s and submit-to-first-event at 2–5 s. Codex without the wait was about 4.8 s, the same as the warm-follow-up baseline. **What did you expect?** The budget should judge start-up latency, which is what its baseline measured. The wait behind a live turn should be visible but not judged. **Steps to reproduce** 1. Run a local chaos storm with C1 tasks. 2. Read `run.performance.budget_exceeded` rows with metric `ttft` for `issue_commented` runs, and compare them with each run's `comment.to_run_created` span. ## What Changed - `native-run-trace.ts`: the TTFT budget check measures from run creation (start of `heartbeat.queue`, or end of the ingress span). The TTFT span itself does not change. - `native-speed-budgets.ts`: breaches can carry `inputWaitMs` (TTFT only). - `tests/restart-chaos/ttft-baseline-stats.ts` and `ttft-baseline.ts`: the wait behind a live turn is its own measured step (`inputWait`), next to the full TTFT and the TTFT from run creation. The TTFT table gains a "Wait behind live turn" column. The full comment-to-first-token time stays the user-visible metric. - `doc/run-log-events.md` and the plan's speed-budget section describe the rule. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-speed-budgets.test.ts src/services/native-runtime/native-run-trace.test.ts`: 43 pass, 2 new. `pnpm exec vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/ttft-baseline-stats.test.ts`: 12 pass, 1 new. They cover a follow-up under budget after a 60 s wait (no breach), a follow-up over budget (breach with `inputWaitMs`), and queueing after run creation still judged. - Server typecheck passes. ## Risks - Low. This is a run-log-only change (AGENTS.md §5.7): no Telemetry and no span attributes change. - A slow wait before run creation is no longer a TTFT budget breach. The TTFT span still records it, and the breach payload names it. Admission holds after run creation are still judged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
35cbd15175 | fix(native): recover a stopped Runner whose journal stayed ready at a settled cut (#19907) | ||
|
Nicky Leach
|
94718ce168 | fix(native): continue on a replacement when a retained Runner retires before claim (#19897) | ||
|
DottaandClaude Opus 5.5
|
2d89486c57 |
fix(native): record OpenCode's rejected model as a model rejection (#19865)
## Thinking Path - Paperclip runs agents through native providers; when a provider rejects the selected model, the person needs to see "Model unavailable" and choose another model. - #19548 made the OpenCode driver report OpenRouter's invalid model ID as a closed `model_unavailable` provider failure. The task banner now shows it. - The server's terminal classifier, however, recognized a model rejection only from Codex's exact ChatGPT error envelope. OpenCode's structured failure was stored for the banner but the run was recorded as `adapter_failed`. - So chat showed a generic failure, and the task was not blocked for a model choice. The `runs-adapters.inspect-failed-run` chat eval caught this. - This change maps an action-required `model_unavailable` failure to `native_provider_model_rejected` for any provider. ## Linked Issues or Issue Description Refs #19548. An OpenCode run whose model OpenRouter rejects (`... is not a valid model ID`, HTTP 400) finishes with `errorCode: adapter_failed`. Expected: `native_provider_model_rejected`, as for Codex. ## What Changed - `native-provider-failure.ts`: `forTerminal` returns `native_provider_model_rejected` when the failed turn's provider failure has category `model_unavailable` and retryability `requires_action`. Transient or unknown retryability still returns null. The diagnostic is the closed turn diagnostic; no provider text or model name is copied. - `NATIVE_MODEL_REJECTION_MESSAGE` no longer names a ChatGPT connection, since it is now the blocker text for any provider. - Tests for both providers, plus negative cases for transient/unknown retryability. - `wake-queue/adapters/postgres.ts`: a native run that failed for a rejected model or incomplete configuration, on a task the arbiter already blocked, is released without opening a `native_continuation_requires_reconciliation` incident. That incident put the run in `recovery_needed`, which hid the run page's Retry, though the provider refused the request and nothing ran to reconcile. Queued messages stay deferred. ## Verification - `pnpm exec vitest run src/services/native-runtime/native-provider-failure.test.ts src/services/native-runtime/status-arbiter.test.ts src/services/native-runtime/native-provider-failure.integration.test.ts` (server): 98 passed. - `pnpm exec vitest run src/modules/wake-queue` (server): passes, including a new test that fails on the old code (it opened the incident). - Server `tsc --noEmit`: clean. - End to end: the `runs-adapters.inspect-failed-run` chat eval will be rerun on the chat-eval candidate with this change. ## Risks - Codex model-rejection blocker wording changes from "current ChatGPT connection" to "current AI connection". - Any provider driver that reports `model_unavailable` with `requires_action` now blocks the task for a model choice instead of the generic failure path, which disables automatic adapter-failure retries for that run. That is the intended behavior for a rejected model. - Configuration-failure native runs on a blocked task no longer get a board reconciliation incident. If such a run had already produced side effects in earlier turns, they are not flagged for reconciliation; a provider model rejection happens before the turn does work. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code, with tool use (shell, file editing). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
cbf811cd3b |
fix(recovery): prove a replaced deployment's copyback owner gone automatically (#19913)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs save their workspace after the result is accepted, under
a copyback owner receipt
> - A deploy can restart the server while that save is in progress
> - The new server runs in a new container, so it cannot see the old
process and fails closed
> - The run then waits on a board action that asks a person to verify
the previous controller
> - Recoveries must continue without a human step, and copy must not ask
anyone to inspect a server
> - This pull request adds a deployment-level rule to the shared
owner-liveness primitive and uses it in workspace finalization
> - The benefit is that a save interrupted by a deploy resumes on its
own, with no second writer
## Linked Issues or Issue Description
**What happened?**
A deploy restarted the server while three runs were saving their
accepted results. Each run kept a copyback owner receipt that names the
old Railway deployment on another container. The restarted server
reported `native_workspace_finalization_owner_unverified`, owned by the
board, with the next action "Verify the previous controller and its
workspace-copyback processes have stopped...". The cloud harness stop
receipt did not arrive, so the runs stayed in "Recovery needed" for
about an hour, until a person confirmed the stop by hand.
**Expected behavior**
The restarted server proves that the previous controller is gone,
resumes the save, and settles the run with no human step. No user-facing
text asks a person to verify a server or controller.
**Steps to reproduce**
1. Start a native run on a Railway service with a volume.
2. Let the result be accepted and the workspace save start.
3. Deploy, so that the server restarts during the save.
4. See that the run waits for a board confirmation.
**Paperclip version or commit**
Observed on
|
||
|
DottaandClaude Opus 5.5
|
99529f9d71 |
feat(observability): local speed budgets and a run-log budget check (#19863)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When someone messages an agent, the reply must start as fast as
possible, and restart-safety work must not slow it
> - The durable ownership plan makes this invariant 8 ("fast turns") and
asks Phase 1 to turn the Phase 0 baseline into p50/p95 budgets per
adapter
> - Without budgets, a regression in time to first token, agent boot or
restart recovery shows up only when someone reruns the baseline by hand
> - This pull request records the budgets and adds a per-run check that
writes a run-log row when a run exceeds its p95 budget
> - The benefit is that every later phase can show "no regression
against the budgets" from the run log, without new waits on the turn
path
## Linked Issues or Issue Description
Refs #19831 (plan, "Performance constraint", rule 4, and invariant 8).
Refs #19846 (the Phase 0 baseline these budgets come from). Stacked on
#19861.
**Problem**
The plan requires p50/p95 budgets per adapter for TTFT, agent boot and
restart recovery latency, and a check against them. Neither exists.
**Expected behavior**
Budgets are recorded in the plan and in code. A run that exceeds a p95
budget gets one run-log row that names the metric, the measurement and
the budget.
**Impact**
Phase 2–5 exit criteria can cite budget breaches from chaos waves and
staging. The check adds no wait before `turn.start`.
## What Changed
- New `server/src/services/native-runtime/native-speed-budgets.ts`: the
local budget table (codex, claude, opencode) for TTFT and agent boot
(cold and warm) and restart recovery latency. Each budget is 1.2× the
baseline, rounded up to 0.5 s. `evaluateSpeedBudget` judges one run
against p95. An unknown start kind uses the looser budget. Remote runs
and adapters without a baseline are not judged.
- `native-run-trace.ts`: the trace takes an optional `budget` subject.
It checks `task.time_to_first_token`, agent boot (end of
`heartbeat.queue` to the end of the first `runner.turn.submit` or
`runner.session.resume`) and restart recovery once each. It writes
`run.performance.budget_exceeded` through `defer`, so the write is
detached from the event path. `finish` now drains diagnostics that defer
more work.
- `native-session-executor.ts`: passes the adapter and environment to
the trace, labels cold or warm at the existing `provider_session` reuse
receipts, and checks restart recovery at a recovered run's first
provider event, measured from the startup recovery `ready` time.
- Docs: budget tables in
`doc/plans/2026-11-07-durable-ownership-and-reconciliation.md` ("Speed
budgets (Phase 1)"), and the new event in `doc/run-log-events.md`.
## Verification
- `npx vitest run
src/services/native-runtime/native-speed-budgets.test.ts
src/services/native-runtime/native-run-trace.test.ts` (in `server/`): 34
tests pass. They cover:
- every budget has p50 ≤ p95;
- the provider-to-adapter mapping;
- p95 judgement, and the looser budget when the start kind is unknown;
- one breach for a slow boot, and none for a second submit or a fast
TTFT;
- a deferred TTFT breach is written before the trace closes;
- restart recovery is checked once;
- remote runs and runs with no budget subject write nothing.
- `npx tsc --noEmit -p server`: no errors.
## Risks
- Low risk. This is a run-log change only (AGENTS.md §5.7, no extra
review). The OTLP and Telemetry paths are unchanged.
- The baseline samples are small (2–12 per cell) on a shared host.
Budgets carry 20% headroom, and the codex restart budget (8.0 s p95) is
tight. Expect some breach rows on a loaded machine. They are diagnostics
and change no behavior.
- Restart-recovery budgets use run 4 of the baseline, as merged in
#19846.
- Restart recovery is measured from the startup recovery `ready` time to
the first provider event, which matches the baseline definition.
- Previous-release story: no persisted run state changes. The new event
is additive.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with
tool use and code execution.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
177f642da7 |
fix(recovery): continue native runs that failed before their first claim (#19911)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs go through a coordinator claim before any provider work
starts
> - A run can fail before that claim, for example while it restores the
previous harness backup
> - The failure path then records a no-replay hold, as if the run had
acted
> - Workspace recovery saves the files, but the task stays blocked and
asks a person to send a message
> - Recoveries must continue without a human step when no side effect is
possible
> - This pull request lets the existing safe-replacement worker continue
such runs once, after their files are saved
> - The benefit is that these tasks resume automatically, and runs that
may have acted keep today's reconciliation
## Linked Issues or Issue Description
**What happened?**
Native runs that failed before their first coordinator claim were never
continued automatically. They had no claim, no provider process and no
provider or tool events. Their result recorded `executionRecovery: {
kind: "bootstrap", providerWorkStarted: false }`. The failure path still
moved the coordinator to `terminal_failure` with
`native_continuation_requires_reconciliation` and recorded a resolved
`preserve_without_replay_v1` hold (replay blocked, board owner).
Workspace recovery then saved the files, but the next action stayed
"Saved work recovered. Send a message to continue."
`nativeReplacementPending` accepted only a few failure codes, and the
interrupted-preparation path covers only legacy runs.
**Expected behavior**
When a native run provably failed before its first claim, Paperclip
continues it once, automatically, after its files are saved (or when it
had no workspace to save). Runs with a claim, a process, a provider or
tool event, or recorded provider work keep the existing reconciliation.
**Steps to reproduce**
1. Start a native run with a remote workspace.
2. Make it fail before `native.coordinator.claim` (for example, abort
the harness-backup restore).
3. Let workspace recovery save the files.
4. See that the task stays blocked and asks for a message.
**Paperclip version or commit**
Observed on
|
||
|
DottaandClaude Opus 5.5
|
76994c5027 |
fix(native): bound failover backup restores by inactivity, not total time (#19906)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native per-turn session that continues an earlier conversation first restores its failover backup from durable storage. This happens before the coordinator claim, so no provider has started. > - That restore ran under a fixed 300 s total deadline (`AbortSignal.timeout(300_000)`). The deadline covers the download and the local decrypt, digest, extraction and provider-artifact steps. > - On staging, restore time for resumed sessions grew during the day. It reached 300 s, and then every resumed run of one review agent failed with the generic "The operation was aborted". Fresh sessions were not affected. > - A failure at this point also enters the saved-work reconciliation path, so the user request stopped. > - This pull request bounds each durable read by inactivity instead of total time. A slow transfer continues while bytes arrive. A read that stops sending bytes fails with a named code. > - The benefit is that resumed sessions no longer fail only because their backup is large or the store is slow. ## Linked Issues or Issue Description Refs #19894 (adds the same idle-deadline rule for workspace snapshot reads; this PR covers the separate native failover-backup reads). **What happened?** On staging, a PR-review agent had 17 runs fail in about 80 minutes. Each failure had the message "The operation was aborted" and the code `native_continuation_requires_reconciliation`. Every failing run resumed an existing session (`sessionIdBefore` set, per-turn lifecycle). Every fresh-session run in the same window succeeded. In each failing run, the error came 304–309 s after the `environment.startup` span, and no `native.coordinator.claim` span was recorded. The run result had `executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`. Earlier resumed runs of the same agent show the time from `environment.startup` to `native.coordinator.claim` growing from about 30 s to about 200 s before the failures started. The only fixed 300 s deadline on that path is the failover-backup restore in `restoreDurableHarnessForExecution`. **Expected behavior** A resumed session restores its backup when the store keeps sending bytes, even when the total time is more than five minutes. A read that stalls fails with a named, diagnosable code. A failed restore publishes no partial backup and keeps the saved backup. **Steps to reproduce** 1. Persist a native failover backup with provider artifacts. 2. Restore it through a storage reader that sends bytes slowly, so the full restore takes longer than the deadline. 3. Before this change, the restore fails with "The operation was aborted". A reader that stops sending bytes, with no caller deadline, waits indefinitely. ## What Changed - `native-harness-backup-storage.ts`: add `downloadIdleTimeoutMs` to `restoreDurableNativeHarnessBackup` and `verifyDurableNativeHarnessBackup`. When set, each durable `getObject` (the backup object and the provider artifacts) runs under an idle timer. Each received chunk renews the timer. The timer is active only while a read is in flight, so local decrypt, digest, extraction and copy are never timed. On expiry the whole restore aborts and throws `native_harness_storage_download_stalled`. Caller cancellation keeps its own error. - `verifyRecoverableNativeHarnessBackup` and the executor's `restoreDurableHarnessForExecution` use the five-minute idle limit (`NATIVE_HARNESS_DOWNLOAD_IDLE_TIMEOUT_MS`) instead of `AbortSignal.timeout(300_000)`. - The whole restore or verification keeps a 30-minute outer limit. It fails with `native_harness_storage_restore_deadline_exceeded`, so a hung local step stays bounded and named. - The executor passes the startup Stop signal (`providerPackSourceAbort`) into the restore, so Stop (also a Stop that came before the restore began) ends it. Only a startup that already completed does not reuse its signal. The restore checks the signal after the local copy and before it publishes `current`. - The wrapped storage is a `Proxy` that replaces only `getObject` and forwards every other member to the real service. - `doc/DEVELOPING.md`: document the idle limit and the named code. Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted shape changes. Backups, artifact rows, receipts and checkpoints saved by the previous release restore unchanged. A deploy that lands during a restore stops the process. The new process starts the restore again from the saved durable backup under the new limit. The existing hash, size, encryption and atomic publication checks are unchanged. ## Verification Run from `server/`: - `pnpm exec vitest run src/services/native-runtime/native-harness-backup-download-deadline.integration.test.ts` — 4 tests passed (embedded PostgreSQL, local-disk storage, an OpenCode backup with provider artifacts). The tests are in a focused suite, so the existing 990-line storage suite does not grow. - `pnpm exec vitest run src/services/native-runtime/native-harness-backup-storage.integration.test.ts` — 68 existing tests passed. - New: "keeps restoring a backup whose download outlasts the idle window while bytes keep arriving". With the former total-deadline policy, the same slow transfer fails with "The operation was aborted" and publishes nothing (the staging message). With the idle limit, the restore completes after more than twice the window, restores the session and the OpenCode launch artifacts, and verification also completes. - New: "names a download that stops sending bytes and publishes no partial backup". It checks restore and verify, checks that no `current` backup is published, and then restores the same saved backup successfully. - New: "stops at the outer bound during local copy and publishes nothing". - New: "lets Stop end a progressing download with its own reason, including a Stop before it starts". - RED: with the production files reverted to `origin/master`, the stall test does not finish and times out after 10 s, because no stall is detected. The other two tests pass on the old code, because the total deadline was the executor's caller policy. The first test reproduces that policy directly. - `pnpm exec vitest run` on `native-harness-online-preservation`, `native-harness-filesystem-independence` and `native-controller-migration` integration suites passed. `native-warm-checkpoint-ownership.integration.test.ts` fails locally on master as well, because the local Rust `paperclip-runnerd` binary is not built. - `node scripts/typecheck-layers.mjs` passed for all layers. Not verified: a live resumed run on staging with this build. The staging evidence shows where the time goes (before the coordinator claim, on resumed sessions only) but does not show which restore step was slowest. The fix removes the total deadline for every step on that path. ## Risks - A restore that progresses very slowly can now take longer than five minutes. Before, it failed. The run still stops on caller cancellation. - A local step that hangs (for example a stalled disk) is now cut off by the 30-minute outer limit instead of 5 minutes. A stalled local writer also pauses the read through backpressure, so it can be reported as `native_harness_storage_download_stalled`. - A failure before the coordinator claim still enters the existing saved-work reconciliation path. This PR does not change that classification. It removes the cause of these failures. ## Model Used - Provider: Anthropic. Model: Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, read-only browser for API inspection) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
988a4b6749 |
feat(recovery): ownable-kind registry and report-only ownership reconciler (#19861)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A control-plane restart leaves in-progress records owned by dead processes, and about 31 per-subsystem proofs decide when to release them > - The durable ownership plan (section C) asks for one registry of ownable record kinds and one reconciler > - Phase 1 must measure first: how many in-progress records exist, who owns them, and which invariants they break, with no behavior change > - This pull request registers 46 kinds from the Phase 0 inventory's section C and adds a report-only pass to the existing native finalization reconciler > - The benefit is per-kind invariant-violation counts from chaos waves and staging. That data unlocks Phase 2 (moving kinds onto one settle path) and the removal of the company-wide admission gate ## Linked Issues or Issue Description Refs #19831 (plan, section C and Phase 1). Refs #19846 (Phase 0 inventory, section C). Stacked on #19853 (the prover) and #19851 (one process identity). **Problem** No single place lists the control plane's in-progress records or checks the plan's invariants. Each subsystem releases its own records by its own rules, so nobody can count how many records break "owned", "settled" or "visible next action". **Expected behavior** One report-only pass lists every registered kind, classifies each owner as gone, alive or unprovable, and records each invariant violation once. It changes no record and stays quiet when nothing is in progress. **Impact** Phase 2 moves kinds onto the shared settle path in the order these counts show. Phase 3 removes the admission gate once the I3 data exists. ## What Changed - New `server/src/services/recovery/ownership-registry.ts`: 46 `OwnableKind` entries (`id`, `obligation`, `storage`, `listInProgress`, `settle: null`, `invariantChecks`). Each record names its owner as a process stamp, a fence token, a run, an operator, or none. The skill-cache lock is registered with a named coverage gap because its lock files live in per-skill roots. - New `server/src/services/recovery/ownership-invariant-report.ts`: one pass classifies every owner with `proveOwnerGone` (or the owning run's status), then runs: - I1 owned: an in-progress record with no owner; - I2 settled: an owner proven gone on two consecutive passes while the record is still in progress; - I3 admission independence: terminal runs whose accounting outlives the 10 min closure, which hold company admission; - I4 exactly-once intents: wake requests ended `skipped`/`failed` on a temporary refusal (heuristic until section E); - I7 visible next action: an agent-assigned `in_progress` issue with no live run, no pending wake and no open recovery action for 5 min. - I5, I6 and I8 are reported as named coverage gaps; I8 comes in the speed-budget PR. - Violations are written once per process to the activity log as `recovery.invariant_violation` (`actorId: ownership-reconciler`, `details.mode: report_only`), insert-only with no live-event or plugin fan-out. Nothing goes to Telemetry (AGENTS.md §5.7). - `native-finalization-reconciler.ts`: `registerOwnershipReconciliationWork` runs the pass in the shared delivery-work scheduler (new queue `ownership-invariant-report`). There is no second loop. - `index.ts` registers it after residual recovery and does not await its first pass, so readiness is not delayed. ### Sleep-aware trigger (rules/sleep-aware-background-work.mdc) - **Trigger:** one pass at registration (startup reconstruction from durable truth). After that, a settled `active-execution-recovery` signal, which run lifecycle, controller renewal and process-evidence writes already emit. Signals are coalesced to at most one pass per 30 s. No producer gained a write. - **Pending predicate and next deadline:** a 30 s follow-up only while the last pass saw a changing in-progress set, or an owner proven gone that needs the second I2 pass. - **Quiet:** an empty set or the same stuck set twice sets no deadline. The test asserts `nextWakeAt() === null` after an idle pass. - **Turn path:** the pass never runs before `turn.start`. Stamps read only cached identity. ## Verification - `npx vitest run src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres, in `server/`): 4 tests pass: - every kind is registered once with no settle path; - verdicts per class: alive (this process's claim), gone (lease of a failed run), unprovable (no owner; another PID space); - each rule fires on its fixture, and I2 fires only on the second pass; - every touched table is byte-identical before and after (report-only); - activity rows are written once per violation, and none are written with `record: false`; - the worker is quiet when idle and schedules a follow-up only while records change. - `npx vitest run src/__tests__/delivery-work-coordinator.test.ts src/services/owner-proof.test.ts src/__tests__/server-startup-feedback-export.test.ts`: pass. - `npx tsc --noEmit -p server`: no errors. ## Risks - No record changes, so no settle risk. The risk is load: one pass runs about 46 small indexed or `limit 201` queries, at most once per 30 s while runs are active, and none when idle. - I4 matches refusal text with a heuristic. It can over- or under-count until section E gives wake requests closed terminal reasons. It is labelled as such in the code. - Violations dedupe per process, so a restart writes a still-open violation again (once per boot). Chaos reports count distinct `(rule, entityId)`. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted run state changes. The pass reads rows of any age, and a missing field classifies as unprovable. - `heartbeat_runs` and `native_run_finalizations` record no host or PID space, so foreign-boot owners there classify as unprovable (`missing_identity`). This is the inventory's finding 2, and the report makes it countable. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f439150cb3 |
fix(recovery): limit the null-host owner fallback to single-host servers and record each use (#19883)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #19868 and #19871 made restart recovery judge a recorded pid only in the PID space that recorded it > - Rows saved before those host columns existed have no recorded host. They kept the previous local dead-pid proof, so in-flight native runs would not be stranded at upgrade > - On an ephemeral (multi-host) server that fallback is unsafe: the row may have been written on another host > - This pull request limits the fallback to single-host servers, records every use, and gives ephemeral servers a working board action > - The benefit is that single-host upgrades still recover on their own, multi-host servers never accept a cross-host proof, and the next release can remove the fallback with evidence ## Linked Issues or Issue Description **Bug description** After #19868 and #19871, a `native_run_finalizations` row with `controller_host_hash = null`, or a run with `process_host_hash = null`, still accepted "the pid is dead here" as proof on every server. That includes an ephemeral server, where the row may come from another host. Nothing recorded how often the fallback was used, so it had no safe removal path. **Expected behavior** - Single-host server: the fallback applies for one release, and each use writes a `legacy_host_unknown_dead_pid_proof` activity-log entry. - Ephemeral server (`PAPERCLIP_EPHEMERAL_SERVER=true`): the row is unprovable, and the board has an action to confirm the previous server stopped. - A mismatched host is always unprovable. Refs #19868, #19871, #19846 ## What Changed - `native-restart-recovery.ts` `evaluateNativeControllerTakeover`: - A coordinator row with `controllerHostHash: null` returns `controller_host_unrecorded` when `ephemeralServerEnabled()` is true. - Otherwise a pid proof from that row carries `legacyHostFallback: true`. - `operator_attested` is accepted as proof (`operator_attested_controller_stopped`). - Receipt-built owners, which have no host key, are unchanged. - New `recordLegacyHostFallbackProof` writes the activity-log entry. It is not a run-log event: run events are progress evidence for restart recovery's crash-loop credit, and the record must not change any recovery decision. It is called from restart recovery, the dead-controller result-lease release, native retirement, and remote warm-session recovery. - New `native-controller-host-recovery.ts`: - `requestNativeControllerStopConfirmation` (board-owned `native_controller_host_unverified` recovery action, idempotent, never displaces another active action). - `confirmNativeControllerStopped`. It verifies the exact recorded controller and stamps `operator_attested` with compare-and-set. It then resolves the action, wakes live-run recovery, and logs `issue.native_controller_stop_confirmed`. - Route `POST /issues/:id/recovery-actions/confirm-controller-stopped`. It is board-only and requires `runtime:confirm_owner_stop`. Generic `restored`/`false_positive` resolution of this cause is refused. A shared `confirmNativeControllerStoppedSchema` validates the body. - UI: `WorkspaceExportRecovery` reuses the "Confirm the previous server stopped" control for this cause, and the generic recovery card hides it. Adds the `issuesApi.confirmControllerStopped` client call. - `recovery/service.ts` orphan reaper: a null `process_host_hash` is judged only when not ephemeral. A successful use records the entry best-effort, after the terminal write. On an ephemeral server the run is left to its existing Stop control. - `ephemeralServerEnabled` moves to a leaf `ephemeral-server-mode.ts` (re-exported by `ephemeral-server-contract.ts`). Importing the contract module from recovery code created an import cycle in the runner child process. - The new route is registered in OpenAPI. - `doc/execution-semantics.md`: documents the host rule and the one-release sunset. ## Verification - `native-restart-recovery.test.ts`: null host on a single-host server is released with the flag. Null host on an ephemeral server is unprovable. A mismatched host is unprovable in both modes. An attested controller is accepted. Receipt owners are unchanged. - New `native-controller-host-recovery.test.ts` (embedded Postgres): - The fallback writes the activity-log entry and no run event. - Ephemeral mode creates exactly one board action. - A wrong run id is refused. - Confirmation stamps the attestation, after which takeover is allowed. The action resolves and the activity is logged. A second confirmation is refused. - `recovery-stale-issue-lock-sweep.test.ts`: on a single-host server a null host is terminalized and records one event. On an ephemeral server it is left alone with no event. The existing audit-failure case still clears the lock. - `WorkspaceExportRecovery.control.test.tsx`: the controller confirmation posts to `confirmControllerStopped` and never to the copyback endpoint. - Locally: these suites plus `remote-warm-session-recovery`, `native-controller-recovery.integration` and `native-startup-cleanup-recovery` (136 passed), and after the CI fixes `native-runner-restart-preparation` and `native-runner-restart-recovery` integration, `openapi-routes` and the suites above (256 passed); the UI recovery suites (52 passed); `node scripts/typecheck-layers.mjs` with no errors in the touched files; `pnpm check:token-gates` clean. ## Risks - Previous-release story: no schema change. Null-host rows saved by earlier releases recover as before on single-host servers, now with an event, and wait for the board on ephemeral servers. The next release removes the fallback; the event count shows whether any rows remain. - On an ephemeral server, a null-host orphaned run process is not reaped automatically. The operator uses Stop. That is the intended "unprovable" outcome. - A board confirmation is an attestation. It applies only to the exact recorded controller (boot id + pid) and only while the host is still unrecorded. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
0189fedffc |
fix(native): bound an idle warm local runner whose controller died (#19879)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A warm runner (`lifecycleMode: warm`) keeps runnerd alive between runs so a follow-up turn starts fast. The follow-up default flip (next PR) makes warm the default for local Codex and OpenCode agents. > - The controller owns a warm runner's idle window. Runnerd ignores the idle timeout itself. > - A graceful controller shutdown suspends idle warm sessions. A controller that dies (SIGKILL, crash) does not. Local runners launch with no process reconnect grace, so the idle runner kept running until its 1-hour connection lease ran out, owned by nobody. > - This pull request gives warm local runners the 7-minute reconnect grace that remote runners already use. > - The benefit: idle runners are reaped by proof within a bounded time, never left orphaned, before warm becomes the default. ## Linked Issues or Issue Description Refs #19874 (warm completion contract), #19846 (TTFT baseline), #19852. **What existing behavior does this improve?** Ownership of an idle warm local runner after its controller dies. **Current behavior** `runnerProcessReconnectGraceMs` is `undefined` for local targets, so runnerd launches without `--reconnect-grace-ms`. An idle warm runner whose controller was killed stays alive until its connection lease (one hour) expires. The restart-chaos harness cannot show this: its K1 kills the controller's whole process tree, runner included. **Proposed behavior** `localRunnerProcessReconnectGraceMs(policy)` returns the remote grace (7 minutes) for a warm policy and `undefined` for per-turn. After the grace, runnerd records `transport_reconnect_grace_exceeded`, stops its provider, keeps durable state and exits. The next run resumes cold. **Reason and benefit** The restart plan requires idle runners to be reaped by proof, never orphaned. Seven minutes is far longer than a controller restart (13–20 s locally) and the long-outage fault holds (3–5 minutes), so a live turn still reattaches. ## What Changed - `native-runner-recovery-contract.ts`: `localRunnerProcessReconnectGraceMs`. - `native-session-executor.ts`: the local runnerd transport passes it as `runnerProcessReconnectGraceMs`. Remote targets keep `null` (lease-owned). - `runnerd-warm-idle-orphan.test.ts` (new): real runnerd and fake Codex. An idle warm runner, after its turn completed, whose controller detaches. - `doc/DEVELOPING.md`: the grace and its bound. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-warm-idle-orphan.test.ts`: with a process grace, the idle runner exits with `transport_reconnect_grace_exceeded` and the next transport resumes the same provider session cold. With no grace (`null`, today's local launch), the runner is still alive after the controller left. - `cd server && npx vitest run src/services/native-runtime/native-runner-recovery-contract.test.ts`: warm gets the remote grace; per-turn keeps none; the grace outlasts a controller restart and a long outage. - `tsc --noEmit` passes for `server` and `packages/paperclip-runner`. - Local restart measurement with warm runners (K1/K2, codex, claude, opencode, n = 3 each): every interrupted run recovered, with recovery latency equal or lower than per-turn. The chaos waves with this change are in the default-flip PR. ## Risks - A warm local runner whose controller stays down for more than 7 minutes exits. A turn in progress then resumes through `resume_dead_runner`, the same path as a controller crash that kills the runner. Remote runners already behave this way. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted state changes. A warm runner launched by the previous release keeps the launch arguments it started with (no grace) until it exits. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
00ad9503fe |
feat(commentary): agents read only their own commentary unless granted commentary:view (#19842)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can submit complaints and suggestions as commentary, and
people with commentary access read them on the Commentary page
> - When a person asks an agent in chat what agents have been
complaining about, the agent cannot read commentary and gets "Board
access required"
> - That response does not say where a person can see it, so the agent
can name the feed but cannot link it
> - Product rule: an agent cannot read other agents' commentary unless
it holds `commentary:view`, which is off by default; it may read its own
> - This pull request implements that rule and adds a remediation to the
denial naming the human page for the agent's own company
> - The benefit is a clear permission boundary and a working link for
the person instead of a dead end
## Linked Issues or Issue Description
**What happened?**
In the `activity.agent-feedback` chat eval, the agent correctly said it
could not read agent commentary and that only the person could open it,
but it gave no link, because the denial named no page.
**Expected behavior**
An agent denied a commentary read learns which page a person with
commentary access can open.
**Steps to reproduce**
1. As an agent, call `GET /api/companies/{companyId}/agent-commentary`.
2. Before this change, the 403 body is only `{ "error": "Board access
required" }`.
## What Changed
- `server/src/routes/agent-commentary.ts`:
- An agent with the `commentary:view` grant reads all commentary its
task access allows. The grant is not among the default agent grants.
- Without the grant, an agent may read its own entries: `?agentId=<its
own id>` on the list, or its own entry by ID.
- Without the grant, listing all commentary, filtering by another agent,
or reading another agent's entry returns 403 "Board access required".
The response's `remediation` names `/<issuePrefix>/activity/commentary`
for the agent's own company.
- A request for another company still fails on company access first,
with no page path.
- Board reads are unchanged: `commentary:view` for the user, plus task
read conditions.
- `agent-commentary.integration.test.ts`: one test covers the full-list
and other-agent denials with the page path, own-list and own-entry
reads, the cross-company denial, the grant allowing a full list, and
that `commentary:view` is not a default agent grant.
## Verification
- `npx vitest run src/__tests__/agent-commentary.integration.test.ts` in
`server`: 21 tests passed.
- The new test fails against the previous route.
- Negative control: with the previous route, the new test fails.
- Related commentary tests: `agent-commentary-reader`,
`agent-commentary-helper`, `commentary-permission-migration` passed.
`agent-commentary-skills` fails the same way with and without this
change (a missing skill file in a temporary directory locally).
- The chat eval rerun with this change is recorded in #19614.
## Risks
- Agents can now read their own commentary when they ask for it
explicitly; before, every agent read was denied.
- The denial's status code and message are unchanged; a `remediation`
string is added for same-company agents.
- There is no UI yet for granting `commentary:view` to an agent.
- An agent learns the company's issue prefix, which it already sees in
task identifiers.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
738058553f |
fix(recovery): resume a continuation that never launched its runner in its own sandbox (#19850)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A chat follow-up on a remote native task runs as a continuation: it starts from its predecessor's saved provider session and, for a warm session, copies the predecessor's runner process identity onto the new run > - When the continuation fails before it asks for a runner of its own, its due same-run retry sees a recorded process identity and reattaches to "this run's" runner. That runner never existed, so reattachment always fails with `native_remote_recovery_lease_mismatch` and the task is blocked > - On staging, 34 OpenCode runs on 31 tasks failed this way in one evening. Each first attempt failed while staging its provider pack, recorded no runner start, and kept its own sandbox lease active > - This pull request proves that shape from durable state, retires the inherited identity, and resumes the run the way its failed attempt ran, in its own retained sandbox > - The benefit is that these retries recover automatically, without a second sandbox and without the unsaved-work hold, while every run that may hold its own provider work keeps verified reattachment ## Linked Issues or Issue Description Refs #19839, Refs #19399, Refs #19755, Refs #19779, Refs #19827 **What happened?** A remote OpenCode chat continuation failed during provider-pack staging. The run had no runner start event, no process identity event, no controller checkpoint, and no provider event of its own. Its saved session (no active turn, no pending request) and its process identity were inherited from the previous run. Its failure detail recorded `providerEventsExist: false`, and its own sandbox lease stayed active. The due same-run retry chose `reattach_remote_runner` because a process identity was recorded. Reattachment failed with `native_remote_recovery_lease_mismatch`, and the run ended in `native_continuation_requires_reconciliation`. **Steps to reproduce** 1. Continue a warm remote native session so the new run inherits the predecessor's runner process identity and saved session. 2. Fail the new run with a retryable error before it requests a runner (for example during provider-pack staging). 3. Let the same-run retry become due. Recovery requests `reattach_remote_runner`, and reattachment fails. **Expected behavior** The retry resumes the run the way its failed attempt ran, in the same sandbox, and continues the inherited provider session. A run with any provider work, runner start, or controller checkpoint of its own keeps verified reattachment. ## What Changed - New `server/src/services/native-runtime/native-inherited-runner-retry.ts`: - `retireInheritedRemoteRunnerIdentity` locks the issue, the coordinator, then the run (the same order as same-run resumption). It retires the recorded process identity only when all of these hold: the coordinator is a result-less, unowned `exact_checkpoint_resume` retry with `providerEventsExist: false`; the run has no runner start, process identity, adapter invocation, tool start, runner event, controller checkpoint, provider event or provider session of its own; the saved session has no active turn and no pending runtime request; and a settled predecessor of the same native session recorded the exact same process identity (PID, group, start time). - It records `nativeInheritedRunnerRetirement` on the run profile, with the run's own sandbox location when that lease is still active, and an `inherited_runner_never_launched` lifecycle event. - `readInheritedRunnerRetainedSandbox` returns that sandbox only while it still matches the run's saved remote location. - `heartbeat/recovery.ts`: a due remote retry calls the retirement first. It returns `retired`, `not_applicable` or `transient`. A retired run (including one a concurrent sweep already retired) goes to the ordinary same-run resume (`claimNativeSessionResumptions`), with every existing admission gate. A lock timeout or database error is `transient`: the sweep defers the run and checks again, and never falls back to reattachment. Any other run keeps verified reattachment. - `heartbeat/workspace-preparation.ts`: an ordinary same-run resume with a retired inherited runner acquires its retained sandbox through the existing exact-lease path (`reattachRemoteLease`), which validates and resumes that lease and never acquires a replacement. It acquires as an ordinary resume does only when the exact-lease failure proves the sandbox is unusable (`stage=environment_acquire|environment_resume`, `field=lease|lease_status|lease_expired|provider_lease_id`). Any other failure, such as a scope mismatch on a live lease, is kept and acquires nothing. - New `server/src/services/retained-sandbox-absent-error.ts`: a sandbox provider's resume reply with no provider lease and `expired: true` (the reply Daytona and other providers send when the sandbox is gone) now throws `RetainedSandboxAbsentError`. Every other mismatched reply keeps the existing error. The exact-lease reattachment in `environment-run-orchestrator.ts` maps it to `native_remote_recovery_lease_mismatch: stage=environment_resume field=lease_expired`, so the same-run resume above acquires as an ordinary resume when the provider has removed the retained sandbox. A transient resume failure is not treated as absence and never acquires another sandbox. - `native-restart-recovery.ts` exports the existing `hasNativeExecutionStartEvidence` so both paths use one definition of "execution reached a runner". - `doc/execution-semantics.md` documents the rule. ## Previous-release state - No schema change. The new profile field is additive. - A run saved by an older release without `providerEventsExist`, without a native session ID, or without a matching predecessor keeps verified reattachment. That path is unchanged. When it loses its runner, it still requests the user's Retry once (#19399). - A run that an older release left with its own runner start, controller checkpoint or provider event keeps verified reattachment, because its outcome may be ambiguous. - A profile without the new marker never changes acquisition. ## Verification - `npx vitest run server/src/services/heartbeat/recovery.test.ts -t "never launched its own runner|keeps verified reattachment"` (embedded PostgreSQL): - Staging shape (inherited session and process identity, no runner start of its own, saved remote workspace, active own lease, recorded PID). Before the fix: fails, because recovery requested reattachment. After the fix: passes. The PID fields are cleared, the marker names the predecessor and the retained lease, `executeRun` is called once with a plain `nativeLeaseOwner` claim, the run still has exactly one (active) lease, and no process is signalled. - Negatives, passing before and after: a provider turn of its own, a runner start of its own, a controller checkpoint of its own, no matching predecessor, a pending runtime request in the saved session, and a failure detail without `providerEventsExist`. Each keeps verified reattachment and gets no marker. - `npx vitest run server/src/services/heartbeat/workspace-preparation.test.ts -t "retained sandbox|retired inherited"`: - A retired inherited runner acquires its retained sandbox with `reattachRemoteLease`. Before: fails. After: passes. - When the retained sandbox is gone, it acquires as an ordinary resume. Before: fails. After: passes. - Without the marker, acquisition is unchanged (passes before and after). - A `nativeWorkspaceSync` reference also resumes its retained sandbox. A live lease's `lease_scope` failure is kept and acquires nothing. - `recovery.test.ts` also covers a second retirement call (returns `retired`, one event) and a held issue lock (defers; no reattachment, no dispatch, PID kept). - `environment-runtime.test.ts`: on exact resume, only a provider reply with no lease and `expired: true` (with or without a workspace sentinel mismatch) is `RetainedSandboxAbsentError`; a different lease or a reply without `expired` keeps the existing error. `environment-run-orchestrator.test.ts`: confirmed absence becomes `stage=environment_resume field=lease_expired` and acquires nothing. - Affected suites pass locally: `recovery.test.ts`, `workspace-preparation.test.ts`, `heartbeat-native-remote-failure-retention.test.ts`, `workspace-recovery-retry-copyback.test.ts`, `native-runner-restart-recovery.integration.test.ts`. `pnpm --filter @paperclipai/server typecheck` passes. ## Risks - **No second executor.** The rule applies only when this run never requested a runner, so no runner of this run exists. The predecessor must be settled, and its identity is matched exactly. The resume then takes the normal same-run claim, which holds the issue lock and admission gates. - **No repeated side effects.** The run has no provider event, turn or tool start of its own. A saved session with a pending runtime request or an active turn keeps verified reattachment. The resumed provider session goes through the existing provider recovery, which refuses a pending side-effecting call with a permanent, named code. - **Bounded.** The retirement takes effect once per run, because afterwards the run has no recorded identity. A contended retirement waits for the next sweep. Further failures use the existing three-attempt provider budget. - I did not run a live Daytona sandbox, or a full preparation through a real sandbox plugin, through this path. The orchestrator's exact-lease checks are covered by its own unit tests. The evidence is recovery and preparation tests on a real database with the staging run shape. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file editing, read-only browser access to staging). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
68db7ed9e3 |
fix(recovery): never accept a dead local pid as proof for an owner on another host (#19868)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery decides that an in-progress record's owner is gone and takes it over > - Two of those proofs use `kill(pid, 0)` on the local host. A pid that is dead here says nothing about a process on another host or in another PID namespace > - So a controller or runtime-control owner that is still alive elsewhere can be taken over while it still runs > - This pull request records the owner's PID space (the existing `controller-process-identity` host hash) and judges a pid only in that same space > - The benefit is that a different host is never proof. Those records follow their existing lease or TTL instead ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 2) found three proofs that accept "the pid is dead here" as proof for an owner on another host: `evaluateNativeControllerTakeover` (`native_run_finalizations` has no host column), the runtime-control sweep (`workspace-operations.ts`), and the orphan reaper `terminalizeOrphanedRunningRun`. Only `controllerProcessExited` checked the host. **Steps to reproduce** 1. Run two control-plane processes on different hosts, or in different PID namespaces, against one database. 2. Let host A own a native finalization lease or a runtime-control operation. 3. On host B, run restart recovery. If A's pid number is not alive on B, B takes over the live owner. **Expected behavior** An owner whose PID space differs from this process's, or cannot be read, is unprovable. Its record waits for its lease, fence, or TTL. Refs #19846 ## What Changed - Migration `0441`: add nullable `native_run_finalizations.controller_host_hash`. - `controller-process-identity.ts`: export `currentProcessSpaceHash()` and `sameProcessSpace()`. An unreadable space is stored as `"unknown"`, which never matches. - `native-restart-recovery.ts`: `currentNativeControllerIdentity` carries `hostHash`, which both coordinator claim writes (`native-restart-recovery.ts`, `native-session-executor.ts`) store. `evaluateNativeControllerTakeover` returns `controller_host_unprovable` before any pid probe when the recorded space is not this one. `native-dead-controller-result-lease.ts` selects the column. - `workspace-operations.ts`: runtime-control owner stamps record `hostHash`. `ownerProcessProvenGone` requires the same PID space (EPERM = alive), otherwise the existing staleness TTL applies. - Not in this PR: the orphan reaper `terminalizeOrphanedRunningRun`. `heartbeat_runs` has no host for `process_pid`, and its writers are spread across legacy and native paths. It follows as a separate PR with its own column. ## Verification - `npx vitest run src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/workspace-runtime-control-recovery.test.ts`: 55 passed. New cases cover another host and an unreadable space (dead and recycled pid) for takeover, plus another host, unreadable space, and legacy stamp for the sweep. The sweep cases also confirm that the TTL still reclaims after the heartbeat lapses. - Red before the fix: the 5 new cases fail on the old code. Green after. - `native-controller-recovery.integration.test.ts`, `native-failed-run-owner-release.test.ts`, `native-startup-cleanup-recovery.test.ts`: 71 passed. - `pnpm db:generate` produced `0441_lucky_chameleon.sql` (one `ADD COLUMN`) and pruned to five snapshots. `pnpm --filter @paperclipai/db check:migrations` passed. Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: `native_run_finalizations` rows saved before this column have `controller_host_hash = null`. They keep the previous local dead-pid proof for this release. Making them unprovable would leave every native run in flight at the upgrade in `awaiting_evidence` with no TTL, because local takeovers have no lease fallback. Rows written by this release onward are strict. Runtime-control stamps without `hostHash` are strict now, because that path has a staleness TTL. - Owners built from other receipts (workspace finalization owner, workspace stop proof) pass no host and keep the previous behavior. Track 2's shared `OwnerStamp` should absorb them. - A new row on a host that really moved (redeploy onto a new machine) waits for its lease or an operator instead of being taken over by pid. That is the intended "different host is never proof" rule. - No new I/O before `turn.start`. The hash is computed once at module load. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
a9ac2e2039 |
feat(native): split local runner startup into stage, launch and session-load spans (#19852)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a person messages an agent, the agent must start its turn fast. Time to first token (TTFT) and agent boot are first-class metrics. > - The Phase 0 baseline (#19846) found that local runner startup costs 1.6–17 s at p50 and up to 50 s at p95. It is the largest part of TTFT. > - The run log records that startup as one span (`runner.session.bootstrap` or `runner.session.resume`). We cannot see if the time goes to staging, to the `paperclip-runnerd` process launch, or to the provider session load. > - This pull request splits local runner startup into three sub-spans in the run log. > - The benefit: the follow-up work to cut follow-up-turn TTFT can measure each step before and after. ## Linked Issues or Issue Description Refs #19846 (TTFT baseline, finding 1 and gap "No sub-spans exist inside local runner startup"). Refs #19838. **What existing behavior does this improve?** The run-log performance spans for local native runner startup. **Current behavior** The local path records one `runner.session.bootstrap` or `runner.session.resume` span that covers runtime staging, the runner process launch and the provider handshake together. The remote path already records `runner.runtime.stage` and `runner.process.launch`; the local path does not. **Proposed behavior** The local path records three sequential child spans of `runner.session.startup`: `runner.runtime.stage`, `runner.process.launch` and `provider.session.load`. **Reason and benefit** The TTFT work needs to know which step dominates before it changes the runner lifecycle. Without these spans, a change to warm runner reuse cannot show which cost it removes. ## What Changed - `RunnerdCodexTransport` has a new optional `onStartupStep` callback. `#start` and `#resume` time three milestones and call it synchronously. A failing callback never changes startup. A nested startup (exact adoption from `thread/start`) keeps the outer clock. - `runner.runtime.stage`: launch preparation before the runner process exists. - `runner.process.launch`: spawn until runnerd accepts the run (`run.prepare`, or the recovery attachment on resume). An adopted live runner records no launch. This keeps the existing meaning in `summarizeStartupReuse`: a launch span means a fresh runner. - `provider.session.load`: `session.open` (or the recovery probe) until the provider identity is confirmed. - `recordNativeRunnerStartupStep` (`native-run-trace.ts`) records each step as a `run.performance.span` row with `mode` and `target: "local"`. It uses `trace.defer`, so the run-log write never delays `turn.start`. - The native executor passes the callback only for a local target. Remote launchers keep their own spans, so there are no duplicates. - `redaction.ts`: allowlist `runner.process.launch` and `provider.session.load` as native span names. - `doc/run-log-events.md`: new section "Local runner startup sub-steps". Data path (AGENTS.md §5.7): run log only, plus Observability spans when an operator sets an OTLP endpoint. The attributes (`mode`, `target`, `outcome`) are already in the closed span-attribute allowlist. No Telemetry change. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-startup-steps.test.ts`: a real runnerd and the fake Codex fixture. Bootstrap and a cold resume each report stage, launch and session load in order, with `ok` outcomes and no overlap. A throwing sink does not change startup. - `cd server && npx vitest run src/services/native-runtime/native-run-trace.test.ts src/__tests__/redaction.test.ts`: the rows have the correct parent, offsets, durations and failed level. A stalled run-log sink does not block the caller. The rows settle before `task.provider_session.measured`. The names survive redaction. - The existing `runnerd-codex-transport*` suites pass (412 tests). Under host load (load average 16–27), the first full run had 7 timing failures in suites that do not set the callback. Each failing file passed when I ran it again alone. `runnerd-codex-transport-settlement` passed 3/3 for the failing case alone. - `tsc --noEmit` passes for `packages/paperclip-runner` and `server`. ## Risks - Low risk. Without `onStartupStep`, the transport behaves as before: the clock is not created and each milestone call is an optional-chain no-op. - More run-log rows: three per local runner startup. - The step boundaries are controller-side observations. `runner.process.launch` includes the runnerd authentication and the `run.prepare` acknowledgement, not only the OS spawn. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
fa86162790 |
refactor(recovery): one process boot id and a cached controller identity (#19851)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Restart recovery decides when a record owned by a dead server process may be released > - One server process minted three different boot UUIDs, one per module (`controller-process-identity.ts:13`, `native-restart-recovery.ts:284`, `legacy-controller-lease.ts:8`) > - The controller identity also ran `ps -o lstart` on macOS at every native claim, preparation check and lease renewal, which is on the path before `turn.start` > - The durable ownership plan (Phase 1, step 1) needs one process identity before it can add one owner stamp and one prover > - This pull request makes the three modules stamp one boot id and reads the process start time from the OS once > - The benefit is one meaning of "owned by this process" across tables, and no `ps` spawn on the turn path ## Linked Issues or Issue Description Refs #19831 (plan: `doc/plans/2026-11-07-durable-ownership-and-reconciliation.md`, Phase 1). Refs #19846 (Phase 0 inventory, finding 1, and the TTFT baseline row "Controller identity"). **Problem** One process has three boot ids, so the same fact ("this record is owned by this process") has three encodings. The uncached controller identity spawns `ps` on macOS on every claim and renewal. **Expected behavior** One boot id per process. The OS start-time read runs once per process. **Impact** Phase 1 needs one identity to build `OwnerStamp` and `proveOwnerGone` on. The turn path loses one subprocess spawn per claim and renewal. ## What Changed - `controller-process-identity.ts` owns the single process boot id. It adds `currentProcessStartedAtMs()`, which caches a successful OS start-time read for the life of the process and retries a failed read. It also exports `currentProcessSpaceHash()` for the coming prover. - `native-restart-recovery.ts`: `currentNativeControllerIdentity()` and `isCurrentNativeControllerProcess()` use the shared boot id and the cached start time. Its own `randomUUID()` boot id is removed. The fallback to the server-info start time is kept. - `legacy-controller-lease.ts`: `legacyControllerBootId` is now the shared boot id. - Tests in `native-controller-identity.test.ts`: one boot id across native claims, legacy leases and creation intents; one OS read across many claims and renewals; a failed read is retried, not cached. ## Verification - `npx vitest run src/services/native-runtime/native-controller-identity.test.ts src/services/legacy-controller-lease.test.ts src/__tests__/heartbeat-pending-cleanup-sweep.test.ts src/__tests__/sandbox-creation-operations.test.ts src/services/heartbeat/upgrade-readiness-post-result.test.ts src/services/heartbeat/recovery.test.ts` (in `server/`): 6 files pass. - `npx tsc --noEmit -p server`: no errors. - I checked every reader of the three ids. Each one compares a stored column only with the id of the module that writes that column (`heartbeat_runs.controller_boot_id` with the legacy id, `native_run_finalizations.controller_boot_id` with the native id, metadata `bootId` with the creation id). No reader compares the ids of two modules, so making them equal changes no decision inside one process. ## Risks - Low risk. No schema change and no persisted shape change. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): rows saved by an older release carry that release's random per-module ids. They never equal the new process's id, exactly as a row from any earlier boot never did. They carry forward with no migration. - The start-time cache is correct because a process's own start time never changes. A failed read is not cached, so a transient `ps` failure does not pin the fallback value. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
824b25654c |
fix(ui): keep a Retry for retained-file holds whose source cannot be verified (#19840)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a native run finishes but its files cannot be saved, the task stays blocked behind a retained-file recovery hold. > - The server records that hold as blocked and retryable, and the task notice tells the user to "Retry to try again". > - The task page hides its own Retry for these holds and relies on the retained-file recovery panel instead. > - That panel only binds the recovery record when a read-only source inspection returns `verified`. > - Inspection can return `blocked` for normal reasons: a sandbox that predates ownership receipts (state saved by an older release), or a workspace that moved on after its files were recovered. > - Then neither surface offers an action, and "Check file recovery status" always answers "File recovery changed. Refresh the task", also after a reload. > - This pull request binds the record by its own identity, uses inspection only to gate the physical file-recovery request, and keeps the task Retry visible whenever the panel has no control of its own. ## Linked Issues or Issue Description Refs #18848 (the change that added the retained-file recovery panel and hid the generic Retry for these holds). **What happened?** A blocked task with an active `native_workspace_sync_out_retry_exhausted` hold showed "The saved files need recovery before work can continue", "The recorded source is unavailable or has changed", and "Saving the agent's files failed after automatic retries. Retry to try again". It showed no Retry and no recover button. "Check file recovery status" always answered "File recovery changed. Refresh the task to inspect the current recovery", and a reload did not change it. The hold was saved by an earlier release: `systemRecovery` exhausted with `allocation_ownership_ambiguous` / `service_required`, and `workspaceRecovery` blocked with `retryable: true`. The source inspection endpoint returned `status: "blocked"` for it. A second task whose files were already recovered showed the same "need recovery" and "has changed" text, because its inspection reported a reference mismatch after recovery. **Expected behavior** A retryable hold always shows a working Retry or recover control. A notice that says "Retry" has a Retry button. Status checks never loop on "changed, refresh". Recovered files show as recovered. When the source cannot be verified, the page names the reason and who must inspect it. **Steps to reproduce** 1. Use an active retained-export hold whose `workspaceRecovery` is blocked and retryable, and whose `systemRecovery` is exhausted. 2. Make the source inspection (`GET /api/issues/:id/recovery-actions?inspectSourceRunId=…&inspectSourceActionId=…`) return `status: "blocked"`. 3. Open the task as a board operator. 4. Before this change, the page shows no Retry and no recover button. "Check file recovery status" reports "File recovery changed. Refresh the task". **Paperclip version or commit** Observed on build `8812c19`. The regressions fail on master `0f5ce5050e` before the production edits. ## What Changed - `WorkspaceExportRecovery`: bind the recovery record by action, run and schema identity, not by inspection status. Inspection status now gates only the physical "Recover saved files" request (and the request re-checks it before POST). - `WorkspaceExportRecovery`: report through `onRecoveryControlChange` whether the panel offers its own control. It reports `true` only while the source is loading or verified and the files are not yet recovered, or while it shows the previous-server stop confirmation. - `IssueDetail`: hide the task Retry for retained-export holds only while the panel reports that it owns the control. Otherwise the existing task Retry (`POST /issues/:id/retry`) shows. The server renews a bounded recovery generation and re-verifies the source. - Show recovered files as recovered even when inspection no longer matches after recovery. - Name an unverifiable source in plain words (with the code for other reasons). State that Paperclip does not apply files it can't verify. - Recovery text never asks a person to inspect a sandbox or environment, and never names an owner or administrator as the one who must act. This covers the retained-file panel, the run source inspector, the copyback wait, and the native cleanup quarantine message. The execution blocker and the recovery action read model describe a spent recovery budget from its current state, so text saved by older releases ("If Retry keeps failing, inspect the run and its environment") is not shown. - Task Retry uses the existing `createUuid()` helper, so it works on plain-HTTP previews without `crypto.randomUUID`. An accepted Retry also refreshes the inspected file-recovery state. After a failed accepted request, the unverified-source reason stays visible; it hides only while recovery runs. - When the recovery record was replaced, "Check file recovery status" reloads the task detail instead of telling the user to refresh. - Tests: component regressions for an old-release hold, a recovered hold with a mismatched source, verified ownership, and a replaced record. A page regression proves Retry is shown and calls `retryTask` for unverified and stale sources. A server safety test proves that Retry for the old-release hold shape is accepted, opens one new bounded generation, queues file recovery again, keeps the hold active, and is idempotent. ## Verification - RED (master `0f5ce5050e` production files, new tests): 5 component tests fail. The failure output shows the exact staging text: "Check file recovery status", "File recovery changed. Refresh the task to inspect the current recovery.", "The recorded source is unavailable or has changed." 2 page tests (unverified and stale) fail with `expected [ 'Check file recovery status', … ] to include 'Retry'`. - GREEN: `cd ui && npx vitest run src/components/WorkspaceExportRecovery.control.test.tsx src/components/WorkspaceExportRecovery.test.tsx src/components/RunWorkspaceRecoverySurface.test.tsx src/components/ExecutionBlockerNotice.test.tsx src/pages/IssueDetail.test.tsx` passes 320 tests. - Server: `cd server && npx vitest run src/services/task-retry.test.ts` passes 81 tests, including the new old-release safety case. This is safety evidence, not a red: the server path was already correct, and the defect was in the UI. - Full UI suite: 11564 pass. 4 tests in `IssuesList.test.tsx` and `CompanyEnvironments.test.tsx` failed under full-suite load and pass in isolation (106/106). They are unrelated to this change. - `pnpm check:token-gates` is clean. `node scripts/run-tsc.mjs -p ui/tsconfig.json --noEmit` reports no errors in the touched files. The remaining errors are local artifacts that were not built (plugin SDK UI entry, generated Storybook catalog). - Staging was read only (GET requests). The inspect request did not loop in a fresh tab: two calls in 30 seconds. Most of the tab's request volume is periodic company issue-list refetches, which this change does not touch. ## Risks - Low risk. This is a UI-only behavior change. Server authorization, source verification, stop receipts, and the bounded recovery policy are unchanged and remain authoritative. - The task Retry is now visible for retained-export holds whose source does not verify. Retry renews one recovery generation. The worker re-runs the same source-identity checks, so Retry cannot bypass an ownership refusal. If the refusal is permanent (for example an ambiguous sandbox allocation), the hold returns to exhausted and the page names the reason. An automatic fallback for such sources is follow-up work. - Wording: a parametrized UI test renders every recovery state and asserts that no administrator, owner, or "inspect the environment" copy appears. A server test proves that a stored old-release message is replaced by state-derived text. Both failed before the change. - Previous-release story: the old-release hold shape (board-owned, `systemRecovery` revision 5 exhausted with `service_required`, `workspaceRecovery` blocked and retryable) is covered by component, page, and server tests. No persisted state shape changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (repository search, file edits, shell, test execution, and a read-only browser). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
630411af53 |
fix(recovery): require a 1 s start-time gap and treat EPERM as alive in owner proofs (#19848)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery code decides whether the process that owns an in-progress run is gone > - Two proofs compare process start times to the millisecond, but `ps -o lstart` on macOS and BSD reports whole seconds > - So a live owner can look like a reused PID, and recovery can release it while it still runs > - Signal-0 handling also disagrees: one check treats EPERM as dead, while all others treat it as alive > - This pull request puts one start-time rule and one EPERM rule in `controller-process-identity.ts` and uses them at every site the Phase 0 ownership inventory named > - The benefit is that no proof releases a live owner because of clock precision or a permission error ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 1) found inconsistent "owner gone" proofs. `native-failed-run-owner-release.ts` (`recordedProcessMayBeLive`, `runProcessMayBeLive`) and `native-session-executor.ts` (`verifiedRecoveryProcessIsAlive`) treat any start-time difference of 1 ms or more as a reused PID. `native-restart-recovery.ts` and `controllerProcessExited` require a gap of at least 1 s. `local-service-supervisor.ts` (`isPidAlive`, `isProcessGroupAlive`) treats EPERM as dead. **Steps to reproduce** 1. On macOS, record a live process start time with millisecond precision. 2. Run the abandoned-owner release pass, or the recovered-runner liveness check, against that live PID. 3. The pass releases the owner, or the check reports the runner as gone. **Expected behavior** A sub-second start-time gap is ambiguous and keeps the owner. EPERM means the process exists. Refs #19846 ## What Changed - `controller-process-identity.ts`: export `processExists` (EPERM = alive) and `processStartsDefinitelyDiffer` (gap >= 1 s; a missing time proves nothing). `controllerProcessExited` uses the new helper. - `native-failed-run-owner-release.ts`: the controller and run-process checks use the shared helpers instead of exact millisecond equality. - `native-session-executor.ts`: `verifiedRecoveryProcessIsAlive` accepts a start time within 1 s as the same runner. Its signal path uses the same check. - `native-restart-recovery.ts`: `definitivelyDifferentProcessStart` delegates to the shared helper. No behavior change. - `local-service-supervisor.ts`: `isPidAlive` and `isProcessGroupAlive` treat EPERM as alive. ## Verification - New `controller-process-identity.test.ts`: the 1 s boundary, EPERM and ESRCH for every signal-0 check, and a live spawned runner whose recorded start is 500 ms off. - `native-failed-run-owner-release.test.ts`: two new cases (controller and run process) with a live child and a recorded start 500 ms off. The owner stays `observed`. - Red before the fix: with the old call sites, all 4 new cases fail (`releaseAbandonedNativeOwners` returns 1 and releases the live owner; EPERM returns false; the live runner reads as gone). Green after the fix. - `npx vitest run src/services/controller-process-identity.test.ts src/services/native-runtime/native-failed-run-owner-release.test.ts src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/local-service-supervisor.test.ts`: 59 passed. ## Risks - Low risk. The change only makes proofs stricter: a sub-second gap or EPERM now keeps an owner that the old code released. Such a row waits for the existing lease, TTL, or real process exit. - A recycled PID whose new process started within 1 s of the recorded one now stays held until it exits. That is the same rule the takeover path already used. - Previous-release story: no persisted shape changes. Rows saved by the previous release are evaluated with the stricter rule. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
e2d490c61b |
feat(observability): anchor native time to first token at the accepted wake (#19838)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When someone messages an agent, the reply must start fast. The durable ownership plan (#19831) makes time to first token (TTFT) a hard invariant. > - Phase 0 of that plan records a TTFT baseline per adapter and environment, measured from the moment the message or wake is accepted. > - Today `task.time_to_first_token` starts at the wake comment, the answered question, or the run row's creation. A wake request that was accepted earlier (deferred, coalesced or retried) is not part of the measurement, and the span does not say which start it used. > - This pull request moves the start back to the earliest accepted input and names it on the span. > - The benefit is that the baseline and later budgets measure what the user waits for, with no new work before `turn.start`. ## Linked Issues or Issue Description Refs #19831 **What is the problem?** `task.time_to_first_token` does not include the time between an accepted wake request and the creation of its run row. It also does not record which input it started from. **What is the expected outcome?** The span starts at the earliest accepted input and carries an `anchor` attribute: `comment`, `question_response`, `wake_request` or `run_created`. **Why does it matter?** Phase 0 of the durable ownership plan sets TTFT budgets from this baseline. A start that skips queue time hides the latency that deferred wakes add. ## What Changed - `resolveNativeTimeToFirstTokenStart` (pure) in `native-run-trace.ts` picks the start and the anchor. `anchor` joins the closed span-attribute allowlist. - `readNativeRunWakeRequestedAtMs` reads `agent_wakeup_requests.requested_at` for the run. It never throws. - `native-session-executor.ts` calls it only after the first deliverable text, so it adds no wait before `turn.start`. The span now ends at the text commit time, not after the trace write. - Updated `doc/run-log-events.md` and `doc/observability.md`. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-run-trace.test.ts src/services/native-runtime/native-run-wake-anchor.test.ts` (26 pass). - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t first` (41 pass). The existing TTFT test now asserts the anchor for each ingress. ## Risks - Low risk. This changes only the run-log and OTLP span. It is not Telemetry (AGENTS.md §5.7), and it does not change runtime behavior. - One indexed primary-key read per native dispatch, after the first text. A read failure drops the anchor and keeps the old start. - TTFT values can grow for deferred wakes. That is the intended correction. `startOffsetMs` can be negative when the wake request comes before the trace start. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] 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 Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
84908d176b |
fix(recovery): name the failed check in native_remote_recovery_lease_mismatch (#19839)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote native runs execute in a sandbox, and recovery can reattach a run to its original sandbox runner > - Reattachment checks the saved lease, workspace, and runner identity in five places. Each failed check throws the same bare `native_remote_recovery_lease_mismatch` > - On staging, 34 OpenCode runs on 31 tasks failed with this bare error in one evening. The run records could not show which check failed, so the cause could not be confirmed from the run alone > - This pull request makes every producer name the failed step and field with closed values > - The benefit is that an operator and the next investigation can see which reattachment check failed, without new logging or identifiers in the error ## Linked Issues or Issue Description Refs #19399, Refs #19733, Refs #19755, Refs #19779 **What happened?** Remote same-run retries failed with `native_remote_recovery_lease_mismatch` and the task was blocked for reconciliation. The error is thrown by environment acquire, environment resume, workspace preparation, workspace sync, and remote runner verification. All of them used the same bare string. The run's `error` field and its run events did not show which check failed. **Steps to reproduce** 1. Start reattachment for a remote native run whose saved lease, workspace reference, or runner identity no longer matches. 2. Read the run's `error`. 3. It shows only `native_remote_recovery_lease_mismatch`. **Expected behavior** The error names the step and the field that differed, with no identifiers, and existing consumers of the code keep working. ## What Changed - New `server/src/services/native-runtime/remote-recovery-lease-mismatch.ts`: the code constant, a closed `stage` and `field` vocabulary, and `remoteRecoveryLeaseMismatch(stage, field)`. It builds `native_remote_recovery_lease_mismatch: stage=<stage> field=<field>`. - `environment-run-orchestrator.ts` (stages `environment_acquire` and `environment_resume`), `heartbeat/workspace-preparation.ts` (`workspace_preparation`), `native-workspace-sync.ts` (`workspace_sync`), and `verifyRemoteRunnerReattachment` in `native-session-executor.ts` (`runner_verification`) now throw the detailed form. Each check keeps the same condition and order. Only the message changes. - `restart-interrupted-turn.ts` uses the shared constant. It already matches the code exactly or as the `code: ` prefix, so automatic continuation after a reattachment loss is unchanged. No other consumer matches the code. - `doc/execution-semantics.md` documents the detail format. ## Previous-release state This change adds no persisted state and changes no saved identity. Runs that a previous release failed keep the bare `native_remote_recovery_lease_mismatch` in `error`. Runs failed by this release store the detailed form. The only consumer (`isReattachmentLoss` in `restart-interrupted-turn.ts`) accepts both forms, and `workspace-recovery-retry-copyback.test.ts` covers both. ## Verification - Exact-detail assertions in the producer tests: - `server/src/__tests__/environment-run-orchestrator.test.ts`: every scope, status, expiry, provider, lease-ID, cwd, plugin-task, and resume case asserts its stage and field. Against the previous producer, 21 of these cases fail (the message was the bare code). With this change, all 57 tests pass. - `native-session-executor.test.ts`: a replaced sandbox reports `runner_verification`/`provider_lease_id`. A missing durable runner identity reports `identity_run_id` before any sandbox read. - `native-workspace-sync.test.ts`, `heartbeat/workspace-preparation.test.ts`, `native-controller-recovery.integration.test.ts` assert their exact detail. - End to end on embedded PostgreSQL: `heartbeat-native-remote-failure-retention.test.ts` records `native_remote_recovery_lease_mismatch: stage=environment_resume field=lease` on the run when the provider no longer resumes the lease, and it still requests the user's Retry once. `workspace-recovery-retry-copyback.test.ts` adds a detailed-form row that still continues once. - The affected server test files pass locally. `pnpm --filter @paperclipai/server typecheck` passes. ## Risks - Low risk. Only the error text changes. Conditions, ordering, and fail-closed behavior are unchanged. - Logs and run errors show a longer message. The detail has closed values only, so it adds no identifiers or secrets. - A first version of this PR also changed remote retry selection. Review showed that a plain resume in a new sandbox hits the remote workspace's unsaved-work hold, and that a `destroy` disposition does not prove the old sandbox is gone. That change is removed. It needs a separate design. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file editing, read-only browser access to staging). ## 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> |
||
|
Dotta
|
86bc3db693 |
feat(runner): search and load unassigned company skills (#19676)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents use tools to read reusable company skills. > - The existing native skill tools read only skills assigned to the agent. > - An agent can stop when a task needs another skill that is already in the company library. > - This pull request adds tools to search and read accessible company skills on demand. > - Agents can use the saved instructions without a persistent assignment change. ## Linked Issues or Issue Description **Problem or motivation** An agent cannot use an unassigned company skill through the standard native skill tools. A generic API workaround needs extra discovery and can return the full library. **Proposed solution** Add `search_skills` and `load_skill` to the native tool catalog. Search returns small pages of stored metadata. Load returns text from a saved version and lists its files. Both tools use the current run identity and existing company and project access checks. **Alternatives considered** Assign each skill before a read, or use generic API discovery. Both add work to a read that the company already permits. **Roadmap alignment** This extends the existing skills discovery path. It does not add a store, permission model or installation flow. Related: #15409 adds an assigned-skill loader to a different adapter. ## What Changed - Add bounded company skill search. Search does not synchronize sources or fetch repositories. - Add versioned text reads for SKILL.md and supporting files. Page text and file lists. Reject unsafe paths and binary files. - Recheck access after storage reads. Keep the existing closed Muse, skill-test and completion-review tool scopes. - Add integration tests with isolated PostgreSQL and real local package storage. Update the native tool count assertion and skill documentation. - Limit text files to 16 MiB before body reads. Accept saved file paths up to 4,096 characters plus the repository prefix. Return the response digest only on the first page. - Add optional byte limits to the package readers. Select only the requested legacy file in PostgreSQL. Cast the size comparison to `bigint` so existing callers can omit the limit. - Cache decoded text and its digest for one run. Limit the cache to 64 MiB and 64 files. Check access on each page and clear the cache when the run closes. - Read repository source files and wrapper inputs through verified warm reads or isolated recovery. These reads do not hold the company startup lock. ## Verification - Local verification tested tree `7ba5828387ca0a9c2ee99655818a28de1235a854`, published as commit `c9d04cba601541cca506f5b72aa6f7272a25379f`. The published tree matches that tested checkout. Current head `33fa3d3d4c7988288685ce7c6499d064e0cb9079` (tree `8daaa2f3264d1cd65a9b6d85dbb9d3729ac4a29d`) removes six comment lines from three source files; its executable code is unchanged from the locally tested revision. - Four native skill and authority Vitest suites: 72 passed and 7 existing sandbox checks skipped. All 20 skill-library integration tests passed. - Skill-library and package-store suites: 65 passed. This includes the 45 package-store cases and the same 20 integration cases. Ran with umask `022` for executable-file tests. - Server source and test typecheck layers passed in low-memory mode. The affected source cycle also passed a final separate check. - `git diff --check` and `node scripts/check-module-boundaries.mjs` passed. - Red/green evidence: the new default inline-read case reproduced PostgreSQL error `22003`. Two max-size paging cases reproduced extra body reads. Warm and cold repository reads timed out while another connection held the company startup lock. All five cases pass after the fixes. - Both 16 MiB paging cases check that the second page performs no additional body read. They also check that source-access revocation blocks a cached page. Warm and cold lock cases load both the generated wrapper and a supporting file while the startup lock remains held. - The first cold-read test attempt had an incorrect helper import. Corrected the test import, reproduced the lock timeout, then verified the fix. - [CI on the previous revision](https://github.com/paperclipai/paperclip/actions/runs/42319652795) passed typecheck, build and policy. Serialized shard 2 failed in the unchanged test `schedules bounded retries for failed accepted interaction continuation wakes`. The same case passed locally: 1 passed; 416 cases were excluded by the filter. The cause of the earlier CI failure remains unknown. [CI for |
||
|
DottaandPaperclip
|
b8e02fc2b4 |
fix(native): preserve bounded larger web inventories (#19832)
## Thinking Path > - Paperclip manages AI agents and preserves their work across server changes. > - Native harness preservation stores retained web-server files in authenticated archives. > - A retained skill can contain hundreds of valid local files. > - The WEB evidence owner rejected any directory or inventory above 128 entries. > - This blocked preservation of two 462-file skills even though their payload fit the existing byte limits. > - This pull request shares finite inventory bounds between capture and archive validation. > - It preserves every file and keeps the execution, identity, writer, and publication guards. ## Linked Issues or Issue Description Refs: #19814 **What happened?** WEB evidence preservation rejected a retained source with 927 local skill files. The source had settled execution ownership. The files fit the existing payload limits. The directory enumeration failed before archive publication because its entry limit was 128. **Expected behavior** Preserve a valid finite inventory within explicit file, payload, path, and metadata limits. Keep remote recovery obligations unchanged. **Steps to reproduce** 1. Retain two 462-file skills and one three-file skill under a settled native source. 2. Request canonical WEB evidence preservation. 3. The old owner rejects directory enumeration. The added integration test reproduces this refusal with real files and PostgreSQL. ## What Changed - Share the WEB limit through the existing controller archive owner: 10,000 total entries, 4 MiB of manifest metadata, and 4,096 bytes per path. - Apply the limits to filesystem capture, published binding validation, and archive descriptor validation. - Keep the existing 16 MiB per-file and 64 MiB payload bounds. Keep authority history at 128 entries and 32 epochs. - Add real archive and cold-restore coverage for 927 files, legacy manifest compatibility, and bound refusals. Document the limits. ## Verification - Genuine baseline RED: the new 927-file integration case reaches the original `captureWebEvidence.directoryNames` limit and fails before publication. - The initial setup attempt failed before test collection because the generated plugin SDK was absent. The required SDK build fixed setup; it was not counted as a regression result. - The first affected three-suite run passed 93 cases. The large roundtrip exceeded the generic 15-second case deadline. That failure is retained. The isolated large case passed with a 60-second case limit, but full CI later exceeded that limit while the other 4,577 tests in its shard passed. Both timeout failures are retained. The final fixture keeps all 927 files and 18.5 MB, uses bounded 16-way independent comparisons, and has a 120-second whole-case limit. The canonical preservation signal remains 60 seconds. No production deadline or assertion changed. - The 927-file case then passed: encrypted archive, authenticated cold restore, all bytes/hashes/modes, unchanged remote leases, and database-only validation with source volume absent and storage reads forbidden. - The final metadata repair adds a genuine RED case: an entry list below 4 MiB previously emitted a larger complete manifest without rejection. The owner now checks the complete serialized manifest. All ten focused cases passed after that repair. The final bounded-I/O fixture passed the same ten cases in 37.26 seconds, including the full 927-file roundtrip. All 93 other affected cases passed in the preceding three-suite run. Fresh required CI supplies broad final-head coverage. - Server TypeScript, module boundaries, and whitespace checks pass. Run the focused PostgreSQL checks from `server/` with the repository wrapper: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts --testNamePattern 'preserves all 927|existing 128-entry WEB|WEB manifest beyond|oversized WEB filesystem' --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks The owner can hold more entries in memory. The fixed 64 MiB payload, 4 MiB metadata, path, and entry bounds constrain that cost. The archive format and old receipt bytes do not change. Older builds still have the 128-entry limit and cannot qualify a larger new receipt. This change does not settle remote recovery, grant execution authority, alter finalizer locks, or change publication CAS rules. No hosted preservation or deployment was performed for this PR. ## Model Used OpenAI `gpt-6.1-sol`, with code execution and repository tools. The context window size is not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
a94881d1a7 |
fix(heartbeat): save a comment refused by a temporary hold and route post-quarantine input to its own gate (#19829)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user's follow-up comment or Retry must be delivered once, eventually, even when a temporary company admission hold (accounting still settling) is active > - #19741 kept claimed runs queued through such a hold and woke held work when it cleared > - A restart-chaos wave still lost a comment, because its wake was refused before admission and written as a terminal skip > - The same wave left a Retry waiting forever for a stop record of a process that never existed > - This pull request saves the refused comment as a deferred receipt, and routes input after a pre-launch quarantine failure to the quarantine's own gate > - The benefit is that the comment is delivered after the hold, and the task asks for the action that actually unblocks it instead of offering a Retry that can never start ## Linked Issues or Issue Description Refs #19741 and #19702. **What happened?** In a restart-chaos wave, a failed run's accounting held the company for 600 s. 1. A user commented while the task's run was live. The enqueue budget gate refused the comment's wake before it reached the deferral behind the live run. Because it was a user request with no saved receipt, it was written as `skipped` (`budget.blocked`). The live run finished without it and the task ended `done`, so the comment was lost. 2. A Retry created a run that the native cleanup-quarantine fence refused before it requested any runner launch (`native_session_cleanup_quarantined`, no process events). A second Retry then waited with `process_identity_missing` ("The previous run has no verified stop record") for a process that never existed. The task kept offering Retry, and that Retry could never start. **Expected behavior** A comment refused by a temporary hold stays saved and is delivered once later. Input after a run that was refused before launch is handled by the quarantine gate: Retry is not offered (send a new message), and a new message gets the verified-stop check of the previous provider session. ## What Changed - `heartbeat/queue.ts`: when the enqueue budget gate refuses a user's comment wake and the block is temporary (not a reached hard stop, per `isBoardOwnedBudgetBlock`), the wake is saved as a `deferred_issue_execution` receipt. The receipt carries the comment ids and an `executionWait` of `budget.blocked`, and the gate notifies the deferred-receipt worker and throws `PersistedExecutionWaitRefusal`. It is not written as `skipped`. Normal promotion (when the live run ends) and #19741's hold-release wakeups deliver it. - `native-local-process-stop.ts`: new `nativeRunRefusedBeforeLaunch`. It returns true for a failed native run with `native_session_cleanup_quarantined`, no recorded process, and no launch-request or identity event. - `explicit-native-continuation.ts`: such a run passes the process-identity check, so the existing quarantine branch decides. A Retry gets `cleanup_quarantined` (shown with `canRetry: false` and "send a new message"). A new message gets `verifyStoppedNativeSessionForContinuation`. Any launch request or identity record keeps the original stop proofs. - `native-session-executor.ts` (`verifyStoppedNativeSessionForContinuation`): when the source has no process record and no launch event, the verifier relies on the previous owner's stop evidence. Before this change it required the source to share the owner's process identity, so a new message stayed held even after the owner stopped. A source that carries a process identity must still match the owner exactly. ## Verification Each new test fails on `master` and passes with this change, on embedded Postgres: - `server/src/__tests__/heartbeat-held-comment-wake.test.ts`: a comment whose wake the hold refuses at enqueue is saved with a `budget.blocked` wait and is not skipped. It is delivered exactly once after accounting settles. - `server/src/services/explicit-native-continuation.test.ts`: - A run refused by the quarantine before launch routes a new message to the quarantine's stop verification and a Retry to `cleanup_quarantined`. These failed on `master` with `process_identity_missing`. - Guards (green on both): a quarantined run that requested a launch, and another failure without a process record, still require `process_identity_missing`. - `server/src/services/quarantined-task-retry.test.ts`: a never-launched quarantined source is certified once the previous owner has stopped (red on `master`). It stays held if the source requested a launch or the owner is still running. All 31 tests pass, and the `stopped native conversation physical cleanup` suite passes (46). - Related suites pass locally (6 files, 620 tests): explicit native continuation, held comment wake, queued comments routes, comment wake batching, refused assignment wake, deferred receipt work. Two `deferred-execution-receipt-work` tests failed once under parallel load and passed when run alone. ## Risks - The saved-comment path applies only to user comment wakes refused by a temporary budget hold. Hard stops, system wakes, case handoffs, durable chat requests, and existing saved receipts are unchanged. - The pre-launch rule is limited to the quarantine failure code with no launch evidence. Every other missing process identity still requires the existing stop proof. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, run as an agent in Claude Code with tool use (shell, file editing, local test execution, kept restart-chaos instance inspection). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
45c2bd94c6 | fix(native): name the interrupted attempt when an exact resume is refused (#19709) | ||
|
DottaandClaude Opus 5.5
|
b2af45f0f1 |
fix(runner): let OpenAI API-key Codex agents generate images with their existing key (#19654)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - New users often connect a Codex agent with an OpenAI API key during
onboarding.
> - A user asked such an agent for a logo. The agent said that the
built-in image generator was not available and that its fallback needed
a separate API key. It then drew an SVG by hand.
> - #19516 removed `--disable image_generation`. That change is correct,
but it does not reach this path. The pinned Codex 0.160.0 shows its
`image_gen` tool only when Codex signs in with ChatGPT. The check is
`image_generation_available` in `codex-rs/core/src/tools/spec_plan.rs`.
An API key never gets the tool.
> - The runner keeps the provider key out of the agent's shell on
purpose, so the skill's `scripts/image_gen.py` fallback cannot run.
> - This pull request adds a Paperclip `generate_image` tool. The server
makes the image with the agent's existing OpenAI key. The key never goes
to the agent process or the shell.
> - The benefit is that API-key Codex agents can make images with no
second key. The spend goes to the agent's budget and the budget can stop
it.
## Linked Issues or Issue Description
Refs #19516
**What happened?**
An agent connected through onboarding with an OpenAI API key (Codex
harness, `gpt-5.6-sol`) could not generate images. The agent said: "The
image skill's built-in generator isn't available in this run, and its
fallback would require a separate API key." It then drew an SVG instead.
**Steps to reproduce**
1. Create a company with a Codex agent that uses an OpenAI API key.
2. Ask the agent for a raster image, for example a logo PNG.
3. The agent reports that image generation is not available. It does not
make the image.
**Expected behavior**
The agent makes the image with the OpenAI key that already powers it.
**Root cause**
The agent ran on the native runner's Codex app-server path, with
`auth.json = {"OPENAI_API_KEY": ...}`. Codex 0.160.0 removes
`image_gen.imagegen` from the model's tools unless
`current_auth_uses_codex_backend()` is true. That function is true only
for a ChatGPT sign-in.
I proved this with the real pinned binary. I gave it the runner's
isolated launch arguments and the API-key auth file, and pointed it at a
local Responses endpoint. The first model request offers only `exec`,
`wait`, `request_user_input` and the collaboration tools. #19516's test
checked only `codex features list`, so it could not see this gate.
## What Changed
- **New `server/src/services/native-runtime/generate-image-tool.ts`:**
- The tool schema and definition.
- `imageGenerationBindingForRun`. It applies only to native Codex runs
on a direct OpenAI `api_key` connection with no router selection.
- The OpenAI Images API call (`gpt-image-2`, `b64_json`, PNG check, 180
s timeout). The reply is read as a stream with a byte cap before it is
parsed. A decoded image larger than 20 MiB is refused.
- A reviewed rate card for `gpt-image-2`: text input $5, image input $8
and image output $30 per 1M tokens, from
developers.openai.com/api/docs/pricing, retrieved 2026-11-06.
- A create-only, symlink-confined local write.
- **`paperclip-runner-tool-authority.ts`:**
- Adds the `imageGeneration` binding.
- Advertises the tool only for standard mode with a workspace. A remote
target must also have an installer. Dot, Muse, Grok and native review do
not get the tool.
- Adds the `#generateImage` handler. Before any spend, it checks again:
the run and task, budget authority, `issue:mutate`, case handoff, and
the connection.
- Stores a claim-before-spend per-call receipt in
`resultJson.imageToolReceipts`. A retry or recovery replays the receipt
and does not pay again.
- Saves a `charged` receipt (usage, price, request ID and charge time)
as soon as OpenAI returns. A failed save, failed accounting write or
crash can then still settle the spend. A retry or recovery records the
cost once and never buys again.
- Records one `cost_events` row through `createCostEventInTransaction`.
The row is `metered_api` with `costStatus: "estimated"`, and the budget
is evaluated. The row is recorded even if saving fails.
- Checks the task's project budget, as well as the company and agent
budgets, before spending.
- Writes content-free `runner.image_generate` activity entries: started,
completed, failed and unconfirmed.
- **`ai-connections.ts`:** adds `selectImageGeneration`. It checks again
that the connection is an active, healthy OpenAI `api_key` connection
with no custom routing. It also checks the responsible user's
membership, the grant audience and the agent or company installation.
These are the same rules that runtime selection uses.
- **`native-session-executor.ts`:** gets the binding from the run's
admitted `contextSnapshot.aiConnection`. The value comes only from the
server, never from tool input.
- **`native-session-resume.ts`:** the native tool contract changes from
`v18` to `v19` and now lists `generate_image`. A persisted provider
thread cannot add tools when it resumes, so older threads must rotate.
- **`doc/runner-api-tools.md`:** a new section about image generation
for OpenAI API-key Codex agents.
- **#19516: kept, not reverted.** Its changes have no effect on the
API-key path. They are still correct for Codex agents that sign in with
ChatGPT, because `image_gen` works there and saves to
`CODEX_HOME/generated_images`. Those agents need the read grant that
#19516 added. On the API-key path, the new tool replaces the built-in
tool. The tool's description tells the agent to use it instead of
`image_gen` or `scripts/image_gen.py`.
## Verification
**Red on `master` (
|
||
|
DottaandPaperclip
|
ae1f781cac |
fix(native): preserve web files without settling remote recovery (#19814)
## Thinking Path > - Paperclip manages work that AI agents perform for users. > - Native sessions retain files on the web server and state in a remote provider. > - Replacing the web server must preserve its files without changing remote recovery authority. > - The existing maintenance owner requires destroyed-provider receipts even when all web writers have settled. > - This can keep the old web volume required only because a remote recovery obligation remains unresolved. > - This pull request adds a closed web-byte evidence mode to the existing preservation owner. > - The benefit is durable web-file coverage with unchanged remote references, failures and continuation policy. ## Linked Issues or Issue Description Refs #19748, #19806. A search for native harness filesystem evidence and open web/harness changes found no duplicate implementation. **What happened?** A settled successful native source can retain a raw controller, a separate warm supervisor and provider-home files while its historical external lease has no termination receipt. The existing filesystem-independence owner refuses before it can preserve these web files. One retained skill file can also remain physically present after its version row was deleted. **Expected behavior** Preserve every nonexcluded web file as authenticated evidence. Keep missing files absent and deleted skill versions deleted. Leave VM references, recovery actions, failures and execution authority unchanged. Validate the published web-byte receipt on an empty replacement using database bindings only. **Steps to reproduce** Use the included successful-source fixture with a settled web owner, an unresolved external lease, real controller/supervisor files and a retained skill whose version row is deleted. The baseline rejects at its destroyed-provider guard. The new explicit maintenance mode preserves the files and validates on a replacement. A separate fixture uses a prior successful source, a failed retained last run, a genuinely absent controller, a warm supervisor and 267 durable checkpoint parts. **Paperclip version/commit** Baseline `384fa2b7be`; final branch rebased onto `585c885168`. Current head: `990210d7e9`. **Deployment mode** Original-volume maintenance followed by an ephemeral replacement. No hosted operation or provider request is part of this PR. ## What Changed - Add explicit `web_evidence` maintenance mode and a separate `paperclip.native-harness-web-evidence.v1` receipt. Keep the existing terminated-provider mode and old manifest shapes. - Archive the closed canonical session-root inventory, the separate warm supervisor and exact admitted skill manifests through the existing encrypted `original_volume_source_evidence` owner. Preserve bytes and modes. Keep canonical credential exclusions. - Preserve local skill edits and deleted-version files as forensic evidence. Create no skill version, executable checkpoint, provider backup or recovery authority. - Require original-volume capability, settled web writers, physical/finalizer locks, exact task/source/cohort/input bindings and final byte/mode/membership comparisons. Keep genuinely absent raw controllers absent. - Bind allocation and stamp identity while allowing later external status and cleanup observations. Validate replacements from durable database and archive bindings without the original anchor, filesystem or storage reads. ## Verification - Node 24.22.2, PostgreSQL 18.1, macOS arm64. The unchanged baseline fails at the real destroyed-provider guard. The final revision passes all 85 owner, original-volume capability and unchanged continuation-policy cases in 21.14 seconds. - Tests perform real encryption, persistence through the S3 storage interface backed by the owned local fixture, authenticated restore and exact byte/mode comparisons. The 281-byte file remains present while its version row remains absent. The evidence archive stays separate from the provider-session backup scope. - The supervisor-only case preserves 267 database checkpoint parts, keeps the raw controller absent and validates without original-volume identity. Existing strict-mode and old receipt controls pass. - Negative controls cover active web writers, expired but owned finalizers, physical owners, foreign controller/supervisor identities, missing manifests, symlinks, changed bytes/modes/membership/source during upload, changed durable bindings, incomplete receipt coverage and invalid late publication. Concurrent attempts retain one valid winner and one audit event. - Server TypeScript, module boundaries, diff checks and private-data scans pass. The verification planner's first invocation used a wrong script path; its canonical invocation refused a plan because fixture inputs changed during selection. Neither attempt counts as a pass. Required CI will provide broader application coverage. - Earlier fixture and treatment failures remain recorded. They exposed a missing helper, JSONB property-order instability, missing separate-supervisor mode coverage and fixture timing/source/count errors. The final same owner suite passes without weakened assertions. - A canonical cleanup audit found two additional observation fields: the cleanup controller and manual-attempt ID. Their real publication updates reproduced false receipt invalidation on the first PR head. The narrow correction keeps WEB coverage valid through those observations while changed allocation identity still fails. The exact same 85-case suite passes afterward. The final `990210d7e9` head has fresh Greptile 5/5 with no comments or unresolved threads. Required CI run [42370607882](https://github.com/paperclipai/paperclip/actions/runs/42370607882) passes: 55 successful checks and four intended skips, plus successful Snyk. The old head's CI was superseded by this correction and is not counted as a pass. From `server/`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts src/services/native-runtime/native-harness-original-volume.test.ts src/services/native-runtime/native-harness-filesystem-independence-continuation.integration.test.ts --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks This changes a maintenance trust boundary. It proves web-byte preservation, not provider recovery or termination. External recovery obligations and existing resume failures remain. Unknown layouts, missing manifests, active local owners and changed authority fail closed. Invalid receipts are not automatically replaced. The inventory has independent limits of 128 entries, 16 MiB per file and 64 MiB total. Larger valid data stays on the original volume and blocks this mode. Canonical credential exclusions remain excluded. Evidence uses the existing encrypted ledger and retention roots; it does not become an executable backup. Old receipts remain byte-compatible. Startup adds bounded per-retained-scope database reads. Measure the affected cohort's actual serving startup cost before enrollment. Validation adds no filesystem or object read, worker or recurring poll. Live source qualification and publication remain separate after review, merge and normal delivery. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and PostgreSQL/filesystem test execution. The runtime does not expose a more specific serving 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> |
||
|
DottaandPaperclip
|
d2efe3392b |
fix(native): preserve retired authority evidence (#19806)
## Thinking Path > - Paperclip manages work that AI agents perform for users. > - Native sessions retain control-plane files when execution authority moves to a later run. > - Volume migration must preserve these historical bytes before a server can stop depending on its old volume. > - The existing preservation owner archives the current controller and supervisor, but refuses a nonempty authority-epoch directory. > - A settled four-run session can therefore remain blocked after its current controller archive is complete. > - This pull request adds a closed historical inventory to the existing encrypted evidence archive. > - The benefit is preserved source evidence and database-only replacement validation without granting execution or recovery authority. ## Linked Issues or Issue Description Refs #19748. A search of pull requests for filesystem independence and issues for authority-epoch history found no duplicate change. This fixes its uncovered historical-file case. **What happened?** A settled native session retained three prior controller files in authority-epoch directories. Its three historical database checkpoint headers were absent. Maintenance preserved the current controller, then refused filesystem independence because those historical files remained uncovered. **Expected behavior** Preserve the exact historical bytes and modes through the existing encrypted evidence owner. Keep original files, prior archive rows, run outcomes, warnings, stamps, session pointers, and recovery policy unchanged. Validate the resulting receipt on a replacement without the source volume. **Steps to reproduce** Use the included four-run fixture with a current-controller-only archive, three prior authority epochs, destroyed provider receipts, released ownership, and absent historical checkpoint headers. Call the canonical filesystem-independence owner. The base rejects the nonempty directory; the treatment preserves the complete evidence and validates it after removing source-volume identity from the test process. **Paperclip version/commit** Baseline `ef83d566cacf1d5f740d701560597dad2bfa4619`; final branch rebased onto `27170dca49`. **Deployment mode** Original-volume maintenance followed by an ephemeral replacement. This PR performs no hosted migration or provider operation. ## What Changed - Add optional closed authority-epoch entries to the existing `original_volume_source_evidence` archive. Preserve nested file bytes and modes through the existing encryption, readback, ledger, audit, and retention owners. - Bind each epoch to its retained run, canonical checkpoint identity when present, or explicit header absence plus the exact admitted authority binding. Keep current-source checkpoint and artifact checks strict. Historical evidence never becomes an executable checkpoint. - Recheck the complete historical inventory at archive and receipt publication fences. Keep physical locks, all-run ownership and termination checks, source capability checks, and final CAS. - Bind the history inventory and its database facts into the authenticated archive manifest. Replacement validation uses database bindings only. Later header presence, removal, retirement, or changed identity invalidates the receipt. - Leave history-free manifest bytes and receipt shapes unchanged. Keep an existing current-controller-only archive when a later complete archive is added. Unknown children and other nonempty history directories remain blockers. ## Verification - Node 24.22.2, PostgreSQL 18.1, macOS arm64. The final four-run fixture fails on the baseline owner at its nonempty-history guard. The same final fixture and treatment pass all 51 focused PostgreSQL/filesystem cases. - The positive case performs actual encryption, object-store persistence, authenticated restoration, and exact byte/mode comparisons. It proves replacement database-only validation with absent source paths and storage reads refused. - Controls cover foreign identities, unknown children, owned prior runs, changed bytes or new files after upload, missing or newly present historical headers, malformed and retired headers, an unknown authority override, receipt-identity tampering, and a late concurrent winner. Existing current-controller evidence and history-free manifest bytes remain intact. - The baseline/treatment driver restored only its two owned source files in `finally` and verified their exact hashes. Earlier setup and treatment failures remain recorded: missing bundled PostgreSQL library links, an overly narrow raw-schema check, and an incorrect synthetic transient-workspace binding. The final baseline and treatment use the corrected same fixture; those earlier attempts are not counted as passes. - Review follow-up `21183b75f4` closes the missing-history-field gap. Deleting history from an actual published receipt reproduced an invalid true result before the fix. Unconditional manifest reconstruction now rejects it; all 46 owner cases pass. Genuine history-free manifests retain their exact bytes. - Review follow-up `f3d305dd36` adds suspended historical Runner-state bytes and mode checks through authenticated restore, active/foreign state refusals, exact 16 MiB and 64 MiB boundaries, and retention of an oversized file without publication. All 51 cases and server TypeScript pass; production behavior is unchanged. - CI run `42362784772` on the previous head had one unrelated 30-second timeout in `manual-private-cell-report-assets.test.ts`, with 6,864 support tests passing. The failure remains recorded. The prior head `21183b75f4b223cd3ec28124cdc194f24b4f79ba` passes [required CI run 42363408227](https://github.com/paperclipai/paperclip/actions/runs/42363408227), including Runner support: 55 successful checks and four intended skips, plus successful Snyk. The final test/documentation head `f3d305dd3690a7db38b57c4a77389c17206cb76b` passes [required CI run 42364958386](https://github.com/paperclipai/paperclip/actions/runs/42364958386): 55 successful checks, four intended skips, one neutral review check, and successful Snyk. Fresh Greptile is 5/5; all four review threads are resolved. No timeout or assertion was weakened. - The count warning is resolved as independent bounds: the actual descriptor accepts 32 control-plane-only epochs (97 entries) and 32 epochs with 31 Runner states (128 entries), and rejects 129 entries. The 32-epoch and 128-entry maxima both apply; the capture budget is unchanged. - Server TypeScript and module-boundary checks pass. The local verification planner selects ten broad application lanes; required CI supplies that wider coverage instead of duplicating it locally. - Reproduce the focused owner suite from `server`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks This changes a maintenance trust boundary. A source capture is limited to 128 entries, 16 MiB per file, and 64 MiB total. These limits are intentionally below the Runner's 256 MiB state maximum. Larger valid epochs remain on the original volume and continue blocking filesystem independence. It uses bounded directory reads and file descriptors, rejects aliases and unknown paths, and rechecks bytes before publication. Storage failures retain original files and ledger evidence; they do not settle recovery or authorize a retry. Startup adds bounded database reads for retained historical header bindings or their absence. It adds no source filesystem or storage reads. The selected cohort's serving startup cost still needs measurement before enrollment. Other historical layouts require their own canonical source qualification. No missing provider history is declared recovered, and no automatic corruption repair is added. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, source editing, shell tools, and PostgreSQL/filesystem test execution. The runtime does not expose a more specific serving 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> |
||
|
DottaandClaude Opus 5.5
|
1674902b4f |
fix(native): settle a crashed controller's running workspace finalize once a later attempt finishes (#19807)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run saves its workspace in a `workspace_finalize` operation after the agent gives a result > - A crash loop can kill the server after it records this operation as `running` and before it finishes > - The next server resumes finalization and records a new attempt, which succeeds > - Nothing settles the first attempt, so it stays `running` forever and looks like live work > - This pull request settles the old attempt when a later attempt for the same run finishes, under the proof that the old controller is gone > - The benefit is that no workspace operation stays `running` after its owner is dead ## Linked Issues or Issue Description Refs #19666, #19759, #19721, #19717 **What happened?** A control-plane restart chaos test sent SIGKILL to the server while a native run finalized its workspace. The server crash-looped. One boot recorded a `workspace_finalize` operation as `running` and then died. A later boot resumed finalization. Its new operation (with `resumedFromOperationId` set to the old one) succeeded, and the run succeeded. The old operation stayed `running` at rest. **Expected behavior** When a later finalization attempt for the same run finishes, and the owner of the earlier attempt is proven gone, the earlier attempt gets a terminal status with a reason. A live owner keeps its attempt. **Steps to reproduce** 1. Start a native run that finalizes a workspace. 2. Kill the server with SIGKILL after it inserts the `workspace_finalize` row, and before it updates the row. 3. Restart the server and let it finalize the run again. 4. See that the first `workspace_finalize` row stays `running`. **Paperclip version or commit** `ef83d566ca` (also on `27170dca49`). ## What Changed - Root cause: `attemptNativeWorkspaceFinalization` in `server/src/services/native-runtime/native-workspace-finalizer.ts` reads the previous attempt only to set `resumedFromOperationId`. It records the new attempt but never settles the previous `running` attempt. The early "already succeeded" return path also left it alone. Only the recorder that started a row can settle it (`workspace-operations.ts` `recordOperation`), and a killed process never does. - Add `settleSupersededNativeWorkspaceFinalizations` in `native-workspace-finalization-supersession.ts`. It sets earlier `running` native finalize attempts for the run to `failed`. It writes `metadata.supersededFinalization` (reason `superseded_workspace_finalization`, superseding operation id and status), an stderr note, a terminal-workspace wake (registered before the write), and a `workspace_operation.reconciled` activity entry. - The proof is the run's workspace-finalization ownership, not age. The caller holds the advisory lock and the durable owner receipt. That claim requires the previous controller to have released it or to be proven stopped (same-host process proof, platform proof, or a completed copyback barrier). The function also skips an attempt that this process is still running, and attempts with independent obligations (`uncertain`, `saveWork`). It only touches rows that `native_workspace_finalizer` recorded and that are older than the superseding attempt. - The finalizer calls it after a new attempt reaches a terminal status, and on the "already succeeded" path. A settlement error is logged and does not change the finalization result. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-workspace-finalization-supersession.test.ts src/services/native-runtime/native-workspace-finalizer.test.ts` on embedded Postgres: 72 passed. - New tests in `native-workspace-finalization-supersession.test.ts` (a separate file, so `native-workspace-finalizer.test.ts` stays under 1,000 lines): - A `running` finalize from a dead controller plus a resumed attempt that succeeds: the old row becomes `failed` with the supersession reason, no `running` rows remain, and one activity entry exists. - The resumed attempt fails: the old row is still settled, and `supersedingStatus` is `failed`. - A `running` finalize older than an already succeeded attempt: the old row is settled on the next sweep. - While another controller holds ownership, the `running` row stays `running`. - A `running` finalize recorded by another service stays `running`. - Attempts with `saveWork` or `uncertain: true` stay `running`. - Red/green: with the settle call disabled, the three settle tests fail (row stays `running`). With the fix, all pass. - `npx tsc --noEmit -p server` passes. ## Risks - Low. The change only writes rows that are already proven abandoned, and only under the finalization ownership that already fences copyback. - A settled attempt now reads as `failed`. Readers pick the latest attempt by creation time, and the superseding attempt is always later, so the run's finalization outcome does not change. - Existing leaked rows on runs that never finalize again stay `running`. This PR does not add a backfill. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit) and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Devin FoleyandPaperclip
|
bf12b2d8ad |
fix: retry reviewed chat setup after temporary contention (#19787)
Retry reviewed chat preparation with the original accepted input. Bound retries for authorization proof waits that expire before provider work. Preserve the original answer receipt and queue later wakes separately. Keep plan confirmation semantics by assigning the interaction marker only to question answer delivery and legacy question payloads. Verification: 433 tests across four backend suites, all 17 project-chat browser tests, and all server source/test typecheck layers passed. The plan approval regression was reproduced on the previous head before the fix. All applicable CI passed and exact-head Greptile Apex scored 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
ef83d566ca |
fix(native): ask for the result once after a restart continuation ends without one (#19771)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs continue a turn that a control-plane restart lost, in the same conversation (#19501, #19699) > - In restart storms 5, 6b and 8, ACPX Claude runs crossed restarts, ran the continuation, and then failed with `native_session_retry_exhausted` > - The continuation completed, but Claude ended it with "I'll wait for the notification when it completes" for a background command, and did not call `paperclip_finish` > - The next same-run attempt then sent no turn at all and failed with "session returned no semantic result" > - This pull request makes the continuation ask for the result, and asks once more in the same attempt if the continuation still ends without one > - The benefit is that a restart-interrupted task settles without a Retry, and the restarts that the run survived cannot block the request ## Linked Issues or Issue Description Refs #19501, #19699, #19720 **What happened?** Storm 8, run 84c532f7 and its Retry 0faead66 (ACPX Claude, task CHA-5). The steps are the same in both runs: 1. `resume_dead_runner`, then `session.resumed`, then `turn.failed` with `provider_turn_lost_on_restore` (recoverable). 2. The restart continuation runs. Claude starts the bench in the background and ends the turn with "I'll wait for the notification when it completes". The turn completes with no `paperclip_finish`. 3. Lifecycle: "provider turn completed without a semantic result; same-run disposition recovery persisted" (attempt 2). 4. 30 s later, attempt 3 restores the session. It sends no turn. After about 2 s it fails with `native_finalization_missing: session returned no semantic result`, and the attempt count makes it `native_session_retry_exhausted`. **Expected behavior** The continuation tells the agent to finish and report its result. If the continuation still completes without a result, Paperclip asks for the disposition one time. Restarts that the run already survived do not block that request. An agent that never reports a result still fails closed. **Root cause** - `packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts` `startTurn` (lines 224 and 248): the restart continuation uses the driver's one-shot disposition marker (`dispositionOnlyRecoveryAvailable`). #19699 uses the marker on purpose, so that a crash can adopt the accepted continuation turn. The marker then stays consumed with the continuation turn id. - `packages/paperclip-runner/src/drivers/codex/codex-session-terminal.ts` (line 226): only a *lost* continuation gives the marker back. A *completed* continuation with no result does not. - `packages/paperclip-runner/src/native-session-runtime.ts` (`dispositionRecoveryWasSubmitted`, about line 2761): on the next attempt, the consumed marker looks like a disposition request that already ran. Recovery replays the continuation's terminal and starts no turn. So the disposition request never happens. - `server/src/services/native-runtime/native-session-executor.ts` (lines 9755-9761 and `nativeSessionFailureDisposition` at 7768): the `semantic_result_missing` same-run retry charges a provider attempt on the same counter as restart resumes. In this run, attempt 3 was the last allowed attempt. - The continuation prompt (`restartContinuation`) did not tell the agent to report a result, or not to stop and wait for a background notification. ## What Changed - Codex-protocol driver (used by Codex and by ACPX through runnerd): when a restart continuation completes without a semantic result, the driver gives back the one-shot disposition marker. The disposition request is not a continuation, so it consumes the marker for good. - Native session runtime: after that, the same attempt sends one disposition request (`Paperclip semantic-result recovery after a restart continuation ... invoke exactly one paperclip_finish or paperclip_block`). It does this only when the driver shows that the continuation borrowed the marker and gave it back. Other drivers keep the current behavior. - Restart continuation prompt: it now says to finish in this turn, not to end the turn to wait for a background command or a notification, and to report one `paperclip_finish` or `paperclip_block`. - `doc/execution-semantics.md`: one additive paragraph. - No server change. The disposition request runs in the same attempt, so it uses no provider attempt. This avoids `native-restart-recovery.ts` (#19747). ## Verification Red before the fix, green after: - `codex-app-server-driver.recovery.test.ts` "keeps one disposition request after a restart continuation completes without a result". Red: marker stays consumed on `turn-2`. Green: the marker is given back, one disposition turn starts, and a resultless disposition turn does not re-arm. - `native-session-runtime.test.ts` "settles a restart continuation that ended without a result through one disposition turn (Codex driver)". Real Codex-protocol driver with a fake transport. Red: only one `turn/start`, then `native_finalization_missing`. Green: the disposition turn `turn-3` reports the result and the run completes. - `native-session-runtime.test.ts` "asks once for the disposition when a restart continuation ends without a result" and "fails closed when the driver keeps the continuation's disposition marker". Also run: - `npx vitest run src/native-session-runtime.test.ts src/drivers/codex/` in `packages/paperclip-runner`: 593 passed. - `src/live/runnerd-restart-continuation.test.ts` (real runnerd and fake Codex, killed runner): 5 passed. - `tsc --noEmit` for `packages/paperclip-runner`. - `pnpm verify` exits 2 because the doc change is a global CI input. Required CI supplies the broad coverage. Not run: a live replay of storm 8 t05 with Claude. The chaos harness needs provider keys from the operator's shell. ## Risks - Low to medium. A run whose continuation ends without a result now gets one more provider turn in the same attempt. The bound is one disposition request per continuation. Each continuation still needs an admitted restart, and the provider-attempt budget bounds those. - If the controller crashes after the continuation completes but before its checkpoint, the checkpoint keeps the stricter consumed marker. That case fails closed as before. - The prompt text changes for restart continuations. It does not change a protocol schema or a profile digest. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, code execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
6adc8b1049 |
fix(recovery): keep restarted Retry attempts as task directories and settle failed re-preparation (#19759)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Task recovery lets a user click Retry after a run fails, and a native run restarts its own preparation after a control-plane restart > - A Retry for a task without a project runs in a fresh task directory under `isolated-workspace-attempts` > - When a restart hit that Retry during bootstrap, preparing it again treated the task directory as the base of a new git worktree and failed > - The failed run then kept its native recovery claim open, so the task showed Retry but every Retry click created no run > - This pull request keeps the restarted attempt in its task directory and settles a failed re-preparation > - The benefit is that a Retry finishes after a restart, and a failed one can be retried again ## Linked Issues or Issue Description Refs #19666 **What happened?** A task without a project failed after a restart (`native_session_interrupted`). The user clicked Retry. The Retry run started in a fresh task directory. A restart hit it during bootstrap. Restart recovery prepared the same run again and failed with `workspace_validation_failed`: "requested isolated_workspace with git_worktree, but base workspace `.../isolated-workspace-attempts/<company>/<issue>/<run>` is not a git checkout". The task rested blocked with Retry shown. A later Retry click created no run: the receipt stayed deferred with "New work must wait while a native run recovers its unfinished accounting." A second storm wave showed the same failure on a Claude C1 task. **Expected behavior** Preparing a run again after a restart reuses the task directory that run already bound. A git worktree base is only the project's checkout or the task's own worktree. A run whose re-preparation fails leaves a working Retry. **Paperclip version or commit** `master` at `51eb4ff705` and `131b566ca4` (both include #19666). Restart-chaos waves on 2026-11-07. **Steps to reproduce** 1. Use a task with no project and isolated workspaces enabled. Let a native run fail after a restart. 2. Click Retry. The run starts in `isolated-workspace-attempts/<company>/<issue>/<run>`. 3. Restart the control plane while the run bootstraps. 4. The re-preparation fails with `git_worktree_base_not_git_checkout`. Click Retry again: no run starts. Root cause: - `server/src/services/fresh-workspace-continuation.ts` `readFreshWorkspaceReadiness` returns `null` for a native run that already has admitted input and a bound workspace (from #19693). This is correct: the restart must resume the admitted workspace. - `server/src/services/heartbeat/environment-selection.ts` (`boundTaskDirectory`) excluded every native recovery binding from the task-directory path. With readiness no longer fresh, `useIsolatedTaskDirectory` was false. The default `isolated_workspace` strategy (`git_worktree`) applied, with the task directory as its base. - The restart recovery claim set the native coordinator to `observed` with its lease owner. `failRunSetup` failed the run, but nothing released that claim. `budgets.ts` reads an open coordinator as unfinished accounting, and Retry admission reads it as `controller_settling`. ## What Changed - Environment selection treats a native recovery binding as the task directory when it is the task's current binding and a bound isolated task directory. The directory strategy is kept; it never becomes a git worktree base. A recovery binding the issue no longer points to keeps its existing path. - New `settleFailedNativeRecoveryPreparation`. When a `bootstrap_incomplete` or `resume_dead_runner` restart claim fails its re-preparation before any provider dispatch, it settles the claimed coordinator as `terminal_failure` and wakes deferred work. It only changes a coordinator that is still `observed`, owned by that claim, without a result, for a failed run. Reattach claims (live runner) are not changed. - `doc/task-recovery.md` describes both rules. ## Verification - `environment-selection.test.ts`, RC-58 cases: a restarted fresh attempt bound to the task re-prepares as its task directory (`useIsolatedTaskDirectory` true). It failed before the fix and passes now. A binding the issue moved away from keeps the old behavior. - `native-runtime/native-recovery-preparation-failure.test.ts` (embedded Postgres): before settlement, Retry admission reports `controller_settling` (the dead Retry). After settlement the coordinator is `terminal_failure` with the run's error code, and admission no longer waits on the controller. Settlement is idempotent. Another owner, a reattach claim, a run that is not failed, and a coordinator finalizing elsewhere are left unchanged. - Local regression: `environment-selection`, `workspace-preparation`, `retry-workspace-continuity`, `native-recovery-preparation-failure`, `explicit-native-continuation` (588 tests passed). - Restart-chaos: local restart-chaos runs on `3a7abef4e5` passed, but neither one reproduced the exact failure (a restart during the Retry's bootstrap). The race depends on timing, so the unit tests above are the red/green proof for both defects. - `--replay w-20261107T011010-s7403/t02` (codex C1 K4 with the original fault sequence): PASS, R1-R6 and T. The replay's Retry did not cross a bootstrap restart. - Wave `w-20261107T064432-s7210` (codex+claude, C1+B1, K1/K4/K6, `--triggers retry`, concurrency 2): 8/8 PASS. It had 4 Retries, including C1 tasks with a fault fired right after the Retry click. No `git_worktree_base_not_git_checkout` and no dead Retry. The server log has no "incomplete native runner bootstrap" restart in this wave either. - `ce7e3f9b19` registers execution work before the settlement writes (Greptile finding). ## Risks - A native recovery now re-prepares a bound task directory through the task-directory path. That path still validates the binding, owner, status and directory identity before use. - Settling a failed re-preparation ends that run's recovery claim. It applies only when the previous runner is verified gone (`bootstrap_incomplete`, `resume_dead_runner`) and no provider dispatch started in this attempt. A Retry still passes its normal stop, ownership and admission checks. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
909f4d1a33 |
fix(native): recover exact historical provider pack code (#19779)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native sessions retain the provider pack that admitted their saved
state.
> - An image update can replace that pack's executable, proxy, or
contract.
> - Older sessions can predate the durable three-file launch artifact
receipt.
> - The current image's archive cannot satisfy a different pinned
manifest.
> - This pull request obtains the exact public code from its recorded
release image.
> - Full source, archive, and installed-file checks still control
provider admission.
## Linked Issues or Issue Description
Refs #19558, #19622, #19755.
A saved OpenCode session can fail with
`runner_remote_provider_archive_invalid` after an image update. Its
original provider state and checkpoint remain valid, but its pinned pack
differs from the controller's pack. The old manifest can name a release
that still has the exact code in GHCR. Matching the OpenCode version
does not prove matching proxy or contract bytes.
## What Changed
- Resolve a pinned historical pack from the recorded source-SHA image in
the public Paperclip GHCR repository. Verify the platform manifest, raw
config, source identity, dedicated code layer, selected pack manifest,
archive contents, paths, and hashes.
- Keep a private temporary cache of verified public code. Rebuild it on
an empty host. Bound each source to 4 GiB and three minutes, with two
active imports or cache verifications, 32 distinct queued sources, and
disk-headroom checks. Share warm-cache verification by manifest while
each caller keeps its own cancellation.
- Select this verified archive through the existing uploader. Preserve
installed-pack and preinstalled-pack checks. Refuse unavailable
historical transport before clearing the target. Keep the current-source
identity guard from #19755. Refuse an oversized layer argument before
any target effect.
- Attach the source wait to the actual per-turn startup. Stop, lease
loss, and startup completion cancel it. Keep existing Runner admission
and turn limits.
- Document the registry retention requirement and the distinction
between public code recovery and encrypted provider-state backup.
## Verification
- Genuine baseline RED: the real uploader rejects the selected old pack
before any remote effect. Treatment installs the exact old code through
that uploader.
- Current head: all 19 source/integrity/cache/deadline controls, all 45
archive controls, and 29 actual-caller and manifest/startup controls
pass (93 total). Four review regressions fail against the prior head and
pass with the fixes. The server and colocated test TypeScript graph
passes. Module-boundary and whitespace checks pass.
- Actual public GHCR cold reconstruction: 1,054,587,008 compressed code
bytes, 115.4 s, about 248 MiB maximum owner RSS. All 114 file layers and
the overlay passed source validation.
- Linux Node 24.22.2 fixture: the real historical pack passes the normal
uploader, full installed-file verifier, and pinned OpenCode version
check. It used one transfer, no application startup, no network, and no
provider calls. Revision12b0d2ba passed in 220.2 s with about 282 MiB
owner RSS. The earlier prototype passed in 108.5 s; both measurements
are retained. The three-minute source limit and the fixture's
four-minute limit were unchanged. The later review fixes add shared
cache verification, hash cancellation, and an early size guard. They do
not change extraction or the accepted Linux fixture input.
- Earlier fixture failures are retained: unsafe hardlink classification
was corrected with confined-link controls; a cached image lacked the
required zstd CLI; Docker Desktop bind storage caused GNU tar
directory-identity errors. The passing fixture uses the official Debian
zstd binary and an owned Linux tmpfs target. Production extraction code
is unchanged.
- Fresh Greptile review is 5/5 on
|
||
|
DottaandPaperclip
|
ae2d2956b3 |
Reject relative local Runner paths before native admission (#19776)
## Thinking Path > - Paperclip manages AI agents and their work. > - Heartbeat selects the runtime that will execute a task. > - A new local Native run uses the controller's Runner binary setting. > - A relative binary path is invalid. The resolver already rejects it, but that check occurs after Native admission. > - This pull request checks the same setting before it creates Native execution authority. > - Remote providers and admitted Native recovery keep their existing behavior. ## Linked Issues or Issue Description **What happened?** A relative `PAPERCLIP_RUNNER_BINARY` setting can reach local Native admission before the binary resolver rejects it. The run has transferred preparation ownership even though this setting cannot start the local Runner. **Expected behavior** Reject this invalid setting before creating a completion contract or Native coordinator. Keep the existing preparation owner and the run row unchanged. **Steps to reproduce** Set `PAPERCLIP_RUNNER_BINARY=relative-runner`. Select a new local `paperclip_runner` Native run. The selection must report the existing absolute-path error before Native admission. Refs: #19466 and #19340. Those merged changes repair Retry authority and saved-workspace recovery. This change only moves local path validation before new admission. ## What Changed - Add a pure absolute-path check beside the existing binary resolver. Both use the actual controller environment setting. - Run that check before a new local Runner admission. Skip remote execution, API-only providers, and an already admitted Native run. - Test the rejection, unchanged preparation ownership, controller setting selection, remote behavior, and immutable recovery. Keep the existing resolver checks. ## Verification - The full binary test file passed: 32 tests. - The full runtime-selection file passed against a dedicated PostgreSQL database: 62 tests. It exercises real admission transactions, contracts, coordinator rows, and instruction-copy controls. It replaces context materialization and remote workspace synchronization at their existing test boundaries. - After normal integration with current master, the normal Shared/SDK, database, eval-kernel and Runner builds passed, together with all seven normal application TypeScript programs. The original recovery-preparation case passed again on this integrated revision against a fresh dedicated database (1 passed; 61 filtered). The earlier full 32-test binary and 62-test selection results carry only where their source and executed boundaries are unchanged. - The first private selection wrapper failed during module resolution, before any case or database use. A TypeScript wrapper with the same body recovered that setup failure. This was not a product regression. - Reproduce with `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-codex-runner.test.ts --maxWorkers=1 --no-file-parallelism`. - Set `PAPERCLIP_RUNTIME_SELECTION_TEST_DATABASE_URL` to a dedicated disposable test database before running `pnpm --filter @paperclipai/server exec vitest run src/services/heartbeat/runtime-selection.test.ts --maxWorkers=1 --no-file-parallelism`. ## Risks - This check validates path syntax only. An absolute path can still fail the existing file-access or artifact checks later. - A wrong branch condition could reject remote or existing Native work. The tests cover these exclusions. - This does not prove that a real provider can start or that a deployed service is healthy. It changes no database schema or retry authority. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with source inspection and code-execution tools. The host does not expose the context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
2d4f80504e |
fix(runner): retain exact ACPX checkpoints after rejected turn starts (#19762)
Preserve the exact idle ACPX conversation when a matching turn.start response rejects work with retryable SESSION_RESUME_REQUIRED. Require verified identity, no pending work, confirmed process cleanup and the provider lifetime fence before normal drain and suspension can publish a reusable checkpoint. Unknown acceptance and arbitrary closed states remain closed. The real Runner regression completes the next turn on attempts 2 and 3, requires successful terminal status for the exact submitted turn, and checks expectedIdentity on both resume requests. Controlled wire faults reject failed, cancelled and missing-identity recovery. Validation: 193 affected Rust tests on unchanged Rust source/fixtures; 134 recovery integration tests, 14 environment-gated skips; 21 Claude effort tests; all server typecheck layers. Exact-head CI: 59 successful checks and statuses, 6 intentional skips. Greptile Apex 5/5, no unresolved findings. Hosted deployment and task acceptance remain separate. Fixes: #19751 Refs: #19366 Refs: #19745 Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Nicky LeachandPaperclip
|
beb3e55072 |
Continue a released Runner after confirmed graceful eviction (#19760)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Core can release a completed Runner while its host retains the session. > - A host can later retire that Runner under its retention policy. > - Graceful retirement saves a suspended session before physical shutdown. > - A cached Core owner still expects the old physical task and rejects a new allocation. > - This pull request retires that dead local controller after verified stop. > - The benefit is ordinary session continuation for Codex and qualified Claude. ## Linked Issues or Issue Description Refs #19611, #19683, #19196, #19733 and #19157. After a host gracefully evicts a released task, the next run can receive a new allocation. A cached controller rejects it with `native_session_released_owner_scope_mismatch`. The detached-controller path can already recover the suspended session. ## What Changed - Require the exact source, old task and current account/workspace scope before releasing a cached controller. - Use the existing physical-stop receipt checks. Detach only the local controller, then use the same suspended-state admission as the detached path. - Preserve the current run's instruction reservation. Retain a failed detach for cleanup. - Add actual Core, PostgreSQL and native Runner tests for Codex and qualified Claude through ACPX. Route them once through the native CI lane. ## Verification - The original real-entry Codex case failed at the cached-task scope check after native retirement, confirmed process exit and a recorded stop receipt. - The current head's native CI lane passed all 16 eviction cases for Codex and qualified Claude. These check cached and detached owners, current instruction preparation, reused-allocation inspection, missing stop/state, foreign identity and non-suspended state. They verify the same provider session, two ordinary turns, one accepted source result and no replacement submit on refusal. - Local checks passed the composition matrix, 12 targeted executor controls, 43 CI lane-selection checks and server TypeScript. All eight refusal cases now assert the expected error and whether the checkpoint request or host read was reached. This addresses the first standard review's test finding. - Commands: `pnpm exec vitest run src/services/native-runtime/plugin-runner-eviction.integration.test.ts`, focused `native-session-executor.test.ts` controls, `node ../node_modules/typescript/bin/tsc --noEmit -p tsconfig.json`, and `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`. - [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42349579107) passed on head `9a25709ef43365b1f1849413b935785838eca19b`. [Standard review](https://github.com/paperclipai/paperclip/pull/19760#issuecomment-6703629255) is 5/5 on this head, with no unresolved findings. Whitespace and added-line secret/PII checks passed. ## Risks - Controlled provider processes and an authenticated local host fixture model the Launcher edge. This does not qualify live vendor transcript storage, mounts or a deployment. - This covers graceful eviction before allocation or before stopped-task inspection during `describe`. Loss after that inspection or an ambiguous claim remains held for reconciliation. No unavailable reply proves physical stop. - Missing or invalid suspended state still refuses continuation. The deterministic refusal rule from #19733 remains unchanged. Abrupt death without a suspended checkpoint stays with the separate #19157 work. - No new capability, mode, schema, dependency, deployment or activation is added. The existing draft #19157 and dependent branches remain unchanged. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), high reasoning, through Codex with repository tools and code execution. The runtime does not expose a context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
3895195f4b |
fix(native): report a retained provider pack the controller does not ship as incompatible (#19755)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote OpenCode runs stage a controller-owned provider pack into the sandbox before the provider starts > - Since #19558, an ordinary OpenCode continuation retains the pack that an earlier release admitted for its saved provider session > - After a controller release changes the pack, the controller no longer ships those bytes, and the continuation must stay blocked > - The configured-pack upload path did not detect that case: it read the build-time archive receipt for the old digest and failed with `runner_remote_provider_archive_invalid`, a generic, retryable error that suggests corrupt image files > - This pull request compares the configured pack's identity with the selected identity before any upload and fails as `runner_remote_provider_artifact_incompatible` > - It also puts a fixed reason in each real archive integrity error > - The benefit is a correct, permanent classification that matches the bundled-image path, no wasted retries, and a diagnosable message for real archive corruption ## Linked Issues or Issue Description Refs #19558, #19743 **What happened?** On a hosted deployment, two remote OpenCode runs of a review agent failed before the provider process started. The error was `runner_remote_provider_archive_invalid` (classified as a generic `native_session_interrupted`). Both runs continued an issue whose provider session was admitted by an earlier release. In both runs, the run log shows `provider_pack.compatibility`, `provider_pack.verify`, and `provider_pack.verify_preinstalled` fail, then `provider_pack.archive.verify` fails after 4–6 ms. Fresh runs of the same agent on the same controller passed pack preparation with the current pack. The cause is not a cache race. The archives are created at image build time (`Dockerfile` `runner-provider-pack` stage) and the server never rewrites them. The retained authority selects the old pack digest. `readProviderPackLayers` then compares the receipt's `providerPackDigest` (current release) with that old digest and throws `runner_remote_provider_archive_invalid`. Directory staging (no archive) instead clears the staged remote pack, uploads the different pack, and fails only at the final verification. **Expected behavior** A continuation pinned to a pack that the controller does not ship fails as `runner_remote_provider_artifact_incompatible`, as the bundled-image path already does ("the image provider pack did not match the bundled release identity"). It does not clear or upload anything. Real archive corruption keeps `runner_remote_provider_archive_invalid` with a fixed reason that the run report shows. **Steps to reproduce** 1. Build a provider pack and its archives for digest A. 2. Call `uploadVerifiedProviderPack` with that pack and a selected (retained) digest B. 3. Before this change: the call fails with `runner_remote_provider_archive_invalid` (v1 and v2 archives), or resolves after it clears and re-uploads the remote pack (directory source). ## What Changed - `uploadVerifiedProviderPack` takes the configured pack's `sourceDigest`. If it is not the selected `expectedDigest`, the call fails with `runner_remote_provider_artifact_incompatible: selected provider pack <12 hex> is not the controller's pack <12 hex>; ...`. The check runs before the remote clear, the archive read, the directory hash and any upload. - The executor passes the configured pack's identity (`readRemoteProviderPackIdentity(root, false)`, the same reader that selects the current pack). - Archive integrity errors keep the `runner_remote_provider_archive_invalid` prefix and add a fixed reason: `receipt_unreadable`, `receipt_identity_mismatch`, `receipt_invalid`, `layer_invalid`, `layers_unreadable`, `archive_unreadable`, `archive_not_file`, `size_mismatch` or `sha256_mismatch`. The run report keeps only the outer message, so the reason must be in the message. Filesystem causes stay on `cause` and do not put private paths in the message. - The v1 reader now uses the shared `verifyArchive` helper. - `doc/execution-semantics.md` documents the rule. ## Verification - Red before the fix (production files unchanged, new tests only): `npx vitest run src/services/native-runtime/provider-pack-archive.test.ts` gave 5 failed / 38 passed. The v1 and v2 cases failed with exactly `runner_remote_provider_archive_invalid`, which is the hosted error. The directory case resolved after it cleared and uploaded the remote pack. The reason tests got a bare `runner_remote_provider_archive_invalid`. - Green after the fix: the same file gave 43 passed. - `npx vitest run src/services/native-runtime/native-session-executor.test.ts`: 1245 passed. This includes the #19558 continuation cases. - `remote-provider-pack`, `remote-provider-pack-authority` and `remote-provider-pack-verification` tests: 69 passed. - `pnpm --filter @paperclipai/server typecheck`: all layers passed. - Review follow-up (`c8156841a9`): an unreadable numbered layer now reports `archive_unreadable`. The layered-corruption test deletes `0.tar.zst` and checks for that reason before any transfer. The archive file gave 43 passed. - Hosted evidence came from read-only run, event and issue-run reads only. No hosted retries, recovery or deploys. ## Risks - This change does not unblock an affected session. It makes the failure honest and permanent: `runner_remote_provider_artifact_incompatible` is already a permanent, pre-session failure code, so the run is not retried with the same result. To recover, the session needs its original artifacts (the captured provider-artifact path, which #19743 repairs for new admissions) or an authorized continuity break. - Low risk for other paths. Fresh launches select the configured pack's own identity, so the digests are equal. Restored provider artifacts and compatible installed packs return before this upload path. - Callers that match `runner_remote_provider_archive_invalid` with a substring still match. No code compares the exact message. - Not tested end to end against a hosted sandbox. The executor-level wiring of `sourceDigest` has typecheck coverage but no dedicated executor test. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Agentic tool use with shell, code editing, test execution and read-only authenticated browser diagnostics. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
167e39c4ef | fix(runner): keep warm run.attach events on ordinary accounting (#19707) | ||
|
DottaandClaude Opus 5.5
|
6d112c74cf |
fix(native): do not charge a restart resume that never started the provider (#19747)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs have an execution recovery budget of three provider attempts, so a run that keeps failing stops retrying > - #19720 stopped charging a crash restart when the previous restart resume made progress > - In a crash loop (several kills within seconds), the controller can die again before the resumed execution starts its runner > - The next claim then charged that resume, although it used no provider work > - This pull request does not charge a resume that never started; the next claim replaces it > - The benefit is that fast restart bursts do not exhaust a healthy run's budget, and the generation cap still bounds real loops ## Linked Issues or Issue Description Refs #19720, #19625 **What happened?** In a restart-chaos storm wave, 8 runs still reached `execution_recovery_budget_exhausted`, mostly in K6 crash loops (three kills in about 20 seconds). Example: a Codex run was claimed at generation 9, attempt 3. The controller was killed 6 seconds later, before the executor recorded `native.process_start_requested`. The next claim found no progress after generation 9's position, charged, and blocked the run at attempt 3. The recorded history of the blocked runs shows that almost every charge after the first restart was for a resume with no start evidence (no runner request and no runner event) before the next claim. Two runs were blocked at generation 12, the existing cap. **Expected behavior** A resume that never started the provider consumed nothing and must not be charged. A resume that started and then made no progress must still be charged. The generation-12 cap stays. **Steps to reproduce** Start a native run. Kill the server during the run. Let a restart resume do work. Then kill the server repeatedly within seconds, before each new controller starts the runner. After one or two such bursts the run is blocked with `execution_recovery_budget_exhausted`. ## What Changed - `native-restart-recovery.ts`: `restartResumeKeepsAttempt()` replaces `previousResumeMadeProgress()`. It reads the previous claim's recorded `progressWatermarkSeq` and `progressAttempt` (`lastRecoveryClaimResume()`). - New `nativeExecutionStartedSince()`: after the watermark, it looks for `native.process_start_requested`, `native.process_identity_recorded`, `adapter.invoke`, `tool.execution.started`, or any event with a runner `source_event_id`. This is the same start evidence as `hasNativeExecutionStartEvidence()`, limited to events after the watermark. - When the previous resume never started, the claim keeps the attempt if the run is already at the attempt that resume was to run as. If the executor never applied that charge (killed before its execution lease), the claim charges once, so the run reaches that attempt and never goes above it. - When the previous resume started, the #19720 progress rule applies unchanged. - The generation-12 cap applies to both rules. - `doc/execution-semantics.md` states the rule. ## Verification New tests in `server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts` (embedded Postgres, real `claimNativeRestartRecoveries`): - `does not charge for a resume killed before it started the provider (after the executor took its lease)`: one progress resume, then four crash claims with no start evidence. The attempt stays 2. A resume that starts and fails then charges to 3. Red on master (charged to 3, then blocked). - `... (before the executor took its lease)`: the same with the lease never taken. Green on master and with the fix, as a guard. - `charges the owed attempt once when an unstarted first resume is replaced`: the replacement claim charges once (attempt 2). - `bounds a crash loop that never starts the provider`: blocks after the generation cap. Red on master (blocked at the third claim). - The #19720 tests now record `native.process_start_requested` in their resumes, as a real resume does. - `native-runner-restart-recovery.integration.test.ts`, `heartbeat-native-restart-suspension.test.ts`, `native-restart-recovery-isolation.test.ts`, `native-remote-retry-admission.test.ts`: 231 passed, 14 skipped. - `tsc --noEmit` for `server` has no errors in the changed files. ## Risks - A crash that happens before the runner start, every time, gets free resumes until generation 12. Then the budget applies. This is the same bound as the progress rule. - Start evidence is the executor's durable `native.process_start_requested` record, written before any backend may spawn a runner. If an older path launched a provider without it, that resume would be treated as unstarted until the generation cap. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
f673bd3cb5 |
fix(native): verify legacy harness filesystem independence (#19748)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native sessions retain provider checkpoints and recovery state across turns. > - Server replacement must preserve every remaining local data dependency. > - A legacy backup stamp can outlive a destroyed provider and an absent local backup. > - Surviving skill files can already have exact immutable package copies. > - Raw controller files also contain stream events that database checkpoints omit. > - This pull request records verified filesystem independence under the original-volume maintenance owner. > - Replacement startup checks durable bindings while existing resume failures and policies remain intact. ## Linked Issues or Issue Description Refs #19715, #19729, #19733. **What happened?** Startup treats every retained legacy backup stamp without a current backup row as a local filesystem dependency. This also blocks sessions whose provider was already destroyed and whose remaining local skill bytes have exact durable copies. **Expected behavior** An authorized maintenance owner can prove that an exact retained source has no uncovered local harness payload. Startup can use that proof after replacement. The proof must preserve failed recovery state and must not invent a provider backup. **Steps to reproduce** 1. Retain a terminal native session, its checkpoint, and its legacy backup stamp. 2. Record exact canonical destroyed-provider receipts. Keep only admitted immutable skill files on the original host. 3. Publish the owner-verified independence receipt. The old startup predicate still blocks; the updated predicate validates its durable bindings. ## What Changed - Add an internal original-volume capability. It requires a private nonce anchor, independently reviewed attachment evidence, current reserved runtime identity, and a witness that is at most 120 seconds old. - Verify retained and selected-source inputs, checkpoints, leases, destroyed-provider receipts, released ownership, and exact immutable skill references. - Inspect canonical persistence roots and compare surviving skill bytes and modes. Refuse any provider directory present outside the current persistence profile, including an old Codex home after a preconfigured-home switch. Unknown data, backups, history, owners, or source changes still block publication. - Preserve raw controller and supervisor bytes through the existing encrypted evidence archive owner. Match recovery records to the database checkpoint, authenticate the exact archive row, and compare the original bytes again before publication. The archive grants no executable authority. - Publish one audited receipt with source locks and a final comparison. Keep original files, stamps, warnings, statuses, and session pointers. A late capture retains a valid published winner and refuses an invalid receipt. - Validate published receipts from current database bindings on replacement hosts. Add no startup filesystem probe, object read, provider call, or recurring worker. ## Verification - Genuine baseline regression: restore the original startup predicate while keeping the actual receipt owner. The positive PostgreSQL/filesystem case fails with expected `false`, actual `true` after valid publication. Restore the changed predicate; the case passes. - Final rebased head `e78f4b4424`: 42 affected PostgreSQL/filesystem cases pass. This includes 32 owner guards, three paired continuation cases, six existing failed-source archive controls, and one existing lease-verified backup migration control. The other 113 tests in the selected existing files were explicitly unselected. - Genuine controller baseline: the unchanged prior owner publishes no raw archive and accepts missing checkpoint parts. Both new regressions fail. The repair passes encrypted byte-for-byte controller/supervisor restore, missing checkpoint, foreign source, active source, and changed-after-upload controls. - Genuine omitted-provider baseline: the unchanged owner accepts old Codex, OpenCode, and ACPX history after a preconfigured-home switch. All three refusal cases fail. The fix passes those cases and permits preconfigured execution with no omitted directory. - Genuine late-publication baseline: both concurrent winner tests fail before the repair. The repair keeps the existing valid receipt, refuses an invalid one, and keeps one audit event. A delayed raw upload also retains the first authenticated archive and its receipt. - Capability controls: 12 passed, including empty replacement refusal, stale or future evidence, changed deployment, missing volume, path aliases, changed anchor, and current remount identity. - The actual server-to-Runner boundary preserves explicit recovery policy on original and empty homes. The separate actual CREATED-sandbox preparation callback preserves checkpoint identity and policy without a backup. Provider I/O and final transport construction are controlled. - Warm sessions retain the same canonical refusal on both homes, make no backend/provider attempt, and retain the saved checkpoints. - Server TypeScript, module boundary checks, diff checks, and a private-data scan pass. The Runner TypeScript build was refreshed after rebase to include two new upstream exports. - Fresh Greptile review is 5/5 with no unresolved threads on `e78f4b44240b0fc33c2fd7cae2d5b4a2578e3153`; check `128330103712` completed successfully. Required CI remains pending. No live preservation or enrollment action ran. Focused command from `server/`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts src/services/native-runtime/native-harness-filesystem-independence-continuation.integration.test.ts src/services/native-runtime/native-controller-migration.integration.test.ts src/services/native-runtime/native-harness-backup-storage.integration.test.ts --testNamePattern 'owner-verified|legacy harness continuation|preserves a failed source online|migrates a lease-verified' --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks - This proves that no uncovered harness payload remains local. It does not recover provider history or certify that no other copy exists. Existing governed-wait and warm-session failures remain. - Unknown providers, retained owners, extra payloads, missing references, or changed source bindings fail closed. Invalid receipts require investigation; this change does not repair or replace them automatically. - The maintenance capability requires independently reviewed Railway attachment evidence. The original anchor remains stable across normal releases; each new staging invocation needs current deployment and mount evidence. - Startup validation adds bounded per-scope database reads. It pages candidate scopes and caps each source history at 500 rows. Measure the affected cohort's actual startup latency on the qualified serving build before enrollment. - Raw archives use the existing backup ledger, encryption, retention, and restore machinery. They preserve evidence only. They do not repair missing provider history or grant a new resume policy. - Local tests control provider and transport boundaries. Live staging remains a separate step after review, merge, normal delivery, and a fresh original-volume capability. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and local code/test execution. The exact runtime variant and context-window limit are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |