mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 22:13:59 +01:00
5d9f6c56c2b325297c467005bf619da16dfd7a75
1847
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
ffd9591ab4 |
fix(recovery): preserve cancelled files after a reusable sandbox stops (#19305)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run writes its task files in a remote sandbox. The task
workspace records that run as its writer until the files are saved.
> - When a user stops or reassigns a run, the cancelled-workspace
preservation worker records the unsaved files as file recovery work.
> - A warm run (`keep_running`) that is not successful releases its
reusable sandbox through stop-and-retain. That release can finish before
the worker sees a terminal coordinator.
> - The worker accepted only an active lease or a released ephemeral
lease. It ignored the released reusable lease, so it never recorded the
file recovery.
> - The workspace kept the ended writer's hold. Each successor run
waited, then failed with "The workspace still has an unfinished writer
or save operation" after the one-hour cutoff.
> - This pull request accepts a released reusable lease that has a
confirmed stop-and-retain receipt as a preservation source.
> - The benefit is that the stopped run's files become recovery work,
and the task is not stranded behind a hold that nothing settles.
## Linked Issues or Issue Description
Refs #18820 (preserves files when Stop finds no live controller). Refs
#17677 (schedules cancelled workspace preservation). Refs #19285 (parks
a workspace wait behind an ended writer).
**What happened?**
A board user reassigned a task during a warm native run. Stop cancelled
the run with `issue_reassigned`, and the Stop acknowledgement was
recorded. The executor settled the coordinator to `terminal_failure`
with no owner. About eight seconds later, terminal release stopped and
retained the reusable Daytona sandbox. The lease became `released` with
`leasePolicy: reuse_by_environment`, a `stopped` termination receipt and
a stop-and-retain receipt. The task workspace stayed `dirty` with
`writerRunId` set to the cancelled run. No file recovery was recorded.
The new assignee's runs waited as `workspace_busy` for one hour. Then
each run failed `setup_failed`, and the task became blocked.
The heartbeat run row showed `nativePhase: "observed"`. That value is
stale: the checkpoint persist wrote it during teardown. The coordinator
itself was `terminal_failure`: the run's execution projection was
`recovery_needed` with no matching recovery action.
**Expected behavior**
The preservation worker records the stopped run's prepared files as file
recovery work. The lease keeps its release status and receipts. No
second stop and no new provider turn start.
**Steps to reproduce**
1. Cancel a native run with an acknowledged run-only Stop. Its remote
workspace reference is `prepared`, and the coordinator is
`terminal_failure` with no owner.
2. Release its reusable lease through stop-and-retain (`released`,
`reuse_by_environment`, confirmed stop receipt).
3. Run the cancelled-workspace preservation worker or the startup scan.
4. On master, the run has no `workspaceRestoreRecovery`, and no recovery
action exists.
## What Changed
-
`server/src/services/native-runtime/native-failed-workspace-recovery.ts`:
add one source check, `cancelledWorkspaceSource`, for cancelled
preservation. The worker check and the startup scan both use it. It
keeps the active and ephemeral cases. It adds a released reusable lease
only when the stop-and-retain receipt is confirmed
(`hasConfirmedSandboxStopAndRetain`) and `preparedRemoteWorkspaceSource`
proves the exact remote-workspace binding. This is the same proof the
restore recorder uses. Native-sync runs have no such proof and stay
ineligible. An ineligible source causes no run write or notification.
- The restore recorder (`preserveLegacyWorkspaceRestoreSources`) already
accepts this source. It checks the confirmed receipt and the exact
prepared workspace binding, and it keeps the lease status, policy and
receipts. This pull request does not change it.
- `native-cancelled-workspace-preservation.test.ts`: add the incident
shape as a regression for the worker and the startup scan, and add
safety cases.
- `doc/execution-semantics.md`: document the stopped reusable source and
the writer hold until recovery.
## Verification
From `server`:
```sh
npx vitest run src/services/native-runtime/native-cancelled-workspace-preservation.test.ts
```
- Red before the fix: the two new regression cases failed (`expected {
nativeCancellation: … } to match object { …, workspaceRestoreRecovery: …
}`). 30 other cases passed.
- Green after the fix: 36 passed.
- Review follow-up: the cases `invalid-receipt`, `expired-lease` and
`native-sync` failed on the previous head
|
||
|
|
dd889e6a0b |
feat: show the assignee's provider usage limit in task properties (#19270)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run on the native Runner through the Codex app-server and the Claude ACP harness > - Both harnesses report the provider account's rate-limit windows: Codex sends `account/rateLimits/updated`, and Claude sends its SDK `rate_limit_event` as `usage_update._meta["_claude/rateLimit"]` > - Paperclip dropped both signals, so an operator could not see that an agent was near or past its provider limit > - This pull request normalizes both signals into one account-scoped snapshot, keeps the latest snapshot per agent, and shows it as one "Usage limit" line in the task properties pane > - The benefit is that an operator sees "5h 78% · resets 15:40" (amber above 80%, "Limited until …" when the provider refuses work, "using credits" when a used-up window falls back to credits) next to the assignee, and can click it for every window ## Linked Issues or Issue Description Refs #19246 Refs #19259 **Is your feature request related to a problem?** Codex and Claude report provider rate-limit windows during a run. Paperclip dropped them: the Codex notification fell through the Rust normalizer with no branch, and ACPX dropped Claude's `_meta`. Operators could not see how close an agent was to its provider limit. **Describe the solution you'd like** Store the latest reported windows per agent and show one compact line in the task properties pane, with a popover that lists every window. **Describe alternatives you've considered** A full gauge in the agent list and the run header. A design review chose one property line with a details popover instead. ## What Changed - Runner (Rust, the runnerd path): new `provider_rate_limits.rs`. Codex `account/rateLimits/updated` becomes a `provider.notice.recorded` event (category `provider.rate_limits`, scope `account`). `CodexProviderState` merges sparse updates and emits a notice only when the windows change. ACPX `usage_update` with a sanitized `rateLimit` field also emits the notice. - Runner (sidecar): `acpx-runtime-sidecar.ts` forwards only `status`, `rateLimitType`, `utilization` and `resetsAt` from `_meta["_claude/rateLimit"]`. This needs the ACPX `_meta` passthrough from #19259. Until that merges, Claude emits nothing. - Runner (TypeScript in-process drivers): the same mapping in `codex-session-notifications.ts` and `codex-acpx-driver.ts` through `drivers/provider-rate-limits.ts`. - Shared: `provider-rate-limits.ts` holds the snapshot type, the parser, the merge, and `summarizeProviderRateLimits` (fullest current window, expiry by reset time, limited versus using credits). - Server: `agent-provider-rate-limits.ts` merges each notice into `agent_runtime_state.state_json.providerRateLimits`, scoped by company and agent. A session reset keeps the snapshot. No migration. - UI: `AgentRateLimitPropertyValue` adds a "Usage limit" row under the assignee in `IssueProperties`. Transcripts treat the notice as run-log only. A run status event refreshes the assignee's runtime state. - Storybook: `Tasks and conversations/Task creation and lifecycle/Usage limit property`. - Docs: `doc/run-log-events.md` describes the notice and its storage. ## Verification - `cd packages/paperclip-runner/runner && cargo test -p paperclip-runner-core --lib rate_limit` and `--test acpx_provider_events`: sparse Codex merge, dedupe, reached window, checkpoint round trip, Claude utilization and rejection, notice shape. - `cd packages/paperclip-runner && npx vitest run src/drivers/provider-rate-limits.test.ts src/drivers/codex` - `cd packages/shared && npx vitest run src/provider-rate-limits.test.ts`: summary, expiry, limited, using credits, parse and merge. - `cd server && npx vitest run src/__tests__/agent-provider-rate-limits.test.ts`: create and merge, out-of-order snapshot, company isolation, unrelated notices, session reset keeps the snapshot. - `cd ui && npx vitest run src/components/transcript src/components/issue-properties src/context/LiveUpdatesProvider` - `pnpm -r typecheck` and `pnpm check:token-gates` pass. - Live test on a local worktree instance: a native Codex agent on a ChatGPT subscription login ran a real task. The runner recorded one `provider.rate_limits` notice. The server stored it. The task properties pane showed "7d 100% · using credits" (this account's weekly window was used up and had credits, and the run succeeded). The transcript showed no extra row. - Not verified live: Claude. The local Claude CLI was not signed in, and the Claude path also needs #19259. The Rust and sidecar paths have tests. - Not run locally: the full `pnpm test:run` and `pnpm build`. CI runs them. ## Risks - Low risk. The snapshot is display data. It does not hold or start work. The existing provider-limit wait (#19246) still reads typed run failures. - Codex can send a rate-limit update often. The runner emits a notice only when the windows change, so run-log growth stays small. - With API-key auth, the harnesses report no windows, so no row appears. This is the intended result. - `CodexProviderState` gains two optional serde fields with defaults, so older checkpoints still load. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, browser) 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: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
31bf6e4482 |
Pin plugin Runner cleanup to its physical task (#19300)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Plugin Runner owners manage a physical task through an environment lease. > - The existing replacement path can assign a new physical task to that same lease. > - A delayed old owner can then stop the replacement or mark it stopped. > - Cleanup must keep the original physical task identity across RPC and database writes. > - This pull request pins that identity and keeps cleanup available to the new owner. ## Linked Issues or Issue Description Refs #19134 and #19157. This fix is based on master and does not enable repeated stopped-source recovery. A search found no duplicate fix. **What happened?** The maintained `createOwnedSandbox` replacement path changes `providerLeaseId` on one lease. An old task owner uses that mutable lease for a later Stop. A stopped-state response already in flight can also update the new task's flags. **Expected behavior** The old owner must address its original task. Its late cleanup writes must not alter the replacement. The replacement's own owner must still stop and complete it. **Steps to reproduce** 1. Create a plugin task owner for a stopped task. 2. Replace that task through the existing describe/startup path on the same lease. 3. Resume an old checkpoint or a held old status response. 4. Observe a Stop for the replacement or an incorrect stopped flag before this fix. ## What Changed - Require each task owner to keep the physical ID returned by its description. Reject a rebind to a different task. - Carry the ID through the existing internal task context after startup ends. Reject a changed physical task before RPC dispatch. - Match the physical task in stopped-state, completion and credential-handoff writes. - Hold the exact lease row through the retained-session database update and local checkpoint save. - Cover the maintained replacement path with real PostgreSQL tests. Preserve positive cleanup for the replacement owner. Document this boundary. ## Verification - Both replacement regressions failed on unmodified master `10a7d039c8`: delayed old-owner Stop targeted the replacement, and a held old status response marked it stopped. - Six focused files passed 160 tests: task authority, task runtime, warm tasks, execution target, credential writeback and task owner. - Four selected executor factory cases passed. They cover task-only providers and the physical task binding after the startup context ends. - Runner TypeScript, plugin SDK and adapter-utils builds passed. Server TypeScript check passed. Diff and added-line secret scans passed. - All 15 setup-probe cases passed with the actual Runner and fake provider binaries. The first local attempt lacked cargo on PATH. A sandboxed native attempt timed out in seven process cases; the unchanged run with loopback permission passed all 15. A separate initial DB integration attempt was denied a loopback listener; its permission-correct run passed. - Review found three missed constructors in the existing SDK/Core lifecycle fixture. They now use the actual describe result. All four cases in that real PostgreSQL and worker RPC file passed. - The first GitHub attempt failed in the unchanged GitHub recovery pagination case in `chat-channels.integration.test.ts`: its zero-request assertion observed a configuration GET. That exact case passed locally without code or assertion changes. The one failed-job-only retry passed; the initial failure log is retained. - [GitHub workflow 42082587576](https://github.com/paperclipai/paperclip/actions/runs/42082587576) passed on its second attempt. All CI checks completed, and Snyk passed. Standard Greptile scored exact head `fb914d98b99e` 5/5 with no unresolved threads. The branch is mergeable and ready for review. ## Risks The task context change is internal. It adds no plugin wire field, schema or Runner mode. A stale owner now gets an error and cannot clean up a replacement. A new owner can still clean up that task. The retained checkpoint write holds one existing lease row during a bounded local file write. Repeated stopped-source incarnation admission, runtime pins and deployments are outside this change. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), high reasoning. Used repository tools, TypeScript and real PostgreSQL tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used with version and capability details - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR using the bug template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change - [x] I have run the focused tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no unresolved findings - [x] I will address reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
8aa28940bf |
fix(sentry): keep a live-heartbeat native capacity retry out of Sentry (#19320)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends unexpected run failures to Sentry, and it keeps known, recovered failures local > - A native Codex "model at capacity" failure gets an automatic retry. That failure must stay out of Sentry while its retry owns recovery (#18295, #19051) > - The check reads the `nativeProviderFailure` diagnostic from the failed run's `resultJson`. On the live heartbeat path, the failed-status write replaces `resultJson` with the adapter's copy, and that copy has no diagnostic > - So every live-path capacity failure with a scheduled retry still goes to Sentry. Only the reconciliation path, which the tests cover, kept the failure local > - This pull request removes the diagnostic check. The scheduled retry run is the proof > - The benefit is that recovered capacity failures no longer create Sentry noise on the normal live path ## Linked Issues or Issue Description Refs #18295 Refs #19051 **What happened?** A native Codex run failed with `native_provider_overloaded`. The committed status decision scheduled the automatic capacity retry, and the retry run existed before the report. The run failure still went to Sentry. In a staging sample, every live-path capacity failure since #19051 was reported (1 of 1). **Expected behavior** When the `native_provider_overloaded` retry run exists for the same task, the failed run does not go to Sentry. The retry reports its own outcome. **Steps to reproduce** 1. Run a native Codex agent through the live heartbeat path (`finalizeNativeRun` without `projectRunStatus`). 2. Let the provider return `serverOverloaded`. The status decision schedules the capacity retry. 3. The heartbeat writes the terminal `failed` status with the adapter's `resultJson`, and `emitTerminalAgentTaskRun` calls `reportRunFailure` with that row. 4. The row has no `nativeProviderFailure`, so `isScheduledNativeCapacityRetry` returns false and the event goes to Sentry. **Relevant logs or output** The failed run row later shows `nativeProviderFailure: codex/server_overloaded`, because a later reconciliation write restores it. The row that the heartbeat write reports does not have the field. The same overwrite also explains the missing `finalizationReasonCode` that #19051 fixed. ## What Changed - `server/src/services/run-failure-report.ts`: `isScheduledNativeCapacityRetry` no longer requires `resultJson.nativeProviderFailure`. The run must still be native and failed, carry `native_provider_overloaded`, have a native task, have no signal, and not be in setup. The same-task successor with `scheduledRetryReason: native_provider_overloaded` is still required. - The successor is enough proof. Only the committed capacity status decision creates that successor, and only `forTerminal` in `native-provider-failure.ts` produces `native_provider_overloaded` together with the typed `codex`/`server_overloaded` diagnostic. - `run-failure-report.test.ts`: a new case keeps the failure local when the row has only the adapter's `resultJson` (no diagnostic, no finalization fields) and the successor exists. The "diagnostic is missing" case now reports only when no retry run exists. The case for a non-capacity diagnostic with a capacity successor is removed, because that state cannot occur. - `doc/observability.md`: the native capacity paragraph now says that the successor is the proof and that the run's own `resultJson` is not required. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/__tests__/run-failure-report.test.ts`: 284/284 pass with this change. - With the `master` version of `run-failure-report.ts`, the new case fails: 1 failed, 283 passed. So the test catches the bug. - `native-provider-failure.test.ts`: 57/57 pass. - `native-provider-overload.integration.test.ts` did not load in my sandbox, because `@paperclipai/plugin-sdk` was not built. CI must run it. ## Risks - Low risk. The change only removes one condition from a Sentry filter. - These failures still report: no retry run, exhausted or held retries, other retry lanes, a successor for a different task, legacy runs, signals, timeouts, setup failures, and unreadable retry evidence. - The change does not affect run status, retries, recovery, or the `all` reporting mode. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use (shell, repository reads, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
62a5aa7705 |
fix(runner): forward configured bindings into OpenCode (#19319)
Forward controller-selected task bindings into OpenCode shell commands. Keep OpenCode runtime configuration isolated. Redact raw and JSON-escaped binding values while preserving the non-secret environment manifest. Fixes #19257 Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
8464a8c9ae |
fix(native): settle a bootstrap retry that lost its task lock (#19307)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run that fails before any provider work gets a bounded same-run bootstrap retry. Restart recovery decides whether that retry can run. > - The retry needs the task's issue lock. A user comment or another wake can move that lock to a new run before the retry is due. > - Restart recovery then returned `issue_execution_lock_changed` without a write. The same check ran on every recovery pass and never ended. The coordinator stayed `retryable_failure`. > - The orphaned-lease sweep waits for a terminal coordinator before it preserves the workspace. So the sandbox lease stayed active, and every later run for the task was cancelled as `workspace_busy`. > - This pull request gives a lock-lost bootstrap retry the same bounded admission that a superseded remote retry already has. > - The benefit is that the retry settles, the board gets a reconciliation action, and the sandbox lease is released instead of blocking the task. ## Linked Issues or Issue Description No public issue exists. Bug description: **What happened?** On a hosted instance, a native agent run failed at startup because the provider rejected `session.open` with a rate limit. Paperclip scheduled a same-run bootstrap retry. Seconds later, a new run for the task took its issue lock, most likely woken by a user comment. From then on, every restart recovery pass for the old run returned `blocked` / `issue_execution_lock_changed` and wrote nothing. The run recorded about 1,000 recovery events over 12 hours. Its coordinator stayed `retryable_failure` with `recoveryMode: bootstrap_retry`. The scoped orphaned-lease sweep found a pending workspace export for the lease. It could not preserve the workspace, because `preserveNativeFailedWorkspace` requires `terminal_failure`. So the lease stayed `active`, and 16 later runs for the task were cancelled as `workspace_busy`. Three more runs on the same instance were stuck in the same state. A board Stop on the failed run moved the coordinator to `terminal_failure`, and the sweep then released the lease within about a minute. **Expected behavior** A bootstrap retry that cannot get its task lock back settles within a bounded number of recovery passes. The coordinator reaches `terminal_failure`, the board gets a reconciliation action, and the existing orphaned-lease sweep can preserve the workspace and release the sandbox lease. **Steps to reproduce** 1. Fail a native remote run before provider work, so its coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry`. 2. Move the task's `executionRunId` to a newer run for the same task, or clear it. 3. Run `claimNativeRestartRecoveries` for the failed run. 4. Before this change, it returns `blocked` with no `retryAt`, and the coordinator stays `retryable_failure` on every pass. The new tests in `native-remote-retry-admission.test.ts` reproduce this. **Paperclip version or commit** Observed on a hosted deployment. Reproduced in tests on `master` at `10a7d039c8e`. Related: #19190 (a `session.open` model rejection no longer schedules a doomed bootstrap retry), #19131 (preserve superseded observed workspaces) and #19281 (release the leases of a cancelled legacy run). None of them changes this admission path. ## What Changed - **Restart recovery (`server/src/services/native-runtime/native-restart-recovery.ts`)**: a new condition, `lockLostBootstrapRetry`, matches a failed run whose coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry` and whose issue lock is not on the run. - A newer run for the task: the existing superseded check now covers this case too. It defers with `native_retry_source_superseded` as permanent, so admission is exhausted at once. - No newer run (the lock was released): recovery defers with `issue_execution_lock_changed` through the existing `deferRemoteRetry`. That path persists a deadline, backs off and exhausts after the system recovery budget. - When admission is exhausted, the existing hold path writes `terminal_failure` / `native_restart_recovery_blocked`, keeps the provider evidence and creates the board reconciliation action. This PR adds no new terminal path. - A bootstrap retry that still holds its own lock is unchanged. The exact-checkpoint remote retry path is unchanged. - **Docs (`doc/execution-semantics.md`)**: the bootstrap retry section now states the lock-lost rule. ## Verification - New test `settles a %s bootstrap retry that lost its task lock instead of re-checking forever` in `server/src/services/native-runtime/native-remote-retry-admission.test.ts`, for the `superseded` and `released` cases. The fixture is a genuine pre-provider failure: a `session.open` rejection with no session checkpoint, no runner checkpoint row and no provider evidence. - On `master` without the fix, both cases fail: recovery returns `blocked` without a deadline, and the coordinator stays `retryable_failure`. - With the fix, `superseded` settles on the first pass, and `released` settles after `SYSTEM_RECOVERY_MAX_RETRIES` deferred passes. Each deferred pass persists its `nextAttemptAt`. Both end in `terminal_failure` with one board recovery action, and later passes open no transaction. - `npx vitest run src/services/native-runtime/native-remote-retry-admission.test.ts` (from `server/`): 92 passed. - Related suites, each run on its own with exit code 0: `native-ingress-bootstrap-retry`, `native-startup-retry`, `native-scheduled-resume-lock`, `native-restart-recovery-isolation`, `native-runner-restart-recovery`, `native-runner-restart-preparation`, `native-runner-restart-diskless`, `native-warm-transition-restart`, `native-controller-recovery`, `native-failed-workspace-recovery`, `native-session-resumption`, `native-startup-cleanup-recovery`, `heartbeat-observed-workspace-preservation`, `heartbeat/recovery`, `runtime-selection`, `remote-workspace-migration` and `native-session-executor` (1,149 tests). Two other suites each had one local failure that this change does not cause: - `heartbeat-process-recovery`: "redacts opaque environment-bound credentials from Sentry diagnostics" fails the same way on this machine without the change. - `chat-channels.integration`: "settles a shared-endpoint accepted-response backlog without retrying its own locks" failed once under full-suite load. It passes alone, with and without the change. - `pnpm run typecheck` in `server/`: passed. ## Risks - Low to medium. The change only affects failed bootstrap retries whose lock is not on the run. Before this change, such a retry could never run, so no working path is removed. - A released lock that comes back to the same run during the backoff still resumes normally, because the condition reads the current lock on each pass. - When the retry settles, the task gets a board reconciliation action. Before this change, the task was silently stuck behind `workspace_busy`. - No migration and no API change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, code search, test runs), extended thinking, and a read-only investigation subagent for the root cause. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
74b11014c6 |
fix(server): stop a queued answer from blocking saved-work recovery (#19318)
## 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. If a server restart interrupts the copy-back, the run keeps an owner receipt for the server that stopped. > - Another server takes over only after the cloud platform proves that the old deployment stopped. After that proof, recovery checks that the task has not moved on, so old files never overwrite newer work. > - That check counted any queued run on the task as newer work. > - The user's answer to the run's question queues the assignee's next wake. That wake cannot claim while the saved run holds the task's execution lock, so it waits for the copy-back. > - Recovery waited for the wake and the wake waited for recovery. The task stayed stuck, and nothing was logged. > - This pull request ignores a later wake for the same assignee that has never started, and logs waits that the worker stops retrying. > - The benefit is that answering a question during an interrupted copy-back no longer strands the task. Every other newer-work check still applies. ## Linked Issues or Issue Description Refs #15824 (added platform-verified takeover and these binding checks) and #19111 (worker concurrency and ordering for the same copy-backs). This pull request keeps the rest of #15824's fence unchanged. **What happened?** On a hosted instance, a server restart interrupted a copy-back after the worker had written its owner receipt. The run had asked the user a question, and the user answered. The answer's wake was queued. Every later server got a valid stop proof for the old deployment and then got `binding_changed` from the shared binding check: the only failing condition was the queued answer wake. The worker treats `binding_changed` as a final wait and returns without logging. The run stayed `running` and the wake stayed `queued` for about 19 hours, across many restarts. **Expected behavior** A wake that is waiting for the saved run does not stop the saved run's recovery. Runs that could change the workspace still prevent takeover, and the publication check still stops a copy-back if such a run appears. **Steps to reproduce** 1. Accept a native result and leave a copy-back owner receipt from a stopped deployment. 2. Queue the assignee's next wake on the same task, for example by answering the run's question. 3. Let the reconciler or the workspace worker revisit the saved result with a verified stop proof. On `master` the takeover returns `binding_changed` and nothing is copied or committed. The new database tests run these steps against PostgreSQL. **Paperclip version or commit** `master` at `03eb49053fe`. **Deployment mode** Cloud-managed instances with platform stop verification. The same binding check also guards operator owner recovery, so self-hosted operator recovery had the same deadlock. ## What Changed - `server/src/services/native-runtime/native-workspace-recovery-binding.ts`: the newer-run check ignores a run when all of these hold: - It is `queued` and has never started (`startedAt` is null). - It belongs to the run's agent, and that agent is the task's assignee. - It was created after the saved run. - Its wake reason is not `source_scoped_recovery_action`. The run claim blocks such a run while the saved run holds the issue's execution lock (`lockIssueExecutionClaim` in `heartbeat/queue.ts`). Runs that have started, other agents' runs, recovery-action wakes and older queued runs still prevent takeover. - `server/src/services/native-runtime/native-workspace-finalization-work.ts`: log `runId` and `reason` when the worker stops retrying a wait (`binding_changed`, `owner_unverified`). It does not log `owner_active`. - `doc/native-workspace-finalization-recovery.md`: describe the waiting-wake rule in the shared binding section. - `server/src/services/native-runtime/native-workspace-queued-wake-recovery.test.ts` (new): the queued-wake tests, with a small fixture of their own. They are in a separate file so `native-workspace-finalizer.test.ts` stays under 1,000 lines. ## Verification - Failing first. These tests in `native-workspace-queued-wake-recovery.test.ts` fail against the original check and pass with this change: - Operator owner recovery with an answer wake queued. - Automatic takeover with the wake still queued. - Automatic takeover where the wake starts during copy-back. On `master` takeover is refused before this point; with the change, publication is stopped. - The real reconciler, from `retryable_failure`, with a stopped-deployment owner and a queued answer wake. It now copies once, commits the saved result and leaves the wake queued with its claim no longer blocked. - The fence is kept: three new cases (a requeued wake that already started, a recovery-action wake, another agent's queued run) still get `binding_changed`. They pass both before and after the change. - Changed test: "does not reopen an old result after a newer task execution exists" now inserts a started run instead of a never-started queued run of the same assignee. The queued case is now the new operator-recovery test above. - `cd server && npx vitest run src/services/native-runtime/native-workspace-queued-wake-recovery.test.ts src/services/native-runtime/native-workspace-finalizer.test.ts`: 2 files, 72 passed. With the original binding file restored, the 4 positive queued-wake tests failed and the 3 fenced cases passed. - Related suites (finalization work, platform proof, export retry, finalizer failure, recovery actions, owner-recovery routes, status reassessment, scoped deadline work): 8 files, 499 passed. - After rebasing onto `03eb49053fe`: finalizer and finalization-work suites, 97 passed. - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Not verified on a live instance yet. The diagnosis comes from that instance's run, finalization, workspace-operation and recovery-action rows, plus a direct stop-proof request that returned a valid receipt. ## Risks - A copy-back can now finish while the assignee's wake is queued. The same-process retry path already does this, because it does not run the binding check. The takeover path now matches it for wakes that cannot start yet. - If some path starts such a wake despite the execution lock, `assertHeld` sees a running newer run and stops before publication. The window between that check and the write is the same as for the other binding conditions. - 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> |
||
|
|
55389554c2 |
fix(runner): paginate workspace discovery before journal persistence (#19311)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents discover reusable workspaces through `list_workspaces`. > - The tool returned every authorized workspace in one result. > - A large inventory exceeded the durable command journal limit. The result stayed pending, and session cleanup blocked the next turn. > - This pull request limits discovery in SQL and returns explicit continuation metadata. > - Agents can inspect every authorized workspace without sending the full inventory in one command. ## Linked Issues or Issue Description Related: #15749 introduced the task workspace tools. No duplicate pagination fix was found. **What happened?** A native workspace discovery call returned a multi-megabyte inventory. Persisting its result failed with `command_payload_too_large`. The preceding answer could still be saved, but cleanup retained the unsettled tool call. A later turn failed before provider work with `native_session_cleanup_quarantined`. **Expected behavior** Workspace discovery must return a manageable page. Agents must be able to request the remaining authorized rows. Existing visibility checks and journal integrity checks must stay effective. **Steps to reproduce** 1. Create 2,000 task-selectable workspaces with ordinary repository paths. 2. Call `list_workspaces` from an admitted native run. 3. On the old code, the real API regression returns 869,781 bytes. The command journal allows about 510 KiB. 4. On the repaired code, follow `nextCursor` to inspect all rows. Pages contain at most 100 rows and fit within 128 KiB. **Paperclip version or commit** Reproduced on source commit `7351a9d74f` before the repair. **Deployment mode** Isolated source checkout with real PostgreSQL and authenticated native agent APIs. Hosted deployment and incident recovery remain separate. ## What Changed - Validate workspace list limits and UUID cursors with a JSON Schema pattern supported by strict validators. Return `workspaces`, `limit`, `returnedCount`, `hasMore`, and `nextCursor`. - Apply company, visibility, lifecycle, and cursor conditions before the SQL limit. Order pages by workspace ID. Fetch one extra row to detect continuation without a count query. - Fit encoded pages within 128 KiB, including JSON escaping and Unicode. If one summary cannot fit, return explicit string previews with `truncatedFields`. The authorized detail API retains its full record. - Keep existing unpaged summary callers on their array response and recent-use order. - Update the native tool descriptor, generated contracts, catalog fixture, and execution documentation. - Add regression coverage for large inventories, complete pagination, stable pages, private and foreign rows, invalid inputs, and legacy summary behavior. Add an opt-in two-turn live Codex acceptance check. ## Verification - Red before repair: `task-workspace-api.test.ts -t "pages a large authorized workspace inventory"` failed because the result was 869,781 bytes instead of below 128 KiB. The transport regression also failed at 2,284,671 bytes. - Green after repair and review changes: 120 tests passed across the workspace API, transport, routes, native authority, coordinator, historical-question, and Codex vertical-slice suites. They cover complete discovery, byte-sized pages, oversized individual summaries, full detail retrieval, private/foreign/lifecycle exclusions, and strict schema compatibility. - Journal safety: two focused durable control-plane tests passed. Oversized result rejection and lossless valid result delivery remain effective. - Tool catalog and reconciliation: 30 tests passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed again on final commit `bc10880da6`. Server typecheck also passed after the review changes. The original affected suites passed on the rebased first commit (31 tests), and the expanded review regression set passed afterward (120 tests). - Real provider acceptance passed again on final commit `bc10880da6`: local macOS arm64, rebuilt native Runner, `gpt-6.1-sol`, high effort, per-turn lifecycle, and 2,000 persisted workspaces. Both turns succeeded. Persisted call inputs and results prove that the second page uses the first result’s cursor, both calls succeed, their IDs do not overlap, and the encoded results stay within 128 KiB. The second turn kept the provider conversation and file. Both task locks were clear after completion, and no recovered execution failure was recorded. Duration: 166.92 seconds. - Full local `pnpm test:run` stopped in the chat shard: 330 tests passed and one Slack registration test failed with `socket hang up`. That exact test passed in isolation without changes, and the final-head CI chat shards passed. A broader local managed-Slack follow-up produced no result after about eight minutes and was interrupted; it is not counted as a pass. The original full-suite failure remains recorded. - The first CI head failed native tool-schema compilation because strict validators did not recognize `format: uuid`. The revised descriptor uses a UUID pattern. The previously failing coordinator, native vertical-slice, authority, and history suites now pass locally. The stronger live cursor-receipt acceptance check passed. Final-head CI completed on `bc10880da6`: 57 checks passed and five optional checks were skipped, with no failures or pending checks. Greptile reviewed that commit at 5/5; both prior findings are resolved. - Dependency cost: paged discovery runs the existing visibility-scoped summary query with an ID cursor and at most 101 returned rows. It adds no count query, filesystem scan, provider call, or background worker. ## Risks - Native `list_workspaces` now returns a page object instead of an array. Individual summaries can be explicit previews when a single record exceeds the byte budget. Its advertised descriptor and generated contracts change together. Agents must follow `nextCursor` until null for a complete inventory. - Each page rechecks current visibility. Concurrent insertions, deletions, or access changes require a fresh scan when a complete point-in-time inventory is needed. - No migration or journal-limit change. An existing cleanup quarantine still requires normal recovery after deployment; this PR does not bypass it. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, shell tools, code editing, and test execution. The exact served model revision 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> |
||
|
|
a695398052 |
fix(server): preserve inactive rebase bookmarks in workspace snapshots (#19310)
Preserve a validated standalone REBASE_HEAD and its commit without changing the source workspace. Keep active Git operations, unmerged indexes, invalid markers and concurrent changes blocked. Verified 156 affected tests, workspace typecheck and build. All hosted CI checks passed, and Greptile scored the exact head 5/5 with no unresolved threads. The bounded full local suite stopped during its fifth chat shard; this incomplete result is recorded in the PR. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
5de779848b |
fix(recovery): save a reassignment-stopped writer's files from its stopped reusable sandbox (#19304)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A remote task workspace stays held by its last writer run until that writer's files are saved > - A reassignment stops the old owner's run. The stopped writer still holds the workspace with unsaved files. > - When the run used a reusable (warm) sandbox, the stop leaves its lease `released` under the reusable policy, with a stop-and-retain receipt > - Cancelled-workspace preservation only admitted `active` or `ephemeral` sources. It never recorded the save for this stopped writer, so no workspace recovery ran, and the hold stayed forever > - The new owner waited, then failed setup after the one-hour cutoff with "The workspace still has an unfinished writer or save operation" > - This pull request admits the stopped reusable source, so Paperclip saves the stopped writer's files and releases the hold. A waiter keeps waiting while that save is in progress. > - The benefit is that a reassigned task continues with the old owner's saved work, and the new owner does not fail setup ## Linked Issues or Issue Description Refs #19285 (parks a waiter behind an ended writer). Refs #19074, #18820. **What happened?** A board user reassigned a task during a native run on a reusable sandbox. The old run was cancelled with `issue_reassigned`. Its sandbox was stopped and its lease was released under `reuse_by_environment`. The workspace still named the stopped run as its dirty writer. No save operation and no workspace recovery action were ever recorded for that run. The new owner's runs were cancelled `workspace_busy` repeatedly. After more than an hour, a new run failed setup: "The workspace still has an unfinished writer or save operation." **Expected behavior** Paperclip saves the stopped writer's files from its stopped sandbox within a bounded time and releases the hold. The new owner waits for that save and then continues. If the save cannot finish, its retries end with a named board recovery action. **Steps to reproduce** 1. Give a native run a prepared remote workspace on a lease with policy `reuse_by_environment`. 2. Stop the run (for example by reassignment), so the lease is stopped and released with a stop-and-retain receipt. 3. Run cancelled-workspace preservation. Before this fix, it records nothing, and the writer hold stays. ## What Changed - `native-failed-workspace-recovery.ts`: Cancelled-workspace preservation (the delivery worker and the startup scan) now admits a `released` reusable lease with a stopped termination receipt and a stop-and-retain receipt. - `preserveNativeFailedWorkspace` retains such a reusable source only with the exact confirmed stop-and-retain receipt, and only while the workspace is still bound to that writer (`preparedRemoteWorkspaceSource`). Otherwise it writes nothing. - `remote-workspace-writer-hold.ts`: For an ended writer, the holder cutoff now uses the latest progress of an in-progress save of that writer's retained files (a workspace recovery that is not complete, exhausted, or recovered). Reservation and the parked `workspace_busy` wait both read this rule. A waiter waits for the save instead of failing setup. - `doc/execution-semantics.md`: Documents the stopped reusable source and the save-progress cutoff. ## Verification - Red before the fix, green after: - `npx vitest run src/services/native-runtime/native-cancelled-workspace-preservation.test.ts` — new cases "preserves a reassignment-stopped writer's files from its stopped reusable sandbox" (worker and startup scan) failed before the fix: no `workspaceRestoreRecovery` was recorded. - `npx vitest run src/__tests__/heartbeat-remote-workspace-busy.test.ts` — new cases "keeps a waiter parked while an ended writer's save is in progress" failed before the fix: the parked retry was released past the cutoff. - Safety cases: - A reusable source without a confirmed stop, a workspace now owned by another writer, and an already saved workspace are not retained, and the run row does not change. (The writer and saved cases fail if the new guard is removed.) - Preservation does not replay the stopped run, does not create a wake, keeps the stop receipt and lease policy unchanged, and keeps the writer hold until the save publishes. - An exhausted or complete save does not extend the cutoff. The existing past-cutoff test (no save in progress) still fails setup and keeps the held files. - Related suites: workspace recovery, remote workspace reservation, native workspace reconstruction, native failed-workspace recovery, legacy workspace restore recovery. - Not verified on staging: this change is not deployed. The staging task needs this fix deployed. Then the preservation worker records the save, workspace recovery saves the files, and the waiting owner continues. ## Risks - More cancelled runs are now admitted for preservation. The retention guard requires the exact stop-and-retain receipt and the workspace bound to this writer, so a reusable sandbox that another run resumed or that no longer holds unsaved work is not retained. - A stopped reusable source stays `reuse_by_environment` while Paperclip saves it. This matches the existing retention path for reusable native sources. - The writer-hold read adds one indexed query (`issue_recovery_actions_source_run_idx`) for an ended writer only. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, code editing, test 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> |
||
|
|
d361f92071 |
feat(task-chat): show harness status notices as chips, out of the answer (#19267)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run through the native runner. The runner turns harness events from Codex and Claude into canonical provider events, and the task chat shows them. > - Harnesses report their own lifecycle: context compaction, retries, model fallback, and warnings. Paperclip lost or mangled these reports. > - Claude (claude-agent-acp 0.73) sends compaction, model-fallback and warning banners as ordinary message text, so "Compacting..." ended up inside the agent's answer and the issue comment. > - Codex sends `error` with the text in `error.message` and a `willRetry` flag. Paperclip read `params.message`, so every Codex error said "Provider notice", and every retry looked fatal. > - This pull request keeps these reports out of the answer and shows them as small status chips between the turn's activity. > - The benefit is that users can see why an agent paused, retried or forgot earlier context, and the answer text stays clean. ## Linked Issues or Issue Description **Subsystem affected** Native runner provider-event mapping (TypeScript driver, ACPX sidecar and Rust runner-core) and the task chat transcript. **Problem or motivation** Claude status banners are mixed into assistant answer text. Codex error notices lose their message and their retry state. The task chat hid all of these notices, and settled turns stripped every provider notice. **Proposed solution** Classify Claude banners as provider notices at the ACPX boundary. Map Codex `TurnError` fields and `willRetry` correctly. Render status notices as inline chips. A retry chip fades after later provider work. A final error shows the real provider message. **Alternatives considered** Patch claude-agent-acp to tag banners in `_meta`. ACPX keeps only `origin`, `kind` and `source` from `_meta`, and other sessions own the ACPX `_meta` passthrough work. Banner chunks already have no `messageId`, and model output always has one, so the classification needs no adapter patch. **Roadmap alignment** This makes existing harness signals visible. It adds no new protocol schema. It uses the existing `paperclip.provider.notice.v1` `category` and `details` fields. ## What Changed - Codex `error` notifications (TypeScript `provider-events.ts` and Rust `runner-core/src/provider_events.rs`): read `error.message`, map `willRetry: true` to a recoverable `provider_retry` notice, keep category `error` for a final error, and add the `CodexErrorInfo` kind, HTTP status, and the "Reconnecting... n/m" attempt count as notice details. Retries in one turn share one notice id. - New `claude-status-banners.ts`: classifies the claude-agent-acp compaction, model-fallback and SDK informational banners. A chunk must have no `messageId` and match a known banner exactly. - ACPX sidecar and the TypeScript ACPX driver emit these banners as `provider_notice` events instead of answer text. Compaction start and end share one notice key and carry a `Phase` detail. - Rust runner-core accepts an optional `noticeKey` and bounded `details` on ACPX `provider_notice` events. - UI: new `TaskChatStatusNotice` chip and `status-notice-presentation.ts`. Status notices become top-level timeline rows, render as chips wherever protocol cards render, and stay visible in settled (historical) turns. Generic warnings stay hidden as before. - New tokens `--tc-status-notice-warning-*` in `ui/src/index.css`. - Storybook story `Tasks and conversations/.../Task chat/Status notices` with eight states. - `doc/run-log-events.md`: describes the status notice categories. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/task-chat src/components/transcript` - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/provider-events.test.ts src/drivers/acpx/claude-status-banners.test.ts src/drivers/acpx/codex-acpx-driver.test.ts src/cli/acpx-runtime-sidecar.test.ts` - `cargo test -p paperclip-runner-core --lib provider_events` and `--test acpx_provider_events` - `pnpm check:token-gates` - Storybook: open `product-task-chat-status-notices--all-states-gallery`. - Live test-drive on an isolated worktree instance: - Codex (gpt-5.6-sol, app-server 0.160): a local pass-through proxy in front of the real OpenAI API cut the first two response streams. Codex ran its own retries. The chat showed an amber "Reconnecting" chip live, then "Recovered after retry" once work continued. With every stream cut, Codex made five retries and then sent a final error with the real message. - Claude (claude-haiku-5-5 through claude-agent-acp 0.73): a long reading task triggered Claude's own auto-compaction twice. The chat showed "Compacting context…" live and "Context compacted" afterwards. The final answer and the issue comment contained no compaction text. - Not verified live: model fallback and Claude informational banners. They need a model refusal or a blocking hook. Unit tests cover them. ## Risks - Low to medium. A Claude model message that is exactly a banner text and has no `messageId` would become a notice. Model output from claude-agent-acp always carries a `messageId`. - Older stored Codex `error` notices (category `error`, not recoverable) now render as a red row instead of being hidden. - Claude retries (`api_retry`) are still not visible. claude-agent-acp reports them only to clients that negotiate session-failure records, and ACPX drops `_meta`. This is a follow-up that depends on the ACPX `_meta` passthrough work. - On a failed Codex turn the thread collapses to a summary, so the red row does not show there. The existing task banner already shows the real provider message. - Some local suites failed on this machine. They do not touch the changed code: Cursor Cloud staging-custody tests, the ACPX Claude dependency-graph pin test (a timeout), and the Rust `task_memory` monitor tests. CI is the reference for these. > 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`) in Claude Code, with tool use, code execution and browser automation. Default context window. Extended reasoning was not enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Refs paperclipai/paperclip#19252 (plan card precedent). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
7351a9d74f |
fix(native): restore retired backups after warm lease handoffs (#19299)
## Thinking Path > - Paperclip manages AI agents and keeps their work across turns. > - A native session can keep the same sandbox through several warm lease handoffs. > - Each handoff keeps the old lease row and records matching receipts on both owners. > - The retired-backup guard required exactly one row for a provider allocation. > - That rule rejected valid history after Paperclip destroyed the sandbox. > - This change uses the existing handoff proof to select the final owner. > - A verified backup can then restore without discarding ownership checks. ## Linked Issues or Issue Description Refs #19208. **What happened?** A follow-up received a new sandbox after its old sandbox was destroyed. The old allocation had a normal warm lease handoff. Both lease rows remained in PostgreSQL. The guard rejected them with `runner_harness_state_mismatch: backup_without_reusable_lease`. Its later same-run retry failed with `native_remote_recovery_lease_mismatch`. **Expected behavior** Restore the verified backup when one final owner has a valid destroyed receipt and every earlier owner has a matching handoff receipt. Keep uncertain ownership blocked. **Steps to reproduce** 1. Complete a native warm turn. 2. Hand the reusable sandbox to another turn. 3. Complete that turn and save its backup. 4. Close the workspace and destroy the sandbox. 5. Send a follow-up. The new allocation must restore the saved session. **Paperclip version or commit** Reproduced in the PostgreSQL regression on the parent of this change. The affected deployed build also contains #19208. ## What Changed - Select the single final lease owner. Validate all predecessor rows with `retiredReusableLeasePredecessors`. - Keep the destroyed receipt, exact backup stamp, and newer-provider-work checks. - Add database regressions for one and three handoffs. Reject missing or changed receipts, an active predecessor, missing history, an unrelated owner, a stopped source, another backup, and later provider work. - Document the valid lease-history shape. ## Verification - Red before the source change: the two valid-handoff cases returned false. The other 16 database cases passed. - Green after the change: all 18 database cases passed. They also passed on current head `d16e098b8c2e6c0dda70984c1df21e3dc792ab34`. - Command: `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-retired-backup-source.integration.test.ts`. - The database, executor, retry-admission, and lease suites passed: 4 files, 1,308 tests. - `pnpm -r typecheck` and `pnpm build` passed. The build stamp names head `4ee733c3a979436196522f36537ff182e9df5398`. The full local suite stopped in chat shard 3 after a 15-second timeout in `materializes direct external-chat finals once and accepts the next turn without agent bookkeeping writes`. Earlier shards passed 702 cases; that shard passed 303 other cases. The isolated test passes on this branch and unchanged master `10a7d039c8efeac0fd655c24970df5ae3658414b`. All current-head CI chat shards also passed. The earlier full-suite attempt remains failed; its timeout cause is not established. No test or timeout was changed. CI is waiting for its last Runner job. - Current head `d16e098b8c2e6c0dda70984c1df21e3dc792ab34` removes four comment lines from the previously tested head. Its 18 database cases passed. Greptile reviewed this head at 5/5 with no actionable findings: https://github.com/paperclipai/paperclip/pull/19299#issuecomment-6665446098. CI remains pending. - Live inspection found the matching predecessor/successor receipts and the final owner's destroyed receipt. This was read-only evidence from the affected build. - A live provider run at the fixed revision has not been exercised. Local Daytona credentials and an immutable test image are unavailable. No deployment or original-task retry is claimed. - Initial wider tests could not resolve the unbuilt plugin SDK. The SDK build is now available. A sandboxed database launch could not bind loopback; the same regression passed with the disposable database outside that restriction. ## Risks - A wrong owner could allow an old backup to rewind a session. The shared validator must account for every lease row and match both sides of each handoff. - Live, unrelated, incomplete, or conflicting history stays blocked. The existing backup integrity and provider-progress checks remain. - No migration, permission, model, trust, timeout, or retry-policy change is included. - The original task still needs the fixed build and normal recovery. Hosted acceptance remains unverified. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, code execution, and tool use. The session does not expose the exact model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
10a7d039c8 |
fix(dispatch): keep a workspace wait parked while an ended writer's save settles (#19285)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run that finds its task workspace held by another run waits as a `workspace_busy` scheduled retry. #19143 keeps that wait as one parked retry while the holder is live. > - A remote task workspace stays held by its last writer run until the writer's save and sandbox release settle. This hold continues after the writer run ends, for example when a reassignment stops the writer. > - The parked-wait check counted only a `running` holder. For an ended writer whose save had not settled, it released the waiter at each due check. Preparation deferred again, cancelled the run, and created a new retry run. > - After a mid-run reassignment, the new owner got five cancelled `workspace_busy` runs in eleven minutes, and the task did no work. > - This pull request makes the parked wait use the same writer-hold rule as workspace reservation, so the waiter stays parked until the hold releases. > - The benefit is one durable wait, one start when the hold releases, and no churn of cancelled runs. ## Linked Issues or Issue Description Refs #19143 (parks a workspace-busy wait). Refs #19184 (releases a recovered hold for a new assignee after reassignment). **What happened?** A board user reassigned a task in the middle of a run. The old run was cancelled with `issue_reassigned`. The new assignee's runs were each created, started and cancelled with `workspace_busy`: "Shared execution workspace is busy: run … ended, but its workspace save has not settled". This repeated every 1–2 minutes (five cancelled runs in 11 minutes). Every cancellation named the stopped run of the previous assignee as the holder. The new assignee's other tasks were not the holder. **Expected behavior** The reassigned task waits as one parked retry. It starts once when the stopped writer's save and sandbox release settle. The wait stops at the one-hour holder cutoff. **Steps to reproduce** 1. Give a task a remote execution workspace whose writer run has ended, but whose save has not settled (`writerRunId` set, `dirty: true`). 2. Wake the task's assignee. The run defers with `workspace_busy` and schedules a retry. 3. Make the retry due and promote scheduled retries. Before this fix, the retry is promoted and dispatched, defers again, and creates another run. ## What Changed - New `server/src/services/remote-workspace-writer-hold.ts`: `readRemoteWorkspaceWriterHold` holds the rule for who holds a remote workspace (a running writer, or an ended writer whose save or sandbox release has not settled). The code is moved from `reserveRemoteWorkspace` without a behavior change. - `reserveRemoteWorkspace` uses that function. It throws the same errors as before. - `parkWorkspaceBusyRetryInTx` (from #19143): when the recorded holder is not running, it reads the same hold for the waited execution workspace. If the recorded holder is an ended writer that still holds it, the retry stays parked. The cutoff clock starts at the writer's end, the same value that reservation measures. Earlier waiters keep their arrival order. - New `WORKSPACE_BUSY_SETTLING_RECHECK_MS` (60 s). Settling has no single completion point that can wake the waiter, so a wait behind a settling writer re-checks every minute. Each check creates no run. In the promotion transaction it reads the workspace, the writer run, its task, and the writer's leases (one row per sandbox lease of that run). The checks last only while the wait exists, up to the one-hour cutoff. - `doc/execution-semantics.md`: documents the parked wait behind a settling writer. ## Verification - Red before the fix: `cd server && npx vitest run src/__tests__/heartbeat-remote-workspace-busy.test.ts`. The two new tests failed: the due retry was promoted (`expected [ … ] to not include '<retry id>'`). - Green after the fix, with: - `parks one waiter while an ended writer's save settles, then admits it once the hold releases`: three due checks keep one parked run, each re-check is bounded by the writer's end plus one hour, there is one `workspace_busy` cancellation in total and no new retry run. After the hold releases, the next check promotes the same run. - `stops parking behind an ended writer once its unsettled save reaches the holder cutoff`: parked before the cutoff, promoted after it (preparation then fails with the existing unfinished-writer error). - Existing suites pass: `heartbeat-workspace-busy.test.ts` (live-holder parking, arrival order, stop, workspace move), `modules/run-dispatch`, `remote-workspace-reservation.test.ts`, `remote-workspaces.integration.test.ts`, `isolated-remote-workspace-source.test.ts`, `heartbeat/workspace-preparation.test.ts` (476 tests in all). - `tsc --noEmit` for `server` shows no errors in the changed files. - Not done: a live run on staging. The fix is not deployed. ## Risks - Low. Parking applies only to a retry that recorded a `workspace_busy` wait on an execution workspace whose recorded writer is that holder. If the hold settles, moves to another run, or the task changes workspace, the retry is released to normal dispatch as before. - The wait stays bounded by the existing one-hour holder cutoff from the writer's end. After that, preparation fails as before. A user can still stop the parked run. - One writer per workspace is unchanged. Reservation still decides admission with the same rule. - The one-minute re-check runs only while a wait exists. It is not idle polling. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser reads of staging diagnostics). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
b05d3db2b3 |
fix(recovery): continue a pre-turn interrupt after recovery, not after the destroy (#19292)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can interrupt a native run with a comment. The next turn should start soon after the run stops > - When the stop leaves sandbox files unsaved, recovery saves them, re-stops the sandbox, and then queues the destroy for an ephemeral lease > - #19224 kept the verified stop on that lease as `workspaceRecoveryCleanup.stopReceipt`, and taught the continuation gates to accept it > - One gate was missed: `acknowledgedNativeStopExecutionHasStopped`. An interrupt that lands before the provider turn opens leaves no gating hold, so the saved comment goes through this gate, not through the explicit continuation gate > - That gate still wanted the raw `remoteExecutionTermination` receipt, so the comment waited about 5 minutes for the destroy retry > - This pull request lets that gate accept the carried-forward stop receipt too > - The benefit is that an interrupt during startup continues right after recovery, as #19224 intended for every lease policy ## Linked Issues or Issue Description Refs #19224 Refs #19206 **What happened?** On hosted staging, an `interrupt: true` comment reached a native run of an ephemeral-lease agent while the run was still in startup (the provider pack upload). The run stopped before any provider turn. Recovery saved its files at once. The follow-up turn started about 6 minutes after the interrupt. The stopped run's lease was released by `pending_cleanup_retry`, and the follow-up started 2 seconds later. Agents whose interrupt landed after the provider turn opened continued in 30 to 50 seconds on the same build. **Expected behavior** The saved comment starts a new turn right after recovery saves the files and the sandbox is stopped. The destroy of a stopped sandbox is cleanup and must not hold the next turn. **Steps to reproduce** 1. Use an agent whose sandbox lease is ephemeral (destroy after each turn). 2. Post an `interrupt: true` comment while its native run is still starting, before the provider turn opens. 3. Let recovery save the files. See the follow-up start only after the destroy retry releases the lease. **Paperclip version or commit** Hosted staging at `e37ab23d` (includes #19206 and #19224). ## What Changed - `server/src/services/acknowledged-native-stop.ts`: `acknowledgedNativeStopExecutionHasStopped` accepts a remote lease that has `hasRemoteTerminationReceipt` or `hasRecoveredCleanupStopReceipt`. Before, it accepted only the first one. - `server/src/services/workspace-recovery.test.ts`: new regression for a pre-turn interrupt on an ephemeral lease. Recovery completes, the destroy is queued, and the saved comment is handed to `resumeRemoteStopComments` twice. With a verified carried stop, exactly one turn starts while the lease is still `pending_cleanup`. Without the stop receipt, the comment stays deferred and no turn starts. - `doc/execution-semantics.md`: one sentence that extends the #19224 paragraph to the pre-turn Stop case. ## Verification - Red before the fix: `cd server && npx vitest run src/services/workspace-recovery.test.ts -t "starts the interrupt redirect once after recovery"` failed the `verified-stop` case with `expected [] to have a length of 1 but got +0`. The `no-stop-receipt` case passed. - Green after the fix: the same command passes both cases. - `npx vitest run src/services/workspace-recovery.test.ts`: 382 passed. - Related suites pass: `acknowledged-native-stop`, `explicit-native-continuation`, `heartbeat/run-control`, `heartbeat/recovery`, `heartbeat/native-stop-evidence-read`, `native-session-executor`, `heartbeat-observed-workspace-preservation`, `native-startup-cleanup-recovery`, `execution-control-work`. Two `run-control` cases timed out at 30 s under parallel load and passed when run alone. - `pnpm --filter @paperclipai/server typecheck` passes. - Not done: a live re-run on staging. This change is not deployed. ## Risks - Low risk. `hasRecoveredCleanupStopReceipt` is the same check #19224 added to the other gates. It is true only for a lease whose recovery cleanup belongs to its own run, which was released, and whose carried stop receipt matches the company, run, lease, provider, and provider lease id. A lease without that receipt still blocks. - The no-replay hold is unchanged. The successor still goes through ordinary admission with all task gates. The test calls the resume twice and sees one turn only. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, browser reads of staging). Extended reasoning was used. ## 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> |
||
|
|
b400f76647 |
feat(ui): submit task composers with Enter by default (#19272)
Use Enter to submit task replies and new tasks, with Shift+Enter for a new line. Add an experimental preference to require Command/Ctrl+Enter. Preserve autocomplete selection and IME confirmation, and retain the modifier shortcut until saved settings load. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
c6996799c4 |
feat(task-chat): show an inline diff for every file edit (#19269)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The task chat shows what an agent did during a run, including each tool call > - Codex and Claude both report the patch for every file edit, but Paperclip dropped it. Claude's ACP `diff` content was never read. The Codex `fileChange` item became a generic item with no patch. Codex `turn/diff/updated` had no native-runner mapping > - A reviewer could not see what an agent changed without opening the workspace > - This pull request carries a bounded patch on each edit call and shows it in the edit row. Each turn that changed files ends with a "Changed in this turn" card > - The benefit is that a reviewer can check every change in the conversation, with the same view for Codex and Claude ## Linked Issues or Issue Description Refs #19252 (precedent: canonical `plan.updated` events as a docked plan card). **Subsystem affected** Paperclip runner (Rust and TypeScript canonical event mapping), PRP provider-event schema, and the task-chat UI. **Problem or motivation** Edit rows say "Edited a file" with no content. Claude Edit/Write `diff` content is lost after the ACPX sidecar. The Codex native runner drops `fileChange.changes` and `turn/diff/updated`. The existing workspace-changes card showed a raw text patch in a dialog, and only on the in-process Codex path. **Proposed solution** Add an optional, bounded `fileChanges` list to `paperclip.tool.execution.v1`. Fill it from Codex `fileChange` items and Claude ACP `diff` content. Map Codex `turn/diff/updated` to `workspace.change.updated` on the native runner. In the UI, show `+N −M` on each edit row, open a compact colored diff, and end the turn with the change set. **Alternatives considered** A runner-side git diff at turn end. It needs workspace access and extra cost on every turn. The harness patches are already available. For Claude, the UI builds the turn card from the edit rows instead of new per-turn runner state. ## What Changed - Protocol: optional `fileChanges[]` on `toolExecution` in `provider-event.schema.json`, with the regenerated schema bundle, validators and manifest. Paths are workspace-relative. A patch is at most 32 KiB, and a call has at most 32 files. - Runner (Rust): new `tool_file_changes.rs`. A Codex `fileChange` item now also emits `tool.execution.*` with `fileChanges`; the native item event stays for the recovery and progress projections. `turn/diff/updated` becomes `workspace.change.updated`. `normalize_acpx_tool_call` re-validates the sidecar's `fileChanges`. `normalize_codex_notification_in` resolves absolute paths against the session cwd. - Runner (Rust): `has_bounded_rich_display_shape` accepts schema-valid workspace diffs. Without this change, durable storage cut each patch string to a 4 KiB preview. Patches use the uncapped secret redactor. - Runner (TS): new `tool-file-changes.ts`. The ACPX sidecar computes `fileChanges` from ACP `diff` content with the working directory. The in-process Codex and ACPX mappers do the same, with an optional `context` argument. - UI: new `TaskChatDiffView` (colored rows, hunk labels, a height cap, "Show N more lines"). Edit rows show counts and the diff. Collapsed activity groups show edit totals. The workspace card is now "Changed in this turn", with in-place per-file diffs, and it sits at the end of the turn. When a harness sends no whole-turn diff (Claude), the card is built from the edit rows. A change set is marked complete when its run ends. - UI: `native-run-events.ts` maps `workspace.change.updated` and attaches each edit's latest patch to its row (the `diff` transcript entries now carry `toolUseId`). - Design tokens: `--diff-add-*`, `--diff-remove-*` and `--sz-edit-diff-max-h` come from the status icon hues. - Storybook: `Tasks/Edit diffs` story. Docs: `doc/run-log-events.md`. ## Verification - Real runs on a local instance (`test-drive`, native runner): - Codex (`gpt-5.6-sol`): the "Applied a patch" rows show `+9` and `+35 −2` with their diffs. The turn ends with "Changed in this turn", 2 files, `+42`, from `turn/diff/updated`. - Claude (`claude-opus-5-5` via ACPX): the Write and Edit rows show `+5`, `+14 −1` and `+9` with correct hunks. The turn card is built from the edit rows. - Codex model note: with `gpt-5.4`, pinned Codex 0.160 reports "model metadata not found". The model then wrote files through the shell, so no `fileChange` items occurred. - `pnpm check:token-gates`: all gates clean. - `pnpm --filter @paperclipai/ui exec vitest run src/components/task-chat src/components/transcript src/adapters`: 1073 passed. - `packages/paperclip-runner`: `vitest run src/provider-events.test.ts src/tool-file-changes.test.ts src/cli/acpx-runtime-sidecar.test.ts` passed. `check:protocol-types` and `check:protocol-manifest` match. - `cargo test -p paperclip-runner-core --lib provider_events tool_file_changes`: passed. `cargo fmt --check`: clean. - Storybook: open `Tasks and conversations/…/Tasks/Edit diffs`. ## Risks - Larger payloads: an edit call can carry up to 96 KiB of patch in Rust, and the sidecar keeps 48 KiB under its 128 KiB frame. Above the budget, a file keeps its counts and drops its patch (`diffTruncated`). - Redaction: each patch goes through the shared secret redactor. Before this change, a 4 KiB preview cap also applied. - Claude turn card: it joins each edit's patch in order. It is not a net diff. For a file the turn created, the counts are net and exact. For an existing file, the counts are summed over its edits. - The workspace card moves to the end of the turn. One adapter test changed to match. - Local only: four Linux cgroup `task_memory` Rust tests fail on macOS. They do not touch these modules. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, browser and file editing) and a long 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 - [ ] 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> |
||
|
|
6293e581f6 |
fix(recovery): resume cancelled workspace retries from verified waits (#19276)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A shared workspace can make a run wait before it calls a provider. > - The scheduler saves a retry for that wait. > - The control plane can cancel the retry while it is still queued. > - Recovery then asks for a process stop receipt for a run that never claimed execution. > - This pull request verifies the retry and its exact workspace-wait parent before it accepts a later user instruction. > - The benefit is that Retry and saved messages can continue through normal admission without replaying old work. ## Linked Issues or Issue Description Refs #13640 and #15483. Related fixes: #18838 covers dispatcher cancellation codes; this change covers a system-cancelled scheduled workspace retry. Related open PR #15383 covers claimed conversation-adapter preparation. This change requires an unclaimed retry with no run profile or owner. **What happened?** A scheduled workspace retry was cancelled before runtime selection. The run retained an expected system cancellation and a `startupCancellation.beforeNativeSelection: false` marker. It had no start timestamp, execution stage, run profile, controller, process, lease or provider events. Recovery recorded an unknown-action hold. A later user message and Retry could not pass that hold. **Expected behavior** A later authorized instruction can start a fresh turn when the saved retry and its exact parent prove that no provider work started. Missing or conflicting evidence must retain the hold. **Steps to reproduce** 1. Record a terminal workspace wait before provider dispatch, with `executionRecovery.kind: workspace_wait` and `providerWorkStarted: false`. 2. Save a scheduled retry for that same company, task and agent. 3. Cancel the retry through the control plane before it claims execution. 4. Send a new user message or request Retry after the cancellation. 5. On the base commit, admission returns no continuation. With this change, normal admission creates one fresh successor. **Paperclip version or commit** Reproduced in source based on master at `499712c68b3ca95801b6114d5f2f0f0ea6759318`. **Deployment mode** Source checkout with isolated embedded PostgreSQL. Hosted rollout and a real provider run remain to be verified after merge. ## What Changed - Extend the existing cancelled-startup proof with one source-bound workspace-retry case. Require the canonical retry parent and reason to match the saved context. - Verify the parent's company, task, agent, time order, immutable Runner claim and typed workspace-wait record. Reject runtime, process, lease, coordinator, result and output evidence on either run. - Keep validation of the full saved result in the database predicate. The bounded execution projection can add empty receipt fields without changing eligibility. - Reuse the existing admission, Retry and deferred-message paths. Preserve cancelled history and unknown action outcomes. Keep permission, Stop, budget and recovery-limit gates. - Add 23 regression cases, including heartbeat message and Retry admission, concurrent saved-message recovery, and 19 conflicting-evidence cases. Document the contract. The cancelled-startup service owns this proof. No provider protocol, schema, UI or shared contract changes. Reads use exact run IDs and return only narrow metadata. The new reads occur only for a cancelled, unstarted Runner retry candidate. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/explicit-native-continuation.test.ts src/services/legacy-execution-recovery.test.ts src/services/execution-blocker-read.test.ts src/services/task-retry.test.ts` — 452 passed. - `pnpm -r typecheck` — passed. - `pnpm build` — passed. - `node scripts/check-module-boundaries.mjs` and `git diff --check` — passed. - Local diff review and secret/private-context scan — passed. Test identities are synthetic. - Regression history: the original positive case failed on unchanged source while 11 negative guards passed. Initial test fixtures then exposed ownership/contract constraints and a missing saved-wake envelope; the fixtures now use valid persisted shapes. The heartbeat Retry test exposed extra keys in the bounded evidence projection. Full-record validation now runs in SQL, and both assembled heartbeat paths pass. The unknown action outcome stays unchanged. - Platform: macOS arm64, Node 25.6.1, pnpm 9.15.4. Tests use real isolated PostgreSQL. They stop at admission and do not call a live provider. - Exact-head GitHub CI: 54 successful checks, 3 skipped, no unfinished or failed checks. Greptile scored `cfbf55cea68e3c69f65f50afcba98f775fa6fa3d` 5/5, with no actionable findings and no review threads. - The full repository `pnpm test:run` was not run locally. A previous full run in this repair effort stalled and was stopped after 2h45. The four complete affected suites pass; full PR CI must pass before merge. ## Risks - False admission could allow work before a predecessor is safe. The proof fails closed on missing parent records, different ownership, a changed retry binding, later parent completion, restore obligations, any execution resource, or any provider/output evidence. - This deliberately covers only system-cancelled, unclaimed workspace retries. Other cancellations retain their current reconciliation path. It does not grant automatic replay, certify unknown external writes, or reset exhausted recovery limits. - No migration. Verify the serving version and an existing saved-message continuation after deployment. ## Model Used - OpenAI `gpt-6.1-sol`, xhigh reasoning, through the Codex harness in T3 Code. Used code editing, shell tools and test execution. The harness 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> |
||
|
|
499712c68b |
fix(runner): restore cold state before inspecting completed handoffs (#19254)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner saves provider sessions and controller state across turns. > - A completed warm handoff leaves a receipt in the controller journal. > - A later cold continuation must restore external Runner state before it can inspect that receipt. > - The transport read the missing physical file before it called the restore hook. > - This pull request prepares admitted external state before that inspection for completed handoffs. > - Live Runner adoption and pending transitions retain their existing ownership checks. The provider thread can continue after a cold restore. ## Linked Issues or Issue Description Related: #19225 and #19157. This fix changes the shared transport read order. It does not depend on the open source-reconciliation extension in #19157. **What happened?** A cold continuation after a completed warm handoff could fail with `runner_remote_state_unavailable`. The saved controller journal was valid. The physical Runner state was absent because its restore hook had not run yet. A later automatic reattachment could then fail an allocation identity check. **Expected behavior** Restore admitted external state before inspecting a completed handoff. Retain the original provider thread. Refuse a denied restore or foreign session without starting provider work. **Steps to reproduce** 1. Open a Codex session and complete a warm handoff to a second run. 2. Suspend the session and retain the controller journal and provider backup. 3. Start a third run with no physical Runner state until the external preparation hook restores it. 4. Read the retained thread. Before this fix, the read fails before preparation. ## What Changed - Prepare external Runner state before inspecting a completed warm-handoff receipt. - Keep live Runner adoption, pending attachments, and active warm transitions on the existing exact-state path. - Add a real Runner and controlled Codex regression for successful restore, denied restoration, and a foreign restored session. - Document the cold-continuation order in the execution contract. ## Verification - Red: the new real-process regression fails on unpatched source because resume reads the absent `runner-state.json` before preparation. - Green: all three new cases pass. The restored thread keeps its identity. The controlled provider starts one thread and resumes it once. No turn is replayed. - Adjacent safety suites: 60 cases passed across warm attachment, interrupted attachment recovery, suspension, and cold restore. The new foreign-session case also passes separately. - Runner TypeScript and generated protocol checks passed. - Full workspace typecheck and build passed. Runner TypeScript checks passed again after the live-adoption guard. The full local test command remains in progress; all required CI test jobs passed on the current head. - The first CI run found two live-adoption regressions: an already completed staged handoff must not restore a backup while its Runner is live. The guard now excludes live adoption. All six cases across cold restore and staged handoff pass locally. The original CI failure remains recorded in run 42047027346. - Greptile reviewed the corrected head `f33a041c8902c39ca52b995d53128fc23b0b5499` at 5/5. There are no review threads or findings. All current-head CI gates passed. - Live staging verification remains pending normal deployment. No pinned serving image or bulk migration is part of this PR. ## Risks The change calls an existing external preparation hook earlier for completed handoffs. That hook must remain idempotent and must enforce restore authority. Live adoption, pending attachments, and active transitions do not gain this earlier call. The negative regressions assert that a refused restore and a foreign session start no provider work and do not change the authority journal. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The host does not expose the exact serving model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
e37ab23dbc |
fix: hold an agent's new work while its provider usage limit is active (#19246)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Routine-driven agents run on the native Runner through ACP harnesses such as Claude > - When the agent's provider account runs out of allowance, every new wake for that agent fails the same way > - The ACP harness reported the exhausted allowance as a generic "terminal limit failure" with an `unknown` category, so Paperclip could not tell it from a context or turn limit > - Paperclip recorded a reset time only for the failing run's own startup retry; other wakes of the same agent kept dispatching, each spent about two minutes on workspace setup, and then failed > - This pull request classifies the limit, keeps its reset time, and holds the agent's new work until that reset > - The benefit is that a limited agent stops burning runs, shows a clear "waiting for usage limit reset" state with a time, and resumes its queued work after the reset ## Linked Issues or Issue Description Refs #13148 Refs #13379 **What happened?** A routine-driven Claude agent on the native Runner failed 29 times in about 15 hours. 20 runs failed with `native_provider_terminal_failed` and "ACP agent reported a terminal limit failure." The recorded provider failure had category `unknown`. 6 runs failed with `provider_session_open_rejected`. Their `session.open` result is `command_execution_failed` with classification `provider_rate_limit`, and the provider failure is HTTP 429 `rate_limit` ("would exceed your account's rate limit"). The startup retry of one such run recorded a cooldown until one hour later. A new wake for a different task of the same agent dispatched 16 minutes later, spent two minutes on workspace setup, and failed with the same 429. Separately, 4 runs failed with "Project authentication conflicts with this agent's managed AI connection". That message did not say which file or setting conflicted. The same message was also shown when the check could not finish. **What did you expect?** A provider usage limit is classified distinctly. Paperclip records the reset time when the provider reports one. New work for that agent waits for the reset with a visible reason. Queued work resumes after the reset, and no doomed run starts. A configuration conflict is reported only when one is found, and the message names the file to fix. **Steps to reproduce** 1. Use a native Runner agent with a Claude ACP harness whose account is rate limited or out of usage. 2. Let a routine or assignment wake the agent on several tasks. 3. See each run fail with a generic terminal limit failure or a `session.open` rejection, and see new runs keep starting. ## What Changed - Runner: The ACPX turn now receives the typed AIR session failure (`onTerminalSessionFailure`) through the runtime host and the ACPX port. The sidecar adds the provider's title to the provider failure message. Only a rate-limit or quota title becomes `rate_limit` or `quota_exhausted`. Context, turn and configured budget limits stay `unknown`. The failure details stay private. - Server: A native `turn.failed` with a `rate_limit` or `quota_exhausted` provider failure is now `native_provider_usage_limit`, the same code as the Codex usage-limit terminal. - Server: `agentProviderLimitCooldown` (extracted from the startup retry) also reads a reset clock in the provider's wording, such as "resets 2am (UTC)". The clock parser moved from the recovery service to `provider-quota-reset.ts` without changes. - Server: New `heartbeat/provider-limit-wait.ts`. Before any workspace or provider work, a queued run for an agent with an active provider limit is cancelled as `provider_limit_wait`. The run records the reset time, whether the provider reported it, the category and the run that reported the limit. Paperclip schedules one retry of the same wake at the reset through the existing AI connection wait lane. That lane does not consume the failure retry budget. Manual runs, recovering runs, conversation turns and pool-routed connections are not held. - Server: The execution projection shows "Waiting for usage limit reset" with the reset time and the next step. - Server: Only a found project authentication conflict is a configuration failure. Its message names the project file and the settings to remove. A check that cannot finish (timeout or another exit code) is now an ordinary failure, not a configuration conflict. - Docs: `doc/execution-semantics.md` describes the agent-level limit wait. ## Verification Red before the fix, green after: - `server/src/__tests__/heartbeat-provider-limit-wait.test.ts`: An automation wake after a 429 `session.open` rejection dispatched the adapter before the fix. Now the run is cancelled as `provider_limit_wait` and a scheduled retry exists at the reset. After the reset passes, the retry is promoted and the work succeeds. A manual run, a later success, expired evidence and an unclassified terminal still dispatch. - `native-startup-retry.integration.test.ts`: "reads the reset clock from an ACP terminal limit failure's provider wording" (it fell back to one hour before). - `native-session-executor.test.ts`: An ACP `turn.failed` with a typed `quota_exhausted` or `rate_limit` failure is a usage-limit terminal; `unknown` is not. - `packages/paperclip-runner/src/drivers/provider-failure.test.ts`: A typed AIR limit title is kept and classified; context, turn and budget limits are not; details do not leak. - `execution-projection.test.ts`, `ai-connections.test.ts`: The wait label, and conflict versus unfinished check. - Safety cases: `runtime-host.test.ts` and `codex-runtime-adapter.test.ts` forward the callback. The existing startup retry, provider capacity, recovery classification, legacy reconciliation, retry scheduling and execution configuration suites pass. Commands: - `cd server && npx vitest run src/__tests__/heartbeat-provider-limit-wait.test.ts src/__tests__/ai-connections.test.ts src/services/native-runtime/ src/services/recovery/ src/services/execution-projection.test.ts src/services/legacy-execution-recovery.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/__tests__/heartbeat-execution-configuration.test.ts` - `cd packages/paperclip-runner && npx vitest run src/drivers/provider-failure.test.ts src/drivers/acpx/runtime-host.test.ts src/drivers/acpx/codex-runtime-adapter.test.ts src/cli/acpx-runtime-sidecar.test.ts` - `tsc --noEmit` for server and paperclip-runner; the ACPX sidecar and provider-failure contract checks pass. I did not run a live provider limit on staging. The live evidence is the staging run records for the affected agent, which I read only. ## Risks - The limit evidence is per agent, not per account. If the board changes the agent's AI connection, automatic wakes can still wait until the old reset (at most one hour when no reset was reported, and at most one day). A manual run is not held, and one successful run clears the evidence. - A new wake that arrives during a limit is now cancelled and retried at the reset, instead of failing. The task stays in its status, and the run shows the reset time. - The Runner change takes effect when the provider pack is rebuilt and deployed. Until then, the server still holds new work using the existing `rate_limit` evidence from `session.open` rejections. - A turn terminal with a typed rate limit or quota is now `native_provider_usage_limit` instead of `native_provider_terminal_failed`. Both are terminal and board-owned. The recovery text now asks the board to restore provider usage capacity. > 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, used through Claude Code with tool use (shell, file edits, 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 - [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> |
||
|
|
d0afeacdd5 |
fix(remote-workspaces): keep retained runner state when replacing a retired root (#19251)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote workspaces save a task's files as snapshots. Runtime state under `.paperclip-runtime` is never in a snapshot, because it belongs to the sandbox > - #19231 lets a parent task on a retained sandbox restore newer child work over its own retired workspace root > - That replace removed the whole retired root, including the parent's retained runner session state under `.paperclip-runtime/paperclip-runner/` > - The parent's native continuation then reads that runner state on the resumed sandbox, cannot find it, and fails with `runner_remote_state_unavailable` > - This pull request moves the retired root's runtime state into the restored root before it removes the retired root > - The benefit is that delegating parents on retained sandboxes continue their session with the children's saved work ## Linked Issues or Issue Description Refs #19231 **What happened?** After #19231, a parent task on an agent with a retained sandbox delegates two subtasks. Both subtasks finish. The parent's wake run now restores the newer saved work successfully, but the run fails right after with `runner_remote_state_unavailable: exit=1 timedOut=false`. The run error code is `native_continuation_requires_reconciliation` and the task becomes `blocked`. In one staging wave, 4 of 4 such parents failed. Root cause: the native continuation on a resumed sandbox reads `<remoteCwd>/.paperclip-runtime/paperclip-runner/sessions/<session>/runner/runner-state.json`. Snapshots exclude `.paperclip-runtime`, so the restored root has only the new restore receipt. #19231 parked the retired root and then removed it, so the runner state was removed too. **Expected behavior** The restored root has the newest saved files and the sandbox's retained runtime state. The parent's continuation reads its runner state and continues. **Steps to reproduce** 1. Prepare a remote workspace on sandbox A. Write files and runner state under `.paperclip-runtime/paperclip-runner/`. Save. 2. Make a second run the writer on sandbox B. It restores, writes new files, and saves. 3. Make a third run the writer on sandbox A again and prepare. The restore replaces the retired root. 4. Read the runner state with `readRemoteRunnerState`. Before this change, it fails with `runner_remote_state_unavailable: exit=1 timedOut=false`. **Paperclip version or commit** `master` at `26391707fb`, which includes #19231. ## What Changed - `server/src/services/remote-workspace-transport.ts`: before it removes the retired root, the restore script moves each entry of the retired `.paperclip-runtime` into the restored root's `.paperclip-runtime`. It does not move `workspace-ready.json`, `workspace-restored.json`, or `workspace-snapshots`, and it does not replace an entry that already exists. The move is a rename, so it keeps sockets and large session state. The script also finishes an interrupted carry when the same restore repeats. Only an authorized replace with that restore identity creates the `<root>.retired-<identity>` path. - `server/src/services/remote-workspaces.integration.test.ts`: the retained-sandbox regression now writes parent runner state, restores newer child work, and reads the state with the same `readRemoteRunnerState` call that the continuation uses. It also covers an interrupted carry on a repeated restore. - `doc/execution-semantics.md`: document that runtime state moves with the restored root. ## Verification - Red before the fix: `npx vitest run src/services/remote-workspaces.integration.test.ts -t "retired root"` failed in the `own retired root`, `own restored root`, and `read-only folder` cases with `Error: runner_remote_state_unavailable: exit=1 timedOut=false`. This is the same error as the staging failure. - Green after the fix: all nine cases pass. The continuation read returns the parent's runner state. The old `workspace-ready.json` is gone and the new restore receipt is present. A repeated restore finishes an interrupted carry and removes the retired root and its stale files. - `npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts`: 250 passed. - Staging evidence (read-only): the failed parent wake run recorded a succeeded `remote_workspace_restore` at the parent's own provider lease, and then failed in the runner-state read. - I did not run a live staging run. This PR does not deploy. ## Risks - Runtime state from the retired root now persists into the restored root. The state is the sandbox's own state for this workspace path, which is the same state that a same-allocation reuse keeps. - If the restored root already has an entry with the same name, the old entry is not moved and is removed with the retired root. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. It used tool use for code search, shell, test runs, and read-only staging inspection in the browser. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
21640b6d51 |
feat(connections): gate the built-in Google Workspace connector behind an experimental toggle (#19247)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Apps catalog lets operators connect external tools, including Google services. > - Paperclip has a built-in Google Workspace connector, but Google has not yet verified Paperclip's Google app. > - The built-in connector also hides the Composio and Arcade Google apps, because native Google definitions claim those app names. > - Users therefore see an unverified connector and cannot find the working aggregator Google apps. > - This pull request puts the built-in connector behind a default-off experimental setting. When the setting is off, the aggregator Google apps appear. > - The benefit is that users get working Google connectors now, and the team can still test the built-in connector on demand. ## Linked Issues or Issue Description **Problem** Google has not verified Paperclip's Google app yet. The Apps page still offers the built-in Google Workspace connector. The built-in Google definitions also suppress the Composio and Arcade Google apps (Gmail, Google Calendar, Google Drive, Docs, Sheets, Slides, Chat), so users cannot find them. **Proposed solution** Add the `enableGoogleWorkspaceConnector` experimental setting. The default is off. When it is off, hide and block the built-in connector, and show the aggregator Google apps. When it is on, keep the current behavior. **Alternatives considered** Delete the built-in connector until verification is complete. We rejected this option because the team must continue to test the connector. **Additional context** Composio documents Gmail (63 tools), Google Calendar (50 tools), and Google Drive toolkits that use Composio-managed OAuth. These toolkits do not need Paperclip's Google app verification. ## What Changed - Added the default-off `enableGoogleWorkspaceConnector` experimental setting to the shared types, validator, feature catalog, server defaults, and the managed-key list for Paperclip Cloud staging. - Added `experimentalFeatureKey: "enableGoogleWorkspaceConnector"` to the `google-workspace` app definition. The existing generic connector gate then hides the connector in the gallery and blocks its setup, discovery, execution, and agent suggestions when the setting is off. - Added a "Google Workspace connector (Paperclip)" toggle card to **Settings → Experimental**. - Apps page (`Browse.tsx`): when the setting is off, built-in Google rows without saved accounts no longer claim aggregator app identities. The Composio and Arcade Google apps then show as aggregator rows. The legacy Google slug filter now applies only to native rows, so the Composio `gmail` row is not dropped. - Saved per-service Google connections (Gmail, Drive, and others) are not gated. They stay visible and usable. - Updated `doc/connections/GOOGLE-WORKSPACE.md`. - Tests: added tests for the hidden and visible Apps page states, the settings toggle, and the definition gate. Existing built-in Google Workspace tests now enable the setting. ## Verification - `cd packages/shared && npx vitest run` passes (1055 tests). - `cd server && npx vitest run src/__tests__/connection-management.integration.test.ts src/__tests__/tool-access-service.test.ts src/__tests__/tool-gateway-service.test.ts src/services/gmail-tool-governance.test.ts src/services/google-chat-tool-policy.test.ts src/__tests__/instance-settings-service.test.ts src/__tests__/health.test.ts src/__tests__/instance-settings-managed-overlay.test.ts` passes (864 tests). - `cd ui && npx vitest run src/pages/apps src/pages/InstanceExperimentalSettings.test.tsx src/features/connections` passes. - `tsc --noEmit` passes for `packages/shared`, `server`, and `ui`. The `ui` run shows errors only in the untouched `storybook/support/feature-map` files. - `pnpm check:token-gates` passes. - Manual check: open **Apps** with the setting off. Search for "Gmail". The Composio Gmail row appears, and the built-in Google Workspace card does not. Turn on the setting in **Settings → Experimental**. The built-in card appears again. ## Risks - Existing built-in `google-workspace` connections stop running while the setting is off, because the generic gate blocks execution. This is intentional, so that the unverified Google app is not used. Turn on the setting on instances that must keep these connections. - Saved per-service Google connections are not affected. - When the setting is on, the behavior does not change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, running in Claude Code with tool use (shell, file edits, web fetch). ## 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> |
||
|
|
03abe1f1a1 |
fix(native): keep the transport cause on a closed notification stream failure (#19240)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs read provider events from a shared session notification stream (Codex, OpenCode and ACPX drivers use it) > - When that stream closes, the driver fails the session with `notification_transport_failed` > - The run error was only "Provider notification transport failed closed." The real cause was only in a diagnostic event > - In a staging soak, many runs on OpenRouter and Grok agents showed this error. Operators read it as a provider fault. The diagnostic events showed a host-side cause: a stopped sandbox plugin worker ended remote runner supervision > - This pull request keeps the redacted, bounded cause on the terminal failure message, and does not name the provider > - The benefit is that a failed run shows why it failed, so operators do not chase the wrong provider ## Linked Issues or Issue Description **What happened?** Native runs failed with `native_provider_terminal_failed` and the error "Provider notification transport failed closed." The run did not show a cause. All failures in one cluster happened inside one minute, on several agents and models (OpenRouter DeepSeek, Grok). Each run's `harness.diagnostic` event had the real cause, for example `runner_process_termination_unconfirmed: runnerd process supervision failed: Cannot call "environmentExecute" — worker for "<plugin>" is stopped`, or `native_adopted_runner_identity_unverifiable: runner liveness could not be revalidated`. The sandbox duplex transport reported a `provider_exit` loss about 30 seconds before (the read-only process probe budget). **Expected behavior** The terminal run error keeps the redacted cause. It does not blame the model provider when the cause is host-side runner supervision. **Steps to reproduce** 1. Open a native Codex-protocol session and start a turn. 2. Make the notification stream throw (for example, a supervision error). 3. Read the `session.failed` and `turn.failed` messages. Before this change they are the bare generic text. ## What Changed - `codex-session-notifications.ts`: the `notification_transport_failed` terminal message is now `Notification transport failed closed: <cause>`. The cause is redacted first, then whitespace is collapsed and it is bounded to 1,000 characters. The `Error: ` prefix is removed. The code and `recoverable: false` do not change. - New regression in `codex-protocol-integrity.test.ts`: one `session.failed`, the cause and code are present, the secret is redacted, the length is bounded, and `turn.failed` has the same message. - `doc/execution-semantics.md`: an additive note that this failure can come from host-side runner supervision, and that the message keeps the redacted cause. ## Verification - Red: before the fix, the new test failed: `expected 'Provider notification transport faile…' to contain 'runner_process_termination_unconfirmed'`. - Green: `cd packages/paperclip-runner && npx vitest run src/drivers/codex/ src/conformance/local-provider-fault-matrix.test.ts src/native-session-runtime.test.ts` (565 passed). - `npx tsc --noEmit -p packages/paperclip-runner/tsconfig.json`: no new errors. The existing `src/eval/workflow-harness.ts` errors come from an unbuilt workspace package. - Staging evidence was read with read-only GETs only (run, events). ## Risks - Low risk. Only the message text changes. The failure code, `recoverable: false`, and the safe-replacement rules do not change. The server matches on the code, not on the message. - This PR does not add a retry in the same turn. Runner termination is unconfirmed when supervision is lost. A retry or replacement could start a second executor. The remote process probe already retries read-only checks inside a 30-second budget. Recovery stays with the existing ownership and recovery checks. - The cause text can include a plugin id. It goes through the existing provider-diagnostic redaction. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used in Claude Code with tool use (shell, file edits, browser reads) 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 - [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> |
||
|
|
b3da1d886a |
fix(ui): retain Coordinator label filters across reloads (#19233)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Operators use task filters to keep a useful view of their work.
> - Coordinator saves status and assignee filters in the task-list URL.
> - A selected label stayed only in React state, so reload removed it
and showed unrelated rows.
> - This change saves and restores selected labels through the same
page-owned URL path.
> - Operators can reload and share that filtered collection without
selecting its labels again.
## Linked Issues or Issue Description
**What happened?**
In Coordinator All tasks, select a status, assignee, and label. Sort by
Task ID and reload. Status, assignee, and sort remain selected, but the
label is cleared and rows without that label appear.
**Expected behavior**
Reload and history navigation retain the selected labels with the other
URL filters. Clear filters removes the selected labels.
**Steps to reproduce**
1. Open Coordinator All tasks with labeled and unlabeled tasks for the
same assignee and status.
2. Select that status, assignee, and a label.
3. Change Task ID sorting and reload the page.
4. Check both the selected label and the displayed tasks.
**Paperclip version or commit**
Source regression reproduced at master
|
||
|
|
e8163fc860 |
fix(remote-workspaces): restore newer saved work over a retained sandbox's own retired root (#19231)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote workspaces save a task's files as snapshots and restore them on the sandbox that runs the next turn > - A parent task can delegate subtasks. The subtasks inherit the parent's workspace, run on their own sandboxes, and save newer snapshots > - When the parent wakes on its retained (warm, reused) sandbox, its earlier workspace root is still on that sandbox at the same published path > - The restore refused that root with `reason=occupied_root`, so the parent run failed at setup and the task became blocked with a board recovery action > - This pull request lets the restore replace that root only when the server proves it is the workspace's own earlier, fully saved allocation on this sandbox > - The benefit is that a delegating parent on a retained sandbox continues with the children's saved work, and unknown occupants are still refused ## Linked Issues or Issue Description **What happened?** A parent task on an agent with a retained sandbox delegates two subtasks to other agents. Both subtasks finish. The parent's wake run fails before the provider starts with `remote_workspace_restore_failed: step=sandbox_restore; ... stage=inspect_root reason=occupied_root`. The task becomes `blocked` with a stranded-work recovery action. Parents on agents with ephemeral sandboxes complete in the same wave. Root cause: the subtasks inherit the parent's execution workspace. Each subtask becomes the writer at a new epoch on its own sandbox, restores at the same published directory name, and saves a newer snapshot. The workspace location then points at the last subtask's sandbox. The parent's wake run gets a lease on its retained sandbox, so preparation is not "same allocation" and must restore the newer snapshot. The target path on the retained sandbox still holds the parent's epoch-1 root, which has a preparation receipt (`workspace-ready.json`) and no restore receipt. The restore script refuses any occupied root that is not the same restore, so it fails with `occupied_root`. A warm reuse cannot just keep the old root, because the snapshot is newer. **Expected behavior** The parent restores the newest saved work (its own files plus the children's results) into its published root and continues. A foreign or unknown occupant is still refused. **Steps to reproduce** 1. Prepare a remote workspace on sandbox A, write files, and save (final boundary). 2. Make a second run the writer on sandbox B. It restores, writes new files, and saves. 3. Make a third run the writer on sandbox A again (same provider lease id, new lease row) and prepare. 4. Before this change, step 3 fails with `stage=inspect_root reason=occupied_root`. **Paperclip version or commit** `master` at `47027e7ba5`. ## What Changed - `server/src/services/remote-workspaces.ts`: add `isRetiredOwnAllocation`. It reads this workspace's snapshot and restore operations recorded at the same environment, provider lease id, and root path. It returns true only when every such use is from an earlier epoch than the current writer, the last use ended in a succeeded `final` save with quiesced save work (`unknown=false`, no active invocations), no other save in that epoch is newer or unsettled, and the saved snapshot is committed or later. Preparation passes the result to the restore as `replaceRetiredRoot`, and only when there is no migration and the authority is clean. - `server/src/services/remote-workspace-transport.ts`: the restore script accepts `replaceRetiredRoot`. In that mode it replaces an occupied root only when the root carries a well-formed Paperclip receipt (`workspace-ready.json` or `workspace-restored.json` with a 64-hex identity). It parks the old root at `<root>.retired-<identity>`, publishes the restored folder, rolls the old root back if the publish rename fails, and then removes the parked copy. A repeated restore with the same identity adopts the published folder and removes any parked copy left by an interrupted attempt. Without a receipt, the script still fails with `reason=occupied_root`. `replaceRetiredRoot` and `allowEmptyRoot` cannot be combined. - `doc/execution-semantics.md`: document the rule. - `server/src/services/remote-workspaces.integration.test.ts`: add a red/green regression and the safety cases. ## Verification - Red before the fix: `npx vitest run src/services/remote-workspaces.integration.test.ts -t "retired root"` failed in the `own retired root` case with `remote_workspace_restore_failed: step=sandbox_restore; remote_workspace_operation_failed: operation=restore exit=1; remote_workspace_restore_failed: stage=inspect_root reason=occupied_root`. This is the same cause text as the staging failure. - Green after the fix: all six cases pass: - `own retired root`: the parent restores on its retained sandbox. The root has the parent's file and the child's file, the parked and staging folders are removed, and the authority records the parent's lease. A repeated preparation (lost reply) keeps the parent's new unsaved edits. - `own restored root`: an occupant with a restore receipt from a different preparation is also replaced. - `receipt missing` and `forged receipt`: the occupant is refused with `occupied_root`, and its files stay. - `later allocation`: a later use of that root on that sandbox without a final save is refused. - `never saved here`: a different sandbox with an unknown folder at that path is refused. - `npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts`: 247 passed. This includes the existing `occupied root` cold restore case, which is still refused. - `tsc --noEmit` shows no errors in the changed files. The local tree has unrelated errors from unbuilt workspace packages. - I did not run a live staging run. Staging is read-only for this work, and this PR does not deploy. ## Risks - The replaced root is removed after the publish. The server proof states that its content is in the saved lineage. If a process wrote to that root after its final save and outside a Paperclip run, those writes are lost. The same assumption applies when authority moves to another allocation today. - The proof uses operation records that have `exportCleanup` location metadata. Older records without that metadata do not qualify, and those cases still fail closed with `occupied_root`. - A warm runner process that kept an open working directory in the old root sees a removed folder. This is not verified live. The new turn is given the published path, which holds the restored files. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. It used tool use for code search, shell, test runs, and read-only staging inspection in the browser. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
5d8d9bf83e |
fix: share native lifecycle policy with instruction checkpoints (#19230)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Warm native Codex turns save registered agent files at each turn boundary. > - Native execution can inherit warm mode from the agent when the environment has no explicit policy. > - File preparation checked only the environment policy and could miss that inheritance. > - The new copy then had no warm checkpoint callback, even though the native session stayed warm. > - This pull request resolves one admitted policy before both owners use it. > - The benefit is a terminal file checkpoint under the selected lifecycle, with the existing ownership and permission fences. ## Linked Issues or Issue Description Refs #14735 and #19225. The first defines warm file checkpoints. The second retains coordinator ownership while cleanup joins. This change fixes the earlier lifecycle selection mismatch. **What happened?** With no explicit environment policy and an agent warm default, native selection chose warm mode with a 900,000 ms idle window. Actual file preparation registered an agent_files copy without its warm flag or checkpoint callback. An unchanged turn could have no save marker. Later retirement of a retained lease could reach the stopped-file retrieval guard and preserve pending collection debt. **Expected behavior** File preparation and native execution use the same admitted lifecycle policy. Warm native Codex copies receive their terminal checkpoint callback. Explicit per-turn overrides and pinned admissions keep their existing precedence. A pending historical copy does not become saved because policy resolution changed. **Steps to reproduce** 1. Configure a native Codex agent with a warm default and a reusable sandbox without an explicit runner lifecycle policy. 2. Prepare its managed agent-file copy through the normal instruction owner. 3. Select native execution through the normal runtime owner. 4. Compare the selected warm policy with the copy's missing warm flag and checkpoint callback. **Paperclip version or commit** Base: `3a090b815b`. The owning policy code also exists on its parent revision. **Deployment mode** Native Codex with registered agent files and a reusable sandbox that inherits its lifecycle policy. ## What Changed - Move the existing lifecycle precedence and backend gates into a narrow shared resolver. - Resolve one value before instruction preparation and pass it into runtime selection. - Select warm file ownership from that value and the existing file capability gate. - Add real-Postgres unchanged and changed terminal checkpoint cases, policy precedence cases and updated preparation coverage. - Exercise the same resolve, timed preparation and runtime-selection orchestration used by heartbeat; preserve preparation error identity and skip selection on preparation failure. - Document new-admission overrides and pinned-policy behavior. ## Verification - Red: actual preparation and runtime-selection producers disagree. The selected native mode is warm, but the real persisted copy has no warm flag. The fixture uses real PostgreSQL, managed-root authorization and a synthetic loopback transport. - Green: 85 owner tests pass. They include actual unchanged and changed terminal checkpoints plus 19 lifecycle hierarchy/capability cases. Changed synthetic bytes reach canonical storage with a saved receipt; unchanged bytes record unchanged. - Safety: 25 existing warm retirement and stopped/destroyed/foreign-owner cases pass. Six authorization, stale-adoption and adopted-owner preservation cases pass. The lease guard and native executor are unchanged. - Workspace typecheck, build and map checks pass. The final candidate's code and test blobs equal the tested source; its exact build stamp is recorded locally. - No new repository-wide local run was repeated. A separate earlier broad command on an older head failed a reproducible GitHub enrollment fixture; it is not a full local pass, established flake or proven caused by this change. Fresh full hosted CI is green on the reviewed final head; both current-head Greptile and native substantive reviews are 5/5 with no actionable findings. - Review regression: removing actual policy propagation to preparation in the shared production orchestration fails both real-Postgres cases while runtime selection still chooses warm mode. Restoring it passes. These cases invoke the production helper used by heartbeat, not the entire outer heartbeat service. - The provider-facing transport is a local loopback fixture. No model turn, actual remote platform, physical stop receipt or deployed recovery is proved by these tests. ## Risks - New Codex copies that inherit agent warm mode now use the existing terminal checkpoint path. This can add the expected file checkpoint where only an unchanged probe ran before. - The resolver preserves explicit environment per-turn overrides, pinned admissions and backend/credential gates. It does not create a timer, provider retry or new permission. - Existing pending copies remain pending. This change does not authorize retrieval from stopped or replaced leases and does not establish recovery of historical files. - No database migration, public API field, agent setting, provider change or manual rollout is included. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, source inspection and local code execution. Context window and reasoning effort are host-managed. ## 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> |
||
|
|
47027e7ba5 |
fix(routines): retain authenticated agent responsibility on creation (#19222)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Routines save work for later execution under a responsible person's authority. > - An agent can create a routine through its normal REST or native API tools. > - The create route kept the agent creator but dropped the authenticated responsible user. > - The service then selected a parent task's user or a company default, even when the source run had a different user. > - This pull request passes the authenticated responsible user separately from creator attribution. > - Future automatic runs then use the authenticated responsible user saved in the routine revision. ## Linked Issues or Issue Description **What happened?** An authenticated native agent created a routine without a parent task. The stored responsible user became a different company owner. An unrelated parent task could also replace the authenticated user. The defect changes the identity saved for future automatic work. **Expected behavior** Creation must use the server-resolved responsible user from the authenticated run or API key. The agent remains the creator. A person who clicks Run now still supplies that manual run's identity. **Steps to reproduce** 1. Create an active company member and a different company owner. 2. Initialize a native agent run with the member's identity. 3. Use `call_api` to create a paused, self-assigned routine without a parent. 4. Read the routine and its first revision. Before this fix, both use the company owner. **Paperclip version or commit** Reproduced on master `56517dda6c5b32bc901b7dbf1605eaaa041c5fba`. **Deployment mode** Local source integration with real HTTP routes, agent authentication, native tool authority, and embedded PostgreSQL. No provider turn is needed for the reproduction. Related work: [#19128](https://github.com/paperclipai/paperclip/pull/19128) adds board controls for saved responsibility. [#15207](https://github.com/paperclipai/paperclip/pull/15207) repairs synthetic identities from built-in installers. Neither fixes this authenticated create path. ## What Changed - Pass the authenticated agent's responsible user as a separate internal actor field. - Select the board user first, then the agent’s server-resolved responsible user, before the existing parent and company fallback. - Keep creator attribution, initial revision attribution, manual dispatch identity, and existing records intact. - Add real native API, HTTP, and PostgreSQL regressions for identity selection, replay, client spoofing, stale claims, revoked membership, cancellation, cross-company calls, API keys, and fallback behavior. - Document the creation and manual dispatch identity rules in the execution contract. Correct the obsolete responsibility sentence in the agent access map. - Make the existing live-retry route fixture wait for its committed comment’s durable wake before draining. Keep all process and saved-owner assertions. ## Verification - Captured a failing native `call_api` test before changing production code. The persisted responsible user was the unrelated company owner. - The same real route and database boundary passes after the fix. - Initial affected routine and identity checks: 131 passed. The existing pending-AI-card fence also passed. - Follow-up routine checks: 118 passed. The owning scheduled-retry suite passed all 31 tests. - Initial CI serialized shard 7/9 failed with a `23503` wake-to-agent foreign key error during test cleanup. A controlled real-route/PostgreSQL pause reproduced that exact error on unchanged base `56517dda6c`. The drain returned before the route registered its deferred comment wake. The same pause passed after the fixture waited for the actual commentId’s durable wake receipt. The original failure is preserved. - Affected server typecheck after the fixture query change: passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - `pnpm test:run`: still running as supporting coverage. The worktree changed during the run: map cleanup, then the six-line fixture wait. Production and routine-test files stayed unchanged. This is mixed-working-tree coverage, not an immutable full-suite pass for a commit. Exact next-head CI remains required. - `node scripts/check-maps.mjs` and `git diff --check`: passed. - Exact head `a12e3f6c42b8ad5826fd58faa3cd9553ea251e09`: all CI gates passed. Greptile and the native substantive review both gave 5/5 with no open findings. The native reviewer did not run tests or exercise shadow-mode authorization. - Live provider execution remains unverified. This change does not grant access to another person's AI account. ## Risks New agent-created routines now use their authenticated responsible user before a parent or company default. This is the intended behavior change. The service keeps its fallback when no bound identity exists. Missing agent-key identity still fails authentication. Missing personal AI configuration still blocks execution. Existing routines, revisions, tasks, grants, and manual dispatch behavior do not change. The fixture waits for a persisted wake record before draining; its original live-process and saved-owner assertions remain. ## Model Used OpenAI `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 - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
3a090b815b |
fix: retain native checkpoint ownership through warm close (#19225)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native warm session must save its checkpoint under its exact coordinator owner. > - A failed turn can return an accepted result before that checkpoint is saved. > - The executor stopped renewal and released the owner before the managed session closed. > - The backup owner guard then refused the uncommitted, released owner. A failed close could remain in the warm session map. > - This pull request keeps the claim and its existing renewal through checkpoint, close and instruction collection. > - The benefit is a valid checkpoint owner and one fenced release after the work joins. ## Linked Issues or Issue Description Refs #19208, #19206 and #14695. Those fixes cover a destroyed backup source, Stop before a provider session opens, and retained agent-file ownership. They do not change this checkpoint ownership order. **What happened?** A failed warm turn accepted its result. Its coordinator reached `workspace_finalizing`, with a result ID and no lease owner. Managed close then called the durable checkpoint writer. The writer correctly refused this state with `native_harness_storage_owner_missing`. The failed close could remain for the next fresh session to retire. **Expected behavior** The current claim and renewal remain valid until the required checkpoint, close and instruction collection join. Cleanup stops and joins renewal, then releases only that exact claim. A changed owner is not cleared. An earlier error keeps its identity. **Steps to reproduce** 1. Run the new real-Postgres regression against the base commit. 2. Complete a structured failed warm turn through the normal control-plane result producer. 3. Let the actual managed-session close call its production checkpoint callback. 4. Observe the missing-owner refusal after coordinator release. **Paperclip version or commit** Base: `56517dda6c`. The native ownership order also exists on the earlier failing revision. **Deployment mode** Native warm execution with durable harness storage and a managed remote workspace. ## What Changed - Join warm checkpoint, managed close and instruction collection before coordinator release. - Keep the existing lease renewal active through that work. Stop and join it before release. - Run the same fenced release after a checkpoint or collection error. Preserve the initiating error unless restart detachment has won. Keep the typed restart outcome and changed-owner fences. Attach only closed cleanup-failure flags. - Add real-Postgres ownership, delayed-renewal, error, retry and denial cases. Route the managed-session regression through the existing artifact-equipped native test lane. Update the continuity test for the retained claim. Document the active renewal lifetime. ## Verification - Red: the producing coordinator had an accepted result, `phase=workspace_finalizing` and `leaseOwner=null` at managed close. The production checkpoint callback refused it with `native_harness_storage_owner_missing`. - Green: all 9 real-Postgres regression cases pass. They cover a real 20-second renewal tick, a failed checkpoint, one force-fresh successor, and wrong-boot and changed-owner denial. Actual restart detachment wins even when checkpoint and joined renewal-stop both reject. The replacement claim remains unchanged. No new provider turn or cancellation receipt is created. A private canary does not enter the closed diagnostic flags or an error cause. - The executor, restart-recovery and heartbeat compatibility suites pass: 1,259 tests, with 20 gated skips. The container-enabled continuity case was not run locally. Its 21-second timer check remains. It now requires the exact claim during `hasChanges()` and checks release after the joined work. - All 39 shard-routing and combined-coverage tests pass. The new regression runs in the existing native lane with the required Runner artifact. - Final local head `0edcc37f35` passes `pnpm -r typecheck`, `pnpm build`, `node scripts/check-maps.mjs` and the 9-case regression. The earlier broader compatibility run used identical source bytes. A separate earlier repo-wide local suite on `6cdd68e915` ended with exit 1. Its general-server stage had one GitHub enrollment acknowledgement failure, 31,469 passing tests and 215 skipped tests. That acknowledgement failure reproduced in one narrow run on the same old tree. It is not an established flake or proven caused by this fix. Later repository stages did not run. No duplicate broad suite or full local pass is claimed. - The first published head failed review for detach-error precedence and the old continuity assertion. Its server shard also failed because the new test lacked a Runner daemon in the general lane. Those failures remain recorded. This candidate fixes their causes. Both exact-head reviews are now 5/5. The hosted native-server lane ran all 9 new ownership tests successfully. One unchanged tool-gateway rate-rule test failed in the first current-head CI run. It passed previously and passed locally, both narrowly and in its full 183-test file. The single same-head failed-job rerun passed. All current-head hosted gates are clean. The initial failure remains recorded; no test assertion was weakened. - The provider transport is an in-memory fixture. The managed wrapper, production checkpoint callback, control-plane result and Postgres ownership are real. No paid provider or live remote process is exercised. This does not prove a deployed recovery or a remote stop receipt. ## Risks - A slow required checkpoint extends the lifetime of the existing active renewal. Its cadence stays at 20 seconds for remote claims and 5 minutes for local claims. It runs only while the admitted checkpoint, close and collection remain active, then stops and joins on completion or unwinding. The tests observe a real remote renewal tick. No idle timer, new retry budget or new worker is added. - The release still checks company, task, run, owner, attempt, boot, PID, process start and expiry. Restart detachment retains its recovery owner. - The storage owner guard is unchanged. No migration, permission, provider setting or deployment change is included. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, source inspection, local code execution and read-only staging metadata. Context window and reasoning effort are host-managed. ## 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> |
||
|
|
b8e8ab4641 |
fix(recovery): continue saved input after a recovered sandbox stops, not after its destroy (#19224)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can stop or interrupt a native run and send a message for the next turn > - When the stop leaves sandbox files unsaved, automatic recovery saves them, re-stops the sandbox, and the saved message continues > - For agents whose sandbox is destroyed after each turn (an ephemeral lease), recovery then queues the destroy. That step removed the lease's stop receipt > - The continuation gate waits for a stop receipt, so the message waited for the destroy and its cleanup retry, about 5 to 8 minutes > - Agents with retained sandboxes continued within seconds, so the delay looked agent specific > - This pull request keeps the verified stop on the cleanup lease and lets the continuation and the next remote writer accept it > - The benefit is that a redirect, Retry or interrupt continues right after its files are saved, for every lease policy ## Linked Issues or Issue Description Refs #19206 **What happened?** On hosted staging, a Stop or `interrupt: true` comment on a native run left a follow-up turn that started 5 to 8 minutes later for some agents, and 25 to 40 seconds later for others. The slow agents use ephemeral sandbox leases. In each slow case, the stopped run's lease was released by `pending_cleanup_retry` about 5 minutes after recovery, and the saved message started one second later. The fast agents use retained leases, which recovery releases at once. **Expected behavior** The saved message continues right after recovery saves the files and the sandbox is stopped. The destroy of a stopped sandbox is resource cleanup and must not hold the next turn. **Steps to reproduce** 1. Use an agent whose sandbox lease is ephemeral (destroy after each turn). 2. Interrupt or stop its native run so that recovery must save the sandbox files. 3. Post a message. See the follow-up start only after the sandbox destroy finishes. **Paperclip version or commit** Hosted, at `56517dda`. ## What Changed - `server/src/services/workspace-recovery.ts`: `scheduleRecoveredWorkspaceCleanup` keeps the verified stop receipt as `workspaceRecoveryCleanup.stopReceipt` when it queues the destroy. - `server/src/services/remote-execution-termination.ts`: new `hasRecoveredCleanupStopReceipt(lease)`. It is true only when the lease has recovery cleanup for its own run, was released, and the carried receipt matches the lease, run, provider and provider lease id. - `server/src/services/explicit-native-continuation.ts`: the `remote_cleanup` gate accepts that receipt. - `server/src/services/execution-blocker.ts`: `hasPendingRunCleanup` ignores a lease with that valid receipt, so a saved native interrupt send (and Retry or fresh-workspace checks) does not wait for the destroy. Other cleanup holds are unchanged. - `server/src/services/remote-workspaces.ts`: the next remote writer accepts that receipt for the ended writer's lease. - `server/src/services/execution-blocker.ts` (from #19206 review, not in the merged head): pre-turn evidence uses the exact startup lifecycle event list, so turn-scoped events such as `workspace.diff.recorded` count as provider work. - `doc/execution-semantics.md`: describes the rule. ## Verification - Red before the fix, green after: `cd server && npx vitest run src/services/workspace-recovery.test.ts -t "admits the interrupt redirect"`. An interrupted native run with recovered files and a queued destroy blocked the user message with `remote_cleanup` before the fix. After the fix the message is admitted. A lease without the stop receipt, or with a receipt for another run, still blocks with `remote_cleanup`. The test runs the same input as a plain message and as a saved native interrupt send (`queuedCommentInterruptId`); `hasPendingRunCleanup` was true before the fix and blocked the interrupt send with `cleanup_pending`. - `cd server && npx vitest run src/services/remote-workspace-reservation.test.ts -t "ended writer"`: the next writer reserves the workspace from an ended writer whose recovered lease awaits destroy; a missing or foreign receipt still blocks. Removing the new branch fails the positive case. - Updated `saves the authoritative remote workspace after native failed ...`: Retry now continues before the destroy, and the cleanup lease carries the stop receipt for its run and lease. - `workspace-recovery.test.ts -t "provider work cannot be excluded"` includes `workspace.diff.recorded` as provider work. - Related suites pass: `workspace-recovery.test.ts`, `explicit-native-continuation.test.ts`, `remote-workspace-reservation.test.ts`, `remote-workspaces.integration.test.ts`, `remote-workspace-lifecycle.test.ts`, `task-retry*.test.ts`, `native-cancelled-workspace-preservation.test.ts`, `workspace-recovery-source.test.ts`. `cd server && npx tsc --noEmit` passes. - Hosted evidence (read only): slow cases have ephemeral leases released by `pending_cleanup_retry` about 5 minutes after `issue.workspace_recovered`, with settlement one second later. Fast cases have retained leases released by recovery. ## Risks - Low to medium. The next turn can start while the old stopped sandbox is still being destroyed. The receipt proves that the sandbox was stopped after the files were saved, and an ephemeral successor uses a new sandbox. A destroy failure stays with the existing cleanup retry. - Leases without the carried receipt behave as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
beb73381ca |
fix(native): let a Stop before the provider session opens continue once (#19206)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A remote task restores its saved workspace into a sandbox before the agent starts. > - When that restore fails, the run records only `remote_workspace_operation_failed: operation=restore exit=1`. > - The run then fails as `setup_failed`. No retry starts, and a `blocked` task gets no recovery action. > - An operator cannot tell which step failed, and nobody owns the next step. > - This pull request makes the sandbox scripts name their failed step in a closed grammar, and it gives a restore failure a bounded retry or a named board recovery action. > - The benefit is a visible cause and a clear owner. Saved work stays protected. ## Linked Issues or Issue Description Refs #19152 and #19187. Those changes name copy-back (save) failures. Refs #18852 and #19195. Those changes added the bounded bootstrap retry that this change reuses. **What happened?** A board reply woke an agent on a `blocked` parent task. The run failed during setup 13 seconds later. The only run event was `remote_workspace_operation_failed: operation=restore exit=1`. The run had no workspace operation row. The task stayed `blocked`, and its recovery actions showed `active: null`. The same opaque family also appeared for workspace preparation (`exit=1` only). **Expected behavior** The run names which restore step failed, without command output or paths. A failure before provider work uses a bounded retry when a retry can help. Otherwise, and when the retry budget is spent, the board gets a named recovery action. The saved version is never changed or overwritten. **Steps to reproduce** 1. Save a remote workspace, then start a new run on a different allocation. 2. Put a folder at the published workspace path in the sandbox before restore. 3. Observe that the base code records only `operation=restore exit=1`, then `setup_failed`, with no recovery action for a `blocked` task. **Paperclip version or commit** Base: `87dfba6818`. **Deployment mode** Legacy runtime with sandbox (Daytona) remote workspace storage. ## What Changed - The prepare, repository, and restore sandbox scripts track their current stage. On failure they print `remote_workspace_<op>_failed: stage=<stage>` plus closed fields: `reason=` (the script's own snake_case refusal), `archive=` (the archive helper's fixed reason), `code=` (errno), `disk=full`, `git=` (a fixed label chosen from Git output), and the existing `exit`/`signal`/`program` fields. - The server accepts that text only when it matches a strict grammar for the same operation. Other output is dropped, as before. - Provider refusals during the restore upload and restore command are recorded by closed labels, like copy-back. - A failed saved-workspace restore raises `RemoteWorkspaceRestoreError` with the server step (`snapshot_lookup`, `snapshot_download`, `sandbox_restore`). It is retryable only when the owner re-reads the same writer, epoch, and saved version with no unsaved work, and the sandbox did not name a refusal reason. - Setup failure handling classifies a pre-dispatch legacy restore failure as `remote_workspace_restore_failed`. A retryable failure uses the existing bounded bootstrap retry. A non-retryable or exhausted failure records a board-owned recovery action with the redacted cause, when stranded-work recovery does not scan the task status (for example `blocked`). - Stranded-work recovery gets a named notice for this code. Generic continuation recovery does not grant it a second budget, and legacy reconciliation does not invent provider actions for it. - `doc/execution-semantics.md` documents the contract. ## Verification - Red: the new transport tests failed on the base code with `remote_workspace_operation_failed: operation=restore exit=1` (and the same for prepare and repository). The new completion tests failed because the run became `setup_failed` with no retry and no recovery action. - Green: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts src/services/remote-workspaces.integration.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/services/legacy-execution-recovery.test.ts src/services/recovery src/services/remote-workspace-save-concurrency.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/__tests__/native-workspace-finalization-work.test.ts` passes (563 tests). - The transport tests use real sandbox commands. They prove an occupied folder reports `stage=inspect_root reason=occupied_root` and keeps its files. They prove a wrong manifest reports `stage=extract archive=manifest_mismatch`. They prove a missing branch reports `stage=checkout_branch`, and a changed origin reports `stage=verify_origin`. Forged output with a path is dropped. - The real-PostgreSQL cold-restore test proves an occupied root gives a non-retryable `sandbox_restore` failure and keeps the unknown files. - The completion tests cover retried, exhausted, refused, post-dispatch, and `in_progress` cases. - `tsc --noEmit` for the server reports no errors in the changed files. ## Risks - A retry keeps the same restore path. If the cause is deterministic but unnamed, the run uses its two retries before the board action. Named refusals skip the retries. - The board action is recorded only for statuses that stranded-work recovery does not scan, and only when no other action is active. This prevents a second recovery identity for one failure. - Script stderr grammar is new. Old sandboxes run the script that the server sends, so no runner update is necessary. - No migration, deployment, or credential change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use, code execution, and browser tools for read-only staging evidence. Context window and reasoning mode are host-managed. ## 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> |
||
|
|
f5891564ec |
fix(recovery): continue from a retired sandbox's backup and hand spent export repairs to the board (#19208)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native tasks keep a warm sandbox and a verified per-turn backup of the provider session. > - Recovery holds stop a task when continuation is not safe. They must have a next action and an owner. > - In a staging soak, two kinds of held tasks could not move forward. Users followed the product's own instruction, and nothing changed. > - First, a reply to a task whose warm sandbox Paperclip had already destroyed failed every time. The label then said "Send a message to continue" again. > - Second, an exhausted system workspace repair stayed active and system-owned for hours. The board saw no owner, and a reply started nothing. > - This pull request lets a reply continue from the backup when the source sandbox is provably gone, and hands a spent system repair to the board with a named Retry. > - The benefit is that productive work always has a next action that can succeed, and nothing replays or runs twice. ## Linked Issues or Issue Description **What happened?** (a) A native warm task ran one successful turn. A later legacy follow-up retired the warm sandbox lease. The next user reply got a new sandbox (`created` lease). The executor found a verified backup but no reusable lease, so it threw `runner_harness_state_mismatch: backup_without_reusable_lease`. The run retried on the restart-recovery path, failed `native_remote_recovery_lease_mismatch`, and ended as `native_continuation_requires_reconciliation`. Workspace recovery then said "Saved work recovered. Send a message to continue." Each new reply repeated the same failure. (b) A run failed `native_workspace_sync_out_retry_exhausted`. System workspace recovery used its 6 bounded attempts and became `exhausted`. The recovery action stayed `active` with `ownerType: system`. The UI hides system-owned recovery actions, so the board saw no action. Nothing ran for more than two hours. **Expected behavior** (a) When Paperclip destroyed the source sandbox and no provider work followed the backup, the reply continues once from the backup. Ambiguous states still fail closed. (b) When the bounded budget is spent, the action names a board owner and a Retry action that can progress it. **Steps to reproduce** (a) Finish a warm native turn. Retire its reusable lease through obsolete-lease cleanup. Send a reply. The new run gets a `created` lease and fails `backup_without_reusable_lease`. (b) Let system workspace recovery reach its deadline or retry limit. Read the recovery action. It is still `ownerType: system`. Related merged work checked first: #19145, #19184, #19187, #19189, #19190, #18943 (added the guard for migrated sessions). Open recovery PRs (`gh pr list --search recovery --state open`) do not touch these paths. ## What Changed - `native-retired-backup-source.ts` (new): `retiredSourceAuthorizesNativeBackupRestore` decides whether a created lease may restore the backup. All must be true: the backup's source lease is `expired` or `failed`; it has a provider termination receipt with state `destroyed`; its `nativeHarnessBackup` stamp names this exact manifest SHA-256, session and runner instance; exactly one host lease maps to the provider allocation; and no provider durable event for the session was recorded after the backup completed. The provider event type set moved here and the executor imports it. - `native-session-executor.ts`: `shouldRestoreNativeHarnessBackupIntoSandbox` accepts `retiredSourceContinuation`. A created lease with a configured reusable lease evaluates it, beside the existing migrated-session path. The restore uses the existing verified restore and stamps the new lease. - `system-recovery.ts`: `updateSystemRecovery` moves an `exhausted` action to `ownerType: board` and writes a next action and workspace recovery message that name the attempt count, the last failure code, and **Retry recovery**. Retry (`requestWorkspaceRecovery`) already starts a new bounded generation and returns ownership to the system. - `doc/execution-semantics.md`: documents both rules. ## Verification - Red before the fix, green after: - `npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "restores a warm backup into a created lease only when"`. Before: the destroyed-source case rejected with `runner_harness_state_mismatch: backup_without_reusable_lease`. After: it restores the runner state and provider home into the new sandbox and stamps the new lease. - `npx vitest run src/services/workspace-recovery.test.ts -t "hands an exhausted system export repair"`. Before: `exhausted` with `ownerType: "system"`. After: `ownerType: "board"`, the blocker says Retry recovery and never says "send a message", no provider run or lease resume happens, and Retry starts one new system generation. - Safety cases (fail closed): a stopped (not destroyed) source, a stamp for another checkpoint, provider work after the backup, a still-reusable `released` lease, a missing receipt, and two host leases for one allocation. These run in the executor test and in `native-retired-backup-source.integration.test.ts` against embedded Postgres. - Related suites pass: `workspace-recovery.test.ts`, `system-recovery-state.test.ts`, `native-session-executor.test.ts`, `task-retry.test.ts`, `explicit-native-continuation.test.ts`, `workspace-recovery-source.test.ts`, `native-workspace-export-retry.test.ts`, `native-remote-retry-admission.test.ts`, `native-workspace-warm-reuse.test.ts`, `native-workspace-sync-retention.test.ts`, `heartbeat-observed-workspace-preservation.test.ts`, `execution-control-work.test.ts` (2,224 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - Not done: a live run against staging. Staging access was read-only. The staging evidence is the run event timeline described above. ## Risks - (a) A backup restore after a lost sandbox could rewind a session if newer provider state existed. The predicate requires a provider `destroyed` receipt and no provider event after the checkpoint. A stopped sandbox, a pending cleanup, or a still-reusable lease still fails closed with the old error. - (a) The event-time check can fail closed if a late event from the backup turn is written after the checkpoint time. That keeps the old behavior; it never authorizes a restore. - (b) The action's next action text stays as the exhausted message while a later Retry generation runs. The task blocker shows the live workspace recovery message first, so users see "Recovering saved work…" during that time. - Not changed: when the predicate fails closed, the run still follows the existing reconciliation path. That path can still end with the "Send a message to continue" receipt. That residual is narrower now and needs a separate change to the workspace recovery receipt. > 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. Agentic coding with tool use (shell, file edits, read-only browser inspection of staging). Default 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 - [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> |
||
|
|
2ce5f03cd8 |
fix(workspaces): name remote restore failures and leave a board recovery action (#19207)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A remote task restores its saved workspace into a sandbox before the agent starts. > - When that restore fails, the run records only `remote_workspace_operation_failed: operation=restore exit=1`. > - The run then fails as `setup_failed`. No retry starts, and a `blocked` task gets no recovery action. > - An operator cannot tell which step failed, and nobody owns the next step. > - This pull request makes the sandbox scripts name their failed step in a closed grammar, and it gives a restore failure a bounded retry or a named board recovery action. > - The benefit is a visible cause and a clear owner. Saved work stays protected. ## Linked Issues or Issue Description Refs #19152 and #19187. Those changes name copy-back (save) failures. Refs #18852 and #19195. Those changes added the bounded bootstrap retry that this change reuses. **What happened?** A board reply woke an agent on a `blocked` parent task. The run failed during setup 13 seconds later. The only run event was `remote_workspace_operation_failed: operation=restore exit=1`. The run had no workspace operation row. The task stayed `blocked`, and its recovery actions showed `active: null`. The same opaque family also appeared for workspace preparation (`exit=1` only). **Expected behavior** The run names which restore step failed, without command output or paths. A failure before provider work uses a bounded retry when a retry can help. Otherwise, and when the retry budget is spent, the board gets a named recovery action. The saved version is never changed or overwritten. **Steps to reproduce** 1. Save a remote workspace, then start a new run on a different allocation. 2. Put a folder at the published workspace path in the sandbox before restore. 3. Observe that the base code records only `operation=restore exit=1`, then `setup_failed`, with no recovery action for a `blocked` task. **Paperclip version or commit** Base: `87dfba6818`. **Deployment mode** Legacy runtime with sandbox (Daytona) remote workspace storage. ## What Changed - The prepare, repository, and restore sandbox scripts track their current stage. On failure they print `remote_workspace_<op>_failed: stage=<stage>` plus closed fields: `reason=` (the script's own snake_case refusal), `archive=` (the archive helper's fixed reason), `code=` (errno), `disk=full`, `git=` (a fixed label chosen from Git output), and the existing `exit`/`signal`/`program` fields. - The server accepts that text only when it matches a strict grammar for the same operation. Other output is dropped, as before. - Provider refusals during the restore upload and restore command are recorded by closed labels, like copy-back. - A failed saved-workspace restore raises `RemoteWorkspaceRestoreError` with the server step (`snapshot_lookup`, `snapshot_download`, `sandbox_restore`). It is retryable only when the owner re-reads the same writer, epoch, and saved version with no unsaved work, and the sandbox did not name a refusal reason. - Setup failure handling classifies a pre-dispatch legacy restore failure as `remote_workspace_restore_failed`. A retryable failure uses the existing bounded bootstrap retry. A non-retryable or exhausted failure records a board-owned recovery action with the redacted cause, when stranded-work recovery does not scan the task status (for example `blocked`). - Stranded-work recovery gets a named notice for this code. Generic continuation recovery does not grant it a second budget, and legacy reconciliation does not invent provider actions for it. - `doc/execution-semantics.md` documents the contract. ## Verification - Red: the new transport tests failed on the base code with `remote_workspace_operation_failed: operation=restore exit=1` (and the same for prepare and repository). The new completion tests failed because the run became `setup_failed` with no retry and no recovery action. - Green: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts src/services/remote-workspaces.integration.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/services/legacy-execution-recovery.test.ts src/services/recovery src/services/remote-workspace-save-concurrency.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/__tests__/native-workspace-finalization-work.test.ts` passes (563 tests). - The transport tests use real sandbox commands. They prove an occupied folder reports `stage=inspect_root reason=occupied_root` and keeps its files. They prove a wrong manifest reports `stage=extract archive=manifest_mismatch`. They prove a missing branch reports `stage=checkout_branch`, and a changed origin reports `stage=verify_origin`. Forged output with a path is dropped. - The real-PostgreSQL cold-restore test proves an occupied root gives a non-retryable `sandbox_restore` failure and keeps the unknown files. - The completion tests cover retried, exhausted, refused, post-dispatch, and `in_progress` cases. - `tsc --noEmit` for the server reports no errors in the changed files. ## Risks - A retry keeps the same restore path. If the cause is deterministic but unnamed, the run uses its two retries before the board action. Named refusals skip the retries. - The board action is recorded only for statuses that stranded-work recovery does not scan, and only when no other action is active. This prevents a second recovery identity for one failure. - Script stderr grammar is new. Old sandboxes run the script that the server sends, so no runner update is necessary. - No migration, deployment, or credential change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use, code execution, and browser tools for read-only staging evidence. Context window and reasoning mode are host-managed. ## 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> |
||
|
|
9e9a628a57 |
feat(ui): turn Artifacts V1 on for every instance and hide its toggle (#19164)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Artifacts show the output of agent work on the Artifacts page, in the panel Artifacts tab, and in the task thread > - #17228 added the Artifacts V1 presentation behind an experimental setting that is off by default > - The presentation is now ready for all users, so an opt-in toggle is no longer necessary > - A new default alone does not reach all instances, because each experimental write persists every flag > - This pull request removes the toggle and turns V1 on for every instance > - The benefit is one artifact presentation for all users, with an off switch for hosting operators ## Linked Issues or Issue Description Refs #17228 Refs #17263 **Is your feature request related to a problem? Please describe.** Artifacts V1 shows only on instances where an admin turned on an experimental setting. Other users see the previous cards. **Describe the solution you would like** Show Artifacts V1 on all instances. Remove the toggle from Experimental settings. **Describe alternatives you have considered** A new default without a change to the hook. This does not work, because many instances have a stored false that nobody chose, and no toggle would remain to change it. ## What Changed - Removed the Artifacts V1 card from Instance Settings, Experimental. - Changed `enableArtifactsV1` to default on in the schema, the feature catalog, and the settings service. - Changed `resolveArtifactsV1Enabled` to ignore a stored false. It honours false only when the value comes from managed config. - Kept the flag and the previous cards, so a hosting operator can hold V1 off. - Kept the key in the Cloud staging list, so a managed false does not hold V1 off on staging. - Updated unit tests, the Artifacts V1 plan, and the directions-lab Today reference. - Moved the attachment and upload e2e specs and the runner download helper to the V1 rows. - Added V1 tests for a Markdown review: expansion, deep link, and open, raw and download actions. ## Verification - `cd ui && npx vitest run`: one test file failed before its fix. That file tests the previous cards, and it now mocks the hook to off. I reran it with the other changed UI tests, and they pass. I did not repeat the full UI run after the fix. - `npx playwright test --config tests/e2e/playwright.config.ts tests/e2e/text-attachment-tabs.spec.ts tests/e2e/artifact-uploads.spec.ts tests/e2e/artifact-tab-arrival.spec.ts`: 10 tests pass on V1. - `tests/e2e/cloud-auth.spec.ts` failed once in CI on a task heading. It does not touch artifacts, and 10 local runs pass. - `cd server && npx vitest run src/__tests__/instance-settings`: 183 tests pass. - `cd packages/shared && npx vitest run src/feature-catalog.test.ts src/validators/instance.test.ts`: 35 tests pass. - `pnpm run check:token-gates` and the `ui` and `server` typechecks pass. I did not run the full repository check. - In the artifact directions lab, with no stored settings, the Today right panel renders the V1 rows. - I did not test this on a live instance. After merge, confirm on staging that the Experimental page has no Artifacts V1 card and that all three surfaces show V1. ## Risks - All users see V1 after the next release, not only staging users. To revert, revert this pull request. - Users cannot turn V1 off. Only a hosting operator can, through managed config, and not on Cloud staging. - An instance with a managed `enableArtifactsV1: false` stays on the previous cards. The cloud harness source has no reference to this key, but I cannot read the live fleet configuration. - V1 surfaces render before settings load. An operator-held instance can briefly show V1 before the previous cards. - Admins who use the API can still write the flag. The write has no effect on unmanaged instances. > 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 and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
ea42a2e417 |
fix(recovery): coalesce duplicate legacy no-replay holds (#17055)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery records keep uncertain execution outcomes from being replayed. > - A settled record can still block replay while it waits for a verified decision. > - Repeated scans can create more settled records for the same failed execution. > - Resolving only one duplicate leaves the task behind another hold. > - This pull request retains the settled hold and applies one verified decision to its exact duplicates. > - The task can continue through the existing delivery path without creating another delivery owner for each duplicate. ## Linked Issues or Issue Description Refs #14600. This change addresses duplicate settled holds and their same-source resolution. The visibility and other deferred-wake paths remain outside this change. Related: #10011 handles unchanged active incidents. This patch handles settled no-replay records. #15285 covers missing reconciliation input. #17031 covers a handoff to a new task owner. External-adapter continuation is covered separately by #15982. ## What Changed - Retain a resolved legacy no-replay decision when a later scan finds the same company, task, cause, fingerprint, and source run. - After a verified reconciliation, resolve duplicate records that also match the selected action's kind. Keep the selected action as the delivery owner for this group. Record the merged action IDs in the activity log. - Preserve independent workspace and system recovery records, existing reconciliations, and records for other sources. - Add PostgreSQL regression tests for repeated scans, historical duplicates, delivery count, repeated resolution, scope boundaries, simultaneous resolutions, and rejected reconciliation rollback. - Update the execution semantics and recovery maps. This patch has no schema migration or adapter-specific code. ## Verification Current head: `3c84bc89325d40bfdda5d28f24ed2fb67d48756c`. It merges `master` at `043f2514e67c50320433a0013fb228a75a51325c` without rewriting the existing commits. The only conflict was in `markExecutionReconciliation`. The resolution retains the upstream transaction and all three queue notifications. Source, sibling, and audit writes use the same transaction. The final merged tree passed: ```sh pnpm exec vitest run server/src/__tests__/issue-recovery-actions.test.ts server/src/services/legacy-execution-recovery.test.ts server/src/__tests__/heartbeat-process-recovery.test.ts server/src/__tests__/execution-control-work.test.ts --maxWorkers=1 # 549 passed; zero skipped pnpm --filter @paperclipai/server exec tsc --noEmit # exit 0 node scripts/check-maps.mjs # passed: 35 areas, 191 capability IDs git diff origin/master --check # passed ``` Three new cases add real PostgreSQL and HTTP-route evidence. Two resolution requests target different duplicate holds while both wait on database locks. They leave one continuation owner, one coalescing audit record, and one queued successor. A live-provider rejection leaves all records unchanged. A checkout rejection after reconciliation writes rolls back the source, siblings, and audit row. The fixture occupies the dispatch slot, so no external model provider runs. The new contention case was also run with only `execution-recovery-resolution.ts` replaced by its parent revision at `65f59e339a9fb6e0635bcbd8c36791bd610326a6`. It failed with two continuation owners instead of one. The source was restored byte-for-byte, and all three new cases passed again before the merge. The final four-suite run above validates the merge with the upstream notification changes. The original positive regressions also failed on that parent revision: three scan-and-settle cycles left four records instead of one, and resolving one historical hold left its two duplicates unreconciled. The scope matrix covers company, task, kind, cause, fingerprint, source run, replay state, protected recovery records, and prior reconciliation. Fresh-checkout collection initially failed because plugin SDK build outputs were absent. After building them, PostgreSQL startup exposed native library symlinks missing from an installation with scripts disabled. The package's symlink setup resolved that failure. The standard server typecheck attempt started a Rust vendor prerequisite build and was stopped; direct TypeScript checking passed. These failed or incomplete attempts are not counted as passes. Full repository build, recursive typecheck, and CI were successful on the earlier `08e5c0285` revision ([CI run](https://github.com/paperclipai/paperclip/actions/runs/39541949702)). They are historical evidence, not results for this merge. The current local proof is the four suites and checks listed above. Full upstream CI for the merge is pending. Greptile reviewed `3c84bc89325d40bfdda5d28f24ed2fb67d48756c` with 5/5 confidence and no actionable findings; that source review is separate from test execution. ## Risks - A match that is too broad could clear an independent hold. Tests cover company, task, kind, cause, fingerprint, source run, replay state, workspace recovery, system recovery, and prior reconciliation boundaries. - The existing issue-lock transaction covers the selected action and its duplicates. The patch does not create a new lock or an automatic retry path. - The patch does not infer that a network or provider failure means no action occurred. Existing reconciliation validation still checks the source, owner, stopped processes, leases, and observed action outcome. - Previously reconciled records and their delivery state remain unchanged. This patch prevents new duplicate delivery owners when it resolves unreconciled duplicates. It does not migrate every historical delivery record. ## Model Used OpenAI Codex, based on GPT-6, assisted with the implementation, tests, and review. It used repository tools and code execution. The exact runtime model ID, context window size, and reasoning setting are not exposed in this session. An independent AGY review also ran; its provider model ID was not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with the available 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 linked the existing issue with `Refs #14600` - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal task IDs or instance details - [x] I have run the focused tests locally and they pass; full upstream CI is pending for the final merge - [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 - [ ] 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: Rockiez <7697772+Rockiez@users.noreply.github.com> Co-authored-by: Devin Foley <devin@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
87dfba6818 |
fix(native): retry an unpublished accepted task reply durably instead of dropping it (#19200)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run commits its status decision and its final reply comment
in separate steps
> - Background presentation recovery publishes an accepted reply when
the live path does not record one
> - That worker dropped the run after one refused attempt and only threw
on an error, with no record on the run
> - A task could then show done while the user never saw the answer,
until an unrelated wake reloaded the queue
> - This pull request makes the publication of an accepted ordinary task
reply durable, bounded, and recorded
> - The benefit is that a task cannot close silently without its answer
## Linked Issues or Issue Description
Refs #19130 (verify requested publication at finalization, for
documents), #19183 (deliver messages saved while a native run
finalizes), #19186 (stale owner done). This change does not duplicate
them. It covers the final reply comment.
**What happened?**
A native run accepted a result with disposition `done`. The status
decision set the task to done. The accepted final reply was not
published as a task comment. The run had no publication record (no
`run.presentation.resolved` event and no `presentationDecision`). The
reply was published by background recovery 58 minutes later, after an
unrelated wake reloaded the presentation queue.
**Expected behavior**
The accepted reply is published promptly. A refused or failed attempt is
retried with backoff and never posts twice. If publication fails
permanently, the run and the task show a named failure with an owner.
**Steps to reproduce**
1. Commit an accepted native task result with disposition `done`, with
the final reply not yet visible to presentation recovery.
2. Run the native response presentation worker check once. The attempt
is refused.
3. On `master`, the worker returns `null` and drops the run. Nothing
records the attempt. Nothing retries it until another notification for
that scope.
**Paperclip version or commit**
`master` at
|
||
|
|
07a2941f0c |
fix(recovery): reclaim verified inactive provider caches on full disks (#19192)
Recover from a full-disk workspace save by reclaiming only a verified inactive provider installation under the existing source and maintenance authority. Commit the preparation inventory in Core before changing cache contents. Bind interrupted cleanup to stable filesystem identities and keep small staged generation markers when directory birth times are unavailable. Skip read-only or permission-denied assets and stop freeing blocks once marker space is available. Preserve task files, Git, session history and uncertain action receipts. Only the normal saved-workspace and provider-stop receipts permit continuation. Validated with 446 complete recovery regressions on the repair revision, subsequent focused recovery and transport suites, typecheck/build, and all exact-head hosted CI. Greptile reviewed the final head at 5/5 with no unresolved findings. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
037c58bb22 |
fix(daytona): keep the provider auto-stop strictly after the warm runner idle window (#19199)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A warm remote agent keeps its sandbox running after a turn. The host keeps a warm runner for an idle window (`runnerIdleTimeoutMs`, or the agent's `idleTimeoutMs`) > - The Daytona provider also stops an idle sandbox after `autoStopInterval` minutes. The default is 15 minutes > - On staging, the warm idle window was 900,000 ms (15 minutes). The two windows were equal > - A follow-up turn at the end of the warm window resumed a sandbox that Daytona was stopping. Daytona refused the resume in about 4 seconds, and the run failed at setup > - #19195 added a bounded retry for that refusal. It did not remove the cause > - This pull request makes the two windows non-overlapping by construction. Daytona derives its auto-stop for a warm reusable sandbox from the same warm idle value that the host uses > - The benefit is that a follow-up inside the warm window always finds a running sandbox, and idle sandboxes still stop ## Linked Issues or Issue Description Refs #19195 (bounded retry for a refused resume; this change removes the common cause). **What happened?** On a staging soak, three follow-up runs failed at setup with a refused sandbox resume. Each resume began 15 minutes 8 to 25 seconds after the previous turn released the sandbox. The warm idle timeout was 900,000 ms. Daytona's default `autoStopInterval` is 15 minutes. Nothing in the code kept these two values apart. **Expected behavior** A reusable sandbox that keeps a warm runner must not auto-stop inside the warm idle window or at its edge. The provider backstop must fire after the host's own idle handling, with a margin. Idle sandboxes must still stop. **Steps to reproduce** 1. Configure a Daytona sandbox environment with `reuseLease` and a warm runner with `runnerIdleTimeoutMs: 900000` (or an agent with `lifecycleMode: warm`, `idleTimeoutMs: 900000`). 2. Run a turn. Do not set `autoStopInterval`. 3. Before this change: the sandbox is created with `autoStopInterval: 15`, equal to the warm window. A follow-up near minute 15 races the provider stop. **Paperclip version or commit** `c656b95b6b` (current `master`). ## What Changed - Daytona plugin (`plugin.ts`): add `effectiveAutoStopIntervalMinutes`. For a reusable lease that keeps a warm runner, the effective auto-stop is at least `ceil(warm idle / 1 min) + 5` minutes. The plugin applies the floor only when the host forwards `warmRunnerIdleTimeoutMs`, or when the environment itself selects `runnerLifecycleMode: warm`. An explicit `0` (disabled), a longer configured value, `inherit` with a per-turn agent, per-turn, and non-reusable sandboxes keep the configured value. - Daytona plugin: use the effective value at sandbox creation. On resume, raise a shorter auto-stop on an existing sandbox. This update is best effort and bounded to 5 seconds; the #19195 retry still covers a refusal. The stale-handle refresh threshold keeps the configured interval, so a cached handle is not trusted for longer before the provider confirms the new interval. The warm window is not written to the saved normalized config. - Host (`runner-environment-lifecycle.ts`): add `resolveWarmRunnerIdleTimeoutMsForRun`. It returns the run's effective warm idle window (environment `runnerIdleTimeoutMs`, else the agent `idleTimeoutMs`) for a warm `paperclip_runner` sandbox run. It shares the lifecycle-mode resolution with `resolveRunnerEnvironmentForRun`. - Host (`environment-run-orchestrator.ts`, `environment-runtime.ts`): pass that value as `warmRunnerIdleTimeoutMs` to the sandbox-provider driver on acquisition and on remote recovery resume. The driver adds it to the provider worker config only. Lease identity, reuse matching, and lease metadata are unchanged. - Daytona manifest: describe the warm floor on `autoStopInterval`. - `doc/execution-semantics.md`: document the contract. ## Verification - Red (before the fix, same tests, with the floor disabled): 5 failures in `plugin.test.ts` "warm idle window versus Daytona auto-stop". The staging case failed with `expected 900000 to be greater than or equal to 1200000`. The resume case failed because `setAutostopInterval` was not called. - Red (host): the new `environment-runtime.test.ts` case first failed because the provider worker config had no `runnerIdleTimeoutMs`. The service wrapper dropped the field. The fix forwards it. - Green: `npx vitest run --config vitest.config.ts` in `packages/plugins/sandbox-providers/daytona`: 563 passed. This includes a setter that never settles (resume completes after the 5-second bound) and `inherit` with no forwarded window (explicit `autoStopInterval: 2` stays 2). - Green: `npx vitest run src/services/runner-environment-lifecycle.test.ts src/__tests__/environment-runtime.test.ts src/__tests__/environment-run-orchestrator.test.ts` in `server/`: 242 passed. This includes recovery resume forwarding the admitted warm window. - The regression tests assert the relationship `autoStop >= warm idle + margin` for several warm windows (125 s, 300 s default, 900 s, 1,800 s) and for an explicit `autoStopInterval: 15`. They are not constant checks. - `node scripts/typecheck-low-memory.mjs` passes. `pnpm typecheck` in the Daytona package passes. ## Risks - A warm reusable Daytona sandbox now stays running up to the warm window plus 5 minutes after its last activity, instead of 15 minutes. With a 15-minute window this is 20 minutes. Cost is bounded and idle sandboxes still stop. - The resume-time `setAutostopInterval` call is best effort. If it fails, the old interval stays and the #19195 retry still applies. - Other providers are unchanged. They receive one extra host-owned config key (`warmRunnerIdleTimeoutMs`) only for warm `paperclip_runner` runs, and they ignore unknown keys. - No lease identity change, so existing retained sandboxes are still reused. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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> |
||
|
|
c656b95b6b |
fix(environments): retry a refused sandbox resume with the bounded bootstrap budget (#19195)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A warm remote agent keeps its sandbox after a turn, so a follow-up
turn resumes the same sandbox and its files
> - The sandbox provider auto-stops an idle sandbox (Daytona: 15 minutes
by default). The warm native idle window is also 15 minutes
> - A follow-up that reaches the sandbox during that stop gets a
provider refusal. The host keeps the sandbox ("the lease was preserved
and no replacement was created"), which is correct
> - But setup then records a generic `setup_failed`. No retry is
scheduled, no cause is recorded, and the reply that woke the agent is
effectively dropped
> - This pull request gives that refusal its own failure code and routes
it through the existing bounded bootstrap retry. The retry resumes the
same recorded sandbox
> - The benefit is that a follow-up continues a few seconds later on the
same saved sandbox, instead of stopping the task with no next action
## Linked Issues or Issue Description
Refs #18852 (the bounded bootstrap retry that this change reuses).
Related: #19152 and #19187 (copy-back causes), #19178 (Daytona
errored-sandbox recovery before stop-only cleanup), #19182 and #19180
(recovery holds). This change does not overlap with them: they cover
cleanup, copy-back, and recovery after a run. This change covers a
resume refusal before provider work starts.
**What happened?**
On a staging soak, two follow-up turns failed at setup after a
successful first turn:
- A board reply woke the agent. The run failed after 4.9 seconds with
`setup_failed`: `Reusable sandbox lease "…" could not be resumed; the
lease was preserved and no replacement was created.` The issue stayed in
review with no recovery action. The reply was not delivered.
- An earlier run on another task failed with the same message.
Both resume attempts started 15 minutes 8–25 seconds after the previous
turn released the sandbox. The first turns used `keep_running` with a
warm idle timeout of 900,000 ms. The Daytona default `autoStopInterval`
is also 15 minutes. So the resume reached the sandbox while the provider
was stopping it. The provider refused within seconds; this was not a
long resume timeout. The provider cause was not recorded, so this timing
is the strongest evidence. The exact provider state remains an
inference.
**Expected behavior**
A refused resume before provider dispatch is a temporary infrastructure
condition. The host retries it with a small, bounded budget. Each retry
resumes the same recorded sandbox. A refusal does not prove that the
sandbox is lost, so the host never destroys it or creates a replacement.
Only an explicit provider not-found or a workspace identity mismatch
with a verified backup permits replacement (unchanged). The run records
why the provider refused, without provider text.
**Steps to reproduce**
1. Run a warm remote turn that keeps its sandbox.
2. Make the provider's `environmentResumeLease` throw a non-transient
error (for example, a 400/409 while the sandbox stops).
3. Wake the agent with a reply.
4. Before this change: the run fails with `setup_failed`, no retry is
scheduled, and the error has no cause.
**Paperclip version or commit**
Seen on staging at `5b99bdae`. The touched code is unchanged on current
`master` (`f7a4c5678f`).
## What Changed
- Move `ReusableSandboxResumeError` to
`server/src/services/reusable-sandbox-resume-error.ts` (re-exported from
`environment-runtime.ts`). Give it the code
`reusable_sandbox_resume_failed`.
- Add the provider refusal to the error message by closed labels only:
`(cause=<label> status=<http> code=<errno>)`. The labels come from the
existing transport classifier, now exported as
`sandboxProviderFailureLabel`. Provider text, paths, and IDs are never
copied.
- Carry the provider status across the plugin worker RPC. A JSON-RPC
error has only a message and a numeric RPC code. The worker host now
attaches a validated `paperclip/environment-resume-error/v1` envelope
(HTTP status and an allowlisted errno only) to `environmentResumeLease`
failures. The host re-validates it and adds it to the recorded cause.
- In setup-failure settlement, find this error through the orchestrator
wrapper. Record `reusable_sandbox_resume_failed` only when provider
dispatch did not start and the run has no native session or runner
identity (the same guard as `remote_workspace_prepare_timeout`).
- Add the code to `isTransientWorkspacePreparationCode`. The existing
scheduler then gives two retries (30 s, 60 s) with its issue gate, Stop,
pause, and dependency checks.
- Exclude the code from generic continuation recovery and from legacy
reconciliation (no unknown provider actions; bootstrap evidence proves
none started), like the template.
- Add a specific stranded-recovery notice: "Saved sandbox could not be
resumed".
- Document the contract in `doc/execution-semantics.md`.
## Verification
- Red (before the fix, same tests): 6 failures. The `fresh` case
recorded `setup_failed` and did not schedule a retry. The error had no
cause. Legacy reconciliation held the run. Generic recovery granted
another budget.
- Green: `npx vitest run src/services/heartbeat/run-completion.test.ts
src/services/legacy-execution-recovery.test.ts src/services/recovery/
src/__tests__/heartbeat-retry-scheduling.test.ts
src/__tests__/environment-runtime.test.ts
src/services/remote-workspace-transport.test.ts
src/services/heartbeat/retries.test.ts` (in `server/`): 619 tests pass.
Also `src/services/recovery/stranded-notice` and
`src/services/remote-workspace*`: 411 pass.
- Safety regressions:
- `run-completion.test.ts`: only a pre-dispatch legacy failure with no
native identity gets the code and a retry. `started`, `retained`
(native), and an untyped copy of the message stay `setup_failed` with no
retry. The recorded error contains the closed cause and not the provider
path.
- `heartbeat-retry-scheduling.test.ts` (real PostgreSQL): two concurrent
schedulers across a restart create exactly one successor per attempt.
The budget ends after two attempts. Stop, pause, and a new dependency
cancel the scheduled retry. Generic stranded recovery creates no extra
run after exhaustion.
- `packages/plugins/sdk/tests/environment-resume-error.test.ts`: through
the real worker RPC, the `JsonRpcCallError` has no status, and the
envelope gives 409 and the errno. Provider text does not cross. Other
methods get no envelope. Malformed envelopes are rejected.
- `environment-runtime.test.ts`: a throwing resume still preserves the
exact lease. The runtime does not call destroy or acquire. The resume
throws a `JsonRpcCallError` whose message has no status. The thrown
message names `cause=provider_error status=409` from the envelope, and
not the provider text.
- `node scripts/typecheck-low-memory.mjs` passes. `git diff --check`
passes.
- Not verified: a live staging run at this revision. The original tasks
were not retried. This PR is not deployed.
## Risks
- Low. The change affects only runs that fail with this typed error
before provider dispatch. Each retry resumes the same lease and inherits
the source wake context, so a reply is delivered once and no second
executor starts.
- If the provider cannot start the sandbox for longer than about 90
seconds, the two retries end and the task gets the specific recovery
notice. The sandbox and its saved files are kept.
- The error message now has a parenthetical cause before the semicolon.
No code parses this message.
- An issue in review does not get the immediate stranded notice
(unchanged existing behavior). The retry, not the notice, delivers the
reply in that case.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edits, browser reads of staging diagnostics).
Extended reasoning enabled.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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>
|
||
|
|
245b4d3b79 |
Preserve cancelled workspaces after controllerless Stop (#18820)
Admit cancelled workspace preservation after an audited Stop acknowledgement when no live controller received an interrupt. Retain the strict dispatched receipt for provider termination and operator handoff. Existing source, lease, coordinator and result ownership checks remain enforced. Validated with complete Stop/preservation suites (134 tests), real-file recovery, workspace typecheck and build, and green hosted CI. Greptile and Paperclip both reviewed the exact head at 5/5 with no unresolved findings. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
3b17b5ce15 |
fix(native): keep a reassigned-away run from closing the new owner's task (#19186)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native agent run reports "done" in an accepted result. A finalizer then decides the task status. > - The board can reassign a task while the run is in flight. The old run is cancelled, but its accepted result still goes to the finalizer. > - The finalizer did not check who owns the task now. The old owner's "done" closed the task of the new owner. > - The new owner never did the work, and the task shows done with no reply from the current owner. > - This pull request makes a done report apply only when the reporting run's agent still owns the task. > - The benefit is that reassignment keeps its meaning: the new owner keeps the open task, and the old result stays as history. ## Linked Issues or Issue Description **What happened?** On a hosted staging instance, agent A started a native run on a task. Agent A finished the work and submitted a "done" result. Then the board reassigned the task to agent B, and Paperclip cancelled agent A's run with `issue_reassigned`. About 45 seconds later, the native finalizer committed agent A's done decision with reason `completion_claim_policy_accepted`. The task became done. Agent B's runs were then cancelled because the task was terminal. Agent B never worked on the task. **Expected behavior** A done report from a run that no longer owns the task must not close the task. The accepted result stays as history. The task status stays as it is for the current owner. **Steps to reproduce** 1. Assign a low-risk native task to agent A and start a run. 2. Submit a done result for the run. 3. Reassign the task to agent B before finalization. 4. Finalize the run. Before this change, the task becomes done. **Paperclip version or commit** Observed on staging at `b9fbbf8d30`. Reproduced on `master` at `06826915d7`. Related: #19130 adds a publication check for requested documents at finalization. It does not check task ownership. ## What Changed - `status-arbiter.ts`: add `ownsNativeTaskCompletion` and `supersedeStaleOwnerCompletion`. A done decision from an agent that is not the current assignee (another agent, a user, or nobody) becomes `preserve` with reason `completion_owner_superseded` and no effects. Other decisions do not change. This is the same ownership rule that the provider-failure path already uses (`ownsProviderFailureDecision`). - `native-run-finalizer.ts`: apply the rule after arbitration and before the deliverable check. Conversation tasks keep their existing path. - `native-status-reassessment.ts`: apply the same rule, so new evidence from a reassigned-away run cannot close the task later. - The status committer does not change. A reassignment during finalization already increments `statusVersion` (migration 0358). The committer then replans, and the retry applies the new rule. - `doc/execution-semantics.md`: describe the rule next to the existing "a late finalizer cannot change a reassigned task" statement. ## Verification New file `server/src/services/native-runtime/native-stale-owner-completion.test.ts`. It uses embedded PostgreSQL, the real coordinator store and the real finalizer. - Control: the owning run's done result closes the task with `completion_claim_policy_accepted`. - Reassigned before finalization: the task stays `in_progress` with agent B. The decision is `completion_owner_superseded`. The accepted result row is unchanged. - Reassigned after arbitration and before the status commit: the commit replans and the task stays `in_progress`. - Reassessment control: later approved evidence closes the owner's task with `decision_superseded_by_new_evidence`. - Reassessment after reassignment: later approved evidence from the old run does not close the task. The decision is `completion_owner_superseded`. This case fails when the reassessment rule is disabled. - Pure policy cases: another agent, a board user and no assignee all preserve status. A non-done decision passes through. Red before the fix (same tests, fix not applied): ```sh cd server && npx vitest run src/services/native-runtime/native-stale-owner-completion.test.ts # Tests 2 failed | 1 passed — both reassignment cases: expected "in_progress", received "done" ``` Green after the fix: ```sh cd server && npx vitest run src/services/native-runtime/native-stale-owner-completion.test.ts # Tests 10 passed cd server && npx vitest run 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 \ src/services/native-runtime/agent-chat-response-recovery.test.ts src/__tests__/native-run-projection-work.test.ts \ src/services/native-runtime/task-reassignment.test.ts src/services/native-runtime/native-provider-failure.integration.test.ts # 11 files, 614 tests passed ``` I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build` locally. CI runs them. ## Risks - A native run that reassigns its own task to a user or to another agent, and then reports done, no longer closes the task through arbitration. The status that the agent set through the API stays. This matches the existing provider-failure ownership rule. - A native run on a task with no assignee cannot close it through arbitration. Native task runs start from an assignment, so this is not expected in normal use. - Review runs and conversation tasks keep their existing paths. Review outcomes already preserve status before this rule. - No schema change and 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 redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Provider: Anthropic. Model: Claude Opus 5.5 (`claude-opus-5-5`), in the Claude Code agent harness. - Capabilities used: tool use, shell and code execution, repository search, browser reads of staging, and extended reasoning. - The model wrote the change, the tests and this description. A human requested the fix. ## 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> |
||
|
|
46fb933b01 |
fix(recovery): keep the stop receipt when a file-preserving restart fails (#19181)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs can execute in a remote sandbox. When a run fails, Paperclip stops the sandbox and records the provider's stop receipt on the environment lease. > - System workspace recovery can restart that stopped sandbox to save its files. Before the restart, it removed the stop receipt from the lease. > - If the restart failed and the provider could not stop the sandbox after that, the lease had no receipt. Continuation admission requires a stop receipt on every remote lease of the failed run. So every Retry and every comment waited for an environment that never stopped. > - This pull request keeps the original receipt until the provider confirms the restart. It restores the receipt when the restart fails and the sandbox cannot stop. A restart that fails because the sandbox disk is full now ends recovery with a fresh-workspace outcome. > - The benefit is that a failed file-preserving restart no longer blocks the task permanently. Saved input and Retry continue in a fresh isolated workspace, and the original sandbox stays retained. ## Linked Issues or Issue Description No public issue exists. This is a bug report in the format of `bug_report.yml`. **What happened?** 1. A native run failed because its Daytona sandbox filled its 10 GB disk. 2. Paperclip stopped the sandbox normally and recorded the stop receipt on the environment lease. 3. Some minutes later, system workspace recovery tried to restart the sandbox to preserve its files. Before the restart, `prepareStoppedSandboxForRecovery` deleted the stop receipt. 4. The restart failed on the full disk. The sandbox went into the provider's `error` state. 5. The stop-only teardown after the failed restart also failed, because the provider cannot stop a sandbox in that state. The lease stayed in `pending_cleanup` with no receipt. The cleanup sweep retried the same failing stop with backoff. 6. `admitExplicitNativeContinuation` requires a provider stop receipt on every remote lease of the failed run. Every Retry and every comment was deferred with "Waiting for the previous environment to stop. Your message will start automatically." The message never started. Also, after recovery was exhausted, a Retry replaced the exhausted generation with a new one. That generation repeated the same restart, and the Retry still reported success. **Expected behavior** A lease must not lose its stop receipt while the real sandbox state is unknown or not running. When the file-preserving restart cannot succeed, recovery must end in a state that lets the task continue, for example in a fresh isolated workspace. **Steps to reproduce** 1. Use a native run with a retained remote sandbox lease that has a `stopped` receipt and a pending workspace recovery. 2. Make the provider resume fail (for example with `no space left on device`), and make the provider stop fail. 3. Run `recoverRetainedWorkspace` until recovery is exhausted. 4. Save a user comment on the task and run the deferred-comment admission. 5. Before this change, the lease is `pending_cleanup` without a receipt. The comment stays deferred with reason `remote_cleanup`, and no successor run starts. **Paperclip version or commit** Observed on a hosted staging build. Reproduced on `master` at `6452e964a36` with the new tests in this PR. **Deployment mode** Hosted, with the Daytona sandbox provider plugin. The defect is in core server recovery code, so it is not specific to one adapter. ## What Changed - `server/src/services/sandbox-stop-and-retain.ts` - `prepareStoppedSandboxForRecovery` still writes the stop-only intent and moves the lease to `pending_cleanup` before the provider call. It now keeps the prior `remoteExecutionTermination` (and `sandboxStopAndRetainReceipt`, if present) as `priorStop` inside the `sandboxStopAndRetain` intent. - New `confirmSandboxRecoveryRestart` removes `priorStop` when the provider resume returns. From that point, the sandbox may be running, and only a fresh stop can settle the lease. - New `restoreSandboxStopAfterFailedRestart` restores the prior receipts and the `released` / `success` lease state. It works only while `priorStop` is present, so only when no restart was confirmed. It checks the restored receipt with `hasRemoteTerminationReceipt` first. It uses the same attempt fence as `settleStopOnlyCleanup`, and it writes through `writeEnvironmentCleanup` so dispatch and cleanup wake up. - `settleStopOnlyCleanup` now shares that attempt fence (`stopOnlyAttempt`). Its behavior does not change. - `server/src/services/workspace-recovery.ts` - After `resumeWorkspaceRecoveryLease` returns, recovery confirms the restart before any other check. - If the restart is not confirmed, recovery still tries the stop-only teardown first. A successful stop records a fresh receipt, as before. If the stop also fails, recovery restores the prior receipt instead of leaving the lease without one. - A later attempt that finds a pending stop-only lease (for example after a process restart during the reopen) uses the same rule when its stop fails. - A resume error that reports a full sandbox disk (`No space left on device` or `Disk quota exceeded`) becomes `source_unavailable` / `use_alternative_strategy`. The generation is exhausted at once, and the existing fresh-workspace continuation applies. Other resume errors keep the existing bounded retry policy (`provider_unavailable`, at most six attempts in 30 minutes). Because the receipt stays, that policy also ends in a state that lets the task continue. - `server/src/services/task-retry.ts`: Retry binds to an exhausted generation with a `use_alternative_strategy` failure and no fresh attempt, the same way it binds to a complete or running generation. It does not create a new generation that repeats a step the policy already rejected. The Retry continues in a fresh isolated workspace. An explicit workspace recovery request (`requestWorkspaceRecovery`) still starts a new generation. - `doc/task-recovery.md`: describes the receipt handling around the reopen and the Retry binding. - `server/src/services/workspace-recovery.test.ts`: four new tests (five cases) with the real database and mocked provider calls. ### Design choice I kept the existing write-ahead fence. The lease still leaves `released` and shows no receipt while the provider reopen is in progress. So no admission, reuse or destructive cleanup can act in that window, and a crash still leaves a stop-only obligation. The original receipt is not deleted. It stays in the stop-only intent and is discarded only when the provider confirms the restart. This makes one simple rule: the suspended receipt exists only while no restart has been confirmed. I did not use "restore only when the provider confirms that the sandbox is not running". The Daytona plugin reports a sandbox in the `error` state as `active` in `inspectLease`, and its `stopLease` cannot stop that state. So with the current plugin, no provider confirmation exists for this exact failure, and the hold would stay. A separate change to the Daytona plugin's `stopLease` can recover an errored sandbox before it stops it. This PR does not depend on that change. If that change lands and its recovery succeeds, the existing stop attempt records a fresh receipt, and the restore path is not used. I also did not change continuation admission to accept a lease in `pending_cleanup`. Many gates treat pending cleanup as unfinished work (`hasPendingRunCleanup`, the remote stop check and others). A change there would be wider and more sensitive. The restored receipt is the provider's original receipt, bound to the same company, run, lease, provider, allocation and plugin. No process ID or run status takes its place. ## Verification All commands ran from `server/` in a clean worktree of `master` on macOS (Apple Silicon), with the embedded PostgreSQL test database. Vitest ran from the real path. - Baseline on `master` (`6452e964a36`), before any change: `npx vitest run src/services/workspace-recovery.test.ts` → exit 0, 331 passed. - The new tests fail on `master`. I reverted only the three source files and kept the new tests: - `npx vitest run src/services/workspace-recovery.test.ts -t "file-preserving restart fails|without repeating a restart|keeps the stop obligation once|interrupted restart that the provider"` → exit 1, 4 failed, 1 passed. - In the comment cases, no successor run starts (`expected [] to have a length of 1`). A temporary log (not committed) showed the lease in `pending_cleanup` in both cases. In the `unknown_failure` case, the comment was deferred with `remote_cleanup` / "Waiting for the previous environment to stop. Your message will start automatically." This is the same hold as in the incident. - The guard test "keeps the stop obligation once the provider has confirmed a restart" passes on `master`, as expected. It makes sure that the fix does not restore a receipt after a confirmed restart. - With this change, the same command → exit 0, 5 passed. - `npx vitest run src/services/workspace-recovery.test.ts` on the committed head → exit 0, 336 passed (331 existing and 5 new). - Neighboring suites (`task-retry`, `explicit-native-continuation`, `heartbeat/recovery`, `environment-runtime`, and the other server test files that import the changed modules: `live-recovery-work`, `retained-native-session-cleanup-work`, `execution-control-work`, `legacy-workspace-restore-recovery`, `deferred-execution-receipt-work`, `issue-agent-mutation-ownership-routes`, `native-workspace-export-retry`, `native-cancelled-workspace-preservation`, `native-workspace-warm-reuse`, `native-workspace-reconstruction`, `native-failed-workspace-recovery`, `heartbeat/workspace-preparation`) → exit 0, 16 files, 1456 passed. - `npx vitest run src/__tests__/chat-channels.integration.test.ts src/__tests__/slack-conversation-lifecycle.test.ts src/services/native-runtime/preserved-native-response-wait.integration.test.ts` (other importers of `task-retry`) → exit 0, 3 files, 2074 passed. - `pnpm --filter @paperclipai/server typecheck` → exit 0. - Not run: the full `pnpm -r typecheck`, `pnpm test:run` and `pnpm build` repo checks. CI runs them. No live Daytona probe ran. New tests: - `lets saved input continue after a file-preserving restart fails on a sandbox that cannot stop` (`disk_full`, `unknown_failure`): resume fails and stop fails. The lease keeps its original receipt and is `released` / `success`. Recovery is exhausted (after one attempt for a full disk, and after six attempts for an unknown failure). The saved comment starts one successor run with `fresh_isolated_workspace` readiness. The source files stay in the retained sandbox. - `continues a Retry in a fresh workspace without repeating a restart that cannot succeed`: after a full-disk exhaustion, Retry keeps the same generation and binds its intent. The restart is not called again, and the Retry starts one run in a fresh isolated workspace. - `keeps the stop obligation once the provider has confirmed a restart`: resume returns, a later check fails, and the stop fails. The lease stays `pending_cleanup` without a receipt, and the intent has no `priorStop`. - `restores the stop receipt after an interrupted restart that the provider never confirmed`: the lease starts in `pending_cleanup` with a suspended receipt, as after a process restart during the reopen. The stop fails, the receipt is restored, and the attempt continues to the normal reopen. Assumption and limits (runtime dependency rule): the provider calls are mocked, and the database is real. The tests do not exercise the real Daytona error text. The full-disk check matches the patterns that Paperclip already uses for sandbox disk-full errors. If the provider error text does not match, recovery uses the bounded retry path, which now also ends in a fresh-workspace continuation. The change adds one database write after each confirmed reopen and one after a failed reopen and stop. It adds no polling. ## Risks - A resume can fail after the provider started the sandbox, for example when the reply is lost. If the stop-only teardown then also fails, recovery restores a `stopped` receipt for a sandbox that can be running. The old run's processes do not survive the earlier stop, so the receipt is still correct as proof that the old execution ended. But the retained sandbox can run until the provider stops it or a later recovery or cleanup stops it. Before this change, the same case caused a permanent hold. - A full-disk restart failure now exhausts recovery at once. The task continues in a fresh workspace without the failed run's files. The files stay in the retained sandbox. An operator can still request workspace recovery explicitly after the disk is freed. - Retry no longer renews an exhausted generation that has a `use_alternative_strategy` failure and no fresh attempt. For destroyed or missing sources, the result is the same as before but without the extra attempt. - No schema or migration change. A suspended receipt is one extra field in the existing `sandboxStopAndRetain` metadata. Older servers ignore the field. The intent equality checks compare the stored value, so a mixed-version deploy keeps working. - #18820 also changes `server/src/services/workspace-recovery.test.ts`. The two PRs touch different tests. > 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 (model ID `claude-opus-5-5`), running as a Claude Code agent with tool use (shell, file reads and edits, local test and typecheck runs) and extended reasoning. The runtime does not expose the context window size to the agent. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (affected suites; full repo runs are left to CI, as noted 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
895cee28b3 |
fix(daytona): recover an errored sandbox before stop-only cleanup (#19178)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Remote agent runs use sandbox provider plugins. Daytona is one of
them.
> - After a failed run, Paperclip stops the sandbox and keeps its files
("stop-only cleanup"). A task can continue only when every remote lease
of the failed run has a provider stop receipt.
> - Daytona rejects Stop for a sandbox in its `error` state. A sandbox
that filled its disk went to that state. The plugin called Stop on it,
so every cleanup sweep failed with the same generic error.
> - The lease stayed in pending cleanup without a stop receipt. Each
Retry or comment waited forever with reason `remote_cleanup`. A manual
Daytona Recover was the only fix.
> - This pull request makes the Daytona stop-only hook recover a
recoverable sandbox once, with a bound, and then stop it. When it cannot
stop the sandbox, it returns a typed failure. The server escalates a
terminal failure at once instead of retrying it like a transient error.
> - The benefit is that a full-disk sandbox no longer blocks its task,
and a sandbox that cannot recover is visible to operators on the first
attempt.
## Linked Issues or Issue Description
No public issue exists. Bug description:
**What happened?**
A task sandbox filled its 10 GB disk. Daytona then reported the sandbox
as `state: error`, `desiredState: started` and `recoverable: true`. Its
`errorReason` was a Docker `no space left on device` error. The Daytona
`stopLease` hook calls Stop on any sandbox that is not `stopped` or
`archived`. Daytona rejects Stop in the `error` state. The pending
cleanup sweep ran about every 30 minutes, and it failed on every pass.
The lease stayed `pending_cleanup` with `cleanup_status=failed` and no
stop receipt. Continuation admission
(`server/src/services/explicit-native-continuation.ts`) requires a stop
receipt on every remote lease of the failed run. So each Retry and
comment was deferred with "Waiting for the previous environment to stop.
Your message will start automatically." It never started.
A manual Daytona `POST /api/sandbox/{id}/recover` fixed it. The
sandboxes reached `started` in about one minute. The next sweep stopped
them, recorded the receipt, and the tasks continued.
**Expected behavior**
Stop-only cleanup recovers a recoverable errored sandbox and then stops
it. When the sandbox cannot be recovered, cleanup returns a clear, typed
failure that the server can tell apart from a transient error. Cleanup
never deletes the retained sandbox, and it never loops on Recover.
**Steps to reproduce**
1. Run a task in a Daytona sandbox until the sandbox fills its disk and
Daytona shows `state: error`, `recoverable: true`.
2. Let the run fail, so the server records a `sandboxStopAndRetain`
intent and calls `environmentStopLease`.
3. Before this change, Stop fails with `Sandbox failed to stop with
status: error` on every sweep, and the task cannot continue.
4. The new tests in `plugin.test.ts` reproduce this with a mock sandbox
whose Stop fails from `error`.
**Paperclip version or commit**
Observed on a hosted deployment. Reproduced in unit tests on `master` at
`6452e964a36`.
**Deployment mode**
Managed Daytona sandbox execution with plugin-backed stop-only cleanup.
Related: #18172 (Daytona exports under disk pressure) and #19050
(session reset on every Daytona restart path). This PR does not change
the resume path in `ensureSandboxStarted`, which already recovers a
recoverable sandbox.
## What Changed
- **Daytona plugin
(`packages/plugins/sandbox-providers/daytona/src/plugin.ts`)**: after
`stopLease` confirms the retention policy, a sandbox in `error` goes
through `recoverSandboxForStop`.
- Recoverable: the hook calls `sandbox.recover()` once. The SDK waits
for `started`. The plugin bounds the wait to 20 s, plus the existing
liveness margin for a hung connection. Then the hook stops the sandbox
normally and returns the usual `stopped` receipt.
- Not recoverable: the hook makes no Recover or Stop call. It throws
`PluginEnvironmentStopError("resource_unrecoverable")`.
- Recover fails: the hook reads the provider state once. A sandbox back
in `error` gives `recovery_failed`, or `resource_unrecoverable` if
Daytona now marks it so. A sandbox that is still starting, or a failed
read, gives `recovery_incomplete`.
- The recovered sandbox fails again before Stop finishes (for example,
the disk fills again): a fresh read that shows `error` gives
`recovery_failed`. The hook never calls Recover a second time in the
same call.
- The hook never deletes or archives the sandbox. The non-error stop
path is unchanged.
- **Plugin SDK (`packages/plugins/sdk/src/environment-stop-error.ts`,
new)**: `PluginEnvironmentStopError`, `environmentStopErrorData` and
`readEnvironmentStopFailure`. This follows the existing
`PluginEnvironmentRunnerIngressError` pattern. The envelope holds only
one closed reason. The reader validates the reason and derives
`terminal` from it. `resource_unrecoverable` and `recovery_failed` are
terminal. `recovery_incomplete` is not.
- **Worker RPC host (`packages/plugins/sdk/src/worker-rpc-host.ts`)**:
forwards that envelope as JSON-RPC error `data` for
`environmentStopLease` only.
- **Pending cleanup sweep
(`server/src/services/heartbeat/recovery.ts`)**: the catch reads the
validated envelope.
- On a terminal failure, it escalates on the first attempt. It writes
the one-time "needs operator attention" warning and uses the existing 30
minute retry interval. Before, this happened only at the fifth attempt.
- The failure log uses `errorKind: "provider_stop_terminal"` and
includes the closed `providerStopFailure` reason. The sweep still never
logs exception fields.
- The lease stays `pending_cleanup`. Automatic cleanup continues, and an
explicit Retry still bypasses the cooldown. Untyped and transient
failures keep their current behavior.
- **Docs**: the SDK README ("Stop failures"), the Daytona README,
`doc/plugins/SANDBOX_PROVIDER_CAPABILITIES.md` and
`doc/native-workspace-finalization-recovery.md` describe the new
behavior.
Shared contract note (module boundaries): the SDK envelope is the
smallest addition that lets the host tell a terminal provider stop
failure from a transient one. The worker host drops all other error data
across the RPC boundary, so the existing contracts cannot carry the
signal. The only consumer is the pending cleanup sweep. When a provider
does not throw the new error, or an older host ignores it, behavior is
unchanged. This PR does not touch
`server/src/services/workspace-recovery.ts` or
`sandbox-stop-and-retain.ts`.
## Verification
All commands ran in a clean worktree from `master`. Vitest ran from the
real path.
- Red on `master` (only the new tests added):
- Daytona, with `plugin.ts` from `master`: `npx vitest run --config
vitest.config.ts src/plugin.test.ts -t "provider error state"` exits 1.
All 7 new tests fail. The main case fails with `Sandbox failed to stop
with status: error`, which is the production error.
- SDK, with `worker-rpc-host.ts` from `master`: `npx vitest run
tests/worker-rpc-host.test.ts -t "typed stop failure"` exits 1 (the
typed case fails).
- Server, with `recovery.ts` from `master`: `npx vitest run
src/__tests__/heartbeat-pending-cleanup-sweep.test.ts -t "terminal
provider stop failure"` exits 1. All 3 cases fail (`errorKind:
"destroy_failed"`, no escalation).
- Green on this branch:
- `packages/plugins/sandbox-providers/daytona`: `npx vitest run --config
vitest.config.ts` exits 0 (534 passed, 26 skipped live tests). `pnpm
typecheck` exits 0. `pnpm build` exits 0.
- `packages/plugins/sdk`: `npx vitest run` exits 0 (222 passed). `pnpm
--filter @paperclipai/plugin-sdk typecheck` exits 0.
- `server`: 15 related suites exit 0 (1498 passed): pending cleanup
sweep, `heartbeat/recovery`, environment runtime, environment cleanup
work, native startup cleanup, native remote failure retention, observed
workspace preservation, retained native session cleanup, idle sleep
safety, explicit native continuation, task retry, workspace recovery,
native failed and cancelled workspace preservation, and Daytona close in
the plugin worker manager.
- `pnpm --filter @paperclipai/server typecheck` exits 0. `pnpm
check:module-boundaries` exits 0.
- New tests:
- Daytona: a recoverable errored sandbox is recovered once
(`recover(20)`) after the retention check and before Stop, then stopped.
An unrecoverable one returns `resource_unrecoverable` with no Recover,
Stop, start, delete or archive call. A failed recovery makes exactly one
Recover call for each outcome (`recovery_failed`,
`resource_unrecoverable`, `recovery_incomplete`). A Recover call that
never settles is cut off by the bound. A recovered sandbox that fails
again before Stop gives `recovery_failed` with no second Recover.
- SDK: the envelope holds only the reason. A forged or non-SDK error
gives no data. Unknown reasons, accessor fields and a sender-supplied
`terminal` flag are not trusted. The worker host forwards the typed
failure and drops a plain error's detail.
- Server: terminal reasons escalate on attempt 1 with the long retry and
one operator warning. `recovery_incomplete` keeps the short retry. The
lease stays `pending_cleanup`, and the provider message never reaches
the logs.
- Not run: the full `pnpm -r typecheck`, `pnpm test:run`, `pnpm build`
and `pnpm run typecheck:build-gaps` repo checks. CI runs them. No live
Daytona probe ran. The mocks model the Daytona SDK `recover()` contract:
it waits for `started` and rejects when the sandbox returns to `error`.
## Risks
- Time budget: the host gives `environmentStopLease` at most 60 s. A
recovered stop uses up to 20 s for Recover and up to 30 s for Stop, plus
short reads. A slower recovery returns `recovery_incomplete`. The SDK
call can continue at Daytona after that, so the next sweep finds the
sandbox started and stops it normally. If the host deadline fires while
the worker still stops the sandbox, the next sweep sees `stopped` and
records the receipt.
- Classification: after a failed Recover, the plugin cannot tell a
rejected request from a sandbox that started and failed again. Both give
`recovery_failed` (terminal) when Daytona still shows `error`. A hung
Recover call with a sandbox still in `error` is also terminal. Terminal
failures still retry every 30 minutes, and each retry makes at most one
Recover call. An explicit Retry also makes one attempt. There is no
tight loop. A sandbox that fails Recover every time gets about two
Recover calls an hour until an operator acts.
- Recover restarts the sandbox container before Stop. The plugin runs no
command in it, and the retention policy is confirmed before Recover.
- The continuation message for a terminally failed lease still says the
message "will start automatically". This PR does not change admission. A
follow-up can use the typed reason there.
- Compatibility: a host without this change ignores the new error data
and keeps the old retry behavior. A plugin without this change sends no
typed data.
> 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, code execution for tests and typechecks).
The runtime does not expose the context window size to the agent.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass (affected suites; full repo
runs are left to CI, as noted 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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
Co-authored-by: Paperclip <noreply@paperclip.ing>
|
||
|
|
f1516e8009 |
fix(native): stop a session.open model rejection instead of a doomed bootstrap retry (#19190)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native Runner runs start a provider session with the `session.open` command before any prompt > - A definite `session.open` rejection is treated as a pre-session failure and gets a bounded same-run bootstrap retry > - The runner also reports a closed classification. Two classifications say that the provider cannot run the selected model: `requested_model_unsupported` and `grok_model_mismatch` > - Paperclip ignored that classification. It scheduled a retry that must fail the same way. On a remote sandbox with a retained runner PID, the due retry entered restart recovery. Restart recovery has no checkpoint for a bootstrap retry, so it failed closed as `native_restart_recovery_blocked` / `ambiguous_remote_recovery_evidence` > - The task then stayed `blocked`. The recovery actions settled as no-replay holds, so no active action told anyone to change the model, and the parent task stalled too > - This pull request classifies those two definite rejections as the existing `native_provider_model_rejected` failure > - The benefit is a terminal failure on the first attempt, an accurate error, and an active board recovery action that names the next step: choose a supported model ## Linked Issues or Issue Description **What happened?** On a hosted staging instance, a Grok agent run failed at `session.open`. The runner log said `ACPX sidecar command session.open was rejected (retryable=false, classification=grok_model_mismatch)`. Paperclip recorded `recoveryMode: "bootstrap_retry"` and scheduled a same-run retry 30 seconds later. The run had a remote sandbox and a recorded sandbox PID, so the due retry went to restart recovery. Restart recovery found no checkpoint and no provider events and failed closed with `ambiguous_remote_recovery_evidence`. The run ended as `native_restart_recovery_blocked`. The task stayed `blocked` with no active recovery action for more than one hour. Its parent task stayed blocked on it. **What did you expect to happen?** The selected model cannot start, so a retry cannot succeed. The run should fail once with the model-rejection error. The task should show the existing "choose a model" action, owned by the board, until someone changes the model and retries. **Steps to reproduce** 1. Start a native run whose `session.open` command fails with `classification=grok_model_mismatch` (or `requested_model_unsupported`). 2. Read the `native_run_finalizations` row. Before this change it is `retryable_failure` with `recoveryMode: "bootstrap_retry"` and a `nextAttemptAt`. 3. On a remote sandbox run with a recorded PID, the due retry enters restart recovery and fails as `native_restart_recovery_blocked`. **Paperclip version or commit** `master` at `f5f5184f23`. Related: #18910 routes due remote retries with a retained PID to restart recovery. #18908 continues verified failed native preparation. Open PRs #19177, #19180, #19182 and #19184 change no-replay hold settlement and workspace hold release. They do not change this classification. This PR does not duplicate them. ## What Changed - `nativeSessionFailureSourceCode` (`server/src/services/native-runtime/native-session-executor.ts`) returns `native_provider_model_rejected` for a definite `session.open` failure or rejection whose runner classification is `requested_model_unsupported` or `grok_model_mismatch`. Other definite `session.open` rejections still return `provider_session_open_rejected` and keep the bounded startup retry. An indeterminate `session.open` outcome stays `native_session_interrupted`. - `native_provider_model_rejected` is already a permanent failure. The run stops on the first attempt with no `nextAttemptAt`, so it never enters the restart-recovery retry path. - When no provider session exists, the coordinator `nextAction` and the board recovery action `nextAction` say: choose a supported model for this agent, then retry the task. - The existing product paths for `native_provider_model_rejected` apply: the task presentation shows `choose_model`, and the recovery service treats it as `configuration_incomplete`. - `doc/execution-semantics.md` describes the rule under "Native provider model capacity". ## Verification Red before the fix (new test, unchanged production code): ```sh cd server && npx vitest run src/services/native-runtime/native-startup-retry.integration.test.ts # Tests 2 failed | 21 passed # failureCode: native_session_interrupted, phase: retryable_failure, recoveryMode: bootstrap_retry, # nextAction "Retry provider bootstrap on this same run ..." - the same record seen on staging ``` Green after the fix: ```sh cd server && npx vitest run src/services/native-runtime/native-startup-retry.integration.test.ts # Tests 23 passed cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts \ src/services/native-runtime/native-startup-retry.integration.test.ts src/services/task-interruption-presentation.test.ts # Test Files 3 passed, Tests 1186 passed pnpm --filter @paperclipai/server typecheck # exit 0 ``` The new integration cases use embedded PostgreSQL and the real executor failure path. For both classifications they check: - coordinator `terminal_failure`, `failureCode: native_provider_model_rejected`, `nextAttemptAt: null`, attempt 1 - run `failed` with `errorCode: native_provider_model_rejected` - task `blocked`, execution lock released - an `active` board recovery action with cause `native_provider_model_rejected`, return owner set to the agent, and the choose-model next action - `settleUnrecoverableExecutions` leaves the action `active`, so it does not become a silent no-replay hold - `claimNativeSessionResumptions` claims nothing, so there is no second executor The unit test also checks that `session_ensure_failed` keeps its bounded retry and that an indeterminate model rejection stays unclassified. A live Grok run with a mismatched model was not run. The failure record in the test matches the staging record field by field. ## Risks - Low risk. The change adds two closed runner classifications to an existing permanent failure code. Other `session.open` rejections keep their current retry. - A model mismatch that a retry could fix would now need an explicit retry. The runner already marks these rejections `retryable=false`. - This PR does not change restart recovery. A remote bootstrap retry with a retained sandbox PID for other `session.open` causes can still fail closed as `ambiguous_remote_recovery_evidence`. That needs a separate change with remote stop evidence. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used in Claude Code with tool use (shell, file edits, browser reads of the staging API). It did the investigation, code change, tests and PR text. ## 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> |
||
|
|
46a50a3bdd |
fix(recovery): release a recovered workspace hold for the new assignee after reassignment (#19184)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a board user reassigns a task in the middle of a run, Paperclip
stops the old run and saves its sandbox files (copy-back).
> - If copy-back fails at the stop, Paperclip records a
file-preservation hold for the stopped run. The hold blocks all
execution on the task until the files are recovered.
> - The new assignee's wakes were already queued. The final dispatch
gate saw the hold and cancelled them.
> - Automatic recovery then saved the files. But the hold stayed
effective for the new assignee: explicit continuation accepts only the
stopped run's own agent, so no message, Retry or stranded-work sweep
could start the new assignee.
> - The task stayed `in_progress` with no run, no wake and no active
recovery.
> - This pull request releases a recovered file-preservation hold for a
different current owner.
> - The benefit is that a reassigned task continues with its new owner
after its files are safe, with no manual action.
## Linked Issues or Issue Description
**What happened?**
A board user reassigned a running native task from agent A to agent B
and posted a comment. A's run ended `cancelled` / `issue_reassigned`.
Copy-back was still in progress, so B's first run was cancelled
`workspace_busy`. Paperclip then recorded a resolved no-replay hold
(`native_continuation_requires_reconciliation`, fingerprint
`native-workspace:<A run>`). The final dispatch gate cancelled B's two
queued runs with `execution_reconciliation_required` ("Recovering saved
work…"). Automatic recovery then saved the files
(`workspaceRecovery.status = recovered`, `workspaceRepairRequired:
false`). The blocker still said "Saved work recovered. Send a message to
continue." with `canContinue: false`. The task stayed `in_progress` with
no run for hours. Two hosted reproductions had the same shape. A sibling
reassignment without a copy-back failure recovered normally.
**Expected behavior**
After the saved-file receipt verifies, the new assignee continues. The
hold must still block every executor while the files are not recovered.
The stopped run's own agent keeps its no-replay hold.
**Steps to reproduce**
1. Start a native run on a task with a retained sandbox.
2. Reassign the task to another agent while the run executes, so that
copy-back records a `workspaceRestoreRecovery` hold.
3. Let automatic workspace recovery finish.
4. Before this change, `getExecutionBlocker` still returns the hold for
the new assignee, and the stranded-work sweep queues nothing.
**Paperclip version or commit**
`b9fbbf8d30` (also seen on `818d2bb6`).
**Deployment mode**
Hosted, PostgreSQL. The test uses embedded PostgreSQL.
Related open pull requests (checked; none covers this path): #19177
(board reconciliation of sibling holds), #19169 (disposition repairs
behind a cancelled blocker), #19143 (park a workspace-busy wait), #19149
(interrupt stop identity).
## What Changed
- New `restoreHoldReleasedForCurrentOwnerPredicate`
(`server/src/services/execution-blocker.ts`) is a SQL predicate. It
matches a file-preservation hold (`workspaceRestoreRecovery`, not
`restore_unsafe_archive`) when all of these are true:
- the stopped run is terminal and finished;
- the task has an agent assignee, and that agent is not the stopped
run's agent;
- the stopped run's agent is not the current review participant;
- `completedWorkspaceRecoveryCondition` is true for the hold: completed
system recovery, `workspaceRecovery.status = recovered`, succeeded
preservation operations bound to this hold, and the `workspace_finalize`
receipt;
- every saved operation is a copyback into the task workspace
(`workspace_recovery`). A snapshot-only save keeps the hold, because the
new owner's ordinary turn does not attach snapshots.
- The predicate is wrapped in `coalesce(..., false)`. Its negation
therefore cannot hide an unrelated hold through NULL JSON reads.
- The proof is the durable completed-recovery receipt. It does not read
live workspace authority (epoch, writer, snapshot), so reuse of the
workspace by the new owner cannot re-arm the hold.
- `getExecutionBlocker` excludes matching holds from both the
file-preservation query and the general blocker query. There is no scan
limit and no extra query per hold.
- `admitExplicitNativeContinuation` excludes the same holds from its
source checks. A released hold therefore cannot reject a new owner's
message with "owner changed".
- `completedWorkspaceRecoveryCondition` also accepts a SQL expression
for the run id. Its behavior is unchanged.
- The hold row, its `automaticRecovery.replay = "blocked"` evidence and
its status are not changed. If the task returns to the stopped run's
agent, the hold applies again.
- `doc/execution-semantics.md` describes the rule next to the existing
"Reassignment does not discard the stopped run's files" sentence.
## Verification
New test in `server/src/services/workspace-recovery.test.ts`: `hands a
reassigned task to its new assignee once the previous owner's files are
recovered`. It uses the real retained-workspace recovery, embedded
PostgreSQL and the real stranded-work sweep. It builds the incident
shape: a cancelled `issue_reassigned` source run, a resolved no-replay
hold, a new assignee, and that assignee's gate-cancelled run.
- Before recovery, the blocker holds the new owner
(`workspaceRepairRequired: true`).
- After recovery, the blocker is `null`. The sweep creates exactly one
pending turn for the new owner. That turn is not a retry of the stopped
run. A second sweep adds no turn.
- The hold row keeps `status: resolved`, `outcome: blocked` and `replay:
blocked`.
- Live repair evidence on the stopped run is then changed, so
`hasVerifiedWorkspaceRepair` returns false (as after a remote reuse).
The blocker stays `null`.
- After a reassignment back to the stopped run's agent, the hold applies
again.
- An unrelated reconciliation hold for the same stopped run, without
file evidence, still blocks the new owner.
- New test `keeps a snapshot-only recovery held for a new assignee`: the
snapshot-only save does not release the hold. This test fails against
the first revision of this pull request.
Red before the fix (test kept, `execution-blocker.ts` restored from
`master`):
```
cd server && npx vitest run src/services/workspace-recovery.test.ts -t "hands a reassigned task"
AssertionError: expected { …(13) } to be null (blocker: "Saved work recovered. Send a message to continue.", canContinue: false)
```
With only the blocker assertion removed, the sweep also failed:
`expected [] to have a length of 1 but got +0`.
Green after the fix: 1 passed.
Related suites, all pass (991 tests): `workspace-recovery`,
`execution-blocker-read`, `legacy-explicit-continuation-recovery`,
`workspace-restore-reconciliation`, `native-failed-workspace-recovery`,
`workspace-restore-repair-route`, `execution-projection-recovery`,
`heartbeat-undispatched-execution-wait`,
`issue-update-comment-wakeup-routes`,
`heartbeat-observed-workspace-preservation`,
`native-workspace-reconstruction`, `explicit-native-continuation`.
Not run: a live run on a deployed instance. The hosted incident tasks
are not recovered by this pull request until it is deployed.
## Risks
- A new owner can start after the files are recovered, while the stopped
run's action outcomes stay unknown. This matches the existing rule for
holds without a workspace: Paperclip does not create those holds for a
reassigned task. The new owner gets a fresh turn, not a replay.
- The hold still blocks every executor until the exact receipt verifies,
so there is no second executor on the workspace during copy-back.
- Other execution holds (non-workspace holds recorded before a
reassignment) keep their current effect. This change does not alter
them.
- A reassignment after a snapshot-only save still needs a board action,
as before. That path is not changed here.
- The release is one correlated `exists` in the existing blocker
queries. Its subqueries use primary keys and the run and issue indexes.
## Model Used
- Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, with tool use
and code execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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>
|
||
|
|
f7a4c5678f |
feat(runtime): pre-copy completed harness history before enrollment (#19137)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - An ephemeral web server needs provider history in shared object
storage.
> - Offline enrollment must preserve old backups before it can detach a
volume.
> - A large copied dataset still exceeded the maintenance deadline with
parallel transfers.
> - Most of its bytes belong to successful turns with released runtime
authority.
> - This pull request pre-copies that history while unrelated executions
continue.
> - The final offline migration still verifies every source and grants
enrollment authority.
## Linked Issues or Issue Description
Refs #19108, #19038, #18943.
**What happened?**
A copied instance has 1,953 local harness backups and 54.8 GB of
declared source bytes. A resumed parallel migration still reached its
25-minute deadline. It cannot safely enroll by moving all history during
maintenance.
**Expected behavior**
Copy eligible completed history while the server remains online. Defer
active, failed or changed sources. Keep all existing data and final
enrollment checks.
**Steps to reproduce**
1. Retain many local native harness backups on a volume-backed instance.
2. Attempt a full offline conversion within the maintenance deadline.
3. Observe that most time moves historical bytes rather than current
execution state.
## What Changed
- Add finite `native-harness-prestage.mjs` preparation from a normal
runtime build. Require explicit opt-in and shared object storage.
Process one source at a time with a 90-minute deadline.
- Use the recorded released runtime owner of each successful, finished
turn. Do not assert that all writers stopped.
- Recheck checkout, related queued/running work, successful result
ownership, session, Runner, exact lease stamp and unchanged backup at
database publication fences.
- Defer source changes, malformed source JSON and unsafe backup roots.
Log each such source and its reason. Keep database failures and
cancellation fatal. Keep completed objects valid for retry. Never return
an enrollment-ready receipt.
- Reuse the bounded migration source walker. Keep offline admission and
final verification unchanged.
- Keep private lifecycle state, task ownership and provider policies
unchanged. Document the operator path and its limits.
## Verification
- A zero-source scan now checks the existing database obligation query.
A known mismatch blocks with a source-missing receipt; empty
fresh/already-durable scans and nonempty deferred scans remain valid
advisory results. The prior command fails the mismatch regression. All
16 command and runtime-bundle tests pass for this follow-up.
- At head `7ba3dbc7aa`, 87 focused command, database-timeout, storage,
archive, publication and runtime-bundle tests passed with real
PostgreSQL and filesystem restoration.
- The current follow-up passes all 68 backup and archive tests. A source
renamed after preparation fails the new regression on the previous head,
then defers and permits the next backup with this fix. An archive output
ENOENT remains fatal. Existing database-error and cancellation tests
pass.
- New race tests fail against the previous storage owner: a changed
backup, queued turn or revoked lease otherwise publishes.
- Tests prove unrelated executions stay running, same-task work defers,
failed history stays deferred, retry verifies existing objects, and
private bytes restore after deleting the old home.
- Command review fixes require the configured existing vault key, retain
it for the process, set database lock/statement/idle bounds, close
clients on cancellation, and cap shutdown at ten seconds. Abort
immediately sets failure status and prints one blocked receipt. A real
child with real shortened timers reproduces the old missing-receipt exit
and verifies the fix. Tests also use a real PostgreSQL lock.
- Malformed manifest or runner JSON and symlink replacement fail the new
preparation and publication-fence regressions on the previous code. The
fixed command defers each damaged source and copies the next eligible
backup. Separate tests keep database SyntaxError and cancellation fatal.
Offline migration still rejects damaged sources.
- The bundle test runs the emitted preparation entry with plain Node
without starting the app.
- A prior copied-data experiment completed with 1,738 new publications,
nine verified current objects and 206 deferred sources. Later checks
found different database and filesystem capture times: 43 source runs
were absent from the older database snapshot, and one required
historical manifest was absent from the retained filesystem archive.
This experiment does not prove a consistent restore or enrollment
readiness.
- Full current-head CI at
|
||
|
|
dbbde744d2 |
fix(workspaces): preserve finalized recovery holds through offline import (#19180)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Offline workspace import must save files before local volume
removal.
> - A task recovery hold can remain after native file finalization
succeeds.
> - The old importer rejects that hold even when its file source is
exact and no controller owns the run.
> - Clearing the hold would discard the task's recovery obligation.
> - This pull request saves files and carries each exact recovery source
forward without resolving the hold.
> - Accepted-result finalizers keep their existing retry path.
## Linked Issues or Issue Description
Refs #19144, #18943, #19038. This PR builds on merged #19144.
**What happened?**
Offline import reports `workspace_recovery_unresolved` for terminal
native runs with finalized workspace references and active task recovery
holds. Some holds also name an exact historical file allocation.
**Expected behavior**
Save the workspace durably. Keep task warnings, recovery rows, results,
coordinator phases and task pointers unchanged. Retain exact future file
recovery and accepted-result finalization retries.
**Steps to reproduce**
1. Retain a local task workspace and an active recovery hold for a
finalized native run.
2. Stop all application writers through offline maintenance.
3. Attempt workspace import.
4. Observe that the old importer rejects the hold.
## What Changed
- Add an owner module that checks the exact terminal native run,
finalized reference, released ownership and task relationship.
- Require the normal finalizer's successful company/run/task receipt for
an accepted-result retry. Support historical receipts without a
workspace foreign key.
- Validate recorded file allocations with the existing recovery-source
owner.
- Journal and recheck hold evidence. Lock recovery, coordinator, run and
allocation rows before publication.
- Give finalized file sources a separate migration marker. Keep legacy
execution-replay suppression out of accepted-result retry paths.
- Use the owning parser for older finalized references that omit
optional fields.
- Redirect future file recovery to immutable snapshot preservation. Keep
provider-stop and source checks intact.
- Verify native copyback or snapshot receipts for each exact allocation.
Require proof for every source, including completed mixed repairs.
- Retain existing successful copyback receipts. Keep a historical
folder's obligation on its real source workspace.
- Retain the finalized migration marker through the execution blocker
projection. Discover finalized child-task sources from their exact
ancestor workspace reference.
- Document the receipt and lock contracts.
## Verification
- Fresh local run at head
|
||
|
|
8c062fa481 | fix(native): deliver plugin VM Runner usage to Core (#19162) | ||
|
|
53937be283 | fix(agents): select environment plugins only for agents on that environment (#19173) | ||
|
|
f5f5184f23 |
fix(workspaces): preserve retained branches and private commit sources (#19147)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Ephemeral web servers must preserve existing task files before
volume removal.
> - An old private checkout can be absent while its recorded branch
still exists.
> - A deleted branch can also retain its exact commit in the original
private object source.
> - The importer currently rejects these surviving sources.
> - This pull request saves the verified committed tree and records its
exact source proof.
> - It keeps task state, branch ownership and recovery obligations
unchanged.
## Linked Issues or Issue Description
Refs #19138, #19144, #18826.
**What happened?**
Existing Git workspace records can have missing checkout folders and
retained branch objects. The importer returns
`local_source_unsafe_or_missing`. A successful deletion receipt can also
have a valid private object source that the importer overlooks in favor
of a missing or changed project cache.
**Expected behavior**
Save the surviving committed tree from the exact recorded branch or
deletion receipt. Reject competing owners, live worktree registrations,
changed branches and invalid source identities. Do not use the current
default branch as a substitute.
**Steps to reproduce**
1. Retain an isolated workspace branch in its managed project cache.
2. Remove its clean checkout but keep the branch and original workspace
record.
3. Attempt workspace migration.
4. Alternatively, retain a deleted commit in its recorded private source
while its shared source is unavailable.
## What Changed
- Pin a retained branch in its verified company and project Git cache.
- Require the recorded private path, no competing workspace owner and no
surviving Git worktree registration or index authority.
- For a user-owned branch, require its exact successful unforced removal
receipt after archival. Preserve the branch ownership metadata.
- Prefer a verified recorded private object source for successful
deletion receipts when that source still exists.
- Recheck branch and receipt proof before publication. Preserve
compatibility with existing migration journals.
- Allow missing active records only through the existing offline writer
fence. Keep their status unchanged.
- Add byte-restoration and rejected-authority tests. Document the source
limits.
- Revalidate and honor interrupted journals' original Git source path.
Reject detached registrations and retained orphan index data. Reuse the
cleanup producer command formatter for paths containing spaces.
## Verification
- 172 focused migration, concurrency and isolated-source tests pass with
real PostgreSQL and Git repositories.
- The previous importer fails the retained-branch and offline-active
restoration cases. The previous source selection also fails the
private-cache and clean user-owned receipt cases.
- Tests remove the old home and restore the pinned commit and task file
bytes. New shared default-branch work is excluded.
- Tests reject competing owners, registered worktrees, missing branch
ownership, deleted branches, changed branch tips, forced removal and
revoked receipts.
- Read-only re-inventory of 79 actual failed source records makes 78
eligible with committed owner overlays. One older local test repository
remains blocked. This is source-overlay qualification, not full
released-image acceptance.
- A separate bounded archive replay completed six imports before its
25-minute deadline. That timing failure remains open; it does not
establish complete migration.
- Local server typecheck has no changed-file diagnostics. Stale linked
declarations prevent a clean local whole-server result. Full clean CI
remains required.
- Serving enrollment and empty-home recovery remain separate acceptance
gates.
- Latest rebased head
|
||
|
|
512fbea0bb |
fix(native): deliver messages saved while a native run finalizes (#19183)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user can reply on a task while the agent's run is still finishing > - A remote native run first commits its result and its status decision (for example `done`). Then its sandbox environment settles (harness backup, warm retention). > - A message that arrives after the final turn took its input is saved as a deferred receipt behind that run > - The saved-message recovery path holds the receipt while the source run still owns its environment lease or its workspace copy-back owner > - Nothing notified that path when the lease or the owner settled, so the messages waited for an unrelated company event > - This pull request notifies the existing deferred-receipt work when a run's environment lease settles or its workspace finalization owner is released > - The benefit is that messages sent while a run finalizes are delivered once, soon after the run settles, and a task closed in that window is reopened for them ## Linked Issues or Issue Description **What happened?** A user posted three plain comments on a task while the agent's remote native run was finalizing. The final turn had already called its finish tool. The comments were saved in one deferred receipt. The run then committed `done` and its status became `succeeded`. No follow-up run started. The task stayed `done` with three unanswered requests. About 20 minutes later, an unrelated event in the same company woke the deferred-receipt worker, and the messages were delivered. **Expected behavior** A message that arrives after the last turn took its input, but before the run releases, gets one follow-up turn as soon as the run settles. If finalization marked the task `done` in that window, the task is reopened for the message. Messages that the finished turn already consumed are not replayed. **Steps to reproduce** 1. Start a native run with a remote (sandbox) environment. 2. After the agent calls its finish tool, post a user comment on the task. 3. Let finalization commit `done` and project the run as `succeeded`. 4. Observe that the comment's deferred receipt stays `deferred_issue_execution` after the environment lease settles. **Root cause** A remote native run is finished by native finalization, which projects the terminal status. The in-process run completion (`releaseIssueExecutionAndPromote`) does not run for it. Its saved messages depend on the stranded-queue sweep in `resumeDeferredExecutionReceipts`, which calls the release with `resumeStrandedQueue`. That release correctly waits while `hasPendingRunCleanup` is true (an unsettled environment lease or a workspace finalization owner). Finalization notifies the deferred-receipt worker when it projects the run, but the cleanup is still pending at that time. The worker then drops the entry. Lease settlement and owner release did not notify the worker again. Refs #19122 (saves a redirect sent while a stopped run settles; this change reuses the same deferred-receipt delivery path for normally completing runs). ## What Changed - `environmentService.releaseLease` notifies deferred-execution work for the company when a run's lease leaves `active` or `pending_cleanup` (released, retained, or failed). - `environmentService.releaseLeasesForRun` notifies deferred-execution work in the same way. - `withNativeWorkspaceFinalizationOwnership` notifies deferred-execution work when it releases its owner. - Delivery uses the existing path: `resumeDeferredExecutionReceipts` → stranded-queue release → deferred wake promotion (with the existing reopen rule for user messages). No new delivery mechanism. - New regression in `deferred-execution-receipt-work.test.ts` with the real deferred-receipt worker, the real release adapter, and the real lease service. - `doc/execution-semantics.md`: one additive paragraph for messages sent while a run finalizes. ## Verification - Staging evidence (read-only): the task's saved queue held the three comments as one `deferred_issue_execution` receipt after the run succeeded. The run had no `run succeeded` lifecycle event (it was finished by native finalization, not run completion). The receipt was promoted about 20 minutes later with `reopenedFrom: done` by the same recovery path. - Red before the fix: the new test failed with `expected null to be truthy` (the receipt stayed deferred with no run after the lease settled). - Green after the fix: `cd server && npx vitest run src/__tests__/deferred-execution-receipt-work.test.ts -t "finishing native run"`. - Safety cases in the new test: - The receipt stays deferred while the environment lease is active (cleanup holds still apply). - After settlement, exactly one run carries both messages in order, and the done task is reopened once. - A second settlement signal does not deliver the messages again. - Related suites: `deferred-execution-receipt-work`, `environment-service`, `environment-cleanup-work`, `native-environment-settlement-work`, `idle-sleep-safety`. - Not done: a live staging reproduction at the fixed revision. Deployment is out of scope. ## Risks - The deferred-receipt worker reloads the company's deferred receipts after each run lease settlement. Each reload is bounded and runs only when deferred receipts exist. The notification is transactional, so a rolled-back release does not wake the worker. - Low risk: no schema change, no API change, and no change to admission or reopen rules. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use (shell, file edits, browser for read-only staging 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 - [ ] 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> |
||
|
|
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> |