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] 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);