Skip to content

Commit 04b767b

Browse files
Merge pull request #526 from corbitsdev/cl-6693-refresh-codex-instructions-on-exec-and-log-eval-request
Refresh Codex instructions on exec boot and record eval diagnostics
2 parents 55f38fa + bc1125a commit 04b767b

7 files changed

Lines changed: 299 additions & 5 deletions

File tree

evals/capability/lib.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ function sampleResult(over: Partial<CaseResult> = {}): CaseResult {
7171
repeat: over.repeat ?? 0,
7272
behaviors: over.behaviors ?? null,
7373
providerFallback: over.providerFallback ?? null,
74+
diagnostics: over.diagnostics ?? null,
7475
};
7576
}
7677

@@ -755,6 +756,38 @@ describe("parseEvalRunReport", () => {
755756
});
756757
});
757758

759+
test("round-trips diagnostics stamping on a case result", () => {
760+
const report = parseEvalRunReport({
761+
version: 3,
762+
provider: "xai",
763+
model: "grok",
764+
cases: [
765+
sampleResult({
766+
diagnostics: {
767+
codexInstructionsHash: "abc123def456",
768+
advertisedTools: ["read_file", "run_shell"],
769+
reasoningEffort: "high",
770+
},
771+
}),
772+
],
773+
});
774+
expect(report.cases[0]!.diagnostics).toEqual({
775+
codexInstructionsHash: "abc123def456",
776+
advertisedTools: ["read_file", "run_shell"],
777+
reasoningEffort: "high",
778+
});
779+
});
780+
781+
test("legacy reports with no diagnostics parse to null", () => {
782+
const report = parseEvalRunReport({
783+
version: 3,
784+
provider: "xai",
785+
model: "grok",
786+
cases: [sampleResult()],
787+
});
788+
expect(report.cases[0]!.diagnostics).toBeNull();
789+
});
790+
758791
test("legacy reports default repeat 0 and null behaviors", () => {
759792
const report = parseEvalRunReport({
760793
version: 2,

evals/capability/lib.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,19 @@ export type CaseResult = {
134134
behaviors: BehaviorMetrics | null;
135135
/** Set when the resolved provider/model differed from what was requested. */
136136
providerFallback: ProviderFallbackInfo | null;
137+
/** Per-cell diagnostics for debugging eval failures; null when unavailable. */
138+
diagnostics: EvalDiagnostics | null;
137139
textPreview?: string;
138140
};
139141

142+
export type EvalDiagnostics = {
143+
/** Short identity (hash) of the pinned Codex instructions text in use; null for non-Codex providers. */
144+
codexInstructionsHash: string | null;
145+
/** Built-in tool names advertised to the model for this run. */
146+
advertisedTools: readonly string[];
147+
reasoningEffort: string | null;
148+
};
149+
140150
export type MetricStats = {
141151
min: number;
142152
median: number;
@@ -726,10 +736,23 @@ function parseCaseResult(raw: unknown): CaseResult {
726736
: 0,
727737
behaviors: parseBehaviorMetrics(raw.behaviors),
728738
providerFallback: parseProviderFallback(raw.providerFallback),
739+
diagnostics: parseEvalDiagnostics(raw.diagnostics),
729740
...(typeof raw.textPreview === "string" ? { textPreview: raw.textPreview } : {}),
730741
};
731742
}
732743

744+
function parseEvalDiagnostics(raw: unknown): EvalDiagnostics | null {
745+
if (!isRecord(raw)) return null;
746+
if (!Array.isArray(raw.advertisedTools)) return null;
747+
const advertisedTools = raw.advertisedTools.filter((t): t is string => typeof t === "string");
748+
return {
749+
codexInstructionsHash:
750+
typeof raw.codexInstructionsHash === "string" ? raw.codexInstructionsHash : null,
751+
advertisedTools,
752+
reasoningEffort: typeof raw.reasoningEffort === "string" ? raw.reasoningEffort : null,
753+
};
754+
}
755+
733756
function parseProviderFallback(raw: unknown): ProviderFallbackInfo | null {
734757
if (!isRecord(raw)) return null;
735758
const resolvedProvider = raw.resolvedProvider;

scripts/eval-capability.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,32 @@ import { join } from "node:path";
55
import { execFile } from "node:child_process";
66
import { promisify } from "node:util";
77

8-
import { initEvalGitRepo, mapPool, parseArgs } from "./eval-capability.ts";
8+
import { initEvalGitRepo, mapPool, parseArgs, buildEvalDiagnostics } from "./eval-capability.ts";
9+
import type { Config } from "../src/config/index.ts";
910

1011
const execFileAsync = promisify(execFile);
1112

13+
function sampleConfig(over: Partial<Config> = {}): Config {
14+
return {
15+
configured: true,
16+
apiKey: "key",
17+
baseURL: "https://example.test",
18+
model: "gpt-5",
19+
providerName: "openai",
20+
cwd: process.cwd(),
21+
task: "do it",
22+
force: true,
23+
dangerouslySkipPermissions: true,
24+
skipPermissionsFromSettings: false,
25+
auto: false,
26+
command: "exec",
27+
globalSettingsPath: "/dev/null",
28+
providers: [],
29+
sessionId: "sess-1",
30+
...over,
31+
} as Config;
32+
}
33+
1234
describe("parseArgs", () => {
1335
const savedConcurrency = process.env.CORBITS_EVAL_CONCURRENCY;
1436

@@ -232,3 +254,30 @@ describe("initEvalGitRepo", () => {
232254
}
233255
});
234256
});
257+
258+
describe("buildEvalDiagnostics", () => {
259+
test("non-Codex provider gets a null instructions hash and the default orchestrator tool list", async () => {
260+
const diagnostics = await buildEvalDiagnostics(sampleConfig({ providerName: "openai" }));
261+
expect(diagnostics.codexInstructionsHash).toBeNull();
262+
expect(diagnostics.advertisedTools).toContain("read_file");
263+
expect(diagnostics.advertisedTools).toContain("run_shell");
264+
expect(diagnostics.reasoningEffort).toBeNull();
265+
});
266+
267+
test("Codex provider gets a non-null instructions hash", async () => {
268+
const diagnostics = await buildEvalDiagnostics(sampleConfig({ providerName: "codex/default" }));
269+
expect(diagnostics.codexInstructionsHash).toMatch(/^[0-9a-f]{12}$/);
270+
});
271+
272+
test("echoes back the configured reasoning effort", async () => {
273+
const diagnostics = await buildEvalDiagnostics(sampleConfig({ reasoningEffort: "high" }));
274+
expect(diagnostics.reasoningEffort).toBe("high");
275+
});
276+
277+
test("--director build reports the director's own advertised allowlist", async () => {
278+
const diagnostics = await buildEvalDiagnostics(sampleConfig({ director: "build" }));
279+
expect(diagnostics.advertisedTools).not.toEqual(
280+
(await buildEvalDiagnostics(sampleConfig({}))).advertisedTools,
281+
);
282+
});
283+
});

scripts/eval-capability.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@ import { tmpdir } from "node:os";
1414
import { join, dirname, resolve } from "node:path";
1515
import { fileURLToPath } from "node:url";
1616
import { spawn } from "node:child_process";
17-
import { loadConfig } from "../src/config/index.js";
18-
import { runExec } from "../src/exec/runner.js";
17+
import { loadConfig, type Config } from "../src/config/index.js";
18+
import { runExec, resolveExecDirectorOverlay } from "../src/exec/runner.js";
1919
import { SETTINGS_DIR_NAME } from "../src/branding.js";
20+
import { codexProfileFromProviderName } from "../src/config/codex-providers.js";
21+
import { codexInstructionsHash } from "../src/auth/codex/instructions.js";
22+
import { advertisedToolNamesForSessionMode } from "../src/agent/tool-search.js";
23+
import { detectLanguageServerAvailable } from "../src/agent/lsp-availability.js";
24+
import { resolveSessionMode } from "../src/config/session-mode.js";
25+
import { loadLocalSettings, localSettingsPath } from "../src/config/settings.js";
2026
import {
2127
loadEvalCases,
2228
filterCases,
@@ -41,6 +47,7 @@ import {
4147
type EvalVariant,
4248
type EvalTokenUsage,
4349
type ProviderFallbackInfo,
50+
type EvalDiagnostics,
4451
} from "../evals/capability/lib.js";
4552
import {
4653
deriveBehaviorMetrics,
@@ -531,6 +538,34 @@ async function resolveVariantLabels(
531538
};
532539
}
533540

541+
/**
542+
* Per-cell diagnostics for debugging eval failures: which Codex instructions
543+
* text was pinned, which built-in tools the model was offered, and the
544+
* requested reasoning effort. Reuses the exec runner's own resolution
545+
* (resolveSessionMode, resolveExecDirectorOverlay) rather than forking the
546+
* logic, so a --director overlay or a non-default session mode here reports
547+
* the same advertised list exec actually runs with.
548+
*
549+
* reasoningEffort echoes the configured value, not the provider's internal
550+
* default when unset — accepted as-is per review.
551+
*/
552+
export async function buildEvalDiagnostics(config: Config): Promise<EvalDiagnostics> {
553+
const codexProfile = codexProfileFromProviderName(config.providerName);
554+
const localSettings = await loadLocalSettings(localSettingsPath(config.cwd)).catch(() => null);
555+
const sessionMode = resolveSessionMode(config.settings, localSettings) ?? "orchestrator";
556+
const overlay = resolveExecDirectorOverlay(config.director);
557+
const advertisedTools =
558+
overlay.advertisedAllow ??
559+
advertisedToolNamesForSessionMode(sessionMode, {
560+
languageServerAvailable: detectLanguageServerAvailable(config.cwd),
561+
});
562+
return {
563+
codexInstructionsHash: codexProfile !== undefined ? codexInstructionsHash() : null,
564+
advertisedTools,
565+
reasoningEffort: config.reasoningEffort ?? null,
566+
};
567+
}
568+
534569
function failResult(
535570
caseDef: EvalCase,
536571
variant: EvalVariant,
@@ -568,6 +603,7 @@ function failResult(
568603
repeat,
569604
behaviors: null,
570605
providerFallback: null,
606+
diagnostics: null,
571607
...partial,
572608
};
573609
}
@@ -633,6 +669,7 @@ async function runCase(
633669
);
634670
}
635671

672+
const diagnostics = await buildEvalDiagnostics(config);
636673
const agentStarted = Date.now();
637674
// runExec runs the agent in-process (no child, unlike verify.sh below), so
638675
// the fixture origin must reach it via process.env directly for the
@@ -770,6 +807,7 @@ async function runCase(
770807
repeat,
771808
behaviors,
772809
providerFallback,
810+
diagnostics,
773811
textPreview,
774812
};
775813
} catch (err) {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
2+
3+
// Reused by both the TUI and exec boot paths (src/tui/runner.ts,
4+
// src/exec/runner.ts) to refresh the pinned Codex instructions before first
5+
// Codex inference. This exercises the shared refresh/fallback logic directly,
6+
// with disk I/O faked so tests never touch the real ~/.corbits cache.
7+
8+
let fakeDisk = new Map<string, string>();
9+
10+
mock.module("node:fs", () => ({
11+
readFileSync: (path: string) => {
12+
const contents = fakeDisk.get(path);
13+
if (contents === undefined) {
14+
const err = new Error("ENOENT") as NodeJS.ErrnoException;
15+
err.code = "ENOENT";
16+
throw err;
17+
}
18+
return contents;
19+
},
20+
writeFileSync: (path: string, contents: string) => {
21+
fakeDisk.set(path, contents);
22+
},
23+
mkdirSync: () => undefined,
24+
}));
25+
26+
const { refreshCodexInstructions, codexInstructions, codexInstructionsHash } = await import(
27+
"./instructions.js"
28+
);
29+
const { GPT_5_CODEX_PROMPT } = await import("./prompts/gpt-5-codex.js");
30+
31+
const VALID_PROMPT = `You are Codex${"x".repeat(1200)}`;
32+
33+
function mockFetchSequence(tag: string, promptResponse: () => Response): typeof fetch {
34+
return (async (input: RequestInfo | URL) => {
35+
const url = String(input);
36+
if (url.includes("releases/latest")) {
37+
return new Response(JSON.stringify({ tag_name: tag }), { status: 200 });
38+
}
39+
return promptResponse();
40+
}) as typeof fetch;
41+
}
42+
43+
describe("refreshCodexInstructions", () => {
44+
const originalFetch = global.fetch;
45+
46+
beforeEach(() => {
47+
fakeDisk = new Map();
48+
});
49+
50+
afterEach(() => {
51+
global.fetch = originalFetch;
52+
});
53+
54+
test("bundled copy is used before any refresh", () => {
55+
expect(codexInstructions()).toBe(GPT_5_CODEX_PROMPT);
56+
});
57+
58+
test("updates in-memory instructions on a successful fetch", async () => {
59+
global.fetch = mockFetchSequence("rust-v1.2.3", () => new Response(VALID_PROMPT, { status: 200 }));
60+
61+
await refreshCodexInstructions();
62+
expect(codexInstructions()).toBe(VALID_PROMPT);
63+
});
64+
65+
test("falls back without throwing the run when the release lookup network call fails", async () => {
66+
const before = codexInstructions();
67+
global.fetch = (() => Promise.reject(new Error("network down"))) as unknown as typeof fetch;
68+
69+
// The function itself rejects; callers (TUI/exec boot) catch this and
70+
// keep running on cache/bundled instructions — see src/exec/runner.ts and
71+
// src/tui/runner.ts refresh call sites.
72+
await expect(refreshCodexInstructions()).rejects.toThrow("network down");
73+
expect(codexInstructions()).toBe(before);
74+
});
75+
76+
test("falls back without throwing when the prompt fetch returns a non-200", async () => {
77+
const before = codexInstructions();
78+
global.fetch = mockFetchSequence("rust-v1.2.3", () => new Response("not found", { status: 404 }));
79+
80+
await expect(refreshCodexInstructions()).rejects.toThrow(/HTTP 404/);
81+
expect(codexInstructions()).toBe(before);
82+
});
83+
84+
test("rejects a 200 response whose body is not a valid Codex prompt (CDN error page)", async () => {
85+
const before = codexInstructions();
86+
global.fetch = mockFetchSequence("rust-v1.2.3", () => new Response("<html>oops</html>", { status: 200 }));
87+
88+
await expect(refreshCodexInstructions()).rejects.toThrow(/unexpected body/);
89+
expect(codexInstructions()).toBe(before);
90+
});
91+
92+
test("rejects within the timeout when a fetch never resolves (hung connection)", async () => {
93+
const before = codexInstructions();
94+
global.fetch = ((_input: RequestInfo | URL, init?: RequestInit) => {
95+
return new Promise((_resolve, reject) => {
96+
const signal = init?.signal;
97+
if (signal) {
98+
signal.addEventListener("abort", () => reject(signal.reason as Error));
99+
}
100+
});
101+
}) as unknown as typeof fetch;
102+
103+
const started = Date.now();
104+
await expect(refreshCodexInstructions()).rejects.toBeTruthy();
105+
expect(Date.now() - started).toBeLessThan(15_000);
106+
expect(codexInstructions()).toBe(before);
107+
}, 20_000);
108+
109+
test("codexInstructionsHash reflects the currently resolved instructions text", async () => {
110+
const hashBefore = codexInstructionsHash();
111+
expect(hashBefore).toMatch(/^[0-9a-f]{12}$/);
112+
113+
const otherPrompt = `You are Codex${"y".repeat(1200)}`;
114+
global.fetch = mockFetchSequence("rust-v1.2.4", () => new Response(otherPrompt, { status: 200 }));
115+
await refreshCodexInstructions();
116+
117+
expect(codexInstructionsHash()).not.toBe(hashBefore);
118+
});
119+
});

0 commit comments

Comments
 (0)