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
37 changes: 28 additions & 9 deletions src/cli/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1505,22 +1505,41 @@ export async function runDoctor(args: string[] = []): Promise<void> {
// cannot break a legitimately green pipeline.

console.log("\nCodex agent role files");
const tomlFallbackRoles = scanCodexAgentRolesWithTomlModelFallback(resolveCodexHomeDirImpl());
if (tomlFallbackRoles.length === 0) {
let roleScanError: unknown;
const tomlFallbackRoles = scanCodexAgentRolesWithTomlModelFallback(
resolveCodexHomeDirImpl(),
cause => {
roleScanError = cause;
},
);
if (roleScanError) {
console.log(` [WARN] unable to scan $CODEX_HOME/agents/*.toml: ${String(roleScanError)}`);
} else if (tomlFallbackRoles.length === 0) {
console.log(" ok no per-role model_fallback fields in $CODEX_HOME/agents/*.toml");
} else {
console.log(` [WARN] ${tomlFallbackRoles.length} agent role file${tomlFallbackRoles.length === 1 ? "" : "s"} contain${tomlFallbackRoles.length === 1 ? "s" : ""} \`model_fallback\`: ${tomlFallbackRoles.join(", ")}`);
console.log(" Codex >= 0.146 rejects that field as unknown and skips the whole role. Move the chains to opencodex config `subagentModelFallbackByModel` (keyed by primary model) and remove the field from the TOML files.");
}
// opencodex does not write these files; the Codex desktop external-agent import does, and it
// drops the model pin on the way in. Observe-only: doctor never repairs or removes them.
const unpinnedDerivedRoles = scanOpencodexDerivedCodexAgentRolesWithoutModelPin(resolveCodexHomeDirImpl());
if (unpinnedDerivedRoles.length === 0) {
console.log(" ok every opencodex-derived role file in $CODEX_HOME/agents/*.toml pins a model");
} else {
console.log(` [WARN] ${unpinnedDerivedRoles.length} opencodex-derived role file${unpinnedDerivedRoles.length === 1 ? "" : "s"} without a \`model\` pin: ${unpinnedDerivedRoles.map(role => `${role}.toml`).join(", ")}`);
console.log(" Codex runs these roles on the parent model, so a spawn records one role and another model. The `ocx-route` directive in the file cannot pin them: it is honoured only on the Claude Code `/v1/messages` path and is inert on `/v1/responses`.");
console.log(" Add `model = \"<id>\"` to each file, or remove them. They usually come from the Codex desktop external-agent import of ~/.claude/agents/ocx-*.md; set `[desktop] external-agent-import-sync-item-types` with `SUBAGENTS = false` to stop it recreating them.");
// Both role scans share the same directory listing, so a failure above already reported the
// cause; skip the second scan rather than warn twice or print a false "ok".
if (!roleScanError) {
const unpinnedDerivedRoles = scanOpencodexDerivedCodexAgentRolesWithoutModelPin(
resolveCodexHomeDirImpl(),
cause => {
roleScanError = cause;
},
);
if (roleScanError) {
console.log(` [WARN] unable to scan $CODEX_HOME/agents/*.toml: ${String(roleScanError)}`);
} else if (unpinnedDerivedRoles.length === 0) {
console.log(" ok every opencodex-derived role file in $CODEX_HOME/agents/*.toml pins a model");
} else {
console.log(` [WARN] ${unpinnedDerivedRoles.length} opencodex-derived role file${unpinnedDerivedRoles.length === 1 ? "" : "s"} without a \`model\` pin: ${unpinnedDerivedRoles.map(role => `${role}.toml`).join(", ")}`);
console.log(" Codex runs these roles on the parent model, so a spawn records one role and another model. The `ocx-route` directive in the file cannot pin them: it is honoured only on the Claude Code `/v1/messages` path and is inert on `/v1/responses`.");
console.log(" Add `model = \"<id>\"` to each file, or remove them. They usually come from the Codex desktop external-agent import of ~/.claude/agents/ocx-*.md; set `[desktop] external-agent-import-sync-item-types` with `SUBAGENTS = false` to stop it recreating them.");
}
}

const dual = collectWslDualInstall();
Expand Down
26 changes: 22 additions & 4 deletions src/codex/subagent-model-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -905,8 +905,16 @@ export function hasCodexAgentModelFallbackField(role: string, codexHome = CODEX_
}

/** Roles whose TOML still carries `model_fallback`, including empty arrays. */
export function scanCodexAgentRolesWithTomlModelFallback(codexHome = CODEX_HOME): string[] {
return listCodexAgentRoles(codexHome).filter(role => hasCodexAgentModelFallbackField(role, codexHome));
export function scanCodexAgentRolesWithTomlModelFallback(
codexHome = CODEX_HOME,
onListError?: (cause: unknown) => void,
): string[] {
try {
return listCodexAgentRoles(codexHome).filter(role => hasCodexAgentModelFallbackField(role, codexHome));
} catch (cause) {
onListError?.(cause);
return [];
}
}

const TOML_MODEL_KEY = /^\s*(?:model|"model"|'model')\s*=/;
Expand Down Expand Up @@ -984,9 +992,19 @@ const OPENCODEX_DERIVED_ROLE_MARKERS = ["generated-by: opencodex", "ocx-route:"]
* authorizes writing to, repairing, or removing these files, and the marker-based ownership rules
* that govern the files opencodex does write are unchanged.
*/
export function scanOpencodexDerivedCodexAgentRolesWithoutModelPin(codexHome = CODEX_HOME): string[] {
export function scanOpencodexDerivedCodexAgentRolesWithoutModelPin(
codexHome = CODEX_HOME,
onListError?: (cause: unknown) => void,
): string[] {
const findings: string[] = [];
for (const role of listCodexAgentRoles(codexHome)) {
let roles: string[];
try {
roles = listCodexAgentRoles(codexHome);
} catch (cause) {
onListError?.(cause);
return [];
}
for (const role of roles) {
let content: string;
try {
content = readFileSync(join(codexHome, "agents", `${role}.toml`), "utf8");
Expand Down
41 changes: 41 additions & 0 deletions tests/codex-integration/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,47 @@ describe("doctor version skew projection", () => {
removeTreeWithRetry(home);
}
}, STORE_BUDGET_MS);

test("a malformed CODEX_HOME/agents path warns instead of aborting the report", async () => {
const home = mkdtempSync(join(tmpdir(), "ocx-doctor-agents-"));
const codexHome = join(home, "codex");
const previousHome = process.env.OPENCODEX_HOME;
const previousCodexHome = process.env.CODEX_HOME;
const previousExitCode = process.exitCode;
const restore: Array<() => void> = [];
try {
mkdirSync(codexHome, { recursive: true });
process.env.OPENCODEX_HOME = home;
process.env.CODEX_HOME = codexHome;
writeFileSync(join(home, "config.json"), JSON.stringify({ ...getDefaultConfig(), port: 9, codexAutoStart: false }));
// A regular file where the role scan expects a readable directory.
writeFileSync(join(codexHome, "agents"), "not a directory");
const logged: string[] = [];
const log = spyOn(console, "log").mockImplementation((...args: unknown[]) => { logged.push(args.map(String).join(" ")); });
restore.push(() => log.mockRestore());
// Other doctor sections probe upstream health; this diagnostic fixture must stay offline.
const fetch = spyOn(globalThis, "fetch").mockImplementation(async () => new Response(null, { status: 503 }));
restore.push(() => fetch.mockRestore());
const live = spyOn(proxyLiveness, "findLiveProxy").mockResolvedValue({
pid: null, port: 9, hostname: "127.0.0.1", source: "config",
});
restore.push(() => live.mockRestore());
await runDoctor([]);
const output = logged.join("\n");
expect(output).toContain("[WARN] unable to scan $CODEX_HOME/agents/*.toml:");
expect(output).not.toContain("no per-role model_fallback fields");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// The dependent derived-role scan shares the listing; a missing guard would warn twice.
expect(output.match(/\[WARN\] unable to scan \$CODEX_HOME\/agents\/\*\.toml:/g) ?? []).toHaveLength(1);
} finally {
for (const cleanup of restore.reverse()) cleanup();
process.exitCode = previousExitCode;
if (previousHome === undefined) delete process.env.OPENCODEX_HOME;
else process.env.OPENCODEX_HOME = previousHome;
if (previousCodexHome === undefined) delete process.env.CODEX_HOME;
else process.env.CODEX_HOME = previousCodexHome;
removeTreeWithRetry(home);
}
}, STORE_BUDGET_MS);
});

describe("doctor reclaim wiring (end to end)", () => {
Expand Down
14 changes: 13 additions & 1 deletion tests/routing/subagent-model-fallback.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test";
import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import {
Expand Down Expand Up @@ -1337,6 +1337,18 @@ test("the native-main drain sentinel covers the flagships without widening to gp
expect(readCodexAgentModelFallback("empty_fallback", dir)).toEqual([]);
});

test("scanCodexAgentRolesWithTomlModelFallback tolerates a malformed agents path", () => {
const dir = codexHomeFixture();
rmSync(join(dir, "agents"), { recursive: true });
writeFileSync(join(dir, "agents"), "not a directory", "utf8");
let scanError: unknown;

expect(scanCodexAgentRolesWithTomlModelFallback(dir, cause => {
scanError = cause;
})).toEqual([]);
expect(scanError).toBeInstanceOf(Error);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

test("scanCodexAgentRolesWithTomlModelFallback recognizes quoted model_fallback keys", () => {
const dir = codexHomeFixture();
writeFileSync(join(dir, "agents", "quoted_key.toml"), [
Expand Down
Loading