mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
bbe1016d81
commit
ad861a7f94
4 files changed
+192
-4
No files matched your search
@@ -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.
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<typeof setTimeout> | 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);
|
||||
Reference in new issue
Block a user