diff --git a/plugins/trace-codex/AGENTS.md b/plugins/trace-codex/AGENTS.md index 1d07d3f..1b51542 100644 --- a/plugins/trace-codex/AGENTS.md +++ b/plugins/trace-codex/AGENTS.md @@ -179,8 +179,8 @@ Invariants to preserve when changing this: keep the server/processor/braintrust layers agent-agnostic. - **Config**: two layers. The agent-specific layer (`src/agents/codex/settings.ts`) reads the user's `config.json` from `PLUGIN_DATA`, maps its friendly camelCase - keys onto `BRAINTRUST_*` / `BRAINTRUST_EVENT_SERVER_*` env vars (env wins over - the file), and is run by the hook client before it boots the server — so the + keys onto `BRAINTRUST_*` / `BRAINTRUST_EVENT_SERVER_*` env vars (the file wins + over env), and is run by the hook client before it boots the server — so the spawned server inherits the resolved env. The generic layer (`src/config.ts`) then reads **only** those env vars and never touches `config.json`. Keep it that way: config-file parsing is deliberately agent-specific. User-facing diff --git a/plugins/trace-codex/README.md b/plugins/trace-codex/README.md index cafbfee..40a8de7 100644 --- a/plugins/trace-codex/README.md +++ b/plugins/trace-codex/README.md @@ -83,7 +83,7 @@ cp ~/.codex/plugins/cache/braintrust-codex-plugins/trace-codex// # now edit config.json with your desired settings ``` -Every setting can be provided as a `config.json` key or as an environment variable; **an environment variable always overrides config.json** +Every setting can be provided as a `config.json` key or as an environment variable; **`config.json` always overrides the environment variable**. An environment variable only takes effect for settings that `config.json` leaves unset. | `config.json` key | Environment variable | Default | Meaning | |----------------------|---------------------------------------|--------------------|----------------------------------------------------------------------------------------------------------------------------------------------| diff --git a/plugins/trace-codex/src/agents/codex/register.ts b/plugins/trace-codex/src/agents/codex/register.ts index f4b194d..47920e8 100644 --- a/plugins/trace-codex/src/agents/codex/register.ts +++ b/plugins/trace-codex/src/agents/codex/register.ts @@ -28,7 +28,7 @@ export interface Agent { terminalEvents: string[]; /** * Read this agent's user settings and apply them to the environment - * (environment wins). Returns the names of applied settings, for diagnostics + * (the config file wins). Returns the names of applied settings, for diagnostics * (never values). Run before booting the server so the client and server * agree on configuration. */ diff --git a/plugins/trace-codex/src/agents/codex/settings.test.ts b/plugins/trace-codex/src/agents/codex/settings.test.ts index 1d371da..f18650d 100644 --- a/plugins/trace-codex/src/agents/codex/settings.test.ts +++ b/plugins/trace-codex/src/agents/codex/settings.test.ts @@ -115,10 +115,19 @@ describe("applySettingsToEnv", () => { expect(applied.sort()).toEqual(["apiKey", "port", "project"]); }); - test("environment wins: does not overwrite an existing env var", () => { + test("file wins: overwrites an existing env var", () => { const env: NodeJS.ProcessEnv = { BRAINTRUST_PROJECT: "from-env" }; const applied = applySettingsToEnv({ project: "from-file", apiKey: "sk" }, env); + expect(env.BRAINTRUST_PROJECT).toBe("from-file"); + expect(env.BRAINTRUST_API_KEY).toBe("sk"); + expect(applied.sort()).toEqual(["apiKey", "project"]); + }); + + test("env var survives for a setting the file omits", () => { + const env: NodeJS.ProcessEnv = { BRAINTRUST_PROJECT: "from-env" }; + const applied = applySettingsToEnv({ apiKey: "sk" }, env); + expect(env.BRAINTRUST_PROJECT).toBe("from-env"); expect(env.BRAINTRUST_API_KEY).toBe("sk"); expect(applied).toEqual(["apiKey"]); diff --git a/plugins/trace-codex/src/agents/codex/settings.ts b/plugins/trace-codex/src/agents/codex/settings.ts index b77f040..2c2622f 100644 --- a/plugins/trace-codex/src/agents/codex/settings.ts +++ b/plugins/trace-codex/src/agents/codex/settings.ts @@ -6,9 +6,10 @@ // onto the BRAINTRUST_* / BRAINTRUST_EVENT_SERVER_* environment variables that // the rest of the code (and the Braintrust SDK) already understand. // -// Precedence: environment variables always win over the file, so power users -// and CI can override the file without editing it. The file is optional; -// missing or malformed files are ignored (never throw). +// Precedence: the file always wins over environment variables, so a setting +// written to config.json takes effect even when an ambient env var is present. +// Env vars only supply a value for settings the file leaves unset. The file is +// optional; missing or malformed files are ignored (never throw). import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -134,8 +135,9 @@ export function loadSettingsFile(path: string): Settings { } /** - * Apply settings to the environment: for each setting, set its env var only if - * that var is not already set (environment wins). Mutates `env`. Returns the + * Apply settings to the environment: for each setting present in the file, + * overwrite its env var so the file wins over any ambient value. Settings the + * file omits leave the existing env var untouched. Mutates `env`. Returns the * list of setting keys that were applied, for diagnostics (never includes * values, so secrets are not logged). */ @@ -148,7 +150,7 @@ export function applySettingsToEnv( const value = settings[key]; if (value === undefined) continue; const envVar = SETTINGS_TO_ENV[key]; - if (env[envVar]) continue; // environment wins + // The file wins: always overwrite, even when the env var is already set. // Objects (additionalMetadata) are serialized as JSON; everything else // stringifies directly (booleans -> "true"/"false", numbers, strings). env[envVar] = typeof value === "object" ? JSON.stringify(value) : String(value); diff --git a/plugins/trace-codex/src/index.ts b/plugins/trace-codex/src/index.ts index 81853fe..8e7a22a 100644 --- a/plugins/trace-codex/src/index.ts +++ b/plugins/trace-codex/src/index.ts @@ -57,7 +57,7 @@ async function main(): Promise { // Default: hook client. The hook is the client-side entry point, so this is // where the agent reads its user settings and maps them onto the environment - // (environment wins). We do this BEFORE loadConfig so the client and the + // (the config file wins). We do this BEFORE loadConfig so the client and the // server it may spawn agree on port/etc. The spawned server inherits this // resolved env. Swallow everything; never break the agent's turn. const agent = codexAgent;