From ad861a7f9418fe53b50dc71ece9fa90913295f59 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:25:59 -0500 Subject: [PATCH 01/11] fix(runner): allow bounded controller admission before welcome (#17735) ## Thinking Path > - Paperclip manages AI agents and preserves their work across execution failures. > - Native Runner authentication exchanges a one-use bootstrap ticket for a connection lease. > - The controller must admit the connection and persist that lease before it sends welcome. > - Runnerd waited only two seconds for welcome, even when its connection setup budget allowed more time. > - A slow valid welcome therefore looked like a consumed-ticket failure before provider work could start. > - This pull request uses the existing ten-second connection setup budget for welcome. > - The benefit is reliable startup during bounded controller delays, with the same authentication and replay checks. ## Linked Issues or Issue Description **What happened?** A valid native Runner bootstrap can fail while the controller completes admission. The runner then reports `bootstrap connection failed closed; provide a fresh one-use ticket`. A later recovery attempt can report an indeterminate authority and hide the startup error behind a finalization deadline. **Expected behavior** A valid welcome can arrive within the connection setup budget. An earlier execution or reconnect deadline still ends the wait. A missing or invalid welcome never permits bootstrap-ticket replay. **Steps to reproduce** 1. Run the real Runner WebSocket transport against an authenticated controller. 2. Hold the welcome for three seconds after the client proof arrives. 3. Before this change, authentication fails with `WebSocket authentication message deadline elapsed` and `bootstrap_maybe_consumed: true`. 4. With this change, the runner accepts the bound lease and restores the normal read timeout. **Paperclip version or commit** Reproduced against `a1f77821b9` before the production change. **Agent adapter(s) involved** Native Paperclip Runner. The transport is shared by its provider drivers. Related: #17401 reduces repeated work in warm-recovery admission. This change covers delayed welcome delivery for fresh authentication too. It does not replace that optimization. #17594 addresses a separate lifetime failure after controller outages. ## What Changed - Use the ten-second connection setup budget for the authenticated welcome. - Keep the execution/reconnect deadline as an earlier bound. - Add real WebSocket regressions for a delayed welcome and a shorter lifecycle deadline. - Add a real Runner process test proving a delayed admission starts one provider and keeps it through two turns. - Document the startup contract and its recovery verification scope. ## Verification - Red: the delayed-welcome regression failed against unchanged production code with a message deadline and `bootstrap_maybe_consumed: true`. - Green: all 402 Runner core library tests pass. The suite includes authentication, ticket fencing after a lost welcome, identity rejection, replay, and no repeated provider effects. - Map and module checks pass. - Workspace `pnpm -r typecheck` and `pnpm build` pass. The new process integration test passes (one provider process, one thread, two turns, suspended with an empty outbox). - Live acceptance at this branch's production Runner build passes on macOS arm64 with pinned OpenCode 1.18.34 and `openrouter/deepseek/deepseek-v4.1-flash-1017`: delay admission by three seconds, create a file, suspend, resume the same provider session under a fresh run authority, read the first file, and write a second file. Both useful outputs match exactly; final runner state is suspended. - Preserved setup failures: a non-pinned executable was rejected, and the debug binary exceeded cold provider startup admission. The pinned executable and release binary passed. The first harness also tried to reuse a completed task object; the follow-up used the normal resume path. Exactly two paid model turns ran. - The full local `pnpm test:run` was attempted and stopped after the Mac exhausted PostgreSQL semaphore capacity (`could not create semaphores`); it is not a local full-suite pass. - Current-head CI is green at `df5d5494c9f9f444114aaf12e095a554110d2e14`: all checks passing or non-applicably skipped, zero failures or pending checks. The additional Superagent security scan triggered by ready-for-review also passed. These include all general/serialized test shards, Runner Rust/Vitest, E2E, typecheck, build, and the final verify gate. Greptile reviewed that exact commit at 5/5 with no findings or inline comments. - This proves the actual native Runner/provider path locally. It does not prove rollout or recovery on a hosted Linux/amd64 execution environment. ## Risks - A stalled welcome can take up to eight seconds longer to fail when the configured lifecycle allows it. Shorter execution/reconnect deadlines still take precedence. - Authentication proofs, credential expiry, identity binding, and one-use-ticket fencing remain enforced. - This does not repair provider-side grammar errors or authorize replay of an indeterminate historical run. Recovery of existing work requires the normal stopped-owner and retained-state checks. - No schema or migration change. A new Runner binary must reach the execution host before this change affects remote runs. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository inspection, code execution, and browser tools. The exact serving submodel identifier and context window were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/execution-semantics.md | 11 +- maps/feature-map/recovery.md | 7 ++ .../runner-core/src/durable/transport.rs | 116 +++++++++++++++++- .../live/runnerd-welcome.integration.test.ts | 62 ++++++++++ 4 files changed, 192 insertions(+), 4 deletions(-) create mode 100644 packages/paperclip-runner/src/live/runnerd-welcome.integration.test.ts diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 52dbcac821e..b64eaaa16f0 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -730,7 +730,16 @@ recovery path with the specific scan cause and an operator action. Generic stranded-work recovery must not grant another budget for these errors. This does not automatically replay historical generic `setup_failed` runs. -### ACP startup handshake bound +### Runner authentication and ACP startup handshake bounds + +The native Runner waits up to the ten-second connection setup budget for the +authenticated welcome. The current execution or reconnect deadline can shorten +that wait. Controller admission and durable lease persistence happen before the +welcome, so a short internal response limit must not discard an otherwise valid +bootstrap ticket during that work. A failed welcome still makes the one-use +ticket unsafe to replay. Authentication proofs, credential expiry, and identity +checks remain required. Steady-state reads return to their 250 ms polling bound +after the welcome. An adapter-backed live path also requires that the ACP startup handshake itself cannot hang forever. The engine bounds the handshake with a fixed startup deadline and a poll of the duplex control-channel disposition. Either condition ends the handshake and reports a closed, typed code, so the issue can reach a settled disposition instead of staying `in_progress` with no observable next action. diff --git a/maps/feature-map/recovery.md b/maps/feature-map/recovery.md index a6f49522fef..b72eb30ea42 100644 --- a/maps/feature-map/recovery.md +++ b/maps/feature-map/recovery.md @@ -175,6 +175,13 @@ provider restart. For that journey, interrupt a disposable deployment after workspace preparation and before its first executor claim, then confirm one provider turn starts on the original run through the normal admission gates. +The [Runner transport regressions](../../packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs) +exercise a real WebSocket welcome delayed beyond two seconds. Authentication +uses the ten-second connection setup budget, clipped by the current lifecycle +deadline. The cases also retain failed-welcome ticket fencing, identity checks, +and reconnect replay without repeated provider effects. These transport cases +do not establish hosted provider acceptance. + Active remote process supervision distinguishes a failed health read from a confirmed exit. The [executor tests](../../server/src/services/native-runtime/native-session-executor.test.ts) exercise timeout recovery with a real listening process, unchanged ownership, diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs b/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs index 135a5feb962..ae2be51ddfb 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs @@ -26,7 +26,7 @@ use super::{ const SECURE_FRAME_SCHEMA: &str = "paperclip.runner.secure-frame.v1"; const AUTH_TIMEOUT: Duration = Duration::from_secs(2); -const WELCOME_TIMEOUT: Duration = Duration::from_secs(2); +const WELCOME_TIMEOUT: Duration = CONNECT_TOTAL_TIMEOUT; const RUNTIME_READ_TIMEOUT: Duration = Duration::from_millis(250); const CONNECT_ATTEMPT_TIMEOUT: Duration = Duration::from_secs(3); const CONNECT_TOTAL_TIMEOUT: Duration = Duration::from_secs(10); @@ -1106,8 +1106,9 @@ impl AuthenticatedTransport { max_frame_bytes: config.max_frame_bytes, }; // Ticket validation and lease persistence happen before the server - // emits welcome. Keep that work bounded without assuming a loaded - // control-plane event loop can always respond within 250 ms. + // emits welcome. Give that durable admission the connection setup + // budget; a two-second wait can discard a consumed ticket under + // controller load. The execution/reconnect deadline still wins. let welcome_deadline = bounded_operation_deadline(connect_deadline, WELCOME_TIMEOUT)?; let welcome_timeout = welcome_deadline.saturating_duration_since(Instant::now()); if welcome_timeout.is_zero() { @@ -2358,6 +2359,115 @@ mod tests { assert!(validate_control_identity(&envelope, &state, Some(&connection)).is_err()); } + #[test] + fn bootstrap_welcome_waits_for_bounded_controller_admission() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let mut config = config(port); + config.max_runtime = Duration::from_secs(10); + let state = test_state(&config); + let server_config = config.clone(); + let server_state = state.clone(); + let server = thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let mut socket = accept(stream).unwrap(); + let expires = current_unix_ms().unwrap() + 60_000; + let mut secure = server_authenticate( + &mut socket, + &server_config, + &server_state, + ServerCredential { + token: "bootstrap-secret", + kind: "bootstrap", + lease_id: None, + expires_at_unix_ms: expires, + revocation_epoch: 0, + }, + ); + // The authority has the client proof. Durable admission and lease + // persistence may take longer than the old two-second welcome wait. + thread::sleep(Duration::from_secs(3)); + send_secure( + &mut socket, + &mut secure, + &server_config, + &welcome( + &server_state, + "connection_1", + Some("lease-secret"), + expires, + 0, + vec![], + ), + ); + }); + let endpoint = RunnerTransportEndpoint::new(&config.connect_url, &config.run_id).unwrap(); + let ticket = BootstrapTicket::new("bootstrap-secret".to_owned()).unwrap(); + let result = AuthenticatedTransport::connect( + &endpoint, + &config, + &state, + Some(&ticket), + None, + Instant::now() + config.max_runtime, + ); + server.join().unwrap(); + let (transport, greeting) = result.unwrap().unwrap(); + assert_eq!(greeting.lease.unwrap().expose().unwrap(), "lease-secret"); + assert_eq!( + transport.socket.read_timeout().unwrap(), + Some(RUNTIME_READ_TIMEOUT) + ); + } + + #[test] + fn bootstrap_welcome_retains_the_shorter_lifecycle_deadline() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let config = config(port); + let state = test_state(&config); + let server_config = config.clone(); + let server_state = state.clone(); + let (release, held) = mpsc::channel(); + let server = thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let mut socket = accept(stream).unwrap(); + server_authenticate( + &mut socket, + &server_config, + &server_state, + ServerCredential { + token: "bootstrap-secret", + kind: "bootstrap", + lease_id: None, + expires_at_unix_ms: current_unix_ms().unwrap() + 60_000, + revocation_epoch: 0, + }, + ); + held.recv_timeout(Duration::from_secs(2)).unwrap(); + }); + let endpoint = RunnerTransportEndpoint::new(&config.connect_url, &config.run_id).unwrap(); + let ticket = BootstrapTicket::new("bootstrap-secret".to_owned()).unwrap(); + let started = Instant::now(); + let result = AuthenticatedTransport::connect( + &endpoint, + &config, + &state, + Some(&ticket), + None, + started + Duration::from_secs(1), + ); + release.send(()).unwrap(); + server.join().unwrap(); + let failure = match result { + Err(error) => error, + Ok(_) => panic!("welcome authenticated after the lifecycle deadline"), + }; + assert!(failure.bootstrap_maybe_consumed); + assert!(started.elapsed() < Duration::from_secs(2)); + assert!(failure.error.to_string().contains("deadline elapsed")); + } + #[test] fn authenticates_bootstrap_and_receives_bound_lease() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); diff --git a/packages/paperclip-runner/src/live/runnerd-welcome.integration.test.ts b/packages/paperclip-runner/src/live/runnerd-welcome.integration.test.ts new file mode 100644 index 00000000000..49d44895434 --- /dev/null +++ b/packages/paperclip-runner/src/live/runnerd-welcome.integration.test.ts @@ -0,0 +1,62 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { expect, it, vi } from "vitest"; +import { createCapabilityRunnerdCodexTransport, defaultCapabilityRunnerdBinary } from "./runnerd-codex-transport.js"; + +it("starts one provider after delayed bootstrap admission and retains it for a second turn", async () => { + const root = await mkdtemp(join(tmpdir(), "runnerd-delayed-welcome-")); + const calls = join(root, "provider-calls.log"); + let admissionDelayed = false; + let admissionTimer: ReturnType | undefined; + const bundle = createCapabilityRunnerdCodexTransport({ + runnerBinary: defaultCapabilityRunnerdBinary(), + codexCommand: resolve(import.meta.dirname, "../../runner/target/debug/fake-codex-app-server"), + codexArgs: ["--state-file", join(root, "provider.json"), "--call-log", calls, "--record-process-start", "--durable-turn-ids"], + stateDirectory: join(root, "state"), + controlPlaneRegistration: async authority => { + const attach = authority.attachWireConnection.bind(authority); + vi.spyOn(authority, "attachWireConnection").mockImplementation(wire => attach({ + sendJson: value => wire.sendJson(value), + close: code => wire.close(code), + onClose: listener => wire.onClose(listener), + onJson: listener => wire.onJson(value => { + if (!admissionDelayed && (value as { kind?: string }).kind === "auth_response") { + admissionDelayed = true; + admissionTimer = setTimeout(() => listener(value), 3_000); + } else listener(value); + }), + })); + await authority.start(); + return { connectUrl: authority.connectUrl, release: () => authority.stop() }; + }, + }); + const terminalTurns: unknown[] = []; + try { + const opened = await bundle.transport.request("thread/start", { cwd: root, dynamicTools: [] }); + expect(admissionDelayed).toBe(true); + for (const text of ["Complete the first turn.", "Continue the same session."]) { + await bundle.transport.request("turn/start", { input: [{ type: "text", text }] }); + for await (const event of bundle.transport.notifications()) { + if (event.method === "turn/completed") { + expect(event.params).toMatchObject({ threadId: opened.thread.id }); + terminalTurns.push(event.params); + break; + } + } + } + await bundle.transport.close(); + const methods = (await readFile(calls, "utf8")).trim().split("\n"); + expect(methods.filter(method => method === "process-start")).toHaveLength(1); + expect(methods.filter(method => method === "thread/start")).toHaveLength(1); + expect(methods.filter(method => method === "turn/start")).toHaveLength(2); + expect(terminalTurns).toHaveLength(2); + const runner = JSON.parse(await readFile(join(root, "state/runner/runner-state.json"), "utf8")); + expect(runner.lifecycle).toBe("suspended"); + expect(runner.outbox).toEqual([]); + } finally { + if (admissionTimer) clearTimeout(admissionTimer); + await bundle.transport.close(); + await rm(root, { recursive: true, force: true }); + } +}, 30_000); From 338bf757617ca447518b472c31002474a1968dc9 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:26:45 -0500 Subject: [PATCH 02/11] fix(chat): admit task Retry after GitHub reconnect (#17733) ## Thinking Path > - Paperclip manages agents and their tasks. > - Task Retry must preserve saved input and enforce current access. > - A GitHub reconnect invalidates old runtime callbacks. > - Retry reused that old callback authority and rejected valid new intent. > - Retry also replayed an older failed request when a follow-up was already saved. > - This change admits a deliberate Retry under current authority and transfers saved input once. ## Linked Issues or Issue Description Refs #16965, #17053 and #17317. These repairs preserve work and contain old executors. This change fixes the subsequent chat admission boundary. A search found no overlapping open reconnect/admission fix. **What happened?** Ordinary task Retry returned `chat_failed_run_retry_not_authorized` after the same GitHub App reconnected. The endpoint was healthy, but the saved request still carried its old runtime generation. A newer queued follow-up also made Retry reject the older failed request instead of continuing the follow-up. No successor was admitted. HTTP qualification found two related defects. Concurrent clicks could omit the canonical chat action from a response. Signed-in board dispatch also passed an incomplete task resource to the permission resolver, which rejected a permitted mutation. **Expected behavior** A deliberate Retry checks current authority for the same App, sender, repository, conversation and task. It continues saved input once. A later reconnect, changed App, revoked access or changed source prevents dispatch. Old runtime callbacks remain invalid. **Steps to reproduce** 1. Admit a GitHub request and record a failed native run. 2. Save a follow-up while execution is blocked. 3. Reconnect the same GitHub App. 4. Click ordinary task Retry. 5. Before this change, admission returns 409. Without reconnect, the newer input alone also causes 409. **Paperclip version or commit** Baseline source: `41c42895fa862c586ff841f8c50200d93ffc83a4`. **Deployment mode** Source checkout on macOS. Regressions use real PostgreSQL transactions and the assembled chat, HTTP route and heartbeat admission services. Provider boundaries are simulated. No hosted recovery is claimed. ## What Changed - Record the admitting App identity for new GitHub deliveries. Historical deliveries can use their original working-comment binding. Reauthorize deliberate Retry under a pinned current generation and credential-reference fingerprint. - Select the complete saved-input queue from the failed run's actual conversation before replaying the older failed batch. Keep exact-run Retry bounded while ordinary task Retry supports larger queues. Transfer its receipts with the dispatch transaction. Recover the same ownership after restart or a lost response. Release unadmitted input after a definitive rejection. - Preserve canonical chat action IDs across concurrent clicks. Supply current task status and assignment fields to board mutation checks. - Keep exact-run Retry, stale webhook, containment, source integrity, publication, pause and budget checks. Update recovery contracts and verification recipes. ## Verification - Red: all three new PostgreSQL regressions failed on unchanged production code. Reconnect failed at chat authorization. Newer input failed at the obsolete-source guard. The combined case also failed. - Green: 61 selected chat integration cases passed. These include 27 new cases and 34 existing cases for containment, retained-source recovery, exact batches and source revocation. - A further red/green PostgreSQL regression reproduces the 51-message overflow identified during review. All 51 messages reach one successor, including a later Retry of that successor. A separate 52-message consumed-batch regression also passes. - A competing conversation retains its own deferred message. Distinct automatic policy contexts are checked independently; revoking only the later policy blocks the whole unadmitted batch. Batch validation still rejects a later message with a changed App binding or deleted source. A real PostgreSQL query capture reduced the 51-message preparation from 1,103 calls to fewer than 180. - All 16 durable task Retry tests passed. - The real HTTP route returns 202 and admits one queued successor for concurrent deliveries. Fault injection covers rollback during input handoff, response loss after dispatch commit, and permission loss when resuming an existing durable wait. A wait only adopts input after current admission succeeds; a one-shot settlement marker keeps status projection read-only. A new deliberate Retry works after another reconnect or a failed successor. - Current-App, repository, sender, source edit/delete, reassignment, endpoint pause and board-permission controls passed. Historical deliveries remain unchanged. - Final-head CI: 55 checks passed, three expected skips. The unchanged minute-bucket rate-limit assertion passed locally against PostgreSQL and on its one CI rerun. Greptile reviewed the final commit at 5/5 with no unresolved threads. - Workspace typecheck and full build passed. Map alignment, module boundaries, UI token gates and `git diff --check` passed. - Hosted task recovery and a live provider turn remain unverified. The local tests hold agent capacity to inspect admission without starting a model or publishing a GitHub response. ## Risks - A deliberate GitHub Retry can admit old saved input under current connection authority. It requires the same App binding and current access. It pins that admission; it does not rewrite the historical delivery or make stale callbacks valid. - The durable chat queue has an optional admission callback. It transfers input only when the transaction contains a valid queued execution or saved wait. Other chat consumers have no callback and retain their behavior. - No migration, Cloud dependency, deployment, configuration change or workspace cleanup is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source inspection, code execution and GitHub tools. The exact serving variant and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/task-recovery.md | 22 +- maps/agent-access-map/recovery.md | 7 + maps/feature-map/recovery.md | 8 + .../chat-channels.integration.test.ts | 353 ++++++++++++++++++ server/src/services/chat-channels.ts | 18 +- .../chat-channels/credential-fingerprint.ts | 10 + .../chat-channels/failed-run-retry.ts | 206 ++++++---- .../chat-channels/github/task-preparation.ts | 2 +- .../github/task-retry-authorization.ts | 126 +++++++ .../inbound/wakeup-authorization.ts | 17 +- .../chat-channels/task-retry-input.ts | 95 +++++ server/src/services/durable-chat-wakeup.ts | 23 +- server/src/services/heartbeat/queue.ts | 28 +- server/src/services/task-retry.ts | 15 + 14 files changed, 808 insertions(+), 122 deletions(-) create mode 100644 server/src/services/chat-channels/credential-fingerprint.ts create mode 100644 server/src/services/chat-channels/github/task-retry-authorization.ts create mode 100644 server/src/services/chat-channels/task-retry-input.ts diff --git a/doc/task-recovery.md b/doc/task-recovery.md index 870b8299a34..714d921a97b 100644 --- a/doc/task-recovery.md +++ b/doc/task-recovery.md @@ -15,8 +15,26 @@ new generation; a repeated HTTP request cannot. The existing exact-run retry endpoint retains its source-bound contract. Connected conversations use their existing inbound authorization owner. A board -Retry changes the workspace strategy, not the external principal, source batch, -conversation access or publication authority. +Retry preserves external principal, conversation access and publication checks. +A GitHub Retry selects already-saved, undelivered input before replaying the +failed run's older request. The selected receipts transfer to the retry owner +under the dispatch transaction. They link to its one successor, including after +a restart or a lost dispatch response. Rejected, unadmitted retries release the +saved input for another deliberate Retry. + +A deliberate GitHub Retry can admit saved input after the same App reconnects. +It verifies the App binding in the accepted delivery or its historical working +comment receipt, then checks current sender, repository, conversation and task +access. The retry records the current runtime generation and credential-reference +fingerprint. Another reconnect invalidates that admission. Historical deliveries, +stale webhook callbacks and the exact-run retry contract retain their fences. + +Task Retry keeps the complete saved-input batch, including batches larger than +the exact-run endpoint's limit. Source rows, receipts and invalidations are read +in batches. Within one transaction, messages with the same principal and automatic +policy context share a current-access check; every message still proves its App, +destination, comment link and immutable source. Different automatic policy contexts +receive separate access checks. Input in another conversation remains queued. A connected-conversation Retry can save its exact request while the matching system recovery generation verifies the old executor. Saving that wait grants diff --git a/maps/agent-access-map/recovery.md b/maps/agent-access-map/recovery.md index e0c6003c9f2..e1973773a38 100644 --- a/maps/agent-access-map/recovery.md +++ b/maps/agent-access-map/recovery.md @@ -4,6 +4,13 @@ Product map: [Recovery](../feature-map/recovery.md). Feature ID: `recovery`. +Ordinary board Retry can reauthorize saved GitHub input for the same App after +reconnect. It pins current runtime authority and preserves sender, repository, +conversation and task checks. Pending input takes precedence over the older +failed request and transfers once to the retry owner. This grants no new agent +Retry API or stale webhook authority. Local PostgreSQL and HTTP-route regressions +cover these transitions; hosted execution and delivery remain separate acceptance. + Source revision: `3bbc2104b65caf9f7763938a01260a5673e12f7a`. Review depth and executed proof are separate. The operation records enumerate the named behavior and access boundaries. Their evidence states describe the reviewed sources; run results and environment limits belong in the PR or verification artifacts. A listed API or source test does not establish a live authorized journey. Follow the [map contract](../README.md). diff --git a/maps/feature-map/recovery.md b/maps/feature-map/recovery.md index b72eb30ea42..8c56541e248 100644 --- a/maps/feature-map/recovery.md +++ b/maps/feature-map/recovery.md @@ -89,6 +89,14 @@ The connected-chat database regression in exercises admission and queue continuation. It does not establish hosted provider recovery or successful delivery. +For GitHub, save a follow-up behind a failed run, then reconnect the same App. +Click ordinary Retry and verify that one successor receives the saved follow-up +instead of replaying the older request. Reload and repeat the same request key; +there must be one successor and no duplicate publication. A second reconnect +after admission, changed App, revoked sender or changed repository must prevent +dispatch. An unadmitted rejection keeps the follow-up available for another +deliberate Retry. Historical deliveries must remain unchanged. + ### `post-stop-message` After a stop, send a new instruction on the same task or inspect one already diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index f844a18bce3..095ca9aa105 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -131,6 +131,7 @@ import { deliverReconciledExecutions } from "../services/execution-recovery-reso import { unadmittedChatWakeupCondition, authorizeFailedChatRunRetryWake, + createDurableChatWakeupRequest, } from "../services/durable-chat-wakeup.js"; import { createChatWebhookDiagnostics, @@ -60661,6 +60662,358 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { return { ...context, issue, action, receipt, runId }; } + async function currentGitHubTaskRetryFixture(withPending = false, pendingCount = 1) { + const f = await failedChatRetryFixture("github", "42", true); + const companyId = f.fixture.companyId, issueId = f.issue.id; + await githubResponseCommentService(db, f.providerFetch).tryAcknowledge(f.action.deliveryId!); + await db.update(heartbeatRuns).set({ runtimeMode: "native", nativeIssueId: issueId, + nativePhase: "terminal_failure", runnerProfileJson: {} }).where(eq(heartbeatRuns.id, f.runId)); + await db.insert(nativeRunFinalizations).values({ companyId, issueId, runId: f.runId, phase: "terminal_failure" }); + await db.insert(heartbeatRunEvents).values({ companyId, agentId: f.fixture.assignedAgentId, runId: f.runId, + seq: 1, eventType: "native.local_process_stopped", payload: { processPid: 999999999, processGroupId: null } }); + let pending: typeof chatActions.$inferSelect | undefined; + if (withPending) { + for (let index = 0; index < pendingCount; index++) { + await deliverMessage({ callbacks: f.runtime.configurations.get(f.endpoint.id)!.callbacks, + endpointId: f.endpoint.id, provider: "github", thread: f.thread.thread, + message: makeMessage({ id: String(990092 + index), text: `@maya continue saved follow-up ${index}`, userId: "42", mentioned: true }), + trigger: "mention" }); + const [{ source }] = await db.select({ source: chatActions }).from(chatActions) + .innerJoin(chatDeliveries, eq(chatDeliveries.id, chatActions.deliveryId)).where(and( + eq(chatActions.conversationId, f.conversation.id), eq(chatActions.kind, "inbound_wakeup"), + eq(chatDeliveries.providerEventId, `${f.thread.thread.id}:${990092 + index}`))); + pending ??= source; + await db.update(agentWakeupRequests).set({ status: "deferred_issue_execution", runId: null }) + .where(eq(agentWakeupRequests.id, source.id)); + await githubResponseCommentService(db, f.providerFetch).tryAcknowledge(source.deliveryId!); + } + } + const [originalDelivery] = await db.select().from(chatDeliveries).where(eq(chatDeliveries.id, f.action.deliveryId!)); + + await db.update(agents).set({ adapterType: "paperclip_runner", status: "running", + runtimeConfig: { heartbeat: { wakeOnDemand: true, maxConcurrentRuns: 1 } } }) + .where(eq(agents.id, f.fixture.assignedAgentId)); + await db.insert(heartbeatRuns).values({ companyId, agentId: f.fixture.assignedAgentId, + status: "running", startedAt: new Date() }); + f.wakeup.mockImplementation(heartbeatService(db).wakeup); + return { ...f, pending, originalDelivery }; + } + + it.each(["reconnected", "newer_input", "reconnected_newer_input"] as const)( + "admits current GitHub task Retry without replaying obsolete input: %s", async kind => { + const f = await currentGitHubTaskRetryFixture(kind.includes("newer_input")); + const { pending, originalDelivery } = f; + try { + const companyId = f.fixture.companyId, issueId = f.issue.id; + if (kind.includes("reconnected")) await f.service.configure(f.endpoint.id, { action: "reconnect" }, "owner-user"); + const input = { db, companyId, issueId, actorId: "owner-user", actorSource: "cloud_tenant", + observedRunId: f.runId, idempotencyKey: randomUUID(), prepareChatRetry: f.service.prepareFailedChatRunRetry }; + const request = await requestTaskRetry(input); + expect(request.accepted).toBe(true); + expect((await requestTaskRetry(input)).requestId).toBe(request.requestId); + expect(await f.service.processFailedChatRunRetry(request.chatActionId!)).toMatchObject({ status: "queued" }); + const [wake] = await readAgentWakeupRequestsById(request.chatActionId!); + expect(wake.runId).toBeTruthy(); + const expectedComment = pending?.payload.commentId ?? f.action.payload.commentId; + const [run] = await readHeartbeatRunsById(wake.runId!); + expect(run).toMatchObject({ status: "queued", retryOfRunId: f.runId, + contextSnapshot: { wakeCommentIds: [expectedComment], taskRetryRequestId: request.requestId } }); + if (pending) expect((await readAgentWakeupRequestsById(pending.id))[0]) + .toMatchObject({ status: "coalesced", runId: wake.runId }); + await f.service.processFailedChatRunRetry(request.chatActionId!); + await heartbeatService(db).resumeQueuedRuns(); + expect((await readHeartbeatRunsByCompanyId(companyId)).filter(r => r.contextSnapshot?.taskRetryRequestId === request.requestId)) + .toHaveLength(1); + expect((await db.select().from(chatDeliveries).where(eq(chatDeliveries.id, originalDelivery.id)))[0].normalizedEvent) + .toEqual(originalDelivery.normalizedEvent); + } finally { await f.service.shutdown(); } + }, + ); + + it.each(["legacy_receipt", "concurrent_clicks", "lost_dispatch_reply", "restart", "retry_failed_successor", "http", "handoff_rollback", "board_permission_wait", "peer_conversation", "backlog_51", "consumed_backlog_52"] as const)( + "preserves one current GitHub Retry across %s", async mode => { + const f = await currentGitHubTaskRetryFixture(true, mode.includes("backlog") ? 51 : mode === "peer_conversation" ? 2 : 1); + try { + const companyId = f.fixture.companyId, issueId = f.issue.id; + if (mode === "consumed_backlog_52") { + const sources = await db.select().from(chatActions).where(and(eq(chatActions.conversationId, f.conversation.id), + eq(chatActions.kind, "inbound_wakeup"))).orderBy(asc(chatActions.createdAt), asc(chatActions.id)); + expect(sources).toHaveLength(52); + const commentIds = sources.map(source => source.payload.commentId); + await db.update(heartbeatRuns).set({ finishedAt: new Date(), contextSnapshot: { + ...(await readHeartbeatRunsById(f.runId))[0].contextSnapshot, + wakeCommentIds: commentIds, wakeCommentId: commentIds.at(-1), + } }).where(eq(heartbeatRuns.id, f.runId)); + await db.update(agentWakeupRequests).set({ runId: f.runId, status: "coalesced", + payload: sql`${agentWakeupRequests.payload} || ${JSON.stringify({ coalescedIntoWakeupRequestId: f.action.id })}::jsonb`, + }).where(and(inArray(agentWakeupRequests.id, sources.map(source => source.id)), + sql`${agentWakeupRequests.id} <> ${f.action.id}::uuid`)); + } + let peerInput: typeof chatActions.$inferSelect | undefined; + if (mode === "peer_conversation") { + const originId = randomUUID(); + await db.insert(chatConversations).values({ ...f.conversation, id: originId, sessionGeneration: 2 }); + await db.update(chatActions).set({ conversationId: originId, + payload: sql`jsonb_set(${chatActions.payload}, '{sessionGeneration}', '2'::jsonb)`, + }).where(inArray(chatActions.id, [f.action.id, f.pending!.id])); + await db.update(chatDeliveries).set({ conversationId: originId }) + .where(inArray(chatDeliveries.id, [f.action.deliveryId!, f.pending!.deliveryId!])); + await db.update(chatMessageLinks).set({ conversationId: originId }) + .where(inArray(chatMessageLinks.deliveryId, [f.action.deliveryId!, f.pending!.deliveryId!])); + [peerInput] = await db.select().from(chatActions).where(and(eq(chatActions.conversationId, f.conversation.id), + eq(chatActions.kind, "inbound_wakeup"))); + expect(peerInput).toBeDefined(); + } + if (mode === "legacy_receipt") { + const [originalEndpoint] = await readChatEndpointsById(f.endpoint.id); + await db.insert(chatActions).values({ companyId, endpointId: f.endpoint.id, + conversationId: f.conversation.id, deliveryId: f.pending!.deliveryId, + kind: "github_response_comment", providerActionId: `github_response:${f.pending!.deliveryId}`, + payload: { botExternalId: originalEndpoint.botExternalId, agentId: f.fixture.assignedAgentId }, + status: "processed" }).onConflictDoNothing(); + await db.update(chatDeliveries).set({ + normalizedEvent: sql`jsonb_set(${chatDeliveries.normalizedEvent}, '{githubAuthority}', + (${chatDeliveries.normalizedEvent}->'githubAuthority') - 'botExternalId')`, + }).where(eq(chatDeliveries.id, f.pending!.deliveryId!)); + } + await f.service.configure(f.endpoint.id, { action: "reconnect" }, "owner-user"); + const input = { db, companyId, issueId, actorId: "owner-user", actorSource: "cloud_tenant", + observedRunId: f.runId, idempotencyKey: randomUUID(), prepareChatRetry: f.service.prepareFailedChatRunRetry }; + if (mode === "board_permission_wait") { + input.actorId = `retry-operator-${companyId}`; + await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: input.actorId, + status: "active", membershipRole: "operator" }); + } + const queryEvents: Record[] = []; + const capture = mode === "backlog_51" ? createDatabaseResponseCapture({ start: Date.now() - 1, + end: Date.now() + 59_000 }, event => queryEvents.push(event)) : undefined; + if (capture) installDatabaseResponseCapture(db, { chatActions, chatDeliveries, chatMessageLinks, issueComments, + agentWakeupRequests, chatEndpoints, chatConversations }, capture); + let one: Awaited>, two: typeof one; + if (mode === "http") { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { req.actor = boardActor(companyId); next(); }); + app.use("/api", issueRoutes(db, createStorageService().storage, { chatRunRetries: f.service })); + app.use(errorHandler); + const send = () => request(app).post(`/api/issues/${issueId}/retry`).send({ + idempotencyKey: input.idempotencyKey, observedRunId: input.observedRunId, + }).expect(202); + const replies = await Promise.all([send(), send()]); + [one, two] = replies.map(response => response.body); + } else [one, two] = await Promise.all([requestTaskRetry(input), requestTaskRetry({ ...input, + idempotencyKey: mode === "concurrent_clicks" ? randomUUID() : input.idempotencyKey })]); + if (capture) { + capture.flush(); + expect(Number(queryEvents.find(event => event.event === "paperclip.db_response_capture.summary.v1")?.startedQueries)) + .toBeLessThan(180); + } + expect(two.requestId).toBe(one.requestId); + expect(one).toMatchObject({ accepted: true, chatActionId: expect.any(String) }); + let service = f.service; + if (mode === "restart") { + await service.shutdown(); + service = createService(f.runtime, f.providerFetch, { wakeup: heartbeatService(db).wakeup }).service; + } + if (mode === "lost_dispatch_reply") f.wakeup.mockImplementationOnce(async (...args) => { + await heartbeatService(db).wakeup(...args); + throw new Error("Simulated response loss after dispatch commit"); + }); + if (mode === "handoff_rollback") f.wakeup.mockImplementationOnce(async (agentId, options) => { + const original = options!.durableChatRequest!; + return heartbeatService(db).wakeup(agentId, { ...options, durableChatRequest: createDurableChatWakeupRequest({ + ...original, onAdmitted: async (tx, receipt) => { + await original.onAdmitted!(tx, receipt); + throw new Error("Simulated interruption before admission commit"); + }, + }) }); + }); + if (mode === "board_permission_wait") await db.update(companyMemberships).set({ membershipRole: "viewer" }) + .where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.principalId, input.actorId))); + await Promise.all([service.processFailedChatRunRetry(one.chatActionId!), + service.processFailedChatRunRetry(one.chatActionId!)]); + if (mode === "handoff_rollback" || mode === "board_permission_wait") { + expect(await readAgentWakeupRequestsById(one.chatActionId!)).toHaveLength(0); + expect((await readAgentWakeupRequestsById(f.pending!.id))[0]) + .toMatchObject({ status: "deferred_issue_execution", runId: null }); + expect((await readHeartbeatRunsByCompanyId(companyId)) + .filter(run => run.contextSnapshot?.taskRetryRequestId === one.requestId)).toHaveLength(0); + } + if (mode === "board_permission_wait") await db.update(companyMemberships).set({ membershipRole: "operator" }) + .where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.principalId, input.actorId))); + await vi.waitFor(async () => { + await service.processFailedChatRunRetry(one.chatActionId!); + expect((await readAgentWakeupRequestsById(one.chatActionId!))[0]?.runId, + JSON.stringify((await db.select().from(chatActions).where(eq(chatActions.id, one.chatActionId!)))[0]?.result)).toBeTruthy(); + }, { timeout: 10_000, interval: 100 }); + const [parent] = await readAgentWakeupRequestsById(one.chatActionId!); + expect(parent.runId).toBeTruthy(); + expect((await readAgentWakeupRequestsById(f.pending!.id))[0]).toMatchObject({ + status: "coalesced", runId: mode === "consumed_backlog_52" ? f.runId : parent.runId, + }); + if (peerInput) expect((await readAgentWakeupRequestsById(peerInput.id))[0]) + .toMatchObject({ status: "deferred_issue_execution", runId: null }); + if (mode === "backlog_51") { + const saved = await db.select({ id: chatActions.id, payload: chatActions.payload, + status: agentWakeupRequests.status, runId: agentWakeupRequests.runId }).from(chatActions) + .innerJoin(agentWakeupRequests, eq(agentWakeupRequests.id, chatActions.id)).where(and( + eq(chatActions.conversationId, f.conversation.id), eq(chatActions.kind, "inbound_wakeup"), + sql`${chatActions.id} <> ${f.action.id}::uuid`, + )).orderBy(asc(chatActions.createdAt), asc(chatActions.id)); + expect(saved).toHaveLength(51); + expect(saved.every(source => source.status === "coalesced" && source.runId === parent.runId)).toBe(true); + expect((await readHeartbeatRunsById(parent.runId!))[0].contextSnapshot?.wakeCommentIds) + .toEqual(saved.map(source => source.payload.commentId)); + } + await service.processFailedChatRunRetry(one.chatActionId!); + expect((await readHeartbeatRunsByCompanyId(companyId)) + .filter(run => run.contextSnapshot?.taskRetryRequestId === one.requestId)).toHaveLength(1); + expect(await db.select().from(chatActions).where(and(eq(chatActions.companyId, companyId), + eq(chatActions.kind, "failed_run_retry")))).toHaveLength(1); + if (mode === "retry_failed_successor" || mode === "backlog_51") { + await db.update(heartbeatRuns).set({ status: "failed", finishedAt: new Date(), + errorCode: "adapter_failed", runtimeMode: "native", nativeIssueId: issueId, nativePhase: "terminal_failure", + }).where(eq(heartbeatRuns.id, parent.runId!)); + await db.update(agentWakeupRequests).set({ status: "failed" }).where(eq(agentWakeupRequests.id, one.chatActionId!)); + await db.update(issues).set({ executionRunId: null, status: "blocked" }).where(eq(issues.id, issueId)); + await db.insert(nativeRunFinalizations).values({ companyId, issueId, runId: parent.runId!, phase: "terminal_failure" }); + await db.insert(heartbeatRunEvents).values({ companyId, agentId: f.fixture.assignedAgentId, runId: parent.runId!, + seq: 1, eventType: "native.local_process_stopped", payload: { processPid: 999999998, processGroupId: null } }); + await service.configure(f.endpoint.id, { action: "reconnect" }, "owner-user"); + const next = await requestTaskRetry({ ...input, observedRunId: parent.runId!, idempotencyKey: randomUUID(), + prepareChatRetry: service.prepareFailedChatRunRetry }); + expect(next.requestId).not.toBe(one.requestId); + expect(await service.processFailedChatRunRetry(next.chatActionId!)).toMatchObject({ status: "queued" }); + const [second] = await readAgentWakeupRequestsById(next.chatActionId!); + expect((await readHeartbeatRunsById(second.runId!))[0]).toMatchObject({ + retryOfRunId: parent.runId, contextSnapshot: { + wakeCommentIds: (await readHeartbeatRunsById(parent.runId!))[0].contextSnapshot!.wakeCommentIds, + }, + }); + } + } finally { await f.service.shutdown(); } + }, + ); + + it("leaves saved GitHub input pending when a persisted Retry wait loses board authority", async () => { + const f = await currentGitHubTaskRetryFixture(true); + try { + const companyId = f.fixture.companyId, issueId = f.issue.id, actorId = `retry-operator-${companyId}`; + await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: actorId, + status: "active", membershipRole: "operator" }); + const intent = await requestTaskRetry({ db, companyId, issueId, actorId, actorSource: "cloud_tenant", + observedRunId: f.runId, idempotencyKey: randomUUID(), prepareChatRetry: f.service.prepareFailedChatRunRetry }); + let savedOptions: Parameters[1]; + f.wakeup.mockImplementationOnce(async (_agentId, options) => { + savedOptions = options; + const request = options!.durableChatRequest!; + // Persist the pre-handoff wait as a server interrupted before queue adoption would. + await db.insert(agentWakeupRequests).values({ id: request.id, companyId, agentId: request.agentId, + source: options!.source, triggerDetail: options!.triggerDetail, reason: options!.reason, + requestedByActorType: request.requestedByActorType, requestedByActorId: request.requestedByActorId, + requestedAt: request.requestedAt, idempotencyKey: request.idempotencyKey, + status: "deferred_issue_execution", payload: { ...options!.payload, + taskRetryRequestId: intent.requestId, _paperclipWakeContext: options!.contextSnapshot } }); + return null; + }); + expect(await f.service.processFailedChatRunRetry(intent.chatActionId!)).toMatchObject({ status: "deferred" }); + await db.update(companyMemberships).set({ membershipRole: "viewer" }).where(and( + eq(companyMemberships.companyId, companyId), eq(companyMemberships.principalId, actorId))); + expect(await heartbeatService(db).wakeup(f.fixture.assignedAgentId, savedOptions!)).toBeNull(); + expect((await readAgentWakeupRequestsById(f.pending!.id))[0]) + .toMatchObject({ status: "deferred_issue_execution", runId: null }); + expect((await readHeartbeatRunsByCompanyId(companyId)) + .filter(run => run.contextSnapshot?.taskRetryRequestId === intent.requestId)).toHaveLength(0); + await db.update(companyMemberships).set({ membershipRole: "operator" }).where(and( + eq(companyMemberships.companyId, companyId), eq(companyMemberships.principalId, actorId))); + await heartbeatService(db).wakeup(f.fixture.assignedAgentId, savedOptions!); + const [parent] = await readAgentWakeupRequestsById(intent.chatActionId!); + expect(parent.runId).toBeTruthy(); + expect((await readAgentWakeupRequestsById(f.pending!.id))[0]).toMatchObject({ status: "coalesced", runId: parent.runId }); + } finally { await f.service.shutdown(); } + }); + + it.each(["changed_bot", "disabled_repository", "retargeted_repository", "revoked_sender", "paused_endpoint", + "edited_source", "deleted_source", "reassigned_task", "second_reconnect", "later_message_wrong_bot", "later_message_deleted", "later_message_revoked_policy"] as const)( + "keeps current GitHub task Retry authority gated: %s", async gate => { + const f = await currentGitHubTaskRetryFixture(true, gate.startsWith("later_message") ? 2 : 1); + try { + const companyId = f.fixture.companyId, issueId = f.issue.id; + await f.service.configure(f.endpoint.id, { action: "reconnect" }, "owner-user"); + const input = { db, companyId, issueId, actorId: "owner-user", actorSource: "cloud_tenant", + observedRunId: f.runId, idempotencyKey: randomUUID(), prepareChatRetry: f.service.prepareFailedChatRunRetry }; + if (gate === "later_message_revoked_policy") { + const [resource] = await db.select().from(chatEndpointResources).where(eq(chatEndpointResources.id, f.conversation.resourceId!)); + await db.update(chatEndpointResources).set({ availability: "available", + metadata: { ...resource.metadata, providerRepositoryId: "418" }, + }).where(eq(chatEndpointResources.id, resource.id)); + await db.insert(chatGitHubConfigurations).values({ companyId, endpointId: f.endpoint.id, + updatedByUserId: "owner-user", configuration: { version: 1, toolsEnabled: true, responsibleUserId: "owner-user", + memberAccess: "all_linked", people: [], repositories: {}, + defaults: { ...defaultGitHubReviewPolicy(), invocation: "linked_authors", events: ["opened", "synchronize"] }, + } }); + const sources = await db.select().from(chatActions).where(and(eq(chatActions.conversationId, f.conversation.id), + eq(chatActions.kind, "inbound_wakeup"), sql`${chatActions.id} <> ${f.action.id}::uuid`)) + .orderBy(asc(chatActions.createdAt), asc(chatActions.id)); + for (const [index, source] of sources.entries()) { + const context = { event: index === 0 ? "opened" : "synchronize", deliveryId: source.deliveryId, + repositoryId: "418", repository: resource.providerResourceId, + pullNumber: 7, title: "Saved review request", body: "", baseSha: "a".repeat(40), headSha: "b".repeat(40), + baseBranch: "master", author: { id: "42", login: "octocat", isBot: false }, + sender: { id: "42", login: "octocat" }, draft: false, labels: [] }; + await db.update(chatDeliveries).set({ + normalizedEvent: sql`${chatDeliveries.normalizedEvent} || ${JSON.stringify({ githubAutomatic: { context } })}::jsonb`, + }).where(eq(chatDeliveries.id, source.deliveryId!)); + } + } + const accepted = await requestTaskRetry(input); + if (gate.startsWith("later_message")) { + const [later] = await db.select().from(chatActions).where(and(eq(chatActions.conversationId, f.conversation.id), + eq(chatActions.kind, "inbound_wakeup"), sql`${chatActions.id} not in (${f.action.id}::uuid, ${f.pending!.id}::uuid)`)); + if (gate === "later_message_wrong_bot") await db.update(chatDeliveries).set({ + normalizedEvent: sql`jsonb_set(${chatDeliveries.normalizedEvent}, '{githubAuthority,botExternalId}', ${JSON.stringify(randomUUID())}::jsonb)`, + }).where(eq(chatDeliveries.id, later.deliveryId!)); + else if (gate === "later_message_deleted") await db.update(issueComments).set({ deletedAt: new Date() }) + .where(eq(issueComments.id, String(later.payload.commentId))); + else await db.update(chatGitHubConfigurations).set({ revision: sql`${chatGitHubConfigurations.revision} + 1`, + configuration: sql`jsonb_set(${chatGitHubConfigurations.configuration}, '{defaults,events}', '["opened"]'::jsonb)`, + }).where(eq(chatGitHubConfigurations.endpointId, f.endpoint.id)); + } + if (gate === "changed_bot") await db.update(chatEndpoints).set({ botExternalId: randomUUID() }) + .where(eq(chatEndpoints.id, f.endpoint.id)); + if (gate === "disabled_repository") await db.update(chatEndpointResources).set({ enabled: false }) + .where(eq(chatEndpointResources.id, f.conversation.resourceId!)); + if (gate === "retargeted_repository") await db.update(chatEndpointResources).set({ providerResourceId: "another/repository" }) + .where(eq(chatEndpointResources.id, f.conversation.resourceId!)); + if (gate === "revoked_sender") await db.update(chatIdentityLinks).set({ status: "revoked" }) + .where(and(eq(chatIdentityLinks.endpointId, f.endpoint.id), eq(chatIdentityLinks.principalId, f.pending!.principalId!))); + if (gate === "paused_endpoint") await db.update(chatEndpoints).set({ status: "paused" }) + .where(eq(chatEndpoints.id, f.endpoint.id)); + if (gate === "edited_source") await db.update(issueComments).set({ body: "Changed after Retry", updatedAt: new Date() }) + .where(eq(issueComments.id, String(f.pending!.payload.commentId))); + if (gate === "deleted_source") await db.update(issueComments).set({ deletedAt: new Date() }) + .where(eq(issueComments.id, String(f.pending!.payload.commentId))); + if (gate === "reassigned_task") await db.update(issues).set({ assigneeAgentId: f.fixture.replacementAgentId }) + .where(eq(issues.id, issueId)); + if (gate === "second_reconnect") await f.service.configure(f.endpoint.id, { action: "reconnect" }, "owner-user"); + expect(await f.service.processFailedChatRunRetry(accepted.chatActionId!)).toMatchObject({ status: "failed" }); + expect((await readAgentWakeupRequestsById(f.pending!.id))[0]).toMatchObject({ status: "deferred_issue_execution", runId: null }); + expect((await readHeartbeatRunsByCompanyId(companyId)) + .filter(run => run.contextSnapshot?.taskRetryRequestId === accepted.requestId)).toHaveLength(0); + const [settled] = await db.select().from(chatActions).where(eq(chatActions.id, accepted.chatActionId!)); + expect(settled.result?.taskRetryInputReconciled).toBe(true); + const [intentReceipt] = await readAgentWakeupRequestsById(accepted.requestId!); + await f.service.processFailedChatRunRetry(accepted.chatActionId!); + expect((await readAgentWakeupRequestsById(accepted.requestId!))[0].updatedAt).toEqual(intentReceipt.updatedAt); + if (gate === "second_reconnect") { + const next = await requestTaskRetry({ ...input, idempotencyKey: randomUUID() }); + expect(next.requestId).not.toBe(accepted.requestId); + expect(await f.service.processFailedChatRunRetry(next.chatActionId!)).toMatchObject({ status: "queued" }); + } + } finally { await f.service.shutdown(); } + }, + ); + it.each([ "settled", "distinct_account", diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index e53608b49de..fc41debb711 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -1,3 +1,4 @@ +import { credentialFingerprint } from "./chat-channels/credential-fingerprint.js"; import { createChatSdkMessageReceiver } from "./chat-channels/runtime/sdk-messages.js"; import { admitDiscordSdkMessage } from "./chat-channels/discord/messages.js"; import { resolveInboundConversationBinding } from "./chat-channels/inbound/conversation-binding.js"; @@ -312,20 +313,6 @@ function runtimeGeneration(setup: ChatEndpointSetupState): number { return Number.isSafeInteger(value) && value >= 0 ? value : 0; } -function credentialFingerprint(refs: ToolCredentialSecretRef[]): string { - const stable = refs - .map((ref) => ({ - configPath: ref.configPath, - secretId: ref.secretId, - versionSelector: ref.versionSelector ?? "latest", - })) - .sort((left, right) => - `${left.configPath}:${left.secretId}:${left.versionSelector}`.localeCompare( - `${right.configPath}:${right.secretId}:${right.versionSelector}`, - ), - ); - return createHash("sha256").update(JSON.stringify(stable)).digest("hex"); -} type ConversationRow = typeof chatConversations.$inferSelect; type DeliveryRow = typeof chatDeliveries.$inferSelect; @@ -847,7 +834,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { const { lockTelegramCallbackNoticeAuthority } = createTelegramCallbackNoticeAuthority({ nonDirectDestinationAllowed, lockCurrentPrincipalAuthorization }); const { stageProviderEffect, processProviderEffect, processPendingProviderEffects } = createProviderEffects(db, { endpointRecord, runtimeCallbackEndpoint, withCredentialMutationLease, runtimeFor, invalidateRuntime, lockCurrentPrincipalAuthorization, redactError, MAX_ERROR_TEXT, nonDirectDestinationAllowed, lockTelegramCallbackNoticeAuthority }); const { publicationFiles } = createOutboundAttachments(db, { get storage() { return options.storage; } }); - const { authorizeInboundWakeup, authorizeInboundWorkStart, authorizeFastResponse } = createInboundWakeAuthority(db, { runtimeCallbackEndpoint, lockCurrentPrincipalAuthorization }); + const { authorizeInboundWakeup, authorizeGitHubTaskRetryBatch, authorizeInboundWorkStart, authorizeFastResponse } = createInboundWakeAuthority(db, { runtimeCallbackEndpoint, lockCurrentPrincipalAuthorization }); const { ingestAttachments, attachmentOmissionDetail } = createInboundAttachments(db, { authorizeInboundWakeup, lifecycleRuntimeFence, sanitizeFilename, issuesSvc, isExternalActionAuthorizationChange, redactError, get storage() { return options.storage; } }); const { stageInboundWakeup, acceptInboundWakeup, settleRejectedInboundWakeups, processInboundWakeup } = createInboundWakeups(db, { authorizeInboundWakeup, authorizeInboundWorkStart, attachmentOmissionDetail, isExternalActionAuthorizationChange, redactError, fetchImpl, get heartbeat() { return options.heartbeat; } }); const { settleTelegramAttachmentOnlyFailure } = createTelegramAttachmentFailure(db, { @@ -1203,6 +1190,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { pendingFailedRetries, } = createChatFailedRunRetry(db, { authorizeInboundWakeup, + authorizeGitHubTaskRetryBatch, isExternalActionAuthorizationChange, inboundWakeScope, lifecycleRuntimeFence, diff --git a/server/src/services/chat-channels/credential-fingerprint.ts b/server/src/services/chat-channels/credential-fingerprint.ts new file mode 100644 index 00000000000..57f472140c7 --- /dev/null +++ b/server/src/services/chat-channels/credential-fingerprint.ts @@ -0,0 +1,10 @@ +import { createHash } from "node:crypto"; +import type { ToolCredentialSecretRef } from "@paperclipai/shared"; + +export function credentialFingerprint(refs: ToolCredentialSecretRef[]): string { + const stable = refs.map(ref => ({ configPath: ref.configPath, secretId: ref.secretId, + versionSelector: ref.versionSelector ?? "latest" })).sort((left, right) => + `${left.configPath}:${left.secretId}:${left.versionSelector}`.localeCompare( + `${right.configPath}:${right.secretId}:${right.versionSelector}`)); + return createHash("sha256").update(JSON.stringify(stable)).digest("hex"); +} diff --git a/server/src/services/chat-channels/failed-run-retry.ts b/server/src/services/chat-channels/failed-run-retry.ts index 6310d448a66..c18a15bdcb8 100644 --- a/server/src/services/chat-channels/failed-run-retry.ts +++ b/server/src/services/chat-channels/failed-run-retry.ts @@ -3,6 +3,8 @@ import { readTaskChatRetryIntent, taskChatRetryCanAwaitRecovery, taskChatRetryEx import { notifyChatActionWork, notifyChatPublicationWork } from "../chat-work-notifications.js"; import { FAILED_RUN_RETRY_STALE_MS, dueChatAction } from "../chat-action-work.js"; import { readChatAuthorizationRun } from "./authorization-run-read.js"; +import { selectTaskRetryInput, claimTaskRetryInput, settleTaskRetryInput, releaseUnadmittedTaskRetryInput, taskRetryInputSchema, type TaskRetryInput } from "./task-retry-input.js"; +import type { TaskRetryWakeAuthority } from "./github/task-retry-authorization.js"; import { createHash, @@ -93,14 +95,16 @@ type RetryRuntimeFence = { credentialFingerprint: string; generation: number }; export type ChatFailedRetrySource = Awaited["failedChatRetrySource"]>>; type ChatFailedRunRetryOptions = { + authorizeGitHubTaskRetryBatch: ReturnType["authorizeGitHubTaskRetryBatch"]; authorizeInboundWakeup: ( database: DbOrTransaction, action: typeof chatActions.$inferSelect, - notice?: { nonblocking: true; terminal: boolean }, + notice?: { nonblocking: true; terminal: boolean; taskRetry?: TaskRetryWakeAuthority }, ) => Promise<{ conversation: Pick; endpoint: Pick; delivery: DeliveryRow; + retryRuntimeFence?: RetryRuntimeFence; }>; isExternalActionAuthorizationChange: (error: unknown) => boolean; inboundWakeScope: (action: typeof chatActions.$inferSelect) => string; @@ -112,6 +116,7 @@ type ChatFailedRunRetryOptions = { export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOptions) { const { authorizeInboundWakeup, + authorizeGitHubTaskRetryBatch, isExternalActionAuthorizationChange, inboundWakeScope, lifecycleRuntimeFence, @@ -131,14 +136,14 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti } } - function retryCommentIds(context: Record): string[] { + function retryCommentIds(context: Record, taskRetryInput?: TaskRetryInput): string[] { const ids = context.wakeCommentIds ?? (context.wakeCommentId ? [context.wakeCommentId] : []); if ( !Array.isArray(ids) || ids.length === 0 || - ids.length > 50 || + (!taskRetryInput && ids.length > 50) || ids.some((id) => typeof id !== "string" || !isUuidLike(id)) || new Set(ids).size !== ids.length || (context.wakeCommentId !== undefined && @@ -168,6 +173,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti failedRunId: string; sourceScopeSha256: string; }>; + taskRetryInput?: TaskRetryInput; sourceScopeSha256: string; }; function hashFailedRetrySource( @@ -651,6 +657,8 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti runId?: string; publication?: boolean; taskRetryIntentId?: string; + taskRetryInput?: TaskRetryInput; + taskRetryReauthorization?: TaskRetryWakeAuthority; /** Internal selector for already committed output, never retry admission. */ committedResponse?: { resultId: string; @@ -734,7 +742,22 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti (context.taskKey !== issue.identifier && context.taskKey !== issue.id) ) throw failedChatRetryDenied(); - const commentIds = retryCommentIds(context); + const selection = input.taskRetryInput && taskRetryInputSchema.safeParse(input.taskRetryInput); + if (selection && !selection.success) throw failedChatRetryDenied(); + const selectedInput = selection?.success ? selection.data : undefined; + if (selectedInput && (!input.taskRetryIntentId || provider !== "github")) throw failedChatRetryDenied(); + const reauthorization = input.taskRetryReauthorization ?? (selectedInput ? { + intentId: input.taskRetryIntentId!, sourceRunId: failedRun.id, runtimeFence: selectedInput.runtimeFence, + } : undefined); + const pendingSources = selectedInput?.sourceActionIds.length ? await tx.select().from(chatActions).where(and( + eq(chatActions.companyId, input.companyId), eq(chatActions.kind, "inbound_wakeup"), + sql`${chatActions.payload}->>'issueId' = ${issue.id}`, + inArray(chatActions.id, selectedInput.sourceActionIds), + )) : null; + if (pendingSources && pendingSources.length !== selectedInput!.sourceActionIds.length) throw failedChatRetryDenied(); + const pendingById = new Map(pendingSources?.map(source => [source.id, source])); + const commentIds = pendingSources ? selectedInput!.sourceActionIds.map(id => + String(pendingById.get(id)!.payload.commentId)) : retryCommentIds(context, selectedInput); const [accepted] = await tx .select() .from(nativeRunResults) @@ -868,7 +891,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti answerDelivery ) throw failedChatRetryDenied(); - if (context.chatFailedRunRetry) { + if (context.chatFailedRunRetry && !pendingSources) { const hint = context.chatFailedRunRetry as Record; const [priorIntent] = await tx .select() @@ -914,6 +937,9 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti committedResponse: undefined, taskRetryIntentId: typeof priorIntent.payload.taskRetryIntentId === "string" ? priorIntent.payload.taskRetryIntentId : undefined, + taskRetryInput: priorIntent.payload.taskRetryInput as TaskRetryInput | undefined, + retryActionId: priorIntent.id, + taskRetryReauthorization: reauthorization, }, ancestors, ); @@ -945,6 +971,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti return hashFailedRetrySource({ ...parentScope, failedRunId: failedRun.id, + ...(selectedInput ? { taskRetryInput: { ...selectedInput, runtimeFence: reauthorization?.runtimeFence } } : {}), retryAncestors: [ ...parent.retryAncestors, { @@ -958,20 +985,21 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti // Include every receipt that fed this run, including a deferred owner's // coalesced children. A mutable latest-comment hint cannot drop a sibling. const admittedBatch = - await tx.execute(sql`select source.id, source.payload->>'commentId' as comment_id + pendingSources ? [] : await tx.execute(sql`select source.id, source.payload->>'commentId' as comment_id from agent_wakeup_requests receipt join chat_actions source on source.id = receipt.id where receipt.company_id = ${input.companyId}::uuid and receipt.agent_id = ${input.agentId}::uuid and (receipt.run_id = ${failedRun.id}::uuid or receipt.id = ${failedRun.wakeupRequestId}::uuid or receipt.payload->>'coalescedIntoWakeupRequestId' = ${failedRun.wakeupRequestId}) - and source.company_id = ${input.companyId}::uuid and source.kind = 'inbound_wakeup' limit 51`); + and source.company_id = ${input.companyId}::uuid and source.kind = 'inbound_wakeup' + ${selectedInput ? sql`` : sql`limit 51`}`); if ( - admittedBatch.length !== commentIds.length || + !pendingSources && (admittedBatch.length !== commentIds.length || admittedBatch.some( (entry) => !commentIds.includes(String(entry.comment_id)), - ) + )) ) throw failedChatRetryDenied(); - const sources = await tx + const sources = pendingSources ?? await tx .select() .from(chatActions) .where( @@ -986,8 +1014,9 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti ), ); if (sources.length !== commentIds.length) throw failedChatRetryDenied(); + const sourcesByCommentId = new Map(sources.map(source => [source.payload.commentId, source])); const ordered = commentIds.map( - (id) => sources.find((action) => action.payload.commentId === id)!, + (id) => sourcesByCommentId.get(id)!, ); const first = ordered[0]!; if ( @@ -1011,39 +1040,53 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti ); if (!identity[0]?.locked) throw new NativeChatReviewPresentationContentionError(); + const actionIds = ordered.map(action => action.id); + const currentActions = await tx.select().from(chatActions).where(and(eq(chatActions.companyId, input.companyId), + inArray(chatActions.id, actionIds))).orderBy(asc(chatActions.id)).for("update", { noWait: true }); + const deliveries = await tx.select().from(chatDeliveries).where(and(eq(chatDeliveries.companyId, input.companyId), + inArray(chatDeliveries.id, ordered.map(action => action.deliveryId!)))) + .orderBy(asc(chatDeliveries.id)).for("update", { noWait: true }); + const comments = await tx.select().from(issueComments).where(and(eq(issueComments.companyId, input.companyId), + eq(issueComments.issueId, issue.id), inArray(issueComments.id, commentIds))) + .orderBy(asc(issueComments.id)).for("share", { noWait: true }); + const receipts = await tx.select().from(agentWakeupRequests).where(and(eq(agentWakeupRequests.companyId, input.companyId), + inArray(agentWakeupRequests.id, [...actionIds, failedRun.wakeupRequestId!, ...(input.retryActionId ? [input.retryActionId] : [])]))) + .orderBy(asc(agentWakeupRequests.id)).for("share", { noWait: true }); + const actionsById = new Map(currentActions.map(action => [action.id, action])); + const deliveriesById = new Map(deliveries.map(delivery => [delivery.id, delivery])); + const commentsById = new Map(comments.map(comment => [comment.id, comment])); + const receiptsById = new Map(receipts.map(receipt => [receipt.id, receipt])); + const lifecycleRows = await tx.select({ + target: sql`${chatDeliveries.normalizedEvent}->'message'->>'targetProviderEventId'`, + context: sql>`${chatDeliveries.normalizedEvent}->'runtimeContext'`, + }).from(chatDeliveries).where(and(eq(chatDeliveries.companyId, input.companyId), + eq(chatDeliveries.endpointId, first.endpointId), + or(isNull(chatDeliveries.conversationId), eq(chatDeliveries.conversationId, first.conversationId)), + ne(chatDeliveries.state, "filtered"), inArray(chatDeliveries.eventKind, ["message_updated", "message_deleted", "message_restored"]), + inArray(sql`${chatDeliveries.normalizedEvent}->'message'->>'targetProviderEventId'`, deliveries.map(delivery => delivery.providerEventId)))); + const lifecycleKey = (target: string, context: Record | null) => + JSON.stringify([target, Object.entries(context ?? {}).sort(([left], [right]) => left.localeCompare(right))]); + const invalidated = new Set(lifecycleRows.map(row => lifecycleKey(row.target, row.context))); + const batch = reauthorization ? await authorizeGitHubTaskRetryBatch(tx, ordered, { + nonblocking: true, terminal: input.publication === true, taskRetry: reauthorization, + }).catch(error => { if (isExternalActionAuthorizationChange(error)) throw failedChatRetryDenied(); throw error; }) : null; const canonicalSources: Array> = []; let destination: Awaited> | null = null; for (const action of ordered) { - destination = await authorizeInboundWakeup(tx, action, { + destination = batch?.get(action.id) ?? await authorizeInboundWakeup(tx, action, { nonblocking: true, terminal: input.publication === true, + ...(reauthorization ? { taskRetry: reauthorization } : {}), }).catch((error) => { if (isExternalActionAuthorizationChange(error)) throw failedChatRetryDenied(); throw error; }); - const [delivery] = await tx - .select() - .from(chatDeliveries) - .where(eq(chatDeliveries.id, action.deliveryId!)) - .for("update", { noWait: true }); - const [current] = await tx - .select() - .from(chatActions) - .where(eq(chatActions.id, action.id)) - .for("update", { noWait: true }); - const [comment] = await tx - .select() - .from(issueComments) - .where( - and( - eq(issueComments.id, String(action.payload.commentId)), - eq(issueComments.companyId, input.companyId), - eq(issueComments.issueId, issue.id), - ), - ) - .for("share", { noWait: true }); + if (reauthorization && !reauthorization.runtimeFence) reauthorization.runtimeFence = destination.retryRuntimeFence; + const delivery = deliveriesById.get(action.deliveryId!); + const current = actionsById.get(action.id); + const comment = commentsById.get(String(action.payload.commentId)); if ( !current || current.status !== "processed" || @@ -1053,43 +1096,17 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti !comment || comment.deletedAt || comment.updatedAt.getTime() !== comment.createdAt.getTime() || - comment.updatedAt > + (!pendingSources && comment.updatedAt > (input.committedResponse ? accepted!.createdAt - : failedRun.finishedAt!) || + : failedRun.finishedAt!)) || destination.endpoint.provider !== provider || JSON.stringify(lifecycleRuntimeFence(delivery)) !== JSON.stringify(lifecycleRuntimeFence(destination.delivery)) ) throw failedChatRetryDenied(); - const [lifecycle] = await tx - .select({ id: chatDeliveries.id }) - .from(chatDeliveries) - .where( - and( - eq(chatDeliveries.companyId, input.companyId), - eq(chatDeliveries.endpointId, first.endpointId), - or( - isNull(chatDeliveries.conversationId), - eq(chatDeliveries.conversationId, first.conversationId), - ), - ne(chatDeliveries.state, "filtered"), - sql`${chatDeliveries.normalizedEvent}->'runtimeContext' = ${JSON.stringify(lifecycleRuntimeFence(delivery))}::jsonb`, - inArray(chatDeliveries.eventKind, [ - "message_updated", - "message_deleted", - "message_restored", - ]), - sql`${chatDeliveries.normalizedEvent}->'message'->>'targetProviderEventId' = ${delivery.providerEventId}`, - ), - ) - .limit(1); - if (lifecycle) throw failedChatRetryDenied(); - const [receipt] = await tx - .select() - .from(agentWakeupRequests) - .where(eq(agentWakeupRequests.id, action.id)) - .for("share", { noWait: true }); + if (invalidated.has(lifecycleKey(delivery.providerEventId, lifecycleRuntimeFence(delivery)))) throw failedChatRetryDenied(); + const receipt = receiptsById.get(action.id); if (!receipt) throw failedChatRetryDenied(); assertFailedRetryReceipt( createDurableChatWakeupRequest({ @@ -1107,6 +1124,17 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti }), receipt, ); + if (pendingSources) { + const direct = receipt.status === "deferred_issue_execution" && !receipt.runId + && receipt.payload?.coalescedIntoWakeupRequestId === undefined; + const owned = input.retryActionId && receipt.status === "coalesced" + && receipt.payload?.coalescedIntoWakeupRequestId === input.retryActionId; + if (!direct && !owned) throw failedChatRetryDenied(); + if (owned && receipt.runId) { + const owner = receiptsById.get(input.retryActionId!); + if (owner?.runId !== receipt.runId) throw failedChatRetryDenied(); + } + } let owner = receipt; const seen = new Set(); for ( @@ -1126,25 +1154,16 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti ) throw failedChatRetryDenied(); seen.add(ownerId); - const [next] = await tx - .select() - .from(agentWakeupRequests) - .where( - and( - eq(agentWakeupRequests.id, ownerId), - eq(agentWakeupRequests.companyId, input.companyId), - ), - ) - .for("share", { noWait: true }); + const next = receiptsById.get(ownerId); if (!next) throw failedChatRetryDenied(); owner = next; } if ( - owner.id !== failedRun.wakeupRequestId || + (!pendingSources && (owner.id !== failedRun.wakeupRequestId || owner.runId !== failedRun.id || owner.agentId !== input.agentId || owner.requestedByActorType !== receipt.requestedByActorType || - owner.requestedByActorId !== receipt.requestedByActorId || + owner.requestedByActorId !== receipt.requestedByActorId)) || ["skipped", "cancelled"].includes(receipt.status) ) throw failedChatRetryDenied(); @@ -1158,7 +1177,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti bodySha256: createHash("sha256").update(comment.body).digest("hex"), omissionReasons: action.payload.attachmentOmissionReasons ?? {}, receiptId: receipt.id, - ownerId: owner.id, + ownerId: pendingSources ? action.id : owner.id, }); } if (!destination) throw failedChatRetryDenied(); @@ -1204,6 +1223,8 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti | "system", requestedByActorId: String(first.payload.requestedByActorId), retryAncestors: [], + ...(selectedInput ? { taskRetryInput: { ...selectedInput, + runtimeFence: selectedInput.runtimeFence ?? destination.retryRuntimeFence } } : {}), }; return hashFailedRetrySource(scope); } @@ -1240,7 +1261,8 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti retryIntentId?: string; }, ): Promise<{ actionId: string; issueId: string }> { - const source = await failedChatRetrySource(tx, { ...input, taskRetryIntentId: input.retryIntentId }); + const taskRetryInput = input.retryIntentId ? await selectTaskRetryInput(tx, input) : undefined; + const source = await failedChatRetrySource(tx, { ...input, taskRetryIntentId: input.retryIntentId, taskRetryInput }); const providerActionId = input.retryIntentId ? `task-retry:${input.retryIntentId}` : `failed-run-retry:${source.failedRunId}`; const [existing] = await tx @@ -1351,6 +1373,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti failedRunId: snapshot.payload.failedRunId, taskRetryIntentId: typeof snapshot.payload.taskRetryIntentId === "string" ? snapshot.payload.taskRetryIntentId : undefined, + taskRetryInput: snapshot.payload.taskRetryInput as TaskRetryInput | undefined, }); if (typeof snapshot.payload.taskRetryIntentId === "string" && !await readTaskChatRetryIntent(tx as unknown as Db, { companyId: input.companyId, issueId: input.issueId, agentId: input.agentId, @@ -1444,7 +1467,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti context.source !== `chat:${source.provider}` || context.taskKey !== source.taskKey || context.retryOfRunId !== source.failedRunId || - JSON.stringify(retryCommentIds(context)) !== + JSON.stringify(retryCommentIds(context, source.taskRetryInput)) !== JSON.stringify(source.commentIds) || hint?.version !== 1 || hint.actionId !== source.intent.id || @@ -1518,8 +1541,19 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti }; } + async function reconcileFailedTaskRetry(action: typeof chatActions.$inferSelect) { + if (!["failed", "cancelled"].includes(action.status) || !action.payload.taskRetryIntentId || + action.result?.taskRetryInputReconciled === true) return action; + await releaseUnadmittedTaskRetryInput(db, { companyId: action.companyId, retryActionId: action.id }); + const [settled] = await db.update(chatActions).set({ + result: sql`coalesce(${chatActions.result}, '{}'::jsonb) || '{"taskRetryInputReconciled":true}'::jsonb`, + }).where(and(eq(chatActions.companyId, action.companyId), eq(chatActions.id, action.id), + inArray(chatActions.status, ["failed", "cancelled"]))).returning(); + return settled ?? action; + } + async function claimFailedChatRunRetry(actionId: string) { - const [candidate] = await db + const [snapshot] = await db .select() .from(chatActions) .where( @@ -1528,7 +1562,10 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti eq(chatActions.kind, "failed_run_retry"), ), ); - if (!candidate) throw notFound("Chat retry request not found"); + if (!snapshot) throw notFound("Chat retry request not found"); + // Reconcile once at settlement; a crash before the marker is safely + // retried here. Status projection itself performs no writes or row locks. + const candidate = await reconcileFailedTaskRetry(snapshot); const now = new Date(); if ( isShuttingDown() || @@ -1571,7 +1608,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti ) .returning(); }); - return failedChatRetryReceipt(current ?? claimed); + return failedChatRetryReceipt(await reconcileFailedTaskRetry(current ?? claimed)); }; try { const scope = { @@ -1644,6 +1681,12 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti true, // The durable wake callback validates admission; later phases recheck containment. ); }, + onAdmitted: async (tx, receipt) => { + if (!source.taskRetryInput) return; + await claimTaskRetryInput(tx, { companyId: claimed.companyId, + retryActionId: claimed.id, sourceActionIds: source.taskRetryInput.sourceActionIds }); + await settleTaskRetryInput(tx, { companyId: claimed.companyId, retryActionId: claimed.id, runId: receipt.runId }); + }, }), }); } @@ -1652,6 +1695,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti .from(agentWakeupRequests) .where(eq(agentWakeupRequests.id, claimed.id)); if (!receipt) throw new Error("chat_retry_receipt_not_committed"); + await settleTaskRetryInput(db, { companyId: claimed.companyId, retryActionId: claimed.id, runId: receipt.runId }); return settle( ["skipped", "failed", "cancelled"].includes(receipt.status) ? "failed" diff --git a/server/src/services/chat-channels/github/task-preparation.ts b/server/src/services/chat-channels/github/task-preparation.ts index 403b45c537e..d44ce61de02 100644 --- a/server/src/services/chat-channels/github/task-preparation.ts +++ b/server/src/services/chat-channels/github/task-preparation.ts @@ -37,7 +37,7 @@ export async function retainGitHubDeliveryAuthority( linkedUserId: string | null, taskUserId: string | null, ): Promise { const guest = !linkedUserId; - await taskTx.update(chatDeliveries).set({ normalizedEvent: sql`${chatDeliveries.normalizedEvent} || ${JSON.stringify({ githubAuthority: { guest, responsibleUserId: taskUserId, sponsorUserId: guest ? taskEndpoint.sponsorUserId : null } })}::jsonb` }).where(eq(chatDeliveries.id, deliveryId)); + await taskTx.update(chatDeliveries).set({ normalizedEvent: sql`${chatDeliveries.normalizedEvent} || ${JSON.stringify({ githubAuthority: { guest, botExternalId: taskEndpoint.botExternalId, responsibleUserId: taskUserId, sponsorUserId: guest ? taskEndpoint.sponsorUserId : null } })}::jsonb` }).where(eq(chatDeliveries.id, deliveryId)); } export async function retainGitHubReviewReceipt(taskTx: DbOrTransaction, endpoint: EndpointRow, issueId: string, deliveryId: string, githubAutomatic: AutomaticRequest): Promise { diff --git a/server/src/services/chat-channels/github/task-retry-authorization.ts b/server/src/services/chat-channels/github/task-retry-authorization.ts new file mode 100644 index 00000000000..52e51f38ebf --- /dev/null +++ b/server/src/services/chat-channels/github/task-retry-authorization.ts @@ -0,0 +1,126 @@ +import { and, eq, inArray } from "drizzle-orm"; +import { chatActions, chatConversations, chatDeliveries, chatEndpointResources, chatMessageLinks, issueComments, toolConnections } from "@paperclipai/db"; +import { forbidden } from "../../../errors.js"; +import type { DbOrTransaction, EndpointRow, LifecycleRuntimeFence } from "../runtime-types.js"; +import { credentialFingerprint } from "../credential-fingerprint.js"; +import { readTaskChatRetryIntent } from "../task-retry-authority.js"; + +export type TaskRetryWakeAuthority = { + intentId: string; + sourceRunId: string; + runtimeFence?: LifecycleRuntimeFence; +}; + +export function githubTaskRetryAppMatches(endpoint: EndpointRow, delivery: typeof chatDeliveries.$inferSelect, + response: { payload: Record } | undefined) { + const original = delivery.normalizedEvent.githubAuthority as { botExternalId?: unknown } | undefined; + return original?.botExternalId !== undefined ? original.botExternalId === endpoint.botExternalId + : response?.payload.botExternalId === endpoint.botExternalId && response.payload.agentId === endpoint.assignedAgentId; +} + +export function githubTaskRetryDestinationMatches(delivery: typeof chatDeliveries.$inferSelect, + conversation: typeof chatConversations.$inferSelect, resource: typeof chatEndpointResources.$inferSelect | null) { + const originalResource = delivery.normalizedEvent.resource as { providerResourceId?: unknown } | undefined; + const originalThread = delivery.normalizedEvent.conversation as { externalThreadId?: unknown } | undefined; + const automatic = (delivery.normalizedEvent.githubAutomatic ?? delivery.normalizedEvent.githubIssue) as + { context?: { repositoryId?: unknown } } | undefined; + return resource && originalResource?.providerResourceId === resource.providerResourceId + && originalThread?.externalThreadId === conversation.externalThreadId + && (automatic?.context?.repositoryId === undefined + || automatic.context.repositoryId === resource.metadata.providerRepositoryId); +} + +export async function githubTaskRetryRuntimeFence(tx: DbOrTransaction, endpoint: EndpointRow, + action: typeof chatActions.$inferSelect, delivery: typeof chatDeliveries.$inferSelect, + authority: TaskRetryWakeAuthority): Promise { + const deny = () => forbidden("This accepted chat message is no longer authorized to start work", + { code: "chat_action_authorization_changed" }); + const retry = await readTaskChatRetryIntent(tx as Parameters[0], { + companyId: action.companyId, issueId: String(action.payload.issueId), + agentId: String(action.payload.agentId), sourceRunId: authority.sourceRunId, intentId: authority.intentId, + }); + if (!retry || endpoint.provider !== "github" || !endpoint.botExternalId || action.kind !== "inbound_wakeup" + || action.status !== "processed" || delivery.state !== "processed") throw deny(); + const [response] = await tx.select({ payload: chatActions.payload }).from(chatActions).where(and( + eq(chatActions.companyId, action.companyId), eq(chatActions.endpointId, endpoint.id), + eq(chatActions.conversationId, action.conversationId!), eq(chatActions.deliveryId, delivery.id), + eq(chatActions.kind, "github_response_comment"), + )); + if (!githubTaskRetryAppMatches(endpoint, delivery, response)) throw deny(); + const [connection] = await tx.select({ refs: toolConnections.credentialSecretRefs, + enabled: toolConnections.enabled, status: toolConnections.status }).from(toolConnections).where(and( + eq(toolConnections.companyId, endpoint.companyId), eq(toolConnections.id, endpoint.connectionId), + )); + if (!connection?.enabled || connection.status !== "active") throw deny(); + const generation = Number((endpoint.setup as { runtimeGeneration?: unknown }).runtimeGeneration ?? 0); + if (!Number.isSafeInteger(generation) || generation < 0) throw deny(); + return authority.runtimeFence ?? { generation, credentialFingerprint: credentialFingerprint(connection.refs ?? []) }; +} + +type GitHubRetryBinding = { + endpoint: EndpointRow; + conversation: typeof chatConversations.$inferSelect; + resource: typeof chatEndpointResources.$inferSelect | null; + delivery: typeof chatDeliveries.$inferSelect; + retryRuntimeFence?: LifecycleRuntimeFence; +}; + +export function createGitHubTaskRetryBatchAuthority(authorize: ( + tx: DbOrTransaction, action: typeof chatActions.$inferSelect, + notice: { nonblocking: true; terminal: boolean; taskRetry: TaskRetryWakeAuthority }, +) => Promise) { + async function authorizeGitHubTaskRetryBatch(tx: DbOrTransaction, actions: Array, + notice: { nonblocking: true; terminal: boolean; taskRetry: TaskRetryWakeAuthority }) { + const deny = () => forbidden("This accepted chat message is no longer authorized to start work", + { code: "chat_action_authorization_changed" }); + const first = actions[0]; + if (!first) throw deny(); + const commonScope = (action: typeof chatActions.$inferSelect) => JSON.stringify([ + action.companyId, action.endpointId, action.conversationId, action.principalId, action.kind, action.status, + ...["version", "issueId", "agentId", "sessionGeneration", "requestedByActorType", "requestedByActorId"] + .map(key => action.payload[key]), + ]); + if (actions.some(action => commonScope(action) !== commonScope(first))) throw deny(); + const deliveries = await tx.select().from(chatDeliveries).where(and(eq(chatDeliveries.companyId, first.companyId), + inArray(chatDeliveries.id, actions.map(action => action.deliveryId!)))); + const byDelivery = new Map(deliveries.map(delivery => [delivery.id, delivery])); + const legacy = await tx.select().from(chatActions).where(and(eq(chatActions.companyId, first.companyId), + eq(chatActions.endpointId, first.endpointId), eq(chatActions.conversationId, first.conversationId!), + eq(chatActions.kind, "github_response_comment"), inArray(chatActions.deliveryId, deliveries.map(delivery => delivery.id)))); + const legacyByDelivery = new Map(legacy.map(action => [action.deliveryId, action])); + const links = await tx.select({ deliveryId: chatMessageLinks.deliveryId, commentId: chatMessageLinks.commentId, + authorUserId: issueComments.authorUserId, deletedAt: issueComments.deletedAt }).from(chatMessageLinks) + .innerJoin(issueComments, and(eq(issueComments.id, chatMessageLinks.commentId), + eq(issueComments.companyId, first.companyId), eq(issueComments.issueId, String(first.payload.issueId)))) + .where(and(eq(chatMessageLinks.companyId, first.companyId), eq(chatMessageLinks.endpointId, first.endpointId), + eq(chatMessageLinks.conversationId, first.conversationId!), eq(chatMessageLinks.direction, "inbound"), + inArray(chatMessageLinks.deliveryId, deliveries.map(delivery => delivery.id)))); + const expectedUserId = first.payload.requestedByActorType === "user" ? first.payload.requestedByActorId : null; + const linked = new Set(links.filter(link => (!link.deletedAt || notice.terminal) && link.authorUserId === expectedUserId) + .map(link => `${link.deliveryId}:${link.commentId}`)); + const groups = new Map(); + const authorized = new Map(); + for (const action of actions) { + const delivery = byDelivery.get(action.deliveryId!); + if (!delivery || !linked.has(`${delivery.id}:${action.payload.commentId}`)) throw deny(); + // Reuse only the same live grant within this transaction. Automatic review + // policies with different contexts still receive their own full check. + const grant = JSON.stringify(delivery.normalizedEvent.githubAutomatic ?? delivery.normalizedEvent.githubIssue ?? null); + let current = groups.get(grant); + if (!current) { + current = await authorize(tx, action, notice); + if (current.endpoint.provider !== "github") throw deny(); + notice.taskRetry.runtimeFence ??= current.retryRuntimeFence; + groups.set(grant, current); + } + if (delivery.state !== "processed" || delivery.endpointId !== current.endpoint.id || + delivery.conversationId !== current.conversation.id || delivery.principalId !== action.principalId || + !githubTaskRetryAppMatches(current.endpoint, delivery, legacyByDelivery.get(delivery.id)) || + !githubTaskRetryDestinationMatches(delivery, current.conversation, current.resource)) throw deny(); + authorized.set(action.id, { ...current, delivery }); + } + return authorized; + } + + return authorizeGitHubTaskRetryBatch; +} diff --git a/server/src/services/chat-channels/inbound/wakeup-authorization.ts b/server/src/services/chat-channels/inbound/wakeup-authorization.ts index ecdca242e9b..f4da4fa7cce 100644 --- a/server/src/services/chat-channels/inbound/wakeup-authorization.ts +++ b/server/src/services/chat-channels/inbound/wakeup-authorization.ts @@ -11,6 +11,7 @@ import { agentWakeupRequests, agents, chatActions, chatConversations, chatDelive import { forbidden } from "../../../errors.js"; import { resolveGitHubWakeAuthority, isGitHubAutomaticDelivery } from "../github/wake-authority.js"; import { assertSlackWakeMention } from "../slack/wake-authority.js"; +import { githubTaskRetryRuntimeFence, githubTaskRetryDestinationMatches, createGitHubTaskRetryBatchAuthority, type TaskRetryWakeAuthority } from "../github/task-retry-authorization.js"; import type { createChatInboundAuthority } from "./authorization.js"; import type { createChatMessageLifecycle } from "./lifecycle.js"; type WakeAuthorityOptions = { @@ -41,7 +42,7 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions async function authorizeInboundWakeup( tx: DbOrTransaction, action: typeof chatActions.$inferSelect, - notice?: { nonblocking: true; terminal: boolean }, + notice?: { nonblocking: true; terminal: boolean; taskRetry?: TaskRetryWakeAuthority }, ) { const payload = action.payload; const deny = () => @@ -106,6 +107,8 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions .limit(1) .then((rows) => rows[0] ?? null); const fence = delivery ? lifecycleRuntimeFence(delivery) : null; + const currentFence = endpoint && delivery && notice?.taskRetry + ? await githubTaskRetryRuntimeFence(tx, endpoint, action, delivery, notice.taskRetry) : fence; if ( !endpoint || endpoint.assignedAgentId !== payload.agentId || @@ -114,8 +117,8 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions delivery.endpointId !== endpoint.id || delivery.conversationId !== action.conversationId || delivery.principalId !== action.principalId || - !fence || - !(await runtimeCallbackEndpoint(tx as unknown as DbTransaction, endpoint.id, fence, [ + !currentFence || + !(await runtimeCallbackEndpoint(tx as unknown as DbTransaction, endpoint.id, currentFence, [ "verifying", "active", ])) @@ -160,6 +163,7 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions endpoint, action.principalId, ); + if (notice?.taskRetry && !githubTaskRetryDestinationMatches(delivery, conversation, resource)) throw deny(); if (endpoint.provider === "github") { const automatic = await resolveGitHubWakeAuthority(tx, endpoint, delivery); if (automatic.kind === "denied") throw deny(); @@ -214,9 +218,12 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions source.authorUserId !== expectedUserId ) throw deny(); - return { issue, delivery, endpoint, conversation }; + return { issue, delivery, endpoint, conversation, resource, + ...(notice?.taskRetry && currentFence ? { retryRuntimeFence: currentFence } : {}) }; } + const authorizeGitHubTaskRetryBatch = createGitHubTaskRetryBatchAuthority(authorizeInboundWakeup); + async function authorizeInboundWorkStart( tx: DbOrTransaction, action: typeof chatActions.$inferSelect, @@ -241,5 +248,5 @@ export function createInboundWakeAuthority(db: Db, options: WakeAuthorityOptions attachments: normalized.message?.attachments?.map(a => `${a.name ?? "Attachment"} (${a.mimeType ?? "unknown type"})`) }; } - return { authorizeInboundWakeup, authorizeInboundWorkStart, authorizeFastResponse }; + return { authorizeInboundWakeup, authorizeGitHubTaskRetryBatch, authorizeInboundWorkStart, authorizeFastResponse }; } diff --git a/server/src/services/chat-channels/task-retry-input.ts b/server/src/services/chat-channels/task-retry-input.ts new file mode 100644 index 00000000000..b5fb8a30233 --- /dev/null +++ b/server/src/services/chat-channels/task-retry-input.ts @@ -0,0 +1,95 @@ +import { z } from "zod"; +import { and, asc, eq, inArray, isNull, sql } from "drizzle-orm"; +import { agentWakeupRequests, chatActions, chatConversations, chatEndpoints, heartbeatRuns, issues, type Db } from "@paperclipai/db"; +import type { DbOrTransaction } from "./runtime-types.js"; + +export const taskRetryInputSchema = z.object({ + // This is a server-selected snapshot of the durable queue, not client input. + // Truncating it strands older requests or makes the newer-input guard reject + // every Retry. Dispatch already carries the full queue with bounded inline text. + sourceActionIds: z.array(z.string().uuid()).refine(ids => new Set(ids).size === ids.length), + runtimeFence: z.object({ generation: z.number().int().nonnegative(), + credentialFingerprint: z.string().regex(/^[a-f0-9]{64}$/) }).optional(), +}); +export type TaskRetryInput = z.infer; + +export async function selectTaskRetryInput(tx: DbOrTransaction, input: { + companyId: string; issueId: string; agentId: string; failedRunId: string; +}) { + const [conversation] = await tx.select({ id: chatConversations.id }).from(chatConversations) + .innerJoin(chatEndpoints, eq(chatEndpoints.id, chatConversations.endpointId)) + .innerJoin(chatActions, and(eq(chatActions.companyId, chatConversations.companyId), + eq(chatActions.conversationId, chatConversations.id))) + .innerJoin(heartbeatRuns, and(eq(heartbeatRuns.companyId, chatConversations.companyId), + eq(heartbeatRuns.wakeupRequestId, chatActions.id))).where(and( + eq(chatConversations.companyId, input.companyId), eq(chatConversations.issueId, input.issueId), + eq(heartbeatRuns.id, input.failedRunId), eq(heartbeatRuns.agentId, input.agentId), + eq(chatEndpoints.provider, "github"), eq(chatEndpoints.assignedAgentId, input.agentId), + eq(chatEndpoints.externalExecutionPolicy, "restricted"), + )).limit(1); + if (!conversation) return undefined; + const pending = await tx.select({ id: chatActions.id }).from(chatActions) + .innerJoin(agentWakeupRequests, and(eq(agentWakeupRequests.companyId, chatActions.companyId), + eq(agentWakeupRequests.id, chatActions.id))).where(and( + eq(chatActions.companyId, input.companyId), eq(chatActions.conversationId, conversation.id), + eq(chatActions.kind, "inbound_wakeup"), eq(chatActions.status, "processed"), + eq(agentWakeupRequests.agentId, input.agentId), eq(agentWakeupRequests.status, "deferred_issue_execution"), + isNull(agentWakeupRequests.runId), + )).orderBy(asc(chatActions.createdAt), asc(chatActions.id)).for("update", { of: agentWakeupRequests }); + return taskRetryInputSchema.parse({ sourceActionIds: pending.map(row => row.id) }); +} + +export async function claimTaskRetryInput(tx: DbOrTransaction, input: { + companyId: string; retryActionId: string; sourceActionIds: string[]; +}) { + if (!input.sourceActionIds.length) return; + await tx.update(agentWakeupRequests).set({ status: "coalesced", updatedAt: new Date(), + payload: sql`coalesce(${agentWakeupRequests.payload}, '{}'::jsonb) || ${JSON.stringify({ + coalescedIntoWakeupRequestId: input.retryActionId })}::jsonb`, + }).where(and(eq(agentWakeupRequests.companyId, input.companyId), + inArray(agentWakeupRequests.id, input.sourceActionIds), eq(agentWakeupRequests.status, "deferred_issue_execution"), + isNull(agentWakeupRequests.runId))); +} + +export async function settleTaskRetryInput(tx: DbOrTransaction, input: { companyId: string; retryActionId: string; runId: string | null }) { + if (!input.runId) return; + await tx.update(agentWakeupRequests).set({ runId: input.runId, updatedAt: new Date() }).where(and( + eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.status, "coalesced"), + isNull(agentWakeupRequests.runId), + sql`${agentWakeupRequests.payload}->>'coalescedIntoWakeupRequestId' = ${input.retryActionId}`, + )); +} + +export async function releaseUnadmittedTaskRetryInput(db: Db, input: { companyId: string; retryActionId: string }) { + await db.transaction(async tx => { + const [snapshot] = await tx.select({ payload: chatActions.payload }).from(chatActions).where(and( + eq(chatActions.companyId, input.companyId), eq(chatActions.id, input.retryActionId), + )); + const issueId = z.string().uuid().safeParse(snapshot?.payload.issueId); + if (!issueId.success) return; + const [issue] = await tx.select({ id: issues.id }).from(issues).where(and( + eq(issues.companyId, input.companyId), eq(issues.id, issueId.data), + )).for("update"); + if (!issue) return; + const [action] = await tx.select().from(chatActions).where(and(eq(chatActions.companyId, input.companyId), + eq(chatActions.id, input.retryActionId), eq(chatActions.kind, "failed_run_retry"), + sql`${chatActions.payload}->>'issueId' = ${issue.id}`, + inArray(chatActions.status, ["failed", "cancelled"]))).for("update"); + if (!action || typeof action.payload.taskRetryIntentId !== "string") return; + const [receipt] = await tx.select({ runId: agentWakeupRequests.runId }).from(agentWakeupRequests).where(and( + eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.id, input.retryActionId), + )).for("update"); + if (receipt?.runId) return; + await tx.update(agentWakeupRequests).set({ status: "deferred_issue_execution", finishedAt: null, + payload: sql`${agentWakeupRequests.payload} - 'coalescedIntoWakeupRequestId'`, updatedAt: new Date(), + }).where(and(eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.status, "coalesced"), + isNull(agentWakeupRequests.runId), + sql`${agentWakeupRequests.payload}->>'coalescedIntoWakeupRequestId' = ${input.retryActionId}`)); + await tx.update(agentWakeupRequests).set({ status: "failed", finishedAt: new Date(), updatedAt: new Date() }).where(and( + eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.id, action.payload.taskRetryIntentId), + eq(agentWakeupRequests.reason, "task_retry"), isNull(agentWakeupRequests.runId), + inArray(agentWakeupRequests.status, ["deferred_issue_execution", "queued", "claimed"]), + sql`${agentWakeupRequests.payload}#>>'{taskRetry,chatActionId}' = ${action.id}`, + )); + }); +} diff --git a/server/src/services/durable-chat-wakeup.ts b/server/src/services/durable-chat-wakeup.ts index 62324a15ece..b9d9f721a28 100644 --- a/server/src/services/durable-chat-wakeup.ts +++ b/server/src/services/durable-chat-wakeup.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { agentWakeupRequests, chatActions, chatDeliveries, chatEndpoints, type Db } from "@paperclipai/db"; -import { and, eq, or, sql, type SQLWrapper } from "drizzle-orm"; +import { and, eq, inArray, or, sql, type SQLWrapper } from "drizzle-orm"; import { HttpError } from "../errors.js"; export interface CommittedChatResponseAuthorizationInput { @@ -211,6 +211,7 @@ export interface DurableChatWakeupRequest { readonly idempotencyKey: string; readonly failedRunRetry?: { readonly failedRunId: string }; readonly authorize: (tx: Db) => Promise; + readonly onAdmitted?: (tx: Db, receipt: { runId: string | null }) => Promise; } const trustedRequests = new WeakSet(); @@ -290,6 +291,26 @@ export function assertDurableChatWakeupReceipt( } } +export function withDurableChatAdmission(request: DurableChatWakeupRequest | undefined, + admit: (tx: Parameters[0]>[0]) => Promise) { + return async (tx: Parameters[0]>[0]) => { + const outcome = await admit(tx); + // An existing deferred receipt is not proof this transaction passed its + // gates. Only dispatch or an explicitly persisted wait admits the input. + if (request?.onAdmitted && (outcome.admitted === true || ["queued", "durable", "coalesced"].includes(outcome.kind))) { + const [receipt] = await tx.select().from(agentWakeupRequests).where(and( + eq(agentWakeupRequests.companyId, request.companyId), eq(agentWakeupRequests.id, request.id), + inArray(agentWakeupRequests.status, ["queued", "claimed", "deferred_issue_execution", "coalesced"]), + )); + if (receipt) { + assertDurableChatWakeupReceipt(request, receipt); + await request.onAdmitted(tx as unknown as Db, { runId: receipt.runId }); + } + } + return outcome; + }; +} + /** Work-start policy only: ongoing tool and publication authorization must not * revoke an already-started run when the operator enables mention gating. */ export async function slackMentionAllowsRunStart( diff --git a/server/src/services/heartbeat/queue.ts b/server/src/services/heartbeat/queue.ts index 35ff18e6e88..2ac1a3f0960 100644 --- a/server/src/services/heartbeat/queue.ts +++ b/server/src/services/heartbeat/queue.ts @@ -1,5 +1,4 @@ -import { authorizationService } from "../authorization.js"; -import { assertTaskRetryConversationAllowed, readTaskRetryIntent, retireSupersededTaskRetry } from "../task-retry.js"; +import { assertTaskRetryConversationAllowed, readTaskRetryIntent, retireSupersededTaskRetry, taskRetryMutationAllowed } from "../task-retry.js"; import { insertIssueComment } from "../issue-comment-writes.js"; import { withExternalAdmissionGuard, assertNoExternalOverlap } from "../../modules/external-agents/index.js"; import { isAgentAwaitingSetup } from "../../modules/agent-lifecycle/index.js"; @@ -50,6 +49,7 @@ import { slackMentionAllowsRunStart, assertDurableChatWakeupReceipt, assertDurableChatWakeupRequest, + withDurableChatAdmission, authorizeFailedChatRunRetryWake, FailedChatRunRetryAuthorizationError, unadmittedChatWakeupCondition, @@ -2809,7 +2809,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende const cancelledRunsToEmit: (typeof heartbeatRuns.$inferSelect)[] = []; const outcome = await db - .transaction(async (tx) => { + .transaction(withDurableChatAdmission(durableRequest, async (tx) => { await tx.execute( sql`select id from issues where id = ${issueId} and company_id = ${agent.companyId} for update`, ); @@ -2847,11 +2847,8 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende if (pending.id !== opts.taskRetryRequestId || retry?.generationId !== pending.id || retry.chatActionId || retry.actorId !== opts.requestedByActorId || retry.sourceRunId !== enrichedContextSnapshot.previousRunId) return { kind: "deferred" as const }; - const permission = await authorizationService(tx as unknown as Db).decide({ - actor: { type: "board", userId: retry.actorId, source: retry.actorSource }, action: "issue:mutate", - resource: { type: "issue", companyId: agent.companyId, issueId }, - }); - if (!permission.allowed) return { kind: "deferred" as const }; + if (!await taskRetryMutationAllowed(tx as unknown as Db, { companyId: agent.companyId, issueId, + actorId: retry.actorId, actorSource: retry.actorSource })) return { kind: "deferred" as const }; } if (opts.queuedCommentRequestId) { const ids = await undeliveredLegacyUserCommentIds(tx as unknown as Db, @@ -3004,11 +3001,8 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende if (!failedChatRetry || retry?.chatActionId !== durableRequest.id || retry.actorId !== taskRetryActorId || retry.sourceRunId !== durableRequest.failedRunRetry?.failedRunId) throw new FailedChatRunRetryAuthorizationError(); - const permission = await authorizationService(tx as unknown as Db).decide({ - actor: { type: "board", userId: retry.actorId, source: retry.actorSource }, action: "issue:mutate", - resource: { type: "issue", companyId: agent.companyId, issueId }, - }); - if (!permission.allowed) return { kind: "deferred" as const }; + if (!await taskRetryMutationAllowed(tx as unknown as Db, { companyId: agent.companyId, issueId, + actorId: retry.actorId, actorSource: retry.actorSource })) return { kind: "deferred" as const }; } } else { if ( @@ -3311,7 +3305,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende payload: sql`jsonb_set(coalesce(${agentWakeupRequests.payload}, '{}'::jsonb), '{executionWait}', ${JSON.stringify(condition)}::jsonb)`, updatedAt: new Date(), }).where(eq(agentWakeupRequests.id, executionWaitRequestId)); - return { kind: "deferred" as const }; + return { kind: "deferred" as const, admitted: true }; } if (durableRequest || wakeCommentId || hasInteractionContinuationWakeContext(enrichedContextSnapshot) || @@ -3348,7 +3342,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende }, }); } - return { kind: "deferred" as const }; + return { kind: "deferred" as const, admitted: true }; }; const explicitContinuationRunId = randomUUID(); const executionBlocker = await getExecutionBlocker( @@ -3974,7 +3968,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende }; } if (admission.kind === "deferred") { - return { kind: "deferred" as const }; + return { kind: "deferred" as const, admitted: true }; } // admission.kind === "proceed": no active run absorbed this wake, // so fall through to the ordinary queue path below. @@ -4357,7 +4351,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende // transitions to "running" — Fix A (lazy locking). return { kind: "queued" as const, run: newRun }; - }) + })) .catch((error) => { if (isExternalChatWaitAuthorizationContention(error)) return { kind: "deferred" as const }; diff --git a/server/src/services/task-retry.ts b/server/src/services/task-retry.ts index 1937712ac23..9300b07d661 100644 --- a/server/src/services/task-retry.ts +++ b/server/src/services/task-retry.ts @@ -8,6 +8,20 @@ import { createSystemRecovery, readSystemRecovery } from "./system-recovery-stat import { updateSystemRecovery } from "./system-recovery.js"; import { instanceSettingsService } from "./instance-settings.js"; import { notifyChatActionWork } from "./chat-work-notifications.js"; +import { authorizationService } from "./authorization.js"; + +export async function taskRetryMutationAllowed(db: Db, input: { + companyId: string; issueId: string; actorId: string; actorSource: TaskRetryIntent["actorSource"]; +}) { + const [issue] = await db.select({ status: issues.status, assigneeAgentId: issues.assigneeAgentId, + assigneeUserId: issues.assigneeUserId }).from(issues).where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId))); + if (!issue) return false; + const permission = await authorizationService(db).decide({ + actor: { type: "board", userId: input.actorId, source: input.actorSource }, action: "issue:mutate", + resource: { type: "issue", companyId: input.companyId, issueId: input.issueId, ...issue }, + }); + return permission.allowed; +} export async function assertTaskRetryConversationAllowed( db: Db, @@ -115,6 +129,7 @@ export async function requestTaskRetry(input: { const intent: TaskRetryIntent = { schema: "paperclip.task-retry.v1", generationId: active?.id ?? id, sourceRunId: latest.id, actorId: input.actorId, actorSource: taskRetryIntentSchema.shape.actorSource.parse(input.actorSource ?? "session"), strategy: "evaluate", + ...(active && activeIntent?.chatActionId ? { chatActionId: activeIntent.chatActionId } : {}), ...(active ? { canonicalIntentId: active.id } : {}) }; // Preserve external chat authority through its existing admission owner. // Generic board Retry must never manufacture a chat-inbound request. From 0373f9f2611088fc019a4206bc68a4d052dfe9fe Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:28:12 -0500 Subject: [PATCH 03/11] fix(recovery): wake retained workspace preservation after Retry (#17741) ## Thinking Path > - Paperclip lets people manage AI agents and their saved work. > - A stopped run can owe workspace preservation before saved input can continue. > - Retry saved a recovery request but did not wake the worker that owns preservation. > - Older recovery records could also take the bounded scan slots before a due request. > - A second Retry could change the revision while the physical owner was still saving files. > - This pull request wakes a separate worker after commit, selects due work first, and keeps the active owner's revision. > - Saved work can progress without replaying the stopped provider turn or blocking other finalization. ## Linked Issues or Issue Description **What happened?** An operator approved a saved design and selected Retry after native workspace finalization failed. The API accepted one canonical request. The source remained scheduled at zero preservation attempts for more than 30 minutes. No new provider turn started. A local regression also found that repeated Retry during export invalidated the physical owner's completion revision. **Expected behavior** Committed Retry must wake bounded source preservation. Due recovery must progress ahead of passive cleanup obligations and unrelated history. Duplicate requests must keep one physical attempt. Saved input must still pass normal admission after verified recovery. **Steps to reproduce** 1. Retain a terminal run with an exhausted workspace recovery and saved authorized input. 2. Let the delivery coordinator become quiet, then request Retry. 3. Observe the recovery state and physical preservation receipt. Repeat Retry while the export is held. **Paperclip version or commit** Observed on served commit `4b89eec644d6e1a99d58926e77ae58927c3fa87b`. Local baseline: `a1f77821b9cd5c9c846fdf7a0fa5cc620ad2b6bb`. Related work: Refs #17053, #17267, #17094. This change repairs explicit Retry delivery. It is separate from the run instruction projection in #17724 and historical unsafe-export discovery in #17687. ## What Changed - Notify a dedicated workspace recovery duty from the transaction that saves Retry, recovery admission, adoption, or recovery progress. - Reconstruct work and saved retry deadlines at startup. Fill each free export slot independently, track physical work for idle drain, and join the dedicated worker's exports on shutdown. - Drain one bounded pass through completed cleanup and saved-input obligations. Restart that pass after committed changes without repeatedly rechecking paused input. - Share source eligibility between deadline selection and candidate consumption. Keep resolved files-only obligations and ignore unrelated resolved or cancelled history. - Select due unfinished preservation before passive historical records. Rotate past source locks held by other workers and retain the indexed source lookup. - Bind the canonical Retry intent without changing an active physical attempt's generation or revision. Preserve that binding across later progress writes. - Add real producer, physical preservation, startup, deadline, idle, and independent finalization regressions. Document the delivery contract. ## Verification - Red controls: Retry without a notification stayed scheduled; duplicate Retry invalidated the active export; whole-batch waiting stranded a third Retry; a scan without cursor rotation starved a source behind two actual advisory locks. Removing unfinished-first selection left HTTP Retry scheduled behind five real recovered sources still owing cleanup. The fixture explicitly verifies their eligibility with the worker's actual query filter. - Further red controls: the previous worker visits only two of three eligible completed sources. Disabling the committed-change cursor reset misses a new earlier source arriving while two callbacks are held. Both pass with the final worker. - Green: the public Retry HTTP route, registered production worker, real file copyback, duplicate canonical receipts, and public recovery receipt reload passed on an owned local fixture. Its agent is paused to keep this files-only check from starting a provider turn. - Full workspace recovery suite passed 56 tests, including the foreign-lock regression. - Execution-control lifecycle suite passed 24 tests. It covers idle SQL, stored deadlines, cancelled and resolved history, independent finalization, and shutdown. One, three, and seven paused completed sources are each visited in a bounded pass, then stay deferred without a successor or SQL during 90 seconds of fake-clock idle time. A committed earlier source is evaluated when it arrives during two held callbacks. - Both GitHub and Telegram `automatic_running` continuation regressions passed with the new canonical intent binding. - `pnpm -r typecheck` passed. - `pnpm build` passed. - `node scripts/check-maps.mjs` and `node scripts/check-module-boundaries.mjs` passed. - Local `pnpm test:run` could not complete: embedded PostgreSQL repeatedly failed to start. The owned run was stopped after this setup failure repeated. Latest-head CI supplies broad coverage. - Earlier setup failures included a localhost sandbox denial and loss of the owned disposable Docker database. These were separate from the product red controls above. - Local database tests use an explicitly named disposable PostgreSQL `_test` database. That opt-in database is reset before the suite. Default embedded test behavior is unchanged. - Exact-head CI is green at `ef7dcf1ba1060864c9d216c33b51f4d9f8f57fbd`; Greptile and Ponytail both rate it 5/5, with no unresolved review threads. Earlier CI caught the indexed discovery assertion and two exact chat intent-binding regressions; all three were fixed and pass locally and in CI. A separate chat shard lost contact with its self-hosted runner without a product assertion result. A later shared runner shutdown interrupted several jobs; one infrastructure rerun passed before the final test-and-worker follow-up triggered a fresh green run. - No staging deployment is part of this PR. The original approval remains saved; deployed recovery still needs acceptance after rollout. ## Risks - More recovery writes now wake a bounded maintenance duty. Completed history must not keep it active. Idle and deadline tests cover this boundary. - Resolved files-only obligations must remain eligible without granting replay. The startup regression checks that path. - Provider stop proof, copyback ownership, source binding, retry budgets, and normal execution admission remain required. There is no schema or public API change. ## Model Used - OpenAI Codex based on GPT-6, with code execution and browser tools. The exact serving model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/execution-semantics.md | 21 +- maps/agent-access-map/recovery.md | 7 + maps/feature-map/recovery.md | 11 + .../__tests__/execution-control-work.test.ts | 143 ++++++++++++- server/src/index.ts | 5 + .../services/delivery-work-notifications.ts | 1 + server/src/services/execution-control-work.ts | 13 +- server/src/services/system-recovery.ts | 54 +++-- server/src/services/task-retry.ts | 12 +- .../src/services/workspace-recovery-source.ts | 23 ++ .../src/services/workspace-recovery-work.ts | 37 ++++ .../src/services/workspace-recovery.test.ts | 198 +++++++++++++++++- server/src/services/workspace-recovery.ts | 65 +++--- 13 files changed, 527 insertions(+), 63 deletions(-) create mode 100644 server/src/services/workspace-recovery-work.ts diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index b64eaaa16f0..486a33f3fa8 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -1385,14 +1385,25 @@ accepted native result. A successful `workspace_recovery` operation and matching recovery record prove persistence; adapter output or a user repair note cannot. The failed run and unknown external effects remain historical facts. -Startup and periodic recovery discover these obligations through the retained -workspace index. Two background exports per server keep unrelated recovery -responsive. Automatic file recovery has three total attempts with a 30-second -delay. An idempotent board request through +Startup and the retained-workspace sweep reconstruct these obligations through +the retained workspace index. Committed Retry, recovery admission, and recovery +progress writes wake a separate preservation worker. It follows the persisted +retry deadline and becomes idle when no eligible recovery remains. Two background +exports per server keep unrelated execution finalization responsive. A resolved +exact-source files-only obligation remains eligible; unrelated resolved or +cancelled recovery history does not keep this worker active. The persisted +attempt budget applies across restarts. An idempotent board request through `POST /api/issues/:id/recovery-actions/recover-workspace` admits one additional attempt after a transient failure. It requires task mutation access, accepts no infrastructure certification, and grants no agent or provider execution authority. -Restarts do not reset the attempt limit. Missing stop or descriptor proof keeps +Each completed export frees its own slot and wakes the next scan. A slow export +does not hold a free slot. Completed cleanup and saved-input obligations receive +one bounded pass through unvisited sources. The pass stops at its tail so paused +input does not keep the worker active; a committed change starts a fresh pass. +Shutdown joins this worker's exports before closing the database. Repeated Retry +during physical preservation binds the canonical intent +without changing the attempt's generation or revision. Progress writes preserve +that newer intent so its fenced completion can still commit. Missing stop or descriptor proof keeps the source allocated and exposes the specific service blocker. File recovery alone never requests another agent turn. Existing saved human diff --git a/maps/agent-access-map/recovery.md b/maps/agent-access-map/recovery.md index e1973773a38..b2581d1d43b 100644 --- a/maps/agent-access-map/recovery.md +++ b/maps/agent-access-map/recovery.md @@ -232,6 +232,13 @@ Gaps: An acknowledged retry can remain pending or fail; report actual useful res Capability ID: `recovery/workspace`. +Committed authorized Retry and workspace-recovery requests notify the internal +preservation worker. Startup reconstructs its source-bound obligations and saved +retry deadlines. A duplicate request during physical preservation does not +replace the owner's generation or revision. Resolved files-only recovery retains +its no-replay restriction. The local PostgreSQL regression checks the producer, +worker, and physical source receipt; it is not deployed provider acceptance. + The internal preservation resume path grants no board or agent execution API. It requires the exact source action, company, run, lease and environment, a held physical workspace owner, and its current stop-only cleanup claim. The diff --git a/maps/feature-map/recovery.md b/maps/feature-map/recovery.md index 8c56541e248..c156b4d7cd7 100644 --- a/maps/feature-map/recovery.md +++ b/maps/feature-map/recovery.md @@ -257,6 +257,17 @@ gap. ### `operator-retry` +The [workspace recovery regression](../../server/src/services/workspace-recovery.test.ts) +uses the real Retry producer and a quiet delivery coordinator. It checks that a +committed request starts one physical source preservation, duplicate Retry keeps +the active owner's receipt valid, and startup reconstructs a resolved files-only +obligation without provider replay. The [execution-control lifecycle suite](../../server/src/__tests__/execution-control-work.test.ts) +checks independent finalization, stored deadlines, and a bounded pass through +completed obligations followed by quiet paused input. +The physical export regression holds one export while another completes, then +verifies a third Retry can use the free slot and shutdown joins the held work. +These local checks do not prove recovery of a deployed allocation. + Automated: [disposition notices](../../ui/src/components/DispositionRecoveryNotice.test.tsx) tests pending/error/gated states and waiting for the real retry acknowledgement. The recovery-card suite covers workspace divergence, reissue, reconcile, and diff --git a/server/src/__tests__/execution-control-work.test.ts b/server/src/__tests__/execution-control-work.test.ts index 9d107b75dd7..0046d8574aa 100644 --- a/server/src/__tests__/execution-control-work.test.ts +++ b/server/src/__tests__/execution-control-work.test.ts @@ -4,7 +4,7 @@ import { localAiLoginService } from "../services/local-ai-login.js"; import { randomUUID } from "node:crypto"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { eq, sql } from "drizzle-orm"; -import { agents, companies, createDb, heartbeatRuns, issues, nativeRunFinalizations, applyPendingMigrations } from "@paperclipai/db"; +import { agents, agentWakeupRequests, companies, createDb, heartbeatRuns, issues, issueRecoveryActions, nativeRunFinalizations, applyPendingMigrations } from "@paperclipai/db"; import { installDatabaseWorkSignals } from "../../../packages/db/src/work-signals.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; @@ -16,7 +16,13 @@ import { terminalizeLegacyExecution } from "../services/legacy-execution-recover import { reconcileAbandonedExecutionControl } from "../services/execution-control-reconciliation.js"; import { issueRecoveryActionService } from "../services/issue-recovery-actions.js"; import { markExecutionReconciliation, deliverReconciledExecutions, settleUnrecoverableExecutions } from "../services/execution-recovery-resolution.js"; +import { nextWorkspaceRecoveryAt } from "../services/system-recovery.js"; +import { createSystemRecovery } from "../services/system-recovery-state.js"; +import { retainedWorkspaceCandidates } from "../services/workspace-recovery.js"; +import { registerWorkspaceRecoveryWork } from "../services/workspace-recovery-work.js"; import { idleWorkSnapshot } from "../services/task-admission.js"; +import { heartbeatService } from "../services/heartbeat.js"; +import { requestTaskRetry } from "../services/task-retry.js"; const coordinators: ReturnType[] = []; afterEach(async () => { for (const worker of coordinators.splice(0)) await worker.stop(); vi.useRealTimers(); }); @@ -94,13 +100,13 @@ describe("execution-control lifecycle", () => { expect(run).toHaveBeenCalledTimes(1); expect(next).toHaveBeenCalledTimes(1); await c.stop(); expect(c.nextWakeAt()).toBeNull(); }); - it("keeps other obligations responsive while one is waiting, and awaits shutdown", async () => { + it.each([DELIVERY_QUEUES.executionReplacement, DELIVERY_QUEUES.workspaceRecovery])("keeps finalization responsive while %s is waiting, and awaits shutdown", async heldQueue => { vi.useFakeTimers(); const owner = fakeOwner(); const c = coordinator(owner); let release!: () => void; const gate = new Promise(resolve => { release = resolve; }); const status = vi.fn(async () => {}); const started = registerExecutionControlWork(c, [ - { queue: DELIVERY_QUEUES.executionReplacement, run: () => gate, next: async () => null }, - { queue: DELIVERY_QUEUES.executionStatus, run: status, next: async () => null }, + { queue: heldQueue, run: () => gate, next: async () => null }, + { queue: DELIVERY_QUEUES.executionFinalization, run: status, next: async () => null }, ]); await vi.advanceTimersByTimeAsync(1); expect(status).toHaveBeenCalledTimes(1); let stopped = false; const stopping = c.stop().then(() => { stopped = true; }); @@ -120,6 +126,7 @@ describe.skipIf(!support.supported)("execution-control durable work", () => { if (external) await applyPendingMigrations(external); else temporary = await startEmbeddedPostgresTestDatabase("execution-sleep-"); db = createDb(external ?? temporary!.connectionString, { maxConnections: 6 }); + await Promise.all(Array.from({ length: 6 }, () => db.execute(sql`select pg_sleep(0.05)`))); }, 90000); afterAll(async () => { await db?.$client.end({ timeout: 1 }); await temporary?.cleanup(); }); beforeEach(async () => { @@ -145,22 +152,140 @@ describe.skipIf(!support.supported)("execution-control durable work", () => { await settle(); expect(c.nextWakeAt()).toBeNull(); const before = calls; await vi.advanceTimersByTimeAsync(90_000); expect(calls).toBe(before); } - async function runRow(status = "running") { - const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status, runtimeMode: "legacy", contextSnapshot: { issueId } }).returning(); + async function runRow(status = "running", sourceIssueId = issueId) { + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status, runtimeMode: "legacy", contextSnapshot: { issueId: sourceIssueId } }).returning(); return run!; } - it("stops all six pending/deadline scans with empty queues and completed history", async () => { + async function pausedRecoveredSource(actionId = randomUUID()) { + const sourceIssueId = randomUUID(); + await db.insert(issues).values({ id: sourceIssueId, companyId, title: "Recovered execution", + status: "in_progress", assigneeAgentId: agentId }); + const run = await runRow("failed", sourceIssueId); + await db.update(heartbeatRuns).set({ resultJson: { startupCancellation: { beforeNativeSelection: true } } }) + .where(eq(heartbeatRuns.id, run.id)); + await db.update(agents).set({ status: "paused" }).where(eq(agents.id, agentId)); + await db.insert(issueRecoveryActions).values({ id: actionId, companyId, sourceIssueId, + kind: "active_run_watchdog", ownerType: "system", cause: "native_workspace_sync_out_retry_exhausted", + fingerprint: run.id, nextAction: "Saved work recovered", evidence: { runId: run.id, + systemRecovery: { ...createSystemRecovery(run.id), state: "complete" }, + workspaceRecovery: { schema: "paperclip.workspace-recovery.v1", runId: run.id, + status: "recovered", stage: "saving_receipts", attempt: 1, attemptLimit: 6, requestIds: [] } } }); + const retry = await requestTaskRetry({ db, companyId, issueId: sourceIssueId, actorId: "board", + observedRunId: run.id, idempotencyKey: randomUUID() }); + expect(retry.accepted).toBe(true); + return { run, retry }; + } + it("stops all pending/deadline scans with empty queues and completed history", async () => { await runRow("succeeded"); const schedule = executionControlSchedule(observed); const c = coordinator(db, undefined, error => { errors.push(error); }); - const topics = [DELIVERY_QUEUES.executionFinalization, DELIVERY_QUEUES.executionReplacement, DELIVERY_QUEUES.executionContinuation, + const topics = [DELIVERY_QUEUES.executionFinalization, DELIVERY_QUEUES.workspaceRecovery, DELIVERY_QUEUES.executionReplacement, DELIVERY_QUEUES.executionContinuation, DELIVERY_QUEUES.executionStatus, DELIVERY_QUEUES.executionDisposition, DELIVERY_QUEUES.localLoginCleanup]; - const runs = [() => reconcileAbandonedExecutionControl(observed), () => reconcileSafeNativeReplacements(observed), + const runs = [() => reconcileAbandonedExecutionControl(observed), async () => {}, () => reconcileSafeNativeReplacements(observed), () => deliverReconciledExecutions(observed, vi.fn(async () => null)), () => deliverExecutionStatuses(observed), () => settleUnrecoverableExecutions(observed), () => localAiLoginService(observed).reapExpired()]; await registerExecutionControlWork(c, Object.values(schedule).map((next, i) => ({ queue: topics[i]!, run: runs[i]!, next }))).ready; await quiet(c); }); + it("schedules only a current unfinished workspace recovery and becomes quiet with retained history", async () => { + const run = await runRow("failed"); + const state = createSystemRecovery(run.id); + const evidence = { runId: run.id, systemRecovery: state, + workspaceRecovery: { schema: "paperclip.workspace-recovery.v1", runId: run.id, + status: "queued", stage: "verifying_stop", attempt: 0, attemptLimit: 6, requestIds: [] } }; + const [action] = await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId, + kind: "active_run_watchdog", ownerType: "system", cause: "native_workspace_sync_out_retry_exhausted", + fingerprint: run.id, nextAction: "Save work", evidence }).returning(); + expect(await nextWorkspaceRecoveryAt(db)).toBe(0); + const due = Date.now() + 60_000; + await db.update(issueRecoveryActions).set({ evidence: { ...evidence, systemRecovery: { ...state, nextAttemptAt: due } } }) + .where(eq(issueRecoveryActions.id, action!.id)); + expect(await executionControlSchedule(db).workspaceRecovery()).toBe(due); + for (const status of ["cancelled", "resolved"] as const) { + await db.update(issueRecoveryActions).set({ status }).where(eq(issueRecoveryActions.id, action!.id)); + expect(await nextWorkspaceRecoveryAt(db)).toBeNull(); + expect(await retainedWorkspaceCandidates(db)).toEqual([]); + } + for (const system of [{ ...state, sourceRunId: randomUUID() }, { ...state, state: "complete" }, { ...state, state: "exhausted" }]) { + await db.update(issueRecoveryActions).set({ status: "active", evidence: { ...evidence, systemRecovery: system } }) + .where(eq(issueRecoveryActions.id, action!.id)); + expect(await nextWorkspaceRecoveryAt(db)).toBeNull(); + } + await db.update(issueRecoveryActions).set({ evidence, cause: "agent_unreachable" }) + .where(eq(issueRecoveryActions.id, action!.id)); + expect(await nextWorkspaceRecoveryAt(db)).toBeNull(); + await db.update(issueRecoveryActions).set({ cause: "native_workspace_sync_out_retry_exhausted", + evidence: { ...evidence, workspaceRecovery: { ...evidence.workspaceRecovery, runId: randomUUID() } } }) + .where(eq(issueRecoveryActions.id, action!.id)); + expect(await nextWorkspaceRecoveryAt(db)).toBeNull(); + expect(await retainedWorkspaceCandidates(db)).toEqual([]); + await db.update(issueRecoveryActions).set({ status: "cancelled", evidence: { ...evidence, + systemRecovery: { ...state, state: "running" } } }).where(eq(issueRecoveryActions.id, action!.id)); + const c = coordinator(db, undefined, error => { errors.push(error); }); + const worker = registerWorkspaceRecoveryWork(observed, c, + {} as Parameters[2], async () => {}); + try { await worker.ready; await quiet(c); } finally { await worker.stop(); } + }); + + it.each([1, 3, 7])("visits all %i paused Retry sources once and becomes quiet after file recovery", async count => { + const sources = []; + for (let index = 0; index < count; index++) sources.push(await pausedRecoveredSource()); + const resume = vi.fn(heartbeatService(observed).resumeRemoteStopComments); + const c = coordinator(db, undefined, error => { errors.push(error); }); + const worker = registerWorkspaceRecoveryWork(observed, c, + {} as Parameters[2], resume); + try { + await worker.ready; + for (let pass = 0; pass < 10 && resume.mock.calls.length < count; pass++) await settle(); + expect(resume).toHaveBeenCalledTimes(count); + await quiet(c); + for (const result of resume.mock.results) { + await expect(result.value).rejects.toThrow("Agent is not invokable in its current state"); + } + for (const { retry } of sources) { + const [saved] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, retry.requestId!)); + expect(saved).toMatchObject({ status: "deferred_issue_execution", runId: null }); + } + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, companyId))).toHaveLength(count); + expect((await db.select().from(agents).where(eq(agents.id, agentId)))[0]?.status).toBe("paused"); + } finally { await c.stop(); await worker.stop(); } + }); + + it("visits a newly committed earlier Retry during a busy recovered-source pass, then becomes quiet", async () => { + const sources = []; + for (let index = 0; index < 3; index++) { + sources.push(await pausedRecoveredSource(`ffffffff-000${index}-4000-8000-${randomUUID().slice(-12)}`)); + } + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const heartbeat = heartbeatService(observed); + const resume = vi.fn(async (run: Parameters[0]) => { + await gate; + return heartbeat.resumeRemoteStopComments(run); + }); + const c = coordinator(db, undefined, error => { errors.push(error); }); + const worker = registerWorkspaceRecoveryWork(observed, c, + {} as Parameters[2], resume); + try { + await worker.ready; + await vi.waitFor(() => expect(resume).toHaveBeenCalledTimes(2)); + const earlier = await pausedRecoveredSource(`00000000-0000-4000-8000-${randomUUID().slice(-12)}`); + sources.push(earlier); + release(); + for (let pass = 0; pass < 10 && sources.some(({ run }) => + !resume.mock.calls.some(([visited]) => visited.id === run.id)); pass++) await settle(); + expect(resume.mock.calls.map(([run]) => run.id)).toContain(earlier.run.id); + await quiet(c); + for (const { run, retry } of sources) { + expect(resume.mock.calls.map(([visited]) => visited.id)).toContain(run.id); + const [saved] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, retry.requestId!)); + expect(saved).toMatchObject({ status: "deferred_issue_execution", runId: null }); + } + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, companyId))).toHaveLength(4); + expect((await db.select().from(agents).where(eq(agents.id, agentId)))[0]?.status).toBe("paused"); + } finally { release(); await c.stop(); await worker.stop(); } + }); + it("bounds real producer bursts with another company's completed history present", async () => { const otherCompany = randomUUID(), otherAgent = randomUUID(); await db.insert(companies).values({ id: otherCompany, name: "Other tenant", issuePrefix: "OTH" }); diff --git a/server/src/index.ts b/server/src/index.ts index 4212387dd04..b4e814959e1 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1,3 +1,4 @@ +import { registerWorkspaceRecoveryWork } from "./services/workspace-recovery-work.js"; import { registerEnvironmentCleanupWork } from "./services/environment-cleanup-work.js"; import { loginCleanupSchedule } from "./services/login-cleanup-state.js"; import { registerLoginCleanupWork } from "./services/login-cleanup-work.js"; @@ -1342,6 +1343,9 @@ async function startServerWithDatabaseTeardown( }); const deliveryWork = app.locals.deliveryWork as ReturnType; const executionSchedule = executionControlSchedule(db); + const workspaceRecoveryRuntime = environmentRuntimeService(db, { pluginWorkerManager }); + const workspaceRecoveryWork = registerWorkspaceRecoveryWork(db, deliveryWork, workspaceRecoveryRuntime, + accountingHeartbeat.resumeRemoteStopComments, () => !heartbeatSchedulerStopped); registerExecutionControlWork(deliveryWork, [ { queue: DELIVERY_QUEUES.executionFinalization, run: () => reconcileAbandonedExecutionControl(db), next: executionSchedule.finalization }, { queue: DELIVERY_QUEUES.executionStatus, run: () => deliverExecutionStatuses(db), next: executionSchedule.status }, @@ -2103,6 +2107,7 @@ async function startServerWithDatabaseTeardown( await skillSourceSync.stop(); unsubscribeChatCompletions(); await deliveryWork.stop(); + await workspaceRecoveryWork.stop(); unregisterTerminalWorkspaceProbe?.(); unsubscribeNativeWorkspaceSyncRetention?.(); if (heartbeatSchedulerInterval) { diff --git a/server/src/services/delivery-work-notifications.ts b/server/src/services/delivery-work-notifications.ts index c4b3b9f2334..66f14532600 100644 --- a/server/src/services/delivery-work-notifications.ts +++ b/server/src/services/delivery-work-notifications.ts @@ -3,6 +3,7 @@ import { signalDatabaseWork, subscribeDatabaseWork } from "@paperclipai/db"; /** Topics for existing durable queues; register intent before their writes. */ export const DELIVERY_QUEUES = { executionFinalization: "execution-finalization", + workspaceRecovery: "workspace-recovery", executionReplacement: "execution-replacement", executionContinuation: "execution-continuation", executionStatus: "execution-status", diff --git a/server/src/services/execution-control-work.ts b/server/src/services/execution-control-work.ts index 1c7a1a6d376..69863954dbc 100644 --- a/server/src/services/execution-control-work.ts +++ b/server/src/services/execution-control-work.ts @@ -1,3 +1,4 @@ +import { nextWorkspaceRecoveryAt } from "./system-recovery.js"; import { and, asc, eq, isNotNull, sql } from "drizzle-orm"; import { adapterAuthSessions, heartbeatRuns, issueRecoveryActions, nativeRunFinalizations, type Db } from "@paperclipai/db"; import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js"; @@ -12,6 +13,7 @@ export function registerExecutionControlWork( queue: DeliveryQueue; run: () => Promise; next: () => Promise; + onSettled?: () => void; }>, canRun: () => boolean = () => true, ) { @@ -20,10 +22,10 @@ export function registerExecutionControlWork( let reconciled = 0; let next: number | null = null; let earliestRunAt = 0; - return coordinator.register(duty.queue, { + const worker = coordinator.register(duty.queue, { retryMs: EXECUTION_RECONCILIATION_INTERVAL_MS, pausePassiveMaintenance: true, - onSettled: () => { requested += 1; }, + onSettled: () => { requested += 1; duty.onSettled?.(); }, run: async () => { if (!canRun()) { requested += 1; next = null; return; } const now = Date.now(); @@ -41,8 +43,9 @@ export function registerExecutionControlWork( hasPending: async () => reconciled !== requested, nextRunAt: () => next, }); + return { ...worker, wake: () => { requested += 1; worker.wake(); } }; }); - return { ready: Promise.all(workers.map(worker => worker.ready)) }; + return { ready: Promise.all(workers.map(worker => worker.ready)), wake: () => { for (const worker of workers) worker.wake(); } }; } export function executionControlSchedule(db: Db) { @@ -63,6 +66,10 @@ export function executionControlSchedule(db: Db) { ]); return deadline([native[0]?.at, legacy[0]?.at]); }, + async workspaceRecovery() { + const at = await nextWorkspaceRecoveryAt(db); + return at === null ? null : deadline([new Date(at)]); + }, async replacement() { const rows = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns) .innerJoin(nativeRunFinalizations, eq(nativeRunFinalizations.runId, heartbeatRuns.id)) diff --git a/server/src/services/system-recovery.ts b/server/src/services/system-recovery.ts index 75eea95666f..8dba366e1af 100644 --- a/server/src/services/system-recovery.ts +++ b/server/src/services/system-recovery.ts @@ -1,10 +1,17 @@ import { and, eq, sql } from "drizzle-orm"; -import { issueComments, issueRecoveryActions, type Db } from "@paperclipai/db"; +import { heartbeatRuns, issueComments, issueRecoveryActions, type Db } from "@paperclipai/db"; +import { writeExecutionControl } from "./execution-control-notifications.js"; +import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js"; import { createSystemRecovery, readSystemRecovery, type SystemRecoveryState } from "./system-recovery-state.js"; -import type { WorkspaceRecoverySourceIdentity } from "./workspace-recovery-source.js"; +import { workspaceRecoverySourceEligible, type WorkspaceRecoverySourceIdentity } from "./workspace-recovery-source.js"; export async function adoptSystemRecovery(db: Db, source: WorkspaceRecoverySourceIdentity) { return db.transaction(async tx => { + const [existing] = await tx.select({ evidence: issueRecoveryActions.evidence }).from(issueRecoveryActions) + .where(and(eq(issueRecoveryActions.companyId, source.companyId), eq(issueRecoveryActions.id, source.actionId), + eq(issueRecoveryActions.sourceIssueId, source.issueId))).for("update"); + if (!existing || existing.evidence.systemRecovery) return; + await notifyDeliveryWork(tx, DELIVERY_QUEUES.workspaceRecovery); const state = createSystemRecovery(source.runId); // Atomic, once per source. An exhausted previous execution budget is history; // adoption grants only the explicitly versioned maintenance capability. @@ -49,17 +56,22 @@ export async function updateSystemRecovery(db: Db, source: WorkspaceRecoverySour stage: next.phase === "verify_execution" ? "verifying_stop" : next.phase === "preserve_files" ? "restoring_files" : "saving_receipts", attempt: next.attempts, attemptLimit: 6, retryable: true, retryAt: next.nextAttemptAt ?? undefined, }; - const [updated] = await db.update(issueRecoveryActions).set({ - ownerType: "system", ownerAgentId: null, ownerUserId: null, - evidence: sql`${issueRecoveryActions.evidence} || jsonb_build_object('systemRecovery', ${JSON.stringify(next)}::jsonb, - 'workspaceRecovery', ${JSON.stringify(compatibility)}::jsonb || jsonb_build_object('requestIds', - coalesce(${issueRecoveryActions.evidence}#>'{workspaceRecovery,requestIds}', '[]'::jsonb)))`, - updatedAt: new Date(), - }).where(and(eq(issueRecoveryActions.companyId, source.companyId), eq(issueRecoveryActions.id, source.actionId), - sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,generationId}' = ${previous.generationId}`, - sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,revision}' = ${String(previous.revision)}`)) - .returning({ id: issueRecoveryActions.id }); - return Boolean(updated); + return writeExecutionControl(db, DELIVERY_QUEUES.workspaceRecovery, async tx => { + const [updated] = await tx.update(issueRecoveryActions).set({ + ownerType: "system", ownerAgentId: null, ownerUserId: null, + evidence: sql`${issueRecoveryActions.evidence} || jsonb_build_object('systemRecovery', ${JSON.stringify(next)}::jsonb || + ${next.generationId === previous.generationId && next.intentId === previous.intentId + ? sql`jsonb_build_object('intentId', ${issueRecoveryActions.evidence}#>'{systemRecovery,intentId}')` + : sql`'{}'::jsonb`}, + 'workspaceRecovery', ${JSON.stringify(compatibility)}::jsonb || jsonb_build_object('requestIds', + coalesce(${issueRecoveryActions.evidence}#>'{workspaceRecovery,requestIds}', '[]'::jsonb)))`, + updatedAt: new Date(), + }).where(and(eq(issueRecoveryActions.companyId, source.companyId), eq(issueRecoveryActions.id, source.actionId), + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,generationId}' = ${previous.generationId}`, + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,revision}' = ${String(previous.revision)}`)) + .returning({ id: issueRecoveryActions.id }); + return Boolean(updated); + }); } export async function loadSystemRecovery(db: Db, source: WorkspaceRecoverySourceIdentity) { @@ -68,3 +80,19 @@ export async function loadSystemRecovery(db: Db, source: WorkspaceRecoverySource eq(issueRecoveryActions.id, source.actionId))); return readSystemRecovery(row?.evidence); } + +export async function nextWorkspaceRecoveryAt(db: Db): Promise { + const rows = await db.select({ at: sql`case + when jsonb_typeof(${issueRecoveryActions.evidence}#>'{systemRecovery,nextAttemptAt}') = 'number' + then (${issueRecoveryActions.evidence}#>>'{systemRecovery,nextAttemptAt}')::bigint else 0 end` }) + .from(issueRecoveryActions).innerJoin(heartbeatRuns, and( + eq(heartbeatRuns.companyId, issueRecoveryActions.companyId), + eq(heartbeatRuns.id, sql`case when ${issueRecoveryActions.evidence}->>'runId' ~* '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$' + then (${issueRecoveryActions.evidence}->>'runId')::uuid end`), + sql`${heartbeatRuns.status} in ('failed', 'interrupted', 'timed_out', 'cancelled', 'succeeded')`, + )).where(and( + workspaceRecoverySourceEligible(), + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' in ('scheduled', 'running', 'awaiting_evidence')`, + )).orderBy(sql`1`).limit(1); + return rows.length ? Number(rows[0].at) : null; +} diff --git a/server/src/services/task-retry.ts b/server/src/services/task-retry.ts index 9300b07d661..afbfbb34d08 100644 --- a/server/src/services/task-retry.ts +++ b/server/src/services/task-retry.ts @@ -1,3 +1,5 @@ +import { writeExecutionControl } from "./execution-control-notifications.js"; +import { DELIVERY_QUEUES } from "./delivery-work-notifications.js"; import { z } from "zod"; import { randomUUID } from "node:crypto"; import { and, desc, eq, inArray, or, sql } from "drizzle-orm"; @@ -73,7 +75,7 @@ export async function requestTaskRetry(input: { prepareChatRetry?: (tx: Db, scope: { companyId: string; issueId: string; agentId: string; failedRunId: string; initiatedByUserId: string; retryIntentId?: string }) => Promise<{ actionId: string }>; }) { - return input.db.transaction(async tx => { + return writeExecutionControl(input.db, DELIVERY_QUEUES.workspaceRecovery, async tx => { const db = tx as unknown as Db; const [issue] = await tx.select().from(issues).where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId))).for("update"); @@ -154,6 +156,11 @@ export async function requestTaskRetry(input: { for (const action of records) { const previous = readSystemRecovery(action.evidence); if (!previous || previous.state === "complete") continue; + if (previous.state === "running") { + await updateSystemRecovery(db, { companyId: issue.companyId, issueId: issue.id, runId: latest.id, + actionId: action.id }, previous, { ...previous, intentId: active?.id ?? id }); + continue; + } // Active repair keeps its attempt and cooldown. An exhausted generation // gets a new budget only because this click is a new durable intent. const next = previous.state === "exhausted" @@ -163,8 +170,7 @@ export async function requestTaskRetry(input: { : { ...previous, revision: previous.revision + 1, intentId: active?.id ?? id, // Retry may advance our backoff, never a provider's cooldown and // never an operation whose physical owner is still running. - nextAttemptAt: previous.state === "running" ? previous.nextAttemptAt - : Math.max(Date.now(), previous.providerCooldownAt ?? 0) }; + nextAttemptAt: Math.max(Date.now(), previous.providerCooldownAt ?? 0) }; if (next.generationId !== previous.generationId) await persistActivity(db, { companyId: issue.companyId, actorType: "system", actorId: "task-retry", entityType: "issue", entityId: issue.id, action: "issue.recovery_generation_replaced", runId: latest.id, diff --git a/server/src/services/workspace-recovery-source.ts b/server/src/services/workspace-recovery-source.ts index ab8517aa88c..61b9320b7d7 100644 --- a/server/src/services/workspace-recovery-source.ts +++ b/server/src/services/workspace-recovery-source.ts @@ -10,6 +10,29 @@ export const workspaceExportCauses = ["native_workspace_sync_out_unrecoverable", export const terminalWorkspaceRunStates = ["failed", "interrupted", "timed_out", "cancelled", "succeeded"]; export type WorkspaceRecoverySourceIdentity = { companyId: string; issueId: string; runId: string; actionId: string }; +export function workspaceRecoverySourceEligible(includeCompleted = false) { + const actionEligible = sql`(${issueRecoveryActions.status} in ('active', 'escalated') or + (${issueRecoveryActions.status} = 'resolved' and ( + (${issueRecoveryActions.evidence}#>>'{automaticRecovery,policy}' = 'preserve_without_replay_v1' + and ${issueRecoveryActions.evidence}#>>'{automaticRecovery,replay}' = 'blocked' + and ${issueRecoveryActions.evidence}#>>'{automaticRecovery,runId}' = ${heartbeatRuns.id}::text) or + (${issueRecoveryActions.evidence}#>>'{historicalWorkspaceSource,schema}' = 'paperclip.historical-workspace-source.v1' + and ${issueRecoveryActions.evidence}#>>'{historicalWorkspaceSource,runId}' = ${heartbeatRuns.id}::text))))`; + return and( + sql`${issueRecoveryActions.evidence} ? 'systemRecovery'`, + includeCompleted ? sql`(${actionEligible} or ${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' = 'complete')` : actionEligible, + sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.issueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${issueRecoveryActions.sourceIssueId}::text`, + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,sourceRunId}' = ${heartbeatRuns.id}::text`, + sql`${issueRecoveryActions.evidence}#>>'{workspaceRecovery,runId}' = ${heartbeatRuns.id}::text`, + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,policyVersion}' = '1'`, + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,schema}' = 'paperclip.system-recovery.v1'`, + sql`${issueRecoveryActions.evidence}#>>'{workspaceRecovery,schema}' = 'paperclip.workspace-recovery.v1'`, + sql`(${issueRecoveryActions.evidence}->'workspaceRestoreRecovery'->>'schema' = 'paperclip.workspace-restore-recovery.v1' + or ${issueRecoveryActions.evidence}->'historicalWorkspaceSource'->>'schema' = 'paperclip.historical-workspace-source.v1' + or ${issueRecoveryActions.cause} in ('native_workspace_sync_out_unrecoverable', 'native_workspace_sync_out_retry_exhausted'))`, + ); +} + /** The preservation owner may reopen only its exact stop-only allocation. * This does not release the source's ordinary execution or destruction fence. */ export async function ownedWorkspaceRecoveryLease(db: Db, input: WorkspaceRecoverySourceIdentity & { diff --git a/server/src/services/workspace-recovery-work.ts b/server/src/services/workspace-recovery-work.ts new file mode 100644 index 00000000000..dea2bf28b46 --- /dev/null +++ b/server/src/services/workspace-recovery-work.ts @@ -0,0 +1,37 @@ +import type { Db } from "@paperclipai/db"; +import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js"; +import { DELIVERY_QUEUES } from "./delivery-work-notifications.js"; +import { executionControlSchedule, registerExecutionControlWork } from "./execution-control-work.js"; +import type { EnvironmentRuntimeService } from "./environment-runtime.js"; +import { reconcileRetainedWorkspaces } from "./workspace-recovery.js"; + +export function registerWorkspaceRecoveryWork(db: Db, coordinator: ReturnType, + runtime: EnvironmentRuntimeService, onRecovered: Parameters[2], + canRun: () => boolean = () => true) { + const pending = new Set>(); + let stopped = false; + let scan: Promise | undefined; + let wakeWhileBusy = false; + let passiveSweep: { cursor?: string } = {}; + const schedule = executionControlSchedule(db); + const worker = registerExecutionControlWork(coordinator, [{ + queue: DELIVERY_QUEUES.workspaceRecovery, + onSettled: () => { passiveSweep = {}; if (pending.size >= 2) wakeWhileBusy = true; }, + run: () => scan = (async () => { + const work = await reconcileRetainedWorkspaces(db, runtime, onRecovered, true, continueSweep => { + if (!stopped && (continueSweep || wakeWhileBusy)) { wakeWhileBusy = false; worker.wake(); } + }, passiveSweep); + for (const job of work ?? []) { + pending.add(job); + const settled = () => { pending.delete(job); }; + void job.then(settled, settled); + } + })(), + next: () => pending.size >= 2 ? Promise.resolve(null) : schedule.workspaceRecovery(), + }], () => !stopped && canRun()); + return { ready: worker.ready, async stop() { + stopped = true; + await scan?.catch(() => {}); + await Promise.all(pending); + } }; +} diff --git a/server/src/services/workspace-recovery.test.ts b/server/src/services/workspace-recovery.test.ts index 9edfdf1d391..e9519384bab 100644 --- a/server/src/services/workspace-recovery.test.ts +++ b/server/src/services/workspace-recovery.test.ts @@ -20,21 +20,29 @@ import { generateKeyPairSync, randomUUID, sign } from "node:crypto"; import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import express from "express"; +import request from "supertest"; import { and, eq, inArray, sql } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { agents, approvals, budgetPolicies, costEvents, completionContracts, issueApprovals, issueTreeHolds, issueTreeHoldMembers, companies, createDb, environmentLeases, environments, heartbeatRuns, issueComments, issueRecoveryActions, issues, nativeRunFinalizations, nativeRunResults, agentWakeupRequests, workspaceOperations } from "@paperclipai/db"; +import { agents, approvals, budgetPolicies, costEvents, completionContracts, issueApprovals, issueTreeHolds, issueTreeHoldMembers, companies, createDb, applyPendingMigrations, environmentLeases, environments, heartbeatRuns, issueComments, issueRecoveryActions, issues, nativeRunFinalizations, nativeRunResults, agentWakeupRequests, workspaceOperations } from "@paperclipai/db"; import { resumePreservedResultFinalization } from "./preserved-result-finalization.js"; import type { AdapterSandboxExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import type { EnvironmentLease } from "@paperclipai/shared"; import { startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js"; +import { resolveExternalTestDatabaseUrl } from "../__tests__/helpers/external-test-database.js"; import { prepareNativeWorkspaceSync } from "./native-runtime/native-workspace-sync.js"; +import { nativeWorkspaceFinalizationLockKey } from "./native-runtime/native-workspace-finalization-ownership.js"; import { remoteTerminationReceipt } from "./remote-execution-termination.js"; import { prepareStoppedSandboxForRecovery } from "./sandbox-stop-and-retain.js"; import { recoverRetainedWorkspace, requestWorkspaceRecovery, scheduleRecoveredWorkspaceCleanup, reconcileRetainedWorkspaces, retainedWorkspaceCandidates } from "./workspace-recovery.js"; import * as activity from "./activity-log.js"; +import { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js"; +import { registerWorkspaceRecoveryWork } from "./workspace-recovery-work.js"; import { heartbeatService } from "./heartbeat.js"; import { hasVerifiedWorkspaceRepair, readWorkspaceRecovery } from "./workspace-recovery-receipt.js"; import { getExecutionBlocker } from "./execution-blocker.js"; +import { issueRoutes } from "../routes/issues.js"; +import { errorHandler } from "../middleware/index.js"; import * as workerProof from "./native-runtime/native-workspace-platform-proof.js"; const targets = vi.hoisted(() => new Map()); vi.mock("./environment-execution-target.js", async importOriginal => { @@ -51,7 +59,13 @@ describe("automatic retained workspace recovery", () => { beforeAll(async () => { root = await mkdtemp(path.join(os.tmpdir(), "workspace-recovery-")); process.env.PAPERCLIP_HOME = path.join(root, "home"); process.env.PAPERCLIP_INSTANCE_ID = "recovery"; - temporary = await startEmbeddedPostgresTestDatabase("workspace-recovery-"); db = createDb(temporary.connectionString); + const external = resolveExternalTestDatabaseUrl("PAPERCLIP_WORKSPACE_RECOVERY_TEST_DATABASE_URL"); + if (external) { + await applyPendingMigrations(external); + temporary = { connectionString: external, cleanup: async () => {} }; + } else temporary = await startEmbeddedPostgresTestDatabase("workspace-recovery-"); + db = createDb(temporary.connectionString); + if (external) await db.execute(sql`TRUNCATE TABLE companies CASCADE`); }, 30_000); afterAll(async () => { await db?.$client.end(); await temporary?.cleanup(); await rm(root, { recursive: true, force: true }); @@ -123,6 +137,178 @@ describe("automatic retained workspace recovery", () => { const action = async (id: string) => (await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, id)))[0]!; const run = async (id: string) => (await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, id)))[0]!; + it("wakes physical preservation from HTTP Retry before passive history without replaying the provider", async () => { + for (let index = 0; index < 5; index++) { + const old = await seed(); + await recoverRetainedWorkspace(old.input); + await db.update(issueRecoveryActions).set({ + id: `00000000-000${index}-4000-8000-${randomUUID().slice(-12)}`, + }).where(eq(issueRecoveryActions.id, old.actionId)); + } + const f = await seed(); + await adoptSystemRecovery(db, f); + const before = await action(f.actionId); + const exhausted = failSystemRecovery(readSystemRecovery(before.evidence)!, + new SystemRecoveryFailure("source_binding_invalid", "service_required")); + await db.update(issueRecoveryActions).set({ cause: "native_workspace_sync_out_retry_exhausted", + evidence: { ...before.evidence, systemRecovery: exhausted } }) + .where(eq(issueRecoveryActions.id, f.actionId)); + const errors = vi.fn(); + const competingCleanup = await retainedWorkspaceCandidates(db, undefined, false, true); + expect(competingCleanup).toHaveLength(5); + expect(competingCleanup.every(row => row.action.id.startsWith("00000000"))).toBe(true); + const coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, canReconcile: () => true, onError: errors }); + const recovered = vi.fn(async () => {}); + const worker = registerWorkspaceRecoveryWork(db, coordinator, f.environmentRuntime, recovered); + try { + await worker.ready; + await db.update(agents).set({ status: "paused" }).where(eq(agents.companyId, f.companyId)); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { req.actor = { type: "board", source: "local_implicit", userId: "board" }; next(); }); + app.use("/api", issueRoutes(db, {} as Parameters[1])); + app.use(errorHandler); + const body = { idempotencyKey: randomUUID(), observedRunId: f.runId }; + const retry = await request(app).post(`/api/issues/${f.issueId}/retry`).send(body).expect(202); + const repeated = await request(app).post(`/api/issues/${f.issueId}/retry`).send(body).expect(202); + const coalesced = await request(app).post(`/api/issues/${f.issueId}/retry`) + .send({ ...body, idempotencyKey: randomUUID() }).expect(202); + expect(repeated.body.requestId).toBe(retry.body.requestId); + expect(coalesced.body.requestId).toBe(retry.body.requestId); + await expect.poll(async () => readSystemRecovery((await action(f.actionId)).evidence)?.state, + { timeout: 20_000 }).toBe("complete"); + expect(await readFile(path.join(f.host, "saved.txt"), "utf8")).toBe("finished work\n"); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); + expect(await db.select().from(nativeRunResults).where(eq(nativeRunResults.companyId, f.companyId))).toHaveLength(0); + const reloaded = await request(app).get(`/api/issues/${f.issueId}/recovery-actions`).expect(200); + expect(JSON.stringify(reloaded.body)).toContain('"state":"complete"'); + expect(errors).not.toHaveBeenCalled(); + } finally { await coordinator.stop(); await worker.stop(); } + }, 30_000); + + it("reconstructs a resolved files-only preservation obligation at startup without replay", async () => { + const f = await seed("legacy"); + await adoptSystemRecovery(db, f); + const before = await action(f.actionId); + const automaticRecovery = { policy: "preserve_without_replay_v1", runId: f.runId, replay: "blocked" }; + await db.update(issueRecoveryActions).set({ status: "resolved", outcome: "blocked", + evidence: { ...before.evidence, automaticRecovery } }).where(eq(issueRecoveryActions.id, f.actionId)); + const errors = vi.fn(); + const coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, canReconcile: () => true, onError: errors }); + const worker = registerWorkspaceRecoveryWork(db, coordinator, f.environmentRuntime, async () => {}); + try { + await worker.ready; + await expect.poll(async () => readSystemRecovery((await action(f.actionId)).evidence)?.state, + { timeout: 20_000 }).toBe("complete"); + expect((await action(f.actionId)).evidence.automaticRecovery).toEqual(automaticRecovery); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); + expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0); + expect(errors).not.toHaveBeenCalled(); + } finally { await coordinator.stop(); await worker.stop(); } + }, 30_000); + + it("fills a free export slot while another export is held and joins both at shutdown", async () => { + const held = await seed(), fast = await seed(); + const fixtures = [held, fast]; + const runtimeFor = (runId: string) => fixtures.find(fixture => fixture.runId === runId)!.environmentRuntime; + const runtime = { ...held.environmentRuntime, + resumeWorkspaceRecoveryLease: (input: Parameters[0]) => + runtimeFor(input.source.runId).resumeWorkspaceRecoveryLease(input), + retryPendingSandboxTeardown: (input: Parameters[0]) => + runtimeFor(input.lease.heartbeatRunId!).retryPendingSandboxTeardown(input), + }; + let active = 0, maximum = 0, release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const countExport = (fixture: typeof held, pause = false) => { + const resume = fixture.runtime.resumeRunLease.getMockImplementation()!; + fixture.runtime.resumeRunLease.mockImplementation(async input => { + active += 1; maximum = Math.max(maximum, active); + try { if (pause) await gate; return await resume(input); } finally { active -= 1; } + }); + }; + countExport(held, true); countExport(fast); + const errors = vi.fn(); + const coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, onError: errors }); + const worker = registerWorkspaceRecoveryWork(db, coordinator, runtime, async () => {}); + try { + await expect.poll(() => held.runtime.resumeRunLease.mock.calls.length, { timeout: 20_000 }).toBe(1); + await expect.poll(async () => readSystemRecovery((await action(fast.actionId)).evidence)?.state, + { timeout: 20_000 }).toBe("complete"); + const third = await seed(); fixtures.push(third); countExport(third); + await requestTaskRetry({ db, companyId: third.companyId, issueId: third.issueId, actorId: "board", + observedRunId: third.runId, idempotencyKey: randomUUID() }); + await expect.poll(async () => readSystemRecovery((await action(third.actionId)).evidence)?.state, + { timeout: 20_000 }).toBe("complete"); + expect(readSystemRecovery((await action(held.actionId)).evidence)?.state).toBe("running"); + expect(maximum).toBeLessThanOrEqual(2); + expect(await readFile(path.join(third.host, "saved.txt"), "utf8")).toBe("finished work\n"); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, third.companyId))).toHaveLength(1); + let stopped = false; + const stopping = Promise.all([coordinator.stop(), worker.stop()]).then(() => { stopped = true; }); + await Promise.resolve(); expect(stopped).toBe(false); + release(); await stopping; + expect(readSystemRecovery((await action(held.actionId)).evidence)?.state).toBe("complete"); + expect(errors).not.toHaveBeenCalled(); + } finally { release(); await coordinator.stop(); await worker.stop(); } + }, 60_000); + + it("rotates past due sources whose physical locks belong to other workers", async () => { + const blocked = [await seed(), await seed()], available = await seed(); + for (const [index, fixture] of [...blocked, available].entries()) { + await adoptSystemRecovery(db, fixture); + const id = `00000001-000${index}-4000-8000-${randomUUID().slice(-12)}` as const; + await db.update(issueRecoveryActions).set({ id }).where(eq(issueRecoveryActions.id, fixture.actionId)); + fixture.actionId = id; fixture.input.actionId = id; + } + let unlock!: () => void; + const gate = new Promise(resolve => { unlock = resolve; }); + const held = blocked.map(fixture => { + let ready!: () => void; + const acquired = new Promise(resolve => { ready = resolve; }); + const lock = db.transaction(async tx => { + await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${nativeWorkspaceFinalizationLockKey(fixture.companyId, fixture.runId)}, 0))`); + ready(); await gate; + }); + return { acquired, lock }; + }); + try { + await Promise.all(held.map(item => item.acquired)); + await Promise.all(await reconcileRetainedWorkspaces(db, available.environmentRuntime, async () => {}, true) ?? []); + expect(blocked.every(fixture => fixture.runtime.resumeRunLease.mock.calls.length === 0)).toBe(true); + await Promise.all(await reconcileRetainedWorkspaces(db, available.environmentRuntime, async () => {}, true) ?? []); + expect(readSystemRecovery((await action(available.actionId)).evidence)?.state).toBe("complete"); + expect(await readFile(path.join(available.host, "saved.txt"), "utf8")).toBe("finished work\n"); + expect(blocked.every(fixture => fixture.runtime.resumeRunLease.mock.calls.length === 0)).toBe(true); + } finally { unlock(); await Promise.all(held.map(item => item.lock)); } + }, 30_000); + + it("coalesces Retry while preservation owns its physical operation", async () => { + const f = await seed(); + await adoptSystemRecovery(db, f); + const resume = f.runtime.resumeRunLease.getMockImplementation()!; + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + f.runtime.resumeRunLease.mockImplementation(async input => { await gate; return resume(input); }); + const preserving = recoverRetainedWorkspace(f.input); + let requestId: string | null | undefined; + try { + await expect.poll(() => f.runtime.resumeRunLease.mock.calls.length).toBe(1); + const request = { db, companyId: f.companyId, issueId: f.issueId, actorId: "board", + idempotencyKey: randomUUID(), observedRunId: f.runId }; + const first = await requestTaskRetry(request); + requestId = first.requestId; + const second = await requestTaskRetry({ ...request, idempotencyKey: randomUUID() }); + expect(second.requestId).toBe(first.requestId); + } finally { release(); await preserving; } + expect(readSystemRecovery((await action(f.actionId)).evidence)?.state).toBe("complete"); + expect(readSystemRecovery((await action(f.actionId)).evidence)?.intentId).toBe(requestId); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); + expect(await readFile(path.join(f.host, "saved.txt"), "utf8")).toBe("finished work\n"); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); + }, 30_000); + it("adopts only the exact historical infrastructure notice, preserving prose and user-action notices", async () => { const f = await seed("legacy", false); const metadata = { version: 1 as const, sourceRunId: f.runId, @@ -285,7 +471,7 @@ describe("automatic retained workspace recovery", () => { expect(await db.select().from(nativeRunResults).where(eq(nativeRunResults.companyId, f.companyId))).toHaveLength(0); expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0); await recoverRetainedWorkspace(f.input); - expect(f.runtime.resumeRunLease).toHaveBeenCalledOnce(); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); expect(await db.select().from(workspaceOperations).where(and(eq(workspaceOperations.companyId, f.companyId), eq(workspaceOperations.phase, "workspace_recovery")))).toHaveLength(1); }, 30_000); @@ -304,9 +490,9 @@ describe("automatic retained workspace recovery", () => { expect(readWorkspaceRecovery((await action(f.actionId)).evidence)?.status).toBe("recovered"); expect(await readFile(path.join(f.host, "saved.txt"), "utf8")).toBe("finished work\n"); expect(await readFile(path.join(f.host, "newer.txt"), "utf8")).toBe("newer local work\n"); - expect(f.runtime.resumeRunLease).toHaveBeenCalledOnce(); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); await recoverRetainedWorkspace({ ...f.input, environmentRuntime: runtime }); - expect(f.runtime.resumeRunLease).toHaveBeenCalledOnce(); + expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0); }, 30_000); @@ -405,7 +591,7 @@ describe("automatic retained workspace recovery", () => { f.runtime.resumeRunLease.mockImplementationOnce(async () => { ready(); await gate; return { providerLeaseId: f.providerLeaseId, metadata: { remoteCwd: f.remote } }; }); const first = recoverRetainedWorkspace(f.input); await started; - try { await recoverRetainedWorkspace(f.input); expect(f.runtime.resumeRunLease).toHaveBeenCalledOnce(); } + try { await recoverRetainedWorkspace(f.input); expect(f.runtime.resumeRunLease.mock.calls.filter(([input]) => input.lease.id === f.lease.id)).toHaveLength(1); } finally { release(); await first; } expect(readWorkspaceRecovery((await action(f.actionId)).evidence)?.status).toBe("recovered"); }, 30_000); diff --git a/server/src/services/workspace-recovery.ts b/server/src/services/workspace-recovery.ts index e2fae8179e5..28b0f301d28 100644 --- a/server/src/services/workspace-recovery.ts +++ b/server/src/services/workspace-recovery.ts @@ -1,3 +1,5 @@ +import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js"; +import { beginIdleTrackedWork } from "./task-admission.js"; import { writeEnvironmentCleanup } from "./environment-cleanup-notifications.js"; import { resumePreservedResultFinalization } from "./preserved-result-finalization.js"; import { and, asc, eq, gt, inArray, ne, or, sql } from "drizzle-orm"; @@ -19,7 +21,7 @@ import { classifyNativeWorkspaceFailure } from "./native-runtime/native-workspac import { adoptSystemRecovery, loadSystemRecovery, updateSystemRecovery } from "./system-recovery.js"; import { createSystemRecovery, failSystemRecovery, readSystemRecovery, reserveSystemRecoveryAttempt, SystemRecoveryFailure, type SystemRecoveryState } from "./system-recovery-state.js"; -import { inspectWorkspaceRecoverySource } from "./workspace-recovery-source.js"; +import { inspectWorkspaceRecoverySource, workspaceRecoverySourceEligible } from "./workspace-recovery-source.js"; import { preserveWorkspaceSnapshot } from "./workspace-recovery-snapshot.js"; import { adoptHistoricalWorkspaceRecoveries } from "./workspace-recovery-backfill.js"; @@ -48,12 +50,13 @@ export async function requestWorkspaceRecovery(input: Source & { db: Db; idempot requestIds: [...(prior?.requestIds ?? []), input.idempotencyKey] }; const previous = readSystemRecovery(action.evidence); const active = previous && ["scheduled", "running", "awaiting_evidence"].includes(previous.state); - const system = active ? { ...previous, revision: previous.revision + 1, - nextAttemptAt: previous.state === "running" ? previous.nextAttemptAt : Math.max(Date.now(), previous.providerCooldownAt ?? 0) } : { ...createSystemRecovery(run.id), + const system = previous?.state === "running" ? previous : active ? { ...previous, revision: previous.revision + 1, + nextAttemptAt: Math.max(Date.now(), previous.providerCooldownAt ?? 0) } : { ...createSystemRecovery(run.id), revision: (previous?.revision ?? -1) + 1, preservationOperationIds: previous?.preservationOperationIds ?? [], providerCooldownAt: previous?.providerCooldownAt ?? null, nextAttemptAt: previous?.providerCooldownAt ?? null }; + await notifyDeliveryWork(tx, DELIVERY_QUEUES.workspaceRecovery); await tx.update(issueRecoveryActions).set({ ownerType: "system", ownerAgentId: null, ownerUserId: null, evidence: { ...action.evidence, workspaceRecovery: state, systemRecovery: system }, updatedAt: new Date() }) .where(eq(issueRecoveryActions.id, action.id)); @@ -286,6 +289,7 @@ export async function recoverRetainedWorkspace(input: Source & { db: Db; environ const cursors = new WeakMap(); const workInFlight = new WeakMap>(); +const unfinishedCursors = new WeakMap(); /** Destruction is a later cleanup obligation, authorized only by a matching * saved-file receipt and the original recorded disposition. */ @@ -309,29 +313,31 @@ export async function scheduleRecoveredWorkspaceCleanup(db: Db, run: typeof hear sql`${environmentLeases.metadata} = ${JSON.stringify(lease.metadata)}::jsonb`, sql`${environmentLeases.metadata}->>'workspaceRecoveryCleanup' is null`))); } -export function retainedWorkspaceCandidates(db: Db, cursor?: string) { +export function retainedWorkspaceCandidates(db: Db, cursor?: string, unfinishedOnly = false, obligationsOnly = false) { + const cleanup = sql`(${heartbeatRuns.runnerProfileJson}#>>'{nativeWorkspaceSync,resourceDisposition}' = 'destroy' and exists ( + select 1 from ${environmentLeases} where ${environmentLeases.companyId} = ${heartbeatRuns.companyId} + and ${environmentLeases.heartbeatRunId} = ${heartbeatRuns.id} + and ${environmentLeases.id}::text = ${heartbeatRuns.runnerProfileJson}#>>'{nativeWorkspaceSync,leaseId}' + and ${environmentLeases.status} = 'released' and ${environmentLeases.cleanupStatus} = 'success' + and ${environmentLeases.metadata}->'workspaceRecoveryCleanup' is null))`; + const waitingInput = sql`exists (select 1 from ${agentWakeupRequests} + where ${agentWakeupRequests.companyId} = ${issueRecoveryActions.companyId} + and ${agentWakeupRequests.status} = 'deferred_issue_execution' and ${agentWakeupRequests.requestedByActorType} = 'user' + and ${agentWakeupRequests.payload}->>'issueId' = ${issueRecoveryActions.sourceIssueId}::text)`; return db.select({ action: issueRecoveryActions, run: heartbeatRuns }).from(issueRecoveryActions) .innerJoin(heartbeatRuns, and(eq(heartbeatRuns.companyId, issueRecoveryActions.companyId), eq(heartbeatRuns.id, sql`case when ${issueRecoveryActions.evidence}->>'runId' ~* '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$' then (${issueRecoveryActions.evidence}->>'runId')::uuid end`))) .where(and(inArray(heartbeatRuns.status, terminal), - sql`(${issueRecoveryActions.evidence}->'workspaceRestoreRecovery'->>'schema' = 'paperclip.workspace-restore-recovery.v1' - or ${issueRecoveryActions.evidence}->'historicalWorkspaceSource'->>'schema' = 'paperclip.historical-workspace-source.v1' - or ${issueRecoveryActions.cause} in ('native_workspace_sync_out_unrecoverable', 'native_workspace_sync_out_retry_exhausted'))`, - sql`${issueRecoveryActions.evidence} ? 'systemRecovery'`, + workspaceRecoverySourceEligible(true), + unfinishedOnly ? sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' in ('scheduled', 'running', 'awaiting_evidence')` : undefined, + obligationsOnly ? sql`(${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' in ('scheduled', 'running', 'awaiting_evidence') + or (${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' = 'complete' and ${cleanup}) or ${waitingInput})` : undefined, sql`(${issueRecoveryActions.evidence}#>>'{systemRecovery,state}' in ('scheduled', 'running', 'awaiting_evidence', 'complete', 'exhausted') and (${issueRecoveryActions.evidence}#>'{systemRecovery,nextAttemptAt}' = 'null'::jsonb or ${issueRecoveryActions.evidence}#>'{systemRecovery,nextAttemptAt}' <= to_jsonb(${Date.now()}::bigint)))`, sql`(${issueRecoveryActions.evidence}#>>'{workspaceRecovery,status}' is distinct from 'recovered' or - (${heartbeatRuns.runnerProfileJson}#>>'{nativeWorkspaceSync,resourceDisposition}' = 'destroy' and exists ( - select 1 from ${environmentLeases} where ${environmentLeases.companyId} = ${heartbeatRuns.companyId} - and ${environmentLeases.heartbeatRunId} = ${heartbeatRuns.id} - and ${environmentLeases.id}::text = ${heartbeatRuns.runnerProfileJson}#>>'{nativeWorkspaceSync,leaseId}' - and ${environmentLeases.status} = 'released' and ${environmentLeases.cleanupStatus} = 'success' - and ${environmentLeases.metadata}->'workspaceRecoveryCleanup' is null)) or exists ( - select 1 from ${agentWakeupRequests} where ${agentWakeupRequests.companyId} = ${issueRecoveryActions.companyId} - and ${agentWakeupRequests.status} = 'deferred_issue_execution' and ${agentWakeupRequests.requestedByActorType} = 'user' - and ${agentWakeupRequests.payload}->>'issueId' = ${issueRecoveryActions.sourceIssueId}::text))`, + ${cleanup} or ${waitingInput})`, cursor ? gt(issueRecoveryActions.id, cursor) : undefined, )).orderBy(asc(issueRecoveryActions.id)).limit(5); } @@ -339,19 +345,29 @@ export function retainedWorkspaceCandidates(db: Db, cursor?: string) { /** Bounded discovery shares the periodic recovery lifecycle. A repaired source * still owes cleanup even without saved input or after task reconciliation. */ export async function reconcileRetainedWorkspaces(db: Db, runtime: EnvironmentRuntimeService, - onRecovered: (run: typeof heartbeatRuns.$inferSelect) => Promise) { + onRecovered: (run: typeof heartbeatRuns.$inferSelect) => Promise, obligationsOnly = false, + onFinished?: (continueSweep: boolean) => void, passiveSweep?: { cursor?: string }) { await adoptHistoricalWorkspaceRecoveries(db); let pending = workInFlight.get(db); if (!pending) { pending = new Set(); workInFlight.set(db, pending); } if (pending.size >= 2) return; - let rows = await retainedWorkspaceCandidates(db, cursors.get(db)); - if (!rows.length) rows = await retainedWorkspaceCandidates(db); + let rows = await retainedWorkspaceCandidates(db, unfinishedCursors.get(db), true); + if (!rows.length) rows = await retainedWorkspaceCandidates(db, undefined, true); + const unfinished = rows.length > 0; + if (!rows.length) rows = await retainedWorkspaceCandidates(db, passiveSweep ? passiveSweep.cursor : cursors.get(db), false, obligationsOnly); + if (!rows.length && !passiveSweep) rows = await retainedWorkspaceCandidates(db, undefined, false, obligationsOnly); + const work: Promise[] = []; + let hasUnvisited = false; for (const { action, run } of rows) { - if (pending.size >= 2) break; - cursors.set(db, action.id); + if (pending.size >= 2) { hasUnvisited = true; break; } + if (unfinished) unfinishedCursors.set(db, action.id); + else if (passiveSweep) passiveSweep.cursor = action.id; + else cursors.set(db, action.id); if (pending.has(run.id)) continue; pending.add(run.id); - void (async () => { try { + const finish = beginIdleTrackedWork(); + const physical = ["scheduled", "running", "awaiting_evidence"].includes(readSystemRecovery(action.evidence)?.state ?? ""); + work.push((async () => { try { await recoverRetainedWorkspace({ db, environmentRuntime: runtime, companyId: action.companyId, issueId: action.sourceIssueId, runId: run.id, actionId: action.id }); const [updated] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, action.id)); const recovered = readWorkspaceRecovery(updated?.evidence ?? null)?.status === "recovered"; @@ -370,6 +386,7 @@ export async function reconcileRetainedWorkspaces(db: Db, runtime: EnvironmentRu } } } catch { logger.warn({ runId: run.id }, "Retained workspace remains pending"); } - finally { pending.delete(run.id); } })(); + finally { pending.delete(run.id); finish(); onFinished?.(physical || hasUnvisited); } })()); } + return work; } From d64cb7be52b6bb05214694dd38f77a608e5014da Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:28:59 -0500 Subject: [PATCH 04/11] fix(ui): identify accounts in AI connection choices (#17738) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent settings select an authorized AI account independently of the harness and model. > - Multiple accounts can have the same provider name. > - The account picker and legacy adoption dialog showed only that name. > - Operators could not distinguish a personal API key from a shared subscription during credential repair. > - This pull request shows the existing sign-in method, sharing scope, and available account identity before selection and adoption. > - Operators can select the intended account while the existing access and validation checks remain effective. ## Linked Issues or Issue Description Refs #14970 for the shared account selection control. No duplicate account-identity repair was found. **What happened?** Two authorized OpenAI accounts appeared as identical `OpenAI` choices. The adoption dialog repeated that ambiguous name. The list response already contained the account method, scope, and identity, but the UI omitted them. **Expected behavior** An operator can distinguish a personal API key from a company-shared subscription and confirm the selected account before saving. **Steps to reproduce** 1. Make a personal OpenAI API-key account and a shared OpenAI subscription available to an agent. Give both the name `OpenAI`. 2. Open Harness / Runtime and choose a managed connection for a legacy agent. 3. Inspect the two choices and the adoption confirmation. **Paperclip version or commit** Reproduced on master `a1f77821b9cd5c9c846fdf7a0fa5cc620ad2b6bb`. **Deployment mode** Hosted staging and the source UI regression. ## What Changed - Show the sign-in method, personal/shared scope, and available account or owner identity in connection labels. - Use the same label in the selected value and legacy adoption confirmation. Match the confirmation to both the connection and grant. Wrap dropdown choices so narrow screens retain the account identity, and expose the full selected label in the trigger title. - Add a behavioral regression with duplicate provider names, exact grant selection, and disabled unavailable accounts. - Update affected Storybook interaction selectors, the account guide, and the agent verification recipe. ## Verification - Before the fix, the new regression fails because duplicate accounts have no distinguishing metadata. The other 13 field tests pass. - After the fix, all 67 tests in `ui/src/components/ai-connections` pass. The regression selects the exact personal grant and keeps the unavailable option disabled. - `pnpm -r typecheck` and `pnpm build` pass locally. The full `pnpm test:run` command was stopped after embedded PostgreSQL failed to initialize after five attempts in an unrelated native-runtime suite. Its log is retained. Current-head CI partitions must establish full test coverage. - Token gates, map checks, Storybook catalog checks, and all 30 map/catalog contract tests pass. - In local Chrome, the existing production-component Storybook dropdown shows method and scope. Its nine interaction steps pass, as do all 19 steps in the new-agent connection-modes story. At a 320-pixel preview width, two shared OpenAI subscriptions show their distinct account identities in full, including during keyboard selection. These fixtures do not establish provider authentication. - Separately, the staging credential-repair workflow validated an existing authorized shared OpenAI connection, persisted the agent binding across reload, and resumed the original task into real provider/tool execution. This configuration recovery does not deploy this UI patch or establish task completion. - All CI completes on final head `a283bda2ad2f026f4f3c0fee03bf2962b0ad6e02`: 55 successful checks, the neutral Paperclip review, and two intentionally skipped Storybook checks; no pending or failed checks. This includes the partitioned tests, typecheck, build, runner suites, canary, and eight browser shards. Greptile and Paperclip review both rate this exact head 5/5 with no unresolved findings. [CI run](https://github.com/paperclipai/paperclip/actions/runs/40338613167). - The review's selected-trigger truncation finding is fixed in the final head. Its title assertion failed before the fix, and all 67 account-selection tests, UI typecheck, and token gates pass afterward. ## Risks - Labels are longer and can truncate in narrow selected-value controls; the trigger title exposes the complete label. Dropdown choices wrap, and the adoption dialog uses the complete label. - The UI uses existing authorized account metadata. It adds no credential reads, access grants, provider fallback, or account selection policy. ## Model Used OpenAI Codex (GPT-6). The exact serving model identifier and context window are not exposed in this session. The agent used code editing, terminal tests, and browser tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/connections/AI-CONNECTIONS.md | 6 ++++ maps/feature-map/agents.md | 8 +++++ .../ai-connections/AiConnectionField.test.tsx | 31 +++++++++++++++++-- .../ai-connections/AiConnectionField.tsx | 12 ++++--- .../ai-connections/AiConnectionSelect.tsx | 12 +++---- ui/src/components/ai-connections/model.ts | 10 ++++++ .../stories/agents/bulk-settings.stories.tsx | 4 +-- .../bulk-agent-settings.stories.tsx | 4 +-- ...provider-routing-06-production.stories.tsx | 17 +++++++--- 9 files changed, 84 insertions(+), 20 deletions(-) diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index 1bd9d7a81da..6a12c8930e3 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -59,6 +59,12 @@ the selected connection owns its provider routing. Changing those separately may make a binding incompatible; saving then requires a compatible choice. Agent configuration cannot grant access to another account. +Agent account choices and legacy-adoption confirmations show the sign-in method, +personal or company-shared scope, and available account or owner identity beside +the connection name. Accounts with the same provider name can be distinguished +before selection. These labels use existing authorized account metadata, never +credentials or authentication paths. + Personal defaults are unique per company, user, and provider. A Claude bot can use one user’s subscription and another user’s API key without changing its harness or model. Explicit shared selections remain pinned to the selected account and method. The first successful personal connection sets a default only when none exists. The additive `ai_provider_defaults` table preserves the legacy per-method preferences. Migration selects each user’s most recently updated provider preference (including unavailable accounts), and rerunning it never overwrites a provider default. New writes maintain the legacy table for older servers. A database trigger propagates older servers’ explicit default updates to the provider default. Inserting an additional method default does not replace an existing provider default. diff --git a/maps/feature-map/agents.md b/maps/feature-map/agents.md index bb3b705cd1d..e19eb2205b6 100644 --- a/maps/feature-map/agents.md +++ b/maps/feature-map/agents.md @@ -128,6 +128,14 @@ from the hiring picker; API fixture creation does not prove this UI journey. Automated: [instruction service](../../server/src/__tests__/agent-instructions-service.test.ts) and [instruction revisions](../../server/src/__tests__/agent-instruction-revisions.test.ts) cover file/revision contracts. +The [AI account field regression](../../ui/src/components/ai-connections/AiConnectionField.test.tsx) +checks duplicate provider names, account identity in choices and adoption confirmation, +the exact selected grant, and disabled unavailable accounts with authorized metadata fixtures. +Manually compare a personal API key and shared subscription with the same name in +Harness / Runtime. Confirm their method, scope and available identity before saving. +Reload the saved choice and verify the next eligible task uses it; labels alone +do not establish credential validity or change account access. + Manual: Save a harmless instruction edit, reload, and verify its revision and effect in the next eligible run. Pause and resume the disposable agent; confirm pause prevents new work and does not erase previous runs. For failed setup, use an unavailable model or invalid command on a disposable agent. diff --git a/ui/src/components/ai-connections/AiConnectionField.test.tsx b/ui/src/components/ai-connections/AiConnectionField.test.tsx index cc9e6395173..ecf976d41cb 100644 --- a/ui/src/components/ai-connections/AiConnectionField.test.tsx +++ b/ui/src/components/ai-connections/AiConnectionField.test.tsx @@ -17,7 +17,6 @@ vi.mock("./AiConnectionCredentialStep", () => ({ return
Provider sign-in
; }, })); -vi.mock("./AiConnectionManagement", () => ({ AiConnectionLegacyNotice: () => null })); vi.mock("./AiProviderSetup", () => ({ AiProviderSetup: () =>
Advanced provider setup
})); vi.mock("@/pages/apps/AppLogo", () => ({ AppLogo: () => null })); @@ -82,6 +81,33 @@ it("reconnects the unavailable personal default in place", async () => { expect(onChange).toHaveBeenCalledWith({ provider: "anthropic", method: "subscription", mode: "responsible_user" }); }); +it("identifies duplicate-named accounts before adopting the exact personal grant", async () => { + const personal = account({ id: "00000000-0000-4000-8000-000000000001", grantId: "00000000-0000-4000-8000-000000000002", provider: "openai", method: "api_key", name: "OpenAI", ownerName: "Avery", status: "connected", isDefault: false }); + const shared = account({ id: "00000000-0000-4000-8000-000000000003", grantId: "00000000-0000-4000-8000-000000000004", provider: "openai", method: "subscription", name: "OpenAI", accountLabel: "engineering@example.test", ownership: "shared", ownerUserId: undefined, status: "connected", isDefault: false }); + const sharedKey = account({ id: "00000000-0000-4000-8000-000000000005", grantId: "00000000-0000-4000-8000-000000000006", provider: "openai", method: "api_key", name: "OpenAI", ownerName: "Engineering", ownership: "shared", ownerUserId: undefined, status: "connected", isDefault: false }); + const unavailable = account({ id: "expired-account", grantId: "expired-grant", provider: "openai", method: "api_key", name: "Expired account", ownerName: "Avery" }); + mocks.list.mockResolvedValue({ currentUserId: "owner", connections: [shared, personal, sharedKey, unavailable], canManageConnections: true }); + flushSync(() => root.render( + + )); + await settle(); + await click("Choose a managed connection"); + const trigger = container.querySelector('[role="combobox"][aria-label="Connection"]')!; + flushSync(() => trigger.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }))); + await settle(); + const choices = [...document.querySelectorAll('[role="option"]')]; + expect(choices.map(choice => choice.textContent)).toContain("OpenAI · API key · Personal · Avery"); + expect(choices.map(choice => choice.textContent)).toContain("OpenAI · ChatGPT subscription · Company shared · engineering@example.test"); + expect(choices.map(choice => choice.textContent)).toContain("OpenAI · API key · Company shared · Engineering"); + expect(choices.find(choice => choice.textContent?.includes("Expired account"))?.getAttribute("aria-disabled")).toBe("true"); + flushSync(() => choices.find(choice => choice.textContent === "OpenAI · API key · Personal · Avery")!.click()); + await settle(); + expect(document.querySelector('[role="dialog"]')?.textContent).toContain("OpenAI · API key · Personal · Avery"); + expect(onChange).not.toHaveBeenCalled(); + await click("Use this binding when saved"); + expect(onChange).toHaveBeenCalledExactlyOnceWith({ provider: "openai", method: "api_key", mode: "delegated", connectionId: personal.id, grantId: personal.grantId }); +}); + it("opens provider setup directly when connecting from the advanced mode", async () => { await mount([], true, true); await click("Connect another account"); @@ -163,7 +189,7 @@ it("offers Pi account selection before choosing its required model", async () => }); it.each(["openai", "anthropic", "google", "openrouter"] as const)("keeps a matching saved Pi %s API-key account selectable", async provider => { - const saved = account({ provider, method: "api_key", name: "Pi provider account", status: "connected" }); + const saved = account({ provider, method: "api_key", name: "Pi provider account", accountLabel: "avery@example.test", status: "connected" }); mocks.list.mockResolvedValue({ currentUserId: "owner", connections: [saved], canManageConnections: true }); flushSync(() => root.render( trigger.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }))); await settle(); const choice = [...document.querySelectorAll('[role="option"]')].find(node => node.textContent?.includes("Pi provider account")); diff --git a/ui/src/components/ai-connections/AiConnectionField.tsx b/ui/src/components/ai-connections/AiConnectionField.tsx index 5fcbe5e2757..d7d40336afd 100644 --- a/ui/src/components/ai-connections/AiConnectionField.tsx +++ b/ui/src/components/ai-connections/AiConnectionField.tsx @@ -14,6 +14,7 @@ import { AiConnectionSelect } from "./AiConnectionSelect"; import { AiProviderSetup } from "./AiProviderSetup"; import { AiConnectionCredentialStep } from "./AiConnectionCredentialStep"; import { AiConnectionLegacyNotice } from "./AiConnectionManagement"; +import { aiConnectionDisplayLabel } from "./model"; import { Button } from "@/components/ui/button"; import { Checkbox } from "@/components/ui/checkbox"; import { @@ -89,6 +90,11 @@ export function AiConnectionField({ enabled: Boolean(provider), }); const personalDefault = accounts.data?.connections.find((account) => account.provider === provider && account.isDefault && account.ownership === "personal" && account.ownerUserId === accounts.data.currentUserId); + const pendingAccount = pendingAdoption?.mode === "responsible_user" + ? personalDefault + : pendingAdoption && pendingAdoption.mode !== "router" + ? accounts.data?.connections.find(account => account.id === pendingAdoption.connectionId && account.grantId === pendingAdoption.grantId) + : undefined; const selectDefault = useMutation({ mutationFn: async (result: NonNullable) => { // Reconnect retains the existing default and its access. A new account @@ -177,10 +183,8 @@ export function AiConnectionField({ {pendingAdoption?.mode !== "router" &&

{pendingAdoption?.mode === "responsible_user" - ? `Responsible user’s default. For you: ${accounts.data?.connections.find((account) => account.isDefault && account.provider === provider)?.name ?? "Not connected"}. Other users use their own default.` - : accounts.data?.connections.find( - (account) => account.id === pendingAdoption?.connectionId, - )?.name} + ? `Responsible user’s default. For you: ${pendingAccount ? aiConnectionDisplayLabel(pendingAccount) : "Not connected"}. Other users use their own default.` + : pendingAccount && aiConnectionDisplayLabel(pendingAccount)}

} {pendingAdoption?.mode !== "router" &&

After adoption, missing credentials block execution. Previous diff --git a/ui/src/components/ai-connections/AiConnectionSelect.tsx b/ui/src/components/ai-connections/AiConnectionSelect.tsx index 4eb82c01f1a..1eb36785f1e 100644 --- a/ui/src/components/ai-connections/AiConnectionSelect.tsx +++ b/ui/src/components/ai-connections/AiConnectionSelect.tsx @@ -14,15 +14,15 @@ import { } from "@/components/ui/select"; import { AppLogo } from "@/pages/apps/AppLogo"; import { Button } from "@/components/ui/button"; -import { aiConnectionProblem, personalAiDefault } from "./model"; +import { aiConnectionDisplayLabel, aiConnectionProblem, personalAiDefault } from "./model"; import type { AiConnectionPickerProps } from "./AiConnectionPicker"; import type { AiConnectionSummary } from "./model"; -function ConnectionLabel({ connection, label }: { connection: AiConnectionSummary; label?: string }) { +function ConnectionLabel({ connection, label, wrap = false }: { connection: AiConnectionSummary; label?: string; wrap?: boolean }) { return ( - {label ?? connection.name} + {label ?? aiConnectionDisplayLabel(connection)} ); } @@ -119,7 +119,7 @@ export function AiConnectionSelect({ onValueChange={change} disabled={readOnly || loading || Boolean(error)} > - + {incompatible && selected && ( - + )} {compatible.map((c) => ( @@ -144,7 +144,7 @@ export function AiConnectionSelect({ value={c.id} disabled={Boolean(aiConnectionProblem(c))} > - + ))} {(canUseDefault || value?.mode === "responsible_user") && ( diff --git a/ui/src/components/ai-connections/model.ts b/ui/src/components/ai-connections/model.ts index e6e645be500..3ce4ad2e080 100644 --- a/ui/src/components/ai-connections/model.ts +++ b/ui/src/components/ai-connections/model.ts @@ -57,6 +57,16 @@ export function aiMethodLabel(provider: AiProvider, method: AiAuthMethod) { : provider === "github" ? "Personal access token" : "API key"; } +export function aiConnectionDisplayLabel(connection: AiConnectionSummary) { + const personal = connection.ownership === "personal"; + return [ + connection.name, + aiMethodLabel(connection.provider, connection.method), + personal ? "Personal" : "Company shared", + connection.accountLabel || connection.ownerName, + ].filter(Boolean).join(" · "); +} + export function matchesAiRequirement( connection: AiConnectionSummary, requirement: AiConnectionRequirement, diff --git a/ui/storybook/stories/agents/bulk-settings.stories.tsx b/ui/storybook/stories/agents/bulk-settings.stories.tsx index b1ae65d3042..690ab6ae5e0 100644 --- a/ui/storybook/stories/agents/bulk-settings.stories.tsx +++ b/ui/storybook/stories/agents/bulk-settings.stories.tsx @@ -120,7 +120,7 @@ export const ConnectionJourney: Story = { play: async ({ canvasElement }) => { await expect(await screen.findByText("1 other selected agent won’t change.")).toBeVisible(); await userEvent.click(screen.getByRole("checkbox", { name: "Change connection" })); await userEvent.click(await screen.findByRole("combobox", { name: "Connection" })); - await userEvent.click(screen.getByRole("option", { name: "OpenAI · Production" })); + await userEvent.click(screen.getByRole("option", { name: /^OpenAI · Production ·/ })); await expect(screen.getByRole("button", { name: "Update 4 agents" })).toBeEnabled(); await userEvent.click(screen.getByRole("button", { name: "Update 4 agents" })); await expect(await within(canvasElement).findByText("Updated 4 Codex agents.")).toBeVisible(); @@ -131,7 +131,7 @@ export const RetryJourney: Story = { args: { failOne: true }, play: async ({ can const screen = await selectCodex(canvasElement); await userEvent.click(await screen.findByRole("checkbox", { name: "Change connection" })); await userEvent.click(await screen.findByRole("combobox", { name: "Connection" })); - await userEvent.click(screen.getByRole("option", { name: "OpenAI · Production" })); + await userEvent.click(screen.getByRole("option", { name: /^OpenAI · Production ·/ })); await expect(screen.getByRole("button", { name: "Update 4 agents" })).toBeEnabled(); await userEvent.click(screen.getByRole("button", { name: "Update 4 agents" })); await expect(await screen.findByText("3 updated · 1 couldn’t be saved")).toBeVisible(); diff --git a/ui/storybook/stories/design-proposals/bulk-agent-settings.stories.tsx b/ui/storybook/stories/design-proposals/bulk-agent-settings.stories.tsx index f92685f6e65..54222b132fe 100644 --- a/ui/storybook/stories/design-proposals/bulk-agent-settings.stories.tsx +++ b/ui/storybook/stories/design-proposals/bulk-agent-settings.stories.tsx @@ -46,7 +46,7 @@ export const ConnectionJourney: Story = { await step("Changing only the connection keeps every model", async () => { await userEvent.click(screen.getByRole("checkbox", { name: "Change connection" })); await userEvent.click(screen.getByRole("combobox", { name: "Connection" })); - await userEvent.click(screen.getByRole("option", { name: "OpenAI · Production" })); + await userEvent.click(screen.getByRole("option", { name: /^OpenAI · Production ·/ })); await userEvent.click(screen.getByRole("button", { name: "Update 4 agents" })); await expect(await canvas.findByText("Updated 4 Codex agents.")).toBeVisible(); await expect(canvas.getByTestId("agent-row-scout")).toHaveTextContent("gpt-5.3-codex"); @@ -100,7 +100,7 @@ export const RetryJourney: Story = { play: async ({ canvasElement }) => { const screen = within(canvasElement.ownerDocument.body); await userEvent.click(await screen.findByRole("combobox", { name: "Connection" })); - await userEvent.click(screen.getByRole("option", { name: "OpenAI · Production" })); + await userEvent.click(screen.getByRole("option", { name: /^OpenAI · Production ·/ })); await userEvent.click(screen.getByRole("button", { name: "Update 4 agents" })); await expect(await screen.findByText("3 updated · 1 couldn’t be saved")).toBeVisible(); await expect(within(canvasElement).getByTestId("agent-row-atlas")).toHaveTextContent("OpenAI · Production"); diff --git a/ui/storybook/stories/runs-adapters/provider-routing-06-production.stories.tsx b/ui/storybook/stories/runs-adapters/provider-routing-06-production.stories.tsx index 540420d7ec1..35235a5ba95 100644 --- a/ui/storybook/stories/runs-adapters/provider-routing-06-production.stories.tsx +++ b/ui/storybook/stories/runs-adapters/provider-routing-06-production.stories.tsx @@ -61,6 +61,13 @@ const accounts: AiManagedConnectionSummary[] = [ isDefault: true, status: "connected", }, + ...["engineering@example.test", "design@example.test"].map((accountLabel, index) => ({ + id: `00000000-0000-4000-8000-00000000000${index + 8}`, + grantId: `00000000-0000-4000-8000-00000000001${index}`, + companyId, provider: "openai" as const, method: "subscription" as const, + name: "OpenAI", ownership: "shared" as const, accountLabel, + isDefault: false, status: "connected" as const, + })), ]; function Providers({ canManageConnections = true }: { canManageConnections?: boolean }) { const [client] = useState(() => { @@ -200,7 +207,7 @@ export const NewAgentConnectionModes: Story = { await expect(canvas.getByRole("radio", { name: "Advanced Custom Gateway" })).toBeChecked(); await expect(canvas.getByRole("button", { name: "Use connection" })).toBeDisabled(); await userEvent.click(canvas.getByRole("combobox", { name: "Connection" })); - await userEvent.click(within(canvasElement.ownerDocument.body).getByRole("option", { name: "Company OpenRouter" })); + await userEvent.click(within(canvasElement.ownerDocument.body).getByRole("option", { name: "Company OpenRouter · API key · Company shared" })); await userEvent.click(canvas.getByRole("button", { name: "Use connection" })); await expect(canvas.getByRole("status")).toHaveTextContent("Continue to Configure with openrouter."); await userEvent.click(canvas.getByRole("radio", { name: "OpenAI Subscription" })); @@ -259,13 +266,15 @@ export const CompatibleConnectionDropdown: Story = { body.getByRole("option", { name: "Responsible user’s default" }), ).toHaveAttribute("data-state", "checked"); await expect( - body.getByRole("option", { name: "Company OpenRouter" }), + body.getByRole("option", { name: "Company OpenRouter · API key · Company shared" }), ).toBeVisible(); + await expect(body.getByRole("option", { name: "OpenAI · ChatGPT subscription · Company shared · engineering@example.test" })).toBeVisible(); + await expect(body.getByRole("option", { name: "OpenAI · ChatGPT subscription · Company shared · design@example.test" })).toBeVisible(); await expect( - body.queryByRole("option", { name: "My Claude subscription" }), + body.queryByRole("option", { name: /My Claude subscription/ }), ).not.toBeInTheDocument(); await userEvent.click( - body.getByRole("option", { name: "Company OpenRouter" }), + body.getByRole("option", { name: "Company OpenRouter · API key · Company shared" }), ); await expect( within(canvasElement).getByRole("combobox", { name: "Connection" }), From 5823e9a6646b1624416a1e33f6016037cb88c1a3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:29:51 -0500 Subject: [PATCH 05/11] fix(runner): preserve disabled Codex web search (#17744) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Codex agents use the saved agent settings to prepare each provider launch. > - Onboarding saves web search as off, but native configuration conversion removes that setting. > - Codex 0.160.0 includes a hosted web-search tool by default, even in an authentication probe that asks it not to use tools. > - A gateway that permits only priced function tools can reject that request before budget admission. > - This change preserves the explicit restriction through native execution and disables web search for authentication probes. ## Linked Issues or Issue Description **What happened?** A native Codex authentication probe can include hosted web search. Normal task execution also loses a saved `search: false` setting. A restricted Responses gateway can reject these requests before it makes a model call. Related repairs: Refs #17445, Refs #17529. The roadmap and related PRs were checked; no duplicate repair was found. **Expected behavior** Authentication probes check the selected account and model without hosted web search. A saved search-off setting remains effective in native task launches, prepared launches, and session recovery. Existing behavior for an absent setting remains unchanged. **Steps to reproduce** 1. Create a Codex agent with search off and choose Paperclip Runner. 2. Use a Responses gateway that rejects hosted tools. 3. Inspect the request from the native authentication probe or task launch. Before this fix, the pinned Codex SDK includes `web_search` despite the saved setting. ## What Changed - Preserve an explicit search-off setting when native agent configuration is built and converted. - Carry the restriction in the durable native input and provider launch requirements. - Apply the restriction to authentication probes and normal launch paths, including custom arguments and recovery. - Reject prepared launches that discard the required restriction. Keep gateway billing rules unchanged. - Admit only the restrictive literal value in the strict plugin task API. Exercise the real plugin setup path. - Add regression coverage and document the policy. - Add the immutable Copilot v60 source identity required by the changed shared files. Preserve v59 and all original live evidence. Copilot authentication probes keep their prior launch requirements. ## Verification - Regression cases were run against the unfixed source and failed at the configuration and launch boundaries. - An isolated capture using the real Codex 0.160.0 app-server reproduced hosted-tool rejection. Disabling web search reached a mocked budget reservation. The helper and child processes permitted only loopback network access and used synthetic credentials. It made zero upstream calls. - Focused checks passed: 284 server cases, 61 native form cases, 198 backend/security/launch/parser cases, and 36 authentication/launch cases. A separate complete transport/launch/parser run passed 393 cases. These groups overlap. - Head `617b98f` passed workspace typecheck/build and 28/28 plugin contract and actual Runner probe tests with no skips. Final rebased head `20ff0c1` passed all current-head CI checks, including full isolated test lanes, and fresh Greptile and Paperclip reviews at 5/5. - The additive Copilot identity update passed 134 compatibility/policy/protocol checks and nine authentication probe checks. The real pinned Codex SDK capture was repeated after the final policy change: all five cases passed with zero upstream calls. - Two local full-suite attempts stopped after native PostgreSQL loader failures. Seven local dependency aliases were restored, and an isolated cluster initialized successfully. A third run later encountered host PostgreSQL semaphore exhaustion and was interrupted before source-profile regeneration. Its zero exit code after SIGINT is explicitly not a full-suite pass. Final full-suite verification runs in isolated CI; the local attempts remain recorded. - The first CI run exposed a missing strict plugin API field. Its five failures were reproduced locally, then passed after repair. The next run passed every underlying job except the source-identity guard. That guard now binds an additive identity with v59 retained. Final-head CI and both fresh reviews passed; addressed review threads are resolved. - The offline capture runs on Darwin ARM64. It does not prove the contents of a prior unretained Linux request or complete Cloud onboarding acceptance. ## Risks - The restriction must survive prepared launches and resumed sessions. Regression tests cover those boundaries. - Existing absent settings retain their prior behavior. This change does not add native search-on support or loosen gateway admission. - Older durable inputs without the optional restriction must remain valid. - Copilot now uses profile v60 for exact launch admission. Historical v59 inputs remain readable. The new declaration adds no live Copilot qualification and does not change its executable or admission flags. - A deployed Cloud acceptance run is still required after this source repair. ## Model Used - OpenAI GPT-6 through Codex, with reasoning, code execution, and delegated review. The exact backend model identifier and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/DEVELOPING.md | 8 ++ .../runner-copilot-capabilities.md | 12 ++- .../codex-local/src/ui/build-config.test.ts | 6 +- .../codex-local/src/ui/build-config.ts | 1 + packages/paperclip-runner/acpx-profiles.json | 4 +- .../src/generated_acpx_profiles.rs | 2 +- .../scripts/copilot-profile-identity.mjs | 4 +- .../src/backends/codex-native-backend.ts | 2 + .../backends/native-backend-factory.test.ts | 23 ++++- .../src/contracts/native-execution.test.ts | 7 ++ .../src/contracts/native-execution.ts | 8 +- .../drivers/acpx/copilot-installation.test.ts | 19 ++++- .../src/drivers/acpx/generated-profiles.ts | 4 +- .../src/drivers/acpx/profile-compatibility.ts | 4 +- .../codex/codex-app-server-driver-impl.ts | 2 +- .../src/drivers/codex/codex-driver-types.ts | 2 + .../drivers/codex/codex-security-config.ts | 2 + .../src/live/runner-provider-launch.test.ts | 49 ++++++++++- .../src/live/runner-provider-launch.ts | 36 ++++++-- .../src/live/runnerd-codex-transport.test.ts | 27 +++++- .../src/live/runnerd-codex-transport.ts | 6 +- .../copilot-profile-v60-identity.json | 85 +++++++++++++++++++ packages/plugins/sdk/src/environment-tasks.ts | 1 + .../sdk/tests/environment-tasks.test.ts | 34 ++++++++ .../services/agent-runner-selection.test.ts | 2 +- server/src/services/agent-runner-selection.ts | 2 +- .../heartbeat-runner-provider-config.test.ts | 8 ++ .../native-execution-input.test.ts | 3 +- .../native-runtime/native-execution-input.ts | 2 + .../native-runtime/native-session-executor.ts | 2 + .../native-runtime/provider-profile.ts | 2 + 31 files changed, 340 insertions(+), 29 deletions(-) create mode 100644 packages/paperclip-runner/test/fixtures/copilot-profile-v60-identity.json diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index f8d408cdff9..cd78289ac2e 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1983,6 +1983,14 @@ runs; immutable run descriptors remain readable. The **Paperclip Runner** experimental setting applies to other native providers. Native Codex, OpenCode, ACPX Claude, and ACPX Pi do not require it. Company access checks still apply. +Native Codex preserves an explicit `adapterConfig.search: false` when converting +or saving an agent. The disabled hosted-search policy is recorded in the native +execution input and launch requirements, so prepared launches and recovery keep +it. An absent setting retains the provider default; this does not add support +for converting a Legacy agent with `search: true`. Authentication hello probes +always disable hosted web search because they only verify the selected account +and model. + OpenCode requires an explicit `provider/model` before testing or finishing setup. An inactive setup draft can be saved before choosing a model. Automatic native selection uses `allow` permissions and a `per_turn` lifecycle. Native `ask` and diff --git a/doc/architecture/runner-copilot-capabilities.md b/doc/architecture/runner-copilot-capabilities.md index 4226846d5af..fcbe1a6f847 100644 --- a/doc/architecture/runner-copilot-capabilities.md +++ b/doc/architecture/runner-copilot-capabilities.md @@ -1,6 +1,16 @@ # Copilot 1.0.88 rich ACP capability audit -Current integration (2026-10-17): **Copilot profile v21 remains pending** with +Current source declaration (2026-10-26): **Copilot profile v60** binds the shared +sources that carry Codex's explicit hosted-search restriction. Its digest is +`sha256:ca3c2b38ed5f6e25cf1c96c1f73781a86cc90c1c0add93f64716c66f28b3f3aa`. +The v59 declaration remains unchanged and readable as history. Exact-current +launch admission still applies. The Copilot executable, arguments, permissions, +receipts, and qualification flags are unchanged. Copilot authentication probes +do not carry the Codex-only setting. Offline compatibility checks do not supply +new live qualification; prior live outcomes remain attached to their original +source identities. + +Historical integration (2026-10-17): **Copilot profile v21 remained pending** with digest `sha256:1dad3d26d91e66c6f911dbfa43e7963f36c0bbf0d7709f4bbb568029a8b4b46a`. It binds the shared provider completion contract, direct driver, production sidecar, and controller continuation resolver. Installation verification requires this generated v21 diff --git a/packages/adapters/codex-local/src/ui/build-config.test.ts b/packages/adapters/codex-local/src/ui/build-config.test.ts index 0557e3c6a57..10c62130ef8 100644 --- a/packages/adapters/codex-local/src/ui/build-config.test.ts +++ b/packages/adapters/codex-local/src/ui/build-config.test.ts @@ -36,6 +36,11 @@ function makeValues(overrides: Partial = {}): CreateConfigVa } describe("buildCodexLocalConfig", () => { + it("preserves explicitly disabled hosted search in native Codex configuration", () => { + expect(buildPaperclipRunnerConfig(makeValues({ search: false }))).toHaveProperty("search", false); + expect(buildPaperclipRunnerConfig(makeValues({ search: true }))).not.toHaveProperty("search"); + }); + describe("qualified Copilot config", () => { it("round-trips Copilot with the exact selected model", () => { const first = buildPaperclipRunnerConfig(makeValues({ model: "gpt-5.6-luna", adapterSchemaValues: { provider: "acpx", acpxAgent: "copilot" } })); @@ -171,7 +176,6 @@ describe("buildPaperclipRunnerConfig", () => { "engine", "agentCommand", "stateDir", - "search", "fastMode", "dangerouslyBypassApprovalsAndSandbox", "command", diff --git a/packages/adapters/codex-local/src/ui/build-config.ts b/packages/adapters/codex-local/src/ui/build-config.ts index b23172938e9..73039f2cc10 100644 --- a/packages/adapters/codex-local/src/ui/build-config.ts +++ b/packages/adapters/codex-local/src/ui/build-config.ts @@ -249,6 +249,7 @@ export function buildPaperclipRunnerConfig(v: CreateConfigValues): Record Option { agent_runtime_package: None, agent_runtime_version: None, command_digest: - "sha256:2f39ffe54a6af832d044b5098c31df9e08d4d27aee0ac675ba4b0c9da7445c40", + "sha256:ca3c2b38ed5f6e25cf1c96c1f73781a86cc90c1c0add93f64716c66f28b3f3aa", requires_provider_policy: true, }, "claude" => AcpxReleaseProfile { diff --git a/packages/paperclip-runner/scripts/copilot-profile-identity.mjs b/packages/paperclip-runner/scripts/copilot-profile-identity.mjs index 7f43d83e8af..d6f02953739 100644 --- a/packages/paperclip-runner/scripts/copilot-profile-identity.mjs +++ b/packages/paperclip-runner/scripts/copilot-profile-identity.mjs @@ -7,9 +7,9 @@ import { fileURLToPath } from "node:url"; import { resolve, dirname } from "node:path"; const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -const fixture = resolve(root, "test/fixtures/copilot-profile-v59-identity.json"); +const fixture = resolve(root, "test/fixtures/copilot-profile-v60-identity.json"); const historical = JSON.parse(readFileSync(resolve(root, "test/fixtures/copilot-profile-v12-identity.json"), "utf8")); -const declaration = { ...historical.declaration, agentProfileVersion: 59 }; +const declaration = { ...historical.declaration, agentProfileVersion: 60 }; delete declaration.permissionIdentitySourceSha256; const sources = { displayEventSourceSha256: "src/drivers/acpx/copilot-events.ts", diff --git a/packages/paperclip-runner/src/backends/codex-native-backend.ts b/packages/paperclip-runner/src/backends/codex-native-backend.ts index 2c223c9d9db..0bb809e49c6 100644 --- a/packages/paperclip-runner/src/backends/codex-native-backend.ts +++ b/packages/paperclip-runner/src/backends/codex-native-backend.ts @@ -154,6 +154,8 @@ function createTransportBackedNativeSessionBackend( return new HarnessDriverBackend( new CodexAppServerDriver({ ...(input.provider.model ? { model: input.provider.model } : {}), + ...(input.provider.kind === "codex" && "webSearch" in input.provider && input.provider.webSearch === "disabled" + ? { disableWebSearch: true } : {}), ...(input.provider.kind === "codex" && "reasoningEffort" in input.provider && input.provider.reasoningEffort ? { reasoningEffort: input.provider.reasoningEffort } : {}), diff --git a/packages/paperclip-runner/src/backends/native-backend-factory.test.ts b/packages/paperclip-runner/src/backends/native-backend-factory.test.ts index 6838f56cdfb..95ad646cfbf 100644 --- a/packages/paperclip-runner/src/backends/native-backend-factory.test.ts +++ b/packages/paperclip-runner/src/backends/native-backend-factory.test.ts @@ -2,7 +2,8 @@ import { QUALIFIED_ACPX_PROFILES, resolveQualifiedAcpxProfile } from "../drivers import { mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import * as codexSecurity from "../drivers/codex/codex-security-config.js"; import { buildNativeModelEnvelope, parseNativeExecutionInput, type NativeExecutionInput } from "../contracts/native-execution.js"; import { NATIVE_RUNTIME_ASSET_SCHEMA, PAPERCLIP_EXECUTION_PROMPT, PAPERCLIP_EXECUTION_PROMPT_REVISION, nativeRuntimePromptDigest, canonicalNativeRuntimeContextDigest, composeNativeSystemInstructions } from "../contracts/runtime-context.js"; @@ -210,6 +211,26 @@ function managedExecution( } describe("native backend factory", () => { + it.each([undefined, "disabled"] as const)("honors saved webSearch=%s with the default process transport", async webSearch => { + const input = preparedExecution(execution()); + if (input.provider.kind !== "codex") throw new Error("Codex fixture required"); + if (webSearch) Object.assign(input.provider, { webSearch }); + const buildArgs = codexSecurity.createIsolatedCodexAppServerArgs; + let generated: string[] | undefined; + const spy = vi.spyOn(codexSecurity, "createIsolatedCodexAppServerArgs").mockImplementation((...args) => { + generated = buildArgs(...args); + throw new Error("offline-stop-before-process-spawn"); + }); + try { + const backend = createNativeSessionBackend(input, { environment: {} }); + await expect(backend.openSession({ identity: { runId: "run", sessionId: "session", companyId: "company", agentId: "agent" }, + workingDirectory: WORKSPACE })).rejects.toThrow("offline-stop-before-process-spawn"); + expect(generated).toBeDefined(); + if (webSearch) expect(generated?.slice(-3)).toEqual(["-c", 'web_search="disabled"', "app-server"]); + else expect(generated).not.toContain('web_search="disabled"'); + } finally { spy.mockRestore(); } + }); + it("constructs the Codex backend without starting its transport", async () => { const backend = createNativeSessionBackend(execution(), { codexTransportFactory: () => { diff --git a/packages/paperclip-runner/src/contracts/native-execution.test.ts b/packages/paperclip-runner/src/contracts/native-execution.test.ts index b161675f19f..9a3edcc531d 100644 --- a/packages/paperclip-runner/src/contracts/native-execution.test.ts +++ b/packages/paperclip-runner/src/contracts/native-execution.test.ts @@ -194,6 +194,13 @@ describe("NativeExecutionInputV1", () => { }); expect(withEffort.provider).toMatchObject({ kind: "codex", reasoningEffort: "ultra" }); expect(parseNativeExecutionInput(withEffort)).toEqual(withEffort); + const disabledSearch = parseNativeExecutionInput({ ...withEffort, provider: { ...withEffort.provider, webSearch: "disabled" } }); + expect(disabledSearch.provider).toHaveProperty("webSearch", "disabled"); + expect(parseNativeExecutionInput(JSON.parse(JSON.stringify(disabledSearch)))).toEqual(disabledSearch); + for (const webSearch of [true, false, "live", "cached", null]) { + expect(() => parseNativeExecutionInput({ ...withEffort, provider: { ...withEffort.provider, webSearch } })).toThrow("webSearch"); + } + expect(() => parseNativeExecutionInput({ ...withEffort, provider: { kind: "codex", model: "gpt-6-astra", approvalPolicy: "on-request", reasoningEffort: "impossible" }, diff --git a/packages/paperclip-runner/src/contracts/native-execution.ts b/packages/paperclip-runner/src/contracts/native-execution.ts index 295e44c89b6..d6d5993e528 100644 --- a/packages/paperclip-runner/src/contracts/native-execution.ts +++ b/packages/paperclip-runner/src/contracts/native-execution.ts @@ -160,7 +160,7 @@ export type NativeProviderConfigV4 = export type NativeProviderConfigV5 = | Exclude - | (Extract & { reasoningEffort?: string }); + | (Extract & { reasoningEffort?: string; webSearch?: "disabled" }); export interface NativeExecutionInputV1 { schema: typeof NATIVE_EXECUTION_INPUT_SCHEMA_V1; @@ -571,7 +571,7 @@ export function parseNativeExecutionInput(value: unknown): NativeExecutionInput : provider.kind === "acpx" ? ["kind", "agent", "model", isV4 ? "permissionMode" : "permissionPolicy", "profile", "piThinkingLevel", ...(isV4 ? ["mode"] : []), ...(isV9 ? ["claudeEffort"] : [])] : provider.kind === "codex" && isV4 - ? ["kind", "model", "approvalPolicy", ...(isV5 ? ["reasoningEffort"] : [])] + ? ["kind", "model", "approvalPolicy", ...(isV5 ? ["reasoningEffort", "webSearch"] : [])] : provider.kind === "opencode" && isV4 ? ["kind", "model", "permissionMode"] : ["kind", "model"], @@ -780,6 +780,9 @@ export function parseNativeExecutionInput(value: unknown): NativeExecutionInput if (isV5 && provider.reasoningEffort !== undefined && !["minimal", "low", "medium", "high", "xhigh", "max", "ultra"].includes(String(provider.reasoningEffort))) { throw new NativeExecutionInputError("input.provider.reasoningEffort is unsupported"); } + if (isV5 && provider.webSearch !== undefined && provider.webSearch !== "disabled") { + throw new NativeExecutionInputError("input.provider.webSearch must be disabled when specified"); + } parsedProvider = { kind: "codex", model: providerModel, @@ -789,6 +792,7 @@ export function parseNativeExecutionInput(value: unknown): NativeExecutionInput ...(isV5 && provider.reasoningEffort !== undefined ? { reasoningEffort: text(provider.reasoningEffort, "input.provider.reasoningEffort") } : {}), + ...(isV5 && provider.webSearch === "disabled" ? { webSearch: "disabled" as const } : {}), }; } if (!Array.isArray(contract.criteria) || contract.criteria.length === 0) { diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-installation.test.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-installation.test.ts index 8bdf9b3950d..8180acde238 100644 --- a/packages/paperclip-runner/src/drivers/acpx/copilot-installation.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-installation.test.ts @@ -87,9 +87,24 @@ vi.mock("./installation-integrity.js", () => ({ verifyNativeAcpxInstallation: vi describe("Copilot build-owned installation", () => { it("admits the current profile with environment launch preparation", () => { - expect(QUALIFIED_ACPX_PROFILES.copilot.agentProfileVersion).toBe(59); + expect(QUALIFIED_ACPX_PROFILES.copilot.agentProfileVersion).toBe(60); expect(QUALIFIED_ACPX_PROFILES.copilot.qualificationStatus).toBeUndefined(); - expect(QUALIFIED_ACPX_PROFILES.copilot.commandDigest).toBe("sha256:2f39ffe54a6af832d044b5098c31df9e08d4d27aee0ac675ba4b0c9da7445c40"); + expect(QUALIFIED_ACPX_PROFILES.copilot.commandDigest).toBe("sha256:ca3c2b38ed5f6e25cf1c96c1f73781a86cc90c1c0add93f64716c66f28b3f3aa"); + }); + + it("preserves v59 while binding the Codex-only hosted search restriction in v60", async () => { + const priorBytes = readFileSync(new URL("../../../test/fixtures/copilot-profile-v59-identity.json", import.meta.url)); + expect(createHash("sha256").update(priorBytes).digest("hex")).toBe("a9116d0981cb0feebec31f61cd23d16b3a263fe6a797b1ba1436c6e41198538c"); + const prior = JSON.parse(priorBytes.toString("utf8")); + const current = JSON.parse(readFileSync(new URL("../../../test/fixtures/copilot-profile-v60-identity.json", import.meta.url), "utf8")); + expect(Object.keys(current.declaration).filter(key => current.declaration[key] !== prior.declaration[key]).sort()).toEqual([ + "agentProfileVersion", "nativeBackendSourceSha256", "nativeExecutionSourceSha256", "profileCompatibilitySourceSha256", "providerLaunchSourceSha256", "runtimeRequestBridgeSourceSha256", + ]); + expect(Object.keys(current.declaration).sort()).toEqual(Object.keys(prior.declaration).sort()); + expect(isSupportedAcpxProfileVersion("copilot", 59)).toBe(true); + expect(current.commandDigest).not.toBe(prior.commandDigest); + await expect(verifyCopilotInstallation({ ...QUALIFIED_ACPX_PROFILES.copilot, + agentProfileVersion: 59, commandDigest: prior.commandDigest })).rejects.toThrow("exact pinned"); }); it("preserves v58 while binding the separate remote process reconnect policy in v59", () => { diff --git a/packages/paperclip-runner/src/drivers/acpx/generated-profiles.ts b/packages/paperclip-runner/src/drivers/acpx/generated-profiles.ts index 36f2bdf29e1..a5d86f192f2 100644 --- a/packages/paperclip-runner/src/drivers/acpx/generated-profiles.ts +++ b/packages/paperclip-runner/src/drivers/acpx/generated-profiles.ts @@ -48,12 +48,12 @@ export const QUALIFIED_ACPX_PROFILE_DATA = { "protocolVersion": 1, "acpxVersion": "0.13.1", "agent": "copilot", - "agentProfileVersion": 59, + "agentProfileVersion": 60, "agentServerPackage": "@github/copilot", "agentServerVersion": "1.0.88", "agentRuntimePackage": null, "agentRuntimeVersion": null, - "commandDigest": "sha256:2f39ffe54a6af832d044b5098c31df9e08d4d27aee0ac675ba4b0c9da7445c40", + "commandDigest": "sha256:ca3c2b38ed5f6e25cf1c96c1f73781a86cc90c1c0add93f64716c66f28b3f3aa", "permissionPolicy": "interactive" }, "claude": { diff --git a/packages/paperclip-runner/src/drivers/acpx/profile-compatibility.ts b/packages/paperclip-runner/src/drivers/acpx/profile-compatibility.ts index 56a5878aa39..82856d06617 100644 --- a/packages/paperclip-runner/src/drivers/acpx/profile-compatibility.ts +++ b/packages/paperclip-runner/src/drivers/acpx/profile-compatibility.ts @@ -2,13 +2,13 @@ import { QUALIFIED_ACPX_PROFILE_DATA } from "./generated-profiles.js"; /** Decodable historical profile revisions, not permission to launch them. * Exact current profile/package/digest admission is checked separately. */ -type HistoricalAcpxProfileVersion = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23 | 24 | 25 | 26 | 27 | 28 | 29 | 30 | 31 | 32 | 33 | 34 | 35 | 36 | 37 | 38 | 39 | 40 | 41 | 42 | 43 | 44 | 45 | 46 | 47 | 48 | 49 | 50 | 51 | 52 | 53 | 54 | 55 | 56 | 57 | 58; +type HistoricalAcpxProfileVersion = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23 | 24 | 25 | 26 | 27 | 28 | 29 | 30 | 31 | 32 | 33 | 34 | 35 | 36 | 37 | 38 | 39 | 40 | 41 | 42 | 43 | 44 | 45 | 46 | 47 | 48 | 49 | 50 | 51 | 52 | 53 | 54 | 55 | 56 | 57 | 58 | 59; type CurrentAcpxProfileVersion = typeof QUALIFIED_ACPX_PROFILE_DATA[keyof typeof QUALIFIED_ACPX_PROFILE_DATA]["agentProfileVersion"]; export type AcpxProfileVersion = HistoricalAcpxProfileVersion | CurrentAcpxProfileVersion; const historicalVersions: Readonly> = { pi: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28], claude: [1, 2, 3, 4, 5], codex: [1, 2, 3, 4, 5], - grok: [1, 2, 3, 4, 5], copilot: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58], + grok: [1, 2, 3, 4, 5], copilot: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59], cursor: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15], }; diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts index 3cb3164b5b7..7103c137c48 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts @@ -710,7 +710,7 @@ export class CodexAppServerDriver implements HarnessDriver { this.#options.transportFactory?.(context) ?? new ProcessCodexAppServerTransport({ workingDirectory, - args: createIsolatedCodexAppServerArgs(this.#options.environment, codexExecutableReadOnlyRoots(this.#options.environment ?? process.env), this.#options.instructionWorkingCopyRoot), + args: createIsolatedCodexAppServerArgs(this.#options.environment, codexExecutableReadOnlyRoots(this.#options.environment ?? process.env), this.#options.instructionWorkingCopyRoot, [], this.#options.disableWebSearch), environment: createSanitizedCodexEnvironment(this.#options.environment), onDiagnostic: message => this.#options.onDiagnostic?.(redactObservabilityDiagnostic(message, this.#options.environment, this.#options.observability?.otelConfig)), processGroup: true, diff --git a/packages/paperclip-runner/src/drivers/codex/codex-driver-types.ts b/packages/paperclip-runner/src/drivers/codex/codex-driver-types.ts index a9256351aa5..a9ba79f2162 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-driver-types.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-driver-types.ts @@ -19,6 +19,8 @@ export interface CodexAppServerDriverOptions { model?: string; /** Per-run reasoning effort sent with each Codex turn, including resumed turns. */ reasoningEffort?: string; + /** Saved hosted-search restriction for the default process transport. */ + disableWebSearch?: boolean; approvalPolicy?: "never" | "on-request" | "untrusted"; /** Paperclip runtime instructions. Codex receives these as additive developer * instructions; the historical option name remains compatible with callers. */ diff --git a/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts b/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts index c5f15d2c5df..78d7d4d2f5d 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts @@ -254,6 +254,7 @@ export function createIsolatedCodexAppServerArgs( /** Server-registered run copy, never an environment/config-supplied root. */ instructionWorkingCopyRoot?: string, protectedPaths: string[] = [], + disableWebSearch = false, ): string[] { const explicitSource = source; source ??= process.env; @@ -356,6 +357,7 @@ export function createIsolatedCodexAppServerArgs( ...(externalRunnerSandbox ? ["--dangerously-bypass-approvals-and-sandbox"] : []), + ...(disableWebSearch ? ["-c", 'web_search="disabled"'] : []), "app-server", ]; } diff --git a/packages/paperclip-runner/src/live/runner-provider-launch.test.ts b/packages/paperclip-runner/src/live/runner-provider-launch.test.ts index 5dd8b66c157..57f237bd57d 100644 --- a/packages/paperclip-runner/src/live/runner-provider-launch.test.ts +++ b/packages/paperclip-runner/src/live/runner-provider-launch.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { createRunnerProviderLaunch, preparedRunnerLaunchWorkingDirectory, runnerLaunchRequirementsDigest, validatePreparedRunnerLaunch, - type RunnerProviderLaunchEnvironment, type RunnerProviderLaunchRequirements } from "./runner-provider-launch.js"; + runnerProviderLaunchRequirements, type RunnerProviderLaunchEnvironment, type RunnerProviderLaunchRequirements } from "./runner-provider-launch.js"; const requirements: RunnerProviderLaunchRequirements = { schema: "paperclip.runner.launch-requirements.v1", @@ -19,6 +19,42 @@ const environment: RunnerProviderLaunchEnvironment = { }; describe("built-in provider launch preparation", () => { + it("binds disabled hosted search to the prepared launch and overrides execution-host arguments", () => { + const disabled = runnerProviderLaunchRequirements({ provider: "codex", disableCodexWebSearch: true }, + requirements.identity, { model: requirements.model }, null, ""); + expect(disabled.settings).toHaveProperty("disableCodexWebSearch", true); + expect(runnerLaunchRequirementsDigest(disabled)).not.toBe(runnerLaunchRequirementsDigest({ ...disabled, settings: {} })); + const launch = createRunnerProviderLaunch(disabled, { ...environment, + options: { codexArgs: ["-c", 'web_search="live"', "app-server"] } }); + expect(launch.args).toEqual(["-c", 'web_search="live"', "-c", 'web_search="disabled"', "app-server"]); + const prepared = { configurationId: "search-policy", requirementsDigest: runnerLaunchRequirementsDigest(disabled), provider: launch }; + expect(() => validatePreparedRunnerLaunch(disabled, prepared)).not.toThrow(); + expect(() => validatePreparedRunnerLaunch(disabled, { ...prepared, + provider: { ...launch, args: ["app-server"] } })).toThrow("runner_prepared_launch_requirements_mismatch"); + for (const leading of ["--", "--search", "--search=true"]) { + expect(() => createRunnerProviderLaunch(disabled, { ...environment, + options: { codexArgs: [leading, "app-server"] } })).toThrow("runner_codex_web_search_policy_override"); + expect(() => validatePreparedRunnerLaunch(disabled, { ...prepared, + provider: { ...launch, args: [leading, ...launch.args as string[]] } })).toThrow("runner_prepared_launch_requirements_mismatch"); + } + expect(() => validatePreparedRunnerLaunch(disabled, { ...prepared, + provider: { ...launch, args: ["--", ...launch.args as string[]] } })).toThrow("runner_prepared_launch_requirements_mismatch"); + for (const tail of [["-c", 'web_search="live"'], ['-cweb_search="live"'], ["--search"], ['--config=web_search="live"'], ["--"]]) { + expect(() => createRunnerProviderLaunch(disabled, { ...environment, + options: { codexArgs: ["app-server", ...tail] } })).toThrow("runner_codex_web_search_policy_override"); + expect(() => validatePreparedRunnerLaunch(disabled, { ...prepared, + provider: { ...launch, args: [...launch.args as string[], ...tail] } })).toThrow("runner_prepared_launch_requirements_mismatch"); + } + + }); + + it.each([undefined, false])("preserves default hosted search when disable is %s", disableCodexWebSearch => { + const request = runnerProviderLaunchRequirements({ provider: "codex", disableCodexWebSearch }, + requirements.identity, { model: requirements.model }, null, ""); + const launch = createRunnerProviderLaunch(request, environment); + expect(launch.args).toEqual(["app-server"]); + }); + it("keeps Core's selection and policy when the execution environment changes", () => { const before = structuredClone(requirements); const local = createRunnerProviderLaunch(requirements, environment); @@ -30,6 +66,17 @@ describe("built-in provider launch preparation", () => { expect(requirements).toEqual(before); }); + it("preserves exact Copilot requirements and launch descriptors when Codex search disabling is absent or false", () => { + const selected = { provider: "acpx" as const, acpxAgent: "copilot" as const, acpxPermissionMode: "deny-all" as const }; + const params = { model: "gpt-5.6-luna", approvalPolicy: "never", permissions: "paperclip-runner-workspace-read-only" }; + const baseline = runnerProviderLaunchRequirements(selected, requirements.identity, params, null, "Copilot policy"); + const scopedProbe = runnerProviderLaunchRequirements({ ...selected, disableCodexWebSearch: false }, + requirements.identity, params, null, "Copilot policy"); + expect(scopedProbe).toEqual(baseline); + expect(runnerLaunchRequirementsDigest(scopedProbe)).toBe(runnerLaunchRequirementsDigest(baseline)); + expect(createRunnerProviderLaunch(scopedProbe, environment)).toEqual(createRunnerProviderLaunch(baseline, environment)); + }); + it("preserves the ACPX profile, run identity, and permission policy", () => { const launch = createRunnerProviderLaunch({ ...requirements, provider: "acpx", model: "claude-selected", settings: { acpxAgent: "claude", acpxPermissionMode: "deny-all" } }, environment); diff --git a/packages/paperclip-runner/src/live/runner-provider-launch.ts b/packages/paperclip-runner/src/live/runner-provider-launch.ts index 6abb9a00db0..5f2ae436d1e 100644 --- a/packages/paperclip-runner/src/live/runner-provider-launch.ts +++ b/packages/paperclip-runner/src/live/runner-provider-launch.ts @@ -23,7 +23,7 @@ export interface RunnerProviderLaunchRequirements { includeCollaborationModeInstructions: boolean; settings: Pick; + "piThinkingLevel" | "claudeEffort" | "managedProfile" | "agentCoreProfile" | "opencodePermissionMode" | "disableCodexWebSearch">; instructions: string; runtimeContext: NativeRuntimeContextSnapshot | null; } @@ -59,6 +59,7 @@ export function runnerProviderLaunchRequirements( piThinkingLevel: options.piThinkingLevel, claudeEffort: options.claudeEffort, managedProfile: options.managedProfile, agentCoreProfile: options.agentCoreProfile, opencodePermissionMode: options.opencodePermissionMode, + ...(options.disableCodexWebSearch === true ? { disableCodexWebSearch: true } : {}), }, approvalPolicy: params.approvalPolicy === "on-request" || params.approvalPolicy === "untrusted" ? params.approvalPolicy : "never", @@ -109,6 +110,14 @@ export function validatePreparedRunnerLaunch( if (descriptor.kind !== requirements.provider || descriptor.model !== requirements.model) reject(); if (requirements.provider === "codex" && (descriptor.approvalPolicy !== requirements.approvalPolicy || Boolean(descriptor.readOnly) !== requirements.readOnly)) reject(); + if (requirements.provider === "codex" && requirements.settings.disableCodexWebSearch === true) { + if (!Array.isArray(descriptor.args) || descriptor.args.some(arg => typeof arg !== "string")) reject(); + const args = descriptor.args as string[]; + const commandIndex = args.indexOf("app-server"); + const boundary = commandIndex < 0 ? args.length : commandIndex; + if (args.some(arg => arg === "--" || arg === "--search" || arg.startsWith("--search=")) || (commandIndex >= 0 && commandIndex !== args.length - 1) + || args[boundary - 2] !== "-c" || args[boundary - 1] !== 'web_search="disabled"') reject(); + } if (requirements.provider === "acpx") { const policy = descriptor.providerPolicy as Record | undefined; if (descriptor.agent !== (requirements.settings.acpxAgent ?? "codex") @@ -303,8 +312,9 @@ export function createRunnerProviderLaunch( args: provider === "opencode" ? [opencodeProxyPath] - : (input.options.codexArgs ?? - createRunnerdCodexAppServerArgs({ + : createRunnerdCodexAppServerArgs({ + args: input.options.codexArgs, + disableWebSearch: settings.disableCodexWebSearch, environment: input.options.environment, codexHome, workingDirectory: input.cwd, @@ -326,7 +336,7 @@ export function createRunnerProviderLaunch( ] : []), ], - })), + }), cwd: input.cwd, model: requirements.model, approvalPolicy: @@ -408,12 +418,14 @@ export function createRunnerdCodexAppServerArgs(input: { readOnlyRoots?: string[]; instructionWorkingCopyRoot?: string; disableProjectInstructions?: boolean; + disableWebSearch?: boolean; + args?: string[]; }): string[] { // The filesystem policy denies HOME and CODEX_HOME to keep credentials and // runner state outside provider reach. Always bind those names to the actual // isolated runner home; a stale controller environment must never cause the // execution workspace itself to become an explicit deny root. - const args = [ + const args = input.args ? [...input.args] : [ ...(input.disableProjectInstructions ? ["-c", "project_doc_max_bytes=0"] : []), ...createIsolatedCodexAppServerArgs( { @@ -426,8 +438,20 @@ export function createRunnerdCodexAppServerArgs(input: { usesPreconfiguredHome(input.environment) && input.runtimeStateRoot ? [input.runtimeStateRoot] : [], ), ]; - if (usesPreconfiguredHome(input.environment) && input.workingDirectory) { + if (!input.args && usesPreconfiguredHome(input.environment) && input.workingDirectory) { args.unshift("-c", `projects={${JSON.stringify(input.workingDirectory)}={trust_level="trusted"}}`); } + if (input.disableWebSearch) { + // Apply Core's restriction after host overrides. Keep the override before + // the Codex subcommand and after any earlier provider-home configuration. + const commandIndex = args.indexOf("app-server"); + // Restricted launches use the owned app-server argument shape. Refuse + // trailing subcommand flags and option terminators rather than guessing + // whether another CLI spelling could override or neutralize this policy. + if (args.some(arg => arg === "--" || arg === "--search" || arg.startsWith("--search=")) || (commandIndex >= 0 && commandIndex !== args.length - 1)) { + throw new Error("runner_codex_web_search_policy_override"); + } + args.splice(commandIndex < 0 ? args.length : commandIndex, 0, "-c", 'web_search="disabled"'); + } return args; } 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 47ac7a92091..d5a33c7853b 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts @@ -3343,6 +3343,29 @@ const fakeCodex = resolve( "../../runner/target/debug/fake-codex-app-server", ); +it("keeps the Copilot authentication launch requirements free of Codex search policy", async () => { + const root = await mkdtemp(join(tmpdir(), "runnerd-copilot-probe-policy-")); + let captured: RunnerProviderLaunchRequirements | undefined; + const launch = vi.fn(() => { throw new Error("provider process must not start"); }); + try { + await expect(probeNativeRunnerEnvironment({ runtimeDirectory: root, provider: "acpx", model: "gpt-5.6-luna", + environment: {}, timeoutMs: 1_000, + transportOptions: { acpxAgent: "copilot", acpxCandidateProfile: "copilot", disableCodexWebSearch: true, + runnerArtifact: { version: "offline-fixture", digest: `sha256:${"a".repeat(64)}` }, + runnerProcessLauncher: launch, + controlPlaneRegistration: async () => { throw new Error("registration must not start"); }, + prepareLaunch: async requirements => { + captured = requirements; + throw new Error("offline-stop-at-Copilot-preparation"); + }, + }, + })).rejects.toThrow("offline-stop-at-Copilot-preparation"); + expect(captured).toBeDefined(); + expect(captured?.settings).not.toHaveProperty("disableCodexWebSearch"); + expect(launch).not.toHaveBeenCalled(); + } finally { await rm(root, { recursive: true, force: true }); } +}); + it.each(["stuck", "late-release", "late-release-error", "stuck-release", "cooperative-release", "launch-failure", "launch-failure-sandbox", "launch-failure-unsandboxed"] as const)( "bounds setup registration teardown and retains unconfirmed state (%s)", async mode => { @@ -3381,7 +3404,7 @@ it.each(["stuck", "late-release", "late-release-error", "stuck-release", "cooper ...(mode === "launch-failure-sandbox" ? { externallySandboxed: true } : mode === "launch-failure-unsandboxed" ? { externallySandboxed: false } : {}), runnerBinary: resolve(import.meta.dirname, "../../package.json"), codexCommand: resolve(import.meta.dirname, "../../package.json"), - sourceCodexHome: "", runnerProcessLauncher: launch, + sourceCodexHome: "", runnerProcessLauncher: launch, disableCodexWebSearch: false, controlPlaneRegistration: async (_authority, _identity, signal) => { template = structuredClone(_authority.store.state.runAttachTemplate!); registrationSignal = signal; @@ -3405,6 +3428,8 @@ it.each(["stuck", "late-release", "late-release-error", "stuck-release", "cooper expect(template).toMatchObject({ provider: { externallySandboxed: mode === "launch-failure-sandbox", readOnly: true, approvalPolicy: "never", collaborationMode: "plan" } }); const args = (template!.provider as Record).args as string[]; expect(args).toContain("project_doc_max_bytes=0"); + expect(args).toContain('web_search="disabled"'); + expect(args.indexOf('web_search="disabled"')).toBeLessThan(args.indexOf("app-server")); expect(args[args.indexOf("project_doc_max_bytes=0") - 1]).toBe("-c"); expect(args.indexOf("project_doc_max_bytes=0")).toBeLessThan(args.indexOf("app-server")); expect(args).not.toContain("--dangerously-bypass-approvals-and-sandbox"); diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index b70954e9aa8..d0d51df3077 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -1287,6 +1287,8 @@ export interface CapabilityRunnerdCodexTransportOptions extends RunnerExecutionD codexCommand?: string; codexArgs?: string[]; disableCodexProjectInstructions?: boolean; + /** Restrict hosted search without changing the provider default for other runs. */ + disableCodexWebSearch?: boolean; /** Controller-visible Codex home used only to seed the isolated runner home. */ sourceCodexHome?: string | null; opencodeCommand?: string; @@ -5529,8 +5531,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { // Keep the durable provider profile and thread identity unchanged. const recordedCommand = record(runAttachTemplate.provider).command; runAttachTemplate.runtimeLaunchArgs = - this.options.codexArgs ?? createRunnerdCodexAppServerArgs({ + args: this.options.codexArgs, + disableWebSearch: this.options.disableCodexWebSearch, environment: this.options.environment, codexHome, workingDirectory: this.options.resumeWorkingDirectory, @@ -7287,6 +7290,7 @@ export async function probeNativeRunnerEnvironment(options: { environment, externallySandboxed: options.transportOptions?.externallySandboxed === true, disableCodexProjectInstructions: true, + disableCodexWebSearch: options.provider === "codex", closeGraceMs: 10_000, lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, }); diff --git a/packages/paperclip-runner/test/fixtures/copilot-profile-v60-identity.json b/packages/paperclip-runner/test/fixtures/copilot-profile-v60-identity.json new file mode 100644 index 00000000000..82b95ed7321 --- /dev/null +++ b/packages/paperclip-runner/test/fixtures/copilot-profile-v60-identity.json @@ -0,0 +1,85 @@ +{ + "commandDigest": "sha256:ca3c2b38ed5f6e25cf1c96c1f73781a86cc90c1c0add93f64716c66f28b3f3aa", + "declaration": { + "schema": "paperclip.acpx_profile_declaration.v1", + "agent": "copilot", + "agentProfileVersion": 60, + "acpxVersion": "0.13.1", + "agentServerVersion": "1.0.88", + "protocolPolicyRevision": "copilot-agent-manual-v1", + "acpxPatchSha256": "00d6af17216cb0cad902b48633eb19bf0ec6b05837a984cb24661fca593f09c1", + "policySha256": "a4be69be43db84052560420a3ca546a665c9165573c24d543eb33d144ba138a3", + "distributionSourceSha256": "c0a65e247fc47b263bd369a3d5ab505c4b323d4e6493be8a9ecbeb0e9dbb9107", + "agentFilesBinding": "registered-runtime-context-v1", + "systemInstructionDelivery": "COPILOT_HOME/copilot-instructions.md:replace-under-lifetime-lease-before-launch:v1", + "sharedRuntimeContract": "paperclip.acpx-runtime-contract.v1", + "innerDistributionSourceSha256": "9bb08347eed01f1ab44e065c8b06ead9be3a25973d6e6329dc4c0a099096279a", + "upstreamAppSha256": "7b48282a19b5b0814a0c96ad5e3a125173d792cc55f9525b2effea962f6063c0", + "patchedAppSha256": "61f8eebe0c30cc03b311c6818b2acb158a6d16ca58c8bfac5ebd42db095c2679", + "messageIdentityContract": "copilot-native-message-id-v1", + "ownedDistributionDelivery": "COPILOT_CLI_DIST_DIR:lease-owned-guarded-inner-distribution:v1", + "permissionContextContract": "copilot-edit-permission-context-v1", + "permissionContextSourceSha256": "de2d38d702b3c574161af0c0c04790fb6f6347916eb3916ce7a131a8c229121b", + "permissionAdapterSourceSha256": "ee5929ed2ee647ad0369465dee9d51a00b2dd28e1ff735b43e969ddc9c164668", + "permissionLocationsSourceSha256": "f39a81da395b6e23aa2a1a45572a28846e5e81bfd3d764d9a456f74a79c752e9", + "permissionRedactionSourceSha256": "ad6770c69269087951790e2e87e22ac088e3d432769adf17ebc865ed512e6c4f", + "permissionClassifierSourceSha256": "1161eabd82e26133e3b3db3ab567f65d52808b03f176e93497e02c8330349fb4", + "semanticToolReceiptContract": "paperclip.semantic_tool_receipt.v2", + "semanticReceiptSourceSha256": "b2795726b2234a663fec1a6ca11f0551ae69882fe9e82aa39686e952d036e791", + "semanticBridgeSourceSha256": "8764b8c9032fdc7d1415b01e75a9fc57c550e7a5e11da4bed7d9100082872ae5", + "toolEvidenceSourceSha256": "6eef5039afc75bb851eaf9cd82a6f099e4bd398ca2deb59dd66f5b2a81a7a706", + "semanticNormalizedInputContract": "validated-forwarded-input-commit-v1", + "semanticSidecarSourceSha256": "17361d1782c0dcebb2b7972635f45c890f2a4081f8bc0ef2d2276550e0b91629", + "semanticDirectDriverSourceSha256": "ff572638f2cb5d9f8249ff8e2f412311c1e3af209ef0bf7e5ea9f10a21d91103", + "semanticValidationSourceSha256": "02fbb92681c78c5de5d2f1aae25aabc25a0e5df6b0ff4baccac652192a8da975", + "semanticNormalizationSourceSha256": "8b628b82e8eacc91cdcc6fdca2210ba4cf282f01e9c6f74e95ff271655b5047e", + "semanticCompletionContractSourceSha256": "3343df6362252d44d12b079ff78c8f6539d1544f3ce5ec36ff36db70ab0fac31", + "semanticValidatorsSourceSha256": "f22d9caf2035b600abc1a6625ae23ca97f1409d7e29ed5b7264bb8f9784c7dc9", + "semanticSchemaBundleSourceSha256": "016391a087cc217aafed4ba1f4efafc6e4881a7525952dce824b7f4d24530603", + "semanticSidecarProtocolSourceSha256": "03c0c95b900f9dc18f29f6be27b8c79bc0bcab29780190d46a74e95328081236", + "semanticSidecarReceiptCommitContract": "correlated-tool-resolve-v1", + "displayEventSourceSha256": "d72f9a6035c247bc6a42c067c05753076c54322c4b58984e93922f3325dc6877", + "displayProjectionSourceSha256": "75952606b26dd93bbc582c1f0a4424487377272ea745fb3a27ceae031fd5dc7e", + "configuredEnvironmentSourceSha256": "5c9008d121979a3a8987e6b8a5752172eafdc4a2b5d725a73f3bbb8fb83be8ff", + "configuredEnvironmentRustSourceSha256": "85c87fae62b8a9377e4f0a4c00302c7519fae4397b1c48253f8f2b7ba7faf994", + "profileActivitySourceSha256": "4fdb6b0f9125fd8372ba6f27945fa5c8a3d3b2ec891df544092e3dc7a75d5282", + "profileExtensionsSourceSha256": "a80ddc95eb4505095bfdbbf85c758199949ab20be9c84b34c77f762eb7180571", + "profileInstallationSourceSha256": "3e61c4cdd4b78e38fcb16eaadae4f1fd65ce3c7127184ea7a80948c8e5d77f88", + "attachmentPolicySourceSha256": "4f7638b7cc64e18f6a4ba33fd46de2b3d49225e32aeab41f5c695b268fa4f8e2", + "singleReadEvidenceSourceSha256": "4069393405089bde613cecec04f5123c426594811da4c668bd96f0e7e283550f", + "nativeSessionRuntimeSourceSha256": "1fe6fa9e6b96d84d70432b85a57c473f833606d4a521563d5022b4d3c41576ce", + "nativeRuntimeContextSourceSha256": "71607f80dbb8dc5cc6c123d0b8f3995d6c5407277695a8c82cbedc90fb8f4842", + "nativeRuntimePromptsSourceSha256": "103b83066da72434602e9e461e6278dfb40bb993d2f3b49b2e93df81bbb08b72", + "nativeSessionContractSourceSha256": "22a8c96ce41ba4f03e976fc37f0cc7c5d05cee99fa789941da31ee1dab440b90", + "harnessSessionContractSourceSha256": "03f4cd7f76f34f6d82e009b66aa51f18f6e23dc235b660778060e35b6ea07412", + "harnessBackendSourceSha256": "dc827e281ae6731545b166ee4d709a4f67f594a556dcbe63c70fe8f1aca26249", + "harnessSessionSourceSha256": "81361733e86b56dd5a66415aefc6f72e69be24acc8d3417370154ebb7fef6e94", + "transportContractSourceSha256": "03fca93d36a89b0cc0917335e293ad097fdf04356d21b307827bb0ea4c13f523", + "nativeBackendSourceSha256": "e7b12d4418c7a71f5f2474b046697d3009eae3613b284bec381df5e9965a1c37", + "nativeBackendFactorySourceSha256": "7e431c926b76d53c4c08e3a4105353dcd34bd940f6588a1a68f79631f6feeb44", + "nativeInstructionsSourceSha256": "9173ebccf09cb56a781d2bf5c7eb27fdafb13d3450d99342c6aff86b42d6b43a", + "providerLaunchSourceSha256": "fa8db1c42c378898287c141d8cea9069166981e369c9b9c51422cb6177de5a2c", + "launchPreparationStoreSourceSha256": "ff6025568bbc0aa6ce545772d73dfca2d3da226740bd3a7b386b069e76cff296", + "runtimeRequestBridgeSourceSha256": "8ddd5d26854f4a153a9559ea8105aef95a3ec94f03209e9a797c95932c162e2a", + "runtimeRequestFacadeSourceSha256": "d0ed28e0c5ddad34db8f437e704c8a755eb0b47f27e08e44185f66d1d2205295", + "runtimeRequestNotificationSourceSha256": "6196c6175f11c69295dc00fc773201f4fdeca26c500653870253c43b81e65211", + "guardedInstallationSourceSha256": "56a177d3e702914818dca4ca50e061f0d25f988bfc4c0582c36ce23c1696d82f", + "nativeExecutionSourceSha256": "3a30d52bffad68c7f02349193be84761050eb99039fc0556c2b449891a36e918", + "profileCompatibilitySourceSha256": "8503e7f2f8175e413b42e85a9ff259a00f2278cd966bbeafc9e9e85417cf7dbb", + "installationSourceSha256": "a1c7c6f103b898216973e77ffbac0ef7572e250aa574d1a4065270e9dfa0dc2d", + "assetOwnershipSourceSha256": "1785e3f31e5dad5a13b53e25d8cf9482a9bd89a27b6c127830aed6b81acc605c", + "assetInstallerSourceSha256": "d3bd9514e5920d0cc3567aa50487fb534c57a6f66b8da0d3f22c5f6e499c841f", + "credentialBindingSourceSha256": "7586d44b47c83395498dc8806287cd3538ef58177ba7c5949f5766ed105a0161", + "sidecarErrorSourceSha256": "b09af4577f8bf42eee7036b2ad9842202cfc763e72b53c3287f82c6090cdad80", + "profileSourceSha256": "f5538b3dbefa7634a332b87d5ededcefb56aa7e5790cede1c7a0d948a548a456", + "metadataProbeSourceSha256": "be4c8416edd27061109bc1f33d0c26071b4facbb3f651beaae6b3c0275cce727", + "permissionToolIdentitySourceSha256": "770b47305e571a81f344883057d0e4a482a1cd59ac77d045836593a8e739deb1", + "semanticContinuationSourceSha256": "d248e47a0fe7f2536d1e64cea0b0b410f34104800cd643d995b4473104ecc8e0", + "runtimeHostSourceSha256": "94a73749267f5a80b5df19413a65b4b9263c00a9bf7e98fda00f00b8f9fd32ec", + "runtimeAdapterSourceSha256": "e4c764c2c3c7dd655a965b39472bd1ad8d577d1e7258b25594b171c9429752b8", + "runtimeSandboxSourceSha256": "92dfa53f5a3ed0062913ae417eeccec243b430d444495efde2a8e9c4c02e78ea", + "cancellationSourceSha256": "a07d1a9e982dec0ae130c1ad1efa5bc75c6a39e013b730ba3ee45256f09ceeca", + "agentFilesSourceSha256": "fb32f6badc13dfa22f231479509b631b2f0eba0c99b1ab9f3cd5c0c601aa03ba", + "nativeDistributionSourceSha256": "aed327c389e957483a503cb1bb8b247ee7cd7ff4b528fb4567c39bcb4577690b" + } +} diff --git a/packages/plugins/sdk/src/environment-tasks.ts b/packages/plugins/sdk/src/environment-tasks.ts index 57f0dea51fa..f4f86e26b53 100644 --- a/packages/plugins/sdk/src/environment-tasks.ts +++ b/packages/plugins/sdk/src/environment-tasks.ts @@ -39,6 +39,7 @@ export const environmentRunnerLaunchRequirementsSchema = z.object({ acpxPermissionModePinned: z.boolean().optional(), piThinkingLevel: z.enum(["off", "low", "high", "max"]).optional(), claudeEffort: z.enum(["low", "medium", "high", "xhigh", "max"]).optional(), opencodePermissionMode: z.enum(["allow", "ask", "deny"]).optional(), + disableCodexWebSearch: z.literal(true).optional(), managedProfile: z.record(z.string(), z.unknown()).optional(), agentCoreProfile: z.record(z.string(), z.unknown()).optional(), }).strict(), approvalPolicy: z.enum(["never", "on-request", "untrusted"]), readOnly: z.boolean(), diff --git a/packages/plugins/sdk/tests/environment-tasks.test.ts b/packages/plugins/sdk/tests/environment-tasks.test.ts index 9db7cbfe2d5..6f9d7aa388b 100644 --- a/packages/plugins/sdk/tests/environment-tasks.test.ts +++ b/packages/plugins/sdk/tests/environment-tasks.test.ts @@ -11,3 +11,37 @@ describe("task lifecycle policy",()=>{ expect(environmentTaskOperationSchema.safeParse({...submit,launch:{...submit.launch,lifecyclePolicy:{mode:"warm",idleTimeoutMs}}}).success).toBe(false); }); }); + + +const prepare = { + kind: "prepare", projectIds: [], requirements: { + schema: "paperclip.runner.launch-requirements.v1", + identity: { runnerInstanceId: "runner", environmentLeaseId: "lease", runId: "run", + normalizedSessionId: "session", turnId: "turn", itemId: "item" }, + provider: "codex", model: "selected-model", instructions: "Verify the selected account.", + runtimeContext: null, settings: {}, approvalPolicy: "never", readOnly: true, + preparedConversation: false, includeCollaborationModeInstructions: false, + }, +}; + +describe("task hosted search restriction", () => { + it("preserves Core's explicit restriction through plugin launch preparation", () => { + const operation = { ...prepare, requirements: { ...prepare.requirements, + settings: { disableCodexWebSearch: true } } }; + expect(environmentTaskOperationSchema.parse(operation)).toEqual(operation); + }); + + it("keeps the provider default when the restriction is absent", () => { + expect(environmentTaskOperationSchema.parse(prepare)).toEqual(prepare); + }); + + it.each([false, null, 0, "true", "disabled"])("rejects a non-restricting flag value %s", disableCodexWebSearch => { + expect(environmentTaskOperationSchema.safeParse({ ...prepare, requirements: { ...prepare.requirements, + settings: { disableCodexWebSearch } } }).success).toBe(false); + }); + + it("continues rejecting unreviewed launch settings", () => { + expect(environmentTaskOperationSchema.safeParse({ ...prepare, requirements: { ...prepare.requirements, + settings: { disableCodexWebSearch: true, hostedSearchBillingMode: "unmetered" } } }).success).toBe(false); + }); +}); diff --git a/server/src/services/agent-runner-selection.test.ts b/server/src/services/agent-runner-selection.test.ts index afec21d6b45..afc3949acdf 100644 --- a/server/src/services/agent-runner-selection.test.ts +++ b/server/src/services/agent-runner-selection.test.ts @@ -74,7 +74,7 @@ describe("server-owned Codex runner selection", () => { const input = { adapterType: "codex_local", target, adapterConfig: { model: "gpt-5.6-sol", modelReasoningEffort: "high", env: { OPENAI_API_KEY: { type: "secret_ref", secretId: "secret" } }, cwd: "/workspace", instructionsFilePath: "/workspace/AGENTS.md", workspaceStrategy: { type: "git_worktree" }, timeoutSec: 60, search: false, fastMode: false, dangerouslyBypassApprovalsAndSandbox: true } }; const resolved = resolveNewAgentRunner(input); expect(resolved).toMatchObject({ adapterType: "paperclip_runner", adapterConfig: { provider: "codex", codexPermissionMode: "never", modelReasoningEffort: "high", env: input.adapterConfig.env, workspaceStrategy: input.adapterConfig.workspaceStrategy, timeoutSec: 60 } }); - expect(resolved.adapterConfig).not.toHaveProperty("search"); + expect(resolved.adapterConfig).toHaveProperty("search", false); expect(resolveNewAgentRunner(resolved)).toEqual(resolved); expect(input.adapterConfig).toHaveProperty("search", false); }); diff --git a/server/src/services/agent-runner-selection.ts b/server/src/services/agent-runner-selection.ts index 7d87a5b9d89..904e3c7aae3 100644 --- a/server/src/services/agent-runner-selection.ts +++ b/server/src/services/agent-runner-selection.ts @@ -270,7 +270,7 @@ function resolveAgentRunner(input: AgentRunnerSelection): { adapterType: string; incompatible(["env.PAPERCLIP_CODEX_ACP_NETWORK_ACCESS"]); } const effort = config.modelReasoningEffort ?? config.reasoningEffort ?? config.effort; - for (const key of ["engine", "command", "extraArgs", "args", "agentCommand", "mode", "nonInteractivePermissions", "stateDir", "warmHandleIdleMs", "dangerouslyBypassApprovalsAndSandbox", "dangerouslyBypassSandbox", "search", "fastMode", "reasoningEffort", "effort"]) delete config[key]; + for (const key of ["engine", "command", "extraArgs", "args", "agentCommand", "mode", "nonInteractivePermissions", "stateDir", "warmHandleIdleMs", "dangerouslyBypassApprovalsAndSandbox", "dangerouslyBypassSandbox", "fastMode", "reasoningEffort", "effort"]) delete config[key]; if (config.model !== undefined && typeof config.model !== "string") incompatible(["model"]); config = { ...config, provider: "codex", model: typeof config.model === "string" && config.model.trim() ? config.model.trim() : PAPERCLIP_RUNNER_DEFAULT_MODELS.codex, codexPermissionMode: config.codexPermissionMode ?? "never", lifecycleMode: config.lifecycleMode ?? "per_turn", ...(effort === undefined ? {} : { modelReasoningEffort: effort }) }; try { resolvePaperclipRunnerNativeProviderInput({ backend: "codex_app_server", adapterConfig: config }); } diff --git a/server/src/services/heartbeat-runner-provider-config.test.ts b/server/src/services/heartbeat-runner-provider-config.test.ts index 5e874641c19..8090eb3cc04 100644 --- a/server/src/services/heartbeat-runner-provider-config.test.ts +++ b/server/src/services/heartbeat-runner-provider-config.test.ts @@ -7,6 +7,14 @@ import { resolvePaperclipRunnerNativeProviderInput, } from "./native-runtime/provider-profile.js"; +describe("native Codex hosted search policy", () => { + it.each([undefined, true, false])("preserves the saved search=%s choice", search => { + const input = resolvePaperclipRunnerNativeProviderInput({ backend: "codex_app_server", adapterConfig: { provider: "codex", model: "gpt-5.4-mini", ...(search === undefined ? {} : { search }) } }); + if (search === false) expect(input).toHaveProperty("codexWebSearch", "disabled"); + else expect(input).not.toHaveProperty("codexWebSearch"); + }); +}); + describe("Paperclip Runner native provider configuration", () => { it.each([ ["gpt-6.1-sol", "0.158.0", "gpt-6-sol"], 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 f0cdc8aca17..f10000ce8c7 100644 --- a/server/src/services/native-runtime/native-execution-input.test.ts +++ b/server/src/services/native-runtime/native-execution-input.test.ts @@ -21,12 +21,13 @@ describe("LCA-05 explicit native work mode", () => { normalizedSessionId: null, provider: "codex", codexReasoningEffort: "high", + codexWebSearch: "disabled", planningContext: workMode === "planning" ? { documentId: null, baseRevisionId: null, baseRevisionNumber: 0, markdown: "", sha256: "a".repeat(64), reviewContext: {} } : null, completionContract: { id: "70000000-0000-4000-8000-000000000007", sha256: `sha256:${"a".repeat(64)}`, schemaVersion: "paperclip.run-result.v1", contract: { revision: "1", objective: "Deliver the requested work", criteria: [{ id: "output", requirement: "Deliver the requested work" }] } }, runtimeContext: nativeRuntimeContextFixture(), }); expect(input.task.workMode).toBe(workMode); - expect(input.provider).toMatchObject({ kind: "codex", reasoningEffort: "high" }); + expect(input.provider).toMatchObject({ kind: "codex", reasoningEffort: "high", webSearch: "disabled" }); expect(input.executionMode).toBe(workMode === "planning" ? "plan" : "default"); expect(input.task.title).toBe(text); } diff --git a/server/src/services/native-runtime/native-execution-input.ts b/server/src/services/native-runtime/native-execution-input.ts index 313e2f7405a..974ea876de6 100644 --- a/server/src/services/native-runtime/native-execution-input.ts +++ b/server/src/services/native-runtime/native-execution-input.ts @@ -102,6 +102,7 @@ export interface BuildNativeExecutionInput { homeMode?: "isolated" | "preconfigured"; codexApprovalPolicy?: NativeCodexApprovalPolicy; codexReasoningEffort?: string; + codexWebSearch?: "disabled"; opencodePermissionMode?: NativeOpenCodePermissionMode; acpxPermissionMode?: NativeAcpxPermissionMode; acpxSessionMode?: "agent" | "plan" | "ask"; @@ -364,6 +365,7 @@ export function buildNativeExecutionInput(input: BuildNativeExecutionInput): Nat model: input.model ?? null, approvalPolicy: input.codexApprovalPolicy ?? "never", ...(input.codexReasoningEffort ? { reasoningEffort: input.codexReasoningEffort } : {}), + ...(input.codexWebSearch ? { webSearch: input.codexWebSearch } : {}), }, completionContract: { id: input.completionContract.id, diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 234f673e664..597c36542d5 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -12012,6 +12012,7 @@ async function createRunnerdBackendWithinSessionClaim( }, codexTransportFactory: recovery => createRunnerdCodexTransport({ provider: provider.kind, stateDirectory: root, ...owner.driver, runnerArtifact: description.runner, + ...(provider.kind === "codex" ? { disableCodexWebSearch: "webSearch" in provider && provider.webSearch === "disabled" } : {}), ...(provider.kind === "acpx" ? { acpxAgent: provider.agent, acpxCandidateProfile: provider.agent === "pi" ? undefined : resolveAcpxQualification(provider, process.env), acpxPermissionMode: provider.permissionMode, acpxMode: provider.mode, piThinkingLevel: provider.piThinkingLevel, @@ -13396,6 +13397,7 @@ async function createRunnerdBackendWithinSessionClaim( codexTransportFactory: (recoveryContext) => createRunnerdCodexTransport({ onSpawn: input.onSpawn, + ...(input.execution.provider.kind === "codex" ? { disableCodexWebSearch: "webSearch" in input.execution.provider && input.execution.provider.webSearch === "disabled" } : {}), homeMode: preconfiguredHome ? "preconfigured" : "isolated", onCredentialsUpdated: (event) => appendNativeCredentialUpdate(input.db, { companyId: input.execution.binding.companyId, diff --git a/server/src/services/native-runtime/provider-profile.ts b/server/src/services/native-runtime/provider-profile.ts index 22f906c59c3..d217c3a05ca 100644 --- a/server/src/services/native-runtime/provider-profile.ts +++ b/server/src/services/native-runtime/provider-profile.ts @@ -84,6 +84,7 @@ export type PaperclipRunnerNativeProviderInput = homeMode?: "isolated" | "preconfigured"; codexApprovalPolicy: "never" | "on-request" | "untrusted"; codexReasoningEffort?: string; + codexWebSearch?: "disabled"; } | { provider: "opencode"; @@ -843,5 +844,6 @@ export function resolvePaperclipRunnerNativeProviderInput(input: { config.codexPermissionMode, ) as "never" | "on-request" | "untrusted", ...(typeof effort === "string" && effort ? { codexReasoningEffort: effort } : {}), + ...(config.search === false ? { codexWebSearch: "disabled" as const } : {}), }; } From df5a78cf91b181ec34f5af234312abf420af600e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= <2178406+clempat@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:44:39 +0100 Subject: [PATCH 06/11] fix(ui): keep iPhone task chat stable while typing and streaming (#17232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip keeps human-agent conversations attached to tasks. > - People need to read and reply comfortably from a phone. > - The mobile task page scrolled the document and positioned its composer with a sticky navigation offset. > - Safari's keyboard resize and later focus pan could move that document again, causing overlap, gaps, and jumps while editing. > - This change gives mobile and desktop the same message scroller and a normal flex footer. > - CSS owns the layout. A small VisualViewport adapter handles the visible height and Safari's stale document focus scroll. > - Streaming no longer measures every historical row while following the bottom. ## Linked Issues or Issue Description Fixes https://github.com/paperclipai/paperclip/issues/16497 Related: https://github.com/paperclipai/paperclip/pull/15245 also addresses keyboard positioning. This change additionally removes mobile document auto-follow and reduces historical-row measurement work. ## What Changed ### One scroll owner and a natural-flow composer Before: ```text Document scroll + mobile auto-follow └─ Conversation + sticky composer + animated navigation offset ``` After: ```text Height-constrained conversation shell ├─ Header ├─ Message scroller — owns follow/reading position └─ Composer — normal flex footer Navigation stays at the layout viewport bottom, behind the keyboard ``` - Reuse TaskMessageScroller on mobile. Delete the separate 136-line useWindowAutoFollow implementation and its superseded tests. - Keep the composer in normal flex flow. Remove its sticky bottom positioning and navigation-offset animation. - Constrain the mobile conversation with CSS. Classic tasks and other pages retain their existing layout. - Add a 47-line useMobileVisualViewport hook. It changes shell height on resize, not on typing or incoming messages. It never translates the shell or hides navigation. - Reject Safari's stale document focus pan only for an unzoomed, keyboard-open conversation. Inner message and editor scrolling stay native. - Keep a saved reading anchor when reading older messages. Skip historical-row measurements while following the bottom. - Update tests and the existing mobile streaming fixture. Keep only two layout-contract sentences in DESIGN.md. **Scope:** 19 files, +367 / -482 lines: 115 fewer lines overall. Production UI/CSS alone is +158 / -190 lines. No API, database, or dependency changes. ## Verification Bottom-follow is also saved explicitly for Back navigation. A new regression covers returning after rotation changes message wrapping, without restoring historical-row scans. ### Physical iPhone acceptance Tested on an iPhone 15 Pro running iOS 26.7.1. The reporter confirmed: - Opening the keyboard positions the composer correctly immediately, before any typing. - The installed web app works. - Long drafts, insertion, deletion, and sending are acceptable. The reporter described long drafts as not perfect but good enough. - Reading older messages during incoming output preserves position; returning to the bottom resumes following. - Rotation, keyboard close/reopen, and attachment/picker transitions work. Remote inspection corroborated the corrected open-keyboard state: visible height 358px, composer bottom 358px, document scrollY 0. Navigation remained display:block at y=631px, behind the keyboard. Earlier candidates failed device testing. The decisive trace showed a correct viewport resize followed about one frame later by a stale 337px native document focus pan. Typing caused Safari to correct it. Tests now cover that event ordering, the reverse ordering, and zoom panning. ### Performance evidence The existing TaskChatPerfHarness supplied 200 historical responses, 4,000 collapsed tools, and 30 updates at 100ms intervals. | Measurement | Before | After | | --- | ---: | ---: | | Historical row geometry reads | 6,000 | 0 | | Renderer main-thread work | 331.2ms | 273.5ms | | Largest task | 27.1ms | 12.3ms | This is one instrumented Chromium development run on the Mac, not a phone benchmark or a statistical speedup claim. The zero-row-measurement result also has a regression test that failed with 6,000 reads before the change. To reproduce, serve ui/tests/task-chat-perf.html with the UI Vite server. Record 30 streaming updates at the bottom. Inspect TaskMessageScroller and row getBoundingClientRect calls. Existing historical-render tests also confirm that unchanged Markdown does not reparse on live-tail updates. ### Automated and full-app checks - [Current-head CI](https://github.com/paperclipai/paperclip/actions/runs/39862524031) is green: 54 successful checks and 2 skipped checks. Greptile rates the current head 5/5, confirms the Back-navigation fix, and has no unresolved findings. - **601 focused tests pass** across the task page, layout, composer, scroller, viewport adapter, and historical-render coverage. - UI typecheck and production build pass. - Token gates, Storybook catalog checks, and catalog/map structural tests pass. - Independent review of the final source tree passed with no findings. - The full source app ran on macOS with an isolated database and saved conversation history. A reply sent through the real UI persisted after reload. Provider execution was not part of this layout test. - **Full local repository checks did not pass.** Build/typecheck stop at missing Rust cargo. The full test run reported 142 failed server tests and 3 failed suites; missing cargo is confirmed in those three suites. The other server failures were not diagnosed in this UI task. Current-head CI now supplies the repository-wide evidence; these local failures remain disclosed. ## Risks - Mobile conversations now share the desktop scroll owner. Navigation, loading, history restoration, and reading-position tests cover that change. - The keyboard-state threshold is a 120px viewport reduction. Accessory-only keyboards retain the normal navigation spacing. - The document-scroll fallback is scoped to rendered conversations with an open keyboard at scale 1. It does not handle inner scrolling or prevent zoom panning. - Long-draft behavior is accepted, with minor polish still possible. - The UI change is reversible and requires no data migration. ## Model Used OpenAI GPT-6 Astra, exact model ID gpt-6-astra, through OpenCode. Capabilities used: reasoning, code editing, tests, Chromium performance tracing, and Safari remote inspection. The harness does not expose the context-window size. An independent general-purpose subagent reviewed the final tree; its serving model ID was not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with Fixes / Closes / Refs OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run focused tests locally and they pass; repository-wide failures are disclosed above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- DESIGN.md | 8 +- ui/src/components/Layout.production.tsx | 13 +- ui/src/components/Layout.tsx | 12 +- ui/src/components/TaskChatThread.test.tsx | 22 +- ui/src/components/TaskChatThread.tsx | 19 +- .../task-chat/TaskChatComposer.test.tsx | 2 +- .../components/task-chat/TaskChatComposer.tsx | 2 +- .../task-chat/TaskChatComposerDock.tsx | 11 +- .../task-chat/TaskChatThreadView.tsx | 2 +- .../task-chat/TaskMessageScroller.test.tsx | 66 +++++ .../task-chat/TaskMessageScroller.tsx | 21 +- .../task-chat/scroll-navigation.tsx | 9 +- .../task-chat/useWindowAutoFollow.test.tsx | 263 ------------------ .../task-chat/useWindowAutoFollow.ts | 136 --------- ui/src/hooks/useMobileVisualViewport.test.tsx | 116 ++++++++ ui/src/hooks/useMobileVisualViewport.ts | 47 ++++ ui/src/index.css | 62 ++++- ui/src/pages/IssueDetail.tsx | 14 +- .../tasks/task-chat-mobile-follow.stories.tsx | 24 +- 19 files changed, 367 insertions(+), 482 deletions(-) delete mode 100644 ui/src/components/task-chat/useWindowAutoFollow.test.tsx delete mode 100644 ui/src/components/task-chat/useWindowAutoFollow.ts create mode 100644 ui/src/hooks/useMobileVisualViewport.test.tsx create mode 100644 ui/src/hooks/useMobileVisualViewport.ts diff --git a/DESIGN.md b/DESIGN.md index a4804cb0e27..bda54f4da2b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -60,8 +60,12 @@ title sizes remain larger. The bottom navigation responds to accumulated scrolling, ignoring small reversals and Safari's edge bounce. It glides out and eases back in with shared -motion tokens; the task composer follows the same motion. Keep page padding -stable while the navigation moves, and honor reduced-motion preferences. +motion tokens. Keep page padding stable while the navigation moves, and honor +reduced-motion preferences. + +Mobile task conversations size their shell to the visible viewport, with one +message scroller and a normal flex footer for the composer. Navigation stays at +the layout viewport bottom, behind the keyboard. Cold task opens reveal the instruction and recent conversation independently of plans, related metadata, and historical run activity. Show the latest human diff --git a/ui/src/components/Layout.production.tsx b/ui/src/components/Layout.production.tsx index 56afb99fc06..cdb984d85d7 100644 --- a/ui/src/components/Layout.production.tsx +++ b/ui/src/components/Layout.production.tsx @@ -45,6 +45,7 @@ import { useSidebar } from "../context/SidebarContext"; import { useKeyboardShortcuts } from "../hooks/useKeyboardShortcuts"; import { useCompanyPageMemory } from "../hooks/useCompanyPageMemory"; import { useMobileNavVisibility } from "../hooks/useMobileNavVisibility"; +import { useMobileVisualViewport } from "../hooks/useMobileVisualViewport"; import { healthApi } from "../api/health"; import { resolveArchivedCompanyBounce, @@ -178,6 +179,7 @@ export function Layout() { const scrollMemory = useRef(new NavigationScrollMemory()); const activeScrollKey = useRef(location.key); const mobileNavVisible = useMobileNavVisibility(isMobile, location.pathname); + const mobileViewportRef = useMobileVisualViewport(isMobile); const [shortcutsOpen, setShortcutsOpen] = useState(false); const matchedCompany = useMemo(() => { if (!companyPrefix) return null; @@ -588,8 +590,10 @@ export function Layout() { return (

@@ -663,12 +667,13 @@ export function Layout() {
) : null}
-
+
(location.key); const mobileNavVisible = useMobileNavVisibility(isMobile, location.pathname); + const mobileViewportRef = useMobileVisualViewport(isMobile); const [shortcutsOpen, setShortcutsOpen] = useState(false); const matchedCompany = useMemo(() => { if (!companyPrefix) return null; @@ -574,8 +576,10 @@ export function Layout({ sidebarSections }: { sidebarSections?: ReactNode }) { return (
-
+
{isMobile && sidebarOpen && ( + : null + ); const renderBrief = useCallback( - () => issueBrief ? <>{props.historyControl} : null, - [issueBrief, props.historyControl], + () => issueBrief ? <>{historyControl} : null, + [issueBrief, historyControl], ); const assignedAgentForNotice = useMemo(() => { @@ -3127,7 +3136,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { {threadHeaderWithBlockers}{!issueBrief ? props.historyControl : null}} + header={<>{threadHeaderWithBlockers}{!issueBrief ? historyControl : null}} renderInteraction={renderInteraction} renderBrief={renderBrief} renderMessageActions={renderMessageActions} diff --git a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx index df3419993d1..0636b1f6267 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx +++ b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx @@ -131,17 +131,18 @@ describe("useLiveRunTranscripts", () => { }); it("expands a compact log tail without hiding or duplicating the latest output", async () => { + let truncatedRunIds: ReadonlySet = new Set(); const row = (seq: number, chunk: string) => JSON.stringify({ seq, chunk, ts: `2026-10-24T00:00:0${seq}Z`, stream: "stdout" }); const result = (content: string) => ({ runId: "run-1", store: "memory", logRef: "log-1", content, nextOffset: 100_000 }); let resolveExpanded!: (value: ReturnType) => void; logMock.mockResolvedValueOnce(result(`${row(2, "Latest answer")}\n`)); logMock.mockImplementationOnce(() => new Promise(resolve => { resolveExpanded = resolve; })); function Harness({ limit, bytes = 100_000 }: { limit: number; bytes?: number }) { - useLiveRunTranscripts({ + ({ truncatedRunIds } = useLiveRunTranscripts({ runs: [{ id: "run-1", status: "succeeded", adapterType: "codex_local", logBytes: bytes }], enableRealtimeUpdates: false, logReadLimitBytes: limit, - }); + })); return null; } const root = createRoot(document.createElement("div")); @@ -149,11 +150,13 @@ describe("useLiveRunTranscripts", () => { try { await act(async () => root.render()); expect(logMock).toHaveBeenLastCalledWith("run-1", 68_000, 32_000, expect.objectContaining({ alignStartToRecord: true })); + expect(truncatedRunIds.has("run-1")).toBe(true); expect(chunks()).toEqual([expect.objectContaining({ chunk: "Latest answer" })]); await act(async () => root.render()); expect(logMock).toHaveBeenLastCalledWith("run-1", 0, 256_000, expect.anything()); expect(chunks()).toEqual([expect.objectContaining({ chunk: "Latest answer" })]); await act(async () => resolveExpanded(result(`${row(1, "Earlier activity")}\n${row(2, "Latest answer")}\n`))); + expect(truncatedRunIds.has("run-1")).toBe(false); expect(chunks()).toEqual([ expect.objectContaining({ chunk: "Earlier activity" }), expect.objectContaining({ chunk: "Latest answer" }), @@ -165,6 +168,30 @@ describe("useLiveRunTranscripts", () => { } }); + it("does not report omitted output when a run grows beyond the window after reading from the start", async () => { + let truncatedRunIds: ReadonlySet = new Set(); + const row = (seq: number) => `${JSON.stringify({ seq, chunk: `Output ${seq}`, stream: "stdout", ts: "2026-10-24T00:00:00Z" })}\n`; + const response = { runId: "run-1", store: "memory", logRef: "log-1" }; + logMock.mockResolvedValueOnce({ ...response, content: row(1), nextOffset: 100 }); + logMock.mockResolvedValueOnce({ ...response, content: row(2), nextOffset: 100_000 }); + function Harness({ bytes }: { bytes: number }) { + ({ truncatedRunIds } = useLiveRunTranscripts({ + runs: [{ id: "run-1", status: "succeeded", adapterType: "codex_local", logBytes: bytes }], + enableRealtimeUpdates: false, logReadLimitBytes: 32_000, + })); + return null; + } + const root = createRoot(document.createElement("div")); + try { + await act(async () => root.render()); + expect(logMock).toHaveBeenLastCalledWith("run-1", 0, 32_000, expect.anything()); + expect(truncatedRunIds.has("run-1")).toBe(false); + await act(async () => root.render()); + expect(logMock).toHaveBeenLastCalledWith("run-1", 100, 32_000, expect.anything()); + expect(truncatedRunIds.has("run-1")).toBe(false); + } finally { await act(async () => root.unmount()); } + }); + it("keeps record alignment pending when an expanded log read needs retry", async () => { const row = (seq: number, chunk: string) => JSON.stringify({ seq, chunk, ts: `2026-10-24T00:00:0${seq}Z`, stream: "stdout" }); const result = (content: string) => ({ runId: "run-1", store: "memory", logRef: "log-1", content, nextOffset: 500_000 }); diff --git a/ui/src/components/transcript/useLiveRunTranscripts.ts b/ui/src/components/transcript/useLiveRunTranscripts.ts index 978f0766479..134e88e766d 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.ts +++ b/ui/src/components/transcript/useLiveRunTranscripts.ts @@ -144,6 +144,7 @@ export function useLiveRunTranscripts({ const pendingLogRowsByRunRef = useRef(new Map()); const logOffsetByRunRef = useRef(new Map()); const logReadLimitByRunRef = useRef(new Map()); + const [truncatedRunIds, setTruncatedRunIds] = useState>(new Set()); const missingTerminalLogRunIdsRef = useRef(new Set()); // PAP-462 B3: buffered runs that dropped out of the `runs` list, mapped to the // wall-clock deadline (ms) after which their buffer may be pruned. A run still @@ -260,6 +261,10 @@ export function useLiveRunTranscripts({ const next = new Map([...previous].filter(([id]) => retainedRunIds.has(id))); return next.size === previous.size ? previous : next; }); + setTruncatedRunIds((previous) => { + const next = new Set([...previous].filter((id) => retainedRunIds.has(id))); + return next.size === previous.size ? previous : next; + }); for (const key of pendingLogRowsByRunRef.current.keys()) { const runId = key.replace(/:records$/, ""); @@ -340,6 +345,15 @@ export function useLiveRunTranscripts({ // Commit the expanded window only after success so retries still align // its first record and failed reads leave the previous parser intact. + if (previousReadLimit === undefined || rewinding) { + setTruncatedRunIds((previous) => { + if (previous.has(run.id) === (offset > 0)) return previous; + const next = new Set(previous); + if (offset > 0) next.add(run.id); + else next.delete(run.id); + return next; + }); + } logReadLimitByRunRef.current.set(run.id, logReadLimitBytes); if (rewinding) { pendingLogRowsByRunRef.current.delete(run.id); @@ -596,7 +610,7 @@ export function useLiveRunTranscripts({ return { transcriptByRun, - hydratedRunIds, errorsByRun, retry, + hydratedRunIds, truncatedRunIds, errorsByRun, retry, isInitialHydrating: normalizedRuns.some((run) => canReadPersistedLog(run) && !hydratedRunIds.has(run.id)), hasOutputForRun(runId: string) { return (chunksByRun.get(runId)?.length ?? 0) > 0 || runById.get(runId)?.hasStoredOutput === true; diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 8543d8961aa..3d3726cc97f 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -1684,6 +1684,100 @@ describe("IssueDetail", () => { expect(windowOpen).not.toHaveBeenCalled(); }); + it.each([null, "queued", "running", "succeeded", "failed"])("hides the history disclosure on a fresh task with first run %s", async (status) => { + queryClient.setQueryData(["issues", "history-expanded", "PAP-1"], false); + mockIssuesApi.get.mockResolvedValue(createIssue()); + const runs = status ? [{ runId: "first-run", status, agentId: "agent-1", createdAt: new Date().toISOString() }] : []; + mockActivityApi.recentRunsForIssue.mockResolvedValue(runs); + await act(async () => renderIssueView()); + await waitForAssertion(() => { + const props = mockIssueChatThreadRender.mock.calls.at(-1)?.[0]; + expect(mockActivityApi.recentRunsForIssue).toHaveBeenCalled(); + expect(props?.linkedRuns).toMatchObject(runs); + expect(props?.compactHistory).toBe(true); + expect(props?.historyControl).toBeNull(); + }); + expect(mockActivityApi.runsForIssue).not.toHaveBeenCalled(); + expect(mockIssuesApi.listComments).not.toHaveBeenCalled(); + }); + + it("loads full first-run activity from the task menu without an older-message disclosure", async () => { + queryClient.setQueryData(["issues", "history-expanded", "PAP-1"], false); + mockIssuesApi.get.mockResolvedValue(createIssue()); + const run = { runId: "first-run", runtimeMode: "native", status: "succeeded", agentId: "agent-1", createdAt: new Date().toISOString() }; + mockActivityApi.recentRunsForIssue.mockResolvedValue([run]); + mockActivityApi.runsForIssue.mockResolvedValue([run]); + await act(async () => renderIssueView()); + await waitForAssertion(() => { + const props = mockIssueChatThreadRender.mock.calls.at(-1)?.[0]; + expect(props?.linkedRuns).toMatchObject([run]); + expect(props?.compactHistory).toBe(true); + expect(props?.historyControl).toBeNull(); + }); + expect(mockActivityApi.runsForIssue).not.toHaveBeenCalled(); + await act(async () => { + container.querySelector('button[aria-label="More task actions"]')!.click(); + }); + const activityButton = Array.from(document.body.querySelectorAll("button")) + .find((button) => button.textContent?.trim() === "Show task activity"); + expect(activityButton).toBeTruthy(); + await act(async () => activityButton!.click()); + await waitForAssertion(() => { + expect(mockActivityApi.runsForIssue).toHaveBeenCalled(); + expect(mockIssueChatThreadRender.mock.calls.at(-1)?.[0].compactHistory).toBe(false); + }); + await act(async () => { + container.querySelector('button[aria-label="More task actions"]')!.click(); + }); + expect(Array.from(document.body.querySelectorAll("button")) + .some((button) => button.textContent?.trim() === "Show task activity")).toBe(false); + }); + + it("does not add a history disclosure when a second run is observed while the page stays open", async () => { + queryClient.setQueryData(["issues", "history-expanded", "PAP-1"], false); + mockIssuesApi.get.mockResolvedValue(createIssue()); + const run = (runId: string) => ({ runId, status: "succeeded", agentId: "agent-1", createdAt: new Date().toISOString() }); + const first = run("first"), second = run("second"); + mockActivityApi.recentRunsForIssue.mockResolvedValue([first]); + await act(async () => renderIssueView()); + await waitForAssertion(() => expect(mockIssueChatThreadRender.mock.calls.at(-1)?.[0].linkedRuns).toMatchObject([first])); + mockActivityApi.recentRunsForIssue.mockResolvedValue([second, first]); + await act(async () => { await queryClient.invalidateQueries({ queryKey: queryKeys.issues.runs("PAP-1") }); }); + await waitForAssertion(() => { + const props = mockIssueChatThreadRender.mock.calls.at(-1)?.[0]; + expect(props?.linkedRuns).toMatchObject([first, second]); + expect(props?.historyControl).toBeNull(); + }); + expect(mockActivityApi.runsForIssue).not.toHaveBeenCalled(); + }); + + it("reveals omitted runs through the history disclosure when there are no older comments", async () => { + queryClient.setQueryData(["issues", "history-expanded", "PAP-1"], false); + mockIssuesApi.get.mockResolvedValue(createIssue()); + const run = (runId: string) => ({ runId, status: "succeeded", agentId: "agent-1", createdAt: new Date().toISOString() }); + const latest = run("latest"), older = run("older"), next = run("next"); + mockActivityApi.recentRunsForIssue.mockResolvedValue([latest, older]); + mockActivityApi.runsForIssue.mockResolvedValue([next, latest, older]); + await act(async () => renderIssueView()); + await waitForAssertion(() => { + const props = mockIssueChatThreadRender.mock.calls.at(-1)?.[0]; + expect(props?.linkedRuns).toMatchObject([latest]); + expect(props?.historyControl).toBeTruthy(); + }); + mockActivityApi.recentRunsForIssue.mockResolvedValue([next, latest]); + await act(async () => { await queryClient.invalidateQueries({ queryKey: queryKeys.issues.runs("PAP-1") }); }); + await waitForAssertion(() => expect(mockIssueChatThreadRender.mock.calls.at(-1)?.[0].linkedRuns).toMatchObject([latest, next])); + const disclosure = mockIssueChatThreadRender.mock.calls.at(-1)?.[0].historyControl; + expect(disclosure).toBeTruthy(); + await act(async () => disclosure.props.onClick()); + await waitForAssertion(() => { + const props = mockIssueChatThreadRender.mock.calls.at(-1)?.[0]; + expect(props?.linkedRuns).toMatchObject([next, latest, older]); + expect(props?.historyControl).toBeNull(); + expect(props?.compactHistory).toBe(false); + }); + }); + it("reveals a cold task and recent exchange while full detail and older history are stalled", async () => { queryClient.setQueryData(["issues", "history-expanded", "PAP-1"], false); const stalled = createDeferred(); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index f4b707bf6b8..33d0cf9d6cb 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -1203,6 +1203,11 @@ function InboxMobileToolbar({ ); } +interface RecentIssueRunWindow { + runs: RunForIssue[]; + hasEarlierRuns: boolean; +} + type IssueDetailChatTabProps = { executionBlocked?: boolean; interruption?: Issue["interruption"]; @@ -1620,37 +1625,48 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ const hasLiveRuns = liveRunCount > 0 || !!resolvedActiveRun; const linkedRunsKey = historyExpanded ? queryKeys.issues.runs(issueQueryRef) - : [...queryKeys.issues.runs(issueQueryRef), "recent"]; + : [...queryKeys.issues.runs(issueQueryRef), "recent-window"]; const { - data: linkedRuns, + data: linkedRunHistory, isPending: linkedRunsPending, isError: linkedRunsError, refetch: refetchLinkedRuns, - } = useQuery({ + } = useQuery({ queryKey: linkedRunsKey, queryFn: async () => { if (historyExpanded) return activityApi.runsForIssue(issueQueryRef); let recent = await activityApi.recentRunsForIssue(issueQueryRef); - let previous = queryClient.getQueryData(linkedRunsKey) ?? []; + const previousWindow = queryClient.getQueryData(linkedRunsKey); + let previous = previousWindow?.runs ?? []; + let reconciledFullHistory = false; if (previous.some((run) => (run.status === "running" || run.status === "queued") && !recent.some((next) => next.runId === run.runId), )) { // After a reconnect, several turns may have completed beyond the small // window. Reconcile watched runs once against authoritative summaries. recent = await activityApi.runsForIssue(issueQueryRef); + reconciledFullHistory = true; const existingRunIds = new Set(recent.map((run) => run.runId)); previous = previous.filter((run) => existingRunIds.has(run.runId)); } // The second row refreshes a just-finished predecessor. Start with only // the latest run, then keep every run observed while this task is open. const visible = new Set([...previous.map((run) => run.runId), ...recent.slice(0, 1).map((run) => run.runId)]); - return [...new Map([...previous, ...recent].filter((run) => visible.has(run.runId)).map((run) => [run.runId, run])).values()]; + return { + runs: [...new Map([...previous, ...recent].filter((run) => visible.has(run.runId)).map((run) => [run.runId, run])).values()], + // A first run is already visible. The disclosure needs evidence of an + // omitted run, and must retain that evidence as later turns arrive. + hasEarlierRuns: recent.some((run) => !visible.has(run.runId)) + || (!reconciledFullHistory && recent.length >= 2 && previousWindow?.hasEarlierRuns === true), + }; }, enabled: !!issueId, refetchInterval: hasLiveRuns || issueStatus === "in_progress" ? 1000 : false, - placeholderData: historyExpanded ? queryClient.getQueryData([...queryKeys.issues.runs(issueQueryRef), "recent"]) : undefined, + placeholderData: historyExpanded ? queryClient.getQueryData([...queryKeys.issues.runs(issueQueryRef), "recent-window"])?.runs : undefined, }); + const linkedRuns = Array.isArray(linkedRunHistory) ? linkedRunHistory : linkedRunHistory?.runs; + const hasEarlierRuns = !Array.isArray(linkedRunHistory) && linkedRunHistory?.hasEarlierRuns === true; const resolvedActivity = useMemo( () => [...new Map([...(activity ?? []), ...(inputFacts.data ?? [])].map((event) => [event.id, event])).values()], [activity, inputFacts.data], @@ -2560,7 +2576,8 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ 0) ? ( + onExpandHistory, + historyControl: !historyExpanded && (hasEarlierMessages || hasEarlierRuns) ? ( @@ -7661,6 +7678,18 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS + {taskChatShellEnabled && !historyExpanded ? ( + + ) : null} {streamlinedTaskDetailEnabled ? ( <>