mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
3e8be15f23d4dd537bafef909c9843ea36cdbffd
3145
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandPaperclip
|
3e8be15f23 |
fix(build): keep maintenance bundle entries optional for typed callers
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
016e069cb2 |
fix(runtime): narrow missing pre-copy stamps before publication checks
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
d7c41047b2 |
fix(maintenance): preserve the app key and bound pre-copy shutdown
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
aec2f48d5f |
fix(runtime): recheck successful source state before pre-copy publication
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
93e2f348cf |
feat(runtime): pre-copy settled harness history while serving
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
a0250284e3 |
fix: preserve proposal revision turns and prove chat completion (#19105)
## Thinking Path > - Paperclip manages work done by AI agents. > - A completed delegated task must report its result to the originating chat. > - That report uses a real provider turn and must stay in cost accounting. > - Ordinary first-task proposals must also support the promised chat acceptance path. > - A revision response can reject an older proposal before it creates the replacement. > - The old rejection transition clears that active response's ownership and blocks its next tool. > - This change proves required completion work, guides ordinary proposal policy and preserves the revision turn under the existing locks. ## Linked Issues or Issue Description No separate issue exists. The original failures remain in [the complete Product run](https://github.com/paperclipai/paperclip/actions/runs/41915965118). The independent continuation repair is #19076. Related #12209 concerns prior-run confirmation binding. This change fixes the ownership reset during rejection, rather than changing binding authorization. **What happened?** The complete 20-case run finished with 13 passes and 7 failures. Three delegated-chat cases counted one required `chat_task_completed` provider turn as unexpected work. Two first-task agents chose optional human-only cards, then correctly received 403 when they tried conversational acceptance. A [reviewed two-case candidate run](https://github.com/paperclipai/paperclip/actions/runs/41931445311) then used the changed first-task skill. Both models inherited the ordinary resolver policy. Clarify/propose/accept passed all 12 predicates. Revise/accept failed after the new response rejected its predecessor's proposal. The saved rejection was followed by `paperclip_runner_tool_binding_not_authorized` on its next tools. The final `native_session_lease_lost` label followed those binding failures; this PR does not assume lease expiry caused them. Both cells cleaned up and published their original grades. **Expected behavior** Keep the existing ordinary-turn bounds and account for one independently proved completion notification. Inherit the normal resolver policy for ordinary proposals that accept chat replies. When an active response rejects an older proposal, save the rejection and retain that response's current ownership so it can create the revised proposal. Keep human, current-source and native completion-review rejection behavior and all real approval restrictions. **Steps to reproduce** The original full run used `4740d251`. The two-case candidate used trusted workflow `7d1e9b34` and target `74da56c4`. Run the focused provider-free controls below. The dual-run API regression first fails because rejection changes the issue to `todo` and clears the newer run's execution and checkout fields. ## What Changed - Prove one completion notification only for native Codex/local `multi-repository`, `clarify-reuse` and `plan-handoff`. Keep the ordinary bounds of 2, 3 and 4. Preserve actual totals of 3, 4 and 5, all run IDs and all usage. - Read those declared totals from the current catalog, which now includes the completion reply. Check the total once and prove that exactly one turn is the required notification. Do not add another turn while waiting. The required-server controls use the actual catalog tasks. - Bind the notification to the same company, agent, conversation generation, completed child, single delivery, accounted native run, saved reply and rendered result link. Reject extra work, duplicate deliveries, unknown events and mutations, including native work without a Paperclip tool call. - Collect workspace operations through the real run API for these three proofs. Keep other collector calls unchanged. Keep the proof controls in the existing required server test lane. - Stop idle polling on a permanent completion-proof error. Continue to retry transient observations within the original 240-second window. - Guide ordinary chat-accepting first-task proposals to omit optional `resolverPolicy` and inherit server/company defaults. Preserve explicit and governed human-only restrictions. Do not lower or recreate saved restricted cards. - Under the existing locked issue and interaction rows, preserve the active responding run when it rejects a different source run's proposal. Change only the ordinary requeue predicate. Keep the saved rejection, notifications, activity and existing Telemetry calls. Keep global status, lease and tool-authority guards unchanged. - Extend the existing native API controls with the actual dual-run rejection/revision sequence and policy, provenance, human, current-source and native completion-review countercontrols. No new workflow or test framework is needed. ## Verification - Current combined head is `a8567ded67e5ab811675390e9a5c5ed2194aee2f`, integrated cleanly on `fb96537e28c4c7bfced4939909cb334392320868`. The upstream completion-reply waiter is preserved. [Current required CI](https://github.com/paperclipai/paperclip/actions/runs/41940780852) passes. Actual test checkout `ff93c034d7bb40f3df7bc75cf51828fd07bb0d29` merges this head into `9c6993f0aab7ebb22a26d78f5d227accb35a6ad1`. - Fresh Greptile is 5/5 on exact head `a8567ded`, with no blocking finding. All addressed review threads are resolved. The separate automated reviewer emitted a paused-work response; that response is not a review approval. - Current required server CI actually passed all 11 native confirmation API cases, 46 completion-proof controls and 13 onboarding asset checks. Canonical typecheck, Build, the native package/server suites, all browser and serialized shards, `ci / e2e` and `ci / verify` pass. No current-head CI retry was needed. - Local dual-run RED: the unmodified transition returned HTTP 200 and saved rejection, then changed the issue to `todo` and cleared the newer run's execution/checkout fields. Whole setup and check took 28.95 seconds; the case took 596 ms. - Local treatment: all 11 real native/API cases pass. The same-session response retains execution/checkout ownership, reads task context and persists the recorded-shape revised confirmation. Its stored lease receipt stays intact and one rejection activity is saved. Human-only rejection, current-source rejection, human rejection with/without a newer owner, Plan/Ask, foreign provenance, authentication and formal approval controls pass. - All 59 existing confirmation resolver controls pass in 16.62 seconds. These cover cancellation, lost ownership, company/user scope, policy changes, transactional rollback, concurrent retries and stale revisions. - All 46 completion controls pass in 4.07 seconds. The two added clock controls preserve the original idle deadline and prove immediate permanent-proof failure plus transient observation retry. The existing live collector-to-proof and native mutation negatives remain in this required server file. - The first current-catalog integration check failed 5 controls in 2.70 seconds. The old consumer added the already-declared reply again and could accept an extra ordinary turn. The corrected consumer passes all 46 controls using the actual catalog. The existing full 20-case catalog/reporting compatibility check also passes in 4.93 seconds. Case identities, configured models and reporting match the declared 20. - The API controls used one owned loopback PostgreSQL instance through existing supported test hooks. It was stopped and removed. Earlier missing-library/setup failures and one corrected current-source test expectation remain retained; they are not counted as product failures or passes. - The unchanged first-task skill has SHA-256 `b789090996e7925676590c006418eb8b57f4514e646f6bca18ffd7b2b8e864de`. Both candidate runtime copies matched it. The inherited policy was observed in both models; one complete acceptance journey passed. This does not qualify the new rejection repair. - The three retained real completion streams pass the stronger local proof, including native items, workspace observations and recorded DOM links. Their original failed grades stay unchanged. - Earlier related checks passed 153 hiring/non-execution controls and 13 asset checks. Their source is unchanged. Required discovery includes the completion, API and asset files exactly once. - Prior candidate `74da56c4` passed all required CI, including canonical typecheck and build, in [CI run 41929148958](https://github.com/paperclipai/paperclip/actions/runs/41929148958). Its fresh Greptile was 5/5. This is prior-head evidence. - The intermediate `053150d9` head also received Greptile 5/5. Its [CI run](https://github.com/paperclipai/paperclip/actions/runs/41940014165) and review stay retained as superseded evidence. They do not qualify the final catalog correction. - A clean full local typecheck/build is not claimed because the reused dependency cache has stale adapter-utils/Runner declarations. No source or lock workaround was made. Fresh canonical CI must pass before merge. - The original full-plan result remains 13/20. The separate two-case probe remains 1/2. A fresh complete saved-plan run against delivered source is required for acceptance. No case, assertion, model, budget, timeout or failed grade is changed. Focused commands: ```sh # From server/; database URLs point only to an owned disposable local instance. node ../node_modules/vitest/vitest.mjs run src/__tests__/chat-completion-proof.test.ts PAPERCLIP_TEST_DATABASE_URL="$OWNED_TEST_DATABASE_URL" node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/confirmation-comment-api.integration.test.ts PAPERCLIP_TEST_POSTGRES_ADMIN_URL="$OWNED_TEST_DATABASE_URL" node ../node_modules/vitest/vitest.mjs run src/__tests__/confirmation-comment-resolution.test.ts ``` ## Risks - The ownership exception applies only to an agent acting as the current execution run against a different non-null source run. Human, unowned and current-source requeue behavior must stay intact. Native completion-review handling remains separate. - The completion proof fails closed on new native tools or event shapes. A new shape needs review. - A stored unchanged lease receipt is supporting local evidence. The final real journey must prove full revision, execution and delivery. - The new head still needs normal protected merge approval. The optional queued Paperclip review is not a passing review. No causal repair of unrelated CI intermittency is claimed. ## Model Used OpenAI `gpt-6.1-sol` in Codex, with `max` reasoning effort. The runtime reports a 475,000-token context window. Used reasoning, code editing, shell tools and provider-free controls. ## 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
|
06826915d7 |
fix(dispatch): park a workspace-busy wait instead of recreating runs (#19143)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Child tasks can reuse their parent's execution workspace. Only one run at a time may write in a shared sandbox workspace. > - A run that finds the workspace held by another live run waits with a `workspace_busy` scheduled retry. > - That wait cancelled the run and created a new retry run every 60–120 seconds, until the holder ended. Each check made one cancelled run record. > - Three sibling tasks that waited for a parent made 15 cancelled runs in 13 minutes. They also got the workspace in random order, not in arrival order. > - This pull request keeps the waiting run as one parked retry while its holder is live, and wakes it when the holder ends. > - The benefit is a clean run history, a prompt handoff, and fair arrival-order access to a shared workspace. ## Linked Issues or Issue Description Refs #18691 (the workspace wait for a live holder). Refs #19075 (sends a stopped writer with a pending save into the same wait). Refs #12587 (an earlier proposal about fairness for this retry). **What happened?** A parent task delegated three subtasks that inherit its execution workspace. Each subtask run was created, then cancelled with `workspace_busy` ("Shared execution workspace is busy: run … is still running"), then created again about every 1.5–2 minutes. One subtask got 8 cancelled runs, one got 5, and one got 2. Every cancellation named a holder that was really running: first the parent run, then the sibling that got the workspace. So serialization was correct. The cost was the churn of run records, and the subtask that waited first ran last. **Expected behavior** A run that needs a busy shared workspace waits as one durable record. It continues when the holder finishes. Siblings that wait for one workspace get it one at a time, in arrival order. Two writers never share the workspace, and the user can still stop a waiting run. **Steps to reproduce** 1. Bind two or more issues to one shared execution workspace in a sandbox environment. 2. Keep a run live on one issue (the holder). 3. Wake the other issues. Before this change, each due check promotes the retry, cancels it with `workspace_busy`, and creates a new retry run. After this change, the retry stays parked until the holder ends. **Paperclip version or commit** Observed on staging at `53735db7`. The code is unchanged on `master`. ## What Changed - `finalizeWorkspaceBusyDeferral` records `contextSnapshot.workspaceBusyWait` = `{ key, holderRunId, waitingSince }`. The key is `execution:<id>` or `project:<id>`. `waitingSince` carries over when the run defers again for the same workspace. - New `server/src/modules/run-dispatch/adapters/workspace-busy-wait.ts`: - `parkWorkspaceBusyRetryInTx` runs in the scheduled-retry promotion transaction, after the existing gate allows the retry. If the recorded holder is still running and is not stale, or an earlier waiter for the same workspace is still scheduled, queued, or running, the retry stays `scheduled_retry`. Only its deadline moves. It does not create a run, an event, or a wake. The fallback deadline is 5 minutes, and never later than the holder's one-hour stale point. - `wakeWorkspaceBusyWaitersInTx` runs in the transaction that ends a running run (`writeRunStatus`). It makes due the retries that wait for that run, and the retries that wait for the same workspace as that run. It registers the scheduled-retry dispatch notification before the write. - `WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS` moves to the run-dispatch domain, so the deferral gate and the parked check use one value. `heartbeat/retries.ts` exports it again. - A retry without a recorded wait (for example, a retry from before this change) keeps the old behavior. - `doc/execution-semantics.md` describes the parked wait. ## Verification - Red before the fix, green after (`server/src/__tests__/heartbeat-workspace-busy.test.ts`, "reused shared execution workspace in a sandbox"): - "waits on a live holder in one parked run, then runs once when the holder ends": before, the retry was promoted while the holder was live. After, it stays parked through due checks at +1 s, +10 min, and +30 min. Ending the holder makes it due at once. It runs exactly once, and there is only one `workspace_busy` cancellation. - "hands the workspace to three waiting siblings in arrival order, one writer at a time": before, nothing woke the waiters when the holder ended. After, exactly one waiter is admitted per handoff, in arrival order. Each runs once, and there are 3 cancellations in total (one first deferral for each sibling). - "still lets the user stop a parked waiter": before, the waiter was promoted while the holder was live. After, Stop cancels the parked retry, and nothing brings it back. - Safety case (green before and after): "lets a stale holder reach the existing quiet-holder failure instead of parking forever". - `npx vitest run src/__tests__/heartbeat-workspace-busy.test.ts`: 35 passed. - `heartbeat-remote-workspace-busy`, `heartbeat-retry-scheduling`, `src/modules/run-dispatch`, `heartbeat/retries.test.ts`, `execution-projection.test.ts`: 266 of 267 passed. The one failure, "records pre-provider quota rejection…", is a timing flake on a loaded machine and passes when it runs alone. It does not use this path. - `tsc --noEmit` reports no errors in the changed files. - Staging evidence was read only. This change is not deployed. ## Risks - One extra indexed `SELECT … FOR UPDATE` on `scheduled_retry` rows when a running run ends. The update and the notification happen only when waiters exist. - If an earlier waiter stays queued for a long time (for example, its agent is paused), later waiters for the same workspace wait behind it. Each check is a deadline update, not a run. The fallback check runs every 5 minutes. - A remote writer that has ended but has not finished its save (#19075) is not parked, because its run is no longer `running`. Such a wait can still make a small number of deferrals until the save settles. - Low migration risk: no schema change. Old retries without the new context key keep the old behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edits, browser read of staging run records) 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> |
||
|
DottaandPaperclip
|
c8013d06ad |
fix(workspaces): preserve task-only warnings during offline migration (#19144)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Ephemeral web servers must save existing workspaces before volume
removal.
> - Offline migration checks file ownership and recovery authority.
> - The same gate also rejects task assignment and configuration
warnings.
> - Those warnings do not require the old filesystem.
> - This pull request preserves known task-only warnings while it saves
files.
> - File recovery and execution obligations keep their existing
blockers.
## Linked Issues or Issue Description
Refs #18943, #19038, #19138.
**What happened?**
Offline workspace migration refuses tasks with assignment,
configuration, disposition or deliberate-wait warnings. It returns
`workspace_recovery_unresolved` even when the warning has no file or
execution recovery authority.
**Expected behavior**
Save local workspace bytes while retaining the task warning unchanged.
Continue to block unknown recovery kinds, file sources, export failures
and execution recovery work.
**Steps to reproduce**
1. Retain a local workspace on a task with an active assignment or
configuration warning.
2. Stop all app writers through the offline maintenance entry.
3. Attempt workspace migration.
## What Changed
- Admit four known task-only warning kinds during offline file
migration.
- Keep file recovery markers, system recovery, native failure markers,
export-failure causes and unknown kinds blocked unless their exact
legacy source is already preserved.
- Keep the existing live-migration recovery exclusion.
- Recheck warning evidence before publication. Do not resolve warnings
or change their owners.
- Add restoration, rejection and publication-race tests. Update the
migration plan.
## Verification
- 112 focused migration and concurrency tests pass with real PostgreSQL
and filesystem archives.
- The four task-only restoration cases fail against the previous
importer with `workspace_recovery_unresolved`.
- Tests delete the old home, restore its saved bytes and compare each
warning row unchanged.
- File and execution markers, unknown kinds and export failures remain
blocked.
- A task warning that gains file recovery during upload prevents
publication.
- Local server typecheck has no changed-file diagnostics. Stale linked
dependency declarations prevent a clean local whole-server result. Clean
full CI remains required before merge.
- The PR was rebased onto the merged Git preservation fix after
resolving a plan-document conflict. The rebased head passed full PR CI
(41943153422). Current head
|
||
|
|
19c710fe93 |
fix(issues): record an interrupt comment as the user's Stop (#19149)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A board user can interrupt a running agent with a comment
(`interrupt: true`) to redirect the work
> - An interrupt is a Stop plus a message, but it did not record the
user's stop identity the way the Stop control does
> - The release path therefore treated an interrupted native run as a
failed run: it blocked the task and took the finalization lease from the
runner that was still settling
> - Workspace preservation then stopped the sandbox under that live
runner, so the runner reconnected for about 6 minutes and the stop could
not be verified afterward
> - The redirect message stayed saved, but it waited on workspace repair
that could not finish
> - This pull request records the board user's stop identity on
interrupts, the same as the Stop control and terminalization stops
> - The benefit is that an interrupt now settles like a Stop and the
redirect can continue
## Linked Issues or Issue Description
Refs #19122 (the Stop-then-redirect fix for the dispatch gate; this PR
covers the interrupt-comment path).
**What happened?**
A board user posted a comment with `interrupt: true` on a task with a
running native run (Daytona sandbox). The run was cancelled with
`operator_interrupted` and the native cancel was acknowledged. Within a
second, the task went to `blocked` and a
`native_continuation_requires_reconciliation` hold was created. The
runner transport then logged `state=reconnecting` for about 6 minutes,
and the run ended with `runner_process_termination_unconfirmed: ...
Daytona sandbox lease ... is no longer active`. Workspace recovery
started about 10–17 minutes later and failed with
`agent_stop_unconfirmed` / `provider_unavailable`. The redirect comment
stayed saved as a deferred wake waiting on `workspace_repair_required`.
No follow-up run started.
**Expected behavior**
An interrupt behaves like Stop followed by a message. The stopped runner
settles normally. The task is not blocked as a failure. The saved
message continues once the previous execution has stopped.
**Steps to reproduce**
1. Start a native run on a remote sandbox.
2. Post a board comment with `interrupt: true`.
3. Observe the task become `blocked` with a
`native-continuation:<runId>` hold, and the run's finalization row
rewritten to `terminal_failure` with no lease owner while the executor
is still settling.
4. Observe workspace preservation stop the sandbox, the runner transport
reconnect until its grace ends, and recovery fail to confirm the stop.
**Root cause**
`recordNativeTerminalRecoveryIfNeeded`
(`server/src/modules/wake-queue/adapters/postgres.ts`) skips intentional
stops through `isAcknowledgedNativeStop`. That check requires
`resultJson.cancelledByActorType === "user"` and `cancelledByUserId`.
The Stop control and terminalization stops record these fields.
`operatorInterruptCancelOptions` (`server/src/routes/issues.ts`)
recorded only `interruptedByActorType` / `interruptedByActorId`. So an
acknowledged interrupt took the failure branch. That branch blocks the
task and sets `nativeRunFinalizations` to `terminal_failure` with
`leaseOwner: null`. Cancelled-workspace preservation then saw a terminal
row with no owner and stopped the sandbox while the runner was still
settling.
**Paperclip version or commit**
Seen on staging at
|
||
|
DottaandClaude Opus 5.5
|
eab672e10a |
fix(workspaces): name the failing remote copy-back step without private output (#19152)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run in a Daytona sandbox saves its workspace after the provider turn. The save is called copy-back. > - Copy-back fails on staging after successful provider turns. Issues become blocked with `retry_exhausted`. > - The recorded cause is only `remote_workspace_operation_failed` or `workspace_sync_out_failed`. Operators cannot see which step failed or why. > - The remote command wrapper drops every unrecognized sandbox failure to the bare code. The finalizer retry keeps only the disk-full cause. > - This pull request records a bounded, redacted cause: the operation, its exit code or signal, and the failing capture stage with structured error fields. > - The benefit is that the next copy-back failure shows its first cause, and sandbox output still cannot leak paths or credentials. ## Linked Issues or Issue Description **What happened?** Native runs on a Daytona sandbox finish their provider turn and accept their result. Then workspace copy-back fails. The first `workspace_finalize` operation records only `remote_workspace_operation_failed`. Each retry records only `workspace_sync_out_failed`. The run has no error text. After the retries stop, the issue is blocked. **Steps to reproduce** 1. Run a native task in a Daytona sandbox. 2. Make the sandbox capture step fail with an error that is not disk-full or a known archive validation error. For example, make Git exit with 128. 3. Read the run's workspace operations. **Expected behavior** The failed operation names the step that failed and a redacted cause. **Actual behavior** The failed operation shows only `remote_workspace_operation_failed` and then `workspace_sync_out_failed`. Refs #19050, #19059, #19061, #19063. These are related Daytona and finalizer repairs. This PR does not duplicate them. ## What Changed - `remote-workspace-snapshot-archive.ts`: add `remoteWorkspaceFailureDetail`. It is self-contained and runs in the sandbox. It reports only fixed keys (`code`, `exit`, `signal`, `syscall`, `killed`, `program=git`, `name`, `reason`). Each value is at most 64 `[A-Za-z0-9_]` characters and comes from structured error properties. It never reports message text, command lines, or paths. The archive child now writes `remote_workspace_archive_failed: <detail>` for an unclassified capture error. - `remote-workspace-transport.ts`: the helper-install and capture scripts write `remote_workspace_capture_failed: stage=helper|archive|publish <detail>`. The server accepts this text only when it matches a strict grammar. Unrecognized failures now throw `RemoteWorkspaceOperationError` with the message `remote_workspace_operation_failed: operation=<op> exit=<n>[ signal=<SIG>][; <validated cause>]`. Existing exact diagnostics (Git operation in progress, full disk, archive validation) do not change. - `native-workspace-finalizer.ts`: a retried remote save records the redacted cause in the operation stderr and in `metadata.workspaceSync.cause`. Other error text still stays off the record. - Tests: new red/green cases. The old privacy tests now assert the anchored redacted form, which still excludes any synthetic private text. ## Verification - Red, before the fix (only the error class existed). 8 tests failed on behavior: - The real Git failure produced `Received: "remote_workspace_operation_failed"`. - The signal-terminated command produced the same bare message. - The forged-cause cases produced the same bare message. - The finalizer retry case lost the cause. - Green, after the fix: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/services/remote-workspace-snapshot-archive.test.ts` reported 126 passed. - The new capture test runs the real helper scripts with a controlled `git` that exits 128 and prints a synthetic private path. The error is exactly `remote_workspace_operation_failed: operation=capture exit=1; remote_workspace_capture_failed: stage=archive exit=128 program=git helper_exit=1`. The private path is absent. The workspace files are unchanged. The test passes with split output and with one-shot output. - Forged causes are dropped. These cases are tested: a path in a value, an extra line, an unknown key, and a prefixed text. - Wider sweep: I ran all 63 server test files that touch remote workspaces, copy-back, or finalization. 6381 tests passed. 2 tests failed in `chat-channels.integration.test.ts` and `git-workspace-expiration.test.ts`. These 2 tests also fail with this PR's three source files reverted to `master`. This PR does not cause them. - Typecheck: `tsc` reports no errors in the changed files. Other errors are local, from unbuilt workspace packages. - Not done: I did not run a live Daytona copy-back. This change adds diagnostics. It does not repair the underlying failure. The staging failures can be diagnosed after this PR is deployed. ## Risks - The error message for an unrecognized remote failure changes from the exact bare code to the same code plus validated fields. No production code compares the exact text. Only tests did, and I updated them. - Privacy: the sandbox detail contains only structured error properties. The server re-validates them with an anchored grammar with a length limit and a field limit. A path, a message text, or an unknown key fails the grammar, and the server drops the whole cause. - Retry and classification policy do not change. `classifyNativeWorkspaceFailure` does not read the new text. - This PR does not address two items: the final snapshot operation stays `running` after retries are exhausted, and no disk-retention cause was proven for the ACPX disk-full report. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, browser read-only GETs on staging, 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> |
||
|
DottaandPaperclip
|
7c66b6b1f6 |
test(onboarding): cover plan, hiring, and execution approval history (#19069)
## Thinking Path > - Paperclip manages agent work and human decisions. > - Onboarding can approve a plan, a hire, and an execution step in separate turns. > - Tests covered single approvals more closely than that full history. > - The repaired gate must retain valid plan approval through later decisions. > - Every consumer must recognize the saved confirmation kind and selected scope. > - This change tests that contract and registers a complete interview-to-execution journey. ## Linked Issues or Issue Description Refs #19043, #17914, #18283. The earlier tests admitted one accepted proposal at a time. Decomposition fixtures used standard confirmations. The direct-task E2E path forbids hiring. Those cases did not cover an accepted checkbox plan followed by hiring and a separate untargeted execution approval. ## What Changed - Add eight PostgreSQL contract cases for standard and checkbox plan approvals. Native tools create the cards. An authenticated human accepts them through HTTP. A later native run reads the approved revision and actual selected scope. - Exercise native task creation, generic REST creation, child REST creation, and accepted-plan decomposition. Check the assigned child and one creation audit event. Check durable replay on the three entry points that support it. A pending execution confirmation still prevents creation. - Register the explicit-only `onboarding-approval-history` suite for local native Codex and ACPX Claude. The browser chooses the interview, selects only drafting in the saved plan, approves one writer hire, and approves a separate Start drafting confirmation. The model must hire the writer and use that writer's own child execution to save the welcome document. - Grade each human decision, the unchanged plan revision and selection, approval timing, one assigned child and worker execution, the exact writer-authored saved revision, no pending requests, and persistence after reload. Calibrate the oracle against missing, forged, premature, duplicate, and changed outcomes. - Preserve synthetic option choices, document/run bindings, and saved revision authorship in the existing public first-task evidence projection. Keep private provider/account data excluded. Document the workflow, evidence, billing, and cleanup bounds. - Align the approval-history fixture with its completion oracle: the onboarding parent must be Done. Test both native profiles against graded completion evidence and reject a completed child whose parent remains In review. Count parent heartbeats separately from the writer: accept one to seven parent records while retaining the twelve-company-run cap and the independent three-approval/one-writer oracle. - Retain the existing 52-cell first-task suite. The new suite has one attempt, seven expected work turns, a twelve-run maximum, a fifteen-minute deadline, and the existing $5 company task budget. It is excluded from `--all`. - Update two stale cleanup assertions to include the existing thirty-second deletion timeout. ## Verification - Red control: temporarily restore the pre-fix admission module and run the new checkbox/native history case. It fails with the expected `First-task proposal requires human acceptance` error after plan, hiring, and execution acceptance. Restore the repaired module after the probe. - The final post-rebase history matrix and existing onboarding safety suite passed together: 74 tests, no skips. - Final focused Runner E2E support passed: 413 Vitest tests and all 370 Node support checks. This covers catalog discovery, journey controls, wrong-outcome calibration, public evidence, and existing first-task behavior. - Review controls: four authorship cases fail before the revision check. A delayed browser-response control fails before the approval-time fix. Both pass after repair. The final oracle requires the exact current document revision, writer ID, native child run ID, matching body, and readback after reload. - Credential-free discovery lists both new native cells. No provider call was made. `OPENAI_API_KEY` and `ANTHROPIC_API_KEY` are absent, so the real provider journey remains unqualified. - The earlier broad credential-free E2E support run reported 6,728 passing cases, three skipped cases, and nine failures in three unrelated files. All nine reproduce with the changed E2E source restored to the unchanged baseline: three expired overhead-fixture cases, one stale Pi profile-version assertion, and five workflow security assertions. This change does not alter those files, paid workflow policy, or provider pins. - Previous full workspace typecheck/build and [Linux CI](https://github.com/paperclipai/paperclip/actions/runs/41921754638) passed on `4f431f5aae99d072cc75027e90d7b847b4c342ca`, including the new eight-case PostgreSQL file. Ponytail subsequently found a fixture/oracle terminal-status mismatch. Both native-cell controls fail on that fixture (`done` observed versus `in_review` expected), while the parent-in-review negative control passes. A second comparison uses the actual `matchesRunCount` predicate: graded evidence with six parent runs and one writer run fails the old exact-seven-parent requirement. Both fixes pass these controls, all 413 focused support tests, all 370 Node checks, and the owning E2E project typecheck. [Fresh CI](https://github.com/paperclipai/paperclip/actions/runs/41936699686) passed on `91c980b110252da451b92eda597c168517de1276`: 48 successful jobs and one expected skip. Greptile reviewed this head at 5/5 with no unresolved findings. Ponytail's terminal-status finding is fixed and its thread is resolved; its current-head re-review remains queued. Live provider cells remain unqualified. ## Risks The production API and approval gate are unchanged. The new paid journey can expose model or product failures at an intermediate approval boundary. It retains those failures and uses the existing diagnostics and cleanup. Registration and deterministic tests do not establish a live provider pass. The unrelated baseline support failures remain explicit. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and code execution. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (focused tests; unrelated baseline failures are recorded above) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
5b99bdae47 |
fix(native): verify requested publication at finalization (#19130)
## Thinking Path > - Paperclip manages work done by AI agents. > - Native execution reports work completion through a saved result. > - Some native providers can build that result from their final message. > - This path can bypass the finish tool's publication check. > - A task can then be marked done without the document the user requested. > - This change checks publication again at finalization and status commit. > - Missing output uses the existing bounded repair path with specific feedback. ## Linked Issues or Issue Description **What happened?** A native provider returned a final message about a local Markdown briefing. It had not published a task document or a verified attachment. Its synthetic result claimed success. The server accepted the claim and marked the task done. A real PostgreSQL regression reproduced this on public master. **Expected behavior** A requested task document must have valid saved publication evidence before the task becomes done. The check must apply even when the provider does not call the finish tool. A plain answer still needs no document. An explicit document-free fallback requires the actual human rejection for the current unchanged request. **Steps to reproduce** 1. Request a Markdown briefing document on a task. 2. Complete a native turn with a synthetic `provider:opencode:agent-message` result and no published output. 3. Admit the result through the normal control-plane port and finalize it. 4. The old code marks the task done. The repaired code keeps it open and queues one repair with publication instructions. **Paperclip version or commit** Reproduced from public master `7d1e9b3477f12780e987c32fb8191d476c210fda`. Related: #18661 added the finish-tool publication guard. This change applies that guard to finalization. #18180 covers a different fallback problem: no final text. This change preserves native provider fallback generation and its saved claims. ## What Changed - Run the existing deliverable guard before accepting a native done decision. Keep the original provider result unchanged. - Route missing publication through the existing single incomplete-result continuation. Include the publication feedback in the wake. A repeated incomplete result uses the existing named recovery action. - Inspect publication through a typed safe projection of the accepted ledger. Keep whole-protocol validation at result admission. An authoritative done decision must pass publication checks even when its accepted ledger is partial. - Recheck publication under the status-commit transaction. Hold only matching proof rows with non-waiting share locks. Changed or locked proof uses the existing bounded status replanning path. - Permit an explicit document-free decline fallback only for the saved human rejection selected by the current continuation. Require the same original request, current saved description, source revision and source contract. Model prose or an unrelated old decision is insufficient. - Add real database controls for publication, plain answers, authenticated decline, stale or wrong decisions, bounded recovery, and deletion between assessment and commit. ## Verification - The first regression failed before the repair: the synthetic result marked an unpublished task done. The retained real PostgreSQL run took 21.846 seconds, including setup and cleanup. - Two description-edit regressions also failed before their repair: a description change after admission or at the commit boundary still let the old decline waive the newly required document. Both now keep the task open. The red run took 25.738 seconds. - The commit-race regression also failed before its repair: removing publication after assessment still allowed done. That run took 17.448 seconds. - All thirteen focused real database controls pass after the repair. They exercise the normal control-plane port, finalizer, semantic publication tools, saved human rejection and status transaction. Owned database processes, data and output storage are removed after each run. - All 469 tests across the seven relevant and affected suites pass on head `f8870469f9879a53551f0faaff0fe5b1234ea303`, based on master `c7390bef20`. The six-suite run passed 426 tests in 97.319 seconds including owned setup and cleanup. The unchanged reassessment-worker suite passed 43 tests in 48.901 seconds. All required current-head checks pass in [CI run 41937653209](https://github.com/paperclipai/paperclip/actions/runs/41937653209), including aggregate verify, typechecking, build, general and serialized tests, native Runner tests and all eight browser shards. Fresh Greptile review is 5/5 on this exact head, with no open findings. - Local scoped server typechecking reported no errors in the four changed files and 11 errors in reused dependency declarations and exports. That setup failure remains retained. Fresh current-head CI passes the full Typecheck + Release Registry gate and Build. Local typecheck is not claimed as passing. - CI on the previous head found one shared status-projection regression across Studio, routine, status-authority and worker controls. Those phases use opaque accepted ledgers. The correction inspects only publication references, preserves the accepted marker and bindings, and forces the publication check for an authoritative done decision. A new real commit control verifies that an empty accepted ledger cannot close a requested-document task. All original affected assertions remain unchanged. The retained failed run is [41934314437](https://github.com/paperclipai/paperclip/actions/runs/41934314437). - No provider call, paid campaign, staging deployment or eval regrade is part of this PR. No model, timeout, assertion, workflow, dependency or lockfile change. Focused suites: ```sh ./node_modules/.bin/vitest run --config "$PWD/server/vitest.config.ts" --root server \ src/services/native-runtime/paperclip-control-plane-port.test.ts \ src/services/native-runtime/native-completion-feedback.test.ts \ src/services/native-runtime/native-deliverable-feedback.test.ts \ src/services/native-runtime/native-skill-test-status.test.ts \ src/services/native-runtime/native-routine-status.test.ts \ src/__tests__/native-status-arbiter-corpus.test.ts \ src/__tests__/native-status-reassessment-work.test.ts ``` ## Risks A native result that previously closed a task without its requested output now leaves it open. The existing repair path can run once and then records a named recovery action. Database contention does not grant completion. The existing first-task decision gate still runs first. The conditional decline check is deliberately conservative; unmatched wording or missing authoritative source data does not waive publication. This change closes a false completion path. It does not establish why a provider omitted publication, prove a live provider repair, or qualify a full evaluation plan. ## Model Used OpenAI Codex, based on GPT-6, assisted with diagnosis, code and controls. The exact serving model identifier and context-window size are not exposed in this session. Capabilities used: repository inspection, code editing, local code execution and GitHub tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal issue identity - [x] I have run relevant tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have documented the behavior and limits in this PR; no operator command or public API changed - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open findings - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandClaude Opus 5.5
|
b9fbbf8d30 |
fix(native): continue runs woken by withdrawn or cancelled interactions (#19163)
Withdrawing a card or cancelling a question wakes the assignee with that interaction id. The native interaction bridge required every wake interaction to be accepted, rejected or answered, so preparation failed with native_interaction_unresolved before the provider started. Project a cancelled interaction as a closed response with its stored result. Consumers grant authority only for accepted, answered or pending items, so the closed response carries context and no approval. Pending requests and governed tool reviews keep their existing checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Devin FoleyandPaperclip
|
5fdfd3f17a |
fix(recovery): bound repeated saved-record response bodies (#19168)
## Thinking Path > - Paperclip manages agent tasks and their saved execution history. > - A continuation carries completed API receipts to prevent duplicate work. > - Repeated saves can copy large record bodies into each new provider prompt. > - The resulting command can exceed the durable PRP frame limit before useful work starts. > - This change projects large successful saved-record responses as compact references. > - The original receipts and authenticated user direction remain available. ## Linked Issues or Issue Description **What happened?** A task with many saved document and comment responses can fail with `command_payload_too_large`. Same-run recovery can then show a runner identity failure that hides this first cause. Completed response bodies repeat content already stored in task records. **Expected behavior** Continue from saved records with completed-action evidence that fits the runner command. Keep user messages, answers and approvals intact. Keep the full original receipts. **Steps to reproduce** 1. Save 39 successful document, comment and task-write responses in a run's API receipt ledger. 2. Give each response a large record body. 3. Build and render a continuation, then queue it through the real durable PRP journal. 4. Before this fix, the regression throws `command_payload_too_large` with a 4.2 MB prompt. Related work: #16980 pages oversized assigned tool results. This change addresses earlier response bodies repeated in a later continuation. It does not change tool-result paging or protocol limits. ## What Changed - Project response data over 4 KiB for three known successful record-write operations. Keep record, revision and status metadata, HTTP outcome, receipt identity and a data digest. - Keep small, failed, unknown, mismatched and foreign-company results in their existing form. - Explain the reference format in shared prompt guidance and the execution contract. - Add database and real-journal regression coverage. Verify the original receipt ledger remains unchanged. The continuation service owns the projection. The shared adapter helper only explains the reference to native and legacy prompt consumers. There are no new dependencies, provider calls, timers or background scans. ## Verification - The unfixed database regression fails at the real PRP command limit with `command_payload_too_large`. - Continuation builder, native execution input and native continuation suites: 118 tests pass. - Shared prompt consumers: 142 tests pass. - Module boundaries and diff whitespace checks pass. - Greptile: 5/5 on `8109e7361c5b5e38f9aad35af9bf4ad7bdc5f338`, with no unresolved review threads. All CI checks pass. - Full workspace `pnpm -r typecheck` and `pnpm build`: passed. - Hosted verification: pending deployment. The source fix does not itself resume the affected hosted task. Local setup failures remain accounted for: the fresh checkout lacked an ignored root SDK link and embedded PostgreSQL library symlinks after an offline install that skipped lifecycle scripts. The maintained workspace-link helper, SDK build and platform hydration script repaired those prerequisites. A later test expectation assumed JSON insertion order across PostgreSQL JSONB; it now checks the digest against the unchanged stored receipt. These failures did not change runtime limits or recovery guards. ## Risks Large responses from these three successful operations now use explicit saved-record references in continuation context. Agents must read the record or source receipt when full content is needed. This is not a universal prompt-size limit: user history, unknown operations and other inputs retain their existing representation. No migration, permission, credential, provider-stop or replay policy changes. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, through Codex in T3 Code. Used for investigation, implementation and local verification. No subagents. ## 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 linked existing issues or described the issue in-PR following the issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal task id - [x] I have run the targeted tests locally and they pass - [x] I have added tests and updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open findings - [x] I will address review comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandClaude Opus 5.5
|
7f9ef38273 |
fix(runner): report the upstream cause of OpenCode provider failures (#19166)
OpenCode reports some OpenRouter failures as an UnknownError whose message is the provider's JSON error body. The provider failure now reads its HTTP status, error type and message from that body, so the run shows the upstream message instead of raw JSON. The OpenCode nativeTurnFailure diagnostic now carries the provider failure's category, retryability and HTTP status (closed values only; no provider text or code), and recognizes the driver's opencode_empty_response code. Run status, error codes and retries do not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Nicky Leach
|
737db73275 | fix(native): measure resumed run usage from the prior run's total (#19161) | ||
|
Devin FoleyandClaude Opus 5.5
|
3c7af35103 |
fix(errors): return tool gateway rejections with their own status (#19158)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents call connected tools through the tool gateway. Some tool calls need a human review before they run. > - A user approves a tool review from a task card. The approve route calls the gateway. > - The gateway can reject the approval with `ToolGatewayHttpError(409, "approved_tool_target_changed")`. This is an expected rejection: the tool or connection changed after the review. > - `ToolGatewayHttpError` does not extend `HttpError`. The shared error handler only keeps the status of an `HttpError`. Thus the route answers 500 "Internal server error" and sends the rejection to Sentry as a crash. > - The user does not see the reason. The user clicks Approve again, and each click makes one more crash report. One user made 69 reports on staging this way. > - This pull request makes the shared error handler keep the status and reason code of client (4xx) gateway errors. > - The benefit is a clear 409 with a reason for the user, and no false crash reports. ## Linked Issues or Issue Description Refs #12775 (related community PR. It changes `ToolGatewayHttpError` to extend `HttpError`, which affects every `instanceof HttpError` check. This PR is smaller and keeps gateway server failures on the crash path.) **What happened?** A user approves a tool review on a task card. The tool definition or connection changed after the review. The API answers 500 "Internal server error". Sentry records the expected rejection "Tool definition or connection changed; request a new review" as a server crash. Each retry records one more event. **Expected behavior** The API answers 409 with `code: "approved_tool_target_changed"` and the message "Tool definition or connection changed; request a new review". Sentry does not record a crash. **Steps to reproduce** 1. Make a connected tool that needs approval, and start a tool call so that a review card appears. 2. Change the tool connection (for example, its config) before you approve. 3. Click Approve on the task card (`POST /api/issues/:id/interactions/:interactionId/accept`). 4. See 500 "Internal server error" and a new Sentry event from `approveActionRequest`. **Paperclip version or commit** `master` at `482ec0aea9ab6f2684baaa1487f2c7a2eedf009e`. ## What Changed - `server/src/middleware/error-handler.ts`: the handler converts a `ToolGatewayHttpError` with a 4xx status to an `HttpError`. The new error keeps the status, the message, the gateway details, and the reason code as `code`. The original stack stays. - Gateway errors with a 5xx status do not change. They still go on the crash path, answer 500 "Internal server error", and go to Sentry. Thus provider text in those messages stays hidden. - `server/src/__tests__/error-handler-tool-gateway.test.ts`: new tests for the 409 mapping, for the unchanged 5xx path, and for an Express route that lets the gateway error escape. ## Verification - `cd server && npx vitest run src/__tests__/error-handler-tool-gateway.test.ts src/__tests__/error-handler.test.ts` passes: 21 tests. - Without the fix, the new tests fail with `expected 500 to be 409`. With the fix, they pass. - `src/__tests__/issue-thread-interaction-routes.test.ts` and `src/__tests__/json-parse-error-handler.test.ts` also pass: 175 tests in 4 suites with the two error handler suites. - I checked the runtime import graph of `services/tool-gateway.ts` (685 modules). It does not import `middleware/error-handler.ts`, so the new import makes no cycle. - I did not run the full server typecheck locally, because it ran out of memory in an 8 GB sandbox. CI must do this check. ## Risks - Low risk. Only gateway errors with a 4xx status change. Before, they became 500. Now they keep their own status. - Some routes outside the gateway router can now return gateway 4xx details in the `details` field. The gateway routes already return the same details through `sendGatewayError`. - `middleware/error-handler.ts` now imports `services/tool-gateway.ts`. Unit tests that import the error handler load more modules. The app already loads this module. - This PR does not fix the cause of some of these rejections. A routine OAuth token refresh changes values in the signed approval snapshot (token secret versions, the order of grant secret refs, and token expiry times). This can make a pending review fail. That fix changes `services/tool-gateway.ts` and comes in a separate PR. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used as a coding agent with tool use (shell, file edits, test runs, and GitHub API). ## 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 (no documentation change is necessary) - [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
|
94fccc3180 |
fix(workspaces): preserve snapshots beside runtime sockets (#19159)
## Thinking Path > - A workspace can contain saved source files alongside transient runtime IPC endpoints. > - The portable archive supports files, directories and confined symlinks; treating a Unix socket as a fatal entry prevents every file beside it from being saved. > - Keep the existing archive owner and validation. Record socket paths as exclusions, leaving the source endpoint and adjacent work intact. ## Linked Issues or Issue Description **What happened?** Retained workspace recovery repeatedly fails with `remote_workspace_archive_unsupported_entry` when the source tree contains a Unix socket, preventing adjacent source files from being saved. A real socket reproduces the error in local capture and the serialized sandbox helper. **Expected behavior** Save regular workspace files, record runtime socket paths as exclusions, and leave the source endpoints and new file edits intact. **Steps to reproduce** 1. Create a temporary workspace with an uncommitted text file. 2. Open a Unix domain socket in that directory with a Node server. 3. Capture the workspace through the local archive function or generated sandbox helper. Before this fix both captures fail; afterward the saved file restores and the socket exclusion is recorded. **Paperclip version or commit** Reproduced on `6fa54d55779295d90c455389cebdd3da96607d20` before this change. The socket fixture establishes this failure class; it does not establish the entry type behind every historical unsupported-entry occurrence. ## What Changed - Exclude Unix domain socket endpoints from portable workspace captures and include their paths in the existing exclusion manifest. - Keep source sockets in place. Preserve normal files and the existing rejection of named pipes, device entries and unsafe links. - Exercise local and serialized capture, an unchanged source endpoint, restoration of adjacent uncommitted work, FIFO rejection, and a socket becoming a real file during capture. A changed source still fails safely and retains that new file. - Document the portable snapshot behavior. ## Verification - Red regression: both local and serialized captures failed with `remote_workspace_archive_unsupported_entry` before the change. - `pnpm exec vitest run server/src/services/remote-workspace-snapshot-archive.test.ts`: **43 passed** using real filesystem sockets, pipes, Git repositories and archives. - `pnpm -r typecheck` and `pnpm build`: passed. - Module boundaries and `git diff --check`: passed. Added source and PR text were scanned for credentials, private incident identifiers and personal data before push. - Snapshot storage and save-concurrency consumer suites pass **23 tests**. Their first local runs exposed missing workspace SDK build links and unhydrated macOS embedded-Postgres library symlinks in the new checkout. Repairing those prerequisites made the unchanged consumer suites pass; the earlier failures remain recorded and no assertion was weakened. - Greptile reviewed `e313565a5088abce448f11b29982462aa338a692` at **5/5** with no unresolved threads. All current-head CI checks are green and the branch is conflict-free. Hosted recovery needs a qualified deployment and original-task verification. ## Risks Socket endpoints represent runtime IPC and are not recreated by archive restore. Their paths are visible in the existing exclusion manifest. This adds no archive entry type, subprocess, recurring work, provider call or dependency. It does not alter source authorization, stop receipts, ordinary file validation, restore safety or recovery budgets. Unix socket/FIFO fixtures are skipped on Windows; existing portable archive coverage still runs there. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) through Codex, xhigh reasoning. Used repository tools, code execution and real local filesystem/process fixtures. ## Checklist - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] Added focused behavioral and preservation regression coverage - [x] Updated the execution contract and documented limits - [x] Typecheck and build pass locally - [x] Scanned the diff and description before push - [x] Current-head CI is fully green - [x] Greptile is 5/5 with no unresolved comments Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandClaude Opus 5.5
|
7edbc1fc07 |
fix(server): keep a successor's issue lock when a finalizing native run releases its checkout (#19156)
`release_checkout` now clears `checkoutRunId` and the execution lock only when they belong to the committing run, so a successor that claimed the issue while its predecessor was finalizing keeps its claim and passes reviewed-chat attestation. `attestReviewedExternalChatRun` reports the first failed check as a closed label. Setup failures store it in `resultJson.reviewedChatBinding.denialReason`, and Sentry run-failure diagnostics export it as `execution.reviewedChatBindingDenialReason` (no IDs or text). The error message is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
482ec0aea9 |
fix(native): run saved copy-back retries on schedule behind a slow export (#19145)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs copy their sandbox workspace back after the provider finishes. A failed copy-back saves a retry 30 seconds later, with a bound of 3 attempts. > - One shared delivery queue runs these retries. A pass of that queue ends only when its slowest check ends. > - A legitimate sandbox export can take 10+ minutes. While it runs, all other due retries wait. > - On staging, a run active across a deploy showed "Reconnecting" with an overdue retry for about 24 minutes. Its retries ran at +24 min and +3.5 min instead of +30 s. It settled only after about 32 minutes. > - This pull request lets idle check slots admit entries that come due during a pass. It does not overlap a running check's resource and does not start work after abort or drain. > - The benefit is that the existing 3-attempt bound settles a failing copy-back in about 1 minute, not after the slowest unrelated export. ## Linked Issues or Issue Description **What happened?** A native run whose result is durable but whose workspace copy-back failed stays in `running` with the label "Reconnecting" for a long time. Its saved retry time is in the past. The retry does not run until an unrelated slow copy-back in the same queue pass finishes. **Steps to reproduce** 1. Start two native runs on different tasks with accepted results. 2. Let the copy-back of run A take many minutes. 3. Let the copy-back of run B fail. It saves a retry 30 seconds later. 4. Watch run B. Its retry does not run until run A's copy-back ends. **Expected behavior** Run B retries on its saved schedule. After 3 failed attempts it settles with `native_workspace_sync_out_retry_exhausted` and a recovery action, while run A continues. **Paperclip version or commit** master at |
||
|
|
c2cc69d1e4 |
fix(recovery): resume and settle preserved native results (#19034)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native execution can save an accepted result before workspace export fails. > - Recover must preserve the files and settle that same result without another agent turn. > - The export failure can own the task's Blocked transition and its recovery notice. > - Recovery must prove preservation, stop, source identity and current authority before settlement. > - Recover can also transfer repair ownership to the system, which must keep explicit board export repair available. > - This change restores those paths while preserving later task state, live owners and unknown action outcomes. ## Linked Issues or Issue Description Related #17916 protects unfinished exports. Related #19004 preserves deliberate Retry authority. This change repairs accepted-result finalization and its recovery notice. **What happened?** A native run accepted its result, then exhausted workspace export. Normal Recover could save the files but leave the result and old recovery notice unsettled. A saved Retry could also use a different generation from preservation. Separately, Recover changed the action owner from board to system. Explicit export Retry then rejected that same source as stale, even when its accepted result and stopped allocation still matched. **Expected behavior** Preserve files, finish the original accepted result and close only its obsolete notice after exact proof. Keep explicit board export repair available after system adoption. Preserve current authority, Stop, later task changes, newer execution and every physical ownership fence. **Steps to reproduce** 1. Accept a native result and exhaust its workspace export. 2. Request ordinary Recover, optionally with an existing saved Retry. 3. Let file preservation complete. Inspect the task and source recovery notice. 4. To reproduce the explicit-repair defect, call export Retry after Recover adopts the action. Before this repair it rejects the valid system-owned source before any provider call. ## What Changed - Record the exact accepted result, contract, task status version and prior decision when export exhaustion creates Blocked. - Bind passive finalization to complete, source-bound preservation and the current chat/review authority. Recheck the proof under status commit locks. - Keep the saved Retry identity separate from its preservation generation. Retain coalescing and reject malformed or mismatched history. - Preserve an existing published GitHub assessment for the no-publication status-unproven wait. Keep all semantic, result, authority and publication checks. - Close an old export notice only after its own accepted result, export or preservation receipts, effects, projection and old Runner ownership settle. Keep newer work and files unchanged. - Permit explicit board export Retry after Recover transfers the exact source to the system. Require valid system and workspace recovery records for that same run, plus every existing result, stop, allocation, task and shared physical-lock check. Reject missing proof, another source and agent ownership before provider access. - Merge current master while retaining the stronger Stop, source and verified preservation checks. - Update the execution contract. No schema, endpoint, Runner protocol, model, trust or provider artifact change. ## Verification - Current head: `9be5a73489801095c9f39c9bfcb285a22b055912`. - The explicit-repair regression fails before the new guard change: ordinary Recover adopts the real database action, then export Retry rejects it as stale. It passes after the repair. - On the merged source, three owning PostgreSQL/file suites pass **220 tests**: `preserved-native-response-wait.integration.test.ts`, `native-workspace-export-retry.test.ts` and `idle-completed-history.test.ts`. The export suite includes four unrelated-owner/source rejection controls and both board-owned and system-adopted active-exporter controls. Results and run counts prove that repair creates no provider turn. - Server layered typecheck, module boundaries, `git diff --check`, and the added-line secret/private-incident scan pass. - Full workspace typecheck (`pnpm -r typecheck`) and build (`pnpm build`) pass. The complete local `pnpm test:run` is still running; its completed serialized chat partitions pass. Current-head CI is green (53 successful checks, 3 expected skips). Greptile reviewed `9be5a73489801095c9f39c9bfcb285a22b055912` at 2026-11-05 00:58 UTC with confidence **5/5**; all review threads are resolved. - Original qualification remains supporting evidence: the accepted-result fixture exercises HTTP Recover, real files, the finalizer, saved Retry coalescing, eight commit races, current principal/resource/head changes, Stop, later task changes, notice closure and repeated delivery. No accepted response, provider turn or publication is replayed. - Earlier failures remain recorded. Original red controls exposed wrong-action preservation, unrelated finalization barriers, generation mismatch and a lost GitHub assessment; the original repair added controls for them. The prior CI run [41908572248](https://github.com/paperclipai/paperclip/actions/runs/41908572248) failed an unchanged completed-history runtime fixture because its loopback service refused connections through the original ten-second readiness window. That owning suite passes unchanged in the 220-case run above. The earlier readiness failure's cause remains unproven; no wait or assertion was weakened. - Local and CI tests do not establish hosted task recovery. The serving revision and original-task outcomes require separate verification after rollout. ## Risks - Historical failures without the exact export-owned Blocked receipt keep their current task status. A saved result does not grant execution or publication authority. - Explicit export repair still resumes only the same retained allocation under the shared physical lock. It cannot take over an active exporter, create another agent turn, choose a replacement sandbox or discard saved files. - Retiring a source notice does not release physical resources or modify a newer task owner, later Done state or saved snapshot history. - The proof adds bounded reads only for concrete recovery obligations. It adds no recurring worker or polling timer. ## Model Used OpenAI GPT-6 through Codex for the original repair. OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, for the merge, integration verification and system-adopted export repair. Both used repository tools, code execution and PostgreSQL tests. 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 - [x] I have checked ROADMAP.md and searched for duplicate or related work - [x] I have described the issue and linked related public PRs - [x] I have included no internal task identifiers or private incident links - [x] I have added behavioral and safety regression tests - [x] I have documented verification limits and risks - [x] I have updated the execution contract - [x] All Paperclip CI gates are green on the current head - [x] Greptile is 5/5 on the current head with no unresolved comments - [x] I will address review comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
DottaandPaperclip
|
9e5039f6c8 |
fix(daytona): retain duplex close ownership after lease cleanup (#19150)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Remote agents use provider plugins to manage sandbox leases and duplex channels. > - The host verifies the exact route and session when a bound channel closes. > - Daytona lease teardown removed that binding before a later host close. > - The incomplete reply made the host retire the shared worker. A later command then failed during setup. > - This pull request retains bounded close receipts and joins the original cleanup. > - Later work can use the same worker while invalid or unconfirmed closes still fail closed. ## Linked Issues or Issue Description **What happened?** A normal lease teardown can close a duplex channel before the host closes its bound route. Daytona then replies with only the route ID. The host requires the original session ID too. It logs `duplex channel close not acknowledged; retiring worker`, stops the worker, and rejects a later `environmentExecute` call as stopped. **Expected behavior** A later close must identify the original route and session and join its existing cleanup. It must not revive the channel or restart the worker. Unknown ownership and an unconfirmed close must retain the existing failure behavior. **Steps to reproduce** 1. Acquire two leases through one Daytona provider worker. 2. Open a duplex channel on the first lease. 3. Release that lease, then close the bound channel through the host. 4. Execute a command on the second lease. **Paperclip version or commit** The real host/worker regression fails on `69dea878c0b920b335594debb0286443ba9f6c35`. This branch includes master `3a72efebe5b050e7594bb309e71a0d5f3072db00`. **Deployment mode** Managed Daytona sandbox execution in Cloud. The regression uses a local real host and SDK worker. Only the external Daytona SDK and PTY are simulated. Related work: Refs #19004, Refs #10429, Refs #18065, Refs #19050. These cover task Retry, managed worker startup, drain release, and stale provider sessions. ## What Changed - Retain at most 4096 retired route/session bindings with their original close promises. - Remove live channel authority before cleanup starts. Concurrent and later closes join the same promise and return the stored binding. - Reject reuse of a retained route ID. Keep unknown and evicted routes unable to provide a session acknowledgement. - Add a real child-process host/SDK-worker/Daytona regression with only the external Daytona SDK and PTY simulated. - Cover release, cancellation, stop, destroy, resume, a stalled close, concurrent closes, failed cleanup, forged IDs and bounded eviction. - Document close receipt scope and its separate provider-termination checks. ## Verification - RED before the repair: the actual host and SDK worker reproduced the retirement log, SIGKILL, stopped state, and later `environmentExecute` rejection. The direct provider cases also failed because the session ID was missing. - Combined-branch provider suite: 364 passed. - Combined-branch host manager, duplex, managed readiness and composed suites: 142 passed. The final test-only manifest-loading change passed all 6 composed cases again. - Full repository typecheck (`node scripts/typecheck-low-memory.mjs`), focused server types and the standalone Daytona build passed. The provider build uses pinned SDK `0.203.0` and TypeScript `5.9.3`. - The composed cases check the same worker/PID, one later command, preserved saved content, revoked channel writes/stops, and continued rejection of commands on closed leases. A stalled close still retires the worker and dispatches no later command. - Reproduce with `pnpm --dir server exec vitest run --config vitest.config.ts src/__tests__/plugin-worker-manager-daytona-close.test.ts` from a checkout with the SDK build dependencies prepared. Run the provider suite with its own Vitest config. - An initial local typecheck lacked the excluded providers' SDK links after an install with scripts disabled. The standard root postinstall linker restored those local links, and full typechecking then passed. The fixture loads its actual built manifest. Failed setup output is retained; no type configuration was relaxed. - The first two composed attempts stopped during fixture setup. They are not the behavioral RED above. Their failed outputs are retained. - The first broad local typecheck stopped at an IPC permission error during preflight. It passed after rerunning with allowed local IPC and the standard SDK links. - [Full repository CI](https://github.com/paperclipai/paperclip/actions/runs/41945022881) passed all 48 jobs on this head. Selection fell back to the complete task plan, so the run was unfiltered. The complete check inventory has 54 successes and two expected Storybook skips; the separate Snyk status passed. Fresh Greptile review on `daebbeb5005b9d9bd59d32ea30094279048b9213` is [5/5 with no actionable comments](https://github.com/paperclipai/paperclip/pull/19150#issuecomment-6646621914). Independent production and fixture review found no actionable findings. Hosted acceptance of this repair has not run. ## Risks - A receipt removed at the 4096-entry bound cannot confirm an old bound close. The existing host guard remains effective in that case. - Close acknowledgements identify channels. They do not certify remote process termination or clear unknown external work. Provider termination receipts and existing recovery gates remain in force. - Hosted evidence confirms the unacknowledged-close retirement class. The historical log does not distinguish a missing session ID from a timeout or another close error. The regression proves and repairs the missing-binding path; it also preserves timeout retirement. - The provider is outside the main pnpm workspace. A new Cloud image must compile its changed source. The unchanged plugin version alone does not prove activation. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, source editing and tool execution. The exact model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
11f2a5293d |
fix(native): name the proof that holds a recovered per-turn owner (#19061)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner per-turn sessions in a remote sandbox keep one conversation across task turns. > - A follow-up must prove that the previous Runner owner is finished before it starts a new Runner. #18846 added a retirement path for an owner that finished without a saved Runner checkpoint. > - On a test deployment, follow-ups still fail with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. The retirement path has about twenty separate fences, and each refusal gives the same error. > - The read-only run data cannot tell which fence held these runs. A code fix without that evidence would be a guess. > - This pull request names the first failed retirement proof in the error. Admission behavior does not change. > - The benefit is that the next failure on a deployment identifies the exact fence, so the follow-up fix can target it. ## Linked Issues or Issue Description Refs #18846 (retirement of recovered per-turn owners). Refs #18992 (settlement from live state). **What happened?** On a test deployment, 16 runs in one day failed before provider work with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. All were per-turn `acpx_runtime` runs in a Daytona sandbox, and each got a new sandbox. In the runs that I inspected, the previous run committed its result. In one of them, `harness_state.backup.persist` and `session.checkpoint.persist` ran for about 346 seconds and failed, so the Runner left no suspension receipt or failover backup. That is the case that #18846 addresses, but the follow-ups still failed. The run log does not show the retirement message, and the error is the same for each fence. Thus the cause cannot be found from the run data. **Expected behavior** When admission holds a follow-up, the error names the proof that failed. **Steps to reproduce** 1. Seed a per-turn sandbox owner that committed its result without a Runner suspension receipt. 2. Make one retirement proof fail. For example, leave the owner phase `observed`, or leave a `runner.suspend` command pending. 3. Admit a follow-up in a new sandbox. 4. Before this change, every case fails with the same error text. **Paperclip version or commit** Observed on a deployment before `e107a75755`. The same error text is on `master`. ## What Changed - `retireFinishedPerTurnRunnerdOwner` returns the refusal reason instead of `false`. `readSettledRunnerdProjection` returns the journal refusal reason instead of `null`. - When admission tries the retirement and it fails, the error is `prior_owner_terminal_state_indeterminate; retirement_refused=<reason>`. A warm session gets no suffix, because retirement does not apply to it. - Reasons use only fixed lists of known names: owner phase, recovery state, controller fence reason, unsettled command type and status, sandbox outcome, and quarantine copy result. Any other stored value appears as `other`. Reasons do not include payloads, file names, or paths. - The same proofs apply in the same order. Admission accepts and refuses the same states as before. - `doc/execution-semantics.md` describes the suffix. ## Verification - **Failure before the change:** I ran the updated `native-recovered-owner-followup.test.ts` against the unchanged executor. 18 tests failed. Example: expected `...indeterminate; retirement_refused=owner_observed`, received `...indeterminate`. - **After the change:** `cd server && pnpm exec vitest run src/services/native-runtime/native-recovered-owner-followup.test.ts`: 22 passed. Each held case now asserts its reason: `owner_retryable_failure`, `controller_remote_controller_lease_live`, `owner_recovery_resuming_session`, `journal_commands_unsettled(runner.suspend.pending)`, `journal_tool_input_unanswered`, `quarantine_journal_identity_mismatch`, `runner_lifecycle_not_suspended`, and `quarantine_runner_lifecycle_not_suspended`. - The success cases still retire the owner and admit the follow-up. - `pnpm exec vitest run src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-runner-restart-recovery.integration.test.ts src/services/native-runtime/remote-warm-session-recovery.test.ts`: 1227 passed, 20 skipped. - `pnpm exec vitest run src/services/native-runtime/native-controller-recovery.integration.test.ts src/__tests__/task-workspace-binding.test.ts`: passed. - `tsc --noEmit` reports no errors in the changed lines. This worktree did not build every workspace package, so the full-package typecheck shows unrelated missing-module errors. CI runs the full typecheck. - **Not verified live:** no deployment has run this change. ## Risks - Low risk. Admission logic is unchanged. Only the error text gets a suffix. - Consumers that match the error by prefix still match. `failed-run-retry.ts` uses `startsWith("runner_state_identity_mismatch: ")`. No code compares the full string outside tests. - This change does not fix the held runs. It finds the fence. A second change is needed after a deployment reports the reason. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use, shell execution, and read-only browser access to a test deployment. The harness 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 - [ ] 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
|
3a72efebe5 |
fix(native): confirm no runner process when remote staging fails before launch (#19059)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native sandbox runs supervise a remote `paperclip-runnerd` process. When that process exits during a turn, the transport restarts it inside a reconnect grace window. > - The server launcher stages the runtime assets first. Then it sends one launch RPC that starts runnerd. > - When staging failed, the launcher reported the exit as unconfirmed. This happened although no launch RPC was sent and no process could exist. > - Reconnect recovery treats an unconfirmed exit as a possible second owner. So it stopped at once with `runner_process_termination_unconfirmed` and did not retry. > - This pull request confirms the exit for failures before the launch RPC. Failures after dispatch stay unconfirmed. > - The benefit is that a transient staging failure, such as a full sandbox disk, uses the normal bounded retry. A real failure ends with `transport_reconnect_grace_exceeded` and keeps the staging error as the detail. ## Linked Issues or Issue Description **What happened?** On a hosted staging deployment, 7 native ACPX runs failed in 24 hours with `Provider notification transport failed closed.` (`native_provider_terminal_failed`). In every run, the runner process exited during the turn. The replacement launch then failed while it uploaded the runtime context into the sandbox: `runner_process_termination_unconfirmed: replacement runner health could not be verified: ... .paperclip-upload-<uuid>.tar.gz: create: ... no space left on device`. The sandbox disk was full. No replacement process was started. But the launcher said that it could not confirm the exit. So recovery did not retry inside its grace window. **Expected behavior** A staging failure before the launch RPC proves that no replacement process exists. Recovery must retry within the configured reconnect grace. If the failure continues, the run must end with the reconnect-grace failure and the staging error. **Steps to reproduce** 1. Create a remote runner launcher whose `ensureArtifact` step throws, for example with an ENOSPC upload error. 2. Wait for the handle completion to reject. 3. Before this change, `isExitConfirmed()` returns `false`, and no launch RPC was sent. The transport then fails closed with `runner_process_termination_unconfirmed`. **Paperclip version or commit** Observed on staging revision `89703c3`. Reproduced on `master` at `be20d4ad97`. Related: #19035 and #19014 change other runner paths (replayed terminal turns and redacted external tool results). They do not change remote launcher exit evidence. #18999 adds a free-space reserve for host-volume seed copies. It does not change sandbox disk use or this classification. ## What Changed - `createRemoteRunnerProcessLauncher` records when it dispatches the launch RPC. If the completion fails before that point, the handle reports `isExitConfirmed() === true`. The original error is kept unchanged. - A failed or timed-out launch RPC still reports an unconfirmed exit, because the RPC may have started runnerd. - Added a launcher regression for a staging failure before dispatch. - Added safety cases for a failed and a timed-out launch RPC. - Added a transport regression with the real `paperclip-runnerd`. A replacement whose staging fails is retried, and the next launch restores the durable session without a notification failure. ## Verification - Red before the fix: `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "remote runner process supervision"` failed `confirms no replacement process exists when staging fails before the launch RPC` with `expected false to be true`. The two launch-RPC safety cases passed. - Green after the fix: `npx vitest run src/services/native-runtime/native-session-executor.test.ts` passes 1131 of 1131 tests. - `npx vitest run src/services/native-runtime/setup-readiness.test.ts` passes 98 of 98 tests. - `cd packages/paperclip-runner && npx vitest run src/live/runnerd-codex-transport.test.ts` passes 246 of 246 tests with the real debug `paperclip-runnerd`. This includes the existing case where a replacement with a dispatched but unverifiable process still fails closed. - `npx tsc --noEmit -p server` and `pnpm --filter @paperclipai/paperclip-runner typecheck` pass. - Not run: a live hosted sandbox with a full disk. The staging data showed the exact error, but this change was not deployed there. ## Risks - Low risk. The change affects only failures that occur before the launch RPC. In that window, the launcher has not run any sandbox command that can start runnerd. - Retries now run for the reconnect grace window, which is bounded and uses the existing backoff. Each failed staging attempt cleans its own upload scratch. - This change does not free sandbox disk space. If the disk stays full, the run still fails, now with `transport_reconnect_grace_exceeded` and the ENOSPC detail. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use, shell execution, and code 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 - [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
|
d8dbe55368 |
fix(workspaces): let a missing task repository ref be corrected (#19072)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can ask Paperclip to prepare a task repository at a specific
ref
> - Paperclip records that request and clones the repository at the next
admission
> - Paperclip did not check that the requested ref exists
> - When the ref was absent, every admission retried the same checkout
and failed setup
> - A corrected request was refused as a conflicting receipt, so the
task stayed blocked with no repair path
> - This pull request identifies a missing ref and lets a corrected
request replace it while nothing was published
> - The benefit is that a bad ref becomes a repairable error instead of
a permanent setup failure
## Linked Issues or Issue Description
**What happened?**
An agent asked for a catalog repository at ref `main`. The repository
default branch is `master` and has no `main` branch. Each later
admission failed with `setup_failed`: `git checkout main` reported
`pathspec 'main' did not match`. The task failed admission for more than
a week and became blocked. A new request with ref `master` returned 409
because the receipt with ref `main` already existed.
**Expected behavior**
Paperclip should report that the requested ref does not exist. A new
request with a valid ref should replace the bad ref when no checkout
exists.
**Steps to reproduce**
1. Use a repository whose only branch is `master`.
2. Request it as a task repository with `ref: "main"`.
3. Admit the task. Preparation fails, and the receipt goes to
`needs_repair`.
4. Request the same repository with `ref: "master"`. Before this change,
the request returns 409.
## What Changed
- `managed-repository-checkout.ts`: When `git checkout <ref>` fails with
Git's unknown-ref message, and each quiet `rev-parse --verify` probe
(`<ref>^{commit}`, `refs/remotes/origin/<ref>^{commit}`) exits 1, throw
`RepositoryRefNotFoundError`. The message tells the caller to request
the repository again with an existing branch, tag, or commit. Other
checkout failures keep their original cause.
- `execution-workspace-repositories.ts`: Record `failureCode:
"repository_ref_not_found"` for that error. Other failures keep
`repository_preparation_failed`.
- `execution-workspace-repositories.ts`: A request for the same
repository with a different ref replaces the ref only when all of these
are true: the receipt is `needs_repair` with `repository_ref_not_found`,
it has no pinned commit, the workspace is local (no remote authority),
and the checkout path is absent. The update is conditional on that state
and writes a `workspace.repository.ref_corrected` activity entry. All
other ref changes keep the existing 409 conflict.
- `execution-workspace-repositories.ts`: Local admission re-reads each
receipt inside the checkout lock and claims it with a conditional update
on its current ref. A correction that commits after the receipts were
listed is used, not discarded.
- Tests: one regression for the full missing-ref path, and one safety
test that keeps the receipt immutable when files exist at the checkout
path or when the failure was not a missing ref.
## Verification
- Red before the fix: `pnpm --filter @paperclipai/server exec vitest run
src/__tests__/task-workspace-repositories-persistence.test.ts` failed
with `expected ... /ref "main" does not exist/ but got 'Failed to
prepare managed checkout fo…'`.
- Green after the fix: the same file passes (11 tests). The new test
admits with ref `main` (fails, `repository_ref_not_found`, no checkout),
corrects to `master`, admits again (ready, pinned to the source commit),
and then confirms that a later ref change after publication still
returns a conflict.
- The regression also holds the checkout lock while admission waits,
corrects the ref, and then releases the lock. Without the in-lock
re-read it fails with 409; with it, admission publishes the corrected
ref.
- `heartbeat-managed-clone-credentials.test.ts` passes: an
authentication failure during checkout keeps its cause and is not
reported as a missing ref.
- Other checkout suites pass (9 files, 317 tests): reopen, base storage,
expiration, project env, rehome, nested clone migration, object cache,
trust, source copy privacy.
- Neighbor suites pass: `managed-repository-checkout.test.ts`,
`task-workspace-repositories.test.ts`,
`task-workspace-repositories-trust.test.ts`,
`heartbeat-project-repositories.test.ts` (47 tests). In one parallel run
under heavy machine load, `heartbeat-project-repositories.test.ts` had
timing failures in different tests on each run. The same file passed
alone with this change (47/47) and on master.
- `tsc --noEmit` for `server` passes.
## Risks
- Low risk. The new correction path is narrow and fails closed. Remote
(sandbox) workspaces keep the existing conflict because the control
plane cannot inspect the remote checkout.
- Existing receipts that failed before this change have
`repository_preparation_failed`. They need one more failed admission to
get `repository_ref_not_found` before a correction is accepted.
- No schema change. `failure_code` is free text.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, agentic coding with
tool use (shell, file edit, browser read-only 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
- [ ] 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
|
5723fd82ca |
fix(native): release a fresh claim when task preparation fails (#19065)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run claims its finalization coordinator before task preparation. Then it starts the provider turn. > - Task preparation can fail after the claim and before any provider turn. > - In that case the executor kept its claim. Failure settlement read the claim as a live owner and stopped. Nothing failed the run. > - The 60-second execution-control watchdog then failed the run as `execution_finalization_deadline_exceeded`. The original error was lost. > - This pull request releases only that exact fresh, unstarted claim after renewal stops. > - The benefit is that ordinary settlement fails the run at once with its real error and keeps the workspace files. The watchdog stays a backstop. ## Linked Issues or Issue Description Refs #19028. That PR keeps the earlier failure code when the watchdog fails a run. In this case no earlier code is saved, so it cannot help. This PR removes the wait for the watchdog. I searched open PRs for duplicates. #18820, #19034 and #19038 touch near code, but none release a preparation claim. **What happened?** A native task with a prepared sandbox workspace failed during preparation, after the controller claim committed. The coordinator stayed `observed` and kept this controller's lease owner. The heartbeat failure path calls workspace preservation. Preservation refuses any lease owner without a matching restart claim, so it raised `NativeWorkspaceFinalizationBusyError`. The heartbeat then treated ownership as held and returned without a terminal write. About 60 seconds later the watchdog failed the run with a generic "verify the provider stopped" action. The real error (for example `native_harness_storage_owner_missing`) stayed only in the workspace operation record. **Expected behavior** When no provider turn started under a fresh claim, settle the failure like a failure before the claim: retain the source, fail the run with its own error, and do not wait for the watchdog. **Steps to reproduce** 1. Start a native run with a prepared remote workspace. 2. Make task preparation fail after `native.coordinator.claim` commits (for example, a preparation callback throws). 3. See that the run stays `running` until the control deadline fails it as `execution_finalization_deadline_exceeded`. ## What Changed - `native-execution-lease.ts`: add `releaseUnstartedNativeExecutionClaim`. It clears the lease owner and expiry only for the exact run, company, issue, owner and attempt, while the phase is `observed` and no result exists. - `native-session-executor.ts`: when task preparation fails after a fresh claim, mark that claim. The startup owner releases it after renewal stops. A restart-recovery claim, a startup-cleanup context, Stop, restart detach, or a different owner keeps the old behavior. - Tests: an executor regression for the fresh and restart cases, and a real-database test for release and workspace retention against the exact owner, a foreign owner, a different attempt and an accepted result. - `doc/execution-semantics.md`: state that the deadline is a backstop and describe the release. ## Verification - Red before the fix: `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "releases only a fresh claim"` fails with "releaseUnstartedNativeExecutionClaim ... Number of calls: 0". - Green after the fix, with the same command. - `npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-failed-workspace-recovery.test.ts src/services/native-runtime/native-execution-lease.test.ts src/services/execution-control-reconciliation.test.ts` passes (embedded Postgres for the retention test). - `pnpm --filter @paperclipai/server typecheck` passes. - Not done: a live run against a real sandbox at this revision. The staging evidence is read-only. ## Risks - The release is fenced by the exact owner, attempt, `observed` phase and no result. Renewal stops before the release, so a renewal cannot see a missing owner and detach or cancel the run. - The fix covers failures inside task preparation only. Failures later in startup, or after a restart-recovery claim expires, still reach the watchdog. Those keep fail-closed behavior and need separate evidence. - If the release write fails, the run keeps the old watchdog path. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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 - [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> |
||
|
DottaandPaperclip
|
9cdbcbeddc |
fix(workspaces): restore archived commits from verified project objects (#19138)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - An ephemeral web server must preserve historical task files outside its local disk. > - Archived isolated Git workspaces can have a successful branch-deletion receipt. > - Their private checkout can be gone while the project cache still owns the exact commit. > - The importer rejects that cache because it applies private-checkout validation. > - This pull request validates the managed object source for receipt-based restoration. > - The importer restores the recorded commit without using the current shared working tree. ## Linked Issues or Issue Description Refs #18826, #18943, #19038. **What happened?** An archived isolated workspace has a valid successful cleanup receipt and a retained commit. Offline migration rejects it with `archived_git_source_unavailable,local_source_unsafe_or_missing`. The private checkout was removed. The company and project cache still contains the exact commit. **Expected behavior** Restore the immutable commit from the verified project object cache. Keep strict validation for saved private edit recipes. Reject missing receipts, a foreign cache, a different origin and revoked cleanup authority. **Steps to reproduce** 1. Create an isolated Git workspace from a managed project cache. 2. Archive it and retain its successful branch-deletion receipt. 3. Remove its private checkout while the verified project cache retains that commit. 4. Attempt offline workspace migration. ## What Changed - Extract the existing managed Git object-source validator. - Use that validator only for exact successful deletion receipts that refer to a canonical company and project cache. - Keep private restore-point source validation unchanged. - Fetch immutable objects at the recorded commit into private scratch. Recheck receipt authority before publication. - Add real Git and database tests for restoration and rejected authority changes. Document the preservation path. ## Verification - 129 focused migration, concurrency and isolated-source tests pass across three files. - The new restoration case fails against the previous importer with the reported source-unavailable error. - Tests remove the old home and restore the retained commit. A newer shared working tree and unrelated files are excluded. - Tests reject a foreign company cache, wrong origin, missing receipt and receipt revocation during upload. - Local server typecheck has no diagnostics in changed files. Stale linked dependency declarations prevent a clean local whole-server result. Current-head full CI passes (54 passing checks, three skipped). Greptile is 5/5 with no unresolved review threads. - A replay of the 32 affected records is running in an isolated staging copy. Two committed owner overlays are under test. This is not released-image or serving-instance acceptance. ## Risks - A deletion receipt must identify an exact commit that still exists. Missing objects continue to block migration. - Shared working-tree contents must never substitute for archived private files. Only the recorded commit is fetched through this path. - The change does not recreate private edit authority, retire recovery records or alter serving configuration. ## Model Used OpenAI GPT-6.1 Codex, exact model ID `gpt-6.1-sol`. Used for reasoning, code editing, shell tools and test execution. The runtime does not expose its 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
|
cda9460ae9 |
fix(native): restart remote bootstrap after a shutdown during preparation (#19056)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native runs in a remote sandbox go through workspace preparation
before the Runner process starts.
> - When the server restarts, restart recovery decides if a remote run
can safely restart its bootstrap.
> - A remote run that never started a Runner process, never had a lease
owner, and has no provider evidence can restart its bootstrap. Recovery
calls this an "unclaimed bootstrap".
> - A graceful shutdown (SIGTERM) suspends every running native
coordinator. It sets `leaseExpiresAt = now`, also when the coordinator
has no lease owner.
> - The unclaimed-bootstrap check requires `leaseExpiresAt` to be null.
So a deploy during workspace preparation turns a safe bootstrap restart
into `native_restart_recovery_blocked` /
`ambiguous_remote_recovery_evidence`. The task is blocked and a board
recovery action is created.
> - This pull request lets the check accept an elapsed expiry when the
coordinator has no lease owner.
> - The benefit is that a deploy during remote workspace preparation no
longer blocks the task. Every other contradictory-evidence guard stays
in force.
## Linked Issues or Issue Description
**What happened?**
On a staging instance, three remote-sandbox runs failed with
`native_restart_recovery_blocked` and
`ambiguous_remote_recovery_evidence` after one deploy. Each run was in
native workspace preparation (`snapshot.git`, `pack`, `seed.verify`,
`stage.workspace`). No run had a process start, a process identity, a
provider event, or a checkpoint. The graceful shutdown event was the
only coordinator change. The next server then blocked each run in
restart recovery.
**Expected behavior**
A remote run that was suspended during preparation, before any Runner
launch or lease claim, restarts its bootstrap on the same run. Recovery
does not block the task.
**Steps to reproduce**
1. Prepare a remote native run (prepared native workspace sync, fenced
preparation, no process, no checkpoint).
2. Call `drainRunningRunsForShutdown("SIGTERM")` for the run.
3. Call `claimNativeRestartRecoveries` as a successor controller with
`restartKind: "hard"`.
4. Observe `{ kind: "blocked", reason:
"ambiguous_remote_recovery_evidence" }`. Without step 2, the same run
gets `bootstrap_incomplete`.
**Paperclip version or commit**
Reproduced on `master` at `be20d4ad97`.
**Deployment mode**
Remote sandbox environments with PostgreSQL.
Related: #18877 (remote checkpoint retry reclaim) and #18854
(pre-provider startup retry). Neither changes this predicate.
## What Changed
- `claimNativeRestartRecoveries`: the unclaimed remote bootstrap check
accepts `leaseExpiresAt` when the coordinator has no lease owner and the
expiry has elapsed. A null owner with an elapsed expiry grants no
authority.
- New regression: a remote run suspended by the real graceful-shutdown
drain during preparation is claimed as `bootstrap_incomplete`, with no
recovery action.
- New safety case: an ownerless coordinator with an unexpired lease
expiry still blocks with `ambiguous_remote_recovery_evidence`.
## Verification
- Red before the fix: `npx vitest run
src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
-t "graceful shutdown suspended during preparation"` returned `blocked`
/ `ambiguous_remote_recovery_evidence`.
- Green after the fix: the same test returns `bootstrap_incomplete` with
`providerAttempt: 0` and `controllerGeneration: 1`.
- The existing contradictory-evidence cases (`claimed`, `generation`,
`process`, `launch`, `provider-event`, `checkpoint`,
`invalid-workspace`, `unfenced`) and the new `unexpired-lease` case
still block.
- Full `native-runner-restart-recovery.integration.test.ts` suite and
server `tsc --noEmit` pass locally.
- Not run: a live deploy on a remote sandbox. The fix is not deployed.
The original blocked tasks are not recovered by this change.
## Risks
- Low risk. The change relaxes one field of one predicate. All other
unclaimed-bootstrap conditions stay: prepared workspace, fenced
preparation, attempt 0, generation 0, no controller identity, no lease
owner, no process identity, no session, no checkpoint, no provider
evidence, and no launch or tool event.
- The executor still requires the durable PRP root to prove an
incomplete bootstrap before it starts a fresh bootstrap.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with
tool use (shell, file edits, browser reads of staging). Standard context
window.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandPaperclip
|
7d46532c6d |
fix(runtime): bound parallel offline harness backup transfers (#19108)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - An ephemeral web server must retain provider history outside its
local disk.
> - Offline enrollment moves verified local harness backups to object
storage.
> - A cold rehearsal reached 699 current backup rows while processing
1,953 sources before its 25-minute deadline.
> - The serial transfer loop prevents this existing dataset from
enrolling.
> - This pull request overlaps at most four transfers within the
existing total scratch limit.
> - The benefit is faster preservation with the same ownership and
publication checks.
## Linked Issues or Issue Description
Refs #19038, #18943, #18932.
**What happened?**
Offline enrollment processed 1,953 local provider backups one at a time.
The normal Cloud image reached 699 current backup rows (including nine
pre-existing rows) before maintenance expired. The serving instance
remained unchanged during the isolated test.
**Expected behavior**
Enrollment must preserve every source within bounded time and disk use.
It must stop all in-flight transfers before it reports failure.
**Steps to reproduce**
1. Prepare a settled instance with multiple verified native harness
backups.
2. Run offline enrollment against shared object storage.
3. Observe that the serial owner waits for each archive, upload,
readback and publication before starting the next source.
## What Changed
- Process groups of up to four verified offline harness backups.
- Reserve space for the archive and encrypted copy before each group
starts. Keep the total scratch limit equal to one maximum-size serial
transfer.
- Derive a smaller archive limit from a closed manifest directory set.
Preserve unlisted files with an exclusive maximum-size reservation.
- Enforce the reservation while streaming. Keep the existing maximum for
ordinary runtime backups.
- Cancel and await peer transfers after failure. Keep completed
publications valid for retry.
- Preserve the original archive-stream failure when the tar writer also
fails after destruction.
- Document the migration bound and add integration regressions for
overlap, restoration, cancellation, retry, extra files and archive
limits.
## Verification
- Serial baseline: the new overlap regression fails with one active
transfer when four are required.
- Candidate: 59 targeted tests pass with real PostgreSQL, S3, filesystem
archives and restored provider bytes. The bundled-entry test is skipped
locally because this source worktree has no runtime bundle. The full
normal image rehearsal remains required.
- Follow-up: 48 archive/storage checks pass after correcting the
PAX-header allowance. The real encoder produces a 972,801,024-byte
full-entry projection, above the old 805,306,368-byte allowance and
below the new 1,024,001,024-byte bound. The existing global object limit
remains in force. The old formula fails the new regression. Exact-size
restoration and one-byte-over-limit rejection pass.
- The previous revision passed full CI after one setup-route timeout
retry. That failure remains recorded; the corrected current revision
|
||
|
|
ea6bd15e2a |
fix(native): record why finalization failed a run (#19063)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - A native run finishes in two steps: the provider turn, then
finalization. Finalization saves the workspace and commits the result.
> - When finalization exhausts its retries, `recordRetryableFailure`
sets the run to `failed` and blocks the task.
> - That code writes the cause only to `resultJson.failureCode`. It does
not write the run's `errorCode` or `error`.
> - So the run list, inbox, and agent status show a failed run with no
reason. Operators cannot tell why the run failed.
> - This pull request writes the finalizer's terminal failure code and a
readable message to the run row when the finalizer fails the run.
> - The benefit is that every failed run now names its cause.
## Linked Issues or Issue Description
**What happened?**
A hosted instance had 11 assigned native runs with status `failed`,
`errorCode` null, and `error` null in one day. In each run, the provider
turn succeeded. The remote workspace copy-back then failed three times.
The finalizer set the run to `failed` and blocked the task. The run kept
no error code. `resultJson.failureCode` was
`native_workspace_sync_out_retry_exhausted`, but no screen shows that
field as the failure reason.
**Expected behavior**
A failed run has an `errorCode` and an `error`. The failure surfaces
show why the run failed.
**Steps to reproduce**
1. Create a native run with a finalization coordinator.
2. Call `recordNativeFinalizationFailure` three times with
`native_workspace_sync_out_failed` and `failureScope: "workspace"`.
3. Read the run. Its status is `failed`, and its `errorCode` and `error`
are null.
**Paperclip version or commit**
`master` at
|
||
|
DottaandPaperclip
|
818d2bb661 |
fix(native): preserve closed post-result failure diagnostics (#19126)
## Thinking Path > - Paperclip manages AI agents and their work. > - Native execution persists a result before it completes cleanup and workspace export. > - A later execution error enters the failure-write path. > - That write must reject an already selected result. The existing code reports lease loss for every rejected write. > - This can hide the category of the first error even when the same controller still owns an unexpired lease. > - This change adds closed diagnostic labels after exact ownership and accepted-result checks. It keeps the existing control error and recovery policy. ## Linked Issues or Issue Description **What happened?** A local real-PostgreSQL regression accepts a result through the production control plane, then injects a harmless filesystem failure. The production executor reports `native_session_lease_lost`. It loses the original failure category because the failure transition requires a null result ID. **Expected behavior** Keep the result guard and the existing lifecycle classification. Record a closed diagnostic category when the selected result is accepted and the controller still has exact, unexpired authority. Keep ordinary lease-loss fencing when that proof is absent or its diagnostic lock is busy. **Steps to reproduce** Run the new post-result cases in `server/src/services/heartbeat/runtime-execution.test.ts`. They use the real executor, control-plane result commit, heartbeat recovery path and PostgreSQL. The provider boundary is controlled. No live provider is used. **Paperclip version or commit** The regression fails on base `7d1e9b3477`. Related open PR #16917 changes legacy lease recovery. Open #19063 adds terminal finalizer reasons, and #15310 preserves database finalization errors. They do not change this executor diagnostic branch. This change only adds native diagnostic evidence and does not reclaim a lease. ## What Changed - Add a typed diagnostic with a closed stage, category and accepted-result flag. Do not retain the source message, path, arbitrary code or lease token. - After a rejected failure write, check the selected accepted result and the exact company, task, run, owner, attempt, generation, process identity and expiry. Use a coordinator-only `NOWAIT` lock. Fall back to the original control error if the diagnostic read fails. - Report the closed event after rollback. Keep `native_session_lease_lost` as the error message and leave every failure-write predicate unchanged. - Add real-PostgreSQL safety cases for stale and expired authority, owner replacement, Stop state, result selection, pending accounting, one provider invocation, closed labels and diagnostic sink failure. - Document the closed event, unchanged authority, and possible late diagnostic sink completion. - Add a competing-row-lock barrier. It verifies actual PostgreSQL lock refusal and control settlement before the competing lock is released. ## Verification - Permanent regression on unchanged base: RED because the closed diagnostic is absent. - NOWAIT mutation: RED when only NOWAIT is removed. PostgreSQL reports lock timeout instead of immediate lock refusal. The test uses barriers and the actual database error, with no elapsed-time assertion. - Final related suites: 56 tests passed across runtime execution and provider failure integration tests. - Full server source and test typecheck: passed. - Full `pnpm -r typecheck`: passed in an independent locked-dependency checkout. - Full `pnpm build`: passed. - Full `pnpm test:run`: running in the independent immutable checkout. All six chat shards passed; the remaining suites are still pending. - The first current-head CI attempt failed in two cases: workspace runtime initial fixture HTTP readiness and instructions-bundle error-message rendering. Both exact cases passed in isolation on unchanged PR head and base. The original causes remain unknown after bounded test-only observation. One controlled same-head rerun of each failed job passed. Current required CI and all other completed checks passed. Original failures remain recorded; no infrastructure or flaky-test claim is made. - Module boundary check and `git diff --check`: passed. - Exact-head Greptile: 5/5 with no actionable findings or open review threads. Automatic native review remains queued. That advisory check is not required by current master rules; no current-head native assessment is claimed. - Earlier fixture setup failures remain recorded. These included missing fixture fields and observing the Drizzle wrapper instead of its PostgreSQL cause. They are not the regression RED. An earlier broad typecheck was interrupted for the required NOWAIT revision; the final full typecheck passed. - No hosted export recovery or original hosted failure cause is claimed. ## Risks The extra read is diagnostic only. It uses NOWAIT and cannot wait for a competing coordinator lock. Its failure preserves the existing control classification. The event sink uses the existing 60-second execution-control bound. A slow sink can finish later and emit a late event. Result selection, Stop, accounting, retry eligibility and provider work do not depend on event success. No schema or lifecycle contract changes are required. ## Model Used OpenAI GPT-6.1, model ID `gpt-6.1-sol`, with reasoning, tool use and code execution. The host did 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
0e27369fa4 |
fix(native): restart sandbox prep interrupted before its sync descriptor (#19054)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs on sandbox environments sync the host workspace into the
sandbox before the provider starts
> - Runtime selection writes the native execution input first. It writes
the workspace sync reference only after the sandbox is seeded, and
before any provider dispatch
> - A server restart between those two writes leaves a run with an input
but no sync reference
> - Restart recovery then fails the run during workspace preparation
with `native_workspace_sync_descriptor_binding_missing`, although the
sandbox never received any work from that run
> - This pull request lets recovery re-admit the host workspace in that
state, so sync preparation starts fresh on the same run
> - The benefit is that a deploy or restart during sandbox preparation
no longer turns queued work into a terminal `setup_failed` run
## Linked Issues or Issue Description
**What happened?**
After a server restart, six native sandbox runs failed with
`setup_failed` and the error
`native_workspace_sync_descriptor_binding_missing`. Each run timeline
shows the same order: run started, `snapshot.git`, `snapshot.baseline`,
then "Server shutdown suspended native controller ownership", then a
restart recovery claim ("Restarting an incomplete native runner
bootstrap" or "Resuming the durable native provider session"), then the
error. No run had a `nativeWorkspaceSync` reference. Each run had a
persisted `nativeExecutionInput`.
**Expected behavior**
When sandbox sync preparation did not finish, restart recovery prepares
the sandbox workspace again from the host workspace and continues the
run.
**Steps to reproduce**
1. Start a native run on a sandbox environment.
2. Restart the server after runtime selection persists
`nativeExecutionInput` but before `prepareNativeWorkspaceSync` persists
its reference.
3. Restart recovery re-enters workspace preparation and fails with
`native_workspace_sync_descriptor_binding_missing`.
**Paperclip version or commit**
Reproduced on `master` at
|
||
|
|
35d086213e |
fix(native): explain failed accepted results on the run (#19091)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run ends when the server projects the accepted result onto the run row. > - Operators read `errorCode` and `error` on a failed run to learn why it failed. > - Three native writers can set a run to `failed` without these fields. #19063 repairs the finalizer retry-exhaustion writer. > - The two result-projection writers still set `failed` with no cause when the provider failure is not classified. > - This pull request makes both projection writers record a cause when they project `failed`. > - The benefit is that, with #19063, no native writer can leave a failed run unexplained. ## Linked Issues or Issue Description Refs #19063. **What happened?** A staging soak found 11 failed native runs with `errorCode` null and `error` null. In each run the provider turn succeeded. Workspace copy-back then failed three times, and the finalizer retry-exhaustion path set the run to `failed`. #19063 fixes that writer. An audit of every `heartbeat_runs` status writer found two more writers with the same defect. `finalizeNativeRun` and `projectCommittedRun` project an accepted result with `runTerminalState: "failed"`. They write the error fields only when a classified provider failure exists. **Expected behavior** A run with status `failed` always has an `errorCode` and an `error`. A cause that is already recorded stays. **Steps to reproduce** 1. Accept a native result with `runTerminalState: "failed"` and no `turn.failed` event. 2. Call `finalizeNativeRun` with `projectRunStatus: true`, or project the committed run through `projectCommittedRun`. 3. Read the run. On master its status is `failed`, and its `errorCode` and `error` are null. ## What Changed - Add `explainFailedRun` in `server/src/services/native-runtime/native-run-failure-explanation.ts`. It builds SQL SET expressions. They keep a non-empty existing `errorCode` or `error`. Otherwise they write `native_run_failed` and a readable message. By default the expressions leave a run that is already `cancelled` unchanged, as the status writers do. - `finalizeNativeRun` uses the helper when it projects `failed`. The existing provider-failure and recovered-failure values remain the preferred causes. - `projectCommittedRun` uses the helper for a `failed` accepted result. This writer sets status unconditionally, so it does not use the cancelled branch. - Writer audit: all legacy and heartbeat callers of `setRunStatus*` that pass `failed` already set both fields. The other direct native writers also set both fields. ## Verification - Red, with the source change stashed: 3 new cases failed with `errorCode: null`. These were the finalizer projection, recovery projection with no cause, and recovery projection with an empty error. The control case with a recorded cause passed. - Green: `npx vitest run src/__tests__/native-run-projection-work.test.ts src/services/native-runtime/native-run-finalizer-telemetry.test.ts src/__tests__/native-status-arbiter-corpus.test.ts` (225 passed). - Wider run: all `server/src/__tests__/native-*.test.ts` and `server/src/services/native-runtime/*.test.ts` (7416 passed). Two failures come from the local environment. `plugin-runner-probe` needs the Rust runner source. `native-ingress-bootstrap-retry` needs the controller-host OpenCode proxy. - This change was not exercised on staging. It changes only fields that are written at terminal projection. ## Risks - Low risk. The change affects only rows that are projected to `failed`. A recorded cause is never replaced. Succeeded and cancelled projections are unchanged. - The staging failures need #19063 too. This PR alone does not change their writer. - The cause behind the staging copy-back failures is still unknown. The sandbox command reports the redacted `remote_workspace_operation_failed`, which also appears in a separate signature. This PR does not change that diagnostic. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, browser reads of staging run records). ## 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: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
1a7eb70a37 |
fix(run-dispatch): save a redirect sent while a stopped run settles (#19122)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A user can stop a running agent and then send a new message that
redirects the work
> - The stopped run keeps settling for some seconds after Stop (backup
and checkpoint persist)
> - A message sent in that window is queued as a normal run before
settlement records the workspace recovery hold
> - The dispatch gate then withholds the unstarted run, but it saved the
message only for sources with a native workspace-sync baseline
> - For other sources (remote workspace, snapshot-only) it marked the
wake `skipped`, so the message was lost and the task stayed in progress
with no run and no wake
> - This pull request saves the withheld message for any source when the
message was written after the Stop
> - The benefit is that a redirect after Stop is acted on exactly once
instead of disappearing
## Linked Issues or Issue Description
**What happened?**
After Stop, a user posted a redirect comment with resume intent. The
follow-up run was created while the stopped run was still persisting its
checkpoint. When settlement finished, the dispatch gate cancelled the
follow-up run with `execution_reconciliation_required` and an
empty-looking error ("Recovering saved work…"). The user's wake was
marked `skipped`. Nothing woke the task again. It stayed `in_progress`
with no active or queued run.
**Expected behavior**
The message sent after Stop runs exactly once after the stopped run
settles. The task never rests in progress with no run and no pending
wake.
**Steps to reproduce**
1. Start a native run whose stopped source has no native workspace-sync
baseline (for example a remote-workspace sandbox run).
2. Stop the run.
3. Post a comment with resume intent while the stopped run still
persists its checkpoint.
4. Let settlement record the workspace recovery hold.
5. Observe that the queued follow-up run is cancelled with
`execution_reconciliation_required` and its wake is `skipped`.
**Root cause**
`deferUnstartedUserComment` in
`server/src/modules/run-dispatch/adapters/postgres.ts` saved a withheld
user comment only when the stopped source had a native workspace-sync
baseline (the copyback replay case). The enqueue path saves or admits
the same message when it arrives after the hold exists. The gap was only
timing.
Refs #18332 (added the native-baseline-only deferral this PR widens).
**Paperclip version or commit**
Reproduced on `master` at
|
||
|
DottaandPaperclip
|
b7ef307a65 |
fix(workspaces): wait for an ended writer's save instead of failing setup (#19074)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs can use a remote task workspace in a sandbox. One writer run at a time holds that workspace. > - When a writer run ends, its workspace save and sandbox release finish a few minutes later. The writer hold stays until then. > - A new run that started in that interval failed with `setup_failed`: "The workspace still has an unfinished writer or save operation." > - A live writer already makes the new run wait (`workspace_busy`, added in #18691). An ended writer that is still settling did not. > - This pull request reports the ended writer as the holder too, so the new run waits instead of failing. > - The benefit is that work continues automatically after a short settle interval. Users do not see a failed run and a blocked task. ## Linked Issues or Issue Description Refs #18691 **What happened?** A task run failed during workspace setup with `setup_failed` and the error "The workspace still has an unfinished writer or save operation." The previous run on the same task had ended a few seconds before. Its sandbox lease was released 4 to 17 minutes later. A staging soak recorded 31 of these failures over 14 tasks in about 29 hours. **What did you expect to happen?** The new run waits for the workspace, as it does when the previous writer is still running. It continues when the save and release settle. **Steps to reproduce** 1. Bind a task to a remote workspace whose authority has `dirty: true` and a `writerRunId` for a run with status `failed` that ended now. 2. Wake the task's agent. 3. The run fails with `setup_failed`. No retry is scheduled. ## What Changed - `reserveRemoteWorkspace` now throws `RemoteWorkspaceWriterActiveError` for an ended writer whose save or sandbox release has not settled. Before, it threw a plain 409 conflict. The error keeps the same message, status and `remote_workspace_writer_pending` code. It uses the writer's `finishedAt` as its last activity and sets `ended: true`. - Workspace preparation already converts that error to a `workspace_busy` deferral with the one-hour holder cutoff. It now passes `holderEnded` to the deferral. - `WorkspaceBusyDeferral` says "ended, but its workspace save has not settled" for an ended holder, instead of "is still running". - A writer that is missing or has no task still gets the plain conflict, as before. - `doc/execution-semantics.md` describes the wait and its cutoff. ## Verification - Red before the fix: the new test "waits for a writer that just ended while its save or sandbox release settles" failed. The run had `status: failed`, `errorCode: setup_failed`, `executionRecovery.kind: bootstrap`. This is the same result as the staging failures. - Green after the fix: `cd server && npx vitest run src/__tests__/heartbeat-remote-workspace-busy.test.ts src/services/remote-workspace-reservation.test.ts` (16 tests pass.) - Safety cases in the new tests: - The waiting run does not change the workspace authority. The unsaved writer's files and hold stay unchanged. - A writer that ended more than one hour ago still causes the unfinished-writer setup failure, with no scheduled retry. The wait is bounded. - Existing cases still pass: live writer wait with Stop and reassignment, the concurrent reservation race (`remote_workspace_writer_pending`), and the unused-reservation takeover. - Also run: `remote-workspaces.integration.test.ts`, `heartbeat-workspace-busy.test.ts`, `remote-workspace-save-concurrency.test.ts`, `workspace-recovery.test.ts`. All 6 suites pass (442 tests). - Not done: a live run on staging at this revision. The fix is not deployed. ## Risks - A task whose ended writer never settles now makes up to about one hour of short `workspace_busy` waits (one scheduled retry every 1 to 2 minutes) before it fails. Before, it failed at once. The end state after the cutoff is the same. - Some of the staging failures have other causes that this change does not fix: - A previous run that stays `running` with no output for more than one hour still causes this failure, as intended. - A dirty workspace whose writer ended without a saved result (for example a workspace finalization retry that never runs) still blocks the task after the cutoff. These need separate fixes. - No schema, API or UI changes. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, browser reads of staging run records) and extended reasoning. ## 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: Paperclip <noreply@paperclip.ing> |
||
|
|
3634c4a845 |
fix(chat): log the underlying error when a publication becomes delivery_unknown (#18875)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat connectors (Slack and others) publish agent messages to external conversations through the publication worker > - When the provider accepts a message but the local commit after the send fails, the publication becomes `delivery_unknown` > - A `delivery_unknown` row blocks the later publications in the same conversation until an operator resolves it > - The log line for this case contains only the generic reason. It drops the error that the post-send commit threw > - Operators cannot see why the commit failed, so they cannot tell a race from a lease loss or another cause > - This pull request adds the underlying error, redacted and length-limited, to the existing warning for this case only > - The benefit is that operators and maintainers can find the real cause of stalled chat threads ## Linked Issues or Issue Description Refs #18874 I searched open and closed issues and pull requests for `delivery_unknown`, Slack publication failures and "could not confirm its durable result". I found no duplicate. #18874 describes the bug and the proposed behavior fix. This pull request does not change behavior. It only makes the cause visible. ## What Changed - `server/src/services/chat-channels/publications/failures.ts`: when `providerAccepted` is true, the `chat publication failed` warning gets a `cause` field. The field contains the underlying error message, redacted with `redactSensitiveText` and limited to `MAX_ERROR_TEXT`, like the existing `error` field. - The stored `redactedError`, the disposition and the operator resolution flow do not change. - `server/src/__tests__/chat-run-milestone-work.test.ts`: the existing `rechecks resolved prompt ownership after delivery ...` test now spies on `logger.warn`. It asserts that `delivery_unknown` logs the `cause`, and that `retry` and `failed` do not log it. ## Verification - Ran in a `node:24-bookworm-slim` container on this branch: - `npx vitest run src/__tests__/chat-run-milestone-work.test.ts` (in `server/`): 72 passed. - `pnpm run typecheck` (in `server/`): passed. - Negative check: I removed only the new `cause` field and ran the test again. The two `delivery_unknown` cases failed. The `retry` and `failed` cases passed. ## Risks - Low risk. The change adds one field to one warning log line, only for publications that the provider already accepted. - The new field uses the same redaction and length limit as the existing `error` field, so it does not add a new path for secrets into the log. > 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`), 1M context window, in Claude Code with tool use (shell, file edits, code search). A human reviewed and requested the change. ## 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 - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: lyuboslavmedarov <285605601+lyuboslavmedarov@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Svetlana ZolotenkovaandClaude Opus 5.5
|
bce9551a36 |
fix(recovery): skip successful-run handoff for routine executions (#9129)
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies, and the
issue board is the surface those agents work from
> - Recovery is the subsystem that notices when an agent run finished
but left no next step, and hands the issue back so the work is not
silently dropped
> - `decideSuccessfulRunHandoff` implements that check: after a
successful run it looks for a valid disposition on the issue and
enqueues a corrective handoff wake when it finds none
> - Routine execution issues (`originKind = "routine_execution"`)
deliberately stay `in_progress` between cron fires — one open issue is
coalesced across dispatches through the
`issues_open_routine_execution_uq` unique index
> - Recovery read that intended steady state as a *missing* disposition,
so every successful routine run enqueued a handoff wake, exhausted its
single allowed attempt, and left the issue `blocked` with a
`source_scoped_recovery_action` that re-woke the assigned agent
> - This pull request exempts routine executions from the successful-run
handoff, mirroring the existing issue-monitor maintenance skip
(`isIssueMonitorMaintenanceRun`)
> - The benefit is that routines stop generating self-inflicted recovery
churn while genuine stranded issues keep being caught
## Issue
_No public issue exists for this yet; describing it inline per
CONTRIBUTING.md ("Link Issues or Describe Them In-PR"), following the
bug report template._
**What happened?**
After every successful run of a routine execution issue, recovery
enqueued a corrective handoff wake because the issue had no terminal
disposition. The single allowed handoff attempt was then exhausted and
the issue landed in `blocked` with an active
`source_scoped_recovery_action`, which re-woke the assigned agent.
Observed in production as roughly 27 runs per hour on a single
orphan-check routine, which additionally tripped a false productivity
review on the assigned agent.
**Expected behavior**
A successful routine run that leaves the routine execution issue
`in_progress` is the intended waiting state. Recovery should skip the
successful-run handoff for `routine_execution` issues and let the
routine lifecycle own their disposition.
**Steps to reproduce**
1. Create a routine that dispatches an execution issue on a short cron
interval.
2. Let one dispatched run complete successfully without writing a
terminal status on the routine execution issue.
3. Observe the corrective handoff wake, the exhausted attempt, and the
issue moving to `blocked` with an active source-scoped recovery action.
4. Repeat on the next cron fire — the cycle restarts and the run rate
climbs.
**Paperclip version or commit**
`master` at the time this branch was cut; the guard is absent on current
`master`.
**Deployment mode / adapters / database**
Adapter-independent — the path is server-side in
`server/src/services/recovery/successful-run-handoff.ts`. Covered in
tests against embedded Postgres.
## What Changed
- `decideSuccessfulRunHandoff` returns `{ kind: "skip", reason: "routine
execution owns its own disposition" }` when `issue.originKind ===
"routine_execution"`, alongside the existing issue-monitor maintenance
skip.
- `originKind` was added to the handoff `IssueRow` projection and to the
issue select in `handleSuccessfulRunHandoff` in the heartbeat service,
so the guard has the field it needs.
- Added a regression test in `successful-run-handoff.test.ts` asserting
that a `routine_execution` issue is skipped rather than enqueued.
**Related PRs** (dedup search):
[#8622](https://github.com/paperclipai/paperclip/pull/8622) touches the
same routine-execution uniqueness index from the migration side;
[#6527](https://github.com/paperclipai/paperclip/pull/6527)
auto-resolves stale source recovery actions, which is the symptom this
PR stops producing. No PR was found that implements this guard.
## Verification
- `cd server && pnpm exec vitest run src/services/recovery --config
vitest.config.ts` — recovery suites pass.
- `cd server && pnpm exec vitest run
src/__tests__/heartbeat-process-recovery.test.ts --config
vitest.config.ts` — passes; 59 tests across both suites.
- Regression check: with the `routine_execution` guard removed, the new
test falls through to `enqueue` and fails; with the guard it passes.
- `pnpm --filter @paperclipai/server typecheck` — clean.
## Risks
- Low risk, behavioural only. No schema change and no migration in this
change.
- The guard is narrow: it keys on `originKind === "routine_execution"`,
so ordinary assigned issues keep their existing successful-run handoff
behaviour unchanged.
- If a routine execution issue ever did need corrective handoff, it
would now be skipped. That trade-off is deliberate — the routine
lifecycle already owns these issues, and the coalescing unique index
guarantees at most one open issue per routine.
## Model Used
- Claude (Anthropic), Claude Opus 4.6 via the Claude Code agent harness,
extended thinking enabled, with tool use (file edits, shell, ripgrep)
against a local checkout.
## 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)
- [ ] 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
---
**Review request (2026-08-19).** Rechecked against `master`
|
||
|
Nicky LeachandClaude Opus 5.5
|
aa8372fd5d |
fix(accounting): post late usage to its owning run (#18991)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner sessions keep one provider conversation across task turns, so Paperclip accounts usage per run. > - Codex can report more usage for a turn after its run's charge was posted. > - The Runner then marked the session `reconciliation_required`. If another run was attached, it also failed that run. Core refused the delivery with `native_usage_after_finalization_requires_reconciliation`. > - That usage has already happened. Blocking the session cannot undo it, and the debt waited for an operator while the run totals stayed wrong. > - Codex totals are cumulative, and the Runner already knows the owning run in most cases. So the correct amount can be posted automatically. > - This pull request reports the exact increment with its owner, and posts it to the owning run as a separate cost event. > - The benefit is that run accounting becomes correct without stopping work, and an old run's usage can no longer fail a newer run. ## Linked Issues or Issue Description No issue exists. I searched open and closed pull requests and issues for "late usage", `native_usage_after_finalization_requires_reconciliation` and `codex_usage_accounting_integrity`. I found no duplicate. **What happened?** Code review found this while diagnosing session recovery failures. When Codex reported usage above the known total for a turn that was not active, the Runner blocked the session. If that turn belonged to a previous run, the Runner also failed the attached run with `codex_usage_accounting_integrity`. Core kept the receipt as debt, and could correct it automatically only if the token counts were unchanged. **Expected behavior** Late usage does not stop any run or session. The charge goes to the run that used the tokens, and it is auditable. **Steps to reproduce** 1. Finish a Codex turn on a native Runner session and attach the next run. 2. Have Codex send `thread/tokenUsage/updated` with a higher total for the finished turn. 3. On the unfixed code, the attached run fails and later attaches are refused. `late_codex_usage_requires_durable_reconciliation_before_or_after_handoff` encoded this behavior. **Paperclip version or commit** `master` at `04a24f4e06`. **Deployment mode** Any deployment that uses a native Runner with Codex. ## What Changed - **Runner:** late usage no longer sets `reconciliation_required` or fails a run. For a turn that belongs to a previous run, the Runner emits `usage.reported` with the increment in `runDelta` and the owner in `reconciliation.priorOrigin`. `runDeltaAvailable` stays `false`, so an older Core does not bill it to the attached run. - **`CodexRunUsage::absorb_late_usage`:** treats only usage above the known thread total as late, and keeps the attached run's delta unchanged. A report at or below that total adds nothing, so the sum across runs always equals the thread total and a replay cannot charge twice. - **Delivery:** `captureLateUsageForOwner` stores the increment as a late receipt on the owning run. The owner must be a finished run in the same company and native session. If the Runner cannot name the owner, Core uses the session's last finished run and records the attribution as `inferred`. - **Local Runner:** late usage stays with the run that is active when it arrives, or the next run. Usage reported between two runs stays in the next run's delta, so the total stays exact. - **Accounting:** `reconcileLateRunUsageReceiptInTransaction` posts a token increase as a new cost event with the key `heartbeat:<run>:late:<receipt>`. It updates the run totals so the accounting integrity check still agrees, records `cost.late_usage_posted` in the activity log, and queues budget evaluation. - **No more refusal:** `persistUsageReceipt` no longer throws after it saves late evidence, so late usage cannot wedge a Runner outbox. - **Covered receipts:** an older same-source receipt at or below the accepted totals, with matching billing, is cleared because a newer cumulative receipt already covers it. Conflicting billing stays as debt. - **Older Runners:** a zero-filled invalidation stays on the delivering run. - **Unchanged:** a newer lower total, a changed model or billing type, and per-model receipts stay as visible accounting debt for an operator. - **Unchanged:** an unexpected provider message during the warm attachment drain still fails closed. A separate change addresses that gate. - **Docs:** added "Late provider usage" to `doc/execution-semantics.md`. ## Verification - `cd packages/paperclip-runner/runner && cargo test --locked -p paperclip-runner-core --test codex_provider`: 99 passed, 2 ignored. - `cargo test --locked -p paperclip-runner-core --lib codex_usage`: 7 passed. This includes new tests for the out-of-order and repeated report cases. - `cd server && pnpm exec vitest run src/services/run-cost-accounting.test.ts src/services/native-runtime/native-usage-delivery.integration.test.ts src/services/native-runtime/native-recovery-usage-delivery.test.ts src/__tests__/cost-data-quality.test.ts`: passed against embedded PostgreSQL before the last review fixes, and the accounting and delivery files were re-run after them. - `npx tsc --noEmit -p .` in `server` reports no errors. - New tests cover exact and inferred owners, repeated delivery, late usage that arrives before the owner's charge posts, another session's run, an older Runner's report with no increment, a same-run increase, and a lower total. They also run the accounting integrity check after posting. - **Review:** Greptile Apex scored 1/5, 4/5, 3/5, 2/5 and 4/5 while findings were fixed, then 5/5 at the current head. - **Local Runner:** driver tests cover usage reported between two runs, including across a restore. No end-to-end local run has exercised it. - **Not run locally:** the full test suite. CI provides that coverage. - **Not verified live:** a read-only check of a test deployment found no late receipts in its history, so this path has only run against fixtures. ## Risks - **Run totals can rise after a run finishes.** This is deliberate. The correction is a separate cost event and an activity log entry. - **Budgets are corrected after the fact.** A late charge can put an agent over budget after later work has started. Budget evaluation runs after each correction, but it cannot prevent that work. - **Local sessions accept imperfect attribution by design.** Their late usage can land on the active or next run, because local sessions have no durable path for routing it. This also applies after a Stop or a pause. The total stays exact. - **Out-of-order reports can leave tokens on the wrong run in the session.** If a prior turn's report arrives after the attached run has already counted past that total, those tokens stay on the attached run. Moving them would need a lower correction to the attached run. The total stays exact. - **Inferred attribution can name the wrong run in the session.** The usage still belongs to that session and the total stays correct. The attribution is recorded as `inferred`. - **A session with no finished run charges the delivering run,** as an inferred late charge after its own charge posts. - **Rollout order:** deploy Core before the Runner. An older Core would keep this event on the attached run as incomplete usage, and would still refuse a same-run late receipt. - **Migrations:** this change has no migration. > 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 reworked. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), through Claude Code, with tool use and code execution. A human directed the design and approved the scope. ## 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 LeachandClaude Opus 5.5
|
2c39a2866b |
feat(plugins): let managed config grant worker variables to a plugin (#19049)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed instance gets its plugins from the control plane, and people do not install them > - Some of these plugins need deployment values, such as a service origin and a capability > - The plugin worker environment is scrubbed, and only a fixed list of first-party providers gets deployment variables > - Core also sends lifecycle events and project instance secrets only to plugins with a saved config row > - So a provisioned plugin cannot work until a person saves configuration for it > - This pull request lets the managed config grant named variables to a selected plugin > - The benefit is that a distribution plugin can run with no tenant configuration, and Core does not need to name it ## Linked Issues or Issue Description No issue exists. This describes the problem. I searched open pull requests for related work and found no duplicate. **What existing behavior does this improve?** How a plugin provisioned through `plugins.autoInstall` receives its deployment settings. **Current behavior** `buildPluginWorkerEnv` forwards variables only from a fixed list that names first-party packages. A distribution plugin cannot be on that list without Core naming it. A plugin with no `pluginConfig` row gets no resource lifecycle events, and project instance secrets refuse it. **Proposed behavior** The managed config can include `plugins.workerEnv`. It maps a catalog key to variable names. The worker of that plugin receives those variables from the server process environment. The managed config can also include `plugins.enabledCompanies`. It maps a catalog key to company IDs. In those companies, the plugin gets lifecycle events and can use project instance secrets with no config row. **Reason and benefit** The control plane stays the only place that decides which plugin gets which values. Tenants do not handle these values, and they are not in the database or in the managed config document. **Alternatives considered** - Create a config or enablement row for each company. The control plane would need a new tenant write API, and a row can stay after the document removes access or return after a database restore. The document is never saved, like the other managed settings. - Give a grant access to every company. This has no per-company opt-in. - Put the values in the managed config document. The document does not hold secrets, and this rule stays true because it holds only names. ## What Changed - `parseManagedConfigEnv` accepts an optional `plugins.workerEnv` object. Each key must be in `plugins.autoInstall`. Names must be uppercase variable names, with no duplicates. - Startup fails if a name looks like a host credential, such as a name with `KEY`, `TOKEN`, `SECRET`, `PASSWORD`, `DATABASE` or `JWT` as a word, or if it controls how the worker loads code, such as `NODE_OPTIONS`. - `parseManagedConfigEnv` accepts an optional `plugins.enabledCompanies` object. Each key must be in `plugins.autoInstall`. Values must be lowercase company UUIDs, with no duplicates. - `managedPluginWorkerEnvGrant` and `managedPluginEnabledCompanies` return values by plugin key. A managed instance only starts a selected plugin from the current image, so the saved package path is not part of this check. It can be old or null. - `buildPluginWorkerEnv` forwards granted variables. This does not depend on the `environment.drivers.register` capability. - Plugin lifecycle delivery includes a plugin with no config row only for the companies the document names. It still skips a company that disabled the plugin. The runtime event listener follows the same rule. - Project instance secrets accept a plugin with no config row only for a company the document names. - A variable grant alone gives no company access. - `doc/plugins/DISTRIBUTION-PLUGINS.md` describes both sections. ## Verification - `npx vitest run` in `server/` on `managed-config`, `managed-plugin-worker-env`, `plugin-lifecycle-delivery`, `project-instance-secrets`, `plugin-database` and `bundled-plugins`: 201 tests pass. - The new tests cover a digest-verified distribution plugin, a stale saved package path, a catalog key named after an inherited property, a company the document does not name, a company that disabled the plugin, a named company that does not exist, a grant with no enabled company, and host credential names. - `tsc --noEmit` in `server/` shows no errors in the changed files. My worktree does not have the built `@paperclipai/paperclip-runner` package, so unrelated files show errors locally. CI runs the full typecheck. - I did not run the full test suite locally. CI runs it. - I did not test this change on a running instance. ## Risks - **Older builds:** a build without this change refuses a document with `plugins.workerEnv` or `plugins.enabledCompanies`, because it fails closed on unknown keys. A control plane must send these sections only to a build that supports them. - **Trust:** the granted plugin gets the variable values in its process. The image owner and the control plane already trust this plugin. The name check stops a mistake with host credentials, but a name check cannot prove that a value is not sensitive. Core cannot use an allowlist, because it must not name the variables of a private plugin. - **Company scope:** the control plane sets which companies a plugin serves. A new company needs a new document, and the server reads the document when it starts. Self-hosted instances do not change. - **Rotation:** a new value reaches the worker only when the server restarts. Rotation without a restart is not part of this change. - **Background calls:** a grant does not add proactive company access. A plugin that makes background calls with no invocation still needs a saved config for that company. - This changes core plugin behavior, and I did not discuss it in `#dev` first. Maintainers can tell me if they want a discussion before review. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), through Claude Code, with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Devin FoleyandPaperclip
|
c7390bef20 |
fix(server): stop one slow copy-back from starving workspace retries (#19111)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run can accept its result before the workspace copy-back finishes. A background worker retries a failed copy-back from the retained sandbox without another provider turn. > - Until that retry succeeds, the run stays `running` and keeps its task's execution lock. User answers and wakes for that task stay queued. > - The worker checked one entry at a time (`batchSize: 1`, no concurrent checks). A slow sandbox export held every other task's due retry for the whole export. > - Work without a saved retry time used a deadline of `0`. Copy-backs that a restart interrupted always sorted ahead of retries that were due hours earlier. > - On a busy server that restarts often, scheduled retries never got to the front of the queue. Their tasks stayed stuck. > - This pull request runs up to four checks at once, one per task, and orders pending work by when it became pending. > - The benefit is that one slow export cannot stall unrelated tasks, and an old retry runs before newer work. ## Linked Issues or Issue Description Refs #17734 (added this worker), #18421 (the same concurrency pattern for native reply recovery) and #19063 (open; records why finalization failed a run). This pull request does not change their behavior. **What happened?** On a hosted instance, sandbox exports were slow for a few hours. A harness-state save timed out after about 5.5 minutes in 27 runs, and worker copy-back attempts took 2 to 34 minutes each. Eleven native runs then waited in `retryable_failure` (or `workspace_finalizing` with an expired lease) for more than three hours. Each one had exactly one copy-back attempt and no retry. The worker's operation receipts show its attempts ran strictly one after another. After each restart, the worker first took the copy-backs that the restart interrupted. They have no `nextAttemptAt`, so their deadline was `0`. The server restarted every 30 to 40 minutes, so the overdue retries never ran. One stuck run held a task whose user had answered a question card. The answer's wake stayed queued for the same three hours. **Expected behavior** A slow copy-back for one task does not delay due copy-backs for other tasks. Due work runs oldest first. New work without a saved retry time does not go ahead of older due retries. **Steps to reproduce** 1. Create two native runs with accepted results on different tasks. Put both in `retryable_failure` with an overdue `nextAttemptAt`. The first is older. 2. Make the copy-back for the older run block. 3. Start the worker. On `master` the second task's copy-back never runs while the first one blocks. The new worker test runs these steps against PostgreSQL. **Paperclip version or commit** `master` at `69dea878c0b`. **Deployment mode** Any server with native runs that use remote sandbox workspaces. ## What Changed - `server/src/services/native-runtime/native-workspace-finalization-work.ts`: - Remove `batchSize: 1`. Add `concurrentChecks: { limit: 4 }` keyed by `issue:<company>:<issue>`. Runs that share a task keep their copy-backs serialized. Ownership is still checked per run by the existing advisory lock and owner receipt. - Work without a saved retry time uses the coordinator's `updatedAt` as its deadline, not `0`. The value is clamped to now, because the database clock can be slightly ahead of the process clock. Saved retry times and lease expiry behave as before. - `doc/native-workspace-finalization-recovery.md`: one paragraph on worker concurrency and ordering. ## Verification - Failing first: "retries another task while an older copy-back is still running". On `master` the second task's copy-back never ran while the older one blocked (the test timed out after 15 s). Now the second task finalizes while the first is still blocked. After release, the first one also finalizes. - Failing first: "orders work without a saved retry behind older overdue retries". On `master` the interrupted copy-back had deadline `0` and sorted first. Now it sorts after the older overdue retry, and its deadline is not in the future. - `cd server && npx vitest run src/__tests__/native-workspace-finalization-work.test.ts src/__tests__/scoped-deadline-work.test.ts src/__tests__/server-startup-feedback-export.test.ts`: 3 files, 159 passed. After rebasing onto `69dea878c0b`, the first two files: 38 passed. - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Not verified: the worker on a live instance under slow exports. The diagnosis comes from that instance's operation receipts and run events. ## Risks - Up to four copy-backs can now run at once on one server. Each one uses its own dedicated database connection and sandbox transfer. Live runs already finalize in parallel, so this is within the existing load pattern. - The batch size is now the default (25). A pass ends when its slowest check ends, but the other lanes keep processing the rest of the batch. - `updatedAt` changes on any coordinator write. A pending entry with new writes moves later in the order, but it stays due. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use (shell, file edits, test runs, read-only database and log queries). The runtime does not expose the context window or reasoning configuration. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
7d1e9b3477 |
fix(recovery): retain exact warm native export sources (#17916)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native task recovery must save unfinished workspace exports before continuation. > - A prepared export can still refer to a sandbox with a warm reuse policy. > - The strict recovery inspector rejects that policy and exhausts source verification. > - Ordinary reuse must not give the same unfinished allocation to another run. > - An existing user Retry can authorize bounded file maintenance after exact ownership checks. > - This change retains that exact source and preserves the original failure record. > - Recovery completes only when files, stop evidence and policy settlement agree. ## Linked Issues or Issue Description **What happened?** A failed native run has a prepared `keep_running` workspace export. Its exact sandbox lease still has `reuse_by_environment` policy. Native export cleanup can preserve that policy while it stops the sandbox. Generic workspace recovery then rejects the source with `lease_policy_not_retained`. A saved user Retry remains blocked before file export. **Expected behavior** Keep unfinished exports out of ordinary warm reuse. With current user authority and an unchanged saved Retry, retain the exact source for one bounded maintenance generation. Preserve files and return continuation to normal admission. Keep all ambiguous sources held. **Steps to reproduce** 1. Prepare a native workspace export with a warm sandbox and `keep_running` disposition. 2. Save a normal board Retry for the failed source run. 3. Persist its first exhausted source-verification failure before file export. 4. Run the normal maintenance worker. Before this change, source policy rejection leaves the saved files unrestored. Refs #17773 for source diagnostics. Related #17839 records a missing preservation obligation during blocked restart. This change handles an existing prepared warm export whose verification already failed. ## What Changed - Exclude unfinished physical allocations from reuse and obsolete-lease cleanup across every historical lease row. Reuse a clean candidate or acquire a fresh sandbox through normal selection. Preserve exact same-run reattachment, workspace-less execution and ordinary task admission checks. - Preserve the newer prepared remote-workspace writer proof for native and legacy runs. Share its binding check between single-source recovery and one batched allocation read. - Keep persisted recovery-action IDs when constructing continuation source checks. Synthetic sources cannot grant snapshot receipt authority. - Permit one audited maintenance adoption for the first exhausted source-verification attempt. Require an unadmitted canonical saved board Retry, current user authority, unchanged assignment and latest source run. - Keep all company, source, allocation, workspace writer, controller and physical finalization ownership checks. The public read-only inspector keeps its strict policy check. - Pin the exact source for file maintenance. Preserve the previous generation and reuse the existing bounded recovery worker. Create no provider turn or new user request. - Restore the warm policy in the same transaction as verified native copyback completion. A failed write or changed settlement keeps a recovery obligation. - When local merge metadata is missing, settle the exact verified snapshot while keeping its unfinished allocation retained. Deliver the saved Retry through normal fresh isolation and attach the verified files. Keep the accepted result as history. - Require native copyback proof before reopening an accepted result's original workspace finalizer. - Validate snapshot receipts through one shared boundary for warm, retained and ephemeral sources. Bind the receipt to the current generation's operation IDs, company, issue, run, recovery action, lease and provider allocation. - Retire fixture input before removing recovery proof, and use the same configured heartbeat for HTTP and worker dispatch. Keep the exact catalog refusal and no-provider-work assertions. - Document the recovery contract in technical documentation. Leave capability maps unchanged for this bug fix. ## Verification - Candidate-selection RED: all six clean-candidate and fresh-allocation controls fail on unchanged `f54e7baf538` because a pending export aborts the whole acquisition. GREEN: the focused suite passes **15/15 cases** after candidate filtering. Both provider paths preserve the protected lease and never resume or destroy its allocation. Obsolete fingerprints cannot send that allocation to cleanup. Historical-row fencing, modern native/legacy writers, exact same-run reuse, workspace-less execution and the single-read allocation-history check remain covered. - The existing reusable-runtime selection cases passed **40/40**. Driver/capability contract suites passed **5/5**. Reusable service controls passed **2/2**. After merging current master, the combined lease/provider suites pass 61 cases and the recovery integration selection passes 36 cases, including the upstream inherited-session and resolved-interruption assertions. This gives 97 passing targeted cases on the final source. A duplicate test-app declaration from the automatic merge was removed while retaining the upstream assertions. - The source-history repair has meaningful red/green evidence: three valid retired-handoff cases could not complete under the old blanket history-row rejection. Settlement now uses the same full source inspector as adoption. It accepts exact retired handoffs and still rejects a predecessor that becomes active after export. The 26 focused history, rollback, snapshot and ambiguity cases passed before this last merge. - Server typecheck passes on the final merged source. Module boundaries and `git diff --check` pass. Full local repository tests and a successful current-head full build are not claimed. Current-head CI is complete on `0ef974d76483e11fb0b0304bcb05bbd3b56e2ef9`: both required gates pass, with 54 successful checks, three expected skips, one neutral Paperclip review check and a successful security status. [Full CI evidence](https://github.com/paperclipai/paperclip/actions/runs/41904339370). Greptile reviewed that exact head at **5/5** with no actionable findings. Its check completed successfully. [Ponytail also reviewed the exact head at 5/5](https://github.com/paperclipai/paperclip/pull/17916#issuecomment-6640083408). No review threads remain unresolved, and the branch is mergeable. - Earlier local broad runs exposed a dropped recovery-action ID in the merged source projection and deferred test input being revisited after its records were removed. Both were repaired. Later local disk exhaustion caused manifest and saved-file reserve failures; generated caches were removed only from these checkouts. The affected saved-file control passed unchanged after cleanup. The original logs are retained. No disk reserve or timeout was lowered. - On the prior head, the first [full CI attempt](https://github.com/paperclipai/paperclip/actions/runs/41899038530) received simultaneous runner shutdowns. Four jobs were cancelled, and two late jobs have lost-runner annotations with unavailable logs. The Runner job had five earlier ACPX failures whose full assertion report was lost on interruption. Their cause remains unproven. Identical Runner source passed the [#19004 control](https://github.com/paperclipai/paperclip/actions/runs/41898305521/job/126814429725), including all seven enabled tests in that file. - That attempt's [browser shard](https://github.com/paperclipai/paperclip/actions/runs/41899038530/job/126817404595) returned to the exact saved task URL, but its conversation was still loading at the unchanged heading deadline. The [artifact](https://github.com/paperclipai/paperclip/actions/runs/41899038530/artifacts/13358840931) is retained. All five Cloud-auth cases passed locally on the unchanged head and in the [same-source CI control](https://github.com/paperclipai/paperclip/actions/runs/41898305521/job/126814429966). The delay remains unexplained. One rerun of failed/interrupted jobs was submitted after the workflow completed. This new source revision requires fresh CI. - The earlier [original-head workflow](https://github.com/paperclipai/paperclip/actions/runs/40552228175) passed all 48 jobs, and prior head `f54e7baf538` received Greptile 5/5. These results do not establish readiness for the new head. The earlier Superagent scan was neutral after a pinned-base checkout failure; its external analysis remains unverified. - These tests use real PostgreSQL and controlled provider boundaries. Hosted rollout and original-task acceptance remain unverified. ## Risks - Maintenance can resume and stop the exact sandbox for file export. It cannot admit a provider turn, choose another allocation or bypass normal continuation gates. - A saved Retry alone is insufficient. A changed source, newer run, active writer, competing lease, lost user authority or previous adoption keeps the hold. - Policy restoration can fail. Completion and restoration now commit together, so normal recovery remains responsible for a retry. - Snapshot preservation cannot repair missing local merge metadata. Its old allocation remains explicitly retained. A snapshot receipt grants no permission to reuse or destroy that allocation, or to replay the accepted result. - The allocation check uses one projected join per candidate over the existing provider lease index. It adds no polling timer or recurring scan. A fresh fallback can create another sandbox while the unfinished source remains retained; it grants no authority to execute a provider turn. - This change does not establish the cause of a prior provider failure or prove that any specific hosted sandbox still contains its original files. ## Model Used OpenAI Codex, GPT-6 family, for the original change. OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, for the merge and integration repairs. Both used repository tools and PostgreSQL verification. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
|
801d9548e3 |
test(server): cover competing recovery action inserts (#18773)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery owners record one active action for each task source. > - Two owners can read an empty source before either inserts its action. > - PostgreSQL rejects one insert, and Drizzle wraps that conflict. > - The production fix for wrapped conflicts already landed in #18885. > - This pull request adds a real database race test for that fix. ## Linked Issues or Issue Description Refs #18885 and #18544. Related recovery work: #18402 and #18705. **What happened?** [CI run 41585908789](https://github.com/paperclipai/paperclip/actions/runs/41585908789/job/125752385968) failed with `issue_recovery_actions_active_source_uq`. The recovery worker did not handle the wrapped PostgreSQL conflict. #18885 now provides the production fix. **Expected behavior** Both competing recovery owners return the same active action. The existing bounded retry handles the exact wrapped unique constraint. **Steps to reproduce** 1. Create a task in real PostgreSQL. 2. Hold both recovery inserts until both owners read an empty source. 3. Release both writes. 4. Check that the losing transaction records a wrapped `23505` conflict and both service calls return one action. ## What Changed - Retain the newer bounded cause-chain handling from #18885. - Add a deterministic database race test. Check the actual wrapped conflict, both returned action IDs, the saved action count, and the attempt count. - Keep the original real-clock settlement and idle-window assertions. ## Verification - The two owning suites passed **98/98 tests** after resolving the conflict: `pnpm exec vitest run server/src/__tests__/issue-recovery-dependencies.test.ts server/src/__tests__/issue-recovery-actions.test.ts`. - Workspace typechecks passed across the root run and a successful server rerun. The first server attempt could not resolve the local plugin SDK from provider sources. Restoring that checkout-only SDK link fixed resolution. The server typecheck rerun passed. - Full local `pnpm test:run` has not run. The current local full build stopped after the host exhausted disk space during native compilation. Generated caches were removed from this checkout; no source or test setting changed. Fresh CI is complete: 53 successful checks, two expected skips and one neutral security scan. The branch is mergeable, and the exact-head Greptile review is 5/5 with zero unresolved threads. - Greptile scored current head `dbaee78272a5b0f1db87ba33340a63571ef16948` **5/5** with no actionable findings. - Historical red proof: before wrapped-conflict handling, this race returned one rejection. The losing transaction carried PostgreSQL code `23505` and constraint `issue_recovery_actions_active_source_uq`. - Preserve prior CI evidence: an [old serialized job](https://github.com/paperclipai/paperclip/actions/runs/41649267044/job/125964435813) hit an agent-audit setup timeout and a heartbeat wait failure. The exact timing cause remained unproven. Its [single rerun](https://github.com/paperclipai/paperclip/actions/runs/41649267044/job/125972675510) passed. This PR changes no waits or assertions in those tests. ## Risks - Low risk. The final diff adds one database regression test. It changes no production code, schema, API, permissions, runner routing, or workflows. ## Model Used - OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, tool use, and code execution. ## Checklist - [x] I have included a thinking path and specified the model used. - [x] I have searched for related work and linked the production fix. - [x] I have included only public issue and PR references. - [x] The changed test passes against real PostgreSQL. - [x] I have documented verification limits and risks. - [x] All Paperclip CI gates are green. - [x] Greptile is 5/5 with no unresolved comments on the final head. - [x] I will address review comments before requesting merge. Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
Devin Foley
|
69dea878c0 |
fix(sentry): prove a native capacity retry from its successor run (#19051)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends unexpected run failures to Sentry as run-failure events > - A native Codex run can fail because the model is at capacity. Paperclip then schedules a bounded automatic retry > - #18295 keeps that scheduled retry out of Sentry. It requires the committed finalization fields on the failed run row > - On staging, the failed run row can lose `finalizationReasonCode` after the decision is committed. The retry run exists, but the report still goes to Sentry > - This pull request uses the successor retry run as the proof, and stops reading the run's own finalization fields > - The benefit is that a capacity failure that has a scheduled retry does not make Sentry noise. Exhausted and held failures still report ## Linked Issues or Issue Description Refs #18295 **What happened?** A native Codex run failed with `native_provider_overloaded`. The status decision scheduled the capacity retry, and the retry run was created. The server still sent a run-failure event to Sentry. The server release contained #18295. The failed run row had `finalizationPhase: "committed"`, but it did not have `finalizationReasonCode`. The retry run was created about two seconds before the failed run's terminal write. **Expected behavior** When the committed capacity decision creates a retry run, the failure stays local. The retry owns recovery. A later failure of the retry reports its own outcome. **Steps to reproduce** 1. Make a native Codex run fail with a typed `codex`/`server_overloaded` diagnostic. 2. Let the status decision schedule the `native_provider_overloaded` retry. 3. Let a late terminal write replace the run's `resultJson` with a version that does not have `finalizationReasonCode`. 4. Report the run failure. Before this change, Sentry gets an event. **Additional context** Only the committed `schedule_retry` effect in `status-decision-committer.ts` creates a successor with `scheduledRetryReason = native_provider_overloaded` (through `materializeNativeProviderCapacityRetry`). Thus the successor row is a stronger proof than the run's own finalization fields. ## What Changed - `server/src/services/run-failure-report.ts`: `isScheduledNativeCapacityRetry` no longer requires `finalizationPhase === "committed"` or `finalizationReasonCode === "native_provider_overloaded"` on the failed run. It still requires a failed native run with `errorCode = native_provider_overloaded`, the typed `codex`/`server_overloaded` diagnostic, a native task, no signal, and a phase that is not setup. It still requires the successor run for the same task with `retryOfRunId = run.id` and `scheduledRetryReason = native_provider_overloaded`. The successor lookup and its index use do not change. - `server/src/services/__tests__/run-failure-report.test.ts`: two new cases. A scheduled retry stays local when the run row has no finalization fields, and when the row has an earlier finalization phase. The "finalization is not committed" must-report case changes to "finalization has no fields and no retry run exists". That case still reports. - `doc/observability.md`: describes the successor-run proof. ## Verification - `cd server && npx vitest run src/services/__tests__/run-failure-report.test.ts`: 284 tests pass. - Red check: with the `master` version of `run-failure-report.ts`, the two new cases fail. Each one captures one Sentry event. All other 282 tests pass. - `npx vitest run src/services/native-runtime/native-provider-failure.test.ts src/services/native-runtime/status-arbiter.test.ts`: 73 tests pass. - I could not load `native-provider-overload.integration.test.ts` in my local sandbox because of a missing environment setting. CI runs it. - Exhausted, held, other-lane, other-task, no-task, legacy, setup, signal and untyped-diagnostic failures still report. The existing must-report cases prove this. ## Risks Low risk. The change removes two field checks. The successor-run check stays. Only the committed capacity decision creates a successor with this retry reason, so a failure without a scheduled retry still reports. No migration. No change to the retry behavior. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`) 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) |
||
|
Nicky LeachandClaude Opus 5.5
|
0173110ee3 |
fix(native): point failed run attachments at the docs (#19052)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a later run cannot attach to a native session, the run fails, and a later run may start a new provider thread. > - The first failure is easy to miss. In 5 of 11 cases on a test deployment, the run ended with a different error before it reported the attach failure. > - People and agents who diagnose a failed run look at its log and events, so that is where an explanation is useful. > - This pull request adds a pointer there at the first failure, with the real cause and a link to the docs. > - The benefit is that a failed attach explains itself, and readers can find the open follow-up in #19047. ## Linked Issues or Issue Description Refs #19047. I searched open and closed pull requests and issues for `prior_attach_failed` and related terms, and found no duplicate. ## What Changed - **Log:** `executeHeartbeatRuntime` writes one stderr line to the run log when a same-run retry follows a failed `run.attach`. - **Events:** `heartbeat.ts` adds a `lifecycle` event with the same message and the code `native_run_attach_failed`, using the stored redacted failure. - **Helper:** `describeFailedAttachment` extracts the real cause and returns nothing for other failures. - **Docs:** "Session settlement after a run" in `doc/execution-semantics.md` now explains this and links #19047. - **Link target:** the pointer links to the repo docs and not the issue. Agents read these logs, and anyone can comment on a public issue. - **Unchanged:** error codes and retry behavior. ## Verification - `cd server && pnpm exec vitest run src/services/heartbeat/runtime-execution.test.ts src/services/native-runtime/remote-warm-session-recovery.test.ts`: 57 passed. - `npx tsc --noEmit -p .` in `server` reports no errors. - New tests cover the log line at the throw site, and the helper's output for attach failures and other failures. - **Not covered:** the new event in `heartbeat.ts` has no test of its own. It uses the tested helper. - **Not verified:** I have not confirmed that agents read this log when they diagnose a failure. - **Not run locally:** the full test suite. CI provides that coverage. ## Risks - **Failure text in the log:** when the failure has no structured message, the log line includes the whole failure string. It passes through the run log's identity redaction. The event uses the stored failure that the executor already redacted. - **Link stability:** the link depends on the docs heading. Renaming that heading would break the anchor but not the page link. - **Migrations:** this change has no migration. > 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 reworked. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), through Claude Code, with tool use and code execution. A human directed the design and approved the scope. ## 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> |
||
|
Devin FoleyandClaude Opus 5.5
|
72d80944a9 |
fix(skills): keep the skill cache lock transaction active (#18996)
Replaces #17907. That branch conflicted with `master` after #17918 added
preparation timing to the same function. This branch applies the same
fix on current `master` and keeps that timing.
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Before a run starts, the server prepares the agent's skills while
holding a per-company skill cache lock.
> - The lock is a Postgres advisory transaction lock on a dedicated
connection. The preparation work runs on the ordinary pool, so the lock
transaction sits idle.
> - Hosted Postgres (Neon) sets `idle_in_transaction_session_timeout`.
If preparation runs longer than that timeout, Postgres ends the lock
session.
> - The lock is then released early, and the commit fails with `write
CONNECTION_CLOSED`. The run fails during setup and the preparation work
is lost.
> - #16340 and #16956 fixed the same problem for the workspace source
lock and the lifecycle lock with `withWorkspaceLockKeepalive`.
> - This pull request applies that keepalive to the skill cache lock.
> - With it, slow skill preparation keeps its lock and the run no longer
fails.
## Linked Issues or Issue Description
Refs #16340
Refs #16956
Supersedes #17907
**What happened?**
A hosted staging run failed during setup with `write CONNECTION_CLOSED
<neon-pooler-host>:5432`. The run trace shows `skills.prepare` running
for 322 seconds before it failed. That is longer than Neon's 5-minute
`idle_in_transaction_session_timeout`. The stack ends in
`postgres/src/connection.js` (`TLSSocket.closed`).
**Expected behavior**
Skill preparation holds the skill cache lock until it finishes, and no
other preparation for the same company can enter the locked section
during that time. The run then continues to execution.
**Steps to reproduce**
1. Set `idle_in_transaction_session_timeout` on the database role to
`1s`.
2. Call `withSkillVersionCacheLock` with an action that waits longer
than 1 second.
3. Without this change, a second `tryOnly` call for the same company
gets the lock while the first call still holds it. The first call then
fails with `write CONNECTION_CLOSED` instead of returning its own result
or error.
## What Changed
- After `withSkillVersionCacheLock` gets the lock and records the
`skills.cache.acquire` phase, it reads
`idle_in_transaction_session_timeout`. It then runs the action inside
the existing `withWorkspaceLockKeepalive` helper. The helper sends
`select 1` on the lock transaction every third of the timeout, and at
least every 30 seconds. When the timeout is 0 (disabled), the helper
does nothing.
- The `.catch` from `master` that ends the timing phase on failure is
kept.
- Added `skill-version-cache-lock.test.ts`. It runs against real
embedded Postgres with a 1-second idle timeout, following the same
pattern as `isolated-remote-workspace-source-lock.test.ts`.
- The edited file no longer has an extra blank line at the end. This is
whitespace only.
## Verification
All checks below ran on this branch's tree, which is `master` at
|
||
|
DottaandPaperclip
|
31266d7159 |
fix(logs): drain existing admissions before finalization (#19055)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Run logs retain execution output for operators. > - Object storage uses bounded admission and asynchronous uploads. > - A diagnostic can wait for capacity when its native execution ends. > - Finalization closed the writer before that existing admission could finish. > - This pull request joins those admissions before it closes the log. > - The benefit is complete accepted output with the existing storage bounds. ## Linked Issues or Issue Description Refs #18944 and #18327. **What happened?** A runner close diagnostic began object-log admission before its recorder retired. The call waited at the 8 MiB pending-byte bound. Completion then closed the writer. The waiting call failed with `Object run log writer is closed`. A valid final log hash covered the stored prefix, but the diagnostic was absent. **Expected behavior** Finalization must retain calls that started before closure. It must reject new calls after closure. Cancellation and storage failures must still settle without a success claim. **Steps to reproduce** 1. Use the actual recorder and segmented object store with a controlled conditional upload. 2. Fill the pending-byte bound. Start native warm-session retirement while the diagnostic waits for capacity. 3. Complete the run through the real completion handler. Release the upload. 4. Check the diagnostic bytes, final hash, source coverage, and warning rows. ## What Changed - Track active admission promises in each object-log writer. - Reject new producers when finalization starts. Join existing bounded admissions, then flush their tail before manifest closure. - Add actual completion coverage and store controls for failure, Stop, global capacity, and independent writers. - Clarify the finalization contract in the storage documentation. Keep feature maps unchanged for this routine fix. ## Verification - Before the fix, the actual completion probe lost a pre-retirement close diagnostic and emitted `object_log_persistence_failed`. Useful canonical output, source coverage, and the final prefix hash remained valid. - After the fix, the exact probe retains the diagnostic and checks the stored bytes and full hash. Ordinary completion and stalled local append controls also pass. - Focused store controls pass, including shared 64 MiB capacity, new-producer rejection, storage failure, Stop, and independent writer progress. - Production and test source at `bc03f8d53a721f01f80a12105a69d8583865e651`: 3 actual completion controls and 53 store, recorder, journal, and diagnostic tests pass. Run with `node node_modules/vitest/vitest.mjs run` from `server/`; select the four safety files plus the completion file with `-t "retired recorder"` in a separate invocation. - Initial treatment repository typechecks and build pass. Current map and module checks pass. A broader four-suite run had 99 passing assertions but failed its existing 30-second completion teardown hook. A later isolated completion run passed all 61 cases at the same limits. The original teardown cause remains unresolved; no timeout was changed. - Repository typechecks and build pass at that source commit. Current head `6652adc7f3379ae250f7b726dad6c58c8482410f` removes only the incidental map paragraph; production and test source are identical. All current-head checks are terminal: 54 successful check runs, one successful legacy status, and three skipped checks. Required `ci / verify` and `ci / e2e` pass. Greptile reviewed exact current head at 5/5. Both review threads are resolved. GitHub reports the PR clean and mergeable. Full repository tests stopped in chat shard 2: a GitHub recovery fair-batch test expected no provider requests but observed `GET /app/hook/config`. Its exact unchanged case passes in isolation. The original cause remains unresolved; this full-suite run is red. First-head remote server lane 2 failed a native-reassessment backlog test: its quiet check observed two active work items instead of zero. The exact unchanged case passes locally. That original CI cause remains unresolved; no job retry or timeout change was made. Other first-head CI lanes passed. The first native review rated the earlier source head 4/5 for the same removed map note. That is not a current-head assessment. Current GitHub rules require zero approving reviews and only the two CI checks above; a fresh native advisory re-review is optional and remains unavailable while its earlier run has an unresolved workspace/accounting obligation. That obligation is tracked separately and is not claimed recovered. - Removed the incidental map paragraph identified by Greptile and the native review. The follow-up contains no runtime or test changes. - This proves the local production-path mechanism. It does not attribute any specific hosted warning to this cause. Hosted replay and served revision checks remain separate acceptance work. ## Risks Finalization now waits for existing bounded capacity admissions. Storage cancellation and the existing operation deadlines still apply. Tests check that failed uploads and Stop settle, and that a separate writer can make progress. No capacity limit, timeout, or execution authority changes. ## Model Used OpenAI `gpt-6.1-sol`, through Codex, with reasoning and tool execution. Context window size is not exposed by the host. ## 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 (56 focused checks; the full-suite red remains documented above) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
86afc2048b |
fix(native): admit follow-ups after an owner finished in this process (#19071)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner per-turn sessions in a remote sandbox keep one conversation across task turns. > - Before a follow-up starts a new Runner, it must prove that the previous owner's controller stopped. #18846 added a retirement path for this proof. > - The native controller runs inside the webserver process. Thus the owner's recorded controller PID is the PID of the webserver. > - When the same server process serves the follow-up, that PID is alive and has the same start time. The proof reads this as "controller still alive" and refuses. The follow-up then fails with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. This continues until a deploy replaces the process. > - This pull request uses the controller's per-process boot identity to recognize that this process recorded the owner. The process then uses its own session registry to prove that the owner's controller stopped. > - The benefit is that follow-ups on a long-lived server no longer fail before provider work. All other ownership fences stay as they are. ## Linked Issues or Issue Description Refs #18846 (retirement of recovered per-turn owners). Refs #19061 (names the refused retirement proof). The two changes are compatible. Whichever merges second needs a small rebase in `retireFinishedPerTurnRunnerdOwner`. **What happened?** On a test deployment, 17 runs over 14 tasks failed before provider work with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`. Each run had a per-turn lifecycle in a Daytona sandbox and got a new sandbox. In each inspected case, the previous run on the task succeeded and committed its result. Its run profile had no `nativeControllerRecovery` pointer. Without that pointer, `evaluateNativeControllerTakeover` is not lease-fenced, so it uses process liveness. The recorded `controllerPid` is `process.pid` of the webserver. When the follow-up is served in the same process, `process.kill(pid, 0)` succeeds and the start time matches. The result is `controller_still_alive`, and retirement refuses. **Expected behavior** A committed owner that this server process finished, released, and no longer runs is proven stopped. The follow-up then starts a new Runner, as #18846 intended. **Steps to reproduce** 1. Seed a per-turn sandbox owner that committed its result and released its lease. Record this process's controller identity (`currentNativeControllerIdentity()`) as the owner. 2. Keep the owner's file-backed journal in the local session root, and remove the database recovery pointer. 3. Admit a follow-up in a new sandbox. 4. Before this change, admission fails with `prior_owner_terminal_state_indeterminate`. **Paperclip version or commit** Observed on `e107a75755`. The same code is on `master`. ## What Changed - `native-restart-recovery.ts`: add `isCurrentNativeControllerProcess(owner)`. It is true only when the owner's `controllerBootId` is this process's per-start boot UUID and `controllerPid === process.pid`. A recycled PID or another container never matches. - `retireFinishedPerTurnRunnerdOwner`: when the owner was finished in this process and no `activeNativeSessions` or `nativeSessionStartups` entry exists for its run, treat its controller as stopped. Otherwise, use `evaluateNativeControllerTakeover` as before. The same in-process check runs again just before the archive. - All other retirement proofs are unchanged: committed result, released lease, no recovery state, settled journal, answered tool inputs, quarantine copies, and new or replacement sandbox. - `native-recovered-owner-followup.test.ts`: the seed helper accepts the owner's controller identity. New cases cover same-process admission (with and without a recovery pointer), an exited legacy controller, another live controller, and the same PID with a different boot identity. - `doc/execution-semantics.md`: describe the same-process stopped-controller proof. ## Verification - **Failure before the change:** with only the new tests applied, `cd server && npx vitest run src/services/native-runtime/native-recovered-owner-followup.test.ts` fails 1 case: "admits a follow-up in the process that finished the owner: local journal only". It expected admission and received `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate`, the exact error from the deployment. The safety cases already passed on the old code. - **After the change:** the same file passes 27 of 27. - Adjacent suites: `native-recovered-owner-followup`, `remote-warm-session-recovery`, `native-runner-restart-recovery.integration`, `native-restart-recovery`, `native-session-resume`: 210 passed, 20 skipped. - `native-session-executor.test.ts`: 1128 passed. - `tsc --noEmit -p server/tsconfig.json`: no errors. - **Not verified live:** this change has not run on a deployment. Run data came from read-only API calls only. No recovery was done on the deployment. ## Risks - **Second executor.** The new proof applies only when this exact process recorded the owner, and this process has no session or startup for that run. The database proofs still require a committed result and a released lease. Another live controller, or the same PID with a different boot identity, still holds the owner. Tests cover both cases. - **Scope.** The change applies only to the per-turn retirement path. The warm remote recovery path (`inspectRemoteWarmSessionRecovery`) is not changed. Warm handoff in the same process uses the in-memory idle owner instead. - **Hypothesis limit.** The deployment API does not expose finalization rows. Thus I inferred that the same process served each follow-up from the evidence: no recovery pointer, a local journal present, and no other failing proof. The local reproduction gives the exact error. #19061 can confirm the refused fence on the deployment. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Agentic coding in Claude Code with tool use (shell, browser for read-only staging API reads, 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> |
||
|
|
c19c4348c5 |
fix(server): record inspector provenance for dynamic import callback errors (#17844)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/49/changes/paperclip/49 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends errors to Sentry when an operator sets a backend DSN. An unhandled rejection is captured in `strict` mode, so the server process exits after the capture. > - A hosted server reported `TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.` two times, eight days apart, on two unrelated releases. Each event stopped the process. > - The stack has no first-party frame. An anonymous `setTimeout` callback at `<anonymous>:1:N` calls `import()`. The column changed between the two events, so the source text changed. > - In local reproduction, only two paths make that frame: `vm.compileFunction` without a filename, and an expression that a client evaluates through the Node inspector. No shipped server code or production dependency uses either path. The current events give no data about the source. > - This pull request adds bounded provenance to that one error: the inspector state when the event is reported (never its URL), allow-listed Node flag names, the process ids, the main-thread flag and the uptime. > - The benefit is that the next event gives evidence about the source of the anonymous code. A `closed` state rules out an expression from a still-open inspector session. An `open` state needs other evidence before a debugger is blamed. ## Linked Issues or Issue Description No public issue exists. This section follows the bug report template. **What happened?** A hosted server process stopped two times with an unhandled rejection: `TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.` The stack is `importModuleDynamicallyCallback (node:internal/modules/esm/utils:280)` ← `Timeout._onTimeout (<anonymous>:1:20)` (first event) or `(<anonymous>:1:41)` (second event) ← `listOnTimeout`. The events came 3 minutes and 17 minutes after process start, on two releases that changed only skill documents and UI files. The Sentry event has no data that shows where the anonymous code came from. **Expected behavior** The error report gives enough data to investigate the source of the anonymous code: whether an inspector session was open in the process, and which inspector-related Node flags were set. **Steps to reproduce** 1. `node -e "require('node:vm').compileFunction(\"setTimeout(() => import('node:fs'), 1)\", [])()"` gives the same error and the same `Timeout._onTimeout (<anonymous>:1:N)` frame. 2. An expression such as `setTimeout(() => import('node:fs'), 1)`, evaluated through the inspector (`Runtime.evaluate`), gives the same error and frame. 3. `vm.Script` (`evalmachine.<anonymous>`), `eval` or `new Function` in a `vm` context (`eval at …`), `new Function`, indirect `eval`, `node -e`, `node -p` and stdin give a different frame or import successfully. **Paperclip version or commit** `master` at `0cb3fc7db591ba23e21fbbc70042bc44ec7d6f20`. The hosted events came from builds `f96a1e64` and `3e7c3ee9`, on Node v24.22.0 and v24.22.1. **Relevant subsystem** Server Sentry error monitoring (`server/src/sentry.ts`). ## What Changed - New `server/src/sentry-dynamic-import-provenance.ts`: `addDynamicImportProvenance(event, hint)` acts only when `hint.originalException.code` is `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`. It adds: - the tag `dynamic_import_inspector` with the value `open`, `closed` or `unavailable` - the context `dynamic_import_provenance` with `inspector`, `nodeFlags`, `mainThread`, `pid`, `ppid` and `uptimeSeconds` - `nodeFlags` holds only the names of `--inspect`, `--inspect-brk`, `--inspect-wait`, `--inspect-port`, `--debug-port`, `--disable-sigusr1` and `--experimental-vm-modules` from `execArgv` and `NODE_OPTIONS`. Flag values and all other flags are dropped. - The hook never sends the inspector URL, because the URL contains a session secret. It loads `node:inspector` lazily, so a Node build without the inspector still starts and reports `unavailable`. The hook never throws. Every other event is unchanged. - The module comment and the doc say that the state is evidence at report time, not proof of origin. - `server/src/sentry.ts`: `beforeSend` is now `enrichServerEvent`. It runs the existing run-link enrichment, then the new hook. - `doc/observability.md`: the default capture set names the new tag and context, and says what is never sent. - `.github/workflows/sentry-contract.yml`: the Real Sentry SDK isolation job also runs the new test file. - Tests in `server/src/__tests__/sentry-dynamic-import-provenance.test.ts`: - the real Node error from `vm.compileFunction` - a real open inspector session: the URL and its secret are absent from the event. The test closes only the session it opened, and it skips when a developer already has an inspector open. - flag filtering with values and other flags present in `NODE_OPTIONS` - other codes and other hint shapes stay unchanged - a failed state read keeps the original event - the real `@sentry/node@10.71.0` SDK sends the original rejection to the server `beforeSend`, and enriches only the matching event. With `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1`, a missing SDK fails the file. ## Verification - With the audited SDK on `NODE_PATH` and `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1`, as in the contract job: `run-failure-sentry-real-sdk.test.ts` and the new test file pass (17 tests). - Without the SDK: the new test file, `sentry.test.ts` and `sentry-links.test.ts` pass (52 pass, 6 skipped). - With `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1` and no SDK: the new test file fails at once, as intended. - Red: with `beforeSend` set back to `addSentryRunLinks`, the real-SDK test fails (`expected [] to have a length of 1`). Green with the change. - Hosted CI on this head: typecheck, build, all test partitions and Real Sentry SDK isolation pass. - Not run locally: the full Vitest suite and browser suites. A live hosted check is not possible before deployment. ## Risks - Low risk. The hook acts only on one error code. All other events keep their current content and grouping. - The process still exits after the capture, as before. This change adds diagnostics. It does not fix the source of the anonymous code. - `pid`, `ppid` and uptime describe the server process only. They hold no tenant, user or request data. - If a later Node version changes the error code, the hook stops acting and the events are the same as today. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through the Claude Agent SDK, with tool use (shell, file edit, code execution, GitHub and Sentry connectors). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
a3313f6944 |
fix(ui): complete read recovery and enforce query error rendering (#18822)
## Thinking Path > - Paperclip lets people manage agent work through the board. > - The board reads task, agent, settings, plugin, and run data from the server. > - A deploy can interrupt those reads after content is already visible. > - Local error branches can replace that content with an error or show raw codes. > - This pull request moves the remaining reads to the shared recovery behavior. > - The query-error guard prevents new local error patterns from returning. ## Linked Issues or Issue Description **What happened?** Loaded lists, settings, task panels, and manual reads could clear their content or show errors during a server restart. Some reads also disabled the shared retry policy. **Expected behavior** Keep loaded content during a temporary failure. Show the shared connection banner. Retry reads after recovery. Replace content after a real access denial or missing resource, with readable copy. **Steps to reproduce** Open a task, settings page, or run log. Return 503 with `tenant_app_unavailable` from the API. Restore the API and check that content refreshes. Repeat with 403 and 404 to check the failure state. Related merged work: Refs #15754, Refs #15772, Refs #17960. This change covers remaining reads and enforcement. Browser persistence and the outage browser suite follow separately. ## What Changed - Repair an existing PostgreSQL deadline fixture at UTC boundaries. A daily cap defers a heartbeat until both its original interval and the next UTC day allow it. Keep the counted run in its seeded day and cover noon, just before midnight and the first second after midnight with explicit clocks and real PostgreSQL. Production scheduling is unchanged. - Integrate the saved list, settings, shell, task, and manual-read recovery changes against current master. Preserve the current task, Inbox, and run-log structure. - Add `useResilientFetch` for component-local reads. Preserve loaded data, cancel stale requests, and retry through the shared connectivity policy. Keep browser and pairing capabilities in memory. - Retain run log chunks across stream changes and catch up after recovery without duplicate chunks or stalled requests. - Keep genuine authorization and missing-resource failures visible. Translate known single-word codes such as `forbidden`. Show known skills and disconnected-app failures before sibling loading states. Keep Retry bound to its failed request. - Make the query-error guard enforce by default and run it in the CI policy job. Document the narrow exceptions and the read recovery contract. - Invalidate pending OAuth discovery when the operator leaves setup or its context changes. Keep reserved-popup cleanup bound to the current attempt. ## Verification - Current read head `09d1db29dea` is merge-ready: all 59 check runs are completed (54 successful, three skipped, two neutral), the legacy Snyk status is successful, and both required `ci / verify` and `ci / e2e` gates pass. Greptile Apex reviewed this exact head at 5/5 with no unresolved threads; the branch has no merge conflicts. The current CI run is `41748339602`. - Current read head `09d1db29dea` includes master `683fa4725e9`. The latest change touches only the existing server deadline test fixture. Application source remains the previously verified `86376fa751e` revision: all 60 affected skills/disconnected-app tests and both application UI typechecks passed, along with token gates and the enforcing query-error guard. Those UI suites were not repeated for this test-only repair. - The fixture repair passes all seven deadline integration cases and the complete server typecheck, including the server test layer. The old assertion deterministically fails at 23:59:06.888 UTC; its one-second-old run also fails the first-second-of-day probe. The corrected fixture passes noon and both midnight probes. The permanent three-case coverage uses seeded 2099 clocks. - Twelve new regressions fail against the previous implementation. Both skills variants and disconnected-app pages show either request's 403 or 429 while the sibling pauses or remains fetching. Permission failures stay visible. Retry stays usable for its failed request and recovers independently. An initial test-fixture query-key typo was corrected before the passing run. The finding is replied to and resolved. Greptile reviewed prior application revision `86376fa751e` at 5/5 with no new actionable findings or unresolved threads. The current test-only head has now received its own Apex 5/5 review with no unresolved threads, and its CI is successful as recorded above. - The preceding OAuth fix passed all 237 setup tests on both application branches. Its ten regressions prove that Back, Cancel, unmount, or context changes retire initial discovery without creating a connection, caching old identities, navigating an abandoned popup, or closing a newer attempt. - Earlier master integration `e992f770339` passed all 309 tests across eight affected Slack, recovery, retry, submission and task files, plus UI typecheck, token gates, enforcing query-error scan and seven guard tests. The integration preserves Slack workspace reconnect and shared recovery, readable execution errors, source-inspection company scoping, and current submission behavior. Newly inherited retained-file recovery controls keep cached data through transient failures and hide it after definitive denial. - The preceding cache head `97c0bc3966f` passed 1,038 focused cache/write/editor/account/task/API tests. All 22 built-UI outage/write/loading cases passed in 4.7 minutes on exact browser head `b7deaaee5da`, with strict checking of all three specs. The latest skills/app rendering fix is covered by its affected component suites; those broader cases were not repeated for this separate read path. - Historical broader UI, CLI and build results and unresolved aggregate server/DB/adapter failures remain in the PR discussion. Interrupted CI runs were audited and recorded separately; they are not counted as passes. Controlled local outages do not prove hosted authentication or a real Cloud restart/deploy. Real hosted acceptance remains required after merge and deployment. Commands for the latest fix: ```sh pnpm --filter @paperclipai/ui exec vitest run src/pages/CompanySkills.test.tsx src/pages/apps/AppNotConnected.test.tsx pnpm --filter @paperclipai/ui typecheck pnpm check:token-gates node scripts/check-query-error-rendering.mjs --enforce --summary ``` ## Risks This migration touches many read surfaces. The main risks are initial loading, request cancellation, retained authorization state, and run-log paging. The tests cover those states. There are no server, schema, dependency, or API contract changes. The follow-up browser suite and hosted deploy acceptance are required before release acceptance. Recovered commits retain their original authors. If squash-merging, preserve `Co-Authored-By: Bender (Fable) <bender@agents.paperclip.ing>` and the Paperclip trailer. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with xhigh reasoning effort, tool use, and code execution. The context window size is not exposed. Saved source commits keep their original authorship. ## 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 described the issue in-PR following the bug report template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] The affected local tests pass; broader limitations are recorded above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented 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: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Bender (Fable) <bender@agents.paperclip.ing> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
52b4b3ccd7 |
test(evals): pin native credential delivery and worker receipt fixes (#19042)
## Thinking Path > - Paperclip must deliver agent work through ordinary human controls. > - The latest three-case run retained three failed originals. > - The personal value was saved, but the native OpenCode child did not receive the configured task environment. > - A worker proof also relied on legacy process identity fields outside that environment contract. > - Timeline and credential checks had independent review defects that could reject truthful outcomes or miss unsafe instructions. > - This PR pins the reviewed fixes together and keeps all prior grades unchanged. > - A new AWS measurement must still prove the real outcome. ## Linked Issues or Issue Description Refs #18963, #18967, #18968, #18971, #18972, #18977, #19027, #19031. Follows merged #18995. This is the next infrastructure pin, not another core implementation PR. The latest launch 2657 retained FAILED originals for timeline, approved-tool replay and personal secret. The personal journey reached secure UI save and a worker wake, but the worker reported no credential and made no endpoint call. These are failed attempts, not new passes. Six of the selected ten cases have independent passes across prior attempts. The uniform final ten and the full catalog remain unqualified. No old result was regraded. ## What Changed - Pin immutable candidate `c6cc59282a45ed3c79880a220f7f60aa3ac1d6a4` on `codex/chat-credential-destination-contract-candidate-20261104`. - Retain the frozen candidate baseline. Add the accepted failed-action completion feedback hint, canonical configured environment delivery to OpenCode, actual native worker output receipt checks, timeline attribution controls, and credential clause safety checks. The feedback change leaves the existing acceptance gate in place. - Declare exactly seven oracle-version changes: timeline v5 to v6; personal v5 to v6; company v4 to v5; secure API key, least-scope delivery, secret proposal and vault reference each external v1 to v2. The four external cases have explicit overrides; the shared default is unchanged. - Sync the manifest rows, explicit changed-field records, descriptor snapshot and expected target. Preserve all 244 identities, original queries, human/native counts, clocks, model, profile, launcher bytes, privacy limits and the existing 196/244 qualification gate. - Include the reviewed retry cancellation readiness fixture repair from `815d47e4b5d7756367d2c5e2da656b640273eac2` in this infrastructure PR only. It awaits the first scheduled batch before polling its state. Its production behavior and assertions are unchanged. This test repair is absent from the model candidate. - Keep the core PRs, including #19027 and #19031, unmerged. A normal merge of this pin does not merge those PRs. ## Verification - Pin-local checks: all 189 existing manifest, TestOps API, adapter, reconciliation and streaming controls passed. The reviewed 815 fixture repair passed its owning real-PG control (one selected test). A temporary delayed-first-batch control failed with the old observer and passed with the new observer. Batch limits, assertions, drain wait and overall budget are unchanged. The original CI cause remains unknown. - Final candidate: 89 credential/receipt consumer controls passed. The exact Runner TypeScript build, 68 driver controls and one owning failed-action PG control passed before the later helper-only changes. Their runtime inputs remain byte-identical; those results are reused explicitly, not claimed as new executions on every source commit. - Real pinned OpenCode 1.18.34 executed a canned local-provider bash call. Synthetic `PUBLISHING_API_KEY` reached its child in a 2.89-second passing control with zero paid provider calls. This proves server-to-bash inheritance; driver spawn controls cover the preceding projection boundary. Actual AWS secure-entry, worker delivery and privacy acceptance remain pending. - **The latest local candidate declaration loader is NOT GREEN.** It exceeded its unchanged 30-second limit in both a combined run and an isolated run. Five other declaration/contract checks passed. Both failures remain recorded. No timeout was widened and no further retry occurred. Descriptor and definition modules match the earlier passing 5aa source; only the transitive credential helper changed. Prior 242-declaration evidence supports unchanged definitions only, not a claim that these failed executions passed. - The snapshot matches the candidate and exactly seven oracle-version leaves differ from source 249. All 244 identities, queries, run/human counts, clocks, model, profile, launcher and privacy limits remain unchanged. The SDK guard reports 506 unchanged inputs, unchanged runtime pins and registry `efa70621e9fb42aa185f7ee616a1a726aa3eccb367e67e28817cedf26ef2ade8`. The driver is outside those SDK roots. No SDK renewal occurred. Reporting-lock and whitespace checks passed. - Genuine exact-head reviews: source 19e3 is 5/5 with no unresolved threads; current pin 988 is 5/5 with no unresolved threads. The prior pin 4/5 and resolved P1 remain historical. Current exact-head normal CI 41917215227 completed successfully: 48 successful jobs and one selector skip, including final verify job 126888275415. Windows 41917215440 succeeded. The full PR rollup is 56 successes and three intentional skips, with no pending or failed checks. Older green heads do not qualify this head. Paperclip has only an earlier-head 5/5 assessment, which is stale for this head and receives no current assessment credit. - Six of ten cases have independent passes across prior attempts. Launch 2657 retains three FAILED originals; no grade was changed. The next measurement is the exact uniform ten, cap eight and zero automatic retries. No uniform-ten or full-catalog acceptance is claimed. - The local-tests checkbox below covers only the 189 pin controls and owning fixture control. It does not claim all candidate or repository tests are green. Full local repository checks were not repeated for this infrastructure diff. Core PRs remain unmerged. No model call, eval dispatch, private archive read, fleet change or merge occurred for this update. ## Risks Environment delivery crosses a real provider process boundary, so secret isolation remains part of the next actual AWS acceptance. A worker receipt must match an actual successful native run and key-authenticated endpoint call; a copied boolean does not qualify. Oracle safety and attribution changes can alter acceptance, so balanced negatives remain required. No current uniform-ten pass is claimed. Normal CI and genuine exact-head review must finish before protected infrastructure merge. Root owns the next TestOps Submit, with the original model, limits, zero automatic retries and maximum eight concurrent case chains. ## Model Used OpenAI `gpt-6.1-sol` through Codex. Capabilities used: source analysis, mechanical integration, local code execution and PR preparation. The host does not expose the context-window limit. The evaluated model remains OpenCode/OpenRouter `deepseek/deepseek-v4.1-flash-1017`. ## 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> |