From b0a5a4980fce6516176d45078c00625741d4790f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 22 Aug 2026 19:01:22 -0700 Subject: [PATCH] Gate TUI deliver on Codex instructions refresh The TUI fired refreshCodexInstructions() without awaiting it, so the in-memory instructions singleton could swap mid-session after inference #1 started. Since codex-responses-adapter puts the instructions at the top of every request body, that swap changes turn #2's whole prefix and forces a guaranteed prompt-cache miss at a nondeterministic point. Hold the refresh promise and gate every queued deliver on it instead of blocking TUI startup: enqueueAgentDeliver now awaits it via deliverAgentMessage's new ready param before building the first request, matching the await in src/exec/runner.ts. The existing .catch fallback is unchanged, so a failed refresh still leaves the session on cached/bundled instructions rather than crashing. --- src/auth/codex/instructions.ts | 3 +- src/tui/deliver-agent-message.test.ts | 61 ++++++++++++++++++++++++--- src/tui/deliver-agent-message.ts | 15 +++++-- src/tui/runner.ts | 32 +++++++++----- 4 files changed, 90 insertions(+), 21 deletions(-) diff --git a/src/auth/codex/instructions.ts b/src/auth/codex/instructions.ts index e53c502c2..7fa9ac56d 100644 --- a/src/auth/codex/instructions.ts +++ b/src/auth/codex/instructions.ts @@ -21,7 +21,8 @@ const PROMPT_SENTINEL = "You are Codex"; const MIN_PROMPT_LENGTH = 1000; // Bounds both network calls below so a black-holed connection can never hang -// exec boot, which awaits refreshCodexInstructions before first inference. +// exec boot or the TUI's first deliver, both of which wait on +// refreshCodexInstructions settling before first inference. const CODEX_INSTRUCTIONS_TIMEOUT_MS = 10_000; export function isValidCodexPrompt(text: string): boolean { diff --git a/src/tui/deliver-agent-message.test.ts b/src/tui/deliver-agent-message.test.ts index c34143bb7..6504ec5b9 100644 --- a/src/tui/deliver-agent-message.test.ts +++ b/src/tui/deliver-agent-message.test.ts @@ -2,12 +2,12 @@ import { describe, expect, test } from "bun:test"; import { deliverAgentMessage } from "./deliver-agent-message.js"; describe("deliverAgentMessage", () => { - test("surfaces a not-delivered notice instead of throwing when the agent is mid-rebuild", () => { + test("surfaces a not-delivered notice instead of throwing when the agent is mid-rebuild", async () => { const notices: string[] = []; const fatal = new Error("agent rebuild failed: provider unreachable"); let delivered = false; - deliverAgentMessage({ + await deliverAgentMessage({ getFatalBuildError: () => fatal, deliverToLiveAgent: () => { delivered = true; @@ -24,10 +24,10 @@ describe("deliverAgentMessage", () => { expect(notices[0]).toContain("provider unreachable"); }); - test("surfaces a not-delivered notice when the live agent throws on delivery", () => { + test("surfaces a not-delivered notice when the live agent throws on delivery", async () => { const notices: string[] = []; - deliverAgentMessage({ + await deliverAgentMessage({ getFatalBuildError: () => null, deliverToLiveAgent: () => { throw new Error("agent is closed"); @@ -40,11 +40,11 @@ describe("deliverAgentMessage", () => { expect(notices[0]).toContain("agent is closed"); }); - test("delivers normally and stays silent when the agent is healthy", () => { + test("delivers normally and stays silent when the agent is healthy", async () => { const notices: string[] = []; let delivered = false; - deliverAgentMessage({ + await deliverAgentMessage({ getFatalBuildError: () => null, deliverToLiveAgent: () => { delivered = true; @@ -55,4 +55,53 @@ describe("deliverAgentMessage", () => { expect(delivered).toBe(true); expect(notices).toHaveLength(0); }); + + test("holds delivery until ready settles so the first request cannot race startup work", async () => { + // Models the Codex instructions refresh: the request body prefix pins the + // instructions text, so delivery — and therefore request building — must + // wait for the in-flight refresh instead of letting it swap the value + // between turn 1 and turn 2. + let instructions = "cached"; + let observedAtDelivery = ""; + let resolveReady!: () => void; + const ready = new Promise((resolve) => { + resolveReady = resolve; + }); + + const pending = deliverAgentMessage({ + getFatalBuildError: () => null, + ready, + deliverToLiveAgent: () => { + observedAtDelivery = instructions; + }, + onDeliverFailure: () => undefined, + }); + + // Refresh still in flight: nothing delivered yet. + await Promise.resolve(); + expect(observedAtDelivery).toBe(""); + + instructions = "refreshed"; + resolveReady(); + await pending; + + // The first request observed the post-refresh value, so a later-settling + // refresh can never change the prefix of an already-started session. + expect(observedAtDelivery).toBe("refreshed"); + }); + + test("delivers immediately when ready is already settled", async () => { + let delivered = false; + + await deliverAgentMessage({ + getFatalBuildError: () => null, + ready: Promise.resolve(), + deliverToLiveAgent: () => { + delivered = true; + }, + onDeliverFailure: () => undefined, + }); + + expect(delivered).toBe(true); + }); }); diff --git a/src/tui/deliver-agent-message.ts b/src/tui/deliver-agent-message.ts index c41e52675..c1764e1d2 100644 --- a/src/tui/deliver-agent-message.ts +++ b/src/tui/deliver-agent-message.ts @@ -4,13 +4,22 @@ * here must be surfaced — a swallowed error here means the transcript claims * delivery for a message that never reached the agent. */ -export type DeliverAgentMessageDeps = { +export interface DeliverAgentMessageDeps { getFatalBuildError: () => Error | null; + /** + * Settles before any delivery. Gates the session's first request on startup + * work that must not race request building — the Codex instructions refresh: + * a late in-memory swap would change the request prefix and forfeit the + * provider prompt cache for the rest of the session. Must never reject + * (callers attach their own fallback handling). + */ + ready?: Promise; deliverToLiveAgent: () => void; onDeliverFailure: (message: string) => void; -}; +} -export function deliverAgentMessage(deps: DeliverAgentMessageDeps): void { +export async function deliverAgentMessage(deps: DeliverAgentMessageDeps): Promise { + if (deps.ready !== undefined) await deps.ready; const fatal = deps.getFatalBuildError(); if (fatal !== null) { deps.onDeliverFailure(`Message not delivered: ${fatal.message}`); diff --git a/src/tui/runner.ts b/src/tui/runner.ts index f994d39da..f5dff513e 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -1282,6 +1282,25 @@ export async function runTUI(initialConfig: Config): Promise { { providerName: config.providerName, model: config.model }, ); + const initialCodexProfile = codexProfileFromProviderName(config.providerName); + const initialXaiProfile = xaiProfileFromProviderName(config.providerName); + + // Refresh the pinned Codex instructions without blocking TUI startup. Every + // deliver below awaits settlement, so the first request never races the + // refresh: codex-responses-adapter places the instructions at the top of + // every request body, and an in-memory swap after inference #1 would change + // the whole prefix and forfeit the provider prompt cache mid-session. + // Best-effort, same as exec boot: on failure the session runs on + // cached/bundled instructions. + const codexInstructionsRefreshed: Promise = + initialCodexProfile === undefined + ? Promise.resolve() + : refreshCodexInstructions().catch((err: unknown) => { + tuiLogger.warn("Codex instructions refresh failed: {error}", { + error: err instanceof Error ? err.message : String(err), + }); + }); + // Reload, interrupt, compaction continuation, and proxy deliver share one queue // so a rebuild never races an in-flight deliver. const sessionOps = createSessionOperationQueue(); @@ -1290,8 +1309,9 @@ export async function runTUI(initialConfig: Config): Promise { // The shell already popped the queue item and painted it as delivered // by the time this runs, so a failed rebuild must be surfaced here — // otherwise the message silently never reaches the agent. - deliverAgentMessage({ + await deliverAgentMessage({ getFatalBuildError: () => fatalBuildError, + ready: codexInstructionsRefreshed, deliverToLiveAgent, onDeliverFailure: systemNotice, }); @@ -1340,16 +1360,6 @@ export async function runTUI(initialConfig: Config): Promise { // When the session starts on a Codex profile, seed the agent with a Responses // source (account id pulled from the resolved catalog entry, session id from // the run) rather than the OpenAI-compatible one. - const initialCodexProfile = codexProfileFromProviderName(config.providerName); - const initialXaiProfile = xaiProfileFromProviderName(config.providerName); - if (initialCodexProfile !== undefined) { - void refreshCodexInstructions().catch((err: unknown) => { - // Best-effort; agent still starts with cached/default instructions. - tuiLogger.warn("Codex instructions refresh failed: {error}", { - error: err instanceof Error ? err.message : String(err), - }); - }); - } const initialCodexAccountId = config.providers.find((p) => p.name === config.providerName)?.codexAccountId; const buildOpenAICompatibleInitialSource = (): InferenceSource => buildOpenAISource({