diff --git a/DESIGN.md b/DESIGN.md index 102b0dd58e..56c43fb5e6 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -40,7 +40,7 @@ Existing tiers already in index.css (~80+ tokens) — extraction maps to these o Do not show a toast for task or run state already visible on the current screen. This includes descendant runs represented by the open subtree. Show local action results in place; keep failures actionable inline. Notifications for other work -remain useful. Expected cancellation is neutral gray, not an error. A paused task replaces the composer with an amber takeover. It says “Task is +remain useful. Expected cancellation is neutral gray, not an error. The composer's Stop action stops the current response and leaves the composer available for a new message. Pause work is a separate explicit task or subtree action. A paused task replaces the composer with an amber takeover. It says “Task is paused.” and “Resume this task to send a message.” with a “Resume task” action. Subtrees use “Subtree is paused.” and “Resume subtree.” The takeover cannot be dismissed, retains drafts, and hides message inputs until the pause is released. diff --git a/Dockerfile b/Dockerfile index c5cf4e19a8..9bfee7ce1c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -316,7 +316,7 @@ COPY packages ./packages COPY server/package.json ./server/package.json COPY ui/package.json ./ui/package.json COPY cli/package.json ./cli/package.json -ARG PAPERCLIP_RUNNER_LOCK_SHA256=6e107822490361b4e46084c98b0c8ac11434bd3388186799d4cf3fe795481e07 +ARG PAPERCLIP_RUNNER_LOCK_SHA256=4bcad707a40cd223497e5a53f84fba4cb890324e14fe46c3b37e1c1daae4c040 RUN printf '%s pnpm-lock.yaml\n' "${PAPERCLIP_RUNNER_LOCK_SHA256}" > /tmp/provider-lock.sha256 \ && sha256sum -c /tmp/provider-lock.sha256 \ && pnpm install --frozen-lockfile --filter '@paperclipai/paperclip-runner...' diff --git a/cli/src/__tests__/doctor.test.ts b/cli/src/__tests__/doctor.test.ts index 2e1d7d8507..9f6a31989a 100644 --- a/cli/src/__tests__/doctor.test.ts +++ b/cli/src/__tests__/doctor.test.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import net from "node:net"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -8,7 +9,18 @@ import type { PaperclipConfig } from "../config/schema.js"; const ORIGINAL_ENV = { ...process.env }; -function createTempConfig(): string { +async function availablePort(): Promise { + const server = net.createServer(); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address() as net.AddressInfo; + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + return address.port; +} + +function createTempConfig(serverPort: number): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-doctor-")); const configPath = path.join(root, ".paperclip", "config.json"); const runtimeRoot = path.join(root, "runtime"); @@ -38,7 +50,7 @@ function createTempConfig(): string { deploymentMode: "local_trusted", exposure: "private", host: "127.0.0.1", - port: 3199, + port: serverPort, allowedHostnames: [], serveUi: true, }, @@ -87,7 +99,7 @@ describe("doctor", () => { }); it("re-runs repairable checks so repaired failures do not remain blocking", async () => { - const configPath = createTempConfig(); + const configPath = createTempConfig(await availablePort()); const summary = await doctor({ config: configPath, diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index bcac24decd..48a7410f72 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -103,8 +103,13 @@ grant's credentials. Inherited credential variables are cleared. Conflicting project authentication and provider-routing overrides are rejected. Managed failure cannot reactivate host or legacy credentials. -Subscription invocations take a grant-scoped, transaction-held database advisory -lease so the lock remains on one backend through transaction-pooling proxies. Two +Subscription invocations take a grant-scoped transaction advisory lease. The +reserved database client keeps one transaction open until cleanup, including on +transaction-pooling proxies such as PgBouncer. Session-level advisory locks must +not be used here: a pooled connection can return to a different backend for +cleanup and leave the original lock behind. The lease transaction disables its +idle timeout and contains no application data writes; cleanup rolls it back. +Two different users' grants can run concurrently; a second invocation of the same subscription receives a retryable busy response while it is in use. Refreshes are merged only into the originating active grant, with reconnect/revocation diff --git a/doc/connections/CONNECTOR-PLAYBOOK.md b/doc/connections/CONNECTOR-PLAYBOOK.md index 5124927b72..ca77a2600b 100644 --- a/doc/connections/CONNECTOR-PLAYBOOK.md +++ b/doc/connections/CONNECTOR-PLAYBOOK.md @@ -146,6 +146,14 @@ These axes produce combinations such as: transport is a REST API. Most current API-key catalog entries authenticate a remote MCP server. +Anthropic accounts use the `runtime_auth` AI connection methods. Its obsolete +`api-key` REST tool method is no longer offered. Existing unsupported REST tool +connections fail health and catalog checks with HTTP 422 and +`tool_connection_transport_unsupported`; they never use local stdio templates +or report a successful MCP probe. Add the provider through its supported account +flow, then remove the obsolete connection. This does not transfer credentials +or grants automatically. + For `mcp_remote`, header credentials and secret-bearing generated URLs have the complete generic runtime path. The schema also names `query`, `body_json`, and `env` key placements for specialized transports, but accepting a value in the diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index e1f9bc3744..677a14969a 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -154,6 +154,11 @@ New comments received during an execution hold retain their individual deferred The conversation groups repeated empty pre-start reconciliation cancellations into a neutral waiting notice. Started runs, actual startup failures, and run history remain inspectable. No historical run records are deleted. +Workspace contention (`workspace_busy`) displays **Waiting for workspace** and +continues automatically when the workspace is available. Internal scheduling +attempts remain in the run log without conversation cancellation markers, +cancellation toasts, or manual Retry controls. Users can keep sending instructions. + The legacy remote ACP process-session relay runs on the control-plane host. Its launch command uses the host's absolute Node executable even when the adapter's launch environment is sanitized for a remote sandbox; the sandbox PATH remains @@ -405,6 +410,8 @@ The handshake failure code is distinct from a session-identity mismatch. A timeo An explicit recovery action is a typed liveness repair path for a source issue. It is the recovery primitive; the action can be rendered directly on the source issue or backed by a separate recovery issue when the repair needs its own work item. +The task thread exposes the existing guarded Retry action for failed or timed-out legacy conversation runs. Where the server supports an explicit new attempt after a stopped legacy conversation, the thread must not hide that action solely because the old run still has a recovery-needed projection. Native and process recovery holds, pending decisions, active execution, and other retry gates remain in force. When a gate hides Retry, the thread says the message is preserved instead of promising an unavailable action. This presentation change does not rewrite historical outcomes or certify prior actions. + A valid recovery action must name: - the source issue and company @@ -827,10 +834,37 @@ apply. Stream closure without a turn terminal is not proof of success. Event replay uses the existing source receipts and never repeats provider work merely to recover recorded output. +Routine task completion and human-input requests must work under Conservative +runner permissions. The isolated Claude runtime grants only the narrow task +tools on the runner-owned bridge; it does not change general tool permissions. +Questions must be created as durable interactions before the agent claims to be +waiting. A direct Board comment reopening completed work has the same passive +response-wait semantics as a comment on an open task, subject to the same source, +identity, and governance checks. An automatic continuation is not a user reply. + +Provider-turn identity separates recovery responses from earlier assistant +output. A recovery turn cannot overwrite a delivered answer. File attachments +and work products refresh in the visible conversation when delivered. Composer +delivery uncertainty is reconciled by the exact durable client request ID; +another comment cannot settle it, and newer draft text must be preserved. + +The composer **Stop** action cancels the current response and verifies termination; +it does not create a pause hold. An acknowledged intentional cancellation remains +neutral even if teardown releases the run lease or returns no semantic result. +**Pause work** separately controls future execution. A crash preventing progress +is **Blocked**; **In Review** requires a concrete human decision. + ### Provider continuity and bounded finalization A permanently unusable native runner session may be replaced only with evidence that its predecessor is stopped and fenced, completed results and workspace state are preserved, required task history is available, and pending effects have been reconciled. A provider-native shell command or external write without a reliable outcome receipt is unknown. Unknown effects, integrity failures, and unverified process ownership never authorize speculative replay. Once automatic recovery is ruled out, Paperclip selects a conservative default: preserve recorded work, stop the affected task, and retain a durable no-replay hold. Unknown action outcomes remain unknown. No reconciliation form or user diagnosis is required. +Local Codex crash replacement can use a complete interrupted-turn inventory, +authenticated process-stop evidence, and unchanged retained-state fingerprints. +Only text and an exactly receipted task-completion call qualify for this path; +unknown operations or partial transcripts do not. Replacement uses a fresh +session and retires only the exact predecessor's obsolete recovery hold while +recording the proof and successor lineage. Retained provider files are not edited. + Bootstrap retries, exact-checkpoint resumes, and fresh replacement sessions share three total provider attempts, including the original attempt. Linked run IDs, controller restarts, and duplicate wakes do not reset this budget. Automatic attempts retain the 30-second delay. Replacement scheduling and predecessor lineage commit together, with one successor per predecessor and admission through the normal task locks, authorization, pause, approval, and budget gates. Provider execution and control-plane finalization have different clocks. A healthy provider can think or execute a long tool without output. Once execution settles, recovery and finalization control steps have a 60-second deadline, checked on startup and every 15 seconds. With a healthy database and scheduler, an abandoned transition must be repaired or surfaced within 90 seconds. Terminal persistence must not wait on provider cleanup or publication; a late finalizer cannot change a reassigned or closed task or release another run's locks. Historical ambiguous runs are never automatically replayed after an upgrade. @@ -1082,3 +1116,45 @@ These actions do not grant permission to hire agents or change their settings. Each action during execution still checks the agent's authority and the responsible user's authority. A denied retry returns before dispatch; it does not create a new failed run or change the task's state. + +### Native controller restart ownership + +The controller persists a newly spawned runner's process identity before +waiting for provider startup. An abrupt controller exit during session opening +can then recover through the same exact process-identity checks as an active turn. + +Both graceful and hot restarts detach the old controller from native sessions. +If shutdown begins while a provider session is opening, its eventual publication +honors the pending detachment before dispatching a turn. Once detached, an old +execution finalizer cannot suspend or signal the durable runner: the next +controller must recover it through the authenticated ownership checks. This +preserves active work and queued messages without treating a server restart as +user cancellation. + +Before either shutdown path exits, idle warm sessions close through their +normal suspend-and-checkpoint path. Remote sessions therefore leave verified +backup authority for the next controller even though their last run is already +complete. Busy sessions use active-run adoption while they remain active; if a +turn finishes during shutdown, its release checkpoints the session before +returning instead of leaving a new idle owner behind. If checkpointing fails, +the retained state continues to block unverified reuse. + +### Warm sandbox continuity + +A warm sandbox's shared workspace binding persists independently of the +experimental isolated-workspaces UI. Ordinary workspace updates remain gated; +the runtime can bind only a validated shared workspace in the issue's company +and project. Follow-ups can therefore reuse the same sandbox and provider +session. A staged provider package is reused only after the complete expected +manifest and artifact hashes verify. A missing, changed, or incompatible package +must be replaced and verified before launch. + +Safe native replacement may clear a Blocked status only with a durable receipt +that the same failed run projected that exact status version. Explicitly +reasserting Blocked or changing its blockers advances the status version, even +when the displayed status is unchanged. Adding a queued comment does not change +that authority. A later block also suppresses replacement at scheduled, queued, +and final dispatch gates. Queued and final native replacement dispatch also +re-read dependency readiness, since new dependencies need not change the +displayed task status. Old blocked rows without a receipt remain held; no +historical status backfill is performed. diff --git a/doc/plans/2026-09-12-fresh-runner-user-journeys.md b/doc/plans/2026-09-12-fresh-runner-user-journeys.md new file mode 100644 index 0000000000..e92b21664b --- /dev/null +++ b/doc/plans/2026-09-12-fresh-runner-user-journeys.md @@ -0,0 +1,50 @@ +# Fresh Runner first-time-user acceptance plan + +Date: 2026-09-12. Baseline: c9021c6721f91e2c74bd9fee9d3fd41c999d17b7 (fresh worktree). + +User: a first-time Paperclip operator who wants useful work from an agent without learning runner internals or manually managing task status. + +Environment: isolated test-drive instance, disposable company and tasks, real API-backed Codex and Claude Code providers, local and Daytona execution. Onboarding/bootstrap fixtures are setup, not an onboarding acceptance result. Task submission and user follow-ups are performed through the production browser interface. No special completion-tool instructions are added to user prompts. + +## Stories and acceptance + +1. Get a concise useful response. Create an ordinary task asking for three practical onboarding tips. Expect a visible answer, a settled Done status, and no generic completion approval. +2. Refine finished work. Return to the completed task and request a shorter result. Expect one continuation, retained context, and automatic completion. +3. Answer a real clarification. Ask the agent to ask a preference before writing a short deliverable. Expect an understandable question, answer submission, one continuation, and the deliverable without status bookkeeping. +4. Stop and change direction. Stop a long-running disposable task, then give a different short request. Expect old work to stop and the new message to proceed without losing it or requiring another message. +5. Pause during startup. Pause a newly launched task, resume, and continue. Expect no execution behind the pause and no permanent startup hold. +6. Survive an interruption. Interrupt only the isolated test runner/controller during harmless ongoing work. Expect truthful recovery feedback and either automatic continuation or one actionable retry that preserves the conversation. +7. Use and revisit output. Ask for a small file, inspect its exposed artifact/file link, refresh and revisit. Expect usable persistent output and accurate status. + +Run basic completion and follow-up on all four provider/environment combinations. Exercise clarification, stop/resume, startup cancellation, and interruption where setup permits, explicitly documenting gaps and how long each stuck state was observed. Do not call a blocked environment a successful run. + +## Evidence and report + +Keep the original prompt, browser actions, task/run identifiers, elapsed times, UI screenshots, and supporting read-only API evidence. Report observed surprises separately from diagnostic hypotheses. Each finding includes reproduction, expected/actual behavior, impact, screenshot, and a proposed general product rule for discussion. The initial phase excluded product fixes; the approved implementation phase below supersedes that limit. + +Related PRs reviewed: #13314 (explicit completion reviews), #13316 (startup cancellation fence), #13261 (healthy native session lifetime), #13254 (remote stop continuation), #13239 (new messages after native stop), #13163 (remote restart recovery). Existing automated runner cases contain explicit tool/completion instructions; these user stories deliberately use ordinary language. + +## Product-rule decisions + +All eight rules were accepted by the user. Product implementation and a fresh live report are authorized, in this same fresh worktree. + +1. **Crash recovery.** Recover automatically after verifying the old execution has stopped. Preserve completed work and queued messages. If an outcome remains uncertain, show a clear blocker and an actionable recovery path. Do not blindly replay actions whose outcomes are uncertain. +2. **Completion permissions.** Task-scoped delivery and completion work in every permission profile, without granting unrelated command, filesystem, or external-action authority. Explicit human reviews remain required. +3. **Delivered answers.** Preserve delivered answers. Recovery adds a distinct update or correction. Streaming drafts may change while unfinished. +4. **Task status.** A crash that prevents progress is Blocked. In Review requires a specific human decision, such as a real review or clarification. +5. **Composer.** Reconcile submission identity with durable server receipts automatically after navigation/reload. Only genuinely unresolved delivery needs user attention. +6. **Stop.** Stop the current response and keep the composer usable for a new direction. Pause future work is a separate explicit action. Ordinary cancellation has neutral feedback. +7. **Startup identity.** Bind provider identity before accepting events. Retry safe startup failures internally within a bounded budget; preserve integrity checks. +8. **Workspace contention.** Present routine scheduling contention as Waiting for workspace, with useful context, rather than cancellation/failure. + +## Implementation and verification + +Fix shared causes in submission persistence, execution lifecycle/projection, provider permission plumbing, event identity, and transcript identity. Add regression tests at the relevant boundaries, including negative authority/idempotency cases. Run targeted checks first, then the repository typecheck/test/build checks appropriate to the broad change. Rebuild and rerun the original natural-language journeys in the isolated app with local Codex and Claude Code and compatible Daytona environments. Retain before/after evidence, report remaining limits explicitly, and clean up disposable remote resources. + +## Retest outcome + +The implementation was exercised through the ordinary browser task flow with local Codex, local Claude Code, Codex in Daytona, and Claude Code in Daytona. Each completed a plain-language request and follow-up. The interrupted Codex task recovered from a deliberately killed local Codex runner without a Retry click, new message, or controller restart. A separate task proved Stop followed by a new completed request. A fresh clarification task proved real questions and one-answer continuation, including after reopening Done. The open task refreshed a newly delivered file automatically, and shared Daytona workspace contention displayed neutral waiting. + +Additional fixes cover live artifact query invalidation, passive waiting on a direct user comment that reopens completed work, and draft text entered during an in-flight submission. The database restart path now verifies the owned PostgreSQL port and data directory before migration. This was prompted by an isolation incident: the QA server briefly connected to another development database and applied migrations 0273/0274 before the wrong company was noticed. No test task was created there; no rollback of another developer's work was attempted. + +Historical ambiguous executions were retained, not force-replayed or manually marked successful. Their old task statuses were not backfilled. Automatic crash replacement was proven only for the narrowly verified local Codex case; remote crash recovery and Claude controller-restart recovery are not claimed. Simple Claude Daytona responses remained slow in this sample. The local acceptance report and selected evidence were delivered through artifact work products, with explicit limits and before/after observations. Disposable Daytona sandbox cleanup was completed. diff --git a/doc/run-log-events.md b/doc/run-log-events.md index d020a8ae33..9be43ceac2 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -68,6 +68,19 @@ server-authored event among these three types; provider source events cannot supply stop authority. These records stay in the local run log and do not add Telemetry or OpenTelemetry data. +## Verified Local Codex Replacement Evidence + +The server writes `native.stopped_text_turn_verified` in the same transaction +that schedules a fresh successor for a stopped local Codex run. It records the +runner and provider process identities, retained-state digests, provider thread +and turn IDs, and IDs of exactly receipted task-completion calls. The server first +checks the complete turn inventory, process-stop receipt, and execution binding. +Unknown actions or changed retained state prevent this event and replacement. + +The record documents why the old execution can be retired. It does not make the +old session resumable, rewrite provider files, or authorize replay on its own. +It remains in the local run log and adds no Telemetry or OpenTelemetry export. + ## Sandbox Startup Run-Log Event Paperclip writes one `run.startup.step` event to the run log for each bring-up diff --git a/docker/daytona-runner/Dockerfile b/docker/daytona-runner/Dockerfile index 4921b79a18..4f6d355a7c 100644 --- a/docker/daytona-runner/Dockerfile +++ b/docker/daytona-runner/Dockerfile @@ -29,7 +29,7 @@ COPY cli/package.json ./cli/package.json # The complete resolved lock (including transitive integrity hashes) is reviewed. # Reject registry-time drift BEFORE installing packages or running lifecycle code. # Refresh this digest together with source/provider dependency changes. -ARG PAPERCLIP_RUNNER_LOCK_SHA256=6e107822490361b4e46084c98b0c8ac11434bd3388186799d4cf3fe795481e07 +ARG PAPERCLIP_RUNNER_LOCK_SHA256=4bcad707a40cd223497e5a53f84fba4cb890324e14fe46c3b37e1c1daae4c040 RUN printf '%s pnpm-lock.yaml\n' "${PAPERCLIP_RUNNER_LOCK_SHA256}" > /tmp/provider-lock.sha256 \ && sha256sum -c /tmp/provider-lock.sha256 \ && pnpm install --frozen-lockfile --filter '@paperclipai/paperclip-runner...' diff --git a/docker/daytona-runner/provider-dependencies.lock.yaml b/docker/daytona-runner/provider-dependencies.lock.yaml index 0fe0025106..ae216df9d2 100644 --- a/docker/daytona-runner/provider-dependencies.lock.yaml +++ b/docker/daytona-runner/provider-dependencies.lock.yaml @@ -15,7 +15,7 @@ overrides: patchedDependencies: '@agentclientprotocol/claude-agent-acp@0.73.0': - hash: axsvv3fhdlmmbmnhpi2cywic6q + hash: riqgguivxjxydoydnzpedjy5au path: patches/@agentclientprotocol__claude-agent-acp@0.73.0.patch '@agentclientprotocol/codex-acp@1.6.2': hash: juyg45ifkipb4jskg7lpym2lj4 @@ -170,7 +170,7 @@ importers: dependencies: '@agentclientprotocol/claude-agent-acp': specifier: ^0.73.0 - version: 0.73.0(patch_hash=axsvv3fhdlmmbmnhpi2cywic6q)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3)) + version: 0.73.0(patch_hash=riqgguivxjxydoydnzpedjy5au)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3)) '@anthropic-ai/sdk': specifier: 0.121.0 version: 0.121.0(zod@4.4.3) @@ -495,7 +495,7 @@ importers: dependencies: '@agentclientprotocol/claude-agent-acp': specifier: 0.73.0 - version: 0.73.0(patch_hash=axsvv3fhdlmmbmnhpi2cywic6q)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3)) + version: 0.73.0(patch_hash=riqgguivxjxydoydnzpedjy5au)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3)) '@agentclientprotocol/codex-acp': specifier: 1.6.2 version: 1.6.2(patch_hash=juyg45ifkipb4jskg7lpym2lj4) @@ -951,6 +951,9 @@ importers: '@vercel/connect': specifier: 0.6.1 version: 0.6.1(@chat-adapter/slack@4.39.0(patch_hash=226jj24akljccsfbw7bcxujz4u)(zod@4.4.3))(better-auth@1.7.2(@opentelemetry/api@1.9.1)(drizzle-kit@0.31.10)(drizzle-orm@0.45.2(@opentelemetry/api@1.9.1)(kysely@0.29.5)(pg@8.18.0)(postgres@3.4.9))(pg@8.18.0)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)(vitest@4.1.11(@opentelemetry/api@1.9.1)(@types/node@24.13.3)(jsdom@30.0.1(@noble/hashes@2.4.0))(vite@8.2.2(@types/node@24.13.3)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.12)(yaml@2.9.0)))) + acorn: + specifier: 8.18.0 + version: 8.18.0 acpx: specifier: 0.13.1 version: 0.13.1(patch_hash=zmi2maauog4djnfd2acdrgmyhe) @@ -9247,7 +9250,7 @@ snapshots: '@adobe/css-tools@4.5.0': {} - '@agentclientprotocol/claude-agent-acp@0.73.0(patch_hash=axsvv3fhdlmmbmnhpi2cywic6q)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3))': + '@agentclientprotocol/claude-agent-acp@0.73.0(patch_hash=riqgguivxjxydoydnzpedjy5au)(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3))': dependencies: '@agentclientprotocol/sdk': 1.4.0(zod@4.4.3) '@anthropic-ai/claude-agent-sdk': 0.3.263(@anthropic-ai/sdk@0.121.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.30.0(zod@4.4.3))(zod@4.4.3) diff --git a/packages/paperclip-runner/docs/adding-a-harness.md b/packages/paperclip-runner/docs/adding-a-harness.md index 81fdf20c17..d4216bcf7f 100644 --- a/packages/paperclip-runner/docs/adding-a-harness.md +++ b/packages/paperclip-runner/docs/adding-a-harness.md @@ -11,7 +11,7 @@ identity, isolation behavior, and conformance coverage are defined. |---|---|---|---| | Codex | `codexPermissionMode` | `never`, `on-request`, `untrusted` | `never` | | OpenCode | `opencodePermissionMode` | `allow`, `ask`, `deny` | `allow` | -| ACPX (Claude, Codex) | `acpxPermissionMode` | `approve-all`, `approve-reads`, `deny-all` | `approve-all` | +| ACPX (Claude, Codex) | `acpxPermissionMode` | `approve-all`, `approve-reads`, `deny-all` | `approve-reads` | The browser-safe source of truth for labels, defaults, and configuration validation is `PAPERCLIP_RUNNER_PERMISSION_CAPABILITIES` in diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs index 42bc7fd9c7..6681fe40c8 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs @@ -3086,6 +3086,11 @@ fn classify_notification_thread( "Codex notification has malformed turn identity", )); } + // This connection-level notification carries no task authority. Codex can + // emit it while loading skills during the first turn. + if method == "skills/changed" && !contains_provider_work_binding(params) { + return Ok(NotificationThread::UnrelatedInformation); + } let thread = notification_thread_id(params); if thread.is_none() && turn_ids.is_empty() @@ -4627,6 +4632,28 @@ mod notification_identity_tests { NotificationThread::Descendant ); } + #[test] + fn skills_changed_is_connection_information_without_execution_authority() { + assert_eq!( + classify_notification_thread("skills/changed", "root", &BTreeSet::new(), &json!({})) + .unwrap(), + NotificationThread::UnrelatedInformation + ); + for params in [ + json!({"threadId": "other"}), + json!({"itemId": "unbound"}), + json!({"threadId": 7}), + ] { + assert!(classify_notification_thread( + "skills/changed", + "root", + &BTreeSet::new(), + ¶ms + ) + .is_err()); + } + } + #[test] fn rejects_missing_authority_and_malformed_turn_identities() { for method in [ diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_events.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_events.rs index c25345b833..317f4980ad 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_events.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_events.rs @@ -86,6 +86,16 @@ impl AcpxEventProjectionContext { self.provider_turn_id.as_deref().unwrap_or(&self.turn_id) } + fn assistant_item_id(&self) -> String { + // Recovery can submit multiple provider turns within one PRP run. Keep + // each delivered answer distinct while coalescing its streaming deltas. + acpx_message_item_id( + "", + &format!("{}:{}", self.item_id, self.active_provider_turn_id()), + "assistant", + ) + } + fn correlation(&self) -> Value { json!({ "runId": self.run_id, @@ -129,11 +139,10 @@ pub fn project_acpx_state_event( { payload.insert("providerItemId".to_owned(), Value::String(provider_item_id)); } - // PRP exposes one canonical assistant item for the turn. This - // lets streamed deltas and the completed provider response - // coalesce by identity while retaining the opaque ACP message - // identity as trace metadata above. - payload.insert("itemId".to_owned(), Value::String(context.item_id.clone())); + payload.insert( + "itemId".to_owned(), + Value::String(context.assistant_item_id()), + ); } Ok(vec![event]) } @@ -237,7 +246,7 @@ pub fn project_acpx_state_event( EventPriority::P1, json!({ "provider": "acpx", - "itemId": context.item_id, + "itemId": context.assistant_item_id(), "kind": "agentMessage", "status": "completed", "channel": "final", diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_event_projection.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_event_projection.rs index c0f8092e9c..9243932064 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_event_projection.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_event_projection.rs @@ -288,7 +288,10 @@ fn projects_assistant_terminal_and_diagnostic_events_fail_closed() { "text":"Done", }), })); - assert_eq!(streamed[0].payload["itemId"], "item-1"); + assert!(streamed[0].payload["itemId"] + .as_str() + .unwrap() + .starts_with("acpx-assistant-")); assert_eq!( streamed[0].payload["providerItemId"], "opaque-provider-message" @@ -299,7 +302,10 @@ fn projects_assistant_terminal_and_diagnostic_events_fail_closed() { text: "Done".to_owned(), }); assert_eq!(assistant[0].event_type, "item.completed"); - assert_eq!(assistant[0].payload["itemId"], "item-1"); + assert_eq!( + assistant[0].payload["itemId"], + streamed[0].payload["itemId"] + ); assert_eq!(assistant[0].payload["channel"], "final"); for (status, expected) in [ @@ -552,3 +558,24 @@ fn runtime_request_projection_preserves_durable_identity_boundaries() { let semantic_validator = jsonschema::validator_for(&semantic_schema).unwrap(); assert!(semantic_validator.is_valid(&semantic[0].payload["semantic_tool"])); } + +#[test] +fn recovery_preserves_the_preceding_provider_turn_answer() { + let first = project(AcpxProviderStateEvent::AssistantMessage { + turn_id: "turn-1".to_owned(), + text: "Useful answer".to_owned(), + }); + let mut recovery = context(); + recovery.provider_turn_id = Some("recovery-turn".to_owned()); + let second = project_acpx_state_event( + &recovery, + &AcpxProviderStateEvent::AssistantMessage { + turn_id: "recovery-turn".to_owned(), + text: "Recovery update".to_owned(), + }, + ) + .unwrap(); + assert_ne!(first[0].payload["itemId"], second[0].payload["itemId"]); + assert_eq!(first[0].payload["text"], "Useful answer"); + assert_eq!(second[0].payload["text"], "Recovery update"); +} diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/local_integrity_boundary_golden.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/local_integrity_boundary_golden.rs index dda0278e83..cc76c54e61 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/local_integrity_boundary_golden.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/local_integrity_boundary_golden.rs @@ -264,7 +264,12 @@ fn acpx_projection_matches_shared_plan_question_final_and_terminal_identity() { .expect("project ACPX final"), "item.completed", ); - assert_eq!(final_message.payload["itemId"], expected_final["itemId"]); + // ACPX provider-state output is identified by its provider turn, rather + // than reusing the presentation item ID supplied by the shared fixture. + assert_eq!( + final_message.payload["itemId"], + "acpx-assistant-a7736694c69c42e1b7d7220b4274f274c9dbba97f2e693ddbc584158015f0123" + ); assert_eq!(final_message.payload["channel"], "final"); assert_eq!(final_message.payload["text"], expected_final["text"]); diff --git a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts index 28a56a3967..b7fec37fa2 100644 --- a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.test.ts @@ -133,6 +133,7 @@ describe("Codex ACPX runtime adapter", () => { PATH: "/verified/bin", PAPERCLIP_ACPX_ISOLATED_CONTEXT: "1", ANTHROPIC_CUSTOM_MODEL_OPTION: providerModel, + PAPERCLIP_ACPX_TASK_TOOL_BRIDGE_URL: "", }); expect(runtime.ensureSession).toHaveBeenCalledWith( expect.objectContaining({ @@ -143,6 +144,33 @@ describe("Codex ACPX runtime adapter", () => { }, ); + it.each(["runner-owned", "unowned", "absent"])( + "pins Claude completion authority to the %s task bridge", + async (binding) => { + const options = openOptions(fakeCommand()); + options.profile = resolveQualifiedAcpxProfile("claude", "claude-sonnet-5"); + options.launchEnvironment = { PAPERCLIP_ACPX_TASK_TOOL_BRIDGE_URL: "http://untrusted.invalid/mcp" }; + options.mcpServers = binding === "absent" ? [] : [{ + name: "paperclip", url: "http://127.0.0.1:3210/mcp", + bearerToken: "bridge-secret", runnerOwned: binding === "runner-owned", + }]; + let runtimeOptions: AcpRuntimeOptions | undefined; + await openCodexAcpxRuntime(options, { + createRegistry: () => registry(), + createStore: () => store(), + createRuntime: (created) => { + runtimeOptions = created; + return fakeRuntime(); + }, + }); + expect(runtimeOptions?.spawnEnvironment?.()).toEqual({ + PAPERCLIP_ACPX_ISOLATED_CONTEXT: "1", + PAPERCLIP_ACPX_TASK_TOOL_BRIDGE_URL: binding === "runner-owned" ? "http://127.0.0.1:3210/mcp" : "", + ANTHROPIC_CUSTOM_MODEL_OPTION: options.profile.reportedModelId, + }); + }, + ); + it("launches only through the verified command lease", async () => { const runtime = fakeRuntime(); const command = fakeCommand(); diff --git a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts index 6e2eae9b71..f44cc832ce 100644 --- a/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts +++ b/packages/paperclip-runner/src/drivers/acpx/codex-runtime-adapter.ts @@ -325,6 +325,11 @@ export async function openQualifiedAcpxRuntime( // host-requested ID so model verification stays exact on resume // and before the first billable prompt. ANTHROPIC_CUSTOM_MODEL_OPTION: options.profile.reportedModelId, + // This URL comes from the runner-owned authenticated tool bridge, + // never provider-supplied permission-request metadata. + PAPERCLIP_ACPX_TASK_TOOL_BRIDGE_URL: options.mcpServers.find( + (server) => server.runnerOwned && server.name === "paperclip", + )?.url ?? "", } : {}), }), diff --git a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts index 8300fdbe50..65e227ebff 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.test.ts @@ -594,8 +594,9 @@ describe("ACPX installation integrity", () => { ); await (await installation.openCommand()).close(); }, - // The real macOS snapshot copies the installed SDK tree; this is an - // integrity check, not a five-second startup performance benchmark. + // This hashes the real installed SDK tree and competes with the complete + // package suite for filesystem I/O, especially on macOS. It is an integrity + // check, not a startup benchmark; small fixtures keep the default timeout. 60_000, ); diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts index 01d3fd2da9..e1ed1b2383 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts @@ -2041,6 +2041,50 @@ it.each([99, 100])( }, ); +it("publishes spawned runner ownership before waiting for provider startup", async () => { + const stateDirectory = await mkdtemp(join(tmpdir(), "runner-spawn-ownership-")); + let releaseOwnership!: () => void; + const persisted = new Promise((resolve) => { releaseOwnership = resolve; }); + const onSpawn = vi.fn(async () => persisted); + const activate = vi.fn(); + const originalStartedAt = "2026-09-01T10:00:00.000Z"; + const spawnRunner = durableControlPlane.spawnRunner; + const spawnSpy = vi.spyOn(durableControlPlane, "spawnRunner").mockImplementation((options) => ({ + ...spawnRunner(options), + // Remote launchers can supply a process timestamp distinct from the + // controller transport construction time. Both ownership APIs must agree. + startedAt: originalStartedAt, + })); + const bundle = createCapabilityRunnerdCodexTransport({ + runnerBinary: defaultCapabilityRunnerdBinary(), + codexCommand: fakeCodex, + codexArgs: fakeCodexArgs(stateDirectory), + stateDirectory, + onSpawn, + controlPlaneRegistration: async (authority) => { + await authority.start(); + return { activate, release: () => undefined }; + }, + }); + const opening = bundle.transport.request("thread/start", { cwd: tmpdir(), dynamicTools: codexSemanticToolSpecs() }); + try { + await vi.waitFor(() => expect(onSpawn).toHaveBeenCalledOnce()); + expect(onSpawn).toHaveBeenCalledWith({ pid: expect.any(Number), processGroupId: expect.any(Number), startedAt: originalStartedAt }); + expect(bundle.transport.processInfo?.().pid).toBeGreaterThan(0); + expect(bundle.transport.processInfo?.().startedAt).toBe(originalStartedAt); + expect(activate).not.toHaveBeenCalled(); + releaseOwnership(); + await opening; + expect(activate).toHaveBeenCalledOnce(); + } finally { + releaseOwnership(); + await opening.catch(() => undefined); + await bundle.transport.close(); + spawnSpy.mockRestore(); + await rm(stateDirectory, { recursive: true, force: true }); + } +}); + it("launches runnerd with its production durable outbox limits", () => { expect(runnerdLaunchProfileInternals.maxOutboxBytes).toBe(16 * 1024 * 1024); expect(runnerdLaunchProfileInternals.p0ReserveBytes).toBe(1024 * 1024); @@ -8326,6 +8370,8 @@ async function verifyLiveRunnerAdoption( mismatchedCheckpoint: boolean, mismatchedArtifact = false, goalMidTurn = false, + detachBeforeCleanup = false, + startWithoutCheckpoint = false, ) { const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-live-adopt-")); const server = createServer(); @@ -8435,7 +8481,7 @@ async function verifyLiveRunnerAdoption( { mode: 0o600 }, ); }; - await compactProviderIdentityEvents(); + if (!startWithoutCheckpoint) await compactProviderIdentityEvents(); const duplicateLauncher = vi.fn(() => { throw new Error("duplicate runner spawn attempted"); @@ -8453,7 +8499,7 @@ async function verifyLiveRunnerAdoption( } : {}), resumeDynamicTools: [], - resumeProviderSession: { + resumeProviderSession: startWithoutCheckpoint ? undefined : { driverSessionId: String(openedThread.id), providerSessionId: mismatchedCheckpoint ? "wrong-provider-session" @@ -8519,13 +8565,19 @@ async function verifyLiveRunnerAdoption( expect(() => process.kill(runnerPid!, 0)).not.toThrow(); return; } - await expect(adopted.transport.request("thread/read", {})).resolves.toEqual( + await expect(adopted.transport.request(startWithoutCheckpoint ? "thread/start" : "thread/read", {})).resolves.toEqual( expect.objectContaining({ thread: expect.objectContaining({ id: "codex-thread-1" }), }), ); expect(adopted.evidence().runnerPid).toBe(runnerPid); expect(adopted.transport.processInfo?.().startedAt).toBe("2026-09-01T10:00:00.000Z"); + if (startWithoutCheckpoint) { + const retained = JSON.parse(await readFile(controlPlaneStatePath, "utf8")); + for (const type of ["run.prepare", "session.open"]) { + expect(retained.commands.filter((command: { type: string }) => command.type === type)).toHaveLength(1); + } + } if (goalMidTurn) { const observed = await Promise.race([ (async () => { @@ -8542,12 +8594,24 @@ async function verifyLiveRunnerAdoption( expect(adopted.evidence().diagnostics).toContain( `adopted runner ${runnerPid} authenticated to its durable PRP authority`, ); - expect(adopted.evidence().diagnostics).toContain( - "restored adopted provider identity from the exact durable checkpoint after PRP event compaction; awaiting live confirmation", - ); - expect(adopted.evidence().diagnostics).toContain( - "confirmed adopted provider identity against authenticated recovery session.snapshot", - ); + if (!startWithoutCheckpoint) { + expect(adopted.evidence().diagnostics).toContain( + "restored adopted provider identity from the exact durable checkpoint after PRP event compaction; awaiting live confirmation", + ); + expect(adopted.evidence().diagnostics).toContain( + "confirmed adopted provider identity against authenticated recovery session.snapshot", + ); + } + if (detachBeforeCleanup) { + await adopted.detachControllerForRestart(); + await adopted.transport.close("old controller finalizer"); + expect(signal).not.toHaveBeenCalled(); + expect(() => process.kill(runnerPid!, 0)).not.toThrow(); + const retained = JSON.parse(await readFile(controlPlaneStatePath, "utf8")); + const commandTypes = retained.commands.map((command: { type: string }) => command.type); + expect(commandTypes).not.toContain("turn.stop"); + expect(commandTypes).not.toContain("runner.suspend"); + } } finally { await adopted?.transport.close().catch(() => undefined); if (runnerPid) { @@ -8587,6 +8651,10 @@ it( it("binds buffered mid-goal items only after the authenticated recovery snapshot", () => verifyLiveRunnerAdoption(false, false, true), 30_000); +it("keeps an adopted runner alive when the detached controller finalizer closes", () => verifyLiveRunnerAdoption(false, false, true, true), 30_000); + +it("adopts an opening session without a checkpoint instead of bootstrapping a duplicate provider", () => verifyLiveRunnerAdoption(false, false, false, false, true), 30_000); + it("surfaces a runner exit while provider-ingress readiness is still pending", async () => { const neverReady = new Promise(() => undefined); const bundle = createCapabilityRunnerdCodexTransport({ diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 208f2d106e..03860fdede 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -1141,6 +1141,9 @@ export interface CapabilityRunnerdCodexTransportOptions { turnStartTimeoutMs?: number; onDiagnostic?: (message: string) => void; onEvidence?: (evidence: Readonly) => void; + /** Persist process ownership immediately after spawn, before waiting for + * provider bootstrap or activating a deferred PRP registration. */ + onSpawn?: (meta: { pid: number; processGroupId: number | null; startedAt: string }) => Promise; stateDirectory?: string; lifecyclePolicy?: | { mode: "per_turn"; idleTimeoutMs: null } @@ -3329,6 +3332,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { #runAttachTemplate: Record | null = null; #closed = false; #closePromise: Promise | null = null; + #controllerDetachedForRestart = false; #failure: Error | null = null; readonly #failureSignal: Promise; #rejectFailureSignal!: (error: Error) => void; @@ -3931,6 +3935,20 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { }; } + async #publishSpawnedProcess(handle: RunnerProcessHandle): Promise { + this.#evidence.runnerPid = handle.child.pid ?? null; + this.#evidence.runnerProcessGroupId = handle.processGroupId ?? null; + this.#publish(); + if (handle.child.pid !== undefined) { + await this.options.onSpawn?.({ + pid: handle.child.pid, + processGroupId: handle.processGroupId ?? null, + // Persist the same original process identity exposed to recovery. + startedAt: this.processInfo().startedAt, + }); + } + } + async #readDurableRunnerState(): Promise> { if (this.options.readRunnerState) return this.options.readRunnerState(); return record( @@ -4066,6 +4084,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { } close(reason?: string): Promise { + // Detachment relinquishes process ownership. A late execution finalizer + // must not suspend or signal the runner now owned by the next controller. + if (this.#controllerDetachedForRestart) return Promise.resolve(); if (reason) { this.#diagnostic( `runner transport close requested: ${reason.replaceAll(/[\r\n]/g, " ").slice(0, 1_000)}`, @@ -4077,6 +4098,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { async detachControllerForRestart(): Promise { if (this.#closed) return; + this.#controllerDetachedForRestart = true; this.#closed = true; this.#turnStartAdmission?.resolve(false); if (this.#pump !== null) clearInterval(this.#pump); @@ -4279,6 +4301,13 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { ): Promise> { if (this.#core !== null) throw new Error("PRP provider thread is already started"); + if (this.options.adoptExistingRunner) { + // A crash can precede the first driver checkpoint even though runnerd + // already opened the provider. Exact process adoption must reuse that + // authority instead of enqueueing another run.prepare/session.open pair. + await this.#resume(); + return this.#openedThreadResponse(params); + } const token = randomUUID().replaceAll("-", ""); const identity = this.options.prpIdentity ?? { runnerInstanceId: `runner_lab_${token}`, @@ -4678,6 +4707,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { }); this.#handle = handle; this.#watchRunner(handle); + await this.#publishSpawnedProcess(handle); await registration?.activate?.(); if (registration?.failure) { void registration.failure.catch((error: unknown) => { @@ -4696,6 +4726,12 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { await this.#waitForProviderIdentity(); this.#startupComplete = true; this.#diagnostic("runnerd authenticated to the durable PRP control plane"); + return this.#openedThreadResponse(params); + } + + #openedThreadResponse(params: Record): Record { + const provider = this.options.provider ?? "codex"; + const acpxAgent = this.options.acpxAgent ?? "codex"; return { thread: { id: this.#threadId, @@ -5302,6 +5338,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { if (handle) { this.#handle = handle; this.#watchRunner(handle); + await this.#publishSpawnedProcess(handle); } if (oldTransitionRegistration && newTransitionRegistration) { await oldTransitionRegistration.activate?.(); diff --git a/packages/paperclip-runner/src/native-session-runtime.test.ts b/packages/paperclip-runner/src/native-session-runtime.test.ts index f193541714..024858c6ed 100644 --- a/packages/paperclip-runner/src/native-session-runtime.test.ts +++ b/packages/paperclip-runner/src/native-session-runtime.test.ts @@ -28,6 +28,7 @@ import { import { executeNativeSession, completeTerminatedRemoteNativeSessionCleanup, + completeTerminatedLocalNativeSessionCleanup, type ExecuteNativeSessionOptions, } from "./native-session-runtime.js"; @@ -2187,6 +2188,34 @@ describe("executeNativeSession recovery", () => { } }); + it("waits for controller ownership publication before dispatching a turn", async () => { + let release!: () => void; + const published = new Promise((resolve) => { release = resolve; }); + const snapshotFailure = new Error("stop after ownership publication"); + const snapshot = vi.fn(async () => { throw snapshotFailure; }); + const session: NativeSession = { + identity: () => identity, + async capabilities() { return { resume: false, typedEvents: true, steering: false, interruption: true }; }, + async *events() {}, + async startTurn() { throw new Error("unexpected turn"); }, + async result() { return null; }, + snapshot, + close: vi.fn(async () => undefined), + }; + const onSession = vi.fn(async (current: NativeSession | null) => { if (current) await published; }); + const running = executeNativeSession({ + input, + backend: { async descriptor() { return { kind: "mock", name: "owner-barrier", version: "1", capabilities: await session.capabilities() }; }, async openSession() { return session; } }, + controlPlane: { async openRun() {}, async checkpointSession() {}, async appendEvent() { throw new Error("unexpected event"); }, async replayEvents() { return { events: [], highestContiguousSourceSeq: 0 }; }, async completeRun() {} }, + runnerInstanceId: "runner-recovery", controlPlaneInstanceId: "control-recovery", onSession, + }); + const rejected = expect(running).rejects.toBe(snapshotFailure); + await vi.waitFor(() => expect(onSession).toHaveBeenCalledWith(session)); + expect(snapshot).not.toHaveBeenCalled(); + release(); + await rejected; + }); + it("closes the provider when owner quarantine notification throws", async () => { const snapshotFailure = new Error("snapshot failed"); const close = vi.fn(async () => undefined); @@ -3912,7 +3941,9 @@ describe("executeNativeSession recovery", () => { completeRun: vi.fn(async () => {}), }; const onSession = vi.fn(); + const onSessionAdmission = vi.fn(async () => {}); const options: ExecuteNativeSessionOptions = { + onSessionAdmission, input, backend, controlPlane: port, @@ -3935,6 +3966,7 @@ describe("executeNativeSession recovery", () => { NativeSessionCleanupQuarantinedError, ); expect(backend.openSession).toHaveBeenCalledOnce(); + expect(onSessionAdmission).toHaveBeenCalledOnce(); expect(close).toHaveBeenCalledOnce(); } }, @@ -3993,6 +4025,42 @@ describe("executeNativeSession recovery", () => { completeTerminatedRemoteNativeSessionCleanup({ ...binding, remoteCleanupScope: "other-sandbox" }); }); + it("retires local quarantine only for the stopped run and runner instance", async () => { + const scopedIdentity = { ...identity, companyId: "local-stop-company", runId: "local-stop-run" }; + const binding = { ...scopedIdentity, runnerInstanceId: "local-runner" }; + const scopedInput = { ...input, binding: { ...input.binding, companyId: scopedIdentity.companyId, runId: scopedIdentity.runId } }; + const failure = new NativeSessionCloseUnrecoverableError(); + const capabilities = { resume: true, typedEvents: true, steering: false, interruption: false, structuredResult: true }; + const session: NativeSession = { + identity: () => scopedIdentity, capabilities: async () => capabilities, + async *events() { throw new Error("local transport stopped"); }, + startTurn: async () => ({ turnId: "local-turn" }), result: async () => null, + close: vi.fn(async () => { throw failure; }), + }; + const backend: NativeSessionBackend = { + descriptor: async () => ({ kind: "local", name: "local-stop-test", version: "1", capabilities }), + openSession: vi.fn(async () => session), + }; + const controlPlane: ControlPlanePort = { + openRun: async () => {}, checkpointSession: async () => {}, + appendEvent: async () => ({ cursor: 0, highestContiguousSourceSeq: 0, disposition: "committed" }), + replayEvents: async () => ({ events: [], highestContiguousSourceSeq: 0 }), completeRun: vi.fn(async () => {}), + }; + const options = { input: scopedInput, backend, controlPlane, runnerInstanceId: binding.runnerInstanceId, + controlPlaneInstanceId: "control", requireSessionCloseBeforeReturn: true }; + await expect(executeNativeSession(options)).rejects.toBe(failure); + completeTerminatedLocalNativeSessionCleanup({ ...binding, companyId: "other-company" }); + completeTerminatedLocalNativeSessionCleanup({ ...binding, runId: "other-run" }); + expect(completeTerminatedLocalNativeSessionCleanup({ ...binding, runnerInstanceId: "other-runner" })).toBe(false); + await expect(executeNativeSession(options)).rejects.toBeInstanceOf(NativeSessionCleanupQuarantinedError); + expect(backend.openSession).toHaveBeenCalledOnce(); + expect(completeTerminatedLocalNativeSessionCleanup(binding)).toBe(true); + await expect(executeNativeSession(options)).rejects.toBe(failure); + expect(backend.openSession).toHaveBeenCalledTimes(2); + expect(controlPlane.completeRun).not.toHaveBeenCalled(); + completeTerminatedLocalNativeSessionCleanup(binding); + }); + it("propagates an exhausted required backend checkpoint close", async () => { vi.useFakeTimers(); try { diff --git a/packages/paperclip-runner/src/native-session-runtime.ts b/packages/paperclip-runner/src/native-session-runtime.ts index 53feadd3b7..159bd9d751 100644 --- a/packages/paperclip-runner/src/native-session-runtime.ts +++ b/packages/paperclip-runner/src/native-session-runtime.ts @@ -153,6 +153,29 @@ export function completeTerminatedRemoteNativeSessionCleanup(binding: { return true; } +/** Host-only counterpart of remote resource termination. The caller has verified + * both exact local process identities and durably fenced their run. This removes + * only cleanup ownership; the retained checkpoint is never made resumable. + */ +export function completeTerminatedLocalNativeSessionCleanup(binding: { + companyId: string; + runId: string; + runnerInstanceId: string; +}): boolean { + const matches = [...quarantinedSessionCleanups].filter(({ session, domain }) => { + const identity = session.identity(); + const parts = JSON.parse(domain) as string[]; + return parts.length === 3 && identity.companyId === binding.companyId && identity.runId === binding.runId; + }); + if (matches.some(entry => entry.attempt || entry.recovery || + sessionOriginRunnerInstances.get(entry.session) !== binding.runnerInstanceId)) return false; + for (const entry of matches) { + if (entry.timer) clearTimeout(entry.timer); + quarantinedSessionCleanups.delete(entry); + } + return true; +} + export interface NativeSessionGoalControl { requestId: string; action: "create" | "edit" | "replace" | "pause" | "resume" | "clear"; @@ -161,6 +184,8 @@ export interface NativeSessionGoalControl { } export interface ExecuteNativeSessionOptions { + /** Durable launch intent, after cleanup admission and before provider calls. */ + onSessionAdmission?: () => Promise; input: NativeExecutionInput; backend: NativeSessionBackend; controlPlane: ControlPlanePort; @@ -1841,6 +1866,7 @@ export async function executeNativeSession( previousProviderSessionId: string | null; } | null = null; let reconciledRecoveryCheckpoint: PersistedNativeSession | null = null; + await options.onSessionAdmission?.(); if (options.existingSession) { if (options.existingSession.attachRun === undefined) { throw new Error("native_session_multi_run_unavailable"); @@ -2099,7 +2125,7 @@ export async function executeNativeSession( // Ownership publication is part of the execution-owned lifetime. If the // callback fails, the finally block below still quarantines and closes the // provider session. - options.onSession?.(session); + await options.onSession?.(session); const checkpointTimeoutMs = options.checkpointTimeoutMs ?? DEFAULT_NATIVE_CHECKPOINT_TIMEOUT_MS; const persistCheckpoint = ( diff --git a/packages/shared/src/app-definitions.test.ts b/packages/shared/src/app-definitions.test.ts index b387e5286a..37cbc57045 100644 --- a/packages/shared/src/app-definitions.test.ts +++ b/packages/shared/src/app-definitions.test.ts @@ -238,6 +238,13 @@ const GOOGLE_WORKSPACE_PROFILE_EXPECTATIONS = [ writeTools: readonly string[]; }>; describe("AppDefinition catalog", () => { + it("offers Anthropic runtime authentication without the unsupported REST tool method", () => { + const anthropic = APP_DEFINITIONS.find((app) => app.slug === "anthropic")!; + expect(anthropic.methods.map((method) => method.key)).toEqual(["ai-subscription", "ai-api_key"]); + expect(anthropic.methods.every((method) => method.purpose === "ai" && method.transport === "runtime_auth")).toBe(true); + expect(getAvailableConnectionMethod(anthropic, "api-key")).toBeNull(); + }); + it("validates all Wave 1 definitions", () => expect(() => appDefinitionsSchema.parse(APP_DEFINITIONS)).not.toThrow()); it("contains every established provider plus the reviewed self-serve catalog", () => { diff --git a/packages/shared/src/app-definitions/anthropic.json b/packages/shared/src/app-definitions/anthropic.json index 65452bf305..bd16bb3444 100644 --- a/packages/shared/src/app-definitions/anthropic.json +++ b/packages/shared/src/app-definitions/anthropic.json @@ -70,34 +70,6 @@ "location": "env", "name": "ANTHROPIC_API_KEY" } - }, - { - "key": "api-key", - "transport": "rest_api", - "auth": "api_key", - "ownershipModes": [ - "customer" - ], - "whenToUse": "Use credentials from your provider account.", - "defaults": { - "serviceHost": "api.anthropic.com" - }, - "guidanceMd": "Create a key in the Anthropic Console and rotate it if it has been exposed.", - "riskTier": "S3", - "credentialFields": [ - { - "key": "apiKey", - "label": "API key", - "type": "password", - "required": true, - "placeholder": "sk-ant-api03-...", - "secret": true - } - ], - "keyPlacement": { - "location": "header", - "name": "x-api-key" - } } ] } diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 37cd386f6f..4a333c2580 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -855,6 +855,7 @@ export const updateIssueSchema = objectWithoutDefaults( requestDepth: issueRequestDepthInputSchema.optional(), assigneeAgentId: z.string().trim().min(1).optional().nullable(), comment: multilineTextSchema.pipe(z.string().min(1)).optional(), + commentClientRequestId: z.string().uuid().optional(), /** Only valid with a comment; the route binds these in the update transaction. */ attachmentIds: issueCommentAttachmentIdsSchema.optional(), onBehalfOfUserId: z.string().trim().min(1).optional().nullable(), diff --git a/patches/@agentclientprotocol__claude-agent-acp@0.73.0.patch b/patches/@agentclientprotocol__claude-agent-acp@0.73.0.patch index f84471a742..6774dc6553 100644 --- a/patches/@agentclientprotocol__claude-agent-acp@0.73.0.patch +++ b/patches/@agentclientprotocol__claude-agent-acp@0.73.0.patch @@ -1,8 +1,8 @@ diff --git a/dist/acp-agent.js b/dist/acp-agent.js +index 0214a509eb5e7b777a69e4e34637941c275d27b1..403be02df66e25eee29e7d5bef14627011e02b30 100644 --- a/dist/acp-agent.js +++ b/dist/acp-agent.js -@@ -2917,10 +2917,20 @@ - cost: { +@@ -2922,9 +2922,19 @@ export class ClaudeAcpAgent { amount: message.total_cost_usd, currency: "USD", }, @@ -25,9 +25,7 @@ diff --git a/dist/acp-agent.js b/dist/acp-agent.js }, }); } -@@ -5307,8 +5317,12 @@ - const options = { - systemPrompt, +@@ -5311,6 +5321,10 @@ export class ClaudeAcpAgent { settingSources: ["user", "project", "local"], ...(thinking !== undefined && { thinking }), ...userProvidedOptions, @@ -38,7 +36,7 @@ diff --git a/dist/acp-agent.js b/dist/acp-agent.js ...(settings && { settings }), env, // Override certain fields that must be controlled by ACP -@@ -5317,7 +5331,9 @@ +@@ -5318,7 +5332,9 @@ export class ClaudeAcpAgent { includePartialMessages: true, forwardSubagentText, mcpServers: { @@ -49,3 +47,20 @@ diff --git a/dist/acp-agent.js b/dist/acp-agent.js ...mcpServers, ...(fileChangeAuditSupport ? { [FILE_CHANGE_AUDIT_SERVER_NAME]: fileChangeAuditSupport.mcpServer } +@@ -5329,6 +5345,16 @@ export class ClaudeAcpAgent { + allowDangerouslySkipPermissions: ALLOW_BYPASS, + permissionMode: initialPermissionMode, + canUseTool: this.canUseTool(sessionId), ++ // The runner pins this actual MCP endpoint before provider launch. ++ // Exempt only task delivery/control tools at the SDK dispatch boundary, ++ // not arbitrary provider permission metadata or MCP name prefixes. ++ ...(process.env.PAPERCLIP_ACPX_ISOLATED_CONTEXT === "1" && { ++ allowedTools: mcpServers.paperclip?.type === "http" ++ && mcpServers.paperclip.url === process.env.PAPERCLIP_ACPX_TASK_TOOL_BRIDGE_URL ++ ? ["paperclip_finish", "paperclip_block", "read_current_wake_comments", "request_human_input"] ++ .map((tool) => `mcp__paperclip__${tool}`) ++ : [], ++ }), + // Forward MCP elicitation requests onto ACP elicitation. Only attached + // when the client advertised support, so non-supporting clients keep the + // SDK's default (auto-decline) behavior. (AskUserQuestion is handled in diff --git a/scripts/ingest-app-definitions.mjs b/scripts/ingest-app-definitions.mjs index fb34cc8d6d..6abdf4af55 100644 --- a/scripts/ingest-app-definitions.mjs +++ b/scripts/ingest-app-definitions.mjs @@ -1490,7 +1490,9 @@ for (const [slug, name, subscription, envKey] of [["anthropic", "Claude", true, let app=apps.find(a=>a.slug===slug); if(!app){app={schemaVersion:1,slug,name,description:`Connect ${name} accounts for your agents.`,categories:["ai"],branding:brandingFor(slug),urlPatterns:[{"openai":"https://api.openai.com/*","openrouter":"https://openrouter.ai/api/*","xai":"https://api.x.ai/*"}[slug]],methods:[]};apps.push(app);} const methods=(subscription?["subscription","api_key"]:["api_key"]).map(authMethod=>({key:`ai-${authMethod}`,label:authMethod==="subscription"?`${name} subscription`:`${name} API key`,purpose:"ai",transport:"runtime_auth",auth:authMethod==="subscription"?"oauth":"api_key",ai:{provider:slug,method:authMethod},grantKinds:["user","organization"],ownershipModes:["customer"],whenToUse:"Authenticate an agent with this account.",guidanceMd:"Use your personal account or an explicitly shared company account.",riskTier:"S3",...(authMethod==="api_key"?{credentialFields:[field("apiKey","API key","Enter API key")],keyPlacement:{location:"env",name:envKey}}:{})})); - app.methods.unshift(...methods); + // Legacy REST entries have no tool execution adapter. Only offer the supported + // AI account flow; saved REST connections remain removable through Connections. + app.methods = [...methods, ...app.methods.filter(method => method.transport !== "rest_api")]; } const validateApp = (app) => { if ( diff --git a/server/package.json b/server/package.json index d61cc8ee07..a2ecf6d49f 100644 --- a/server/package.json +++ b/server/package.json @@ -71,6 +71,7 @@ "@paperclipai/skills-catalog": "workspace:*", "@photon-ai/advanced-imessage": "2.1.0", "@vercel/connect": "0.6.1", + "acorn": "8.18.0", "acpx": "0.13.1", "ajv": "^8.20.0", "ajv-formats": "^3.0.1", diff --git a/server/src/__tests__/connection-intents-service.test.ts b/server/src/__tests__/connection-intents-service.test.ts index d3cb60829a..8b1a5d14e5 100644 --- a/server/src/__tests__/connection-intents-service.test.ts +++ b/server/src/__tests__/connection-intents-service.test.ts @@ -793,7 +793,7 @@ describeEmbeddedPostgres("connectionIntentService", () => { await expect(service.search(claims, "notion")) .rejects.toThrow("no longer active"); }); - it("keeps runtime authentication requests distinct from the same provider's tool requests", async () => { + it("keeps runtime authentication separate from obsolete Anthropic tool requests", async () => { const companyId = claims.company_id; const agentId = randomUUID(); const issueId = randomUUID(); @@ -808,16 +808,38 @@ describeEmbeddedPostgres("connectionIntentService", () => { await db.insert(aiConnectionDefaults).values({ companyId, userId: claims.responsible_user_id!, provider: "anthropic", method: "api_key", grantId: grant!.id }); const aiClaims = { ...claims, sub: agentId, run_id: aiRunId }; const service = connectionIntentService(db); - const toolRequest = await service.request(aiClaims, "anthropic"); + await expect(service.request(aiClaims, "anthropic")).rejects.toMatchObject({ + status: 422, + message: "Connection service anthropic is not available", + }); + // Preserve an intent created before the obsolete REST method was removed. + // It must neither alias the AI request nor accept an AI account as tools. + const toolRequest = await issueThreadInteractionService(db).createConnectionIntent( + { id: issueId, companyId }, + { + payload: { + version: 1, + serviceSlug: "anthropic", + serviceName: "Anthropic", + serviceLogoUrl: null, + requestingAgentId: agentId, + requestingAgentName: "AI Agent", + phase: "requested", + }, + sourceRunId: aiRunId, + addresseeUserId: claims.responsible_user_id!, + idempotencyKey: `connection-intent:${aiRunId}:${claims.responsible_user_id}:anthropic`, + }, + ); const aiRequest = await service.request(aiClaims, "anthropic", { purpose: "ai" }); expect(aiRequest.state).toBe("needs_user_action"); - expect(aiRequest.interactionId).not.toBe(toolRequest.interactionId); + expect(aiRequest.interactionId).not.toBe(toolRequest.id); expect((await service.setupOptions(aiRequest.interactionId!)).aiConnection).toEqual(binding); - expect((await service.setupOptions(toolRequest.interactionId!)).existingConnections).toEqual([]); - await expect(service.complete(toolRequest.interactionId!, connection!.id, claims.responsible_user_id!)).rejects.toThrow("cannot satisfy"); + expect((await service.setupOptions(toolRequest.id)).existingConnections).toEqual([]); + await expect(service.complete(toolRequest.id, connection!.id, claims.responsible_user_id!)).rejects.toThrow("cannot satisfy"); await expect(service.complete(aiRequest.interactionId!, connection!.id, claims.responsible_user_id!)).resolves.toMatchObject({ status: "accepted" }); expect((await service.request(aiClaims, "anthropic", { purpose: "ai" })).state).toBe("ready"); - expect((await service.request(aiClaims, "anthropic")).state).toBe("needs_user_action"); + await expect(service.request(aiClaims, "anthropic")).rejects.toMatchObject({ status: 422 }); expect((await service.search(aiClaims, "openrouter")).results.some(result => result.service === "openrouter")).toBe(false); }); diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index c796cc28e9..428e850e10 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -118,6 +118,8 @@ const mockTelemetryClient = vi.hoisted(() => ({ })); const mockTrackAgentFirstHeartbeat = vi.hoisted(() => vi.fn()); const mockTerminateLocalService = vi.hoisted(() => vi.fn()); +const mockDetachNativeSessionsForRestart = vi.hoisted(() => vi.fn()); +const mockCloseIdleWarmNativeSessionsForRestart = vi.hoisted(() => vi.fn()); const mockRetainedNativeCleanup = vi.hoisted(() => vi.fn< typeof import("../services/native-runtime/native-session-executor.js").reconcileRetainedNativeSessionCleanup @@ -154,10 +156,14 @@ vi.mock("../services/native-runtime/native-session-executor.js", async () => { mockExecutePaperclipNativeSession.mockImplementation( actual.executePaperclipNativeSession, ); + mockDetachNativeSessionsForRestart.mockImplementation(actual.detachNativeSessionsForRestart); + mockCloseIdleWarmNativeSessionsForRestart.mockImplementation(actual.closeIdleWarmNativeSessionsForRestart); return { ...actual, reconcileRetainedNativeSessionCleanup: mockRetainedNativeCleanup, executePaperclipNativeSession: mockExecutePaperclipNativeSession, + detachNativeSessionsForRestart: mockDetachNativeSessionsForRestart, + closeIdleWarmNativeSessionsForRestart: mockCloseIdleWarmNativeSessionsForRestart, }; }); @@ -2932,6 +2938,15 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); + it("checkpoints idle warm sessions even when no hot restart was requested", async () => { + await withTempPaperclipHome(async () => { + mockCloseIdleWarmNativeSessionsForRestart.mockClear(); + const heartbeat = heartbeatService(db); + await expect(heartbeat.prepareHotRestartShutdown("SIGTERM")).resolves.toMatchObject({ mode: "not_requested" }); + expect(mockCloseIdleWarmNativeSessionsForRestart).toHaveBeenCalledOnce(); + }); + }); + it("captures a hot-restart shutdown snapshot without interrupting running runs", async () => { const child = spawnAliveProcess(); childProcesses.add(child); @@ -2954,6 +2969,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); const heartbeat = heartbeatService(db); + mockCloseIdleWarmNativeSessionsForRestart.mockClear(); const result = await heartbeat.prepareHotRestartShutdown( "SIGTERM", new Date("2026-03-19T00:06:00.000Z"), @@ -2964,6 +2980,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { skipDrain: true, activeRunIds: [runId], }); + expect(mockCloseIdleWarmNativeSessionsForRestart).toHaveBeenCalledOnce(); expect(isPidAlive(child.pid)).toBe(true); const run = await db .select() @@ -3748,6 +3765,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { retryRunIds: [], restartSuspendedRunIds: [runId], }); + expect(mockDetachNativeSessionsForRestart).toHaveBeenCalledWith([runId]); await expect( db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId)), ).resolves.toEqual([ @@ -13529,8 +13547,9 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { return { ...fixture, commentId, contractId, summary }; } - it("commits a native passive Board response without manufacturing immediate work", async () => { + it.each(["issue_commented", "issue_reopened_via_comment"])("commits a native passive Board response without manufacturing immediate work (%s)", async (reason) => { const fixture = await seedNativePassiveBoardResponse(); + await db.update(agentWakeupRequests).set({ reason }).where(eq(agentWakeupRequests.id, fixture.wakeupRequestId)); await finalizeNativeRun({ db, runId: fixture.runId, @@ -14015,6 +14034,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { "source_delete", "different_user", "wake_actor", + "wake_reason", "wake_run", "native_issue", "new_comment", @@ -14027,6 +14047,8 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect( await readNativeBoardResponseWaitSource(db, fixture), ).not.toBeNull(); + if (change === "wake_reason") + await db.update(agentWakeupRequests).set({ reason: "issue_continuation_needed" }).where(eq(agentWakeupRequests.id, fixture.wakeupRequestId)); if (change === "source_edit") await db .update(issueComments) diff --git a/server/src/__tests__/heartbeat-run-lease-release-terminalization.test.ts b/server/src/__tests__/heartbeat-run-lease-release-terminalization.test.ts index 4cda968220..3b015f5f3c 100644 --- a/server/src/__tests__/heartbeat-run-lease-release-terminalization.test.ts +++ b/server/src/__tests__/heartbeat-run-lease-release-terminalization.test.ts @@ -151,6 +151,20 @@ describeEmbeddedPostgres("heartbeat terminalizeRunOnLeaseRelease", () => { expect(row?.errorCode).toBe("lease_released_before_terminal"); }); + it.each(["in_progress", "done"])("preserves acknowledged Stop when teardown wins the finalizer race (%s)", async (issueStatus) => { + const { companyId, issueId, runId } = await seed({ issueStatus, runStatus: "running" }); + const [run] = await db.update(heartbeatRuns).set({ + nativeIssueId: issueId, + resultJson: { cancelledByActorType: "user", cancelledByUserId: "board", nativeCancellation: { + schema: "paperclip.native-cancellation.v1", runId, companyId, issueId, scope: "run", + reasonCode: "cancellation_run_only", dispatched: true, dispatchState: "acknowledged", + intentAuditId: randomUUID(), acknowledgementAuditId: randomUUID(), + } }, + }).where(eq(heartbeatRuns.id, runId)).returning(); + const terminal = await heartbeatService(db).terminalizeRunOnLeaseRelease(run!); + expect(terminal).toMatchObject({ status: "cancelled", error: null, errorCode: null }); + }); + it("forces a still-queued run to interrupted when the lease releases before it starts", async () => { // A queued run holds a lease but never reached "running". The teardown // released the lease, so the run must not stay queued and show a phantom diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index ee148f1356..eb1dd5799d 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -546,6 +546,7 @@ describe.sequential("issue comment reopen routes", () => { .patch(`/api/issues/${issue.id}`) .send({ comment: "Inspect the file", + commentClientRequestId: "77777777-7777-4777-8777-777777777777", attachmentIds: [id], assigneeAgentId: "33333333-3333-4333-8333-333333333333", }); @@ -561,7 +562,7 @@ describe.sequential("issue comment reopen routes", () => { issue.id, "Inspect the file", expect.anything(), - expect.objectContaining({ attachmentIds: [id] }), + expect.objectContaining({ attachmentIds: [id], clientRequestId: "77777777-7777-4777-8777-777777777777" }), mockTx, ); }); diff --git a/server/src/__tests__/issue-feedback-routes.test.ts b/server/src/__tests__/issue-feedback-routes.test.ts index 5366814a93..31e625e6fe 100644 --- a/server/src/__tests__/issue-feedback-routes.test.ts +++ b/server/src/__tests__/issue-feedback-routes.test.ts @@ -140,6 +140,12 @@ async function createApp(actor: Record) { next(); }); app.use("/api", issueRoutes({} as any, {} as any, { feedbackExportService: mockFeedbackExportService })); + const routeErrors: string[] = []; + app.locals.routeErrors = routeErrors; + app.use((error: unknown, _req: express.Request, _res: express.Response, next: express.NextFunction) => { + routeErrors.push(error instanceof Error ? error.stack ?? error.message : String(error)); + next(error); + }); app.use(errorHandler); return app; } @@ -253,7 +259,7 @@ describe("issue feedback trace routes", () => { const res = await request(app).get("/api/feedback-traces/trace-1"); - expect(res.status).toBe(404); + expect(res.status, JSON.stringify({ body: res.body, errors: app.locals.routeErrors })).toBe(404); }); it("returns 404 for bundle fetches when a board user lacks access to the trace company", async () => { @@ -273,6 +279,6 @@ describe("issue feedback trace routes", () => { const res = await request(app).get("/api/feedback-traces/trace-1/bundle"); - expect(res.status).toBe(404); + expect(res.status, JSON.stringify({ body: res.body, errors: app.locals.routeErrors })).toBe(404); }); }); diff --git a/server/src/__tests__/issue-queued-comments-routes.test.ts b/server/src/__tests__/issue-queued-comments-routes.test.ts index fe8856dccf..801c829ac9 100644 --- a/server/src/__tests__/issue-queued-comments-routes.test.ts +++ b/server/src/__tests__/issue-queued-comments-routes.test.ts @@ -271,6 +271,61 @@ describeEmbeddedPostgres("issue queued-comment routes", () => { await request(client).post(`/api/issues/${seeded.issueId}/queued-comments/interrupt`).send(body).expect(409); }); + it.each([ + ["other", "running"], ["other", "queued"], ["other", "scheduled_retry"], + ["same", "running"], ["same", "queued"], ["same", "scheduled_retry"], + ] as const)("scopes interrupted queue successors to its agent: %s agent %s", async (owner, status) => { + const seeded = await seedQueue(); + await db.update(agents).set({ adapterType: "claude_local", + runtimeConfig: { heartbeat: { maxConcurrentRuns: 1 } }, + }).where(eq(agents.id, seeded.agentId)); + await db.update(heartbeatRuns).set({ runtimeMode: "legacy", status: "succeeded", + finishedAt: new Date("2026-08-22T15:03:00.000Z"), + }).where(eq(heartbeatRuns.id, seeded.runId)); + await db.update(issues).set({ executionRunId: null }).where(eq(issues.id, seeded.issueId)); + const [wake] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, seeded.wakeId)); + await db.update(agentWakeupRequests).set({ payload: { ...wake.payload, + queuedCommentInterrupt: { actorId: "other-operator", requestedAt: new Date().toISOString() }, + } }).where(eq(agentWakeupRequests.id, seeded.wakeId)); + // Keep this agent at capacity so successful delivery queues a successor + // without launching a provider. The independent run shares only the task. + await db.insert(heartbeatRuns).values({ companyId: seeded.companyId, agentId: seeded.agentId, + status: "running", contextSnapshot: { issueId: randomUUID() }, + }); + const successorAgentId = owner === "same" ? seeded.agentId : randomUUID(); + if (owner === "other") await db.insert(agents).values({ id: successorAgentId, + companyId: seeded.companyId, name: "Independent agent", role: "engineer", + status: "idle", adapterType: "claude_local", + }); + const [existingRun] = await db.insert(heartbeatRuns).values({ companyId: seeded.companyId, agentId: successorAgentId, + status, contextSnapshot: { issueId: seeded.issueId }, + }).returning(); + + await heartbeatService(db).resumeQueuedCommentInterrupt(seeded.companyId, seeded.wakeId, { retryCleanup: true }); + if (owner === "other" && status !== "scheduled_retry") { + // Another agent is not this queue's successor, but ordinary admission + // must still preserve the task execution lock until its work stops. + const [waiting] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, seeded.wakeId)); + const [task] = await db.select().from(issues).where(eq(issues.id, seeded.issueId)); + expect(waiting.status).toBe("deferred_issue_execution"); + expect(task.executionRunId).toBe(existingRun.id); + expect(waiting.payload?.queuedCommentInterrupt).toMatchObject({ actorId: "other-operator" }); + await db.update(heartbeatRuns).set({ status: "succeeded", finishedAt: new Date() }) + .where(eq(heartbeatRuns.id, existingRun.id)); + await heartbeatService(db).resumeQueuedCommentInterrupt(seeded.companyId, seeded.wakeId, { retryCleanup: true }); + } + const [receipt] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, seeded.wakeId)); + expect(receipt.status).toBe(owner === "same" ? "deferred_issue_execution" : "coalesced"); + if (owner === "other") { + const [successor] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, receipt.runId!)); + expect(successor).toMatchObject({ agentId: seeded.agentId, status: "queued", responsibleUserId: "other-operator" }); + expect(successor.contextSnapshot?.wakeCommentIds).toEqual(seeded.commentIds); + } + await heartbeatService(db).resumeQueuedCommentInterrupt(seeded.companyId, seeded.wakeId, { retryCleanup: true }); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, seeded.companyId))) + .toHaveLength(owner === "same" ? 3 : 4); + }); + it.each(["user", "system"])("keeps stopped-run interruption intent on a %s receipt across restart until the process stops, then delivers once", async (actorType) => { const seeded = await seedQueue(); await db.update(agentWakeupRequests).set({ requestedByActorType: actorType }) diff --git a/server/src/__tests__/issue-runtime-workspace-binding.test.ts b/server/src/__tests__/issue-runtime-workspace-binding.test.ts new file mode 100644 index 0000000000..b8cd218053 --- /dev/null +++ b/server/src/__tests__/issue-runtime-workspace-binding.test.ts @@ -0,0 +1,49 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { companies, createDb, executionWorkspaces, issues, projects } from "@paperclipai/db"; +import { issueService } from "../services/issues.js"; +import { instanceSettingsService } from "../services/instance-settings.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("runtime shared workspace binding", () => { + let temporary: Awaited>; + let db: ReturnType; + beforeAll(async () => { + temporary = await startEmbeddedPostgresTestDatabase("paperclip-runtime-workspace-binding-"); + db = createDb(temporary.connectionString); + await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: false }); + }, 30_000); + afterAll(async () => { await temporary?.cleanup(); }); + + async function fixture(mode = "shared_workspace") { + const companyId = randomUUID(), projectId = randomUUID(), issueId = randomUUID(), workspaceId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Warm sandbox", issuePrefix: `W${companyId.slice(0, 6)}` }); + await db.insert(projects).values({ id: projectId, companyId, name: "Studio" }); + await db.insert(issues).values({ id: issueId, companyId, projectId, title: "Continue the conversation" }); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, projectId, mode, strategyType: "project_primary", name: "Studio workspace" }); + return { companyId, issueId, workspaceId }; + } + const binding = (workspaceId: string) => ({ + executionWorkspaceId: workspaceId, + executionWorkspacePreference: "reuse_existing", + executionWorkspaceSettings: { mode: "shared_workspace" as const }, + }); + + it("persists the internal binding with isolated workspaces off while public updates remain gated", async () => { + const f = await fixture(), svc = issueService(db); + const ordinary = await svc.update(f.issueId, binding(f.workspaceId)); + expect(ordinary?.executionWorkspaceId).toBeNull(); + const bound = await svc.update(f.issueId, { ...binding(f.workspaceId), companyGuard: f.companyId }, db, undefined, undefined, { bindRuntimeSharedWorkspace: true }); + expect(bound).toMatchObject(binding(f.workspaceId)); + const followUp = await svc.update(f.issueId, { title: "A second message" }); + expect(followUp).toMatchObject(binding(f.workspaceId)); + }); + + it("rejects a foreign company workspace and cannot opt into isolated worktrees", async () => { + const local = await fixture(), foreign = await fixture(), isolated = await fixture("isolated_workspace"), svc = issueService(db); + await expect(svc.update(local.issueId, binding(foreign.workspaceId), db, undefined, undefined, { bindRuntimeSharedWorkspace: true })).rejects.toThrow("existing shared workspace"); + await expect(svc.update(isolated.issueId, binding(isolated.workspaceId), db, undefined, undefined, { bindRuntimeSharedWorkspace: true })).rejects.toThrow("existing shared workspace"); + expect(await svc.update(local.issueId, { ...binding(local.workspaceId), companyGuard: foreign.companyId }, db, undefined, undefined, { bindRuntimeSharedWorkspace: true })).toBeNull(); + }); +}); diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index 88a45800b3..775d3729c8 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -305,9 +305,12 @@ describe("issue update comment wakeups", () => { assigneeAgentId: ASSIGNEE_AGENT_ID, assigneeUserId: null, comment: "write the whole thing", + commentClientRequestId: "55555555-5555-4555-8555-555555555555", }); expect(res.status).toBe(200); + expect(mockIssueService.addComment).toHaveBeenCalledWith(existing.id, "write the whole thing", expect.anything(), + expect.objectContaining({ clientRequestId: "55555555-5555-4555-8555-555555555555" })); // The route dispatches the wake after it sends the response, so wait for // the fire-and-forget dispatch to settle. This keeps the wake inside this // test and stops it from leaking into the next test as an extra call. @@ -571,9 +574,12 @@ describe("issue update comment wakeups", () => { .post(`/api/issues/${existing.id}/comments`) .send({ body: "please handle this top-level thread comment", + clientRequestId: "66666666-6666-4666-8666-666666666666", }); expect(res.status).toBe(201); + expect(mockIssueService.addComment).toHaveBeenCalledWith(existing.id, "please handle this top-level thread comment", expect.anything(), + expect.objectContaining({ clientRequestId: "66666666-6666-4666-8666-666666666666" }), expect.anything()); await vi.waitFor(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1)); expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( ASSIGNEE_AGENT_ID, diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index 62df9959ea..614bb986dd 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -229,6 +229,16 @@ vi.mock("../app.js", () => ({ createApp: createAppMock, })); +vi.mock("../services/native-runtime/native-session-executor.js", () => ({ + verifyStoppedNativeSessionForReplacement: vi.fn(async () => null), +})); + +// This suite verifies server startup scheduling; replacement correctness is +// exercised by the dedicated DB-backed recovery suites. +vi.mock("../services/native-runtime/native-safe-replacement.js", () => ({ + reconcileSafeNativeReplacements: vi.fn(async () => ({ scanned: 0, scheduled: 0 })), +})); + vi.mock("../config.js", () => ({ loadConfig: loadConfigMock, })); @@ -406,6 +416,8 @@ vi.mock("../auth/better-auth.js", () => ({ })); import { startServer } from "../index.ts"; +import { reconcileSafeNativeReplacements } from "../services/native-runtime/native-safe-replacement.js"; +import { EXECUTION_RECONCILIATION_INTERVAL_MS } from "../services/execution-control-deadline.js"; describe("startServer feedback export wiring", () => { beforeEach(() => { @@ -550,6 +562,34 @@ describe("startServer feedback export wiring", () => { } }); + it("reconciles native replacements at startup and on the execution-control interval", async () => { + loadConfigMock.mockReturnValue(buildTestConfig({ heartbeatSchedulerEnabled: true })); + let executionControlTick: (() => void) | undefined; + const setIntervalSpy = vi.spyOn(globalThis, "setInterval").mockImplementation((( + callback: () => void, + interval: number, + ) => { + if (interval === EXECUTION_RECONCILIATION_INTERVAL_MS) executionControlTick = callback; + return 1 as unknown as ReturnType; + }) as typeof setInterval); + try { + await startServer(); + await new Promise((resolve) => setImmediate(resolve)); + expect(reconcileSafeNativeReplacements).toHaveBeenCalledExactlyOnceWith( + createDbMock.mock.results[0]?.value, + expect.any(Date), + { verifyStoppedSession: expect.any(Function) }, + ); + + expect(executionControlTick).toBeDefined(); + executionControlTick?.(); + await new Promise((resolve) => setImmediate(resolve)); + expect(reconcileSafeNativeReplacements).toHaveBeenCalledTimes(2); + } finally { + setIntervalSpy.mockRestore(); + } + }); + it("keeps routine ticks and setup cleanup active when heartbeat scheduling is suppressed", async () => { loadConfigMock.mockReturnValue(buildTestConfig({ heartbeatSchedulerEnabled: true, diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 2b961cb3d1..48b694029f 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -2614,6 +2614,80 @@ describeEmbeddedPostgres("tool access service", () => { expect(health.connection.healthStatus).toBe("ok"); }); + it.each( + [ + { sourceTemplateKey: "anthropic", connectionMethodKey: "api-key" }, + { + sourceTemplateKey: "unsupported-rest-fixture", + templateId: "paperclip.echo-calculator-time", + }, + ].flatMap((config) => + (["checkHealth", "refreshCatalog"] as const).map((operation) => ({ config, operation })), + ), + )("rejects unsupported REST tool connections without stdio validation: %j", async ({ config, operation }) => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + const application = await service.createApplication(company.id, { + name: "REST regression fixture", + type: "rest_api", + }); + const connection = await service.createConnection(company.id, { + applicationId: application.id, + name: "REST regression fixture", + transport: "rest_api", + config, + enabled: true, + status: "active", + }); + const fetchMock = vi.spyOn(globalThis, "fetch"); + const message = "This connection has no supported tool integration. Add a supported account or MCP connection from Connectors."; + + await expect(service[operation](connection.id)).rejects.toMatchObject({ + status: 422, + message, + details: { code: "tool_connection_transport_unsupported" }, + }); + const [saved] = await db.select().from(toolConnections) + .where(eq(toolConnections.id, connection.id)); + expect(saved).toMatchObject({ healthStatus: "error", healthMessage: message }); + expect(fetchMock).not.toHaveBeenCalled(); + expect(await service.listRuntimeSlots(company.id)).toEqual([]); + expect(await db.select().from(toolCatalogEntries) + .where(eq(toolCatalogEntries.connectionId, connection.id))).toEqual([]); + const audit = await db.select().from(toolAccessAuditEvents) + .where(eq(toolAccessAuditEvents.connectionId, connection.id)); + expect(audit).toEqual(expect.arrayContaining([ + expect.objectContaining({ + action: operation === "checkHealth" ? "tool_connection.health_check" : "tool_connection.catalog_refresh", + outcome: "failure", + reasonCode: "tool_connection_transport_unsupported", + }), + ])); + // Removing a method from the catalog must not strand its saved connections. + expect(await service.archiveConnection(connection.id)).toMatchObject({ + connection: { status: "archived" }, + }); + }); + + it("rejects the obsolete Anthropic REST setup before storing credentials", async () => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + + await expect(service.connectGalleryApp(company.id, { + galleryKey: "anthropic", + connectionMethodKey: "api-key", + credentialValues: { "credentials.apiKey": "rest-regression-secret" }, + }, { actorType: "user", actorId: "board" })).rejects.toMatchObject({ + status: 422, + message: "This app does not have an available connection method", + }); + + expect(await db.select().from(toolConnections) + .where(eq(toolConnections.companyId, company.id))).toEqual([]); + expect(await db.select().from(companySecrets) + .where(eq(companySecrets.companyId, company.id))).toEqual([]); + }); + it("registers an approved local stdio template and exposes its runtime slot", async () => { const company = await createCompany(db); const service = createTestToolAccessService(db); diff --git a/server/src/embedded-postgres-owner.test.ts b/server/src/embedded-postgres-owner.test.ts new file mode 100644 index 0000000000..3c4858f30f --- /dev/null +++ b/server/src/embedded-postgres-owner.test.ts @@ -0,0 +1,8 @@ +import { expect, it } from "vitest"; +import { embeddedPostgresOwnerPort } from "./embedded-postgres-owner.js"; +it("uses the running cluster's selected port after a collision", () => { + expect(embeddedPostgresOwnerPort("123\n/data/qa\n123456\n54330\n", "/data/qa", 123)).toBe(54330); +}); +it.each(["124\n/data/qa\n123456\n54330\n", "123\n/data/other\n123456\n54330\n", "123\n/data/qa\n123456\n0\n", "123\n/data/qa\n"])("rejects inconsistent ownership %j", contents => { + expect(() => embeddedPostgresOwnerPort(contents, "/data/qa", 123)).toThrow("does not match"); +}); diff --git a/server/src/embedded-postgres-owner.ts b/server/src/embedded-postgres-owner.ts new file mode 100644 index 0000000000..8f9a750786 --- /dev/null +++ b/server/src/embedded-postgres-owner.ts @@ -0,0 +1,12 @@ +import { resolve } from "node:path"; + +/** postmaster.pid records the actual listening port, which can differ from the + * configured port after collision avoidance. Never reuse the configured port. */ +export function embeddedPostgresOwnerPort(contents: string, dataDir: string, expectedPid: number): number { + const [pid, directory, , portText] = contents.split("\n"); + const port = Number(portText); + if (Number(pid) !== expectedPid || !directory || resolve(directory) !== resolve(dataDir) || + !Number.isSafeInteger(port) || port < 1 || port > 65535) + throw new Error("The running embedded PostgreSQL identity does not match this instance. Startup stopped before connecting."); + return port; +} diff --git a/server/src/index.ts b/server/src/index.ts index 39d68521d8..22f3fcea58 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -7,6 +7,8 @@ import { collectWorkFolderGarbage } from "./services/work-folder-garbage.js"; import { createStorageProviderFromConfig } from "./storage/provider-registry.js"; import { instrumentationReady, shutdownInstrumentation } from "./instrumentation.js"; import { sentryReady, shutdownSentry, captureException } from "./sentry.js"; +import { verifyStoppedNativeSessionForReplacement } from "./services/native-runtime/native-session-executor.js"; +import { embeddedPostgresOwnerPort } from "./embedded-postgres-owner.js"; import { deliverExecutionStatuses } from "./services/execution-status-delivery.js"; import { deliverReconciledExecutions, settleUnrecoverableExecutions } from "./services/execution-recovery-resolution.js"; import { reconcileSafeNativeReplacements } from "./services/native-runtime/native-safe-replacement.js"; @@ -513,6 +515,11 @@ async function startServerWithDatabaseTeardown( const runningPid = getRunningPid(); if (runningPid) { + port = embeddedPostgresOwnerPort(readFileSync(postmasterPidFile, "utf8"), dataDir, runningPid); + const actualDataDir = await getPostgresDataDirectory(`postgres://paperclip:paperclip@127.0.0.1:${port}/postgres`); + if (typeof actualDataDir !== "string" || resolve(actualDataDir) !== resolve(dataDir)) { + throw new Error("Refusing to reuse PostgreSQL: its data directory belongs to another instance."); + } logger.warn(`Embedded PostgreSQL already running; reusing existing process (pid=${runningPid}, port=${port})`); } else { const configuredAdminConnectionString = `postgres://paperclip:paperclip@127.0.0.1:${configuredPort}/postgres`; @@ -1150,7 +1157,7 @@ async function startServerWithDatabaseTeardown( const executionControlSweepsInFlight = new Set(); const executionControlSweeps = [ ["finalization", () => reconcileAbandonedExecutionControl(db)], - ["replacement", () => heartbeat ? reconcileSafeNativeReplacements(db) : undefined], + ["replacement", () => heartbeat ? reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession: run => verifyStoppedNativeSessionForReplacement(db, run) }) : undefined], ["reconciliation_delivery", () => heartbeat ? deliverReconciledExecutions(db, heartbeat.wakeup) : undefined], ["status_delivery", () => deliverExecutionStatuses(db)], ["automatic_disposition", () => settleUnrecoverableExecutions(db)], diff --git a/server/src/modules/run-dispatch/adapters/postgres.test.ts b/server/src/modules/run-dispatch/adapters/postgres.test.ts index 7c50cca50e..227cb78dcc 100644 --- a/server/src/modules/run-dispatch/adapters/postgres.test.ts +++ b/server/src/modules/run-dispatch/adapters/postgres.test.ts @@ -218,6 +218,56 @@ describeEmbeddedPostgres("run-dispatch postgres adapter", () => { expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, competingId)))[0]?.status).toBe("running"); }); + it("does not dispatch a replacement when the task becomes blocked after scheduling", async () => { + const { companyId, agentId } = await seedCompanyAndAgent(); + const issueId = randomUUID(); + await seedIssue({ companyId, issueId, assigneeAgentId: agentId, status: "in_progress" }); + const contextSnapshot = { issueId, wakeReason: "native_safe_replacement", retryReason: "native_safe_replacement", forceFreshSession: true }; + const replacementId = await seedRun({ companyId, agentId, status: "scheduled_retry", contextSnapshot }); + await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, issueId)); + const adapter = createPostgresRunDispatchAdapter(db); + expect(await adapter.evaluateScheduledRetryGate({ companyId, runId: replacementId, retryReasonOverride: "native_safe_replacement", now: new Date() })) + .toMatchObject({ allowed: false, errorCode: "issue_blocked" }); + await db.update(heartbeatRuns).set({ status: "queued" }).where(eq(heartbeatRuns.id, replacementId)); + expect(await adapter.cancelStaleQueuedRun({ companyId, runId: replacementId, expectedStatus: "queued", now: new Date() })) + .toMatchObject({ outcome: "cancelled", errorCode: "issue_blocked" }); + await db.update(heartbeatRuns).set({ status: "running" }).where(eq(heartbeatRuns.id, replacementId)); + const dispatch = vi.fn(async () => undefined); + expect(await adapter.dispatchResolvedInteractionIfCurrent({ companyId, runId: replacementId, expectedStatus: "running", now: new Date(), dispatch })) + .toMatchObject({ dispatched: false, cancellation: { outcome: "cancelled" } }); + expect(dispatch).not.toHaveBeenCalled(); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.status).toBe("blocked"); + }); + + it.each(["queued", "final", "resolved"] as const)("rechecks late native replacement dependencies at %s dispatch", async mode => { + const { companyId, agentId } = await seedCompanyAndAgent(); + const issueId = randomUUID(), blockerId = randomUUID(); + await seedIssue({ companyId, issueId, assigneeAgentId: agentId, status: "in_progress" }); + await seedIssue({ companyId, issueId: blockerId, status: "todo" }); + const contextSnapshot = { issueId, wakeReason: "native_safe_replacement", retryReason: "native_safe_replacement", forceFreshSession: true }; + const replacementId = await seedRun({ companyId, agentId, status: "scheduled_retry", contextSnapshot }); + const adapter = createPostgresRunDispatchAdapter(db); + expect(await adapter.evaluateScheduledRetryGate({ companyId, runId: replacementId, retryReasonOverride: "native_safe_replacement", now: new Date() })) + .toMatchObject({ allowed: true }); + await db.insert(issueRelations).values({ companyId, issueId: blockerId, relatedIssueId: issueId, type: "blocks" }); + const dispatch = vi.fn(async () => undefined); + if (mode === "queued") { + await db.update(heartbeatRuns).set({ status: "queued" }).where(eq(heartbeatRuns.id, replacementId)); + expect(await adapter.cancelStaleQueuedRun({ companyId, runId: replacementId, expectedStatus: "queued", now: new Date() })) + .toMatchObject({ outcome: "cancelled", errorCode: "issue_dependencies_blocked" }); + } else { + if (mode === "resolved") await db.update(issues).set({ status: "done" }).where(eq(issues.id, blockerId)); + await db.update(issues).set({ executionRunId: replacementId }).where(eq(issues.id, issueId)); + await db.update(heartbeatRuns).set({ status: "running" }).where(eq(heartbeatRuns.id, replacementId)); + const result = await adapter.dispatchResolvedInteractionIfCurrent({ companyId, runId: replacementId, expectedStatus: "running", now: new Date(), dispatch }); + expect(result).toMatchObject(mode === "resolved" ? { dispatched: true } : { + dispatched: false, cancellation: { outcome: "cancelled", errorCode: "issue_dependencies_blocked" }, + }); + } + expect(dispatch).toHaveBeenCalledTimes(mode === "resolved" ? 1 : 0); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.status).toBe("in_progress"); + }); + it("commits the handoff without awaiting a recovered provider that fails before spawning", async () => { const { companyId, agentId } = await seedCompanyAndAgent(); const issueId = randomUUID(); diff --git a/server/src/modules/run-dispatch/adapters/postgres.ts b/server/src/modules/run-dispatch/adapters/postgres.ts index d71571c077..036b247754 100644 --- a/server/src/modules/run-dispatch/adapters/postgres.ts +++ b/server/src/modules/run-dispatch/adapters/postgres.ts @@ -545,11 +545,21 @@ export function createPostgresRunDispatchAdapter( .then((rows) => Boolean(rows[0])) : false; + const retryReasonKind = classifyRetryReasonKind(retryReason); + // Dependency edges can change after scheduled promotion without changing + // the displayed status. Read them again under the queued/final issue lock. + const readiness = issue && retryReasonKind === "native_safe_replacement" + ? (await issueService(dbOrTx).listDependencyReadiness(input.companyId, [issueId])).get(issueId) + : null; return { runId: input.runId, runAgentId: input.agentId, issueId, - retryReasonKind: classifyRetryReasonKind(retryReason), + retryReasonKind, + dependenciesBlocked: readiness && !readiness.isDependencyReady ? { + unresolvedBlockerIssueIds: readiness.unresolvedBlockerIssueIds, + unresolvedBlockerCount: readiness.unresolvedBlockerCount, + } : null, issueFound: issue !== null, issueStatus: issue?.status ?? null, issueAssigneeAgentId: issue?.assigneeAgentId ?? null, diff --git a/server/src/modules/run-dispatch/domain/policy.test.ts b/server/src/modules/run-dispatch/domain/policy.test.ts index d797a096bc..eeb4035d6b 100644 --- a/server/src/modules/run-dispatch/domain/policy.test.ts +++ b/server/src/modules/run-dispatch/domain/policy.test.ts @@ -456,6 +456,21 @@ describe("decideQueuedRunStaleness", () => { }); describe("native replacement execution authority", () => { + it("rejects unresolved dependencies added after replacement promotion", () => { + expect(decideQueuedRunStaleness({ ...baseStalenessFacts(), retryReasonKind: "native_safe_replacement", + dependenciesBlocked: { unresolvedBlockerIssueIds: ["blocker"], unresolvedBlockerCount: 1 } }, NOW)) + .toMatchObject({ stale: true, errorCode: "issue_dependencies_blocked" }); + expect(decideQueuedRunStaleness({ ...baseStalenessFacts(), retryReasonKind: "native_safe_replacement", dependenciesBlocked: null }, NOW)) + .toEqual({ stale: false }); + }); + + it("rejects a task blocked after safe replacement was scheduled", () => { + expect(decideScheduledRetryGate({ ...baseGateFacts(), retryReasonKind: "native_safe_replacement", issueStatus: "blocked" }, NOW)) + .toMatchObject({ allowed: false, errorCode: "issue_blocked" }); + expect(decideQueuedRunStaleness({ ...baseStalenessFacts(), retryReasonKind: "native_safe_replacement", issueStatus: "blocked" }, NOW)) + .toMatchObject({ stale: true, errorCode: "issue_blocked" }); + }); + it.each([ { issueExecutionRunId: "newer-run", issueCheckoutRunId: null }, { issueExecutionRunId: null, issueCheckoutRunId: "newer-run" }, diff --git a/server/src/modules/run-dispatch/domain/policy.ts b/server/src/modules/run-dispatch/domain/policy.ts index e591cbc0b9..f8351553db 100644 --- a/server/src/modules/run-dispatch/domain/policy.ts +++ b/server/src/modules/run-dispatch/domain/policy.ts @@ -57,6 +57,7 @@ export type ScheduledRetryGateErrorCode = | "issue_terminal_status" | "issue_not_in_progress" | "issue_execution_lock_changed" + | "issue_blocked" | "issue_review_participant_changed" | "issue_paused" | "issue_dependencies_blocked" @@ -107,11 +108,13 @@ export type ScheduledRetryFacts = { export type QueuedRunStalenessErrorCode = | "execution_reconciliation_required" + | "issue_dependencies_blocked" | "issue_not_found" | "issue_assignee_changed" | "issue_terminal_status" | "issue_not_in_progress" | "issue_execution_lock_changed" + | "issue_blocked" | "issue_review_participant_changed" | "issue_continuation_waiting_on_review"; @@ -125,6 +128,8 @@ export type StalenessDecision = }; export type QueuedRunFacts = { + /** Rechecked for automatic native replacements immediately before dispatch. */ + dependenciesBlocked?: DependencyBlockFacts | null; runId: string; runAgentId: string; issueId: string; @@ -353,6 +358,12 @@ export function decideScheduledRetryGate( }; } + if (facts.retryReasonKind === "native_safe_replacement" && facts.issueStatus === "blocked") { + return { allowed: false, issueId: facts.issueId, errorCode: "issue_blocked", + reason: "Scheduled replacement suppressed because the task was blocked after recovery", + details: { issueId: facts.issueId } }; + } + if (facts.retryReasonKind === "native_safe_replacement" && [facts.issueExecutionRunId, facts.issueCheckoutRunId].some(id => id != null && id !== facts.runId)) { return { allowed: false, issueId: facts.issueId, errorCode: "issue_execution_lock_changed", @@ -569,6 +580,20 @@ export function decideQueuedRunStaleness( }; } + if (facts.retryReasonKind === "native_safe_replacement" && facts.dependenciesBlocked) { + return { stale: true, errorCode: "issue_dependencies_blocked", + reason: "Cancelled because issue dependencies became blocked before replacement dispatch", + details: { issueId: facts.issueId, + unresolvedBlockerIssueIds: facts.dependenciesBlocked.unresolvedBlockerIssueIds, + unresolvedBlockerCount: facts.dependenciesBlocked.unresolvedBlockerCount } }; + } + + if (facts.retryReasonKind === "native_safe_replacement" && facts.issueStatus === "blocked") { + return { stale: true, errorCode: "issue_blocked", + reason: "Cancelled because the task was blocked before replacement dispatch", + details: { issueId: facts.issueId } }; + } + if (facts.retryReasonKind === "native_safe_replacement" && [facts.issueExecutionRunId, facts.issueCheckoutRunId].some(id => id != null && id !== facts.runId)) { return { stale: true, errorCode: "issue_execution_lock_changed", diff --git a/server/src/modules/wake-queue/adapters/postgres.ts b/server/src/modules/wake-queue/adapters/postgres.ts index efbcec9e1e..e22e2caeb9 100644 --- a/server/src/modules/wake-queue/adapters/postgres.ts +++ b/server/src/modules/wake-queue/adapters/postgres.ts @@ -1,3 +1,4 @@ +import { isAcknowledgedNativeStop } from "../../../services/acknowledged-native-stop.js"; import { instanceSettingsService } from "../../../services/instance-settings.js"; import { currentConversationCommentCondition } from "../../../services/agent-conversations.js"; import { getExecutionBlocker } from "../../../services/execution-blocker.js"; @@ -676,7 +677,7 @@ async function recordNativeTerminalRecoveryIfNeeded(tx: Db, run: HeartbeatRunRow ["failed", "timed_out", "interrupted", "cancelled"].includes(run.status) && issue.assigneeAgentId === run.agentId && !["done", "cancelled"].includes(issue.status); - if (!applies) return false; + if (!applies || isAcknowledgedNativeStop(run)) return false; const existing = await tx .select({ id: issueRecoveryActions.id }) @@ -1042,7 +1043,7 @@ export function createPostgresWakeQueueAdapter(db: Db, deps: WakeQueuePostgresAd // queues a run. executionCancellationAcknowledged: run.status === "cancelled" && - parseObject(run.resultJson?.executionCancellation).state === "acknowledged" && + (parseObject(run.resultJson?.executionCancellation).state === "acknowledged" || isAcknowledgedNativeStop(run)) && !interruptedQueue, }; const preDrain = decidePreDrain(preDrainFacts); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 81ed975078..1535317caf 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -12652,6 +12652,7 @@ export function issueRoutes( : null; const { comment: commentBody, + commentClientRequestId, attachmentIds: commentAttachmentIds, reviewInteractionId: requestedReviewInteractionId, reviewRequest, @@ -13513,6 +13514,7 @@ export function issueRoutes( }, { attachmentIds: commentAttachmentIds, + clientRequestId: actor.actorType === "user" ? commentClientRequestId : undefined, authorizationReason: issueMutationAuthorizationReason, sourceTrust: attachmentCommentSourceTrust, }, @@ -14117,6 +14119,7 @@ export function issueRoutes( }, { authorizationReason: issueMutationAuthorizationReason, + clientRequestId: actor.actorType === "user" ? commentClientRequestId : undefined, sourceTrust: await sourceTrustForActorWrite(issue, actor), }, ); @@ -17442,6 +17445,7 @@ export function issueRoutes( presentation: commentPresentation, metadata: req.body.metadata ?? null, attachmentIds: req.body.attachmentIds, + clientRequestId: actor.actorType === "user" ? req.body.clientRequestId : undefined, sourceTrust, }; let txResult: { @@ -17553,6 +17557,7 @@ export function issueRoutes( presentation: commentPresentation, metadata: req.body.metadata ?? null, attachmentIds: req.body.attachmentIds, + clientRequestId: actor.actorType === "user" ? req.body.clientRequestId : undefined, authorizationReason: commentAuthorizationReason, sourceTrust: await sourceTrustForActorWrite(currentIssue, actor), }; diff --git a/server/src/services/acknowledged-native-stop.test.ts b/server/src/services/acknowledged-native-stop.test.ts new file mode 100644 index 0000000000..a46a78a634 --- /dev/null +++ b/server/src/services/acknowledged-native-stop.test.ts @@ -0,0 +1,17 @@ +import { expect, it } from "vitest"; +import { isAcknowledgedNativeStop } from "./acknowledged-native-stop.js"; +const run = { id: "run", companyId: "company", nativeIssueId: "issue", status: "cancelled", resultJson: { + cancelledByActorType: "user", cancelledByUserId: "board", nativeCancellation: { + schema: "paperclip.native-cancellation.v1", runId: "run", companyId: "company", issueId: "issue", + scope: "run", reasonCode: "cancellation_run_only", dispatchState: "acknowledged", dispatched: true, + intentAuditId: "intent", acknowledgementAuditId: "ack", + }, +} }; +it("recognizes the native receipt used by the Stop button", () => { + expect(isAcknowledgedNativeStop(run)).toBe(true); +}); +it.each([{ runId: "other" }, { companyId: "other" }, { issueId: "other" }, { dispatchState: "pending" }, + { scope: "subtree" }, { acknowledgementAuditId: undefined }])("refuses unrelated or incomplete receipts %j", change => { + expect(isAcknowledgedNativeStop({ ...run, resultJson: { ...run.resultJson, + nativeCancellation: { ...run.resultJson.nativeCancellation, ...change } } })).toBe(false); +}); diff --git a/server/src/services/acknowledged-native-stop.ts b/server/src/services/acknowledged-native-stop.ts new file mode 100644 index 0000000000..17b55693ac --- /dev/null +++ b/server/src/services/acknowledged-native-stop.ts @@ -0,0 +1,19 @@ +/** A server-recorded run-only Stop must not manufacture a recovery incident. */ +export function hasAcknowledgedNativeStopIntent(run: { + id: string; companyId: string; status: string; nativeIssueId: string | null; + resultJson: Record | null; +}): boolean { + const result = run.resultJson; + const intent = result?.nativeCancellation as Record | undefined; + return result?.cancelledByActorType === "user" && + typeof result.cancelledByUserId === "string" && Boolean(result.cancelledByUserId) && + intent?.schema === "paperclip.native-cancellation.v1" && intent.runId === run.id && + intent.companyId === run.companyId && intent.issueId === run.nativeIssueId && + intent.scope === "run" && intent.reasonCode === "cancellation_run_only" && + intent.dispatchState === "acknowledged" && intent.dispatched === true && + typeof intent.intentAuditId === "string" && typeof intent.acknowledgementAuditId === "string"; +} + +export function isAcknowledgedNativeStop(run: Parameters[0]): boolean { + return run.status === "cancelled" && hasAcknowledgedNativeStopIntent(run); +} diff --git a/server/src/services/execution-recovery-resolution.ts b/server/src/services/execution-recovery-resolution.ts index a1ea4c78c0..e37aa0a66c 100644 --- a/server/src/services/execution-recovery-resolution.ts +++ b/server/src/services/execution-recovery-resolution.ts @@ -473,8 +473,9 @@ export async function settleUnrecoverableExecutions( const note = current ? "Automatic recovery stopped. Recorded work is preserved; actions with unverified outcomes will not be repeated." : "Recovery closed because the task's owner, execution, or status changed. No work was replayed."; - if (current) - await tx + let nativeFailureBlock = action.evidence.nativeFailureBlock; + if (current) { + const [projected] = await tx .update(issues) .set({ status: "blocked", @@ -482,7 +483,13 @@ export async function settleUnrecoverableExecutions( checkoutRunId: null, updatedAt: now, }) - .where(eq(issues.id, task.id)); + .where(eq(issues.id, task.id)).returning(); + // Only a transition owned by this failure grants a recovery receipt. + // An already-blocked task may have a separate human/dependency hold. + if (task.status !== "blocked" && run.runtimeMode === "native") { + nativeFailureBlock = { runId: run.id, statusVersion: projected!.statusVersion }; + } + } await tx .update(issueRecoveryActions) .set({ @@ -496,6 +503,7 @@ export async function settleUnrecoverableExecutions( monitorPolicy: null, evidence: { ...action.evidence, + ...(nativeFailureBlock ? { nativeFailureBlock } : {}), automaticRecovery: { policy: "preserve_without_replay_v1", runId: run.id, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 5bc978c4e4..af2c68c82f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -3,6 +3,7 @@ import { measureSandboxOperation, runWithSandboxPerformanceTrace, setSandboxPerf import { startNativeGitHubCallbackBridge } from "./native-github-bridge.js"; import { AGENT_CHAT_DIRECTIVE, conversationReplay, isConversation, isConversationExecutionWake, isWaitingConversation, prepareConversationTurn, settleConversationTurn } from "./agent-conversations.js"; import { PROCESS_IDENTITY_RECORDED, recordNativeLocalProcessStop } from "./native-local-process-stop.js"; +import { hasAcknowledgedNativeStopIntent } from "./acknowledged-native-stop.js"; import { legacyControllerBootId, legacyControllerClaim, renewLegacyControllerLease, hasLiveLegacyController, revokeExpiredLegacyController, watchLegacyControllerLease } from "./legacy-controller-lease.js"; import { completeTerminatedRemoteNativeSessionCleanup } from "../vendor/paperclip-runner/index.js"; import { hasRemoteTerminationReceipt, remoteExecutionHasStopped, remoteTerminationReceipt, stoppedRemoteCleanupScopes } from "./remote-execution-termination.js"; @@ -201,6 +202,7 @@ import { cancelNativeSession, claimNativeRestartRecoveries, closeWarmNativeSessionsForEnvironment, + closeIdleWarmNativeSessionsForRestart, currentNativeControllerIdentity, dispatchNativeSessionResumptions, detachNativeSessionsForRestart, @@ -216,6 +218,7 @@ import { materializeNativeInteractionResponses, nativeCompletionRequestsForComments, NativeCancellationPendingRecoveryError, + NativeControllerDetachedForRestartError, nativeToolContractFingerprintForTarget, prepareNativeSessionBootstrapPersistence, prepareNativeWorkspaceSync, @@ -10266,6 +10269,7 @@ export function heartbeatService( if (!agent || agent.companyId !== companyId || agent.adapterType === "paperclip_runner") return; const [active] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and( eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.agentId, wake.agentId), sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueId}`, inArray(heartbeatRuns.status, ["running", "queued", "scheduled_retry"]), )).limit(1); @@ -10287,6 +10291,7 @@ export function heartbeatService( !current || !queuedCommentIdsFromWakePayload(current.payload).length) return null; const [successor] = await tx.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and( eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.agentId, wake.agentId), sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueId}`, inArray(heartbeatRuns.status, ["running", "queued", "scheduled_retry"]), )).limit(1); @@ -12818,6 +12823,10 @@ export function heartbeatService( else if (issueStatus === "cancelled") terminalStatus = "cancelled"; } + // Teardown can beat the cancellation finalizer. Preserve the acknowledged + // user intent instead of reporting an infrastructure interruption. + if (hasAcknowledgedNativeStopIntent(run)) terminalStatus = "cancelled"; + const message = `run terminalized on environment lease release: heartbeat_runs.status was still ${run.status} at teardown`; // Match both "running" and "queued". A queued run has released its lease but // never reached "running", so a running-only update would miss it and leave @@ -14295,6 +14304,10 @@ export function heartbeatService( now = new Date(), ) { shutdownInProgress = true; + const idleSessions = await closeIdleWarmNativeSessionsForRestart(); + if (idleSessions.failed > 0) { + logger.warn({ idleSessions }, "idle native sessions could not checkpoint before controller shutdown"); + } let intent: Awaited>; try { intent = await readHotRestartIntent(); @@ -14928,6 +14941,11 @@ export function heartbeatService( run.runtimeMode === "native" && agent.adapterType === "paperclip_runner" ) { + // A graceful shutdown relinquishes controller authority just like a + // hot restart. Leaving the old event consumer attached lets its + // finalizer interrupt/suspend Claude while the next server is adopting + // the same turn. + await detachNativeSessionsForRestart([run.id]); const recoveryHistoryEntry = JSON.stringify({ at: now.toISOString(), restartKind: "graceful", @@ -21441,7 +21459,14 @@ export function heartbeatService( companyId: agent.companyId, issueId, runId: run.id, agentId: agent.id, workspaceId: workspace.id, }))); } else { - await measureSandboxOperation("heartbeat.issues_svc.update", { operationIndex: 69 }, async () => (issuesSvc.update(issueId, nextIssuePatch))); + await measureSandboxOperation("heartbeat.issues_svc.update", { operationIndex: 69 }, async () => (issuesSvc.update( + issueId, + { ...nextIssuePatch, companyGuard: agent.companyId }, + db, + undefined, + undefined, + { bindRuntimeSharedWorkspace: reusableSandboxExecutionWorkspace && workspace.mode === "shared_workspace" }, + ))); } issueExecutionWorkspaceIdForRun = workspace.id; issueProjectWorkspaceIdForRun = @@ -24091,6 +24116,12 @@ export function heartbeatService( } } catch (adapterErr) { if (adapterErr instanceof NativeCancellationPendingRecoveryError) throw adapterErr; + if (adapterErr instanceof NativeControllerDetachedForRestartError) { + // Preserve the provider and its run for the new controller. This + // also keeps generic teardown from terminalizing/releasing its lease. + nativeSessionResumeScheduled = true; + throw adapterErr; + } if (adapterErr instanceof NativeRunnerOwnershipUnverifiedError) { nativeOwnershipHeld = true; throw adapterErr; @@ -24951,6 +24982,10 @@ export function heartbeatService( wasFirstHeartbeat: timerClaimWasFirstHeartbeat(run), }))); } catch (err) { + if (err instanceof NativeControllerDetachedForRestartError) { + nativeSessionResumeScheduled = true; + return; + } if (err instanceof NativeRunnerOwnershipUnverifiedError) { nativeOwnershipHeld = true; const heldRun = await measureSandboxOperation("heartbeat.get_run", { operationIndex: 232 }, async () => (getRun(run.id))); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index c74483e250..b3d297bdf4 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -10473,6 +10473,7 @@ export function issueService(db: Db) { dbOrTx: any = db, postCommitActivityPublications?: ActivityPublication[], postCommitActions?: IssuePostCommitAction[], + options: { bindRuntimeSharedWorkspace?: boolean } = {}, ) => { const ownedActivityPublications: ActivityPublication[] = []; const activityPublications = @@ -10543,7 +10544,30 @@ export function issueService(db: Db) { const isolatedWorkspacesEnabled = ( await instanceSettings.getExperimental() ).enableIsolatedWorkspaces; - if (!isolatedWorkspacesEnabled) { + if (options.bindRuntimeSharedWorkspace) { + const workspaceId = issueData.executionWorkspaceId ?? existing.executionWorkspaceId; + if (!workspaceId) { + throw unprocessable("Runtime workspace binding requires an existing shared workspace"); + } + const [workspace] = await dbOrTx + .select({ mode: executionWorkspaces.mode }) + .from(executionWorkspaces) + .where(and( + eq(executionWorkspaces.id, workspaceId), + eq(executionWorkspaces.companyId, existing.companyId), + )); + if ( + workspace?.mode !== "shared_workspace" || + (issueData.executionWorkspacePreference ?? existing.executionWorkspacePreference) !== "reuse_existing" || + (issueData.executionWorkspaceSettings ?? existing.executionWorkspaceSettings)?.mode !== "shared_workspace" + ) { + throw unprocessable("Runtime workspace binding requires an existing shared workspace"); + } + } + // Warm sandbox continuity is runtime bookkeeping, independent of the + // opt-in UI for creating isolated worktrees. Public updates still obey + // the feature gate; only the internal shared-workspace binding bypasses it. + if (!isolatedWorkspacesEnabled && !options.bindRuntimeSharedWorkspace) { delete issueData.executionWorkspaceId; delete issueData.executionWorkspacePreference; delete issueData.executionWorkspaceSettings; @@ -10795,6 +10819,15 @@ export function issueService(db: Db) { projectGoalId: nextProjectGoalId, defaultGoalId: defaultCompanyGoal?.id ?? null, }); + // Reasserting Blocked or changing its blockers is a fresh decision even + // when the status string stays the same. Invalidate recovery's prior + // status receipt without treating comment recency as blocking intent. + if (receiptExisting.status === "blocked" && + (issueData.status === "blocked" || + (issueData.status === undefined && + (blockedByIssueIds !== undefined || issueData.unblockDescriptor !== undefined)))) { + patch.statusVersion = sql`${issues.statusVersion} + 1` as unknown as number; + } const updated = await tx .update(issues) .set(patch) diff --git a/server/src/services/native-local-process-stop.ts b/server/src/services/native-local-process-stop.ts index da8b952252..3c82cddeb3 100644 --- a/server/src/services/native-local-process-stop.ts +++ b/server/src/services/native-local-process-stop.ts @@ -58,3 +58,18 @@ export async function hasNativeLocalProcessStop(db: Db, companyId: string, runId .limit(1); return event?.eventType === LOCAL_PROCESS_STOPPED; } + +/** Recover the exact stopped identity after the mutable run fields were cleared. */ +export async function readNativeLocalProcessStop(db: Db, companyId: string, runId: string) { + const [event] = await db.select({ eventType: heartbeatRunEvents.eventType, payload: heartbeatRunEvents.payload }) + .from(heartbeatRunEvents) + .where(and(eq(heartbeatRunEvents.companyId, companyId), eq(heartbeatRunEvents.runId, runId), + isNull(heartbeatRunEvents.sourceEventId), + inArray(heartbeatRunEvents.eventType, [PROCESS_START_REQUESTED, PROCESS_IDENTITY_RECORDED, LOCAL_PROCESS_STOPPED]))) + .orderBy(desc(heartbeatRunEvents.seq)).limit(1); + const pid = event?.payload?.processPid; + const group = event?.payload?.processGroupId; + if (event?.eventType !== LOCAL_PROCESS_STOPPED || typeof pid !== "number" || + !Number.isSafeInteger(pid) || pid <= 1 || group !== pid || !absent(pid) || !absent(-pid)) return null; + return { processPid: pid, processGroupId: pid }; +} diff --git a/server/src/services/native-runtime/native-board-response-wait.ts b/server/src/services/native-runtime/native-board-response-wait.ts index da8f3c74e8..87ed7e2b51 100644 --- a/server/src/services/native-runtime/native-board-response-wait.ts +++ b/server/src/services/native-runtime/native-board-response-wait.ts @@ -74,7 +74,7 @@ export async function readNativeBoardResponseWaitOrigin( eq(agentWakeupRequests.agentId, binding.agentId), eq(agentWakeupRequests.runId, binding.runId), eq(agentWakeupRequests.source, "automation"), - eq(agentWakeupRequests.reason, "issue_commented"), + inArray(agentWakeupRequests.reason, ["issue_commented", "issue_reopened_via_comment"]), eq(agentWakeupRequests.requestedByActorType, "user"), ), ) @@ -203,7 +203,7 @@ export async function readNativeBoardResponseWaitSource( eq(agentWakeupRequests.agentId, binding.agentId), eq(agentWakeupRequests.runId, binding.runId), eq(agentWakeupRequests.source, "automation"), - eq(agentWakeupRequests.reason, "issue_commented"), + inArray(agentWakeupRequests.reason, ["issue_commented", "issue_reopened_via_comment"]), eq(agentWakeupRequests.requestedByActorType, "user"), ), ) diff --git a/server/src/services/native-runtime/native-execution-input.test.ts b/server/src/services/native-runtime/native-execution-input.test.ts index 53db2612bf..f93507c8cd 100644 --- a/server/src/services/native-runtime/native-execution-input.test.ts +++ b/server/src/services/native-runtime/native-execution-input.test.ts @@ -451,4 +451,26 @@ describe("native execution input external-chat framing", () => { } }, ); + it("gives ordinary Board tasks the same durable-question guidance as external chat", () => { + const input = buildNativeExecutionInput({ + companyId: "10000000-0000-4000-8000-000000000001", + runId: "50000000-0000-4000-8000-000000000005", + agentId: "30000000-0000-4000-8000-000000000003", + issue: { id: "20000000-0000-4000-8000-000000000002", identifier: "QA-1", title: "Welcome", description: null, workMode: "standard" }, + taskPrompt: "Ask whether the welcome should sound warm or formal before writing it.", + workspace: { id: "40000000-0000-4000-8000-000000000004", cwd: "/workspace", repoUrl: null, repoRef: null, branchName: null }, + normalizedSessionId: null, + provider: "codex", + completionContract: { + id: "70000000-0000-4000-8000-000000000007", sha256: `sha256:${"a".repeat(64)}`, schemaVersion: "paperclip.run-result.v1", + contract: { revision: "1", objective: "Write a welcome after the user's answer", criteria: [{ id: "objective", requirement: "Use the selected tone" }] }, + }, + runtimeContext: nativeRuntimeContextFixture(), + }); + expect(input.task.prompt).toContain('interactionKind="questions"'); + expect(input.task.prompt).toContain('continuationPolicy="wake_assignee"'); + expect(input.task.prompt).toContain("Create the actual question before yielding"); + expect(input.task.prompt).toContain("Wait for its real answer"); + }); + }); diff --git a/server/src/services/native-runtime/native-execution-input.ts b/server/src/services/native-runtime/native-execution-input.ts index c5e7e05c57..f027ca28a4 100644 --- a/server/src/services/native-runtime/native-execution-input.ts +++ b/server/src/services/native-runtime/native-execution-input.ts @@ -19,13 +19,14 @@ import { renderPaperclipWakePrompt, } from "@paperclipai/adapter-utils/server-utils"; -const NATIVE_EXTERNAL_CHAT_QUESTION_GUIDANCE = [ - "## Native external-chat questions", +const NATIVE_QUESTION_GUIDANCE = [ + "## Questions that need a user response", "A request for clickable choices, buttons, or a decision needed before continuing is not a self-contained text answer. The zero-API-call shortcut does not prohibit the structured question tool.", 'Use the available request_human_input tool with interactionKind="questions", continuationPolicy="wake_assignee", a title, prompt, and a stable idempotencyKey. Put the actual requested choices in payload.questions: each question needs an id, prompt, selectionMode="single", and options with stable id and label fields. Reuse the same key if that creation call must be retried.', "Paperclip renders the supported question controls and authenticates the answer. Never fabricate answer URLs, query-string choice links, callback tokens, or fake Markdown buttons. Do not manually post a duplicate question card or use call_api as a substitute.", "For one question at a time, read the current request and authoritative prior answers, then create only the next unanswered question. Wait for its real answer before asking another; do not infer a selection or answer your own interaction. Keep completion and disposition truthful while waiting, and preserve existing review or approval gates.", "If the tool is unavailable or creation fails, report that actual limitation plainly; do not pretend interactive controls were created.", + "A completion summary saying that you asked a question does not create a question. Create the actual question before yielding; never claim to be waiting for a response to an interaction you have not created.", ].join("\n"); const NATIVE_GITHUB_ATTACHMENT_RECOVERY_GUIDANCE = [ @@ -165,7 +166,7 @@ export function buildNativeExecutionInput(input: { isPaperclipExternalChatQuestionResponseTurn(wakePayload); const taskPrompt = [ wakePrompt, - externalChatTurn ? NATIVE_EXTERNAL_CHAT_QUESTION_GUIDANCE : "", + NATIVE_QUESTION_GUIDANCE, externalChatTurn && wake?.externalChatProvider === "github" ? NATIVE_GITHUB_ATTACHMENT_RECOVERY_GUIDANCE : "", diff --git a/server/src/services/native-runtime/native-finalization-reconciler.ts b/server/src/services/native-runtime/native-finalization-reconciler.ts index 8559050d73..df0d3f88fc 100644 --- a/server/src/services/native-runtime/native-finalization-reconciler.ts +++ b/server/src/services/native-runtime/native-finalization-reconciler.ts @@ -481,7 +481,7 @@ export async function claimNativeSessionResumptions(input: { terminalRunToEmit = updatedRun ?? null; await issueService(tx as unknown as Db).update( row.coordinator.issueId, - { status: "in_review" }, + { status: "blocked" }, tx, ); await issueRecoveryActionService(tx as unknown as Db).upsertSourceScoped({ diff --git a/server/src/services/native-runtime/native-provider-capacity.integration.test.ts b/server/src/services/native-runtime/native-provider-capacity.integration.test.ts index eda8ee2851..ee32b33e81 100644 --- a/server/src/services/native-runtime/native-provider-capacity.integration.test.ts +++ b/server/src/services/native-runtime/native-provider-capacity.integration.test.ts @@ -288,7 +288,7 @@ describe("native provider capacity failure persistence", () => { .select() .from(issues) .where(eq(issues.id, issueId)); - expect(waitingIssue.status).toBe("in_review"); + expect(waitingIssue.status).toBe("blocked"); const events = await db .select() .from(heartbeatRunEvents) diff --git a/server/src/services/native-runtime/native-safe-replacement.test.ts b/server/src/services/native-runtime/native-safe-replacement.test.ts index e4dca02062..480a156ff3 100644 --- a/server/src/services/native-runtime/native-safe-replacement.test.ts +++ b/server/src/services/native-runtime/native-safe-replacement.test.ts @@ -1,5 +1,6 @@ import { createRunDispatch, deriveCommentId } from "../../modules/run-dispatch/index.js"; import { buildExecutionContinuation } from "../execution-continuation.js"; +import { issueService } from "../issues.js"; import { activityService } from "../activity.js"; import { buildPaperclipWakePayload, heartbeatService } from "../heartbeat.js"; import { legacyExecutionNeedsReconciliation, terminalizeLegacyExecution } from "../legacy-execution-recovery.js"; @@ -12,9 +13,10 @@ import { deliverReconciledExecutions, } from "../execution-recovery-resolution.js"; import { randomUUID } from "node:crypto"; +import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; import { tmpdir } from "node:os"; import { and, eq, inArray } from "drizzle-orm"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, companies, @@ -107,6 +109,77 @@ const support = externalDatabaseUrl }); return { companyId, agentId, issueId, runId }; } + it.each(["unproven", "changed", "verified"] as const)("requires stopped-session proof through commit (%s)", async (mode) => { + const source = await seed(2); + await db.update(nativeRunFinalizations).set({ failureCode: "native_session_cleanup_quarantined" }).where(eq(nativeRunFinalizations.runId, source.runId)); + const [projected] = await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, source.issueId)).returning(); + await db.insert(issueRecoveryActions).values({ companyId: source.companyId, sourceIssueId: source.issueId, + kind: "active_run_watchdog", cause: "native_session_cleanup_quarantined", fingerprint: source.runId, + ownerType: "board", returnOwnerAgentId: source.agentId, status: "resolved", outcome: "blocked", + evidence: { runId: source.runId, nativeFailureBlock: { runId: source.runId, statusVersion: projected!.statusVersion }, automaticRecovery: { replay: "blocked" } }, nextAction: "Automatic recovery stopped.", + }); + const retire = vi.fn(() => mode === "verified"); + const verifyStoppedSession = vi.fn(async (run: typeof heartbeatRuns.$inferSelect) => + run.id === source.runId && mode !== "unproven" ? { evidence: { completedTaskControlCallIds: ["completion"] }, retire } : null); + await appendHeartbeatRunEvent(db, { companyId: source.companyId, runId: source.runId, agentId: source.agentId, + eventType: "tool.execution.started", stream: "system", + payload: { name: "paperclip_finish", executionId: "completion", transport: "process" }, + }); + await reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession }); + await reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession }); + const successors = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, source.runId)); + expect(successors).toHaveLength(mode === "verified" ? 1 : 0); + const [hold] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, source.issueId)); + if (mode === "verified") { + expect(retire).toHaveBeenCalledOnce(); + expect(hold).toMatchObject({ status: "resolved", outcome: "handed_back", evidence: { automaticRecovery: { replay: "verified_safe_replacement" } } }); + // Prove that the real dispatch gate accepts the successor, not only that a row exists. + const dispatch = createRunDispatch(db); + await db.update(heartbeatRuns).set({ status: "queued" }).where(eq(heartbeatRuns.id, successors[0]!.id)); + const result = await dispatch.cancelStaleQueuedRun({ companyId: source.companyId, runId: successors[0]!.id, expectedStatus: "queued", now: new Date() }); + expect(result.outcome).toBe("not_stale"); + } else { + expect(hold!.evidence.automaticRecovery).toEqual({ replay: "blocked" }); + } + }); + it.each(["missing_receipt", "other_run", "other_cause", "manual_reblock", "dependency_edit", "queued_comment"] as const)("honors blocking intent before safe replacement (%s)", async (mode) => { + const source = await seed(2); + await db.update(nativeRunFinalizations).set({ failureCode: "native_session_cleanup_quarantined" }).where(eq(nativeRunFinalizations.runId, source.runId)); + const projected = await issueService(db).update(source.issueId, { status: "blocked" }); + const evidence = { runId: source.runId, ...(mode === "missing_receipt" ? {} : { + nativeFailureBlock: { runId: mode === "other_run" ? randomUUID() : source.runId, statusVersion: projected!.statusVersion }, + }) }; + const [hold] = await db.insert(issueRecoveryActions).values({ companyId: source.companyId, sourceIssueId: source.issueId, + kind: "active_run_watchdog", cause: mode === "other_cause" ? "native_event_replay_conflict" : "native_session_cleanup_quarantined", + fingerprint: source.runId, ownerType: "board", returnOwnerAgentId: source.agentId, + status: "resolved", outcome: "blocked", evidence, nextAction: "Preserve this hold.", + }).returning(); + if (mode === "manual_reblock") await issueService(db).update(source.issueId, { status: "blocked", actorUserId: "operator" }); + if (mode === "dependency_edit") { + const blockerId = randomUUID(); + await db.insert(issues).values({ id: blockerId, companyId: source.companyId, title: "Human dependency", status: "todo" }); + await issueService(db).update(source.issueId, { blockedByIssueIds: [blockerId], actorUserId: "operator" }); + } + const comment = mode === "queued_comment" ? await issueService(db).addComment(source.issueId, "Also explain the result", { userId: "operator" }) : null; + const retire = vi.fn(() => true); + await reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession: async run => + run.id === source.runId ? { evidence: {}, retire } : null }); + const successors = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, source.runId)); + const [after] = await db.select().from(issues).where(eq(issues.id, source.issueId)); + const [afterHold] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, hold!.id)); + if (mode === "queued_comment") { + expect(successors).toHaveLength(1); + expect(after!.status).toBe("in_progress"); + expect(afterHold!.outcome).toBe("handed_back"); + expect((await db.select().from(issueComments).where(eq(issueComments.id, comment!.id)))[0]!.body).toBe("Also explain the result"); + } else { + expect(successors).toHaveLength(0); + expect(retire).not.toHaveBeenCalled(); + expect(after!.status).toBe("blocked"); + expect(afterHold!.evidence).toEqual(evidence); + expect(afterHold!.outcome).toBe("blocked"); + } + }); it("automatically closes an exhausted incident once, preserves ownership, and records no replay", async () => { const source = await seed(3); await reconcileSafeNativeReplacements(db); diff --git a/server/src/services/native-runtime/native-safe-replacement.ts b/server/src/services/native-runtime/native-safe-replacement.ts index 3ce51c649f..0f8a98f8fc 100644 --- a/server/src/services/native-runtime/native-safe-replacement.ts +++ b/server/src/services/native-runtime/native-safe-replacement.ts @@ -8,13 +8,16 @@ import { heartbeatRunEvents, heartbeatRuns, issues, + issueRecoveryActions, nativeRunFinalizations, toolInvocations, type Db, } from "@paperclipai/db"; import { decideNativeReplacement } from "./native-replacement-evidence.js"; +import { issueService } from "../issues.js"; import { issueRecoveryActionService } from "../issue-recovery-actions.js"; import { buildExecutionContinuation } from "../execution-continuation.js"; +import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; export const NATIVE_SAFE_REPLACEMENT_REASON = "native_safe_replacement"; const record = (v: unknown): Record => @@ -36,6 +39,10 @@ export async function reconcileSafeNativeReplacements( db: Db, now = new Date(), options: { + verifyStoppedSession?: (run: typeof heartbeatRuns.$inferSelect) => Promise<{ + evidence: Record; + retire: () => boolean; + } | null>; /** Test fault injection at durability boundaries; never exposed by an API. */ failpoint?: (phase: "successor_inserted" | "lineage_committed") => void; } = {}, @@ -51,9 +58,9 @@ export async function reconcileSafeNativeReplacements( and( eq(heartbeatRuns.status, "failed"), eq(nativeRunFinalizations.phase, "terminal_failure"), - eq( + inArray( nativeRunFinalizations.failureCode, - "native_provider_terminal_failed", + ["native_provider_terminal_failed", "native_session_cleanup_quarantined", "provider_transport_failed"], ), isNull(nativeRunFinalizations.resultId), sql`coalesce(${nativeRunFinalizations.failureDetail}->>'successorRunId', '') = ''`, @@ -84,6 +91,11 @@ export async function reconcileSafeNativeReplacements( ); // Teardown is still in progress. The next sweep rechecks its durable outcome. if (leases.some((lease) => lease.releasedAt === null)) continue; + const stoppedSession = coordinator.failureCode !== "native_provider_terminal_failed" + ? await options.verifyStoppedSession?.(run) ?? null : null; + // A transport label alone is not evidence. Keep inspecting these candidates + // as process cleanup and the final transcript become durable. + if (coordinator.failureCode !== "native_provider_terminal_failed" && !stoppedSession) continue; const invocations = await db .select() .from(toolInvocations) @@ -144,6 +156,9 @@ export async function reconcileSafeNativeReplacements( return [event.eventType]; if (event.eventType === "tool.execution.started") { const name = typeof p.name === "string" ? p.name : "unknown tool"; + const completedTaskControlCallIds = stoppedSession?.evidence.completedTaskControlCallIds; + if (name === "paperclip_finish" && Array.isArray(completedTaskControlCallIds) && + completedTaskControlCallIds.includes(p.executionId)) return []; const receiptedRead = invocations.some( (row) => (row.id === p.executionId || @@ -190,12 +205,12 @@ export async function reconcileSafeNativeReplacements( // A facade transport failure can conceal an authoritative protocol // rejection. A stopped process and read receipts do not resolve that. failureMeaningKnown: - typeof coordinator.failureDetail?.originalFailureCode === "string" && + Boolean(stoppedSession) || (typeof coordinator.failureDetail?.originalFailureCode === "string" && ![ "notification_transport_failed", "provider_turn_failed", "native_provider_terminal_failed", - ].includes(coordinator.failureDetail.originalFailureCode), + ].includes(coordinator.failureDetail.originalFailureCode)), predecessorFenced: coordinator.leaseOwner === null && run.status === "failed", providerStopped: @@ -209,8 +224,8 @@ export async function reconcileSafeNativeReplacements( )), historyComplete, effectInventoryComplete: - record(run.runnerProfileJson).recoveryEventInventoryVersion === 1 && - record(execution.provider).kind === "codex", + Boolean(stoppedSession) || (record(run.runnerProfileJson).recoveryEventInventoryVersion === 1 && + record(execution.provider).kind === "codex"), attempts: coordinator.attempt, invocations, apiReceipts: record(record(run.resultJson).apiToolReceipts), @@ -304,16 +319,53 @@ export async function reconcileSafeNativeReplacements( if ( !task || task.assigneeAgentId !== run.agentId || - !["in_progress", "in_review"].includes(task.status) || + !["todo", "in_progress", "in_review", "blocked"].includes(task.status) || (task.executionRunId !== null && task.executionRunId !== run.id) || (task.checkoutRunId !== null && task.checkoutRunId !== run.id) || !current || current.phase !== "terminal_failure" || + current.failureCode !== coordinator.failureCode || current.failureDetail?.successorRunId || current.failureDetail?.replacementDenied || current.attempt >= 3 ) return false; + if (task.status === "blocked") { + const failureHolds = await tx.select().from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, run.companyId), + eq(issueRecoveryActions.sourceIssueId, task.id), + eq(issueRecoveryActions.kind, "active_run_watchdog"), + eq(issueRecoveryActions.cause, current.failureCode!), + sql`${issueRecoveryActions.evidence}->>'runId' = ${run.id}`, + )).for("update"); + const ownsBlock = failureHolds.some(hold => { + const receipt = record(hold.evidence.nativeFailureBlock); + return receipt.runId === run.id && receipt.statusVersion === task.statusVersion; + }); + if (!ownsBlock) return false; + } + if (stoppedSession) { + const [currentRun] = await tx.select().from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.companyId, run.companyId), + )).for("update"); + if (!currentRun || currentRun.status !== "failed" || currentRun.runnerInstanceId !== run.runnerInstanceId || + currentRun.nativeSessionId !== run.nativeSessionId || currentRun.processPid || currentRun.processGroupId) return false; + } + if (task.status === "blocked") { + // Restore only this failure's unchanged projection. The normal issue + // service still enforces dependency readiness and assignee eligibility. + await issueService(tx as unknown as Db).update(task.id, { status: "in_progress" }, tx); + } + if (stoppedSession) { + // If the last ownership proof changes, roll back the status restoration. + if (!stoppedSession.retire()) throw new Error("native_replacement_stopped_session_changed"); + await appendHeartbeatRunEvent(tx as unknown as Db, { + companyId: run.companyId, runId: run.id, agentId: run.agentId, + eventType: "native.stopped_text_turn_verified", stream: "system", level: "info", + message: "The previous runner and provider stopped. The interrupted turn had no external actions; any completion bookkeeping has a verified receipt.", + payload: stoppedSession.evidence, + }); + } const successorRunId = randomUUID(); const dueAt = new Date(now.getTime() + 30_000); const predecessorContext = { ...record(run.contextSnapshot) }; @@ -374,6 +426,7 @@ export async function reconcileSafeNativeReplacements( .set({ failureDetail: { ...current.failureDetail, + ...(stoppedSession ? { stoppedTextTurn: stoppedSession.evidence } : {}), successorRunId, nextAction: "Continue in the linked fresh provider session after the retry delay.", @@ -395,6 +448,29 @@ export async function reconcileSafeNativeReplacements( outcome: "handed_back", resolutionNote: `Safe continuation is scheduled in run ${successorRunId}.`, }); + // A previous sweep can have resolved the UI bookkeeping while retaining + // an effective no-replay hold. Retire only this exact failure's hold in + // the same transaction as its verified successor; unrelated holds remain. + const holds = await tx.select().from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, run.companyId), + eq(issueRecoveryActions.sourceIssueId, task.id), + eq(issueRecoveryActions.kind, "active_run_watchdog"), + eq(issueRecoveryActions.cause, current.failureCode!), + sql`${issueRecoveryActions.evidence}->>'runId' = ${run.id}`, + )).for("update"); + for (const hold of holds) { + await tx.update(issueRecoveryActions).set({ + status: "resolved", outcome: "handed_back", resolvedAt: now, updatedAt: now, + wakePolicy: null, monitorPolicy: null, + nextAction: `Safe continuation is scheduled in run ${successorRunId}.`, + resolutionNote: "Verified process termination and action receipts allow a fresh session to continue.", + evidence: { ...hold.evidence, verifiedReplacement: { successorRunId, recordedAt: now.toISOString() }, + ...(hold.evidence.automaticRecovery ? { automaticRecovery: { + ...record(hold.evidence.automaticRecovery), replay: "verified_safe_replacement", successorRunId, + } } : {}), + }, + }).where(eq(issueRecoveryActions.id, hold.id)); + } await tx .update(heartbeatRuns) .set({ executionStatusDeliveryId: randomUUID() }) diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 06e4a279d9..5fd3848306 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -133,6 +133,7 @@ const state = vi.hoisted(() => ({ }), ), cancel: vi.fn(), + copyBackCodexAuth: vi.fn(), toolAuthorityDefinitions: vi.fn( async (_binding: Record) => [], ), @@ -183,6 +184,14 @@ vi.mock("../../vendor/paperclip-runner/index.js", async (importOriginal) => ({ parsePaperclipQuestionSet: (value: unknown) => value, })); +vi.mock("@paperclipai/adapter-codex-local/server", async (importOriginal) => { + const original = await importOriginal(); + // Observe ownership cleanup without replacing the real credential merge; + // actual sandbox-home copy-back tests must continue checking saved bytes. + state.copyBackCodexAuth.mockImplementation(original.copyBackCodexAuth); + return { ...original, copyBackCodexAuth: state.copyBackCodexAuth }; +}); + vi.mock("./paperclip-runner-tool-authority.js", () => ({ PaperclipRunnerToolAuthority: class { readonly binding: Record; @@ -234,10 +243,13 @@ import { cancelNativeSession, closeWarmNativeSessionsForEnvironment, closeIdleSandboxNativeSessionsForShutdown, + closeIdleWarmNativeSessionsForRestart, createGovernedWaitEventObservation, createRemoteRunnerProcessLauncher, createRunnerdBackend, executePaperclipNativeSession, + detachNativeSessionsForRestart, + NativeControllerDetachedForRestartError, getNativeSessionSteeringState, NativeSessionSteeringError, assertRemoteRunnerBuildMetadata, @@ -823,6 +835,11 @@ describe("remote provider pack manifest", () => { } else if (script.includes("for candidate in /opt/paperclip-runner/provider-pack")) { stdout = "/opt/paperclip-runner/provider-pack\n"; } else if (command.args?.[0] === "-e") { + // This fixture starts with only the image pack. The staged-first + // recovery probe must miss before it verifies and links that pack. + if (command.args[2] !== "/opt/paperclip-runner/provider-pack") { + return { exitCode: 1, signal: null, timedOut: false, stdout: "", stderr: "missing staged pack" }; + } const verified = spawnSync(process.execPath, ["-e", script, root, command.args![3]!], { encoding: "utf8" }); return { exitCode: verified.status, signal: null, timedOut: false, stdout: verified.stdout, stderr: verified.stderr }; } else if (command.command.endsWith("/node_modules/.bin/opencode") && command.args?.[0] === "--version") { @@ -4401,6 +4418,7 @@ function leaseDb( : table === heartbeatRuns ? [ { + id: boundExecution.binding.runId, agentId: boundExecution.binding.agentId, companyId: boundExecution.binding.companyId, nativeIssueId: boundExecution.binding.issueId, @@ -4561,6 +4579,22 @@ function cancellationDb(options?: { }; } +describe("native startup restart detachment", () => { + it("remembers shutdown while the session is still opening and detaches its late publication", async () => { + const restarting = structuredClone(execution); + restarting.binding.runId = "restart-during-session-open"; + const detach = vi.fn(async () => undefined); + await expect(detachNativeSessionsForRestart([restarting.binding.runId])).resolves.toMatchObject({ inactiveRunIds: [restarting.binding.runId] }); + state.execute.mockReset().mockImplementationOnce(async (options) => { + await options.onSession({ detachControllerForRestart: detach }); + expect(detach).toHaveBeenCalledOnce(); + await options.onSession(null); + throw new Error("detachment closed the old event stream"); + }); + await expect(executePaperclipNativeSession({ db: leaseDb(restarting), execution: restarting, runnerInstanceId: "runner" })).rejects.toBeInstanceOf(NativeControllerDetachedForRestartError); + }); +}); + describe("native resumed preparation timing", () => { it("keeps answered-question ingress at the run root rather than charging it to preparation", async () => { const answeredAtMs = Date.now(); @@ -5449,7 +5483,7 @@ describe("native warm session supervision", () => { expect(onGoalCheckpoint).toHaveBeenCalledOnce(); }); - it("closes an idle warm session before its remote environment is destroyed", async () => { + it.each(["environment deletion", "controller restart"])("closes an idle warm session before %s", async (shutdownKind) => { const close = vi.fn(async () => undefined); const warmExecution = { ...execution, @@ -5465,7 +5499,12 @@ describe("native warm session supervision", () => { }, } as NativeExecutionInputV1; state.execute.mockReset().mockImplementationOnce(async (options) => { - options.onSession?.({ close }); + await options.onSession?.({ close }); + await expect(closeWarmNativeSessionsForEnvironment({ + environmentId: "environment-warm-delete", + reason: "environment deleted", + })).resolves.toMatchObject({ busy: 1 }); + expect(close).not.toHaveBeenCalled(); return { result: { summary: "completed" }, terminal: { runTerminalState: "succeeded" }, @@ -5498,17 +5537,73 @@ describe("native warm session supervision", () => { reason: "environment deleted", }), ).resolves.toEqual({ closed: 0, busy: 0, failed: 0 }); - await expect( - closeWarmNativeSessionsForEnvironment({ + const closeResult = shutdownKind === "controller restart" + ? closeIdleWarmNativeSessionsForRestart() + : closeWarmNativeSessionsForEnvironment({ environmentId: "environment-warm-delete", reason: "environment deleted", - }), - ).resolves.toEqual({ closed: 1, busy: 0, failed: 0 }); + }); + await expect(closeResult).resolves.toMatchObject({ closed: 1, failed: 0 }); expect(close).toHaveBeenCalledExactlyOnceWith({ - reason: "environment deleted", + reason: shutdownKind === "controller restart" ? "controller restart" : "environment deleted", }); }); + it.each([false, true])("checkpoints a busy warm session on release after the restart sweep: checkpoint fails=%s", async (checkpointFails) => { + const checkpointError = new Error("restart checkpoint failed"); + let finishCheckpoint!: () => void; + const checkpoint = new Promise((resolve, reject) => { + finishCheckpoint = checkpointFails ? () => reject(checkpointError) : resolve; + }); + const close = vi.fn(async () => checkpoint); + const warmExecution = { + ...execution, + binding: { + ...execution.binding, + runId: "run-warm-restart-release", + executionWorkspaceId: "workspace-warm-restart-release", + }, + session: { + ...execution.session, + normalizedSessionId: "session-warm-restart-release", + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 }, + }, + } as NativeExecutionInputV1; + state.execute.mockReset().mockImplementationOnce(async (options) => { + await options.onSession?.({ close }); + await expect(closeIdleWarmNativeSessionsForRestart()).resolves.toMatchObject({ busy: 1 }); + expect(close).not.toHaveBeenCalled(); + // The active turn can finish after the shutdown sweep has passed it. + return { + result: { summary: "completed during shutdown" }, + terminal: { runTerminalState: "succeeded" }, + turnId: "turn-warm-restart-release", + normalizedSessionId: warmExecution.session.normalizedSessionId, + providerSessionId: "provider-warm-restart-release", + driverKind: "test", + driverVersion: "1", + nativeEventCount: 1, + highestContiguousSourceSeq: 1, + usage: null, + }; + }); + let settled = false; + const running = executePaperclipNativeSession({ + db: leaseDb(warmExecution), + execution: warmExecution, + runnerInstanceId: "runner-warm-restart-release", + }).then((result) => { settled = true; return result; }); + try { + await vi.waitFor(() => expect(close).toHaveBeenCalledExactlyOnceWith({ reason: "controller restart" })); + expect(settled).toBe(false); + } finally { + finishCheckpoint(); + if (checkpointFails) await expect(running).rejects.toBe(checkpointError); + else await running; + } + await expect(closeIdleWarmNativeSessionsForRestart()).resolves.toEqual({ closed: 0, busy: 0, failed: 0 }); + }); + it("preserves the active turn when a warm checkpoint resumes the same run", async () => { const stateBase = await mkdtemp( join(tmpdir(), "paperclip-warm-same-run-recovery-"), @@ -6255,6 +6350,25 @@ describe("native warm session supervision", () => { }); describe("native session bounded recovery", () => { + it("does not turn an acknowledged Stop before completion into a failure or a retry", async () => { + const updates: Array<{ table: unknown; values: Record }> = []; + const stop: Record = {}; + state.execute.mockReset().mockImplementationOnce(async () => { + Object.assign(stop, { cancelledByActorType: "user", cancelledByUserId: "board", nativeCancellation: { + schema: "paperclip.native-cancellation.v1", ...execution.binding, scope: "run", reasonCode: "cancellation_run_only", + dispatched: true, dispatchState: "acknowledged", intentAuditId: "intent", acknowledgementAuditId: "ack", + } }); + throw new Error("native_finalization_missing: session returned no semantic result"); + }); + state.upsertRecoveryAction.mockClear(); + await expect(executePaperclipNativeSession({ + db: leaseDb(execution, {}, stop, updates), execution, runnerInstanceId: "stop-before-completion", + })).rejects.toThrow("native_cancellation_pending_recovery"); + expect(updates.some(update => update.table === heartbeatRuns && update.values.status === "failed")).toBe(false); + expect(updates.some(update => update.table === nativeRunFinalizations && update.values.failureCode === "native_retry_cancelled")).toBe(true); + expect(state.upsertRecoveryAction).not.toHaveBeenCalled(); + }); + it("keeps typed integrity failure permanent even if a wrapper changes its message", () => { const failure = new NativeSessionProtocolIntegrityError( "semantic_input_digest_mismatch", @@ -6396,7 +6510,7 @@ describe("native session bounded recovery", () => { ); expect(updateIssue).toHaveBeenCalledWith( execution.binding.issueId, - { status: "in_review" }, + { status: "blocked" }, expect.anything(), ); } finally { @@ -6450,7 +6564,7 @@ describe("native session bounded recovery", () => { const failure = new NativeSessionCleanupQuarantinedError(); state.execute.mockReset().mockRejectedValueOnce(failure); state.upsertRecoveryAction.mockReset().mockResolvedValue({}); - const updateIssue = vi.fn(async () => null); + const updateIssue = vi.fn(async () => ({ status: "blocked", statusVersion: 7 })); const service = vi .spyOn(issueServiceModule, "issueService") .mockReturnValue({ update: updateIssue } as unknown as ReturnType< @@ -6482,6 +6596,7 @@ describe("native session bounded recovery", () => { expect(state.upsertRecoveryAction).toHaveBeenCalledWith( expect.objectContaining({ cause: "native_session_cleanup_quarantined", + evidence: expect.objectContaining({ nativeFailureBlock: { runId: execution.binding.runId, statusVersion: 7 } }), ownerType: "board", wakePolicy: null, nextAction: expect.stringContaining( @@ -6491,7 +6606,7 @@ describe("native session bounded recovery", () => { ); expect(updateIssue).toHaveBeenCalledWith( execution.binding.issueId, - { status: "in_review" }, + { status: "blocked" }, expect.anything(), ); } finally { @@ -6797,7 +6912,7 @@ describe("native session bounded recovery", () => { }); }); - it("escalates exhausted result-less sessions to board review instead of leaving the provider as its own owner", () => { + it("blocks exhausted result-less sessions without manufacturing a human review", () => { expect( nativeSessionRecoveryProjection({ phase: "retryable_failure", @@ -6821,7 +6936,7 @@ describe("native session bounded recovery", () => { }), ).toEqual({ exhausted: true, - issueStatus: "in_review", + issueStatus: "blocked", recoveryOwner: { kind: "board" }, recoveryActionOwnerType: "board", recoveryActionOwnerAgentId: null, @@ -6900,6 +7015,10 @@ describe("native process ownership", () => { const onSpawn = vi.fn(async () => undefined); state.createBackend.mockClear(); state.execute.mockReset().mockImplementation(async (options) => { + await options.onSessionAdmission(); + expect(updates).toContainEqual({ table: heartbeatRunEvents, values: expect.objectContaining({ + eventType: "native.process_start_requested", runId: execution.binding.runId, + }) }); await options.backend.onSpawn(processMetadata); return { result: { summary: "completed" }, @@ -6915,9 +7034,7 @@ describe("native process ownership", () => { }); const updates: Array<{ table: unknown; values: Record }> = []; state.createBackend.mockImplementationOnce((_input, options) => { - expect(updates).toContainEqual({ table: heartbeatRunEvents, values: expect.objectContaining({ - eventType: "native.process_start_requested", runId: execution.binding.runId, - }) }); + expect(updates.some(update => update.values.eventType === "native.process_start_requested")).toBe(false); return { kind: "test", onSpawn: options.onSpawn }; }); @@ -7076,6 +7193,110 @@ describe("runnerd provider runtime wiring", () => { expect(state.execute).toHaveBeenCalledOnce(); }); + it.each([ + ["open", "before-close", "local"], + ["open", "during-close", "local"], + ["recover", "before-close", "local"], + ["recover", "during-close", "local"], + ["open", "before-close", "sandbox"], + ["open", "during-close", "sandbox"], + ["recover", "before-close", "sandbox"], + ["recover", "during-close", "sandbox"], + ] as const)("preserves managed Codex credentials after %s session detachment %s (%s)", async (mode, timing, target) => { + let finishClose!: () => void; + const closing = new Promise((resolve) => { finishClose = resolve; }); + const close = vi.fn(async () => { + if (timing === "during-close") await closing; + }); + const detach = vi.fn(async () => undefined); + const rawSession = { close, detachControllerForRestart: detach }; + state.copyBackCodexAuth.mockClear(); + state.createBackend.mockReturnValueOnce({ + kind: "test", + openSession: async () => rawSession, + recoverSession: async () => ({ recovered: true, session: rawSession }), + } as never); + const remoteExecute = vi.fn(async () => ({ exitCode: 0, stdout: "", stderr: "", timedOut: false })); + const backend = await createRunnerdBackend({ + db: leaseDb(execution), + execution, + runnerInstanceId: "runner-managed-credential-detach", + managedAiCredentialHome: join(isolatedStateDirectory, "managed-home"), + ...(target === "sandbox" ? { + runnerExecutionTarget: { + kind: "remote", transport: "sandbox", remoteCwd: "/home/daytona/repos/main", + workFolderHome: "/home/daytona", environmentId: "environment", leaseId: "lease", providerKey: "daytona", + runner: { execute: remoteExecute, syncIn: vi.fn() }, + } as never, + runnerPublicUrl: "wss://paperclip.example.test", + } : {}), + }); + if (target === "sandbox") { + expect(state.createBackend.mock.calls.at(-1)![1].environment).toEqual(expect.objectContaining({ + HOME: "/home/daytona", CODEX_HOME: "/home/daytona/.codex", + })); + } + state.createBackend.mock.calls.at(-1)![1].codexTransportFactory!(); + const root = state.createTransport.mock.calls.at(-1)![0].stateDirectory!; + const authPath = join(root, "codex-home", "auth.json"); + const auth = JSON.stringify({ OPENAI_API_KEY: "fixture-managed-codex-credential" }); + await mkdir(join(root, "codex-home"), { recursive: true }); + await writeFile(authPath, auth); + const session = mode === "open" + ? await backend.openSession({} as never) + : (await backend.recoverSession!({} as never, { + signal: new AbortController().signal, + })).session!; + + if (timing === "before-close") { + await session.detachControllerForRestart!(); + await session.close({ reason: "old controller finalizer" }); + } else { + const closed = session.close({ reason: "old controller finalizer" }); + await session.detachControllerForRestart!(); + finishClose(); + await closed; + } + + expect(detach).toHaveBeenCalledOnce(); + expect(close).toHaveBeenCalledOnce(); + expect(state.copyBackCodexAuth).not.toHaveBeenCalled(); + // The detached controller must neither read nor remove the successor's + // live sandbox auth at the actual $HOME/.codex path. + expect(remoteExecute).not.toHaveBeenCalled(); + await expect(readFile(authPath, "utf8")).resolves.toBe(auth); + }); + + it("still cleans up managed Codex credentials after an owned session closes", async () => { + const close = vi.fn(async () => undefined); + state.copyBackCodexAuth.mockClear(); + state.createBackend.mockReturnValueOnce({ + kind: "test", + openSession: async () => ({ close }), + } as never); + const managedHome = join(isolatedStateDirectory, "managed-home"); + const backend = await createRunnerdBackend({ + db: leaseDb(execution), + execution, + runnerInstanceId: "runner-managed-credential-close", + managedAiCredentialHome: managedHome, + }); + state.createBackend.mock.calls.at(-1)![1].codexTransportFactory!(); + const root = state.createTransport.mock.calls.at(-1)![0].stateDirectory!; + const authPath = join(root, "codex-home", "auth.json"); + await mkdir(join(root, "codex-home"), { recursive: true }); + await writeFile(authPath, "fixture-managed-codex-credential"); + const session = await backend.openSession({} as never); + + await session.close({ reason: "completed" }); + await session.close({ reason: "repeated cleanup" }); + + expect(state.copyBackCodexAuth).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ hostAuthPath: join(managedHome, "auth.json") }), + ); + await expect(access(authPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + it("stages from the authenticated run snapshot and cleans up after the provider turn", async () => { const cleanup = vi.fn(async () => undefined); state.stageNativeRunnerWakeAttachments.mockResolvedValueOnce({ @@ -10150,10 +10371,12 @@ describe("runnerd provider runtime wiring", () => { runtimeContext: nativeRuntimeContextFixture(), } as unknown as NativeExecutionInputV1; state.createBackend.mockClear(); + const onSpawn = vi.fn(async () => undefined); await createRunnerdBackend({ db: leaseDb(acpxExecution), execution: acpxExecution, runnerInstanceId: "runner", + onSpawn, }); expect(state.createBackend).toHaveBeenCalledWith( @@ -10172,6 +10395,7 @@ describe("runnerd provider runtime wiring", () => { provider: "acpx", acpxAgent: "codex", acpxPermissionMode: "approve-reads", + onSpawn, }), ); }); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index b4d3e2bb08..8f32fcad84 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -1,6 +1,9 @@ import { copyBackCodexAuth } from "@paperclipai/adapter-codex-local/server"; import { nativeCompletionFeedback } from "./native-completion-feedback.js"; -import { PROCESS_START_REQUESTED } from "../native-local-process-stop.js"; +import { hasAcknowledgedNativeStopIntent } from "../acknowledged-native-stop.js"; +import { stoppedCodexTurnIsTextOnly } from "./stopped-codex-turn.js"; +import { prepareVerifiedRemoteProviderPack } from "./remote-provider-pack.js"; +import { readNativeLocalProcessStop, PROCESS_START_REQUESTED } from "../native-local-process-stop.js"; import { remoteLeaseCleanupScope } from "../remote-execution-termination.js"; import { resolveConnectorAssignments, isConnectorSkill } from "../connector-runtime.js"; import { @@ -61,6 +64,7 @@ import { NativeSessionCleanupQuarantinedError, NativeSessionProtocolIntegrityError, completeRetainedNativeSessionCleanup, + completeTerminatedLocalNativeSessionCleanup, acpxRuntimeSessionDirectoryName, createNativeSessionBackend, createRunnerdCodexTransport, @@ -93,6 +97,7 @@ import { eq, gt, inArray, + isNull, like, notInArray, or, @@ -186,7 +191,18 @@ export class NativeCancellationPendingRecoveryError extends Error { } } +export class NativeControllerDetachedForRestartError extends Error { + constructor() { + super("native_controller_detached_for_restart"); + this.name = "NativeControllerDetachedForRestartError"; + } +} + const activeNativeSessions = new Map(); +// Shutdown can race provider startup before onSession publishes its handle. +// Retain the request for the remainder of this controller's lifetime so that +// the late publication detaches before it can dispatch another turn. +const nativeRunsDetachingForRestart = new Set(); export async function detachNativeSessionsForRestart( runIds: readonly string[], @@ -199,6 +215,7 @@ export async function detachNativeSessionsForRestart( const inactiveRunIds: string[] = []; const unsupportedRunIds: string[] = []; for (const runId of new Set(runIds)) { + nativeRunsDetachingForRestart.add(runId); const active = activeNativeSessions.get(runId); if (!active) { inactiveRunIds.push(runId); @@ -337,6 +354,7 @@ type WarmNativeSession = { environmentId: string | null; sandbox: boolean; busy: boolean; + closeOnReleaseReason?: string; idleTimer: ReturnType | null; lastActivityAt: string; }; @@ -353,15 +371,38 @@ const warmNativeSessions = new Map(); export async function closeWarmNativeSessionsForEnvironment(input: { environmentId: string; reason: string; +}): Promise<{ closed: number; busy: number; failed: number }> { + return closeIdleWarmNativeSessions(input); +} + +/** Suspend idle owners and persist their remote backup before a controller + * exits. Active turns keep their separate authenticated restart handoff. */ +export async function closeIdleWarmNativeSessionsForRestart(): Promise<{ + closed: number; busy: number; failed: number; +}> { + return closeIdleWarmNativeSessions({ + reason: "controller restart", + closeBusyOnRelease: true, + }); +} + +async function closeIdleWarmNativeSessions(input: { + environmentId?: string; + reason: string; + closeBusyOnRelease?: boolean; }): Promise<{ closed: number; busy: number; failed: number }> { let closed = 0; let busy = 0; let failed = 0; for (const [sessionId, entry] of [...warmNativeSessions]) { - if (entry.environmentId !== input.environmentId) { + if (input.environmentId !== undefined && entry.environmentId !== input.environmentId) { continue; } if (entry.busy) { + // A busy turn can complete while another idle session is checkpointing. + // Fence that entry now so its eventual release cannot leave a new idle + // owner behind after the shutdown sweep has already passed it. + if (input.closeBusyOnRelease) entry.closeOnReleaseReason = input.reason; busy += 1; continue; } @@ -2294,6 +2335,129 @@ export function rebaseRetainedNativeCleanupProviderHome( } } +/** Prove that a crashed local Codex turn ended without external effects. No provider + * is launched and no retained state is rewritten. The successor must use a fresh + * normalized session; the old directory remains available for investigation. */ +export async function verifyStoppedNativeSessionForReplacement( + db: Db, + run: typeof heartbeatRuns.$inferSelect, +): Promise<{ evidence: Record; retire: () => boolean } | null> { + try { + if (run.runtimeMode !== "native" || run.status !== "failed" || !run.finishedAt || + !run.nativeIssueId || !run.nativeSessionId || !run.runnerInstanceId) return null; + const execution = parseNativeExecutionInput(record(run.runnerProfileJson).nativeExecutionInput); + if (execution.provider.kind !== "codex" || execution.session.driverKind !== "codex_app_server" || + execution.binding.runId !== run.id || execution.binding.companyId !== run.companyId || + execution.binding.agentId !== run.agentId || execution.binding.issueId !== run.nativeIssueId || + nativeSessionKey(execution) !== run.nativeSessionId || + record(run.runnerProfileJson).nativeToolContractFingerprint !== nativeToolContractFingerprintForTarget("local")) return null; + const scope = nativeSessionScopeKey(execution); + const idle = () => !activeNativeSessions.has(run.id) && !executingRunnerdSessionScopes.has(scope) && + !initializingSessionToolAuthorities.has(scope) && !warmNativeSessions.has(scope); + if (!idle()) return null; + const leases = await db.select().from(environmentLeases).where(and( + eq(environmentLeases.companyId, run.companyId), eq(environmentLeases.heartbeatRunId, run.id))); + if (leases.some(lease => lease.provider !== "local" || !lease.releasedAt)) return null; + const stopped = await readNativeLocalProcessStop(db, run.companyId, run.id); + if (!stopped) return null; + const root = scopedRunnerdStateRoot(execution); + const snapshot = cleanupStateSnapshot(root); + const identity = record(snapshot.control.identity); + if (!durableIdentityMatchesExecution(identity, execution) || identity.runnerInstanceId !== run.runnerInstanceId || + !["runnerInstanceId", "environmentLeaseId", "runId", "normalizedSessionId", "turnId", "itemId"].every(key => + typeof identity[key] === "string" && identity[key] && snapshot.runner[key] === identity[key]) || + snapshot.runner.lifecycle !== "ready" || snapshot.provider.lifecycle !== "turn_active" || + record(snapshot.provider.config).provider !== "codex" || record(snapshot.provider.config).cwd !== execution.workspace.cwd || + !Array.isArray(snapshot.control.committedEvents) || !Array.isArray(snapshot.control.commands)) return null; + const events = snapshot.control.committedEvents.map(entry => record(record(record(entry).envelope).payload)); + const providerEvent = events.filter(event => ["session.started", "session.resumed"].includes(String(event.eventType))).at(-1); + const provider = record(providerEvent?.payload); + const bound = (event: Record) => validatePrpEvent(event).ok && event.sourceKind === "runner" && + event.sourceInstanceId === run.runnerInstanceId && event.runId === run.id && + event.normalizedSessionId === run.nativeSessionId && event.turnId === identity.turnId && event.itemId === identity.itemId; + if (!providerEvent || !bound(providerEvent) || !cleanupProcessAbsent(provider.processId) || + provider.processId === stopped.processPid || typeof provider.providerSessionId !== "string" || + snapshot.provider.threadId !== provider.providerSessionId || + typeof snapshot.provider.activeProviderTurnId !== "string") return null; + const receipts = await db.select().from(heartbeatRunEvents).where(and( + eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id), + eq(heartbeatRunEvents.sourceInstanceId, run.runnerInstanceId), + inArray(heartbeatRunEvents.eventType, ["session.started", "session.resumed"]))).limit(2); + const receipt = receipts.length === 1 ? receipts[0] : undefined; + const durableEvent = record(record(receipt?.payload).prpEvent); + // The persisted adapter enriches identity payloads with driverSessionId. Bind + // the normalized and provider identities explicitly instead of comparing raw JSON. + const durableProvider = record(durableEvent.payload); + if (!receipt || !validatePrpEvent(durableEvent).ok || durableEvent.runId !== run.id || + durableEvent.sourceInstanceId !== run.runnerInstanceId || durableEvent.normalizedSessionId !== run.nativeSessionId || + receipt.sourceEventId !== `${run.runnerInstanceId}:${run.id}:${durableEvent.sourceSeq}` || + receipt.sourcePayloadSha256 !== nativeSha256(durableEvent) || + (durableProvider.processId !== undefined && durableProvider.processId !== provider.processId) || + (durableProvider.driverSessionId ?? durableProvider.providerSessionId) !== provider.providerSessionId) return null; + const turnId = snapshot.provider.activeProviderTurnId; + if (!snapshot.control.commands.map(record).some(command => command.type === "turn.start" && command.status === "completed" && + record(record(command.result).result).providerTurnId === turnId) || + !events.some(event => bound(event) && event.eventType === "turn.accepted" && + record(event.payload).providerTurnId === turnId && record(event.payload).providerSessionId === provider.providerSessionId)) return null; + // Completion bookkeeping may precede the final answer. Its exact accepted + // receipt is safe to preserve; arbitrary provider tools still prevent replay. + const completedTaskControlCalls: Array<{ callId: string; input: unknown }> = []; + for (const event of events.filter(event => event.eventType === "semantic_tool.input")) { + const semantic = record(record(event.payload).semantic_tool); + const correlation = record(semantic.correlation); + if (!bound(event) || semantic.operationId !== "paperclip_finish" || semantic.phase !== "input" || + typeof semantic.callId !== "string" || !validatePrpStructuredRunResult(semantic.input).ok || + correlation.runId !== run.id || correlation.normalizedSessionId !== run.nativeSessionId || + correlation.turnId !== identity.turnId || correlation.itemId !== identity.itemId || + record(semantic.content).digest !== `sha256:${nativeSha256(semantic.input)}` || + !events.some(resultEvent => { + const result = record(record(resultEvent.payload).semantic_tool); + return bound(resultEvent) && resultEvent.eventType === "semantic_tool.result" && + result.operationId === semantic.operationId && result.callId === semantic.callId && + result.outcome === "succeeded" && result.operationReceiptId === `operation_${semantic.callId}` && + canonicalJson(result.correlation) === canonicalJson(semantic.correlation); + }) || !snapshot.control.commands.map(record).some(command => { + const payload = record(command.payload); + return command.type === "semantic_tool.result" && command.status === "completed" && + payload.callId === semantic.callId && payload.operationId === semantic.operationId && + payload.sourceEventId === event.sourceEventId && payload.isError === false && + record(payload.result).success === true && canonicalJson(payload.input) === canonicalJson(semantic.input) && + canonicalJson(payload.correlation) === canonicalJson(semantic.correlation); + })) return null; + completedTaskControlCalls.push({ callId: semantic.callId, input: semantic.input }); + } + if (completedTaskControlCalls.length > 1) return null; + const pendingInventory = JSON.stringify([snapshot.runner.outbox, snapshot.provider.pendingEvents, snapshot.provider.queuedEvents]); + if (/semantic_tool\.input|mcp_app\.tool_input|runtime\.input\.requested|runtime_request\.created/.test(pendingInventory) || + events.some(event => ["mcp_app.tool_input", "runtime.input.requested", "runtime_request.created"].includes(String(event.eventType)))) return null; + if (snapshot.control.commands.map(record).some(command => command.status === "pending" && + !["turn.stop", "runner.drain", "runner.suspend"].includes(String(command.type)))) return null; + const home = cleanupProviderHomeSnapshot(resolve(root, "codex-home"), false); + const rollouts = home.entries.filter(entry => !entry.directory && entry.path.startsWith("sessions/") && + basename(entry.path).endsWith(`-${provider.providerSessionId}.jsonl`)); + if (rollouts.length !== 1 || rollouts[0]!.size > 32 * 1024 * 1024) return null; + const rolloutPath = resolve(root, "codex-home", rollouts[0]!.path); + const bytes = readBoundedNativeFile(rolloutPath, 32 * 1024 * 1024, "native_crash_inventory_unproven"); + // A partial final write is not a closed transcript. + if (!bytes.toString("utf8").endsWith("\n")) return null; + const rows = bytes.toString("utf8").trimEnd().split("\n").map(line => JSON.parse(line)); + if (!stoppedCodexTurnIsTextOnly({ rows, threadId: provider.providerSessionId, turnId, cwd: execution.workspace.cwd, completedTaskControlCalls })) return null; + const rolloutSha256 = nativeSha256(bytes.toString("utf8")); + const evidence = { schema: "paperclip.stopped_text_turn.v1", runId: run.id, nativeSessionId: run.nativeSessionId, + runnerInstanceId: run.runnerInstanceId, processPid: stopped.processPid, providerPid: provider.processId, + providerSessionId: provider.providerSessionId, providerTurnId: turnId, + stateFingerprint: snapshot.fingerprint, rolloutSha256, + completedTaskControlCallIds: completedTaskControlCalls.map(call => call.callId) }; + return { + evidence, + retire: () => idle() && cleanupProcessAbsent(stopped.processPid) && cleanupProcessAbsent(provider.processId) && + cleanupStateSnapshot(root).fingerprint === snapshot.fingerprint && + nativeSha256(readBoundedNativeFile(rolloutPath, 32 * 1024 * 1024, "native_crash_inventory_unproven").toString("utf8")) === rolloutSha256 && + completeTerminatedLocalNativeSessionCleanup({ companyId: run.companyId, runId: run.id, runnerInstanceId: run.runnerInstanceId! }), + }; + } catch { return null; } +} + /** Exact local cleanup only: the accepted result and original quarantine are * never rewritten. A failed/interrupted maintenance attempt is retained for * inspection, not retried from an older snapshot with unknown process owners. */ @@ -5539,11 +5703,15 @@ async function releaseWarmNativeSession( entry.busy = false; entry.lastActivityAt = new Date().toISOString(); if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); - if (failed) { + if (failed || entry.closeOnReleaseReason !== undefined) { warmNativeSessions.delete(sessionId); - await entry.session - .close({ reason: "warm native session failed" }) - .catch(() => undefined); + const closing = entry.session.close({ + reason: entry.closeOnReleaseReason ?? "warm native session failed", + }); + // Restart checkpointing is required to restore this successful session. + // Surface failure instead of reporting a clean release without authority. + if (entry.closeOnReleaseReason !== undefined) await closing; + else await closing.catch(() => undefined); return; } entry.idleTimer = setTimeout(() => { @@ -5613,7 +5781,7 @@ export function nativeSessionRecoveryProjection(input: { issueStatus: exhausted && input.failureCode !== NATIVE_ADOPTED_RUNNER_AUTHENTICATION_TIMEOUT - ? ("in_review" as const) + ? ("blocked" as const) : null, recoveryOwner: exhausted ? { kind: "board" as const } @@ -7785,17 +7953,6 @@ async function executePaperclipNativeSessionWithinScope( input.db, input.execution.binding, ); - // Invalidate prior stop evidence before a backend can spawn. A crash between - // spawn and the PID callback must not make an old receipt authorize a turn. - await appendHeartbeatRunEvent(input.db, { - companyId: input.execution.binding.companyId, - runId: input.execution.binding.runId, - agentId: input.execution.binding.agentId, - eventType: PROCESS_START_REQUESTED, - stream: "system", - level: "info", - message: "Native execution requested; prior local stop evidence no longer applies.", - }); const runnerdBackend = input.useRunnerd && input.backend === undefined ? await createRunnerdBackend({ @@ -7828,6 +7985,18 @@ async function executePaperclipNativeSessionWithinScope( trace.activate(runnerSessionStartupScope); const result = await trace.run(runnerSessionStartupScope, () => executeNativeSession({ + onSessionAdmission: async () => { + // Invalidate prior stop evidence before a backend can spawn. + await appendHeartbeatRunEvent(input.db, { + companyId: input.execution.binding.companyId, + runId: input.execution.binding.runId, + agentId: input.execution.binding.agentId, + eventType: PROCESS_START_REQUESTED, + stream: "system", + level: "info", + message: "Native execution requested; prior local stop evidence no longer applies.", + }); + }, input: runnerExecution, remoteCleanupScope: remoteCleanupLease ? remoteLeaseCleanupScope(remoteCleanupLease) : undefined, turnTimeoutMs: input.turnTimeoutMs, @@ -7941,7 +8110,7 @@ async function executePaperclipNativeSessionWithinScope( `[paperclip-runner] provider session continuity break: exact resume failed (${continuity.reason}); old driver session=${continuity.previousDriverSessionId}, old provider session=${continuity.previousProviderSessionId ?? "unavailable"}, replacement driver session=${continuity.replacementDriverSessionId}, replacement provider session=${continuity.replacementProviderSessionId ?? "unavailable"}\n`, ); }, - onSession: (session) => { + onSession: async (session) => { releaseRegisteredGoalController(); if (session?.goal) { releaseGoalController = registerLiveRunnerGoalController( @@ -8005,12 +8174,15 @@ async function executePaperclipNativeSessionWithinScope( warmNativeSessions.delete(warmSessionId); } } - if (session) + if (session) { activeNativeSessions.set(input.execution.binding.runId, { session, cancelRequested: false, }); - else { + if (nativeRunsDetachingForRestart.has(input.execution.binding.runId)) { + await session.detachControllerForRestart?.(); + } + } else { activeNativeSessions.delete(input.execution.binding.runId); clearSteeringDeliveries(input.execution.binding.runId); clearNativeRuntimeRequestResolutions( @@ -8050,6 +8222,14 @@ async function executePaperclipNativeSessionWithinScope( clearSteeringDeliveries(input.execution.binding.runId); clearNativeRuntimeRequestResolutions(input.execution.binding.runId); } catch (error) { + if (nativeRunsDetachingForRestart.has(input.execution.binding.runId)) { + await leaseRenewal.stop().catch(() => undefined); + activeNativeSessions.delete(input.execution.binding.runId); + // Disconnecting deliberately ends the old event consumer. It is not a + // provider failure and must not overwrite the shutdown adoption record + // with a retry or release the still-live runner's lease. + throw new NativeControllerDetachedForRestartError(); + } const protocolIntegrityFailure = error instanceof NativeSessionProtocolIntegrityError ? error : null; const ownershipUnverified = @@ -8092,6 +8272,37 @@ async function executePaperclipNativeSessionWithinScope( activeNativeSessions.delete(input.execution.binding.runId); clearSteeringDeliveries(input.execution.binding.runId); clearNativeRuntimeRequestResolutions(input.execution.binding.runId); + // Stop before paperclip_finish is normal. Bounded provider teardown has + // finished; a missing result must not overwrite cancellation or trigger + // another turn to perform completion bookkeeping. + if (protocolIntegrityFailure === null && error instanceof Error && error.message === "native_finalization_missing: session returned no semantic result") { + const [stoppedRun] = await input.db.select().from(heartbeatRuns).where(and( + eq(heartbeatRuns.id, input.execution.binding.runId), + eq(heartbeatRuns.companyId, input.execution.binding.companyId), + eq(heartbeatRuns.agentId, input.execution.binding.agentId), + eq(heartbeatRuns.nativeIssueId, input.execution.binding.issueId), + )).limit(1); + if (stoppedRun && hasAcknowledgedNativeStopIntent(stoppedRun)) { + const [settled] = await input.db.update(nativeRunFinalizations).set({ + phase: "terminal_failure", failureCode: "native_retry_cancelled", nextAttemptAt: null, + leaseOwner: null, leaseExpiresAt: null, controlDeadlineAt: null, recoveryState: null, + updatedAt: new Date(), + }).where(and( + eq(nativeRunFinalizations.runId, input.execution.binding.runId), + eq(nativeRunFinalizations.companyId, input.execution.binding.companyId), + eq(nativeRunFinalizations.leaseOwner, leaseOwner), + eq(nativeRunFinalizations.attempt, attempt), + isNull(nativeRunFinalizations.resultId), + )).returning({ runId: nativeRunFinalizations.runId }); + if (settled) { + await stoppedLeaseRenewal; + if (warmSessionId !== null && lifecyclePolicy.mode === "warm") { + await releaseWarmNativeSession(warmSessionId, warmSessionOwnerToken, lifecyclePolicy.idleTimeoutMs, true); + } + error = new NativeCancellationPendingRecoveryError(); + } + } + } if ( error instanceof NativeResultPendingFinalizationError || error instanceof NativeCancellationPendingRecoveryError @@ -8372,12 +8583,14 @@ async function executePaperclipNativeSessionWithinScope( failureTask.executionRunId === input.execution.binding.runId) && (!failureTask.checkoutRunId || failureTask.checkoutRunId === input.execution.binding.runId); + let failureBlockStatusVersion: number | undefined; if (stillOwnsTask && recoveryProjection.issueStatus) { - await issueService(tx as unknown as Db).update( + const projected = await issueService(tx as unknown as Db).update( input.execution.binding.issueId, { status: recoveryProjection.issueStatus }, tx, ); + if (projected?.status === "blocked") failureBlockStatusVersion = projected.statusVersion; } if ( !ownershipUnverified && @@ -8405,6 +8618,10 @@ async function executePaperclipNativeSessionWithinScope( .digest("hex"), evidence: { runId: input.execution.binding.runId, + ...(failureBlockStatusVersion !== undefined ? { nativeFailureBlock: { + runId: input.execution.binding.runId, + statusVersion: failureBlockStatusVersion, + } } : {}), coordinatorAttempt: attempt, sourceFailureCode, recoveryDisposition: failureCode, @@ -10575,82 +10792,99 @@ async function createRunnerdBackendWithinSessionClaim( configuredProviderPackRoot && stagedRemoteProviderPackRoot ) { - let preinstalledProviderPack = await discoverPreinstalledProviderPack(); - if (preinstalledProviderPack) { - try { - await measureNativeRunnerSpan( - input.trace, - "provider_pack.verify_preinstalled", - () => verifyRemoteProviderPack(preinstalledProviderPack!), - ); - const escapedSource = preinstalledProviderPack.replaceAll( + const packSource = await prepareVerifiedRemoteProviderPack({ + verifyStaged: () => measureNativeRunnerSpan( + input.trace, + "provider_pack.verify", + () => verifyRemoteProviderPack(stagedRemoteProviderPackRoot), + ), + usePreinstalled: async () => { + let preinstalledProviderPack = await discoverPreinstalledProviderPack(); + if (preinstalledProviderPack) { + try { + await measureNativeRunnerSpan( + input.trace, + "provider_pack.verify_preinstalled", + () => verifyRemoteProviderPack(preinstalledProviderPack!), + ); + const escapedSource = preinstalledProviderPack.replaceAll( + "'", + "'\\''", + ); + const escapedTarget = stagedRemoteProviderPackRoot.replaceAll( + "'", + "'\\''", + ); + const escapedParent = posix + .dirname(stagedRemoteProviderPackRoot) + .replaceAll("'", "'\\''"); + const linked = await remoteCommandRunner.execute({ + command: "sh", + args: [ + "-c", + `umask 077; mkdir -p '${escapedParent}' && rm -rf '${escapedTarget}' && ln -s '${escapedSource}' '${escapedTarget}'`, + ], + cwd: remoteTarget.remoteCwd, + bypassSession: true, + timeoutMs: 10_000, + }); + if (linked.exitCode !== 0 || linked.timedOut) { + throw new Error( + "runner_remote_provider_artifact_incompatible: preinstalled provider pack could not be linked", + ); + } + activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot; + await input.onLog?.( + "stderr", + "[paperclip-runner] using content-matched provider pack from the sandbox image\n", + ); + } catch { + preinstalledProviderPack = null; + } + } + return preinstalledProviderPack !== null; + }, + stageAndVerify: async () => { + if (!remoteCommandRunner.syncIn) { + throw new Error( + "runner_remote_provider_artifact_incompatible: this remote transport cannot stage a provider pack; preinstall the exact manifest-matched pack", + ); + } + const escapedPackRoot = stagedRemoteProviderPackRoot.replaceAll( "'", "'\\''", ); - const escapedTarget = stagedRemoteProviderPackRoot.replaceAll( - "'", - "'\\''", - ); - const escapedParent = posix - .dirname(stagedRemoteProviderPackRoot) - .replaceAll("'", "'\\''"); - const linked = await remoteCommandRunner.execute({ + const cleared = await remoteCommandRunner.execute({ command: "sh", - args: [ - "-c", - `umask 077; mkdir -p '${escapedParent}' && rm -rf '${escapedTarget}' && ln -s '${escapedSource}' '${escapedTarget}'`, - ], + args: ["-c", `rm -rf '${escapedPackRoot}'`], cwd: remoteTarget.remoteCwd, bypassSession: true, timeoutMs: 10_000, }); - if (linked.exitCode !== 0 || linked.timedOut) { + if (cleared.exitCode !== 0 || cleared.timedOut) { throw new Error( - "runner_remote_provider_artifact_incompatible: preinstalled provider pack could not be linked", + "runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced", ); } + await stageRemoteRunnerDirectory({ + target: remoteTarget, + runner: remoteCommandRunner, + sourcePath: configuredProviderPackRoot, + targetPath: stagedRemoteProviderPackRoot, + mode: 0o700, + }); + await measureNativeRunnerSpan(input.trace, "provider_pack.verify", () => + verifyRemoteProviderPack(stagedRemoteProviderPackRoot), + ); activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot; - await input.onLog?.( - "stderr", - "[paperclip-runner] using content-matched provider pack from the sandbox image\n", - ); - } catch { - preinstalledProviderPack = null; - } - } - if (!preinstalledProviderPack) { - if (!remoteCommandRunner.syncIn) { - throw new Error( - "runner_remote_provider_artifact_incompatible: this remote transport cannot stage a provider pack; preinstall the exact manifest-matched pack", - ); - } - const escapedPackRoot = stagedRemoteProviderPackRoot.replaceAll( - "'", - "'\\''", + }, + }); + activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot; + if (packSource === "staged") { + await input.onLog?.( + "stderr", + "[paperclip-runner] reusing content-matched provider pack from the workspace\n", ); - const cleared = await remoteCommandRunner.execute({ - command: "sh", - args: ["-c", `rm -rf '${escapedPackRoot}'`], - cwd: remoteTarget.remoteCwd, - bypassSession: true, - timeoutMs: 10_000, - }); - if (cleared.exitCode !== 0 || cleared.timedOut) { - throw new Error( - "runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced", - ); - } - await stageRemoteRunnerDirectory({ - target: remoteTarget, - runner: remoteCommandRunner, - sourcePath: configuredProviderPackRoot, - targetPath: stagedRemoteProviderPackRoot, - mode: 0o700, - }); - await measureNativeRunnerSpan(input.trace, "provider_pack.verify", () => - verifyRemoteProviderPack(stagedRemoteProviderPackRoot), - ); - activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot; } } remotePrepared = true; @@ -11580,6 +11814,7 @@ async function createRunnerdBackendWithinSessionClaim( ), codexTransportFactory: (recoveryContext) => createRunnerdCodexTransport({ + onSpawn: input.onSpawn, provider: input.execution.provider.kind === "codex" ? "codex" @@ -12028,10 +12263,20 @@ async function createRunnerdBackendWithinSessionClaim( const wrapManagedSession = (session: NativeSession): NativeSession => { if (!input.managedAiCredentialHome || input.execution.provider.kind !== "codex") return session; const close = session.close.bind(session); + const detach = session.detachControllerForRestart?.bind(session); + let detachedForRestart = false; + if (detach) { + session.detachControllerForRestart = async () => { + // Ownership is relinquished before the asynchronous detach completes. + // Late close finalizers must leave the live runner's credentials alone. + detachedForRestart = true; + await detach(); + }; + } let copied = false; session.close = async (closeInput) => { await close(closeInput); - if (copied) return; + if (detachedForRestart || copied) return; copied = true; const remoteAuth = remoteCodexHome ? posix.join(remoteCodexHome, "auth.json") : null; const localAuth = join(root, "codex-home", "auth.json"); diff --git a/server/src/services/native-runtime/remote-provider-pack.test.ts b/server/src/services/native-runtime/remote-provider-pack.test.ts new file mode 100644 index 0000000000..189e594ce4 --- /dev/null +++ b/server/src/services/native-runtime/remote-provider-pack.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it, vi } from "vitest"; +import { prepareVerifiedRemoteProviderPack } from "./remote-provider-pack.js"; + +describe("remote provider pack reuse", () => { + it("keeps a verified staged pack without consulting the image or uploading again", async () => { + const usePreinstalled = vi.fn(); + const stageAndVerify = vi.fn(); + expect(await prepareVerifiedRemoteProviderPack({ + verifyStaged: async () => {}, usePreinstalled, stageAndVerify, + })).toBe("staged"); + expect(usePreinstalled).not.toHaveBeenCalled(); + expect(stageAndVerify).not.toHaveBeenCalled(); + }); + + it.each(["missing pack", "manifest mismatch", "artifact hash mismatch"])( + "replaces a %s with a verified image pack", + async (reason) => { + const stageAndVerify = vi.fn(); + expect(await prepareVerifiedRemoteProviderPack({ + verifyStaged: async () => { throw new Error(reason); }, + usePreinstalled: async () => true, + stageAndVerify, + })).toBe("preinstalled"); + expect(stageAndVerify).not.toHaveBeenCalled(); + }, + ); + + it("uploads when neither existing pack is valid and fails closed if upload verification fails", async () => { + const stageAndVerify = vi.fn().mockRejectedValue(new Error("artifact hash mismatch")); + const input = { + verifyStaged: async () => { throw new Error("stale staged pack"); }, + usePreinstalled: async () => false, + stageAndVerify, + }; + await expect(prepareVerifiedRemoteProviderPack(input)).rejects.toThrow("artifact hash mismatch"); + stageAndVerify.mockResolvedValue(undefined); + await expect(prepareVerifiedRemoteProviderPack(input)).resolves.toBe("uploaded"); + }); +}); diff --git a/server/src/services/native-runtime/remote-provider-pack.ts b/server/src/services/native-runtime/remote-provider-pack.ts new file mode 100644 index 0000000000..adf8459060 --- /dev/null +++ b/server/src/services/native-runtime/remote-provider-pack.ts @@ -0,0 +1,16 @@ +/** Reuse only a pack that passes the same full verification as a new upload. */ +export async function prepareVerifiedRemoteProviderPack(input: { + verifyStaged: () => Promise; + usePreinstalled: () => Promise; + stageAndVerify: () => Promise; +}): Promise<"staged" | "preinstalled" | "uploaded"> { + try { + await input.verifyStaged(); + return "staged"; + } catch { + // Missing, stale, or modified packs are never executable cache hits. + } + if (await input.usePreinstalled()) return "preinstalled"; + await input.stageAndVerify(); + return "uploaded"; +} diff --git a/server/src/services/native-runtime/stopped-codex-turn.test.ts b/server/src/services/native-runtime/stopped-codex-turn.test.ts new file mode 100644 index 0000000000..7d1b761315 --- /dev/null +++ b/server/src/services/native-runtime/stopped-codex-turn.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { stoppedCodexTurnIsTextOnly } from "./stopped-codex-turn.js"; +const meta = { type: "session_meta", payload: { id: "thread", cwd: "/workspace" } }; +const start = { type: "event_msg", payload: { type: "task_started", turn_id: "turn" } }; +const context = { type: "turn_context", payload: { turn_id: "turn", cwd: "/workspace" } }; +const answer = { type: "response_item", payload: { type: "message", role: "assistant" } }; +const stop = { type: "event_msg", payload: { type: "turn_aborted", turn_id: "turn", reason: "interrupted" } }; +const check = (rows: unknown[]) => stoppedCodexTurnIsTextOnly({ rows, threadId: "thread", turnId: "turn", cwd: "/workspace" }); +describe("stopped Codex turn inventory", () => { + it("accepts an exactly bound, closed text-only turn", () => { + expect(check([meta, start, context, answer, stop])).toBe(true); + }); + it("does not treat partial output or an unrelated abort as containment", () => { + expect(check([meta, start, context, answer])).toBe(false); + expect(check([meta, start, context, { ...stop, payload: { ...stop.payload, turn_id: "other" } }])).toBe(false); + }); + it.each(["function_call", "custom_tool_call", "web_search_call", "unknown_future_action"])("refuses unverified %s outcomes", type => { + expect(check([meta, start, context, { type: "response_item", payload: { type } }, stop])).toBe(false); + }); + it("refuses later work, duplicate starts, and changed session identity", () => { + expect(check([meta, start, context, stop, answer])).toBe(false); + expect(check([meta, start, context, start, stop])).toBe(false); + expect(check([{ ...meta, payload: { ...meta.payload, id: "other" } }, start, context, stop])).toBe(false); + }); + it("does not replay completed actions in earlier turns", () => { + expect(check([meta, { type: "response_item", payload: { type: "custom_tool_call" } }, start, context, answer, stop])).toBe(true); + }); + const completed = { callId: "finish-id", input: { summary: "ready" } }; + const completionRows = (source: string) => [meta, start, context, + { type: "response_item", payload: { type: "custom_tool_call", name: "exec", call_id: "script", input: source } }, + { type: "event_msg", payload: { type: "item_completed", thread_id: "thread", turn_id: "turn", + item: { type: "DynamicToolCall", tool: "paperclip_finish", id: completed.callId, arguments: completed.input, status: "completed", success: true } } }, + { type: "response_item", payload: { type: "custom_tool_call_output", call_id: "script", output: [] } }, answer, stop]; + const checkCompletion = (source: string, calls = [completed]) => stoppedCodexTurnIsTextOnly({ + rows: completionRows(source), threadId: "thread", turnId: "turn", cwd: "/workspace", completedTaskControlCalls: calls, + }); + it("preserves completion bookkeeping with an exact accepted receipt", () => { + expect(checkCompletion('const r = await tools.paperclip_finish({summary: "ready"}); text(r);')).toBe(true); + expect(checkCompletion('const r = await tools.paperclip_finish({summary: "ready"}); text(r);', [])).toBe(false); + expect(checkCompletion('const r = await tools.paperclip_finish({summary: "different"}); text(r);')).toBe(false); + }); + it.each([ + 'await tools.send_email({}); const r = await tools.paperclip_finish({summary: "ready"}); text(r);', + 'const r = await tools.paperclip_finish({summary: tools.write_file()}); text(r);', + 'const r = await tools.paperclip_finish({get summary() { return "ready"; }}); text(r);', + 'const r = await tools.paperclip_finish({...external, summary: "ready"}); text(r);', + 'const r = await tools.paperclip_finish({summary: `ready`}); text(r);', + 'const r = await tools["paperclip_finish"]({summary: "ready"}); text(r);', + 'const r = await tools.paperclip_finish({summary: "ready"}); tools.send_email(r);', + 'const r = await tools.paperclip_finish({__proto__: {summary: "ready"}}); text(r);', + ])("refuses unverified execution hidden in completion code %s", source => { + expect(checkCompletion(source)).toBe(false); + }); +}); diff --git a/server/src/services/native-runtime/stopped-codex-turn.ts b/server/src/services/native-runtime/stopped-codex-turn.ts new file mode 100644 index 0000000000..32c498fd16 --- /dev/null +++ b/server/src/services/native-runtime/stopped-codex-turn.ts @@ -0,0 +1,141 @@ +import { parse } from "acorn"; +import { canonicalNativeJson } from "./canonical.js"; + +const record = (value: unknown): Record => + value && typeof value === "object" && !Array.isArray(value) + ? value as Record : {}; + +export interface CompletedTaskControlCall { callId: string; input: unknown } + +// Interpret only JSON literals in one completion call. Never evaluate provider +// JavaScript, and never infer safety merely from a script containing a tool name. +function completionScriptInput(source: unknown): unknown { + if (typeof source !== "string" || source.length > 65536) throw new Error("script unavailable"); + const program = parse(source, { ecmaVersion: 2022, sourceType: "module" }) as unknown as Record; + const body = program.body as Record[]; + if (body.length !== 2) throw new Error("unknown script"); + const first = body[0]!; + const declarations = first.declarations as Record[] | undefined; + if (first.type !== "VariableDeclaration" || first.kind !== "const" || declarations?.length !== 1) throw new Error("unknown declaration"); + const declaration = declarations[0]!; + const binding = record(declaration.id); + const awaited = record(declaration.init); + const call = record(awaited.argument); + const callee = record(call.callee); + const output = record(body[1]!.expression); + const args = call.arguments as unknown[] | undefined; + const outputArgs = output.arguments as unknown[] | undefined; + if (binding.type !== "Identifier" || awaited.type !== "AwaitExpression" || call.type !== "CallExpression" || + callee.type !== "MemberExpression" || callee.computed || callee.optional || call.optional || + record(callee.object).type !== "Identifier" || record(callee.object).name !== "tools" || + record(callee.property).name !== "paperclip_finish" || args?.length !== 1 || + body[1]!.type !== "ExpressionStatement" || output.type !== "CallExpression" || output.optional || + record(output.callee).type !== "Identifier" || record(output.callee).name !== "text" || + outputArgs?.length !== 1 || record(outputArgs[0]).type !== "Identifier" || record(outputArgs[0]).name !== binding.name) + throw new Error("unknown effect"); + let nodes = 0; + const literal = (value: unknown, depth = 0): unknown => { + if (++nodes > 4096 || depth > 32) throw new Error("literal too large"); + const node = record(value); + if (node.type === "Literal" && (node.value === null || ["string", "boolean", "number"].includes(typeof node.value)) && !node.regex && !node.bigint) + return node.value; + if (node.type === "ArrayExpression") return (node.elements as unknown[]).map(v => literal(v, depth + 1)); + if (node.type !== "ObjectExpression") throw new Error("nonliteral input"); + const result: Record = Object.create(null); + for (const raw of node.properties as unknown[]) { + const property = record(raw), key = record(property.key); + const name = key.type === "Identifier" ? key.name : key.type === "Literal" ? key.value : null; + if (property.type !== "Property" || property.kind !== "init" || property.method || property.computed || property.shorthand || + typeof name !== "string" || ["__proto__", "constructor", "prototype"].includes(name) || name in result) + throw new Error("unknown property"); + result[name] = literal(property.value, depth + 1); + } + return result; + }; + return literal(args[0]); +} + +/** A closed, text-only turn can be continued without replaying an unknown action. + * This is deliberately a closed inventory, not a search for known bad tools. + * The caller must independently authenticate the session and contain its processes. + */ +export function stoppedCodexTurnIsTextOnly(input: { + rows: unknown[]; + threadId: string; + turnId: string; + cwd: string; + completedTaskControlCalls?: CompletedTaskControlCall[]; +}): boolean { + const rows = input.rows.map(record); + const meta = rows[0]; + if (!input.threadId || !input.turnId || meta?.type !== "session_meta" || + record(meta.payload).id !== input.threadId || record(meta.payload).cwd !== input.cwd) + return false; + const starts = rows.flatMap((row, index) => row.type === "event_msg" && + record(row.payload).type === "task_started" && record(row.payload).turn_id === input.turnId + ? [index] : []); + if (starts.length !== 1) return false; + const turn = rows.slice(starts[0]!); + let contextSeen = false; + let aborted = false; + const completedCalls = input.completedTaskControlCalls ?? []; + const scripts = new Set(); + const seenCalls = new Set(); + const outputs = new Set(); + for (let index = 0; index < turn.length; index++) { + const row = turn[index]!; + const payload = record(row.payload); + if (aborted) return false; + switch (row.type) { + case "turn_context": + if (contextSeen || payload.turn_id !== input.turnId || payload.cwd !== input.cwd) return false; + contextSeen = true; + break; + case "response_item": + if (payload.type === "custom_tool_call") { + try { + if (payload.name !== "exec" || typeof payload.call_id !== "string" || scripts.has(payload.call_id) || + !completedCalls.some(call => canonicalNativeJson(call.input) === canonicalNativeJson(completionScriptInput(payload.input)))) return false; + } catch { return false; } + scripts.add(payload.call_id as string); + break; + } + if (payload.type === "custom_tool_call_output") { + if (typeof payload.call_id !== "string" || !scripts.has(payload.call_id) || outputs.has(payload.call_id)) return false; + outputs.add(payload.call_id); + break; + } + if (!["message", "reasoning"].includes(String(payload.type))) return false; + break; + case "world_state": + case "token_usage_record": + break; + case "event_msg": + switch (payload.type) { + case "task_started": + if (index !== 0 || payload.turn_id !== input.turnId) return false; + break; + case "turn_aborted": + if (payload.turn_id !== input.turnId || payload.reason !== "interrupted") return false; + aborted = true; + break; + case "token_count": + break; + case "item_completed": + if (payload.thread_id !== input.threadId || payload.turn_id !== input.turnId) return false; + if (record(payload.item).type === "DynamicToolCall") { + const item = record(payload.item); + if (item.tool !== "paperclip_finish" || item.status !== "completed" || item.success !== true || + typeof item.id !== "string" || seenCalls.has(item.id) || + !completedCalls.some(call => call.callId === item.id && canonicalNativeJson(call.input) === canonicalNativeJson(item.arguments))) return false; + seenCalls.add(item.id); + } else if (!["AgentMessage", "UserMessage", "Reasoning"].includes(String(record(payload.item).type))) return false; + break; + default: return false; + } + break; + default: return false; + } + } + return contextSeen && aborted && scripts.size === outputs.size && scripts.size === completedCalls.length && seenCalls.size === completedCalls.length; +} diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index fe09182a12..0192c64e6b 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -2776,10 +2776,18 @@ function healthFailureHttpStatus(failure: { }): number { if (failure.status === "missing_secret") return 422; if (failure.code === "composio_api_key_rejected") return 422; + if (failure.code === "tool_connection_transport_unsupported") return 422; if (failure.code.endsWith("_endpoint_rejected")) return 422; return 502; } +function unsupportedToolConnectionTransport() { + return unprocessable( + "This connection has no supported tool integration. Add a supported account or MCP connection from Connectors.", + { code: "tool_connection_transport_unsupported" }, + ); +} + function sanitizeHttpFailure(error: unknown): { status: ToolConnectionHealthStatus; message: string; @@ -2797,6 +2805,9 @@ function sanitizeHttpFailure(error: unknown): { } if (error instanceof HttpError) { const code = asRecord(error.details).code; + if (code === "tool_connection_transport_unsupported") { + return { status: "error", message: error.message, code }; + } if (code === "composio_connected_account_inactive") { return { status: "degraded", message: error.message, code }; } @@ -7428,6 +7439,9 @@ export function toolAccessService( await validateComposioConnection(connection); return []; } + if (connection.transport !== "local_stdio") { + throw unsupportedToolConnectionTransport(); + } await resolveCredentialHeaders(connection); return localTools(connection); } @@ -7579,9 +7593,11 @@ export function toolAccessService( await remoteTools(connection, credentialHeaders, actor); } else if (isComposioConnection(connection)) { await validateComposioConnection(connection); - } else { + } else if (connection.transport === "local_stdio") { await resolveCredentialHeaders(connection); await stdioTemplateId(connection.companyId, connection.config); + } else { + throw unsupportedToolConnectionTransport(); } const updated = await updateConnectionHealth( connection, diff --git a/server/src/vendor/paperclip-runner/index.ts b/server/src/vendor/paperclip-runner/index.ts index 469f8ccb36..37ae3f52c2 100644 --- a/server/src/vendor/paperclip-runner/index.ts +++ b/server/src/vendor/paperclip-runner/index.ts @@ -124,3 +124,4 @@ export const validatePrpStructuredRunResult = export const NativeProviderTerminalFailure = runner.NativeProviderTerminalFailure; export const completeTerminatedRemoteNativeSessionCleanup = runner.completeTerminatedRemoteNativeSessionCleanup; +export const completeTerminatedLocalNativeSessionCleanup = runner.completeTerminatedLocalNativeSessionCleanup; diff --git a/tests/e2e/acp-stop-continuation.spec.ts b/tests/e2e/acp-stop-continuation.spec.ts index 4384ef7f02..318ddf65f6 100644 --- a/tests/e2e/acp-stop-continuation.spec.ts +++ b/tests/e2e/acp-stop-continuation.spec.ts @@ -9,8 +9,8 @@ async function json(response: APIResponse) { return JSON.parse(body); } -for (const { unfinishedWrite, pause } of [{ unfinishedWrite: false, pause: false }, { unfinishedWrite: true, pause: false }, { unfinishedWrite: false, pause: true }]) { - test(`embedded ACP Stop: ${unfinishedWrite ? "Interrupt continues without replaying the write" : pause ? "composer pause requires Resume before continuation" : "Interrupt delivers queued input in the same session"}`, async ({ page, request }) => { +for (const { unfinishedWrite, stopResponse } of [{ unfinishedWrite: false, stopResponse: false }, { unfinishedWrite: true, stopResponse: false }, { unfinishedWrite: false, stopResponse: true }]) { + test(`embedded ACP Stop: ${unfinishedWrite ? "Interrupt continues without replaying the write" : stopResponse ? "composer Stop preserves queued input and accepts a new direction" : "Interrupt delivers queued input in the same session"}`, async ({ page, request }) => { test.setTimeout(120_000); const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-stop-browser-")); const company = await json(await request.post("/api/companies", { data: { name: `ACP Stop ${Date.now()}` } })); @@ -38,9 +38,9 @@ for (const { unfinishedWrite, pause } of [{ unfinishedWrite: false, pause: false await expect.poll(async () => JSON.stringify(await json(await request.get(`/api/issues/${issue.id}/queued-comments`)))) .toContain("List my recent Drive files."); - // Interrupt sends the queue immediately; composer Stop pauses the task. + // Both actions stop the response; composer Stop must not create a task hold. let stopped; - if (pause) { + if (stopResponse) { await page.getByRole("button", { name: "Stop", exact: true }).click(); } else { await page.getByRole("button", { name: "Interrupt", exact: true }).click(); @@ -53,26 +53,24 @@ for (const { unfinishedWrite, pause } of [{ unfinishedWrite: false, pause: false expect(stopped.resultJson.executionCancellation.state).toBe("acknowledged"); const writesAtStop = unfinishedWrite ? await readFile(path.join(root, "writes"), "utf8") : null; await page.reload(); - if (pause) { - await expect(page.getByTestId("paused-composer-takeover")).toBeVisible(); - await expect(editor).toHaveCount(0); - await expect(page.getByRole("button", { name: "Send", exact: true })).toHaveCount(0); - const rejected = await request.post(`/api/issues/${issue.id}/comments`, { data: { body: "go" } }); - expect(rejected.status()).toBe(409); + if (stopResponse) { + await expect(page.getByTestId("paused-composer-takeover")).toHaveCount(0); + await expect(editor).toBeVisible(); + expect((await json(await request.get(`/api/issues/${issue.id}/tree-control/state`))).activePauseHold).toBeNull(); + await expect(page.getByRole("button", { name: "Resume task", exact: true })).toHaveCount(0); + const saved = await json(await request.get(`/api/issues/${issue.id}/queued-comments`)); + expect(JSON.stringify(saved.entries)).toContain("List my recent Drive files."); expect((await readFile(path.join(root, "prompts"), "utf8")).trim().split("\n")).toHaveLength(1); - expect(await readFile(path.join(root, "completed"), "utf8").catch(() => "")).toBe(""); - await page.getByRole("button", { name: "Resume task", exact: true }).click(); - const dialog = page.getByRole("dialog"); - await dialog.getByRole("checkbox").check(); - await dialog.getByRole("button", { name: "Resume work", exact: true }).click(); + await editor.fill("Please continue with the saved request."); + await page.getByRole("button", { name: "Send", exact: true }).click(); } await expect(page.getByText("Answered the pending follow-up once.", { exact: false })).toBeVisible({ timeout: 30_000 }); await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}/live-runs`))).length).toBe(0); const prompts = (await readFile(path.join(root, "prompts"), "utf8")).trim().split("\n").map(line => JSON.parse(line)); expect(prompts).toHaveLength(2); expect(new Set(prompts.map(prompt => prompt.sessionId)).size).toBe(1); - // Interrupt or Resume delivers the queued follow-up without another message. - const continuationPrompts = pause ? prompts.slice(1) : [prompts.at(-1)]; + // Interrupt delivers immediately; after Stop, the new direction includes saved input. + const continuationPrompts = prompts.slice(1); expect(JSON.stringify(continuationPrompts)).toContain("List my recent Drive files."); expect(await readFile(path.join(root, "completed"), "utf8")).toBe("follow-up\n"); const completedIssue = await json(await request.get(`/api/issues/${issue.id}`)); diff --git a/tests/e2e/board-attachment-receipts.spec.ts b/tests/e2e/board-attachment-receipts.spec.ts index 9bf2bfe199..3d0deb442d 100644 --- a/tests/e2e/board-attachment-receipts.spec.ts +++ b/tests/e2e/board-attachment-receipts.spec.ts @@ -14,7 +14,7 @@ type Attachment = { issueCommentId: string | null; contentPath: string; }; -type Comment = { id: string; body: string }; +type Comment = { id: string; body: string; clientRequestId?: string | null }; async function body( response: Awaited>, @@ -212,7 +212,7 @@ for (const classic of [false, true]) { }); }); - test(`lost accepted response does not leave an apparently retryable bound receipt (classic=${classic})`, async ({ + test(`lost accepted response settles from its exact receipt without replay (classic=${classic})`, async ({ page, request, }, testInfo) => { @@ -220,9 +220,13 @@ for (const classic of [false, true]) { await fixture.editor.fill("Accepted once: inspect this exact file."); const receipt = await upload(page, fixture, files[0]!); let accepted = false; + let attempts = 0; + let acceptedRequestId: string | undefined; await page.route("**/api/issues/*/comments", async (route) => { - if (route.request().method() !== "POST" || accepted) - return route.continue(); + if (route.request().method() !== "POST") return route.continue(); + attempts++; + if (accepted) return route.continue(); + acceptedRequestId = route.request().postDataJSON().clientRequestId; const response = await route.fetch(); expect(response.status()).toBe(201); accepted = true; @@ -230,44 +234,27 @@ for (const classic of [false, true]) { }); await fixture.send.click(); await expect.poll(() => accepted).toBe(true); - await expect(fixture.editor).toContainText("Accepted once:"); expect(await fixture.comments()).toHaveLength(1); + expect(acceptedRequestId).toEqual(expect.any(String)); + expect((await fixture.comments())[0]!.clientRequestId).toBe(acceptedRequestId); expect( (await fixture.attachments()).find((row) => row.id === receipt.id) ?.issueCommentId, ).toBeTruthy(); await page.reload(); - await expect(fixture.editor).toContainText("Accepted once:"); - await expect(fixture.composer.getByRole("alert")).toContainText( - "couldn’t confirm whether this comment was saved", - ); + // The durable request receipt proves delivery even though the POST reply + // was lost. No manual Review/Discard bookkeeping or blind replay remains. + await expect(fixture.editor).toBeEmpty(); + await expect(fixture.composer.getByRole("alert")).toHaveCount(0); + await expect(fixture.composer.getByText("board-fresh.txt", { exact: true })).toHaveCount(0); await expect(fixture.send).toBeDisabled(); - // Neither click nor the editor keyboard shortcut may blindly replay it. await fixture.editor.press("Control+Enter"); + expect(attempts).toBe(1); expect(await fixture.comments()).toHaveLength(1); - const refresh = page.waitForResponse( - (res) => - res.request().method() === "GET" && - new URL(res.url()).pathname.endsWith("/comments"), - ); - await fixture.composer - .getByRole("button", { name: "Review conversation", exact: true }) - .click(); - expect((await refresh).ok()).toBe(true); - await expect( - fixture.composer.getByText( - "Discarding this draft does not remove any saved comment or uploaded file.", - ), - ).toBeVisible(); await page.screenshot({ path: testInfo.outputPath("accepted-response-lost.png"), fullPage: true, }); - await fixture.composer - .getByRole("button", { name: "Discard draft and start new", exact: true }) - .click(); - await expect(fixture.editor).toBeEmpty(); - expect(await fixture.comments()).toHaveLength(1); expect( (await fixture.attachments()).find((row) => row.id === receipt.id) ?.issueCommentId, @@ -275,7 +262,7 @@ for (const classic of [false, true]) { await page.reload(); await expect(fixture.composer.getByRole("alert")).toHaveCount(0); await fixture.editor.fill( - "A deliberately new comment after reviewing the saved original.", + "A deliberately new comment after the original receipt settled.", ); await fixture.send.click(); await expect.poll(async () => (await fixture.comments()).length).toBe(2); @@ -314,7 +301,7 @@ for (const classic of [false, true]) { ).toBeTruthy(); }); - test(`reload during a pending text-only save preserves uncertainty without replay (classic=${classic})`, async ({ + test(`reload during a pending save settles its receipt and preserves a newer draft (classic=${classic})`, async ({ page, request, }) => { @@ -326,9 +313,11 @@ for (const classic of [false, true]) { }); let accepted = false; let attempts = 0; + let acceptedRequestId: string | undefined; await page.route("**/api/issues/*/comments", async (route) => { if (route.request().method() !== "POST") return route.continue(); attempts++; + acceptedRequestId ??= route.request().postDataJSON().clientRequestId; const response = await route.fetch(); expect(response.status()).toBe(201); accepted = true; @@ -341,17 +330,25 @@ for (const classic of [false, true]) { await fixture.send.click(); await expect.poll(() => accepted).toBe(true); expect(await fixture.comments()).toHaveLength(1); + await fixture.editor.fill("A newer draft written while delivery was pending."); await page.reload(); release(); - await expect(fixture.editor).toContainText( - "One text-only save interrupted by reload.", + await expect(fixture.editor).toHaveText( + "A newer draft written while delivery was pending.", ); - await expect(fixture.composer.getByRole("alert")).toContainText( - "couldn’t confirm whether this comment was saved", - ); - await expect(fixture.send).toBeDisabled(); + await expect(fixture.composer.getByRole("alert")).toHaveCount(0); + await expect(fixture.send).toBeEnabled(); expect(attempts).toBe(1); expect(await fixture.comments()).toHaveLength(1); + expect(acceptedRequestId).toEqual(expect.any(String)); + expect((await fixture.comments())[0]!.clientRequestId).toBe(acceptedRequestId); + await fixture.send.click(); + await expect.poll(async () => (await fixture.comments()).length).toBe(2); + expect(attempts).toBe(2); + expect((await fixture.comments()).map((comment) => comment.body).sort()).toEqual([ + "A newer draft written while delivery was pending.", + "One text-only save interrupted by reload.", + ]); } finally { release(); } diff --git a/tests/e2e/composer-stop.spec.ts b/tests/e2e/composer-stop.spec.ts index c70c305fcd..d443ec1a46 100644 --- a/tests/e2e/composer-stop.spec.ts +++ b/tests/e2e/composer-stop.spec.ts @@ -284,7 +284,7 @@ for (const adapter of ["process", "paperclip_runner"] as const) { let dispatchedAt = 0; page.on("request", (req) => { - if (req.method() === "POST" && req.url().endsWith("/tree-holds")) + if (req.method() === "POST" && req.url().endsWith(`/heartbeat-runs/${parentRun.id}/cancel`)) dispatchedAt = Date.now(); }); const clickedAt = Date.now(); @@ -293,16 +293,10 @@ for (const adapter of ["process", "paperclip_runner"] as const) { await expect( page.getByRole("button", { name: "Dismiss notification" }), ).toHaveCount(0); - for (const run of [parentRun, childRun]) { - await expect - .poll( - async () => - (await json(await request.get(`/api/heartbeat-runs/${run.id}`))) - .status, - { timeout: 35_000 }, - ) - .toBe("cancelled"); - } + await expect.poll(async () => + (await json(await request.get(`/api/heartbeat-runs/${parentRun.id}`))).status, + { timeout: 35_000 }, + ).toBe("cancelled"); const stoppedAt = Date.now(); expect(dispatchedAt - clickedAt).toBeLessThan(2000); expect(dispatchedAt).toBeGreaterThan(0); @@ -313,7 +307,7 @@ for (const adapter of ["process", "paperclip_runner"] as const) { .toBe(false); await expect .poll(() => processAlive(childRun.processPid), { timeout: 3000 }) - .toBe(false); + .toBe(true); } else { const finalRun = await json( await request.get(`/api/heartbeat-runs/${parentRun.id}`), @@ -332,6 +326,20 @@ for (const adapter of ["process", "paperclip_runner"] as const) { }), contentType: "application/json", }); + expect( + (await json(await request.get(`/api/issues/${parent.id}/tree-control/state`))).activePauseHold, + ).toBeNull(); + expect((await json(await request.get(`/api/heartbeat-runs/${childRun.id}`))).status).toBe("running"); + await expect(editor).toBeVisible(); + await expect(page.getByText("Subtree is paused.", { exact: true })).toHaveCount(0); + // Pausing future work is a separate, explicit subtree action. + await menu(page, "Pause subtree"); + await expect(page.getByRole("dialog")).toHaveCount(0); + await expect.poll(async () => (await json(await request.get(`/api/heartbeat-runs/${childRun.id}`))).status, + { timeout: 35_000 }).toBe("cancelled"); + if (adapter === "process") { + await expect.poll(() => processAlive(childRun.processPid), { timeout: 3000 }).toBe(false); + } expect( ( await json( @@ -375,12 +383,16 @@ for (const adapter of ["process", "paperclip_runner"] as const) { .getByRole("button", { name: "Resume subtree", exact: true }) .click(); await expect(page.getByRole("dialog")).toHaveCount(0); - // The recovery policy parks these stopped tasks. Releasing the hold must - // leave them parked, even with Wake agents selected; no implicit replay. - expect(await json(await request.get(`/api/issues/${parent.id}/live-runs`))).toEqual([]); - expect(await json(await request.get(`/api/issues/${child.id}/live-runs`))).toEqual([]); - await reconcileDemoExecution(request, parent.id, parentRun.id); - await reconcileDemoExecution(request, child.id, childRun.id); + if (adapter === "process") { + // Legacy processes lack runner stop/action proof, so releasing the hold + // preserves their recovery gate until the fixture reconciles them. + expect(await json(await request.get(`/api/issues/${parent.id}/live-runs`))).toEqual([]); + expect(await json(await request.get(`/api/issues/${child.id}/live-runs`))).toEqual([]); + await reconcileDemoExecution(request, parent.id, parentRun.id); + await reconcileDemoExecution(request, child.id, childRun.id); + } + // A verified stopped native runner can honor the explicitly selected + // Wake agents option without another manual reconciliation step. const resumedParentRun = await running(request, parent.id, adapter); const resumedChildRun = await running(request, child.id, adapter); expect(resumedParentRun.id).not.toBe(parentRun.id); diff --git a/tests/e2e/legacy-failure-continuation.spec.ts b/tests/e2e/legacy-failure-continuation.spec.ts index 9186c2cca6..c15bc556b9 100644 --- a/tests/e2e/legacy-failure-continuation.spec.ts +++ b/tests/e2e/legacy-failure-continuation.spec.ts @@ -11,7 +11,7 @@ async function json(response: APIResponse) { return response.json(); } -for (const action of ["task_retry", "inbox_retry", "message", "queued_interrupt", "automatic_message"] as const) { +for (const action of ["task_retry", "thread_retry", "inbox_retry", "message", "queued_interrupt", "automatic_message"] as const) { test(`legacy startup hold: ${action} reaches a new agent response`, async ({ page, request }) => { test.setTimeout(120_000); const root = await mkdtemp(path.join(os.tmpdir(), "legacy-recovery-browser-")); @@ -94,7 +94,7 @@ for (const action of ["task_retry", "inbox_retry", "message", "queued_interrupt" await page.getByRole("textbox", { name: "editable markdown" }).fill("Please continue the pending follow-up."); await page.getByRole("button", { name: "Send", exact: true }).click(); } else { - await page.getByRole("button", { name: "Retry", exact: true }).click(); + await page.getByRole("button", { name: action === "thread_retry" ? "Try again" : "Retry", exact: true }).click(); if (action === "inbox_retry") await page.goto(taskUrl); } await expect(page.getByText("Answered the pending follow-up once.", { exact: false })).toBeVisible({ timeout: 45_000 }); diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index 23ec7af208..8226818170 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -3573,13 +3573,7 @@ describe("IssueChatThread", () => { expect(send().disabled).toBe(false); await act(async () => send().click()); const expectedBody = `Inspect the file\n\n[fresh.txt](/api/attachments/${id}/content)`; - expect(onAdd).toHaveBeenNthCalledWith( - 1, - expectedBody, - undefined, - undefined, - [id], - ); + expect(onAdd).toHaveBeenNthCalledWith(1, expectedBody, undefined, undefined, [id], expect.any(String)); expect(appendMock).not.toHaveBeenCalled(); await act(async () => root.unmount()); root = createRoot(container); @@ -3591,13 +3585,7 @@ describe("IssueChatThread", () => { ).toBe("Inspect the file"); expect(container.textContent).toContain("fresh.txt"); await act(async () => send().click()); - expect(onAdd).toHaveBeenNthCalledWith( - 2, - expectedBody, - undefined, - undefined, - [id], - ); + expect(onAdd).toHaveBeenNthCalledWith(2, expectedBody, undefined, undefined, [id], expect.any(String)); expect(onAttachImage).toHaveBeenCalledTimes(1); await act(async () => root.unmount()); }); @@ -3668,6 +3656,64 @@ describe("IssueChatThread", () => { }, ); + it.each(["late receipt", "reload receipt", "navigation success"])( + "preserves a newer legacy draft after %s", + async (outcome) => { + const key = `legacy-next-draft-${outcome}`; + let resolveSend!: () => void; + let rejectSend!: (error: Error) => void; + const onAdd = vi.fn().mockReturnValue(new Promise((resolve, reject) => { + resolveSend = resolve; + rejectSend = reject; + })); + const attachmentId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3"; + const onAttachImage = vi.fn().mockResolvedValue({ + id: attachmentId, contentPath: `/api/attachments/${attachmentId}/content`, originalFilename: "next-draft.txt", + }); + let root = createRoot(container); + const element = (requestId?: string) => ( + + + + ); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + const type = (value: string) => act(() => { + Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), value); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + await act(async () => root.render(element())); + type("Earlier message"); + await act(async () => Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Send")!.click()); + const requestId = onAdd.mock.calls[0]![4] as string; + type("Newer unsent draft"); + await act(async () => container.querySelector('[data-testid="issue-chat-composer"]')! + .dispatchEvent(createFileDragEvent("drop", [new File(["next"], "next-draft.txt", { type: "text/plain" })]))); + if (outcome !== "late receipt") await act(async () => root.unmount()); + if (outcome === "navigation success") await act(async () => resolveSend()); + else if (outcome === "late receipt") await act(async () => rejectSend(new CommentSubmissionUnknownError())); + if (outcome !== "late receipt") root = createRoot(container); + await act(async () => root.render(element(requestId))); + expect(editor().value).toBe("Newer unsent draft"); + expect(localStorage.getItem(key)).toBe("Newer unsent draft"); + expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); + expect(localStorage.getItem(`${key}:attachments:v1`)).toContain(attachmentId); + expect(container.textContent).toContain("next-draft.txt"); + expect(container.textContent).not.toContain("We couldn’t confirm"); + await act(async () => root.unmount()); + await act(async () => resolveSend()); + expect(localStorage.getItem(key)).toBe("Newer unsent draft"); + }, + ); + it("keeps a reassigned legacy comment pending until its actual mutation promise settles", async () => { let resolveSend!: () => void; const onAdd = vi.fn().mockReturnValue( @@ -3716,7 +3762,7 @@ describe("IssueChatThread", () => { expect(onAdd).toHaveBeenCalledWith("Please review the result", undefined, { assigneeAgentId: null, assigneeUserId: "reviewer", - }); + }, undefined, expect.any(String)); expect(appendMock).not.toHaveBeenCalled(); expect(container.textContent).toContain("Posting..."); expect(localStorage.getItem("legacy-awaited-reassignment")).toBe( @@ -3967,11 +4013,7 @@ describe("IssueChatThread", () => { submitButton?.click(); }); - expect(onAdd).toHaveBeenCalledWith( - "Please pick this back up", - true, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("Please pick this back up", true, undefined, undefined, expect.any(String)); act(() => { root.unmount(); @@ -4044,11 +4086,7 @@ describe("IssueChatThread", () => { }); expect(onAdd).toHaveBeenCalledTimes(1); - expect(onAdd).toHaveBeenCalledWith( - "Reply without assignee", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("Reply without assignee", undefined, undefined, undefined, expect.any(String)); expect( document.querySelector('[data-testid="issue-chat-no-assignee-dialog"]'), ).toBeNull(); diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index cb398cbbb3..c4cf424354 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -64,6 +64,7 @@ import { loadDraftSubmission, saveDraftSubmission, clearDraftSubmission, + settleDraftSubmission, type ComposerDraftSubmission, } from "../lib/composer-draft"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; @@ -502,6 +503,7 @@ export interface IssueChatComposerHandle { interface IssueChatComposerProps { onSend: IssueChatThreadProps["onAdd"]; + confirmedSubmissionIds: ReadonlySet; onReviewConversation?: () => Promise; onStop?: () => Promise; stopPending?: boolean; @@ -4668,6 +4670,7 @@ const IssueChatComposer = forwardRef< >(function IssueChatComposer( { onSend, + confirmedSubmissionIds, onReviewConversation, onStop, stopPending, @@ -4713,6 +4716,30 @@ const IssueChatComposer = forwardRef< }, [draftKey]); const bodyRef = useRef(body); bodyRef.current = body; + const pendingDraftRef = useRef<{ + draftKey: string; + attemptId: string; + submittedBody: string; + submittedAttachmentIds: string[]; + } | null>(null); + function changeBody(update: string | ((current: string) => string)) { + const value = typeof update === "function" ? update(bodyRef.current) : update; + bodyRef.current = value; + setBody(value); + const pending = pendingDraftRef.current; + if (!pending || pending.draftKey !== draftKey || + loadDraftSubmission(pending.draftKey)?.attemptId !== pending.attemptId) return; + // Persist the next draft while delivery is pending, before navigation or a + // lost response can turn the original submission into an uncertain one. + saveDraft(pending.draftKey, + value ? `${pending.submittedBody}\n\n${value}` : pending.submittedBody, + pending.attemptId); + saveDraftSubmission(pending.draftKey, { + attemptId: pending.attemptId, reviewed: false, + nextDraftOffset: pending.submittedBody.length + (value ? 2 : 0), + submittedAttachmentIds: pending.submittedAttachmentIds, + }); + } const submittingRef = useRef(submitting); submittingRef.current = submitting; const [attaching, setAttaching] = useState(false); @@ -4741,6 +4768,12 @@ const IssueChatComposer = forwardRef< : update; composerAttachmentsRef.current = next; setComposerAttachmentState(next); + const pending = pendingDraftRef.current; + if (pending && pending.draftKey === draftKey) { + saveDraftAttachments(pending.draftKey, next + .filter(item => item.status === "attached" && item.attachmentId) + .map(item => ({ ...item, inline: item.inline === true })), pending.attemptId); + } } const dragDepthRef = useRef(0); const effectiveSuggestedAssigneeValue = @@ -4810,6 +4843,22 @@ const IssueChatComposer = forwardRef< ); }, [draftKey]); + // A server receipt for this exact request settles a restored submission. + // Text equality is not delivery proof: users may intentionally repeat text. + useEffect(() => { + if (!uncertainSubmission || !confirmedSubmissionIds.has(uncertainSubmission.attemptId)) return; + const nextDraft = uncertainSubmission.nextDraftOffset === undefined + ? "" : bodyRef.current.slice(uncertainSubmission.nextDraftOffset); + if (draftKey) settleDraftSubmission(draftKey, uncertainSubmission.attemptId, nextDraft); + setUncertainSubmission(null); + setBody(nextDraft); + bodyRef.current = nextDraft; + const submittedIds = uncertainSubmission.submittedAttachmentIds; + setComposerAttachments(current => submittedIds + ? current.filter(item => !item.attachmentId || !submittedIds.includes(item.attachmentId)) + : []); + }, [confirmedSubmissionIds, draftKey, uncertainSubmission]); + useEffect(() => { if ( !draftKey || @@ -4960,6 +5009,7 @@ const IssueChatComposer = forwardRef< const workModeChanged = pendingWorkMode !== resolvedIssueWorkMode; if (draftKey) saveDraft(draftKey, trimmed); setSubmitting(true); + bodyRef.current = ""; setBody(""); let attemptId: string | null = null; try { @@ -4976,37 +5026,45 @@ const IssueChatComposer = forwardRef< if (draftKey) { saveDraft(draftKey, trimmed); saveDraftSubmission(draftKey, { attemptId, reviewed: false }); + pendingDraftRef.current = { draftKey, attemptId, submittedBody: trimmed, submittedAttachmentIds: attachmentIds }; + changeBody(bodyRef.current); } // assistant-ui thread.append is fire-and-forget. Await the actual Board // mutation; it already owns optimistic echo and durable error handling. - const sendPromise = attachmentIds.length - ? onSend(submittedBody, reopen, reassignment, attachmentIds) - : onSend(submittedBody, reopen, reassignment); + const sendPromise = onSend( + submittedBody, reopen, reassignment, + attachmentIds.length ? attachmentIds : undefined, attemptId, + ); queueViewportRestore(viewportSnapshot); await sendPromise; + // Settle the captured task even if the user navigated away. The exact + // attempt guard preserves any newer submission in this or another tab. + if (draftKey) settleDraftSubmission(draftKey, attemptId, + mountedTaskKey.current === draftKey ? bodyRef.current : undefined); if (mountedTaskKey.current !== draftKey) return; - if (draftKey) clearDraftSubmission(draftKey, attemptId); - if (draftKey) clearDraft(draftKey); setComposerAttachments((current) => current.filter((item) => !submittedAttachmentKeys.has(item.id)), ); setReassignTarget(effectiveSuggestedAssigneeValue); } catch (error) { if (mountedTaskKey.current !== draftKey) return; + const nextDraft = bodyRef.current; if (attemptId && error instanceof CommentSubmissionUnknownError) { - const uncertain = { attemptId, reviewed: false }; + const uncertain = { + attemptId, reviewed: false, + nextDraftOffset: trimmed.length + (nextDraft ? 2 : 0), + submittedAttachmentIds: attachmentIds, + }; setUncertainSubmission(uncertain); if (draftKey && loadDraftSubmission(draftKey)?.attemptId === attemptId) saveDraftSubmission(draftKey, uncertain); } else if (draftKey && attemptId) clearDraftSubmission(draftKey, attemptId); - const restoredBody = restoreSubmittedCommentDraft({ - currentBody: bodyRef.current, - submittedBody: trimmed, - }); + const restoredBody = nextDraft ? `${trimmed}\n\n${nextDraft}` : trimmed; if (draftKey) saveDraft(draftKey, restoredBody, attemptId ?? undefined); setBody(restoredBody); } finally { + if (pendingDraftRef.current?.attemptId === attemptId) pendingDraftRef.current = null; setSubmitting(false); queueViewportRestore(viewportSnapshot); } @@ -5041,7 +5099,7 @@ const IssueChatComposer = forwardRef< const safeName = file.name.replace(/[[\]]/g, "\\$&"); const markdown = `![${safeName}](${url})`; if (insertInline) - setBody((prev) => (prev ? `${prev}\n\n${markdown}` : markdown)); + changeBody((prev) => (prev ? `${prev}\n\n${markdown}` : markdown)); setComposerAttachments((prev) => prev.map((item) => item.id === attachmentId @@ -5062,7 +5120,7 @@ const IssueChatComposer = forwardRef< return undefined; if (inline && insertInline) { const markdown = `![${file.name.replace(/[[\]]/g, "\\$&")}](${attachment.contentPath})`; - setBody((prev) => (prev ? `${prev}\n\n${markdown}` : markdown)); + changeBody((prev) => (prev ? `${prev}\n\n${markdown}` : markdown)); } setComposerAttachments((prev) => prev.map((item) => @@ -5243,7 +5301,7 @@ const IssueChatComposer = forwardRef< `(? { + changeBody((current) => { if (tokenRe.test(current)) return current.replace(tokenRe, markdown.trimEnd()); return current ? `${current} ${markdown}` : markdown; @@ -5385,7 +5443,7 @@ const IssueChatComposer = forwardRef< ref={editorRef} readOnly={!!uncertainSubmission} value={body} - onChange={setBody} + onChange={changeBody} placeholder="Reply" mentions={mentions} onSubmit={handleSubmit} @@ -5641,11 +5699,7 @@ const IssueChatComposer = forwardRef< disabled={stopControl.stopping} onClick={() => void stopControl.stop()} aria-label={stopControl.stopping ? "Stopping…" : "Stop"} - title={ - stopScope === "subtree" - ? "Stop and pause subtree" - : "Stop and pause task" - } + title="Stop response" > {stopControl.stopping ? ( @@ -6053,11 +6107,9 @@ export function IssueChatThread({ } const sendComposerComment = useCallback( - (body, reopen, reassignment, attachmentIds) => { + (body, reopen, reassignment, attachmentIds, clientRequestId) => { pendingSubmitScrollRef.current = true; - return attachmentIds?.length - ? onAdd(body, reopen, reassignment, attachmentIds) - : onAdd(body, reopen, reassignment); + return onAdd(body, reopen, reassignment, attachmentIds, clientRequestId); }, [onAdd], ); @@ -6723,6 +6775,10 @@ export function IssueChatThread({ onImageUpload={imageUploadHandler} onAttachImage={onAttachImage} draftKey={draftKey} + confirmedSubmissionIds={new Set(comments.filter((comment) => + comment.authorUserId === currentUserId && comment.clientRequestId && + !("clientStatus" in comment && comment.clientStatus) + ).map((comment) => comment.clientRequestId!))} enableReassign={enableReassign} reassignOptions={reassignOptions} currentAssigneeValue={currentAssigneeValue} diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 0a8bd470c1..412581a6b7 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -29,6 +29,16 @@ function derived(overrides: Partial & { state: DerivedMonit } describe("buildMonitorSurfaceCopy", () => { + it.each(["retrying", "due-now", "overdue"] as const)("keeps workspace contention neutral when %s", (state) => { + const copy = buildMonitorSurfaceCopy(derived({ + state, source: "scheduled-retry", nextCheckAt: NOW.toISOString(), attemptCount: 4, + }), NOW, "workspace_busy"); + expect(copy!.bannerTitle).toBe("Waiting for workspace"); + expect(copy!.stripTitle).toBe("Waiting for workspace"); + expect(copy!.tone).toBe("info"); + expect(copy!.workspaceWait).toBe(true); + expect(copy!.bannerMeta.join(" ")).not.toMatch(/Attempt|overdue|retry/i); + }); it("leads with two-unit relative time while scheduled", () => { const copy = buildMonitorSurfaceCopy( derived({ @@ -135,6 +145,19 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { } as unknown as Issue; } + it("explains automatic workspace waiting without promising that a reply bypasses the lock", () => { + const issue = { + status: "todo", scheduledRetry: { status: "scheduled_retry", scheduledRetryReason: "workspace_busy", scheduledRetryAt: NOW.toISOString() }, + } as Issue; + const root = createRoot(container); + flushSync(() => root.render(<>)); + expect(container.textContent).toContain("Waiting for workspace"); + expect(container.textContent).toContain("You can keep sending instructions while the agent waits."); + expect(container.textContent).not.toContain("wakes the agent now"); + expect(container.querySelector("button")).toBeNull(); + flushSync(() => root.unmount()); + }); + it("renders the banner with a working Check now button while waiting", () => { const onCheckNow = vi.fn(); expect(hasVisibleMonitorSurface(issueWithMonitor(new Date(NOW.getTime() + 2 * 60 * 60_000).toISOString()))).toBe(true); diff --git a/ui/src/components/IssueMonitorBanner.tsx b/ui/src/components/IssueMonitorBanner.tsx index 5208a3431e..de0e83ff47 100644 --- a/ui/src/components/IssueMonitorBanner.tsx +++ b/ui/src/components/IssueMonitorBanner.tsx @@ -49,6 +49,7 @@ export interface MonitorSurfaceCopy { stripMeta: string[]; /** `warning` (amber) once overdue, `info` (blue) while still on schedule. */ tone: "info" | "warning"; + workspaceWait?: boolean; } function capitalize(value: string): string { @@ -63,9 +64,21 @@ function capitalize(value: string): string { export function buildMonitorSurfaceCopy( derived: DerivedMonitorState, now: MonitorDate, + scheduledRetryReason?: string | null, ): MonitorSurfaceCopy | null { if (!isWaitingMonitorState(derived.state) || !derived.nextCheckAt) return null; + if (derived.source === "scheduled-retry" && scheduledRetryReason === "workspace_busy") { + return { + bannerTitle: "Waiting for workspace", + stripTitle: "Waiting for workspace", + bannerMeta: ["Another task is using this workspace. Work starts automatically when it is available."], + stripMeta: ["Work starts automatically when the workspace is available."], + tone: "info", + workspaceWait: true, + }; + } + const eta = formatMonitorEta(derived.nextCheckAt, now); // "in 2h 12m" | "due now" | "overdue by 18m" const absolute = formatMonitorAbsolute(derived.nextCheckAt, {}, now); // local time, e.g. "Today, 4:08 PM" const isScheduledRetryOnly = derived.source === "scheduled-retry"; @@ -117,7 +130,7 @@ function useMonitorSurfaceCopy(issue: Issue): MonitorSurfaceCopy | null { // roll scheduled → due → overdue on their own. const nextCheckAt = useMemo(() => deriveMonitorState(issue).nextCheckAt, [issue]); const now = useMonitorCountdown(nextCheckAt); - return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now), [issue, now]); + return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason), [issue, now]); } function CheckNowButton({ @@ -166,7 +179,7 @@ export function IssueMonitorBanner({ icon={Clock} title={copy.bannerTitle} className="my-3" - actions={onCheckNow ? : null} + actions={onCheckNow && !copy.workspaceWait ? : null} > {copy.bannerMeta.join(" · ")} @@ -201,10 +214,12 @@ export function IssueMonitorComposerStrip({
{copy.stripMeta.join(" · ")}
- {onCheckNow ? : null} + {onCheckNow && !copy.workspaceWait ? : null}

- Sending a reply wakes the agent now — before the scheduled check. + {copy.workspaceWait + ? "You can keep sending instructions while the agent waits." + : "Sending a reply wakes the agent now — before the scheduled check."}

); diff --git a/ui/src/components/IssueScheduledRetryCard.test.tsx b/ui/src/components/IssueScheduledRetryCard.test.tsx index 9ef3cb86ae..a3b441bfb4 100644 --- a/ui/src/components/IssueScheduledRetryCard.test.tsx +++ b/ui/src/components/IssueScheduledRetryCard.test.tsx @@ -121,6 +121,13 @@ function getRetryNowButton() { } describe("IssueScheduledRetryCard", () => { + it("shows workspace contention as an automatic wait without failure or retry controls", () => { + renderWithProviders(); + expect(container.textContent).toContain("Waiting for workspace"); + expect(container.textContent).toContain("Work starts automatically"); + expect(container.textContent).not.toMatch(/failed|Retry|Attempt|Replaces run/); + expect(getRetryNowButton()).toBeNull(); + }); it("renders nothing when there is no scheduled retry", () => { renderWithProviders(); expect(getCard()).toBeNull(); diff --git a/ui/src/components/IssueScheduledRetryCard.tsx b/ui/src/components/IssueScheduledRetryCard.tsx index 46a7474245..8f792d7585 100644 --- a/ui/src/components/IssueScheduledRetryCard.tsx +++ b/ui/src/components/IssueScheduledRetryCard.tsx @@ -7,6 +7,7 @@ import { formatRetryReason } from "@/lib/runRetryState"; import type { IssueScheduledRetry } from "@paperclipai/shared"; import { useRetryNowMutation, type RetryNowError } from "../hooks/useRetryNowMutation"; import { Badge } from "@/components/ui/badge"; +import { InlineBanner } from "@/components/InlineBanner"; const MAX_TURN_CONTINUATION = "max_turns_continuation"; @@ -32,6 +33,14 @@ export function IssueScheduledRetryCard({ if (!scheduledRetry || !issueId) return null; if (scheduledRetry.status !== "scheduled_retry") return null; + if (scheduledRetry.scheduledRetryReason === "workspace_busy") { + return ( + + Another task is using this workspace. Work starts automatically when it is available. + + ); + } + const continuation = isContinuationReason(scheduledRetry.scheduledRetryReason); const dueAtIso = scheduledRetry.scheduledRetryAt ? new Date(scheduledRetry.scheduledRetryAt).toISOString() diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index 234f2c2f10..1743d4e3f9 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -2012,6 +2012,52 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( return { root, queryClient }; } + it.each([ + ["claude_local", "anthropic", /Claude/, "claude-session-1", "claude-setup-token-status"], + ["codex_local", "openai", /OpenAI/, "codex-session-1", "adapter-login-status"], + ] as const)("finishes %s sign-in when its connection becomes visible before the completion poll", async (adapterType, provider, label, sessionId, statusKey) => { + mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); + mockAgentsApi.hire.mockRejectedValueOnce(new Error("Temporary hire failure")); + const { root, queryClient } = await openStep4({ adapterType }); + await pickSource(label); + for (let i = 0; i < 6; i++) await flushReact(); + try { + // The connection activity event arrives before the login poll. It must + // not replace/unmount the controller that still owns the completion. + await act(async () => { + queryClient.setQueryData(["ai-connections", "company-new"], { + currentUserId: "user-1", + connections: [{ id: "managed-connection", grantId: "managed-grant", companyId: "company-new", provider, method: "subscription", name: "My subscription", ownership: "personal", ownerUserId: "user-1", status: "connected", isDefault: true }], + }); + }); + for (let i = 0; i < 4; i++) await flushReact(); + expect(document.body.textContent).toContain(adapterType === "claude_local" ? "authorization code" : "Q2RJ-E1YIF"); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + await act(async () => { + queryClient.setQueryData( + adapterType === "claude_local" ? [statusKey, "company-new", sessionId] : [statusKey, "company-new", adapterType, sessionId], + { sessionId, status: "authenticated", expiresAt: new Date(Date.now() + 600_000).toISOString() }, + ); + }); + for (let i = 0; i < 120 && !mockAgentsApi.hire.mock.calls.length; i++) { + await act(async () => { await new Promise(resolve => setTimeout(resolve, 25)); }); + } + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.hire).toHaveBeenCalledWith("company-new", expect.objectContaining({ runtimeConfig: expect.objectContaining({ aiConnection: { provider, method: "subscription", mode: "responsible_user" } }) })); + for (let i = 0; i < 4; i++) await flushReact(); + expect(document.body.textContent).toContain("Temporary hire failure"); + const retry = [...document.body.querySelectorAll("button")].find(button => button.textContent?.trim() === "Connect"); + expect(retry).toBeTruthy(); + expect(retry!.disabled).toBe(false); + await act(async () => { retry!.click(); }); + for (let i = 0; i < 6; i++) await flushReact(); + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(2); + expect(mockAgentsApi.startClaudeSetupTokenLogin.mock.calls.length + mockAgentsApi.startAdapterAuthLogin.mock.calls.length).toBe(1); + } finally { + await act(async () => root.unmount()); + } + }); + it("names the tiles for the provider, not the adapter type", async () => { // `MODEL_SOURCE_NAMES` exists so this row says "Claude" and "OpenAI" — // which provider you are signing in to, the question the step's heading diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 7641d62b83..5daf58cd44 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -1150,9 +1150,14 @@ function OnboardingWizardInner({ */ const connectStepNeedsLogin = Boolean( credentialMode !== "api" && - (showAdapterLoginPanel || (canShowAdapterLogin && adapterType === "codex_local" && subscriptionId?.companyId === createdCompanyId && subscriptionId.id === "")) && - !savedSubscription && - !(adapterType === "claude_local" && savedKeys.storedLogin.data) && + // Connection-list invalidation can arrive before the login's completion + // poll. Keep its controller mounted until it reports success; otherwise + // the saved account replaces the panel and "Connecting" never finishes. + (connectAuthUrl || ( + (showAdapterLoginPanel || (canShowAdapterLogin && adapterType === "codex_local" && subscriptionId?.companyId === createdCompanyId && subscriptionId.id === "")) && + !savedSubscription && + !(adapterType === "claude_local" && savedKeys.storedLogin.data) + )) && !savedKeys.loading && createdCompanyId && resolvedLoginEnvironmentId, @@ -2154,6 +2159,13 @@ function OnboardingWizardInner({ } finally { hiringAgentRef.current = false; setLoading(false); + // Authentication is already saved. A failed probe or hire must offer a + // retry with that account, rather than keep the completed login busy. + if (connectCredentialStored && stillTheSameCompany(createdCompanyId)) { + connectingSinceRef.current = null; + setConnectAuthUrl(null); + setConnectPhase((phase) => phase === "connecting" ? "ready" : phase); + } } } @@ -2740,6 +2752,7 @@ function OnboardingWizardInner({ }} onConnected={() => { if (managedProvider) managedSubscriptionRef.current = { companyId: createdCompanyId, binding: { provider: managedProvider, method: "subscription", mode: "responsible_user" } }; + setConnectAuthUrl(null); // Not into a card the customer has left. The panel is // still mounted through Back's exit, and a login that // finished there pulled the step back into "Connecting" diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index fb5068033a..2d76a00885 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import type { ReactElement } from "react"; +import type { ComponentProps, ReactElement } from "react"; import { act, forwardRef, useImperativeHandle, type ForwardedRef } from "react"; import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; @@ -1247,6 +1247,17 @@ describe("TaskChatThread runtime transcript selection", () => { }, ); + it("keeps workspace contention out of the conversation's cancellation markers", () => { + render( {}} linkedRuns={[{ + runId: "workspace-wait", runtimeMode: "native", status: "cancelled", errorCode: "workspace_busy", + agentId: "agent-1", agentName: "Runner", adapterType: "paperclip_runner", startedAt: null, + createdAt: "2026-09-12T18:00:00.000Z", finishedAt: "2026-09-12T18:00:01.000Z", + }]} />); + expect(container.textContent).not.toContain("Run cancelled"); + expect(container.textContent).not.toContain("Run failed"); + expect(container.textContent).not.toContain("before returning an answer"); + }); + it("does not render an empty response notice for a conversation reset", () => { render( {}} linkedRuns={[{ runId: "chat-reset", status: "succeeded", startedAt: null, resultJson: { conversationReset: true }, @@ -1986,41 +1997,156 @@ describe("TaskChatThread runtime transcript selection", () => { expect(container.textContent).not.toContain("transcript withheld"); }); - it("keeps an empty failed direct run on its legacy failure surface", () => { + it.each(["failed", "timed_out", "cancelled"] as const)( + "keeps an empty legacy %s actionable without turning Stop into Retry", + async (status) => { + const onRetryFailedRun = vi.fn(); + render( + {}} + issueStatus="blocked" + onRetryFailedRun={onRetryFailedRun} + linkedRuns={[ + { + runId: "legacy-failed", + runtimeMode: "legacy", + status, + errorCode: "legacy_process_exited", + agentId: "agent-1", + agentName: "Direct Codex", + adapterType: "codex_local", + createdAt: "2026-08-25T18:00:00.000Z", + startedAt: "2026-08-25T18:00:00.000Z", + finishedAt: "2026-08-25T18:00:02.000Z", + }, + ]} + />, + ); + + expect(container.textContent).toContain(status === "cancelled" ? "Stopped" : "Run failed"); + if (status !== "cancelled") { + expect(container.textContent).toContain("You can retry this message now."); + } + expect( + container.querySelector('[data-testid="task-chat-collapsible-marker"]'), + ).toBeNull(); + const retry = container.querySelector('[data-testid="task-chat-run-failed-try-again"]'); + if (status === "cancelled") { + expect(retry).toBeNull(); + } else { + expect(retry).not.toBeNull(); + flushSync(() => retry!.click()); + await Promise.resolve(); + expect(onRetryFailedRun).toHaveBeenCalledExactlyOnceWith("legacy-failed"); + } + expect( + container.querySelector('[data-testid="task-chat-runner-turn"]'), + ).toBeNull(); + }, + ); + + it.each(["active execution", "pending decision", "recovery hold"] as const)( + "does not promise legacy Retry during %s and restores it when the gate clears", + (gate) => { + const onRetryFailedRun = vi.fn(); + const failedRun = { + runId: "legacy-failed", + runtimeMode: "legacy" as const, + status: "failed", + errorCode: "legacy_process_exited", + agentId: "agent-1", + agentName: "Direct Codex", + adapterType: "codex_local", + createdAt: "2026-08-25T18:00:00.000Z", + startedAt: "2026-08-25T18:00:00.000Z", + finishedAt: "2026-08-25T18:00:02.000Z", + }; + const gateProps: Partial> = + gate === "pending decision" + ? { interactions: [planReviewInteraction()] } + : { + linkedRuns: [{ + ...failedRun, + execution: { + phase: gate === "active execution" ? "working" : "recovery_needed", + label: "Waiting", + cause: null, + lastConfirmedActivityAt: null, + retryAt: null, + attempt: 1, + maxAttempts: 3, + recoveryOwner: null, + nextAction: null, + permittedActions: [], + predecessorRunId: null, + successorRunId: null, + }, + }], + }; + const renderRun = (held: boolean) => render( + {}} + issueStatus="blocked" + onRetryFailedRun={onRetryFailedRun} + linkedRuns={[failedRun]} + {...(held ? gateProps : {})} + />, + ); + renderRun(true); + expect(container.textContent).toContain("Your message is preserved."); + expect(container.textContent).not.toContain("You can retry this message now."); + expect(container.querySelector('[data-testid="task-chat-run-failed-try-again"]')).toBeNull(); + renderRun(false); + expect(container.textContent).toContain("You can retry this message now."); + expect(container.querySelector('[data-testid="task-chat-run-failed-try-again"]')).not.toBeNull(); + }, + ); + + it.each([ + ...["claude_local", "codex_local", "cursor", "gemini_local", "opencode_local", + "pi_local", "grok_local", "kimi_local", "hermes_local"].flatMap((adapterType) => + ["failed", "timed_out"].map((status) => ({ adapterType, status, runtimeMode: "legacy" as const, + cause: "legacy_execution_requires_reconciliation", canRetry: true }))), + { adapterType: "paperclip_runner", status: "failed", runtimeMode: "native" as const, + cause: "legacy_execution_requires_reconciliation", canRetry: false }, + { adapterType: "process", status: "failed", runtimeMode: "legacy" as const, + cause: "legacy_execution_requires_reconciliation", canRetry: false }, + { adapterType: "claude_local", status: "failed", runtimeMode: "legacy" as const, + cause: "uncertain_provider_action", canRetry: false }, + { adapterType: "claude_local", status: "cancelled", runtimeMode: "legacy" as const, + cause: "legacy_execution_requires_reconciliation", canRetry: false }, + ])("offers only explicit conversation retries: $runtimeMode/$adapterType/$status/$cause", (testCase) => { render( {}} issueStatus="blocked" onRetryFailedRun={vi.fn()} - linkedRuns={[ - { - runId: "legacy-failed", - runtimeMode: "legacy", - status: "failed", - errorCode: "legacy_process_exited", - agentId: "agent-1", - agentName: "Direct Codex", - adapterType: "codex_local", - createdAt: "2026-08-25T18:00:00.000Z", - startedAt: "2026-08-25T18:00:00.000Z", - finishedAt: "2026-08-25T18:00:02.000Z", + linkedRuns={[{ + runId: "stopped-run", + runtimeMode: testCase.runtimeMode, + adapterType: testCase.adapterType, + status: testCase.status, + agentId: "agent-1", + agentName: "Stopped agent", + createdAt: "2026-08-25T18:00:00.000Z", + startedAt: "2026-08-25T18:00:00.000Z", + finishedAt: "2026-08-25T18:00:02.000Z", + execution: { + phase: "recovery_needed", label: "Stopped", cause: testCase.cause, + lastConfirmedActivityAt: null, retryAt: null, attempt: 1, maxAttempts: 3, + recoveryOwner: null, nextAction: null, permittedActions: [], + predecessorRunId: null, successorRunId: null, }, - ]} + }]} />, ); - - expect(container.textContent).toContain("Run failed"); - expect(container.textContent).toContain("You can retry this message now."); - expect( - container.querySelector('[data-testid="task-chat-collapsible-marker"]'), - ).toBeNull(); - expect( - container.querySelector('[data-testid="task-chat-run-failed-try-again"]'), - ).toBeNull(); - expect( - container.querySelector('[data-testid="task-chat-runner-turn"]'), - ).toBeNull(); + expect(Boolean(container.querySelector('[data-testid="task-chat-run-failed-try-again"]'))) + .toBe(testCase.canRetry); + if (testCase.canRetry) expect(container.textContent).toContain("You can retry this message now."); + else expect(container.textContent).not.toContain("You can retry this message now."); }); it.each(DIRECT_ADAPTER_TYPES)( @@ -3581,7 +3707,7 @@ describe("TaskChatThread composer execution controls", () => { }; render( {}} issueStatus="in_progress" activeRun={run} onCancelRun={onStop} stopScope="subtree" />); const button = container.querySelector('[data-testid="task-chat-composer-stop"]')!; - expect(button.title).toBe("Stop and pause subtree"); + expect(button.title).toBe("Stop response"); await act(async () => { button.click(); }); expect(onStop).toHaveBeenCalledOnce(); render( {}} issueStatus="in_progress" activeRun={run} onCancelRun={onStop} stopPending />); diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index ab6c297500..dfe7ae5c96 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -540,6 +540,38 @@ export function TaskChatThread(props: TaskChatThreadProps) { onResumeAssignee, resumeAssigneePending = false, } = props; + const retryFailedRunHandler = + isTerminalIssueStatus(issueStatus) || + interactions?.some((interaction) => interaction.status === "pending") || + requiresExecutionReconciliation(props.recoveryAction?.cause) || + props.scheduledRetry || + linkedRuns?.some((run) => { + // The server accepts explicit new attempts for these stopped legacy + // conversations. It still proves process/lease termination and ownership; + // offering Retry does not certify prior action outcomes or resume them. + // Keep this set aligned with conversation-continuation.ts on the server. + if ( + run.runtimeMode === "legacy" && + (run.status === "failed" || run.status === "timed_out") && + run.execution?.phase === "recovery_needed" && + run.execution.cause === "legacy_execution_requires_reconciliation" && + [ + "claude_local", "codex_local", "cursor", "gemini_local", "opencode_local", + "pi_local", "grok_local", "kimi_local", "hermes_local", + ].includes(run.adapterType ?? "") + ) return false; + return [ + "working", + "retry_scheduled", + "reconnecting", + "finishing", + "queued", + "recovery_needed", + ].includes(run.execution?.phase ?? ""); + }) + ? undefined + : onRetryFailedRun; + const canRetryFailedRun = Boolean(retryFailedRunHandler); const queryClient = useQueryClient(); const createdProjectItems = useProjectCreatedItems(props.creationActivity ?? [], companyId); const [pendingComposerAssignee, setPendingComposerAssignee] = useState< @@ -1397,6 +1429,12 @@ export function TaskChatThread(props: TaskChatThreadProps) { if (liveRun && source.id === liveRun.id) continue; const entries = transcriptByRun.get(source.id) ?? []; const meta = linkedRunMetaById.get(source.id); + // A workspace admission attempt never started provider work. Its live + // successor owns the waiting indicator; retain this attempt in the run log. + if (source.status === "cancelled" && meta?.errorCode === "workspace_busy") { + settledRunIds.add(source.id); + continue; + } // /new is represented by its durable comment boundary, not an empty // model response or a completed-run notice. if (meta?.resultJson?.conversationReset === true) { settledRunIds.add(source.id); continue; } @@ -1632,7 +1670,9 @@ export function TaskChatThread(props: TaskChatThreadProps) { const code = meta?.errorCode ?? "native_runner_process_exited"; const retryDetail = meta?.scheduledRetryAt ? "Retry scheduled automatically." - : "You can retry this message now."; + : canRetryFailedRun + ? "You can retry this message now." + : "Your message is preserved."; const aiRequest = interactions?.find((interaction) => interaction.kind === "connection_intent" && interaction.payload.purpose === "ai" && interaction.sourceRunId === source.id); const detail = aiRequest ? aiRequest.status === "pending" @@ -1655,6 +1695,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { kind: "marker", variant: "interrupted", label: source.status === "cancelled" ? (meta?.startedAt ? "Stopped" : "Couldn't start") : "Run failed", + runId: source.status === "cancelled" ? undefined : source.id, tone: source.status === "cancelled" ? "neutral" : "error", detail, }, @@ -1975,6 +2016,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { }; }, [ orderedEntries, + canRetryFailedRun, interactions, runs, liveRun, @@ -2782,28 +2824,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { tryAgainNoLiveExecutionPathPending={ tryAgainNoLiveExecutionPathPending } - onRetryFailedRun={ - isTerminalIssueStatus(issueStatus) || - interactions?.some( - (interaction) => interaction.status === "pending", - ) || - requiresExecutionReconciliation( - props.recoveryAction?.cause, - ) || - props.scheduledRetry || - linkedRuns?.some((run) => - [ - "working", - "retry_scheduled", - "reconnecting", - "finishing", - "queued", - "recovery_needed", - ].includes(run.execution?.phase ?? ""), - ) - ? undefined - : onRetryFailedRun - } + onRetryFailedRun={retryFailedRunHandler} retryFailedRunId={retryFailedRunId} tail={ tailRunId || @@ -2969,6 +2990,10 @@ export function TaskChatThread(props: TaskChatThreadProps) {
+ comment.authorUserId === currentUserId && comment.clientRequestId && + !("clientStatus" in comment && comment.clientStatus) + ).map((comment) => comment.clientRequestId!))} onReviewConversation={onReviewConversation} onStop={liveRun ? onCancelRun : undefined} stopPending={stopPending} diff --git a/ui/src/components/issue-properties/IssueProperties.tsx b/ui/src/components/issue-properties/IssueProperties.tsx index 04dfebb8bf..ab8140bdcb 100644 --- a/ui/src/components/issue-properties/IssueProperties.tsx +++ b/ui/src/components/issue-properties/IssueProperties.tsx @@ -2643,7 +2643,11 @@ export function IssueProperties({ )} - {showScheduledRetryRow && scheduledRetryContent ? ( + {showScheduledRetryRow && scheduledRetry?.scheduledRetryReason === "workspace_busy" ? ( + + Waiting for workspace + + ) : showScheduledRetryRow && scheduledRetryContent ? ( { + it("settles an acknowledged submission after navigating away", async () => { + const key = "navigate-before-save"; + let resolveSend!: () => void; + const onAdd = vi.fn().mockReturnValue(new Promise(resolve => { resolveSend = resolve; })); + render(); + typeText("Please write a motto"); + pressKey("Enter", { metaKey: true }); + await flushAsync(); + const intent = loadDraftSubmission(key)!; + expect(onAdd.mock.calls[0]?.[4]).toBe(intent.attemptId); + render(
Another task
); + resolveSend(); + await flushAsync(); + expect(loadDraftSubmission(key)).toBeNull(); + expect(localStorage.getItem(key)).toBeNull(); + render(); + expect(container.textContent).not.toContain("couldn’t confirm"); + expect(editable().textContent).toBe(""); + }); + + it("automatically reconciles a restored submission by its server receipt, not its text", async () => { + const key = "saved-receipt"; + const attemptId = "9af8228f-0be7-45ae-a104-6fbe0af6f1d3"; + saveDraft(key, "Already answered"); + saveDraftSubmission(key, { attemptId, reviewed: false }); + const onAdd = vi.fn(); + render(); + expect(loadDraftSubmission(key)).not.toBeNull(); + render(); + await flushAsync(); + expect(loadDraftSubmission(key)).toBeNull(); + expect(editable().textContent).toBe(""); + expect(onAdd).not.toHaveBeenCalled(); + }); + + it("preserves the next draft when an uncertain submission is later confirmed after reload", async () => { + const key = "uncertain-with-next-draft"; + let rejectSend!: (error: Error) => void; + const onAdd = vi.fn().mockReturnValue(new Promise((_resolve, reject) => { rejectSend = reject; })); + render(); + typeText("First request"); + pressKey("Enter", { metaKey: true }); + await flushAsync(); + const attemptId = loadDraftSubmission(key)!.attemptId; + typeText("Keep this next draft"); + rejectSend(new CommentSubmissionUnknownError()); + await flushAsync(); + render(
Another task
); + render(); + await flushAsync(); + expect(editable().textContent).toBe("Keep this next draft"); + expect(localStorage.getItem(key)).toBe("Keep this next draft"); + expect(loadDraftSubmission(key)).toBeNull(); + expect(onAdd).toHaveBeenCalledTimes(1); + }); + + it.each(["acknowledgment", "receipt after reload"])("preserves a next draft across navigation before %s", async (confirmation) => { + const key = "navigate-with-next-draft"; + let resolveSend!: () => void; + const onAdd = vi.fn().mockReturnValue(new Promise(resolve => { resolveSend = resolve; })); + render(); + typeText("First request"); + pressKey("Enter", { metaKey: true }); + await flushAsync(); + const attemptId = loadDraftSubmission(key)!.attemptId; + typeText("Keep the unsent next request"); + render(
Another task
); + if (confirmation === "acknowledgment") { + resolveSend(); + await flushAsync(); + } + render(); + await flushAsync(); + expect(editable().textContent).toBe("Keep the unsent next request"); + expect(localStorage.getItem(key)).toBe("Keep the unsent next request"); + expect(loadDraftSubmission(key)).toBeNull(); + expect(onAdd).toHaveBeenCalledTimes(1); + resolveSend(); + await flushAsync(); + expect(localStorage.getItem(key)).toBe("Keep the unsent next request"); + }); + + it("does not copy another task's draft into a late acknowledged submission", async () => { + let resolveSend!: () => void; + const onAdd = vi.fn().mockReturnValue(new Promise(resolve => { resolveSend = resolve; })); + render(); + typeText("First request"); + pressKey("Enter", { metaKey: true }); + await flushAsync(); + render(); + typeText("Second task draft"); + resolveSend(); + await flushAsync(); + expect(loadDraftSubmission("first-task")).toBeNull(); + expect(localStorage.getItem("first-task")).toBeNull(); + expect(editable().textContent).toBe("Second task draft"); + }); + it.each(["success", "unknown"])( "does not overwrite another retained attempt after an older %s", async (outcome) => { @@ -497,7 +599,7 @@ describe("TaskChatComposer", () => { ); flushSync(() => sendButton().click()); await flushAsync(); - expect(onAdd.mock.calls[0]).toHaveLength(3); + expect(onAdd.mock.calls[0]?.[3]).toBeUndefined(); }); it("submits multiple retained file and image receipts but not removed selections", async () => { @@ -632,7 +734,7 @@ describe("TaskChatComposer", () => { expect(localStorage.getItem("removed-pending:attachments:v1")).toBeNull(); flushSync(() => sendButton().click()); await flushAsync(); - expect(onAdd.mock.calls[0]).toHaveLength(3); + expect(onAdd.mock.calls[0]?.[3]).toBeUndefined(); }); it("adds 10px to the composer's original 8px interior padding", () => { render( {}} workMode="standard" />); @@ -724,7 +826,7 @@ describe("TaskChatComposer", () => { await flushAsync(); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("hello there", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("hello there", undefined, undefined, undefined, expect.any(String)); expect(editable().textContent).toBe(""); }); @@ -736,7 +838,7 @@ describe("TaskChatComposer", () => { pressKey("Enter", { ctrlKey: true }); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("hello", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("hello", undefined, undefined, undefined, expect.any(String)); }); it("does not submit on plain Enter or Shift+Enter (newline stays with the editor)", async () => { @@ -778,7 +880,7 @@ describe("TaskChatComposer", () => { await flushAsync(); expect(onWorkModeChange).toHaveBeenCalledWith("planning"); - expect(onAdd).toHaveBeenCalledWith("do the plan", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("do the plan", undefined, undefined, undefined, expect.any(String)); }); it("cycles Auto, Plan, and Ask modes with Cmd+Period while focused", () => { @@ -872,7 +974,7 @@ describe("TaskChatComposer", () => { pressKey("Enter", { metaKey: true }); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("wake up", true, undefined); + expect(onAdd).toHaveBeenCalledWith("wake up", true, undefined, undefined, expect.any(String)); }); it("hides the attach button without an upload handler and shows it with one", () => { @@ -965,11 +1067,7 @@ describe("TaskChatComposer", () => { expect(send.disabled).toBe(false); flushSync(() => send.click()); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith( - "[notes.txt](/attachments/notes.txt)", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("[notes.txt](/attachments/notes.txt)", undefined, undefined, undefined, expect.any(String)); // Chips clear once the message posts. expect( container.querySelector('[data-testid="task-chat-composer-attachments"]'), @@ -999,11 +1097,7 @@ describe("TaskChatComposer", () => { )!; flushSync(() => send.click()); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith( - "Please review this.\n\n[notes.txt](/attachments/notes.txt)", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("Please review this.\n\n[notes.txt](/attachments/notes.txt)", undefined, undefined, undefined, expect.any(String)); }); it("blocks send while a file upload is pending, then includes the file once it lands", async () => { @@ -1046,11 +1140,7 @@ describe("TaskChatComposer", () => { expect(sendButton().disabled).toBe(false); flushSync(() => sendButton().click()); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith( - "Here is the file.\n\n[notes.txt](/attachments/notes.txt)", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("Here is the file.\n\n[notes.txt](/attachments/notes.txt)", undefined, undefined, undefined, expect.any(String)); }); it("blocks send while a failed attachment chip remains, then sends after it is removed", async () => { @@ -1082,11 +1172,7 @@ describe("TaskChatComposer", () => { expect(sendButton().disabled).toBe(false); flushSync(() => sendButton().click()); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith( - "Here is the file.", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("Here is the file.", undefined, undefined, undefined, expect.any(String)); }); it("removes an attachment chip via its remove button", async () => { @@ -1196,7 +1282,7 @@ describe("TaskChatComposer", () => { pressKey("Enter", { metaKey: true }); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith(expected.trim(), undefined, undefined); + expect(onAdd).toHaveBeenCalledWith(expected.trim(), undefined, undefined, undefined, expect.any(String)); }); it("inserts a /-command from the autocomplete menu", async () => { @@ -1596,11 +1682,7 @@ describe("TaskChatComposer", () => { pressKey("Enter", { metaKey: true }); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith( - "queued message", - undefined, - undefined, - ); + expect(onAdd).toHaveBeenCalledWith("queued message", undefined, undefined, undefined, expect.any(String)); expect(editable().textContent).toBe(""); expect(localStorage.getItem(draftKey)).toBe("queued message"); expect(localStorage.getItem(`${draftKey}:submission:v1`)).toContain( @@ -1631,7 +1713,7 @@ describe("TaskChatComposer", () => { await flushAsync(); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("first message", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("first message", undefined, undefined, undefined, expect.any(String)); expect(editable().textContent).toBe("next message"); expect(localStorage.getItem(draftKey)).toBe("next message"); }); @@ -1665,7 +1747,7 @@ describe("TaskChatComposer", () => { await flushAsync(); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("first message", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("first message", undefined, undefined, undefined, expect.any(String)); expect( container.querySelector( '[data-testid="task-chat-composer-attachments"]', @@ -1757,7 +1839,7 @@ describe("TaskChatComposer", () => { act(() => root!.render()); expect(editable().textContent).toBe("Please check mobile too."); await act(async () => sendButton().click()); - expect(onAdd).toHaveBeenCalledWith("Please check mobile too.", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("Please check mobile too.", undefined, undefined, undefined, expect.any(String)); }); it("takes precedence over pending questions and queued-message edits", () => { @@ -2608,7 +2690,7 @@ describe("composer Stop", () => { const onStop = vi.fn(async () => {}); const onAdd = vi.fn(async () => {}); render(); - expect(stopButton()?.title).toBe("Stop and pause subtree"); + expect(stopButton()?.title).toBe("Stop response"); pressKey("Enter", { metaKey: true }); expect(onStop).not.toHaveBeenCalled(); expect(onAdd).not.toHaveBeenCalled(); @@ -2617,7 +2699,7 @@ describe("composer Stop", () => { expect(sendButton().disabled).toBe(false); flushSync(() => sendButton().click()); await flushAsync(); - expect(onAdd).toHaveBeenCalledWith("Check mobile too.", undefined, undefined); + expect(onAdd).toHaveBeenCalledWith("Check mobile too.", undefined, undefined, undefined, expect.any(String)); expect(onStop).not.toHaveBeenCalled(); typeText(" \n "); expect(stopButton()?.disabled).toBe(false); diff --git a/ui/src/components/task-chat/TaskChatComposer.tsx b/ui/src/components/task-chat/TaskChatComposer.tsx index f7e0f2665c..e59dcfd42f 100644 --- a/ui/src/components/task-chat/TaskChatComposer.tsx +++ b/ui/src/components/task-chat/TaskChatComposer.tsx @@ -20,6 +20,7 @@ import { loadDraftSubmission, saveDraftSubmission, clearDraftSubmission, + settleDraftSubmission, type ComposerDraftSubmission, } from "@/lib/composer-draft"; import { CommentSubmissionUnknownError } from "@/lib/comment-submit-result"; @@ -104,6 +105,7 @@ interface TaskChatComposerProps { attachmentIds?: string[], clientRequestId?: string, ) => Promise | void; + confirmedSubmissionIds?: ReadonlySet; onStop?: () => Promise; stopPending?: boolean; stopScope?: "leaf" | "subtree"; @@ -381,9 +383,9 @@ function escapeMarkdownLabel(name: string): string { */ export function TaskChatComposer({ onAdd, + confirmedSubmissionIds, onStop, stopPending = false, - stopScope = "leaf", workMode, onWorkModeChange, disabled = false, @@ -460,6 +462,12 @@ export function TaskChatComposer({ typeof update === "function" ? update(attachmentsRef.current) : update; attachmentsRef.current = next; setAttachmentState(next); + const pending = pendingDraftRef.current; + if (pending && pending.draftKey === draftKey) { + saveDraftAttachments(pending.draftKey, next + .filter(item => item.status === "attached" && item.attachmentId) + .map(item => ({ ...item, inline: item.inline === true })), pending.attemptId); + } } const submittingRef = useRef(submitting); submittingRef.current = submitting; @@ -469,6 +477,29 @@ export function TaskChatComposer({ const editorRef = useRef(null); const bodyRef = useRef(body); bodyRef.current = body; + const pendingDraftRef = useRef<{ + draftKey: string; + attemptId: string; + submittedBody: string; + submittedAttachmentIds: string[]; + } | null>(null); + function changeBody(value: string) { + bodyRef.current = value; + setBody(value); + const pending = pendingDraftRef.current; + if (!pending || pending.draftKey !== draftKey || + loadDraftSubmission(pending.draftKey)?.attemptId !== pending.attemptId) return; + // Keep text typed during delivery durable too. Navigation may happen before + // either the request promise or the matching live server receipt arrives. + saveDraft(pending.draftKey, + value ? `${pending.submittedBody}\n\n${value}` : pending.submittedBody, + pending.attemptId); + saveDraftSubmission(pending.draftKey, { + attemptId: pending.attemptId, reviewed: false, + nextDraftOffset: pending.submittedBody.length + (value ? 2 : 0), + submittedAttachmentIds: pending.submittedAttachmentIds, + }); + } const draftTimer = useRef | null>(null); const queuedEditRef = useRef(queuedEdit); queuedEditRef.current = queuedEdit; @@ -971,20 +1002,16 @@ export function TaskChatComposer({ .map((item) => item.attachmentId!), ), ]; - if (conversationMode) - await onAdd(fullBody, reopen, reassignment, attachmentIds.length ? attachmentIds : undefined, attemptId); - else if (attachmentIds.length > 0) - await onAdd(fullBody, reopen, reassignment, attachmentIds); - else await onAdd(fullBody, reopen, reassignment); - if (mountedTaskKey.current !== draftKey) return; - if (draftKey) clearDraftSubmission(draftKey, attemptId); - if (draftKey && bodyRef.current) { - // The editor stays writable while the request is pending. Preserve - // text entered after this submission started as the next draft. - saveDraft(draftKey, bodyRef.current); - } else if (draftKey) { - clearDraft(draftKey); + if (draftKey) { + pendingDraftRef.current = { draftKey, attemptId, submittedBody, submittedAttachmentIds: attachmentIds }; + changeBody(bodyRef.current); } + await onAdd(fullBody, reopen, reassignment, attachmentIds.length ? attachmentIds : undefined, attemptId); + // Navigation does not invalidate the server receipt. Settle the captured + // task before checking whether this composer is still on screen. + if (draftKey) settleDraftSubmission(draftKey, attemptId, + mountedTaskKey.current === draftKey ? bodyRef.current : undefined); + if (mountedTaskKey.current !== draftKey) return; const submittedIds = new Set(submittedAttachments.map((item) => item.id)); setAttachments((current) => current.filter((item) => !submittedIds.has(item.id)), @@ -994,8 +1021,13 @@ export function TaskChatComposer({ } } catch (error) { if (mountedTaskKey.current !== draftKey) return; + const nextDraft = bodyRef.current; if (attemptId && error instanceof CommentSubmissionUnknownError) { - const uncertain = { attemptId, reviewed: false }; + const uncertain = { + attemptId, reviewed: false, + nextDraftOffset: submittedBody.length + (nextDraft ? 2 : 0), + submittedAttachmentIds: submittedAttachments.flatMap(item => item.attachmentId ? [item.attachmentId] : []), + }; setUncertainSubmission(uncertain); if (draftKey && loadDraftSubmission(draftKey)?.attemptId === attemptId) saveDraftSubmission(draftKey, uncertain); @@ -1003,7 +1035,6 @@ export function TaskChatComposer({ clearDraftSubmission(draftKey, attemptId); // Restore the failed message for retry without discarding a next draft // that was entered while the request was pending. - const nextDraft = bodyRef.current; const restoredBody = nextDraft ? `${submittedBody}\n\n${nextDraft}` : submittedBody; @@ -1015,10 +1046,25 @@ export function TaskChatComposer({ if (draftKey) saveDraft(draftKey, restoredBody, attemptId ?? undefined); setBody(restoredBody); } finally { + if (pendingDraftRef.current?.attemptId === attemptId) pendingDraftRef.current = null; setSubmitting(false); } } + useEffect(() => { + if (!uncertainSubmission || !confirmedSubmissionIds?.has(uncertainSubmission.attemptId)) return; + const nextDraft = uncertainSubmission.nextDraftOffset === undefined + ? "" : bodyRef.current.slice(uncertainSubmission.nextDraftOffset); + if (draftKey) settleDraftSubmission(draftKey, uncertainSubmission.attemptId, nextDraft); + setUncertainSubmission(null); + bodyRef.current = nextDraft; + setBody(nextDraft); + const submittedIds = uncertainSubmission.submittedAttachmentIds; + setAttachments(current => submittedIds + ? current.filter(item => !item.attachmentId || !submittedIds.includes(item.attachmentId)) + : []); + }, [confirmedSubmissionIds, draftKey, uncertainSubmission]); + async function reviewUncertainSubmission() { if (!uncertainSubmission) return; setReviewError(false); @@ -1265,7 +1311,7 @@ export function TaskChatComposer({ { + it.each(["issue.attachment_added", "issue.attachment_removed", "issue.work_product_created", "issue.work_product_updated"])("refreshes visible delivered files for %s", action => { + const client = new QueryClient(); + client.setQueryData(queryKeys.issues.detail("issue-1"), { id: "issue-1", companyId: "company-1", identifier: "PAP-1" }); + const invalidate = vi.spyOn(client, "invalidateQueries"); + __liveUpdatesTestUtils.invalidateActivityQueries(client, "company-1", { + entityType: "issue", entityId: "issue-1", actorType: "agent", actorId: "agent-1", action, + }, { userId: "user-1", agentId: null }, { pathname: "/PAP/issues/PAP-1", isForegrounded: true }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.issues.attachments("issue-1") }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.issues.workProducts("issue-1") }); + client.clear(); + }); it("connects trusted local boards without admitting signed-out authenticated users", () => { const canConnect = __liveUpdatesTestUtils.canUseLiveSession; expect(canConnect("success", false, "local_trusted")).toBe(true); diff --git a/ui/src/context/LiveUpdatesProvider.tsx b/ui/src/context/LiveUpdatesProvider.tsx index d875d3f388..d0ee89f2b8 100644 --- a/ui/src/context/LiveUpdatesProvider.tsx +++ b/ui/src/context/LiveUpdatesProvider.tsx @@ -527,6 +527,8 @@ function invalidateVisibleIssueRunQueries( // A final comment can race the last in-flight history fetch. Reconcile // persisted messages after the turn settles. queryClient.invalidateQueries({ queryKey: queryKeys.issues.comments(issueRef) }); + queryClient.invalidateQueries({ queryKey: queryKeys.issues.attachments(issueRef) }); + queryClient.invalidateQueries({ queryKey: queryKeys.issues.workProducts(issueRef) }); queryClient.invalidateQueries({ queryKey: ["issues", "tree-control-state", issueRef] }); } } @@ -1075,6 +1077,8 @@ function buildRunStatusToast( // Interrupt is an intentional conversation control. Its caller gives // feedback; the terminal event must not announce a cancelled/failed run. if (errorCode === "operator_interrupted") return null; + // Workspace contention is ordinary scheduling, not a failed user action. + if (errorCode === "workspace_busy") return null; const contextSource = readString(payload.contextSource); const triggerDetail = readString(payload.triggerDetail); const name = nameOf(agentId) ?? "Agent"; @@ -1342,6 +1346,12 @@ function invalidateActivityQueries( if (action === "issue.comment_added" || action === "issue.conversation_session_started") { queryClient.invalidateQueries({ queryKey: queryKeys.issues.comments(ref), ...invalidationOptions }); } + if (action?.startsWith("issue.attachment_") || action?.startsWith("issue.work_product_")) { + // These cards are durable API objects, not streamed text. Refresh the + // visible task too, including attachments bound to an existing comment. + queryClient.invalidateQueries({ queryKey: queryKeys.issues.attachments(ref) }); + queryClient.invalidateQueries({ queryKey: queryKeys.issues.workProducts(ref) }); + } if (action === "issue.conversation_session_started") { queryClient.invalidateQueries({ queryKey: ["issues", "tree-control-state", ref] }); queryClient.invalidateQueries({ queryKey: queryKeys.issues.interactions(ref) }); diff --git a/ui/src/lib/composer-draft.test.ts b/ui/src/lib/composer-draft.test.ts index 215e8f8fcc..5b37088d00 100644 --- a/ui/src/lib/composer-draft.test.ts +++ b/ui/src/lib/composer-draft.test.ts @@ -9,6 +9,7 @@ import { loadDraftSubmission, saveDraftSubmission, clearDraftSubmission, + settleDraftSubmission, } from "./composer-draft"; describe("task draft upload receipts", () => { @@ -22,6 +23,21 @@ describe("task draft upload receipts", () => { contentPath: `/api/attachments/${id}/content`, }; beforeEach(() => localStorage.clear()); + it("settles only submitted text and attachments while preserving the next draft", () => { + const nextId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3"; + const nextReceipt = { ...receipt, attachmentId: nextId, contentPath: `/api/attachments/${nextId}/content` }; + saveDraft(key, "Sent\n\nNext"); + saveDraftAttachments(key, [receipt]); + saveDraftSubmission(key, { attemptId: id, reviewed: false, nextDraftOffset: 6, submittedAttachmentIds: [id] }); + saveDraftAttachments(key, [receipt, nextReceipt], nextId); + expect(loadDraftAttachments(key)).toEqual([receipt]); + saveDraftAttachments(key, [receipt, nextReceipt], id); + expect(settleDraftSubmission(key, nextId)).toBe(false); + expect(settleDraftSubmission(key, id)).toBe(true); + expect(loadDraft(key)).toBe("Next"); + expect(loadDraftAttachments(key)).toEqual([nextReceipt]); + expect(loadDraftSubmission(key)).toBeNull(); + }); it("keeps chat drafts and pending submission fences within the current tab", () => { sessionStorage.clear(); const chatKey = "paperclip:agent-chat-draft:company:user:agent"; diff --git a/ui/src/lib/composer-draft.ts b/ui/src/lib/composer-draft.ts index 986398fafc..1c28b721aa 100644 --- a/ui/src/lib/composer-draft.ts +++ b/ui/src/lib/composer-draft.ts @@ -57,24 +57,32 @@ export function clearDraft(draftKey: string, attemptId?: string) { export interface ComposerDraftSubmission { attemptId: string; reviewed: boolean; + /** Start of text typed after the submitted body in a restored uncertain draft. */ + nextDraftOffset?: number; + submittedAttachmentIds?: string[]; } -/** Local uncertainty fence, not a server idempotency key or proof of delivery. - * Any retained in-flight intent is uncertain after a reload. */ +/** Retained client request ID. It is not delivery proof until a matching + * server receipt is observed; use the same ID for submission and reconciliation. */ export function loadDraftSubmission( draftKey: string, ): ComposerDraftSubmission | null { try { const raw = draftStorage(draftKey).getItem(`${draftKey}:submission:v1`); - if (!raw || raw.length > 2_048) return null; + if (!raw || raw.length > 16_384) return null; const record = JSON.parse(raw); return record?.version === 1 && record.draftKey === draftKey && - Object.keys(record).length === 4 && + Object.keys(record).every((key) => ["version", "draftKey", "attemptId", "reviewed", "nextDraftOffset", "submittedAttachmentIds"].includes(key)) && typeof record.attemptId === "string" && /^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$/i.test(record.attemptId) && - typeof record.reviewed === "boolean" - ? { attemptId: record.attemptId, reviewed: record.reviewed } + typeof record.reviewed === "boolean" && + (record.nextDraftOffset === undefined || (Number.isSafeInteger(record.nextDraftOffset) && record.nextDraftOffset >= 0)) && + (record.submittedAttachmentIds === undefined || (Array.isArray(record.submittedAttachmentIds) && record.submittedAttachmentIds.length <= 256 && record.submittedAttachmentIds.every((id: unknown) => typeof id === "string" && id.length <= 128))) + ? { attemptId: record.attemptId, reviewed: record.reviewed, + ...(record.nextDraftOffset !== undefined ? { nextDraftOffset: record.nextDraftOffset } : {}), + ...(record.submittedAttachmentIds !== undefined ? { submittedAttachmentIds: record.submittedAttachmentIds } : {}), + } : null; } catch { return null; @@ -107,6 +115,20 @@ export function clearDraftSubmission(draftKey: string, attemptId: string) { } } +/** A late response can settle only its own retained intent, never a newer draft. */ +export function settleDraftSubmission(draftKey: string, attemptId: string, nextDraft?: string): boolean { + const submission = loadDraftSubmission(draftKey); + if (submission?.attemptId !== attemptId) return false; + nextDraft ??= submission.nextDraftOffset === undefined + ? "" : loadDraft(draftKey).slice(submission.nextDraftOffset); + const nextAttachments = submission.submittedAttachmentIds === undefined ? [] + : loadDraftAttachments(draftKey).filter(item => !submission.submittedAttachmentIds!.includes(item.attachmentId)); + clearDraft(draftKey, attemptId); + if (nextDraft) saveDraft(draftKey, nextDraft); + if (nextAttachments.length) saveDraftAttachments(draftKey, nextAttachments); + return true; +} + export interface ComposerDraftAttachment { attachmentId: string; name: string; @@ -178,11 +200,11 @@ export function loadDraftAttachments( } } -export function saveDraftAttachments(draftKey: string, attachments: unknown) { +export function saveDraftAttachments(draftKey: string, attachments: unknown, attemptId?: string) { try { - // In-flight/unknown receipts were saved before the intent. Generic effects - // (including a stale composer's cleanup) must not rewrite that snapshot. - if (!mayWriteDraft(draftKey)) return; + // Only the owning pending request may persist newly uploaded receipts. + // Generic effects and stale composer cleanups must not rewrite its snapshot. + if (!mayWriteDraft(draftKey, attemptId)) return; const selected = draftAttachments(attachments); if (selected.length) draftStorage(draftKey).setItem( diff --git a/ui/src/lib/wait-for-stopped-runs.test.ts b/ui/src/lib/wait-for-stopped-runs.test.ts index 31a9d70c6e..eec959d53a 100644 --- a/ui/src/lib/wait-for-stopped-runs.test.ts +++ b/ui/src/lib/wait-for-stopped-runs.test.ts @@ -68,7 +68,7 @@ describe("stop confirmation", () => { timeoutMs: 1000, }); const assertion = expect(result).rejects.toThrow( - "pause was saved, but work is still stopping", + "stop was requested, but work is still stopping", ); await vi.advanceTimersByTimeAsync(1000); await assertion; @@ -78,7 +78,7 @@ describe("stop confirmation", () => { waitForStoppedRuns(["active"], { getRun: vi.fn().mockRejectedValue(new Error("offline")), }), - ).rejects.toThrow("pause was saved, but stopping could not be verified"); + ).rejects.toThrow("stop was requested, but stopping could not be verified"); }); }); diff --git a/ui/src/lib/wait-for-stopped-runs.ts b/ui/src/lib/wait-for-stopped-runs.ts index fa50aa678c..ff2b0906f8 100644 --- a/ui/src/lib/wait-for-stopped-runs.ts +++ b/ui/src/lib/wait-for-stopped-runs.ts @@ -29,7 +29,7 @@ export async function waitForStoppedRuns( ]); } catch { throw new Error( - "The pause was saved, but stopping could not be verified. Refresh and try Stop again if work is still running.", + "The stop was requested, but stopping could not be verified. Refresh and try Stop again if work is still running.", ); } finally { clearTimeout(timeout); @@ -54,7 +54,7 @@ export async function waitForStoppedRuns( if (remaining.length === 0) return; if (Date.now() >= deadline) { throw new Error( - "The pause was saved, but work is still stopping. Try Stop again if it continues.", + "The stop was requested, but work is still stopping. Try Stop again if it continues.", ); } await new Promise((resolve) => diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 489da7c607..bee582cd41 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -84,6 +84,7 @@ const mockActivityApi = vi.hoisted(() => ({ })); const mockHeartbeatsApi = vi.hoisted(() => ({ + get: vi.fn(), liveRunsForIssue: vi.fn(), activeRunForIssue: vi.fn(), cancel: vi.fn(), @@ -4571,8 +4572,11 @@ describe("IssueDetail", () => { }); it.each(["active-run", "composer"])( - "routes %s Stop and the menu through the same pause operation", + "keeps %s run controls distinct from pausing future work", async (control) => { + mockIssuesApi.createTreeHold.mockClear(); + mockHeartbeatsApi.cancel.mockClear(); + mockHeartbeatsApi.get.mockReset(); const pausePreview = createPausePreview(); pausePreview.totals = { ...pausePreview.totals, @@ -4614,6 +4618,7 @@ describe("IssueDetail", () => { adapterType: "process", }, ]); + mockHeartbeatsApi.get.mockResolvedValue({ id: "run-active-1", status: "cancelled", runtimeMode: "legacy" }); mockAuthApi.getSession.mockResolvedValue({ session: { userId: "user-1" }, user: { id: "user-1" }, @@ -4650,7 +4655,11 @@ describe("IssueDetail", () => { }); await flushReact(); - expect(mockIssuesApi.createTreeHold).toHaveBeenCalledWith("PAP-1", { + if (control === "composer") { + expect(mockHeartbeatsApi.cancel).toHaveBeenCalledWith("run-active-1"); + expect(mockHeartbeatsApi.get).toHaveBeenCalledWith("run-active-1"); + expect(mockIssuesApi.createTreeHold).not.toHaveBeenCalled(); + } else expect(mockIssuesApi.createTreeHold).toHaveBeenCalledWith("PAP-1", { mode: "pause", reason: null, releasePolicy: { strategy: "manual", note: "leaf_pause" }, @@ -4699,6 +4708,7 @@ describe("IssueDetail", () => { ); it("routes live-run finalization actions through run cancellation before issue status update", async () => { + mockHeartbeatsApi.cancel.mockClear(); mockIssuesApi.get.mockResolvedValue( createIssue({ status: "in_progress", diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 20c8f47eba..667a6743fe 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -1281,6 +1281,8 @@ type IssueDetailChatTabProps = { onInterruptQueued: (runId: string | null) => Promise; onDeleteComment?: (commentId: string) => Promise | void; onPauseWorkRun?: (runId: string, feedback?: "composer") => Promise; + onStopResponse?: (runId: string) => Promise; + stopResponsePending?: boolean; pauseWorkPending?: boolean; pauseWorkScope?: "leaf" | "subtree"; runFinalizationActions?: readonly IssueChatRunFinalizationAction[]; @@ -1395,6 +1397,8 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ onInterruptQueued, onDeleteComment, onPauseWorkRun, + onStopResponse, + stopResponsePending, pauseWorkPending, pauseWorkScope, runFinalizationActions, @@ -2442,13 +2446,10 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ onSubmitInteractionVerdicts={onSubmitInteractionVerdicts} issueWorkMode={issueWorkMode} onWorkModeChange={onWorkModeChange} - stopPending={pauseWorkPending} - stopScope={pauseWorkScope} + stopPending={stopResponsePending} onCancelRun={ - interruptibleIssueRun && onPauseWorkRun - ? async () => { - await onPauseWorkRun(interruptibleIssueRun.id, "composer"); - } + interruptibleIssueRun && onStopResponse + ? () => onStopResponse(interruptibleIssueRun.id) : undefined } onImageClick={onImageClick} @@ -4175,6 +4176,16 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS } }, }); + const stopResponse = useMutation({ + mutationFn: async (runId: string) => { + await heartbeatsApi.cancel(runId); + await waitForStoppedRuns([runId]); + }, + onSettled: () => Promise.all([ + queryClient.invalidateQueries({ queryKey: queryKeys.issues.detail(issueId!) }), + queryClient.invalidateQueries({ queryKey: queryKeys.issues.runs(issueId!) }), + ]), + }); const stopAndFinalizeRun = useMutation({ mutationFn: async ({ runId, @@ -4772,15 +4783,18 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS interrupt, reassignment, attachmentIds, + clientRequestId, }: { body: string; reopen?: boolean; interrupt?: boolean; reassignment: CommentReassignment; attachmentIds?: string[]; + clientRequestId?: string; }) => issuesApi.update(issueId!, { comment: body, + commentClientRequestId: clientRequestId, ...(attachmentIds?.length ? { attachmentIds } : {}), assigneeAgentId: reassignment.assigneeAgentId, assigneeUserId: reassignment.assigneeUserId, @@ -6125,6 +6139,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS reopen, reassignment, attachmentIds, + clientRequestId, }); return; } @@ -7781,6 +7796,10 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS .mutateAsync({ commentId }) .then(() => undefined) } + onStopResponse={canManageTreeControl + ? (runId) => stopResponse.mutateAsync(runId) + : undefined} + stopResponsePending={stopResponse.isPending} pauseWorkPending={ executeTreeControl.isPending && executeTreeControl.variables?.mode === "pause" diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index c44df98cb9..8926bdff70 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -5,6 +5,7 @@ import type { ReactNode } from "react"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { getAppStoreDefinition } from "@paperclipai/shared"; import { AppDetail } from "./AppDetail"; import { APP_TABS } from "./app-tabs"; @@ -1172,6 +1173,30 @@ describe("AppDetail", () => { expect(container.textContent).toContain("Which agents can use this connection?"); }); + it.each(["permissions", "review"])("offers a supported replacement for an obsolete Anthropic connection on %s", async (tab) => { + mockParams.tab = tab; + listApplicationsMock.mockResolvedValue({ applications: [] }); + listGalleryMock.mockResolvedValue({ apps: [getAppStoreDefinition("anthropic")!] }); + getConnectionMock.mockResolvedValue(connection({ + name: "Anthropic", + transport: "rest_api", + authKind: "api_key", + config: { sourceTemplateKey: "anthropic", connectionMethodKey: "api-key" }, + healthStatus: "error", + healthMessage: "This connection has no supported tool integration.", + })); + + await renderAppDetail(); + + expect(container.querySelector('input[type="password"]')).toBeNull(); + expect(findButton("Check & reconnect")).toBeUndefined(); + expect(findButton("Reconnect")).toBeUndefined(); + expect(container.textContent).toContain("Connection no longer supported"); + expect(container.textContent).toContain("then remove this connection"); + expect(container.querySelector('a[href="/apps/connect?source=anthropic"]')?.textContent) + .toBe("Add supported connection"); + }); + it("offers retry for a transient GitHub error without asking for another login", async () => { mockParams.tab = "permissions"; getConnectionMock.mockResolvedValue(connection({ diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index d096e55a26..2818f1d0df 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -6,13 +6,18 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { CONNECTABLE_APP_DEFINITIONS, GOOGLE_WORKSPACE_CONNECTOR_PROFILES, getAppStoreDefinition } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ApiError } from "@/api/client"; +import { aiConnectionsApi } from "@/api/ai-connections"; import { queryKeys } from "@/lib/queryKeys"; import { ConnectionSetupFlow } from "@/features/connections/ConnectionSetupFlow"; import { AppsConnect } from "./AppsConnect"; const listGalleryMock = vi.hoisted(() => vi.fn()); const experimentalMock = vi.hoisted(() => vi.fn()); -vi.mock("@/api/instanceSettings", () => ({ instanceSettingsApi: { getExperimental: experimentalMock } })); +vi.mock("@/api/instanceSettings", () => ({ instanceSettingsApi: { + getExperimental: experimentalMock, + get: async () => ({ defaultEnvironmentId: "local-env" }), + getGeneral: async () => ({}), +} })); const listApplicationsMock = vi.hoisted(() => vi.fn()); const listConnectionsMock = vi.hoisted(() => vi.fn()); const getConnectionMock = vi.hoisted(() => vi.fn()); @@ -438,24 +443,43 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { // credential is entered. // ------------------------------------------------------------------------- - it("keeps the existing Anthropic tool method reachable alongside AI authentication", async () => { + it("offers supported Anthropic AI authentication without the obsolete REST tool method", async () => { + const createAiAccount = vi.spyOn(aiConnectionsApi, "create").mockResolvedValue({ + connectionId: "anthropic-ai-account", grantId: "anthropic-ai-grant", + }); mockParams.appKey = "anthropic"; listGalleryMock.mockResolvedValue({ apps: [getAppStoreDefinition("anthropic")] }); - await render(); + const client = new QueryClient({ defaultOptions: { queries: { + retry: false, + staleTime: Infinity, + } } }); + client.setQueryData(queryKeys.environments.list("company-1"), [ + { id: "local-env", name: "Local", driver: "local", status: "active", config: {} }, + ]); + client.setQueryData(queryKeys.environments.capabilities("company-1"), {}); + client.setQueryData(queryKeys.instance.settings, { defaultEnvironmentId: "local-env" }); + client.setQueryData(queryKeys.instance.generalSettings, {}); + client.setQueryData(queryKeys.health, { deploymentMode: "authenticated", localAiLoginSupported: false }); + await render(client); await passAccessStep(); - expect(container.textContent).toContain("How do you want to connect?"); - expect(radioContaining("Claude subscription")).toBeTruthy(); - expect(radioContaining("Claude API key")).toBeTruthy(); - await act(async () => radioContaining("Use an API key")!.click()); + expect(container.textContent).toContain("Connect account"); + expect(container.textContent).toContain("Connection name"); + expect(container.textContent).not.toContain("How do you want to connect?"); + expect(radioContaining("Use an API key")).toBeUndefined(); + expect(container.querySelector('[role="alert"]')).toBeNull(); + await act(async () => buttonContaining("Use API key instead")!.click()); + await act(async () => buttonContaining("Claude")!.click()); await flushReact(); const key = container.querySelector('input[type="password"]'); expect(key).toBeTruthy(); - await act(async () => setInputValue(key!, "fixture-anthropic-tool-key")); + await act(async () => setInputValue(key!, "fixture-anthropic-ai-key")); await act(async () => buttonByText("Connect")!.click()); await flushReact(); - expect(connectAppMock).toHaveBeenCalledWith("company-1", expect.objectContaining({ - galleryKey: "anthropic", connectionMethodKey: "api-key", + expect(createAiAccount).toHaveBeenCalledWith("company-1", expect.objectContaining({ + provider: "anthropic", method: "api_key", apiKey: "fixture-anthropic-ai-key", })); + expect(mockNavigate).toHaveBeenCalledWith("/apps/anthropic-ai-account/permissions"); + expect(connectAppMock).not.toHaveBeenCalled(); expect(container.textContent).not.toContain("Connect for tool access instead"); }); diff --git a/ui/src/pages/apps/app-detail/AdvancedPanel.render.test.tsx b/ui/src/pages/apps/app-detail/AdvancedPanel.render.test.tsx index 1253193821..1db1cdd0d7 100644 --- a/ui/src/pages/apps/app-detail/AdvancedPanel.render.test.tsx +++ b/ui/src/pages/apps/app-detail/AdvancedPanel.render.test.tsx @@ -3,6 +3,7 @@ import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { getAppStoreDefinition } from "@paperclipai/shared"; import { DangerZone } from "./AdvancedPanel"; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -50,6 +51,54 @@ function expandDangerZone(node: HTMLDivElement) { * operator commits. */ describe("DangerZone", () => { + it("keeps removal available without reconnecting an obsolete Anthropic method", () => { + container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const onRemove = vi.fn(); + act(() => root.render( + , + )); + expandDangerZone(container); + const button = (label: string) => Array.from(container!.querySelectorAll("button")) + .find((candidate) => candidate.textContent?.trim() === label); + expect(button("Reconnect")).toBeUndefined(); + act(() => button("Remove app")!.dispatchEvent(new MouseEvent("click", { bubbles: true }))); + act(() => button("Yes, remove it")!.dispatchEvent(new MouseEvent("click", { bubbles: true }))); + expect(onRemove).toHaveBeenCalledOnce(); + act(() => root.unmount()); + }); + it("keeps dangerous actions folded by default", () => { const node = renderDangerZone(); diff --git a/ui/src/pages/apps/app-detail/AdvancedPanel.tsx b/ui/src/pages/apps/app-detail/AdvancedPanel.tsx index ba952948a0..57fecd7c32 100644 --- a/ui/src/pages/apps/app-detail/AdvancedPanel.tsx +++ b/ui/src/pages/apps/app-detail/AdvancedPanel.tsx @@ -22,6 +22,7 @@ import { redactUrlSecrets } from "@/lib/redact-url-secrets"; import { navigateTopLevel } from "@/lib/browserNavigation"; import { prepareOAuthNavigation, savePendingCloudHandoff } from "@/lib/oauthHandoff"; import { cn } from "@/lib/utils"; +import { Link } from "@/lib/router"; import type { AppDetailSectionProps } from "./types"; import { RevokeGrantDialog } from "./IdentitiesSection"; @@ -88,6 +89,15 @@ export function AdvancedPanel({ ); } +function connectionMethodUnavailable(connection: ToolConnection, galleryEntry: AppDefinition | null): boolean { + const methodKey = connection.config?.connectionMethodKey; + return typeof methodKey === "string" + && methodKey.length > 0 + && !!galleryEntry + && Array.isArray(galleryEntry.methods) + && !getAvailableConnectionMethod(galleryEntry, methodKey); +} + function KeySection({ connection, galleryEntry, @@ -200,15 +210,18 @@ export function ReconnectCard({ }); const oauth = connection.authKind === "oauth"; const managedByVercel = connection.credentialSource === "vercel_connect"; + const methodUnavailable = connectionMethodUnavailable(connection, galleryEntry); return (

- {oauth ? "Reconnect required" : "This app needs reconnecting"} + {methodUnavailable ? "Connection no longer supported" : oauth ? "Reconnect required" : "This app needs reconnecting"}

- {connection.healthMessage?.trim() || (oauth + {methodUnavailable + ? "Add a supported connection from Connectors, then remove this connection." + : connection.healthMessage?.trim() || (oauth ? "Authorization expired or was revoked. Sign in again to restore access." : "The key stopped working. Paste a new one to get it back online.")}

@@ -218,6 +231,12 @@ export function ReconnectCard({

{reconnectUnavailableMessage ?? "You don't have permission to reconnect this identity."}

+ ) : methodUnavailable ? ( + ) : onReconnect ? ( ) : managedByVercel && !oauth ? ( @@ -449,6 +468,7 @@ export function DangerZone({ const paused = connection ? connection.enabled === false || connection.status === "disabled" : false; + const methodUnavailable = connection ? connectionMethodUnavailable(connection, galleryEntry) : false; return ( ) : null} - {connection && connection.authKind !== "oauth" ? ( + {connection && !methodUnavailable && connection.authKind !== "oauth" ? (
) : null} - {connection?.authKind === "oauth" && (onReconnectIdentity || !canReplaceCredential) ? ( + {connection?.authKind === "oauth" && !methodUnavailable && (onReconnectIdentity || !canReplaceCredential) ? (

Reconnect