diff --git a/registry/coder-labs/modules/codex/README.md b/registry/coder-labs/modules/codex/README.md index f2fb81cee..2ce186b32 100644 --- a/registry/coder-labs/modules/codex/README.md +++ b/registry/coder-labs/modules/codex/README.md @@ -13,7 +13,7 @@ Install and configure the [Codex CLI](https://github.com/openai/codex) in your w ```tf module "codex" { source = "registry.coder.com/coder-labs/codex/coder" - version = "5.1.1" + version = "5.2.0" agent_id = coder_agent.main.id openai_api_key = var.openai_api_key } @@ -33,7 +33,7 @@ locals { module "codex" { source = "registry.coder.com/coder-labs/codex/coder" - version = "5.1.1" + version = "5.2.0" agent_id = coder_agent.main.id workdir = local.codex_workdir openai_api_key = var.openai_api_key @@ -64,7 +64,7 @@ resource "coder_app" "codex" { ```tf module "codex" { source = "registry.coder.com/coder-labs/codex/coder" - version = "5.1.1" + version = "5.2.0" agent_id = coder_agent.main.id workdir = "/home/coder/project" enable_ai_gateway = true @@ -88,7 +88,7 @@ When `enable_ai_gateway = true`, the module configures Codex to use the `aigatew ```tf module "codex" { source = "registry.coder.com/coder-labs/codex/coder" - version = "5.1.1" + version = "5.2.0" agent_id = coder_agent.main.id workdir = "/home/coder/project" openai_api_key = var.openai_api_key @@ -117,7 +117,7 @@ The module exposes the `scripts` output: an ordered list of `coder exp sync` nam ```tf module "codex" { source = "registry.coder.com/coder-labs/codex/coder" - version = "5.1.1" + version = "5.2.0" agent_id = coder_agent.main.id openai_api_key = var.openai_api_key } @@ -142,6 +142,28 @@ resource "coder_script" "post_codex" { When no custom `base_config_toml` is provided, the module uses a minimal default with `preferred_auth_method = "apikey"`. For advanced options, see [Codex config docs](https://developers.openai.com/codex/config-advanced). +> [!NOTE] +> Content you add outside the managed block is preserved across workspace restarts. The module structures the file as: +> +> ```toml +> # bare keys you add above the managed block (root scope) +> my_flag = true +> +> # >>> coder-managed: codex module >>> +> preferred_auth_method = "apikey" +> # ... module-managed content ... +> # <<< coder-managed: codex module <<< +> +> # [sections] you add below the managed block +> [mcp_servers.my_tool] +> command = "my-tool" +> ``` +> +> Two constraints to keep the TOML valid: +> +> - Bare keys you place **above** the managed block must not duplicate keys already inside it (duplicate keys are a TOML error). +> - `[section]` headers you place **below** the managed block must not duplicate a table name already defined inside it (same reason). Bare keys placed after the managed block will fall under whatever `[section]` preceded them — place bare keys above the block to keep them at root scope. + ## Troubleshooting Check the log files in `~/.coder-modules/coder-labs/codex/logs/` for detailed information. diff --git a/registry/coder-labs/modules/codex/main.test.ts b/registry/coder-labs/modules/codex/main.test.ts index 44ce9e96a..da82621a5 100644 --- a/registry/coder-labs/modules/codex/main.test.ts +++ b/registry/coder-labs/modules/codex/main.test.ts @@ -171,6 +171,9 @@ const runScripts = async ( } }; +const MANAGED_START = "# >>> coder-managed: codex module >>>"; +const MANAGED_END = "# <<< coder-managed: codex module <<<"; + setDefaultTimeout(60 * 1000); describe("codex", async () => { @@ -231,8 +234,10 @@ describe("codex", async () => { }); await runScripts(id, scripts); const resp = await readFileContainer(id, "/home/coder/.codex/config.toml"); - expect(resp).toContain('sandbox_mode = "danger-full-access"'); - expect(resp).toContain('preferred_auth_method = "apikey"'); + expect(resp).toContain(MANAGED_START); + expect(resp).toContain(MANAGED_END); + expect(resp).toMatch(/sandbox_mode\s*=\s*"danger-full-access"/); + expect(resp).toMatch(/preferred_auth_method\s*=\s*"apikey"/); expect(resp).toContain("[custom_section]"); }); @@ -259,7 +264,9 @@ describe("codex", async () => { const { id, scripts } = await setup(); await runScripts(id, scripts); const resp = await readFileContainer(id, "/home/coder/.codex/config.toml"); - expect(resp).toContain('preferred_auth_method = "apikey"'); + expect(resp).toContain(MANAGED_START); + expect(resp).toContain(MANAGED_END); + expect(resp).toMatch(/preferred_auth_method\s*=\s*"apikey"/); expect(resp).not.toContain("model_provider"); expect(resp).not.toContain("[model_providers."); expect(resp).not.toContain("model_reasoning_effort"); @@ -314,8 +321,8 @@ describe("codex", async () => { id, "/home/coder/.codex/config.toml", ); - expect(configToml).toContain('model_provider = "aigateway"'); - expect(configToml).toContain('model_reasoning_effort = "none"'); + expect(configToml).toMatch(/model_provider\s*=\s*"aigateway"/); + expect(configToml).toMatch(/model_reasoning_effort\s*=\s*"none"/); expect(configToml).toContain("[model_providers.aigateway]"); }); @@ -330,7 +337,7 @@ describe("codex", async () => { id, "/home/coder/.codex/config.toml", ); - expect(configToml).toContain('model_reasoning_effort = "high"'); + expect(configToml).toMatch(/model_reasoning_effort\s*=\s*"high"/); expect(configToml).not.toContain("model_provider"); }); @@ -346,8 +353,8 @@ describe("codex", async () => { id, "/home/coder/.codex/config.toml", ); - expect(configToml).toContain(`[projects."${workdir}"]`); - expect(configToml).toContain('trust_level = "trusted"'); + expect(configToml).toMatch(new RegExp(`\\[projects\\..*${workdir}.*\\]`)); + expect(configToml).toMatch(/trust_level\s*=\s*"trusted"/); }); test("no-workdir-no-project-section", async () => { @@ -380,7 +387,7 @@ describe("codex", async () => { id, "/home/coder/.codex/config.toml", ); - expect(configToml).toContain('model_provider = "aigateway"'); + expect(configToml).toMatch(/model_provider\s*=\s*"aigateway"/); expect(configToml).toContain("[model_providers.aigateway]"); }); @@ -425,6 +432,63 @@ describe("codex", async () => { expect(installLog).toContain("Installed Codex CLI"); }); + test("base-config-plus-mcp-combined", async () => { + const baseConfig = [ + 'sandbox_mode = "danger-full-access"', + 'preferred_auth_method = "apikey"', + ].join("\n"); + const mcpConfig = [ + "[mcp_servers.github]", + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-github"]', + 'type = "stdio"', + ].join("\n"); + const { id, scripts } = await setup({ + moduleVariables: { + base_config_toml: baseConfig, + mcp: mcpConfig, + }, + }); + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + expect(config).toMatch(/sandbox_mode\s*=\s*"danger-full-access"/); + expect(config).toMatch(/preferred_auth_method\s*=\s*"apikey"/); + expect(config).toContain("mcp_servers"); + expect(config).toMatch(/command\s*=\s*"npx"/); + }); + + test("all-config-sources-combined", async () => { + const baseConfig = [ + 'sandbox_mode = "danger-full-access"', + 'preferred_auth_method = "apikey"', + ].join("\n"); + const mcpConfig = [ + "[mcp_servers.github]", + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-github"]', + 'type = "stdio"', + ].join("\n"); + const { id, coderEnvVars, scripts } = await setup({ + moduleVariables: { + enable_ai_gateway: "true", + base_config_toml: baseConfig, + mcp: mcpConfig, + }, + }); + await runScripts(id, scripts, coderEnvVars); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + expect(config).toMatch(/sandbox_mode\s*=\s*"danger-full-access"/); + expect(config).toMatch(/preferred_auth_method\s*=\s*"apikey"/); + expect(config).toMatch(/command\s*=\s*"npx"/); + expect(config).toContain("[model_providers.aigateway]"); + }); + test("custom-config-drops-reasoning-effort", async () => { const baseConfig = [ 'sandbox_mode = "danger-full-access"', @@ -441,7 +505,366 @@ describe("codex", async () => { id, "/home/coder/.codex/config.toml", ); - expect(configToml).toContain('sandbox_mode = "danger-full-access"'); + expect(configToml).toMatch(/sandbox_mode\s*=\s*"danger-full-access"/); expect(configToml).not.toContain("model_reasoning_effort"); }); + + // --- idempotency tests: marker-block semantics --- + + test("idempotent-user-section-survives-restart", async () => { + const { id, scripts } = await setup(); + await runScripts(id, scripts); + + // User adds a custom section after the managed block. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +[mcp_servers.user_tool] +command = "my-tool" +args = ["--serve"] +type = "stdio" +EOF`, + ]); + + // Second run: managed block is regenerated, user section survives. + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // Managed content still present + expect(config).toMatch(/preferred_auth_method\s*=\s*"apikey"/); + expect(config).toContain(MANAGED_START); + expect(config).toContain(MANAGED_END); + // User section preserved + expect(config).toContain("[mcp_servers.user_tool]"); + expect(config).toMatch(/command\s*=\s*"my-tool"/); + // User section must appear after the managed block, not inside or before it. + const endIdx = config.indexOf(MANAGED_END); + const sectionIdx = config.indexOf("[mcp_servers.user_tool]"); + expect(sectionIdx).toBeGreaterThan(endIdx); + }); + + test("idempotent-user-bare-keys-stay-at-root-scope", async () => { + const { id, scripts } = await setup(); + await runScripts(id, scripts); + + // User prepends bare keys before the managed block and appends a section after it. + await execContainer(id, [ + "bash", + "-c", + `config=/home/coder/.codex/config.toml +{ printf 'my_custom_key = "hello"\\nsandbox_mode = "full"\\n\\n'; cat "$config"; } > /tmp/codex_c.toml && mv /tmp/codex_c.toml "$config" +cat >> "$config" << 'EOF' + +[mcp_servers.user_tool] +command = "my-tool" +EOF`, + ]); + + // Second run + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + + // Bare keys placed before the managed block must remain before MANAGED_START. + const startIdx = config.indexOf(MANAGED_START); + const customKeyIdx = config.indexOf('my_custom_key = "hello"'); + const sandboxIdx = config.indexOf('sandbox_mode = "full"'); + expect(customKeyIdx).toBeGreaterThan(-1); + expect(sandboxIdx).toBeGreaterThan(-1); + expect(customKeyIdx).toBeLessThan(startIdx); + expect(sandboxIdx).toBeLessThan(startIdx); + + // Section appended after the managed block must remain after MANAGED_END. + const endIdx = config.indexOf(MANAGED_END); + const sectionIdx = config.indexOf("[mcp_servers.user_tool]"); + expect(sectionIdx).toBeGreaterThan(endIdx); + }); + + test("idempotent-managed-block-regenerated", async () => { + const { id, scripts } = await setup({ + moduleVariables: { + model_reasoning_effort: "high", + }, + }); + await runScripts(id, scripts); + + // User modifies a value inside the managed block. + await execContainer(id, [ + "bash", + "-c", + "sed -i 's/model_reasoning_effort.*/model_reasoning_effort = \"low\"/' /home/coder/.codex/config.toml", + ]); + + // Verify user edit took effect. + const edited = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + expect(edited).toMatch(/model_reasoning_effort\s*=\s*"low"/); + + // Second run: managed block is regenerated with original values. + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // Original managed value restored + expect(config).toMatch(/model_reasoning_effort\s*=\s*"high"/); + expect(config).not.toMatch(/model_reasoning_effort\s*=\s*"low"/); + }); + + test("idempotent-user-comments-preserved", async () => { + const { id, scripts } = await setup(); + await runScripts(id, scripts); + + // User adds a bare-key comment, a bare key, then a section with comments. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +# My personal top-level setting +my_flag = true + +# My personal MCP server +[mcp_servers.notes] +command = "notes-server" +# This server is for my personal notes +type = "stdio" +EOF`, + ]); + + // Second run + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // Bare-key comment preserved in output + expect(config).toContain("# My personal top-level setting"); + // Section comments preserved below managed block + expect(config).toContain("# My personal MCP server"); + expect(config).toContain("# This server is for my personal notes"); + expect(config).toContain("[mcp_servers.notes]"); + }); + + test("idempotent-stable-after-roundtrip", async () => { + const { id, scripts } = await setup(); + + // First run: write the managed block. + await runScripts(id, scripts); + + // User appends content outside the managed block. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +roundtrip_key = "present" + +# User's personal server +[mcp_servers.roundtrip] +command = "roundtrip-tool" +type = "stdio" +EOF`, + ]); + + // Second run: managed block is regenerated with user content in place. + await runScripts(id, scripts); + const configAfterSecond = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + + // Third run: output must be byte-identical (no double-hoisting or newline drift). + await runScripts(id, scripts); + const configAfterThird = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + + expect(configAfterThird).toEqual(configAfterSecond); + expect(configAfterThird).toContain('roundtrip_key = "present"'); + expect(configAfterThird).toContain("[mcp_servers.roundtrip]"); + }); + + test("idempotent-mcp-new-servers-added-existing-kept", async () => { + const mcpConfig = [ + "[mcp_servers.github]", + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-github"]', + 'type = "stdio"', + ].join("\n"); + const { id, scripts } = await setup({ + moduleVariables: { mcp: mcpConfig }, + }); + await runScripts(id, scripts); + + // User adds their own MCP server after the managed block. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +[mcp_servers.custom] +command = "my-tool" +args = ["--serve"] +type = "stdio" +EOF`, + ]); + + // Second run + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // Module's github server still present (in managed block) + expect(config).toContain("[mcp_servers.github]"); + expect(config).toMatch(/command\s*=\s*"npx"/); + // User's custom server preserved (outside managed block) + expect(config).toContain("[mcp_servers.custom]"); + expect(config).toMatch(/command\s*=\s*"my-tool"/); + }); + + test("no-markers-first-run-overwrites", async () => { + const { id, scripts } = await setup(); + + // Simulate a legacy config without markers (pre-upgrade). + await execContainer(id, [ + "bash", + "-c", + `mkdir -p /home/coder/.codex && cat > /home/coder/.codex/config.toml << 'EOF' +preferred_auth_method = "login" +legacy_key = "old_value" + +[mcp_servers.legacy] +command = "legacy-tool" +type = "stdio" +EOF`, + ]); + + // First run: no markers found, file is overwritten entirely by the managed block. + await runScripts(id, scripts); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // Managed block is written + expect(config).toContain(MANAGED_START); + expect(config).toContain(MANAGED_END); + // Legacy content is gone + expect(config).not.toContain('preferred_auth_method = "login"'); + expect(config).not.toContain('legacy_key = "old_value"'); + expect(config).not.toContain("[mcp_servers.legacy]"); + + // Second run: output must be stable. + await runScripts(id, scripts); + const configAfterSecond = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + expect(configAfterSecond).toEqual(config); + }); + + test("idempotent-all-sources-user-content-survives", async () => { + const baseConfig = [ + 'sandbox_mode = "danger-full-access"', + 'preferred_auth_method = "apikey"', + ].join("\n"); + const mcpConfig = [ + "[mcp_servers.github]", + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-github"]', + 'type = "stdio"', + ].join("\n"); + const { id, coderEnvVars, scripts } = await setup({ + moduleVariables: { + enable_ai_gateway: "true", + base_config_toml: baseConfig, + mcp: mcpConfig, + }, + }); + await runScripts(id, scripts, coderEnvVars); + + // User adds content outside the managed block. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +# User's personal MCP server +[mcp_servers.personal] +command = "personal-server" +type = "stdio" +EOF`, + ]); + + // Second run + await runScripts(id, scripts, coderEnvVars); + const config = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + // All managed content correct + expect(config).toMatch(/sandbox_mode\s*=\s*"danger-full-access"/); + expect(config).toMatch(/preferred_auth_method\s*=\s*"apikey"/); + expect(config).toContain("[mcp_servers.github]"); + expect(config).toContain("[model_providers.aigateway]"); + // User content preserved + expect(config).toContain("# User's personal MCP server"); + expect(config).toContain("[mcp_servers.personal]"); + expect(config).toMatch(/command\s*=\s*"personal-server"/); + }); + + test("idempotent-multiple-restarts-user-content-stable", async () => { + const mcpConfig = [ + "[mcp_servers.github]", + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-github"]', + 'type = "stdio"', + ].join("\n"); + const { id, scripts } = await setup({ + moduleVariables: { mcp: mcpConfig }, + }); + await runScripts(id, scripts); + + // User adds content outside managed block. + await execContainer(id, [ + "bash", + "-c", + `cat >> /home/coder/.codex/config.toml << 'EOF' + +# User customizations +[mcp_servers.custom] +command = "custom-tool" +type = "stdio" +EOF`, + ]); + + // Run 2 + await runScripts(id, scripts); + const configAfterSecond = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + + // Run 3: should be byte-identical to run 2 + await runScripts(id, scripts); + const configAfterThird = await readFileContainer( + id, + "/home/coder/.codex/config.toml", + ); + + expect(configAfterThird).toEqual(configAfterSecond); + // User content still present + expect(configAfterThird).toContain("# User customizations"); + expect(configAfterThird).toContain("[mcp_servers.custom]"); + }); }); diff --git a/registry/coder-labs/modules/codex/scripts/install.sh.tftpl b/registry/coder-labs/modules/codex/scripts/install.sh.tftpl index 07dffee7f..46112c712 100644 --- a/registry/coder-labs/modules/codex/scripts/install.sh.tftpl +++ b/registry/coder-labs/modules/codex/scripts/install.sh.tftpl @@ -136,27 +136,86 @@ function populate_config_toml() { local config_path="$HOME/.codex/config.toml" mkdir -p "$(dirname "$${config_path}")" + local MANAGED_START="# >>> coder-managed: codex module >>>" + local MANAGED_END="# <<< coder-managed: codex module <<<" + + # Reject inputs that contain the managed-block markers to prevent embedding + # spurious markers that would corrupt the block structure on subsequent runs. + for _arg in "$${ARG_BASE_CONFIG_TOML}" "$${ARG_MCP}"; do + if [ -n "$${_arg}" ] && printf '%s' "$${_arg}" | grep -qF -e "$${MANAGED_START}" -e "$${MANAGED_END}"; then + printf 'Error: a codex module input contains a managed-block marker; aborting.\n' >&2 + return 1 + fi + done + + # Build managed-block content in a temp file. + local managed="" config_tmp="" + trap 'rm -f "$${managed:-}" "$${config_tmp:-}"' EXIT + managed=$(mktemp) + if [ -n "$${ARG_BASE_CONFIG_TOML}" ]; then printf "Using provided base configuration\n" - echo "$${ARG_BASE_CONFIG_TOML}" > "$${config_path}" + printf '%s\n' "$${ARG_BASE_CONFIG_TOML}" > "$${managed}" else printf "Using minimal default configuration\n" - write_minimal_default_config "$${config_path}" + write_minimal_default_config "$${managed}" fi if [ -n "$${ARG_MCP}" ]; then printf "Adding MCP servers\n" - echo "$${ARG_MCP}" >> "$${config_path}" + printf '%s\n' "$${ARG_MCP}" >> "$${managed}" fi if [ "$${ARG_ENABLE_AI_GATEWAY}" = "true" ] && [ -n "$${ARG_AIBRIDGE_CONFIG}" ]; then - if ! grep -q '\[model_providers\.aigateway\]' "$${config_path}" 2>/dev/null; then + if ! grep -q '\[model_providers\.aigateway\]' "$${managed}" 2>/dev/null; then printf "Adding AI Gateway configuration\n" - echo -e "\n$${ARG_AIBRIDGE_CONFIG}" >> "$${config_path}" + printf '\n%s\n' "$${ARG_AIBRIDGE_CONFIG}" >> "$${managed}" else printf "AI Gateway provider already defined in config, skipping append\n" fi fi + + # Preserve user content from the existing config. + # user_before = everything before MANAGED_START + # user_after = everything after MANAGED_END + # On the first run (no markers), both are empty and the file is written fresh. + local existing_config="" user_before="" user_after="" + if [ -s "$${config_path}" ]; then + existing_config=$(cat "$${config_path}") + fi + if [ -n "$${existing_config}" ]; then + if printf '%s\n' "$${existing_config}" | grep -qF "$${MANAGED_START}"; then + if printf '%s\n' "$${existing_config}" | grep -qF "$${MANAGED_END}"; then + user_before=$(printf '%s\n' "$${existing_config}" | awk -v m="$${MANAGED_START}" '$0==m{exit} {print}') + user_after=$(printf '%s\n' "$${existing_config}" | awk -v m="$${MANAGED_END}" 'found{print} $0==m{found=1}') + user_before=$(printf '%s\n' "$${user_before}" | sed '/./,$!d') + user_after=$(printf '%s\n' "$${user_after}" | sed '/./,$!d') + else + # Start marker present but end marker missing (file was truncated or hand-edited). + # Treat everything as user_before to avoid silently deleting data. + printf "Warning: codex config has managed-block start marker but no end marker; treating entire file as user content\n" + user_before="$${existing_config}" + fi + fi + fi + + # Assemble final config atomically: + # user_before (content that was before the managed block) + # managed block + # user_after (content that was after the managed block) + config_tmp=$(mktemp "$${config_path}.XXXXXX") + { + if [ -n "$${user_before}" ]; then + printf '%s\n\n' "$${user_before}" + fi + printf '%s\n' "$${MANAGED_START}" + cat "$${managed}" + printf '%s\n' "$${MANAGED_END}" + if [ -n "$${user_after}" ]; then + printf '\n%s\n' "$${user_after}" + fi + } > "$${config_tmp}" && mv "$${config_tmp}" "$${config_path}" + rm -f "$${managed}" } function setup_workdir() {