From 55ea9cfa1bc7a5469ab0cf76ed2c487803238711 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 6 Nov 2026 08:54:09 -0800 Subject: [PATCH] fix(runner): match retained cleanup and Codex close types to their contracts (#19495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Paperclip Runner runs native agent sessions, such as Codex, behind typed contracts > - #18839 started to typecheck the Runner tests, and it listed contract types that do not match their runtime use > - First, retained runnerd cleanup accepts any backend kind, but cleanup works only when that kind is `"runner"` > - Second, `CodexHarnessSession.close` makes its input optional, but the `HarnessSession` contract requires a reason > - This pull request narrows both types to match their contracts > - The benefit is that the compiler now rejects a cleanup proof that would silently release nothing ## Linked Issues or Issue Description No issue exists. #18839 listed both items under its product questions and did not change them. **What existing behavior does this improve?** 1. `completeRetainedNativeSessionCleanup` builds its release key from `proof.backend.kind`. The quarantine key comes from the backend descriptor's `kind`, and every production backend uses `"runner"`. If the two kinds differ, the function returns 0 and the quarantine stays. `RetainedRunnerdMaintenanceInput` and `RetainedRunnerdCleanupProof` typed `backend.kind` as any string. 2. `HarnessSession.close` takes `{ reason: string; force?: boolean }`. `CodexHarnessSession.close` took `input?: { reason: string }`. **Current behavior** The only production caller passes `backend: { kind: "runner", name: "codex_app_server" }`. All production `close` calls pass a reason. So nothing fails today. **Proposed behavior** The types state the same rules as the contracts. **Reason and benefit** A future caller that passes another kind gets a compile error, not a quarantine that stays without notice. **Breaking changes** None for production callers. Both types are narrower, and every current caller already passes the narrow values. ## What Changed - `packages/paperclip-runner/src/live/runnerd-codex-transport.ts`: `backend.kind` is `"runner"` in `RetainedRunnerdMaintenanceInput` and `RetainedRunnerdCleanupProof`. `name` stays a string. - `packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts`: `close` takes `{ reason: string; force?: boolean }`, as the contract does. The body still reads `input?.reason`, because a generated test script calls `close()` with no arguments. Codex closes the same way with or without `force`. - `packages/paperclip-runner/src/live/runnerd-contract-types.test.ts`: new type test. The Runner test typecheck fails if either type accepts another backend kind, or if the Codex `close` parameters differ from the contract. With the master types, this file has 4 type errors. Not in this pull request: #18839 also listed that `PrpEvent.normalizedSessionId` is required in the type but optional in the event schemas. A separate pull request handles it in `validatePrpEvent`. ## Verification - `pnpm exec vitest run src/live/runnerd-contract-types.test.ts` (in `packages/paperclip-runner`): passes 2 tests. - `tsc --noEmit` passes in `packages/paperclip-runner` for `tsconfig.json`, `tsconfig.surfaces.json`, `tsconfig.test.json` and `tsconfig.surfaces.test.json`. - `cd server && pnpm typecheck`: passes. The server builds the maintenance input with the `"runner"` literal. - In `packages/paperclip-runner`, after `pnpm run test:typescript:prep`: `pnpm exec vitest run src/live/runnerd-codex-transport-settlement.test.ts src/drivers/codex/codex-protocol-cleanup.test.ts src/drivers/codex/codex-app-server-driver.lifecycle.test.ts` passes 3 files and 43 tests. - Not run locally: the full Runner `pnpm test` (including Rust tests) and the root `pnpm build`. Required CI covers them. ## Risks - Low risk. The change affects types only, plus one comment in the Codex session. Runtime behavior does not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits) and extended thinking. ## 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 - [ ] 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 --- .../drivers/codex/codex-harness-session.ts | 4 +++- .../src/live/runnerd-codex-transport.ts | 5 +++-- .../src/live/runnerd-contract-types.test.ts | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 packages/paperclip-runner/src/live/runnerd-contract-types.test.ts diff --git a/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts b/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts index fdde2d14d90..6bbbe3e7585 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts @@ -886,9 +886,11 @@ export class CodexHarnessSession }; } - async close(input?: { reason: string }): Promise { + // `force` is part of the contract. Codex closes the same way with or without it. + async close(input: { reason: string; force?: boolean }): Promise { this.cancelPendingRequests("session_closed"); this.eventQueue.close(); + // Untyped callers (for example generated scripts) may still omit the input. await this.transport.close(input?.reason); } diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 2f19d3aa461..503ef9cf878 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -2104,7 +2104,8 @@ export function readRunnerdArtifactBinding(runnerBinaryPath: string): { export interface RetainedRunnerdCleanupProof { readonly binding: Readonly; readonly identity: Readonly; - readonly backend: Readonly<{ kind: string; name: string }>; + /** Must match the quarantine domain, which every production backend builds with kind "runner". */ + readonly backend: Readonly<{ kind: "runner"; name: string }>; readonly providerSessionId: string; readonly activationDirectory: string; readonly sourceFingerprint: string; @@ -2400,7 +2401,7 @@ export interface RetainedRunnerdMaintenanceInput { requestId: string; binding: NativeRunIdentity; identity: DurableRecoveryIdentity; - backend: { kind: string; name: string }; + backend: { kind: "runner"; name: string }; stateDirectory: string; activationDirectory: string; sourceFingerprint: string; diff --git a/packages/paperclip-runner/src/live/runnerd-contract-types.test.ts b/packages/paperclip-runner/src/live/runnerd-contract-types.test.ts new file mode 100644 index 00000000000..09f52dacba0 --- /dev/null +++ b/packages/paperclip-runner/src/live/runnerd-contract-types.test.ts @@ -0,0 +1,22 @@ +import { describe, expectTypeOf, it } from "vitest"; +import type { HarnessSession } from "../contracts/harness-driver.js"; +import type { CodexHarnessSession } from "../drivers/codex/codex-harness-session.js"; +import type { RetainedRunnerdCleanupProof, RetainedRunnerdMaintenanceInput } from "./runnerd-codex-transport.js"; + +// The Runner test typecheck checks these assertions. Vitest only runs them. +describe("Runner contract types", () => { + it("accepts only the runner backend kind for retained cleanup", () => { + // The quarantine domain uses the descriptor kind, which is "runner" for every + // production backend. Another kind would release nothing. + expectTypeOf().toEqualTypeOf<"runner">(); + expectTypeOf().toEqualTypeOf<"runner">(); + + // @ts-expect-error: a backend kind other than "runner" is not allowed. + const remote: RetainedRunnerdMaintenanceInput["backend"] = { kind: "remote", name: "codex_app_server" }; + void remote; + }); + + it("matches the HarnessSession close contract in the Codex session", () => { + expectTypeOf>().toEqualTypeOf>(); + }); +});