mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 11:46:36 +01:00
fix(runner): fail a pending Codex turn start at once with the runner's own terminal failure (#19722)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The control plane starts each Codex turn through runnerd and waits for the turn's start event (`#startTurnWork` in `runnerd-codex-transport.ts`) > - In restart storm 5, runnerd failed an accepted continuation turn before it started (`turn_binding_mismatch`, fixed at its source in #19718) > - The transport held that terminal for the pending start and waited the full 30 s bound > - It then reported "runnerd did not report the provider turn identity", which hid the real cause > - This pull request makes a pending start fail at once with the runner's own terminal failure > - The benefit is a faster failure that names its real cause, so recovery and operators act on the correct fact ## Linked Issues or Issue Description Refs #19718, #19699, #19608 **What happened?** Restart storm 5 (seed 7106), task t01 (Codex). Runnerd accepted the restart continuation turn. About 0.5 s later it committed a `harness.diagnostic`, a non-recoverable `turn.failed` (`turn_binding_mismatch`) and a failed `run.terminal`. The transport's event pump holds a turn terminal while a turn start is pending, so the start loop never saw it. The start waited 30 s, the #19608 adoption check found no active turn, and the run failed with "runnerd did not report the provider turn identity". **Expected behavior** When runnerd reports a final failure of the pending turn (a non-recoverable `turn.failed` or a failed `run.terminal`) before the turn starts, the start fails at once. The error names the runner's code and message. **Steps to reproduce** 1. Use fake Codex with `--foreign-usage-before-turn-started --turn-started-delay-ms 20000`. 2. Start a turn through the runnerd Codex transport. 3. Before this change, `turn/start` rejects after the full bound with "runnerd did not report the provider turn identity". ## What Changed - `runnerd-codex-transport.ts`: new `runnerdPendingTurnStartFailure()`. It finds a non-recoverable `turn.failed` or a failed `run.terminal` among the terminals held for the pending start and the committed events after the cursor. - `#startTurnWork` checks it while it waits for `turn/started`, before the adoption snapshot, and in the post-adoption wait. It throws `runnerd failed the provider turn before it started: <code>: <message>`. - The check runs only when the turn has not started. A recoverable terminal does not trigger it. The #19608 late-start adoption path is unchanged. - Fake Codex: new `--foreign-usage-before-turn-started` flag. It sends usage for a turn the runner does not own right after it accepts a turn. This makes runnerd fail the turn with `turn_binding_mismatch` before it starts. - New test in `runnerd-codex-transport-deadlines.test.ts` with real runnerd and fake Codex. ## Verification - Red before the fix: `npx vitest run src/live/runnerd-codex-transport-deadlines.test.ts` → the new test failed after 16.4 s with "runnerd did not report the provider turn identity". - Green after the fix: the start rejects in under 5 s with `turn_binding_mismatch`, and fake Codex saw exactly one `turn/start`. - The #19608 tests ("adopts a durably accepted turn whose start notification outlives the turn-start bound" and "does not adopt an accepted turn the runner no longer reports active") still pass. - `npx vitest run src/live/runnerd-codex-transport`: 9 files, 399 tests pass. - `pnpm run typecheck` in `packages/paperclip-runner` passes. `cargo test --bin fake-codex-app-server` passes. `cargo fmt` is clean. - `pnpm verify` exits 2 (broad input: the fake Codex binary). Required CI supplies the broad coverage. ## Risks - Low risk. The new failure fires only when runnerd itself already reported a final failure and no turn start was observed. In that state the start could not succeed, and before this change it failed after the bound anyway. - If a provider sends a non-recoverable terminal before its own turn start for a turn that later starts, the start now fails earlier. Codex sends `turn/started` before its terminal, and runnerd's terminal for a protocol failure is final, so I do not expect this. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file editing). Default context window and reasoning mode. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1a60d47804
commit
d1ee25d0f7
3 files changed
+81
No files matched your search
@@ -1795,6 +1795,18 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
|
||||
"tokenUsage": {"total": {"inputTokens": 100, "outputTokens": 20}, "last": {"inputTokens": 50, "outputTokens": 10}}
|
||||
}}))?;
|
||||
}
|
||||
// A notification for a turn the runner never owned fails the
|
||||
// accepted turn before it starts.
|
||||
if args
|
||||
.iter()
|
||||
.any(|value| value == "--foreign-usage-before-turn-started")
|
||||
{
|
||||
send(json!({"method": "thread/tokenUsage/updated", "params": {
|
||||
"threadId": state.thread_id,
|
||||
"turnId": "provider-turn-foreign",
|
||||
"tokenUsage": {"total": {"inputTokens": 100, "outputTokens": 20}, "last": {"inputTokens": 50, "outputTokens": 10}}
|
||||
}}))?;
|
||||
}
|
||||
// A loaded host can accept a turn long before it starts it.
|
||||
if let Some(delay) = argument(&args, "--turn-started-delay-ms") {
|
||||
thread::sleep(Duration::from_millis(delay.parse()?));
|
||||
|
||||
@@ -80,6 +80,32 @@ it("does not adopt an accepted turn the runner no longer reports active", async
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it("fails a pending turn start at once with the runner's own terminal failure", async () => {
|
||||
// The runner can fail an accepted turn before its turn/started arrives
|
||||
// (RC-20d: turn_binding_mismatch). That terminal is final, so the start must
|
||||
// report it at once rather than wait out the bound under a generic message.
|
||||
const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-failed-turn-start-"));
|
||||
const callsPath = join(stateDirectory, "calls.log");
|
||||
const bundle = createCapabilityRunnerdCodexTransport({
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(), codexCommand: fakeCodex,
|
||||
codexArgs: fakeCodexArgs(stateDirectory, "--foreign-usage-before-turn-started",
|
||||
"--turn-started-delay-ms", "20000", "--call-log", callsPath),
|
||||
stateDirectory, turnStartTimeoutMs: 15_000,
|
||||
});
|
||||
try {
|
||||
await bundle.transport.request("thread/start", { cwd: tmpdir(), dynamicTools: [] });
|
||||
const started = Date.now();
|
||||
await expect(bundle.transport.request("turn/start", { input: [{ type: "text", text: "Fail before start." }] }))
|
||||
.rejects.toThrow("turn_binding_mismatch");
|
||||
expect(Date.now() - started).toBeLessThan(5_000);
|
||||
const calls = (await readFile(callsPath, "utf8")).trim().split(/\r?\n/);
|
||||
expect(calls.filter(call => call === "turn/start")).toHaveLength(1);
|
||||
} finally {
|
||||
await bundle.transport.close().catch(() => undefined);
|
||||
await rm(stateDirectory, { recursive: true, force: true });
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it("keeps a quiet active Codex turn in the same process across connection lease expiry", async () => {
|
||||
const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-renew-active-"));
|
||||
const callsPath = join(stateDirectory, "calls.log");
|
||||
|
||||
@@ -867,6 +867,34 @@ function terminalMayBelongToPendingTurnStart(input: {
|
||||
/** Part of the turn-start bound kept for the accepted-turn adoption check. */
|
||||
const TURN_START_ADOPTION_CHECK_RESERVE_MS = 5_000;
|
||||
|
||||
/**
|
||||
* The runner's own final failure of a turn whose start is still pending: a
|
||||
* non-recoverable turn terminal held for that start, or a failed run terminal.
|
||||
* Neither can be followed by a live start, so the start reports it at once.
|
||||
*/
|
||||
export function runnerdPendingTurnStartFailure(
|
||||
events: ReadonlyArray<{ eventType: string; envelope: { payload?: unknown } }>,
|
||||
): string | null {
|
||||
for (const event of events) {
|
||||
const payload = record(record(event.envelope.payload).payload);
|
||||
const error = record(payload.error);
|
||||
if (
|
||||
event.eventType === "turn.failed" &&
|
||||
(payload.recoverable === false || error.recoverable === false)
|
||||
) {
|
||||
const code = typeof error.code === "string" ? error.code
|
||||
: typeof payload.code === "string" ? payload.code : "provider_turn_failed";
|
||||
const message = typeof error.message === "string" ? error.message
|
||||
: typeof payload.message === "string" ? payload.message : null;
|
||||
return message ? `${code}: ${message}` : code;
|
||||
}
|
||||
if (event.eventType === "run.terminal" && payload.runTerminalState === "failed") {
|
||||
return "run_terminal_failed";
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function turnStartNotificationDisposition(input: {
|
||||
responsePending: boolean;
|
||||
expectedProviderTurnId: string | null;
|
||||
@@ -7107,11 +7135,24 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
TURN_START_ADOPTION_CHECK_RESERVE_MS,
|
||||
Math.floor(Math.max(0, deadline - Date.now()) / 4),
|
||||
);
|
||||
// The runner can end the accepted turn before it starts. Its terminal is
|
||||
// held for the pending start, so report that real cause at once.
|
||||
const throwIfRunnerFailedStart = () => {
|
||||
const failure = runnerdPendingTurnStartFailure([
|
||||
...this.#deferredTurnStartEvents,
|
||||
...(this.#core?.store.state.committedEvents ?? []).filter(
|
||||
(event) => event.sourceSeq > this.#eventSourceSeq,
|
||||
),
|
||||
]);
|
||||
if (failure !== null)
|
||||
throw new Error(`runnerd failed the provider turn before it started: ${failure}`);
|
||||
};
|
||||
const waitForStarted = async () => {
|
||||
while (!providerTurnStarted() && Date.now() < adoptionCheckAt) {
|
||||
this.#throwIfFailed();
|
||||
this.#pumpEvents();
|
||||
if (providerTurnStarted()) break;
|
||||
throwIfRunnerFailedStart();
|
||||
if (await this.#runnerHasExited())
|
||||
throw new Error("runnerd exited before provider turn startup");
|
||||
await new Promise((resolveWait) => setTimeout(resolveWait, 10));
|
||||
@@ -7135,6 +7176,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
// report a successful start on it.
|
||||
this.#pumpEventsSafely();
|
||||
this.#throwIfFailed();
|
||||
if (!providerTurnStarted()) throwIfRunnerFailedStart();
|
||||
if (!providerTurnStarted() && this.#acceptedTurnProvablyLive({
|
||||
snapshot, expectedProviderTurnId, acceptingProviderPid,
|
||||
acceptingProviderStartedAt, acceptingCommandSeq,
|
||||
@@ -7151,6 +7193,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
this.#throwIfFailed();
|
||||
this.#pumpEvents();
|
||||
if (providerTurnStarted()) break;
|
||||
throwIfRunnerFailedStart();
|
||||
if (await this.#runnerHasExited())
|
||||
throw new Error("runnerd exited before provider turn startup");
|
||||
await new Promise((resolveWait) => setTimeout(resolveWait, 10));
|
||||
|
||||
Reference in new issue
Block a user