Skip to content
Open
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
27 changes: 22 additions & 5 deletions registry/coder-labs/modules/codex/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.0.0"
version = "5.1.0"
agent_id = coder_agent.main.id
openai_api_key = var.openai_api_key
}
Expand All @@ -33,7 +33,7 @@ locals {

module "codex" {
source = "registry.coder.com/coder-labs/codex/coder"
version = "5.0.0"
version = "5.1.0"
agent_id = coder_agent.main.id
workdir = local.codex_workdir
openai_api_key = var.openai_api_key
Expand Down Expand Up @@ -64,7 +64,7 @@ resource "coder_app" "codex" {
```tf
module "codex" {
source = "registry.coder.com/coder-labs/codex/coder"
version = "5.0.0"
version = "5.1.0"
agent_id = coder_agent.main.id
workdir = "/home/coder/project"
enable_ai_gateway = true
Expand All @@ -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.0.0"
version = "5.1.0"
agent_id = coder_agent.main.id
workdir = "/home/coder/project"
openai_api_key = var.openai_api_key
Expand All @@ -107,17 +107,34 @@ module "codex" {
args = ["-y", "@modelcontextprotocol/server-github"]
type = "stdio"
EOT

mcp_config_remote_path = [
"https://example.com/team-mcp-servers.toml",
"https://raw.githubusercontent.com/your-org/your-repo/main/.codex/mcp.toml",
]
}
```

> [!NOTE]
> Servers configured through `mcp` or `mcp_config_remote_path` are appended to `~/.codex/config.toml`, so they apply to every Codex session in the workspace. Each remote URL should return a body in Codex's native TOML format, e.g.:
>
> ```toml
> [mcp_servers.my-tool]
> command = "my-tool-server"
> args = ["--port", "8080"]
> type = "stdio"
> ```
>
> Fetch failures (network errors or non-2xx responses) log a warning and the install continues with the remaining URLs. Bodies are appended verbatim without further validation, so make sure the URL returns valid Codex TOML.

### Serialize a downstream `coder_script` after the install pipeline

The module exposes the `scripts` output: an ordered list of `coder exp sync` names for the scripts this module creates (pre_install, install, post_install). Scripts that were not configured are absent.

```tf
module "codex" {
source = "registry.coder.com/coder-labs/codex/coder"
version = "5.0.0"
version = "5.1.0"
agent_id = coder_agent.main.id
openai_api_key = var.openai_api_key
}
Expand Down
80 changes: 80 additions & 0 deletions registry/coder-labs/modules/codex/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,86 @@ describe("codex", async () => {
expect(installLog).toContain("Installed Codex CLI");
});

test("mcp-config-remote-path", async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit [DEREM-6] Test duplicates setup() helper boilerplate.

The mcp-config-remote-path test inlines ~35 lines of container setup (runTerraformApply, runContainer, registerCleanup, writeExecutable) that the existing setup() helper at line 92 already handles. Other tests in this file use setup({moduleVariables: {...}}). This test could do the same and add only the remote-mcp.toml write step after.

(Netero)

🤖

const remoteToml = [
"[mcp_servers.remote-fetched]",
'command = "remote-mcp-cmd"',
'args = ["--from-url"]',
'type = "stdio"',
].join("\n");
const projectDir = "/home/coder/project";
const moduleDir = path.resolve(import.meta.dir);
const state = await runTerraformApply(moduleDir, {
agent_id: "foo",
workdir: projectDir,
install_codex: "false",
mcp_config_remote_path: JSON.stringify([
"http://localhost:19999/mcp.toml",
"file:///tmp/remote-mcp.toml",
]),
});
const scripts = collectScripts(state);
const coderEnvVars = extractCoderEnvVars(state);

const id = await runContainer("codercom/enterprise-node:latest");
registerCleanup(async () => {
if (process.env["DEBUG"] === "true" || process.env["DEBUG"] === "1") {
console.log(`Not removing container ${id} in debug mode`);
return;
}
await removeContainer(id);
});

await execContainer(id, ["bash", "-c", `mkdir -p '${projectDir}'`]);
await writeExecutable({
containerId: id,
filePath: "/usr/bin/coder",
content: "#!/bin/bash\nexit 0\n",
});
await writeExecutable({
containerId: id,
filePath: "/usr/bin/codex",
content: await Bun.file(
path.join(moduleDir, "testdata", "codex-mock.sh"),
).text(),
});
// Drop the remote TOML payload at a path the install script will fetch
// via file://. Keeps the test self-contained (no external network).
await execContainer(id, [
"bash",
"-c",
`cat > /tmp/remote-mcp.toml <<'EOF'\n${remoteToml}\nEOF`,
]);

await runScripts(id, scripts, coderEnvVars);

const installLog = await readFileContainer(
id,
"/home/coder/.coder-modules/coder-labs/codex/logs/install.log",
);
// Both URLs were attempted.
expect(installLog).toContain("http://localhost:19999/mcp.toml");
expect(installLog).toContain("file:///tmp/remote-mcp.toml");
// First URL fails gracefully.
expect(installLog).toContain(
"Warning: Failed to fetch MCP configuration from 'http://localhost:19999/mcp.toml'",
);
// Second URL succeeds.
expect(installLog).not.toContain(
"Warning: Failed to fetch MCP configuration from 'file:///tmp/remote-mcp.toml'",
);
expect(installLog).toContain(
"Appending MCP servers from file:///tmp/remote-mcp.toml",
);

const configToml = await readFileContainer(
id,
"/home/coder/.codex/config.toml",
);
expect(configToml).toContain("[mcp_servers.remote-fetched]");
expect(configToml).toContain('command = "remote-mcp-cmd"');
});

test("custom-config-drops-reasoning-effort", async () => {
const baseConfig = [
'sandbox_mode = "danger-full-access"',
Expand Down
7 changes: 7 additions & 0 deletions registry/coder-labs/modules/codex/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ variable "mcp" {
default = ""
}

variable "mcp_config_remote_path" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 [DEREM-4] mcp_config_remote_path is singular but its type is list(string).

This is a new public API surface shipping in v5.1.0. Once released, renaming to mcp_config_remote_paths is a breaking change. The description confirms the intent: "List of URLs that return MCP server configurations." The name appears in 14 locations across 5 files (main.tf, install.sh.tftpl, main.tftest.hcl, main.test.ts, README.md).

Renaming now is cheap. Renaming after release requires a major version bump.

(Netero)

🤖

type = list(string)
description = "List of URLs that return MCP server configurations in TOML format (matching Codex's native config format). Fetched at install time and appended to config.toml."
default = []
}

variable "model_reasoning_effort" {
type = string
description = "The reasoning effort for the model. One of: none, minimal, low, medium, high, xhigh. See https://platform.openai.com/docs/guides/latest-model#lower-reasoning-effort"
Expand Down Expand Up @@ -141,6 +147,7 @@ locals {
ARG_WORKDIR = local.workdir != "" ? base64encode(local.workdir) : ""
ARG_BASE_CONFIG_TOML = var.base_config_toml != "" ? base64encode(var.base_config_toml) : ""
ARG_MCP = var.mcp != "" ? base64encode(var.mcp) : ""
ARG_MCP_CONFIG_REMOTE_PATH = base64encode(jsonencode(var.mcp_config_remote_path))
ARG_ENABLE_AI_GATEWAY = tostring(var.enable_ai_gateway)
ARG_AIBRIDGE_CONFIG = var.enable_ai_gateway ? base64encode(local.aibridge_config) : ""
ARG_MODEL_REASONING_EFFORT = var.model_reasoning_effort
Expand Down
31 changes: 31 additions & 0 deletions registry/coder-labs/modules/codex/main.tftest.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,34 @@ run "test_workdir_optional" {
error_message = "scripts output should have install script even without workdir"
}
}

run "test_mcp_config_remote_path" {
command = plan

variables {
agent_id = "test-agent"
workdir = "/home/coder"
mcp_config_remote_path = [
"https://example.com/mcp-one.toml",
"https://example.com/mcp-two.toml",
]
}

assert {
condition = strcontains(local.install_script, base64encode(jsonencode(var.mcp_config_remote_path)))
error_message = "install script should embed the base64-encoded mcp_config_remote_path JSON"
}
}

run "test_mcp_config_remote_path_default" {
command = plan

variables {
agent_id = "test-agent"
}

assert {
condition = length(var.mcp_config_remote_path) == 0
error_message = "mcp_config_remote_path should default to an empty list"
}
}
18 changes: 18 additions & 0 deletions registry/coder-labs/modules/codex/scripts/install.sh.tftpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ARG_CODEX_VERSION=$(echo -n '${ARG_CODEX_VERSION}' | base64 -d)
ARG_WORKDIR=$(echo -n '${ARG_WORKDIR}' | base64 -d)
ARG_BASE_CONFIG_TOML=$(echo -n '${ARG_BASE_CONFIG_TOML}' | base64 -d)
ARG_MCP=$(echo -n '${ARG_MCP}' | base64 -d)
ARG_MCP_CONFIG_REMOTE_PATH=$(echo -n '${ARG_MCP_CONFIG_REMOTE_PATH}' | base64 -d)
ARG_ENABLE_AI_GATEWAY='${ARG_ENABLE_AI_GATEWAY}'
ARG_AIBRIDGE_CONFIG=$(echo -n '${ARG_AIBRIDGE_CONFIG}' | base64 -d)
ARG_MODEL_REASONING_EFFORT='${ARG_MODEL_REASONING_EFFORT}'
Expand All @@ -24,6 +25,7 @@ printf "workdir: %s\n" "$${ARG_WORKDIR}"
printf "enable_ai_gateway: %s\n" "$${ARG_ENABLE_AI_GATEWAY}"
printf "install_codex: %s\n" "$${ARG_INSTALL}"
printf "model_reasoning_effort: %s\n" "$${ARG_MODEL_REASONING_EFFORT}"
printf "mcp_config_remote_path: %s\n" "$${ARG_MCP_CONFIG_REMOTE_PATH}"
echo "--------------------------------"

function add_path_to_shell_profiles() {
Expand Down Expand Up @@ -155,6 +157,22 @@ function populate_config_toml() {
echo "$${ARG_MCP}" >> "$${config_path}"
fi

if [ -n "$${ARG_MCP_CONFIG_REMOTE_PATH}" ] && [ "$${ARG_MCP_CONFIG_REMOTE_PATH}" != "[]" ]; then
if ! command -v jq > /dev/null 2>&1; then
printf "Error: 'jq' is required to process mcp_config_remote_path but was not found. Skipping remote MCP config fetch.\n" >&2
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 [DEREM-5] "Error:" label on a non-fatal message.

This message says Error: 'jq' is required... but the script continues. In this same file, line 101's Error: exits with exit 1, while lines 61 and 167 use Warning: and continue. The jq message follows the warning pattern (non-fatal, script continues) but uses the error label.

When mcp_config_remote_path is configured and jq is absent, the user's requested remote MCP configs are silently dropped while the script reports success. An operator scanning logs sees "Error" but the exit code says otherwise.

Fix: either change to Warning: for consistency, or exit 1 since the explicitly requested feature is non-functional.

(Netero)

🤖

else
for url in $(echo "$${ARG_MCP_CONFIG_REMOTE_PATH}" | jq -r '.[]'); do
Comment thread
35C4n0r marked this conversation as resolved.
echo "Fetching MCP configuration from: $${url}"
mcp_toml=$(curl -fsSL "$${url}") || {
echo "Warning: Failed to fetch MCP configuration from '$${url}', continuing..."
continue
}
printf "Appending MCP servers from %s\n" "$${url}"
printf '\n%s\n' "$${mcp_toml}" >> "$${config_path}"
done
fi
fi

if [ "$${ARG_ENABLE_AI_GATEWAY}" = "true" ] && [ -n "$${ARG_AIBRIDGE_CONFIG}" ]; then
if ! grep -q '\[model_providers\.aigateway\]' "$${config_path}" 2>/dev/null; then
printf "Adding AI Gateway configuration\n"
Expand Down
Loading