4 Commits

Include renames

Author	SHA1	Message	Date
Devin FoleyandPaperclip	b5bf86ea09	Clear process identity when settled native runs stop (#17374) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each heartbeat run stores the process identity of its runner in `heartbeat_runs.process_pid`, `process_group_id` and `process_started_at` > - Only cancellations (`release_run_resources`), restart recovery and heartbeat recovery cleared these fields > - Failed native runs take the `schedule_retry` path, so they kept a PID after the run finished, for weeks > - Liveness checks for old runs then depend on PID-reuse defenses, and code that reads a set PID as "running" is misled > - This pull request releases the process identity of settled native runs when the process is confirmed gone, and backfills old sandbox rows > - The benefit is that a finished run no longer looks like it owns a live process, while live, warm and retrying processes keep their identity ## Linked Issues or Issue Description Refs #17358. Bug report described in-PR (bug report template). **What happened?** On one instance, 16 of 23 old native workspace sync folders belonged to failed or cancelled runs that finished more than a week earlier and still had `process_pid` set. The PID is written when the run's process starts (for remote targets it is the sandbox PID). The status arbiter clears it only through `release_run_resources`, which only cancellations emit. Failed native runs take `schedule_retry`, and other terminal paths (finalization reconciler, restart recovery terminal failures, watchdog timeouts) do not clear it either. **Expected behavior** When a run is terminal and its process is confirmed gone (or no longer owned), the process identity is cleared. A process that may still be alive keeps its identity. **Steps to reproduce** 1. Run a native task whose provider turn fails, so the arbiter decides `run_failed_partial_evidence_preserved` with a `schedule_retry` effect. 2. Wait for the run to finish and the runner to exit. 3. Read the run row: `status = 'failed'` and `process_pid` / `process_group_id` / `process_started_at` are still set. **Paperclip version or commit:** `master` at 75fcb0b3fc. **Deployment mode:** any. **Agent adapter(s) involved:** native runtime (`paperclip_runner`); local and sandbox targets. ## What Changed - `releaseSettledNativeRunProcess` (in `native-local-process-stop.ts`) clears `process_pid`, `process_group_id` and `process_started_at` of a native run under a row lock, only when all of these are true: - the status is `failed`, `cancelled`, `timed_out` or `interrupted`, and `finished_at` is set; - the finalization coordinator is absent, `committed` or `terminal_failure` (a `retryable_failure` same-run retry keeps the existing heartbeat recovery path; unfinished finalization keeps its identity); - the run has a session checkpoint or provider evidence, so native session resume never mistakes it for an unused bootstrap; - local target: every lease is released without failed cleanup, the latest `native.process_identity_recorded` event names this host and the same PID/group, and the PID and group probe absent. The release writes the existing `native.local_process_stopped` receipt in the same transaction, so `hasNativeLocalProcessStop`, `readNativeLocalProcessStop`, explicit continuation and acknowledged Stop keep their proof; - sandbox target: every lease has a remote termination receipt (sandbox PIDs are never probed on this host). - Succeeded runs keep their identity: a warm runner can serve the next turn, and chat retry matches warm predecessors by PID. - The heartbeat executor calls the release after lease and scratch cleanup at a terminal boundary (not when a same-run resume, workspace finalization or held ownership is pending). - Background work stays idle (see `rules/sleep-aware-background-work.mdc`): - Trigger: the executor settlement calls `settleNativeRunProcess`. If the release is not ready yet (process still alive, local lease not released, remote termination receipt missing), the run goes into an in-memory pending set. - Startup: the first `reapOrphanedRuns` pass after boot runs `discoverSettledNativeRunProcesses` once, in pages of 50, over the durable rows. This reconstructs work after a restart and covers terminal transitions outside an executor (reconciler, restart recovery, watchdog, timeouts). - Retry: later passes call `retryPendingSettledNativeRunProcesses` only for pending runs. It makes no database call when the set is empty. A run leaves the set when it is released, becomes ineligible, or reaches 20 attempts. - Runs that this process still executes are skipped. - Stop receipts now also record `processStartedAt`. `withReleasedNativeRunProcess` restores a released identity from the latest stop receipt. Retained session cleanup (`reconcileRetainedNativeSessionCleanup`) and warm transition restart checks use it. Quiescent runnerd state recovery reads the stop receipt when the prior run's identity was released. Without a receipt, all three still treat the identity as unproven. - Migration `0377_clear_settled_native_run_process_identity` backfills sandbox runs (without bumping `updated_at`) with the same rules: native, settled status, `finished_at` older than 1 day, settled coordinator phase, checkpoint or provider evidence, and at least one lease where every lease has a valid remote termination receipt. The snapshot is carried forward as `0377_snapshot.json`. - Not changed: legacy (non-native) runs. Case handoff release, scratch cleanup and conversation continuation read the terminal row's local PID/group as process-tree absence evidence, so clearing them needs separate work. `release_run_resources` cancellation behavior and `native_run_finalizations.controller_*` fields (they track the server controller, not the run process) are unchanged. ### Migration margin The 1-day margin is defense in depth. The real proof is the remote termination receipt: the provider confirmed that the sandbox stopped or was destroyed. One day is far longer than lease release, sandbox stop, restart recovery takeover and the finalization reconciler sweep, so no in-flight teardown or same-run recovery can still read these rows. Local rows are not backfilled in SQL: only the host that recorded the process can verify that it stopped and write the stop receipt. The runtime sweep releases them on that host. ## Verification - `pnpm exec vitest run src/__tests__/native-settled-process-release.test.ts` (new, 20 tests): release with a stop receipt and restore of the identity from it; `cancelled`/`timed_out`/`interrupted`; live process kept, then released after exit; kept for succeeded, `retryable_failure`, `workspace_finalizing`, unreleased local lease, another host, no evidence, unconfirmed sandbox, mixed leases; sandbox release with receipts; startup discovery paging and skip; pending retry; retry cap; no database access while nothing is pending; `reapOrphanedRuns` discovery, then retry after the process exits; executor release for a failed native run and no release for a succeeded run. - `native-provider-failure.integration.test.ts`: a real `schedule_retry` decision (`run_failed_partial_evidence_preserved`) leaves the failed run with its PID after finalization, and the release then clears it and the stop receipt reads back. - `native-session-executor.test.ts`: quiescent recovery and retained session cleanup accept a released identity with a stop receipt and reject one without a receipt. - `packages/db` `client.test.ts` migration rule: the backfill does not set `updated_at`. - `native-finalization-migration.test.ts`: migration 0377 clears only old settled native sandbox rows with confirmed termination (15 fixtures, including recent, succeeded, running, legacy, unsettled phases, no evidence, unconfirmed or local leases, no lease). - Related suites, all passing: `pnpm exec vitest run` on heartbeat-process-recovery, native-workspace-sync-retention(-config), native-finalization-{migration,settled-candidates,recovery}, chat-failed-run-retry-boundary, acknowledged-native-stop, explicit-native-continuation, conversation-continuation, native-session-resume, native-run-finalizer-telemetry, native-session-executor, native-safe-replacement, native-finalization-reconciler-timing, native-restart-recovery, native-finalization-recovery-state, run-lifecycle, agent-instruction/directory working copies, heartbeat-case-handoff (1920 tests across 23 files). - `pnpm run typecheck` in `server/`: passes. `pnpm run check:migrations` and the snapshot drift test in `packages/db`: pass. - Mutation checks (each reverted alone; a test failed every time): executor call, startup discovery, pending retry in `reapOrphanedRuns`, retry cap, pending-set cleanup on retry, start time in the stop receipt, retained cleanup receipt fallback, coordinator phase guard, succeeded exclusion, liveness probe, host check, remote receipt check, provider-evidence guard, local lease check, quiescent recovery receipt fallback, migration margin, migration NULL-safe receipt check, migration phase guard. The NULL-safe check came from a real bug the migration test found. The warm transition restart call site has no direct test: that path has no existing fixture. It uses the same tested helper. - Not run: the full server suite and e2e tests. ## Risks - Behavior change: more native runs now have a null `process_pid` after they settle. Every native reader was checked: explicit continuation, acknowledged Stop and agent stop use the stop receipt for null identities; cancelled-startup detection also requires no process events, so a released run is not mistaken for a cancelled startup; native session resume selection is protected by the evidence guard; chat retry reads its warm-predecessor PIDs from succeeded runs, which are not released. - A recycled local PID keeps a row unreleased (conservative), and sandbox runs from providers without termination receipts keep their identity. - Background cost: one paged discovery at startup. After that, only pending runs are checked, at most 20 times each. With nothing pending there is no recurring query. - A transition outside an executor that happens after startup, with no executor settlement, is released at the next startup discovery. - Migration: a data-only `UPDATE` on `heartbeat_runs` with a selective `WHERE`; it passes the migration safety check. It is safe to re-run. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-10-23 15:09:43 -07:00
3 people	d747752496	fix(runner): record why an OpenCode turn fails when opencode serve stops (#16863) ... When `opencode serve` stops during a turn, the OpenCode driver now fails the active turn itself with a closed-vocabulary cause (server exit code or known signal, allowlisted socket error code, HTTP status, or closed stream) instead of leaving runnerd to close it with `error: null`. The proxy flushes stdout before a fatal exit so the failed turn reaches runnerd, and the server records OpenCode failed turns as `nativeTurnFailure` (live and persisted-recovery paths) so the run-failure diagnostic names the cause. Provider text is never copied. Run status, retries and recovery behaviour do not change. Co-Authored-By: Paperclip Sentry triage <Paperclip-Paperclip@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-19 12:20:24 -07:00
3 people	275189d8d6	fix(native): report why an unclassified Codex turn failed (#16354) ... <!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs use the Paperclip runner to drive a provider session, for example the Codex app server > - When a Codex turn fails, two classifiers look for known failures: a model rejection and a server capacity failure > - When no classifier matches, the run fails with the generic message `Native session failed` and the error code `adapter_failed` > - The run failure report to Sentry then has no data about the cause, because the server drops the Codex `codexErrorInfo` value > - This pull request records a closed-vocabulary diagnostic for that failed turn and sends it to the Sentry `provider_failure` context > - The benefit is that operators can see which Codex failure kind causes these generic failures, and then write a targeted fix ## Linked Issues or Issue Description **What happened?** Hosted staging reports many run failures with the title `Error: Native session failed`. The events have `agent_adapter=paperclip_runner`, `error_code=adapter_failed`, `driverKind=codex_app_server`, and `exitCode=1`. The events do not show why the Codex turn failed. **Expected behavior** A run failure report for a failed Codex turn shows the failure kind, so that an operator can find the cause. The report must not contain provider response text, prompts, or model names. **Steps to reproduce** 1. Start a native Codex run. 2. Make the Codex app server end the turn with `status: "failed"` and an error that is not a model rejection or `serverOverloaded`. For example, use `codexErrorInfo: "contextWindowExceeded"`. 3. Look at the Sentry event for the run failure. Before this change, the `provider_failure` context is empty. **Agent adapter(s) involved** Codex (native runner, `codex_app_server` driver). ## What Changed - `native-provider-failure.ts`: The provider failure observation now records a diagnostic for each failed Codex turn. The diagnostic has only closed values: `provider: "codex"`, `category: "turn_failed"`, a known `CodexErrorInfo` kind (or `unrecognized` / `absent`), an HTTP status from 100 to 599 for the HTTP variants, and a runner failure code from an explicit allowlist of the codes the runner itself writes on a failed Codex turn (any other string becomes `unrecognized`, because a provider error object can carry its own `code`). It never copies the provider message. - New `unclassifiedTurnFailure()` gives this diagnostic only for a failed terminal turn that `forTerminal()` does not classify. `forTerminal()`, error codes, error messages, retries, and finalizer decisions do not change. - New `readNativeTurnFailureDiagnostic()` rebuilds a persisted value from the closed vocabulary and rejects all other values. - `native-session-executor.ts`: The adapter result keeps the generic message. It adds `resultJson.nativeTurnFailure` when the turn failure is unclassified. - `run-failure-diagnostics.ts`: The run failure report copies the validated diagnostic into the `provider` context. Sentry sends this as `provider_failure`. - Tests: unit tests for the vocabulary, HTTP status bounds, unknown kinds, runner codes, and privacy; a diagnostics test; and an update to the end-to-end executor test for the `unknown-error` scenario. ## Verification - `pnpm exec vitest run src/services/native-runtime/native-provider-failure src/services/__tests__/run-failure-diagnostics.test.ts src/services/__tests__/run-failure-report*.test.ts src/__tests__/sentry*.test.ts` (in `server/`): 6 files, 432 passed, 5 skipped. - `pnpm exec vitest run src/services/native-runtime/native-session-executor.test.ts` (in `server/`): 698 passed. - I could not run the full `tsc --noEmit` in my sandbox: the memory limit stopped the process, and the runner vendor build needs `cargo`. CI runs the full typecheck. - To check by hand: make a Codex turn fail with an unclassified error. Then make sure that the Sentry event `provider_failure` context shows `category: turn_failed` and the `codexErrorInfo` kind, and no provider text. ## Risks - Low risk. The change adds diagnostic data only. Run status, error code, error message, retry, and finalizer logic do not change. - Privacy: the diagnostic uses an allowlist of Codex enum names, a bounded integer, and an allowlist of runner-issued failure codes. Tests make sure that provider text does not get through. - If Codex adds a new `CodexErrorInfo` kind, the diagnostic shows `unrecognized` until the allowlist gets the new kind. - This change does not fix the root cause. It gives the data that is necessary to find the cause. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Related: #15347 (capacity classifier), #15304 (model rejection diagnostics). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-14 20:14:10 -07:00
Devin FoleyandPaperclip	1d5a439e2b	Diagnose native model rejections and preserve repairable reviews (#15304) ... ## Thinking Path > - Paperclip manages agent execution and recovery. > - A native provider can return a failed turn with an accepted result. > - This path replaced a structured model rejection with a generic failure. > - Finalization could also queue the same failed request for recovery. > - This change retains a bounded model and authentication diagnosis. > - Workers wait for a configuration fix; reviews retain their assignment. ## Linked Issues or Issue Description **Describe the bug** A native Codex turn can report HTTP 400 because the selected model is not supported by its ChatGPT connection. When the turn also returns a failed semantic result, the run shows `Native session failed` and `adapter_failed`. The native failed-result finalizer can create a retry intent before the adapter error is saved. **To Reproduce** Use a native Codex session that returns a structured `invalid_request_error` model/ChatGPT rejection and an accepted failed result. The regression tests reproduce this with a stub provider and a real isolated PostgreSQL database. No provider account or live request is needed. **Expected behavior** Show an actionable error and stop automatic retries of the rejected request. Block the current worker for a configuration fix. Preserve a pending review so its reviewer can explicitly retry after repair and resolve the original assignment. A stale run must not block a new task owner or override a completed review. **Version and deployment mode** The affected source path is present at `858094ba81`. This change applies to native Codex execution in local and remote environments. Related work: #13853 covers CLI protocol compatibility, #14942 refreshes runtime/model pins, and #12767 proposes legacy adapter model validation. This change concerns accepted native failed results and does not replace those changes. ## What Changed - Recognize only a bounded structured HTTP 400 provider error for the exact selected model and failed turn. - Save a fixed actionable error and four fixed diagnostic fields. Exclude raw provider text and model names from the new diagnostic. - Re-derive finalization evidence from the pinned execution and committed runner event, including its canonical hash and identity bindings. - Block only the current worker. For a current pending reviewer, preserve the original review decision and status version, release execution, and create no automatic recovery action. Recheck execution ownership under the issue lock. Preserve superseded and completed reviews. - Keep unknown failures, accepted results, cleanup, and ownership fences intact. Advance the status policy version to `phase6-v8`. - Route this exact failure code through the existing configuration recovery path. Permit an authenticated Board retry of this native failure only for its saved, still-pending review; discard caller review markers and revalidate at dispatch. ## Verification - Node 24: `pnpm -r typecheck` and `pnpm build` passed. - Independent focused validation: 222 tests passed across the new observer and PostgreSQL integration suites, status arbiter, diagnostic redaction, recovery classification, wake policy, and wake use cases. - The integration tests use actual result persistence, finalization, and the wake dispatcher. They cover zero retry dispatch for an authorized rejection, repeated reconciliation, pending/superseded/resolved reviews, reassignment, and a same-agent successor at an unchanged status version. - Final-commit verification: full typecheck and build passed; independent route/integration/arbiter/review validation passed 136 tests with no remaining source findings. This includes authenticated retry refusals, actual heartbeat retry admission, atomic queued-review claim, and acceptance of the original review. - The local aggregate test run was stopped to avoid duplicating the canonical Linux CI aggregate on this development host. Its partial log showed no failures; no full local-suite pass is claimed. The full hosted Linux CI aggregate passed on the final commit. - No live provider calls, runtime changes, or schema changes were made. ## Risks The classifier is intentionally narrow. A changed provider response format remains an unknown failure. No global model restriction is introduced. The worker blocker requires current task authority and does not replace newer assignments or completed review decisions. A pending review remains in review and requires an explicit retry after configuration repair; it is not rebound to a new status decision. The retry decision is durable. There remains a small existing gap between status/effect commit and diagnostic metadata persistence; a crash there can leave a generic diagnostic while preserving the blocked decision. No schema migration is required. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The exact serving model suffix and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #123` / `Refs #123` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub references) - [x] My branch name describes the change and contains no internal ticket id or instance-derived details - [x] I have run focused tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-10-05 20:57:33 -07:00