mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(runner): adopt an accepted Codex turn whose start outlives the bound (#19608)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native Codex runs submit each turn through runnerd. The transport
answers `turn/start` only after it sees the provider's `turn/started`
for the turn the durable command result named
> - That wait has a fixed 30 s bound
> - At host load around 50, after two hard restarts, Codex accepted a
turn and started it about 27 s after the command, beyond the bound once
scheduling delay was added. The transport failed the start, but the
provider kept running the turn and made a tool call
> - No controller owned that turn. Its close could not settle, and the
task's Retry was refused at once
> - This pull request adopts the accepted turn when the bound expires,
but only when the runner still reports that turn active
> - The benefit is that a slow but live provider turn stays owned, so
its events, Stop and close keep working
## Linked Issues or Issue Description
**What happened?**
In a restart chaos replay (two SIGKILLs about 20 s apart, local runner,
real Codex, load 50), a resumed run failed with `runnerd did not report
the provider turn identity`. The runner journal shows the durable
`turn.start` result with the provider turn id, then `turn.started` for
that same turn 27 s later, followed by items and a semantic tool call.
The run's close then stayed incomplete (`semanticResultsSettled:
false`), and a Retry of the task failed with
`native_session_cleanup_quarantined: prior session cleanup remains
incomplete`.
**Steps to reproduce**
1. Use a Codex provider that answers `turn/start` and emits
`turn/started` later than the transport's turn-start bound.
2. Start a turn through the runnerd transport.
**Expected behavior**
The accepted turn is owned and observed by the controller.
**Actual behavior**
The start fails after the bound, while the provider still runs the turn.
## What Changed
- `runnerd-codex-transport.ts` (`#startTurnWork`):
- Near the end of the bound, if `turn/started` has not arrived, the
transport reads an authenticated `session.snapshot`. It reserves at most
5 s (or a quarter of the bound) for this read.
- It binds the accepted turn only when the turn is provably live:
- the runner reports that exact provider turn as active;
- the runner attributed no provider launch
(`provider_startup_ownership`) to a command after this `turn.start`;
- for a local provider, the same process (pid and start time) that
accepted the turn is still alive.
- Otherwise it keeps waiting for `turn/started` until the configured
bound and fails exactly as before. A turn lost across a provider
relaunch keeps the same-run resume path.
- `fake-codex-app-server.rs`: new test options `--turn-started-delay-ms`
(a delay between the `turn/start` response and `turn/started`) and
`--exit-before-turn-started`.
## Verification
- Red before the fix: `pnpm --filter @paperclipai/paperclip-runner exec
vitest run src/live/runnerd-codex-transport-deadlines.test.ts -t "adopts
a durably accepted"` (real runnerd and fake Codex, 12 s start delay, 8 s
bound) failed with `runnerd did not report the provider turn identity`.
This is the replay's error.
- Green after the fix: the start resolves with `provider-turn-1`, and
`turn/started` and the turn's terminal follow in order. There is one
provider `turn/start`.
- Negative case (`does not adopt an accepted turn the runner no longer
reports active`): the provider accepts the turn and exits before
starting it. The runner relaunches the provider and still lists the
turn, but the turn is not adopted. The start fails within the bound plus
scheduling slack. A snapshot-only version of this change adopted that
dead turn, and it hung for 20 s with no terminal.
- `src/native-session-runtime.test.ts`,
`src/live/runnerd-restart-continuation.test.ts` and
`src/live/runnerd-codex-transport-deadlines.test.ts` pass. The package
typecheck passes.
- Live chaos replays: results are added in a comment.
## Risks
- A provably live provider that accepts a turn and never starts it now
waits on the turn's own timeout and Stop paths instead of failing after
30 s. A dead or relaunched provider still fails at the bound.
- Remote runners can adopt only on the runner's own evidence (active
turn and no later provider launch), because their provider pid is not a
local process.
- A stale `turn/started` for a previous turn that arrives after the new
command result is still rejected. The transport cannot attribute that
turn's following events, so rejecting it fails only the transport, and
the same-run resume continues the task (seen in the replay). Delivering
it would attach the old turn's items and usage to the new turn.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.
## 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
Refs #19532, #19539.
🤖 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
06bbcea12f
commit
e05bced39f
3 files changed
+156
-3
No files matched your search
@@ -1781,6 +1781,17 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
|
||||
"id": id,
|
||||
"result": {"turn": {"id": provider_turn_id, "status": "inProgress"}}
|
||||
}))?;
|
||||
// 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()?));
|
||||
}
|
||||
// An accepted turn can be lost before it starts.
|
||||
if args
|
||||
.iter()
|
||||
.any(|value| value == "--exit-before-turn-started")
|
||||
{
|
||||
return Err("configured exit before turn started".into());
|
||||
}
|
||||
send(json!({
|
||||
"method": "turn/started",
|
||||
"params": {"turn": {"id": provider_turn_id}}
|
||||
|
||||
@@ -24,6 +24,62 @@ function fakeCodexArgs(stateDirectory: string, ...args: string[]): string[] {
|
||||
return ["--state-file", join(stateDirectory, "fake-codex-state.json"), ...args];
|
||||
}
|
||||
|
||||
it("adopts a durably accepted turn whose start notification outlives the turn-start bound", async () => {
|
||||
// A loaded host can accept a turn (durable command result with its provider
|
||||
// turn id) and start it only after the turn-start bound. The provider still
|
||||
// runs that turn, so failing the start would abandon a live turn.
|
||||
const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-late-turn-started-"));
|
||||
const callsPath = join(stateDirectory, "calls.log");
|
||||
const bundle = createCapabilityRunnerdCodexTransport({
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(), codexCommand: fakeCodex,
|
||||
codexArgs: fakeCodexArgs(stateDirectory, "--turn-started-delay-ms", "12000", "--fail-turn-immediately", "--call-log", callsPath),
|
||||
stateDirectory, turnStartTimeoutMs: 8_000,
|
||||
});
|
||||
try {
|
||||
await bundle.transport.request("thread/start", { cwd: tmpdir(), dynamicTools: [] });
|
||||
const started = await bundle.transport.request("turn/start", { input: [{ type: "text", text: "Start late." }] });
|
||||
expect(started).toMatchObject({ turn: { id: "provider-turn-1" } });
|
||||
const methods: string[] = [];
|
||||
for await (const notification of bundle.transport.notifications()) {
|
||||
methods.push(notification.method);
|
||||
if (notification.method === "turn/completed") {
|
||||
expect(notification.params).toMatchObject({ turnId: "provider-turn-1" });
|
||||
break;
|
||||
}
|
||||
}
|
||||
expect(methods).toContain("turn/started");
|
||||
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("does not adopt an accepted turn the runner no longer reports active", async () => {
|
||||
// The provider accepts the turn, then its process ends before the turn
|
||||
// starts. The accepted turn is not live, so the start fails within its bound.
|
||||
const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-lost-turn-start-"));
|
||||
const bundle = createCapabilityRunnerdCodexTransport({
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(), codexCommand: fakeCodex,
|
||||
codexArgs: fakeCodexArgs(stateDirectory, "--exit-before-turn-started"),
|
||||
stateDirectory, turnStartTimeoutMs: 2_000,
|
||||
});
|
||||
try {
|
||||
await bundle.transport.request("thread/start", { cwd: tmpdir(), dynamicTools: [] });
|
||||
const started = Date.now();
|
||||
// The runner may still list the turn after relaunching its provider; the
|
||||
// process that accepted it is gone, so the turn is not adopted.
|
||||
await expect(bundle.transport.request("turn/start", { input: [{ type: "text", text: "Lose this turn." }] }))
|
||||
.rejects.toThrow("runnerd did not report the provider turn identity");
|
||||
// The adoption check stays inside the configured bound (plus scheduling slack).
|
||||
expect(Date.now() - started).toBeLessThan(2_000 + 1_500);
|
||||
} 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");
|
||||
|
||||
@@ -864,6 +864,9 @@ 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;
|
||||
|
||||
function turnStartNotificationDisposition(input: {
|
||||
responsePending: boolean;
|
||||
expectedProviderTurnId: string | null;
|
||||
@@ -6756,6 +6759,35 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
return this.options.measureOperation ? this.options.measureOperation("runner.transport.turn_start", () => this.#startTurnWork(params), {inputBytes:Buffer.byteLength(JSON.stringify(params)), tokenDataAvailable:false}) : this.#startTurnWork(params);
|
||||
}
|
||||
|
||||
/**
|
||||
* Live-turn proof for adopting an accepted turn whose start event is late:
|
||||
* the runner still reports it active, the runner attributed no provider
|
||||
* launch to a command after this turn.start, and, when the accepting provider is a
|
||||
* local process, that exact process (pid and start time) is still alive.
|
||||
*/
|
||||
#acceptedTurnProvablyLive(input: {
|
||||
snapshot: Record<string, unknown> | null;
|
||||
expectedProviderTurnId: string;
|
||||
acceptingProviderPid: number | null;
|
||||
acceptingProviderStartedAt: string | null;
|
||||
acceptingCommandSeq: number;
|
||||
}): boolean {
|
||||
if (input.snapshot?.activeProviderTurnId !== input.expectedProviderTurnId) return false;
|
||||
if (!Number.isSafeInteger(input.acceptingCommandSeq)) return false;
|
||||
const relaunched = (this.#core?.store.state.committedEvents ?? []).some((event) => {
|
||||
if (event.eventType !== "harness.diagnostic") return false;
|
||||
const payload = record(record(event.envelope.payload).payload);
|
||||
return payload.code === "provider_startup_ownership" &&
|
||||
Number(record(record(payload.startup).command).controllerSeq) > input.acceptingCommandSeq;
|
||||
});
|
||||
if (relaunched) return false;
|
||||
if (input.acceptingProviderPid === null || this.#evidence.providerPid !== input.acceptingProviderPid) return false;
|
||||
// A remote provider's pid names no local process; the runner's own
|
||||
// launch reports above are its replacement evidence.
|
||||
if (input.acceptingProviderStartedAt === null) return true;
|
||||
return readLocalProcessStartedAt(input.acceptingProviderPid) === input.acceptingProviderStartedAt;
|
||||
}
|
||||
|
||||
async #startTurnWork(
|
||||
params: Record<string, unknown>,
|
||||
): Promise<Record<string, unknown>> {
|
||||
@@ -6848,6 +6880,14 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
throw error;
|
||||
}
|
||||
this.#expectedProviderTurnId = expectedProviderTurnId;
|
||||
// The exact local provider process that accepted this turn. Adoption
|
||||
// below requires that same process to still be alive.
|
||||
const acceptingProviderPid = this.#evidence.providerPid;
|
||||
const acceptingProviderStartedAt = this.#evidence.providerProcessStartedAt;
|
||||
// The durable controller sequence of this turn.start command. A provider
|
||||
// launch the runner attributes to a later command replaced the provider.
|
||||
const acceptingCommandSeq = Number(record([...(this.#core?.store.state.commands ?? [])]
|
||||
.reverse().find((command) => command.type === "turn.start")?.result).controllerSeq);
|
||||
// Command completion only means runnerd accepted the command. Bind the
|
||||
// provider turn from the subsequent turn/started event before answering
|
||||
// the strict driver. Correlate that event with the exact identity in this
|
||||
@@ -6865,8 +6905,14 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
expectedProviderTurnId,
|
||||
boundTurnId: this.#turnId,
|
||||
});
|
||||
// Keep the adoption check inside the configured bound: stop waiting for
|
||||
// the start event early enough to read the runner's snapshot before it.
|
||||
const adoptionCheckAt = deadline - Math.min(
|
||||
TURN_START_ADOPTION_CHECK_RESERVE_MS,
|
||||
Math.floor(Math.max(0, deadline - Date.now()) / 4),
|
||||
);
|
||||
const waitForStarted = async () => {
|
||||
while (!providerTurnStarted() && Date.now() < deadline) {
|
||||
while (!providerTurnStarted() && Date.now() < adoptionCheckAt) {
|
||||
this.#throwIfFailed();
|
||||
this.#pumpEvents();
|
||||
if (providerTurnStarted()) break;
|
||||
@@ -6874,8 +6920,48 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
throw new Error("runnerd exited before provider turn startup");
|
||||
await new Promise((resolveWait) => setTimeout(resolveWait, 10));
|
||||
}
|
||||
if (!providerTurnStarted())
|
||||
throw new Error("runnerd did not report the provider turn identity");
|
||||
if (!providerTurnStarted()) {
|
||||
// The durable command result already carries the provider's own
|
||||
// acceptance and turn id. A loaded provider can start that turn after
|
||||
// this bound, and it still runs it. Failing would abandon a live turn
|
||||
// that no controller owns, so its tools and close could never settle.
|
||||
// Bind the exact accepted identity when the runner proves it is live;
|
||||
// its turn/started is then accepted in order, and the turn's own
|
||||
// terminal, timeout and Stop paths own it from here.
|
||||
this.#throwIfFailed();
|
||||
if (await this.#runnerHasExited())
|
||||
throw new Error("runnerd exited before provider turn startup");
|
||||
const snapshot = await this.#commandResult(
|
||||
"session.snapshot", {}, deadline,
|
||||
).catch(() => null);
|
||||
// Ingest any runner-reported provider launch committed before it. A
|
||||
// pump that rejected a stale start has failed the transport; never
|
||||
// report a successful start on it.
|
||||
this.#pumpEventsSafely();
|
||||
this.#throwIfFailed();
|
||||
if (!providerTurnStarted() && this.#acceptedTurnProvablyLive({
|
||||
snapshot, expectedProviderTurnId, acceptingProviderPid,
|
||||
acceptingProviderStartedAt, acceptingCommandSeq,
|
||||
})) {
|
||||
this.#turnId = expectedProviderTurnId;
|
||||
this.#observedTurnStartEpoch = responseEpoch;
|
||||
this.#diagnostic(
|
||||
"provider turn start outlived its bound; adopting the durably accepted active provider turn",
|
||||
);
|
||||
}
|
||||
// Not provably live: keep the original contract and wait for the
|
||||
// start event until the configured bound, then fail as before.
|
||||
while (!providerTurnStarted() && Date.now() < deadline) {
|
||||
this.#throwIfFailed();
|
||||
this.#pumpEvents();
|
||||
if (providerTurnStarted()) break;
|
||||
if (await this.#runnerHasExited())
|
||||
throw new Error("runnerd exited before provider turn startup");
|
||||
await new Promise((resolveWait) => setTimeout(resolveWait, 10));
|
||||
}
|
||||
if (!providerTurnStarted())
|
||||
throw new Error("runnerd did not report the provider turn identity");
|
||||
}
|
||||
};
|
||||
if (this.options.measureOperation) await this.options.measureOperation("runner.transport.started_event", waitForStarted);
|
||||
else await waitForStarted();
|
||||
|
||||
Reference in new issue
Block a user