Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/auth/codex/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
61 changes: 55 additions & 6 deletions src/tui/deliver-agent-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand All @@ -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;
Expand All @@ -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<void>((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);
});
});
11 changes: 10 additions & 1 deletion src/tui/deliver-agent-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,20 @@
*/
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<void>;
deliverToLiveAgent: () => void;
onDeliverFailure: (message: string) => void;
}

export function deliverAgentMessage(deps: DeliverAgentMessageDeps): void {
export async function deliverAgentMessage(deps: DeliverAgentMessageDeps): Promise<void> {
if (deps.ready !== undefined) await deps.ready;
const fatal = deps.getFatalBuildError();
if (fatal !== null) {
deps.onDeliverFailure(`Message not delivered: ${fatal.message}`);
Expand Down
32 changes: 21 additions & 11 deletions src/tui/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,25 @@ export async function runTUI(initialConfig: Config): Promise<number> {
{ 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<void> =
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();
Expand All @@ -1327,8 +1346,9 @@ export async function runTUI(initialConfig: Config): Promise<number> {
// 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,
});
Expand Down Expand Up @@ -1381,16 +1401,6 @@ export async function runTUI(initialConfig: Config): Promise<number> {
// 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;
Expand Down
Loading