fix(runner): allow warm release to drain after provider restart (#20290)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Warm Runner sessions park after a turn so follow-up tasks can claim
the same process.
> - Claude restarts its ACPX provider after the controller receives the
terminal result.
> - That restart can delay the drain receipt beyond the release path's
one-second limit.
> - A failed drain prevents parking, so the idle timer later closes the
retained Runner.
> - This pull request uses the existing warm admission budget for
release drain.
> - The benefit is retained Runner reuse without moving provider restart
onto task dispatch.

## Linked Issues or Issue Description

Fixes: #20225. Refs #20190 and #20145. Related performance follow-up:
#20224.

## What Changed

- Align the release drain timeout with the existing 30-second warm
admission budget for every provider. Close paths keep their explicit
remaining deadlines.
- Keep the existing immediate completion when the drain receipt proves
settlement. Add no sleep, retry, provider branch or dispatch work.
- Add a real Runner regression with a fake ACPX sidecar whose
replacement start takes 1.5 seconds. One release call must park the
Runner, then a follow-up must claim the same Runner process.
- Document the release drain budget.

## Verification

- Targeted command in `packages/paperclip-runner`: `vitest run
src/live/runnerd-codex-transport-release-resume.test.ts
src/live/runnerd-codex-transport-settlement.test.ts
src/live/runnerd-codex-transport-deadlines.test.ts`: **46 passed**
across 3 files, 157.78 seconds.
- Independent parent verification reran the slow-restart and ordinary
cached cases: **2 passed**. Source review confirmed that release is the
only caller using the changed default; close callers supply explicit
deadlines.
- Negative control: with the original one-second timeout, the new test
fails because the single release call returns `false`. The fixed code
passes the new case.
- Built only the current-source Runner and two fixture binaries required
by these tests. The first sandboxed invocation could not bind its
loopback listener (`EPERM`); the test ran with loopback access
afterward.
- Full GitHub PR workflow
[42644210993](https://github.com/paperclipai/paperclip/actions/runs/42644210993)
passed on head `9cfda0927b514d8d40443b612afa92525b35b700`. Standard
Greptile scored this same head 5/5 with no review threads. Full
repository checks ran in GitHub CI, per the requested local test scope.
No live Claude session or hosted deployment was tested.

## Risks

- An unresponsive Runner can now hold release drain for up to the
existing 30-second warm admission limit. A healthy Runner returns as
soon as its receipt arrives. The provider restart stays at turn end;
task dispatch gets no fixed delay.
- Saved state, protocol and provider identity formats do not change.
Previous-release state carries forward under the existing validation.
- The controlled fake-sidecar test proves the restart timing race and
process reuse. It does not qualify live provider latency or a hosted
rollout.

## Model Used

- OpenAI GPT-6 through Codex, with repository tools, code editing and
targeted test execution. The exact serving revision 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 <noreply@paperclip.ing>
This commit is contained in:
Nicky LeachandPaperclip authored and GitHub committed 2026-11-09 07:43:27 -08:00
1 parent dc9ad17515
commit 09412e652a
3 files changed
+21 -4

No files matched your search

@@ -83,6 +83,13 @@ a scripted harness, removes the controller's launch files, then recovers the
same process and provider session and completes another turn. This exercises the
real transport; it does not qualify a hosted OpenCode or ACPX deployment.
## Warm Runner release
Before parking a settled Runner, the controller waits for its durable drain
receipt within the same 30-second budget used for warm admission. A Claude
turn-end provider restart can delay that receipt beyond one second. The wait
returns as soon as the drain is proved; it adds no fixed delay to a ready Runner.
## ACPX checkpoints after a rejected turn start
A matching `turn.start` response with `SESSION_RESUME_REQUIRED` and
@@ -78,16 +78,19 @@ async function nativeWire(port: number, path: string, closeGate?: Promise<void>)
});
}
it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cache adoption", "acpx cache adoption", "unavailable", "claim cancellation", "expired ticket", "late claim reply", "held past deadline", "release admission", "no cleanup callback", "adoption no cleanup callback", "resume no cleanup callback", "deferred turn after close", "late wire close", "empty goal snapshot", "early claim", "early claim abandoned", "early claim cancellation", "early claim expired", "early claim stale ticket"] as const)(
it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cache adoption", "acpx cache adoption", "acpx slow restart", "unavailable", "claim cancellation", "expired ticket", "late claim reply", "held past deadline", "release admission", "no cleanup callback", "adoption no cleanup callback", "resume no cleanup callback", "deferred turn after close", "late wire close", "empty goal snapshot", "early claim", "early claim abandoned", "early claim cancellation", "early claim expired", "early claim stale ticket"] as const)(
"releases and resumes the native Codex source before attachment (%s)", async mode => {
const root = await mkdtemp(join(tmpdir(), "runner-release-resume-"));
const acpx = mode === "acpx cache adoption";
const acpx = mode === "acpx cache adoption" || mode === "acpx slow restart";
const sidecar = join(root, "sidecar"), sidecarScript = join(root, "sidecar-entry");
const restartStarted = join(root, "restart-started");
if (acpx) {
const quote = (value: string) => `'${value.replaceAll("'", "'\"'\"'")}'`;
const fake = resolve(import.meta.dirname, "../../runner/target/debug/fake-acpx-sidecar");
await writeFile(sidecarScript, "qualified fixture entry");
await writeFile(sidecar, `#!/bin/sh\nexec ${quote(fake)} --mode source-reconcile --profile-digest ${quote(QUALIFIED_ACPX_PROFILES.claude.commandDigest)} --journal ${quote(join(root, "calls"))} --source-fault ${quote(join(root, "fault"))}\n`, { mode: 0o700 });
const restartDelay = mode === "acpx slow restart"
? `if [ -e ${quote(join(root, "calls"))} ]; then touch ${quote(restartStarted)}; sleep 1.5; fi\n` : "";
await writeFile(sidecar, `#!/bin/sh\n${restartDelay}exec ${quote(fake)} --mode source-reconcile --profile-digest ${quote(QUALIFIED_ACPX_PROFILES.claude.commandDigest)} --journal ${quote(join(root, "calls"))} --source-fault ${quote(join(root, "fault"))}\n`, { mode: 0o700 });
}
const listener = createServer();
await new Promise<void>(resolve => listener.listen(0, "127.0.0.1", resolve));
@@ -214,6 +217,12 @@ it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cac
const opened = await bundle.transport.request("thread/start", { cwd: root, dynamicTools: [], ...(acpx ? { model: "explicit-source-model" } : {}) });
const threadId = String((opened.thread as { id: string }).id);
await complete();
if (mode === "acpx slow restart") {
await vi.waitFor(async () => expect(await readFile(restartStarted, "utf8")).toBe(""));
expect(await bundle.transport.release!()).toEqual(identity);
expect(pair.release).toHaveBeenCalledOnce();
expect(authority!.store.state.commands.filter(command => command.type === "runner.drain")).toHaveLength(1);
}
if (mode === "empty goal snapshot") {
await writeFile(goalGate, "release late goal");
await vi.waitFor(() => expect(authority!.store.state.committedEvents.some(event => event.eventType === "session.goal.updated"
@@ -5185,8 +5185,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
return false;
}
// Release shares the warm admission budget: a turn-end restart can delay the drain receipt.
async #drainSettledProviderEventsBeforeSuspend(
timeoutMs = 1_000,
timeoutMs = coldAdmissionTimeoutMs(this.options.provider, this.options.acpxAgent, false),
providerStopConfirmed = false,
): Promise<boolean> {
const core = this.#core;