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
230 changes: 229 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2172,6 +2172,26 @@ export function loadConfig(): OcxConfig {
warnDegradedAgentTaskRecovery(parsed);
return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed));
}
// Still failing, but if every complaint is about one or more named entries
// in an independent section, drop exactly those and keep the rest. Falling
// back to defaults here would silently retire the operator's providers,
// keys and prices over a mistake in one routing profile.
const salvaged = salvageConfigCandidate(merged, retryResult.error);
if (salvaged) {
{
warnDroppedConfigSections(configPath, salvaged.dropped, salvaged.issues);
const config = normalizeApiKeyIds(salvaged.parsed);
warnDegradedHostname(parsed, config);
warnDegradedApiKeys(parsed, config);
warnDegradedCodexAccountPriorities(parsed, config);
warnDegradedClaudeSubagentEffort(parsed);
warnDegradedNativeSubagentConfig(parsed, config);
warnDegradedCodexAccountPicker(parsed);
warnDegradedUpstreamHostCircuitThreshold(parsed);
warnDegradedAgentTaskRecovery(parsed);
return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed));
}
}
// Merge couldn't fix it — truly broken config
warnAndBackupInvalidConfig(configPath, result.error);
return getDefaultConfig();
Expand Down Expand Up @@ -2450,11 +2470,31 @@ function configDiagnosticsFromRaw(raw: string): ConfigDiagnostics {
return validFileConfigDiagnostics(normalizeApiKeyIds(result.data as OcxConfig), parsed);
}

const retryResult = configSchema.safeParse(mergeConfigDefaults(parsed));
const merged = mergeConfigDefaults(parsed);
const retryResult = configSchema.safeParse(merged);
if (retryResult.success) {
return validFileConfigDiagnostics(normalizeApiKeyIds(retryResult.data as OcxConfig), parsed);
}

// #1785: one invalid routing profile must not make diagnostics report the built-in
// defaults AS the config, because a later config write persists those defaults over the
// operator's providers, keys and prices.
//
// The failure is still reported. `source` stays "fallback" and `error` keeps the real
// schema message -- diagnostics is the surface that tells callers the file is invalid,
// and every consumer that must refuse an invalid config (provider reload, catalog sync,
// cost reconcile, codex admission) gates on exactly those two fields. Only `config`
// changes: it carries the salvaged document instead of factory defaults, so a caller
// that ignores the error and writes it back preserves what the operator configured.
const salvaged = salvageConfigCandidate(merged, retryResult.error);
if (salvaged) {
return {
config: normalizeApiKeyIds(salvaged.parsed),
source: "fallback",
error: schemaDiagnosticsError(result.error),
};
}

return { config: getDefaultConfig(), source: "fallback", error: schemaDiagnosticsError(result.error) };
} catch {
return { config: getDefaultConfig(), source: "fallback", error: "invalid_json" };
Expand Down Expand Up @@ -3428,6 +3468,194 @@ function warnConfigRepaired(configPath: string, error: z.ZodError): void {
console.error(`opencodex config at ${configPath}: repaired missing field(s) [${fields}] with defaults. Your providers and accounts are preserved.`);
}

/**
* Sections whose entries are independent of one another, so one bad entry is
* safe to drop without changing what the rest mean.
*
* Both are validated entry-by-entry in the `superRefine` above, which raises
* every finding as a *document*-level issue. That is what made a single routing
* candidate naming a disabled provider discard the operator's whole config —
* all eleven providers, every API key, and the entire `modelCosts` table —
* while the proxy carried on serving from built-in defaults and reporting
* healthy.
*/
const SALVAGEABLE_CONFIG_SECTIONS = ["routingProfiles", "combos"] as const;

/**
* Drop just the named entries a parse failure blamed, so the rest of the
* document survives.
*
* Returns `null` when the failure was not confined to those sections — the
* caller then keeps its existing behaviour rather than guessing.
*
* The whole entry goes, not the individual offending candidate. A routing
* profile that quietly loses one candidate still routes, just not where the
* operator said it should, and a policy that silently changed shape is a worse
* outcome than one that is plainly absent. Absent is also the loud option: a
* dry-run against it answers `unknown_profile`, which — paired with the warning
* this emits — points at the real mistake.
*/
function dropInvalidConfigSections(
parsed: unknown,
error: z.ZodError,
): { candidate: Record<string, unknown>; dropped: string[] } | null {
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null;

const doomed = new Map<string, Set<string>>();
for (const issue of error.issues) {
if (isUnsalvageableIssue(issue)) return null;
const [section, id] = issue.path;
if (typeof section !== "string" || typeof id !== "string") return null;
if (!(SALVAGEABLE_CONFIG_SECTIONS as readonly string[]).includes(section)) return null;
// A complaint about the container itself ("combos must be an object") is
// not about one entry, so there is nothing selective to drop.
if (issue.path.length < 2) return null;
let ids = doomed.get(section);
if (!ids) doomed.set(section, ids = new Set());
ids.add(id);
}
if (doomed.size === 0) return null;

const candidate: Record<string, unknown> = { ...(parsed as Record<string, unknown>) };
const dropped: string[] = [];
for (const [section, ids] of doomed) {
const current = candidate[section];
if (!current || typeof current !== "object" || Array.isArray(current)) return null;
const kept: Record<string, unknown> = {};
for (const [key, value] of Object.entries(current as Record<string, unknown>)) {
if (ids.has(key)) dropped.push(`${section}.${key}`);
else kept[key] = value;
}
candidate[section] = kept;
}
return dropped.length > 0 ? { candidate, dropped } : null;
}

/**
* Salvage until the document parses, not just once.
*
* One pass is not enough because the sections depend on each other: routing
* profiles are validated against the combo map, so dropping an invalid combo can
* expose a profile that referenced it. A single-pass salvage sees that second
* failure and gives up, discarding the whole config -- the exact outcome this
* code exists to prevent.
*
* `rawDocument` is the operator's document before defaults were merged in. When
* supplied, the same entries are deleted from it too, so a diagnostics caller can
* still tell an absent optional setting from one we injected.
*/

/**
* Findings that must never be salvaged away.
*
* Salvage removes the entry a finding blamed, which is right for an ordinary
* validation mistake and wrong for a namespace collision: the collision is a
* *relationship* between a combo/profile and a Codex account selector, and it is
* reported on the combo. Dropping that combo makes the document parse and quietly
* admits the account selector the schema just refused, turning a hard admission
* boundary into a config that loads. Refuse the whole document instead.
*/
const UNSALVAGEABLE_ISSUE_MESSAGES: readonly string[] = [
CODEX_ACCOUNT_NAMESPACE_COMBO_ALIAS_COLLISION_ERROR,
];

function isUnsalvageableIssue(issue: z.ZodIssue): boolean {
return UNSALVAGEABLE_ISSUE_MESSAGES.some(message => issue.message.includes(message));
}
function salvageConfigCandidate(
merged: unknown,
initialError: z.ZodError,
rawDocument?: unknown,
): {
candidate: Record<string, unknown>;
rawCandidate: unknown;
parsed: OcxConfig;
dropped: string[];
issues: z.ZodIssue[];
} | null {
let candidate: unknown = merged;
let rawCandidate: unknown = rawDocument;
let error = initialError;
const dropped: string[] = [];
const issues: z.ZodIssue[] = [];
// Bounded by construction: every pass must remove at least one entry, and there
// are only so many entries to remove.
const budget = countSalvageableEntries(merged) + 1;
for (let pass = 0; pass < budget; pass++) {
const step = dropInvalidConfigSections(candidate, error);
if (!step || step.dropped.length === 0) return null;
dropped.push(...step.dropped);
issues.push(...error.issues);
candidate = step.candidate;
rawCandidate = deleteEntryPaths(rawCandidate, step.dropped);
const result = configSchema.safeParse(candidate);
if (result.success) {
return { candidate: step.candidate, rawCandidate, parsed: result.data as OcxConfig, dropped, issues };
}
error = result.error;
}
return null;
}

function countSalvageableEntries(document: unknown): number {
if (!document || typeof document !== "object" || Array.isArray(document)) return 0;
let total = 0;
for (const section of SALVAGEABLE_CONFIG_SECTIONS) {
const value = (document as Record<string, unknown>)[section];
if (value && typeof value === "object" && !Array.isArray(value)) {
total += Object.keys(value as Record<string, unknown>).length;
}
}
return total;
}

/** Delete `section.id` entries from a copy of the raw document. */
function deleteEntryPaths(document: unknown, entryPaths: readonly string[]): unknown {
if (!document || typeof document !== "object" || Array.isArray(document)) return document;
const next: Record<string, unknown> = { ...(document as Record<string, unknown>) };
for (const entryPath of entryPaths) {
const separator = entryPath.indexOf(".");
if (separator <= 0) continue;
const section = entryPath.slice(0, separator);
const id = entryPath.slice(separator + 1);
const container = next[section];
if (!container || typeof container !== "object" || Array.isArray(container)) continue;
const kept: Record<string, unknown> = { ...(container as Record<string, unknown>) };
delete kept[id];
next[section] = kept;
}
return next;
}

/**
* Entry ids are operator-chosen and can be token-shaped, so nothing dynamic reaches
* the log unredacted. Static section names stay readable -- they are the part that
* tells the operator where to look.
*/
function redactEntryPath(entryPath: string): string {
const separator = entryPath.indexOf(".");
if (separator <= 0) return redactSecretString(entryPath);
return entryPath.slice(0, separator) + "." + redactSecretString(entryPath.slice(separator + 1));
}

function redactIssuePath(path: readonly PropertyKey[]): string {
return path
.map((segment, index) => (index === 0 && typeof segment === "string" ? segment : redactSecretString(String(segment))))
.join(".");
}

function warnDroppedConfigSections(configPath: string, dropped: string[], issues: readonly z.ZodIssue[]): void {
if (warnedConfigFallbacks.has(configPath)) return;
warnedConfigFallbacks.add(configPath);
const reasons = issues
.map(issue => `${redactIssuePath(issue.path)}: ${redactSecretString(issue.message)}`)
.join("; ");
console.error(
`opencodex config at ${configPath}: dropped [${dropped.map(redactEntryPath).join(", ")}] and loaded the rest — ${reasons}. `
+ "Everything else in your config, including providers and modelCosts, is preserved.",
);
}

export function readPidFileValue(): number | null {
try {
return parsePidFile(readFileSync(getPidPath(), "utf-8"));
Expand Down
Loading
Loading