refactor(registry/coder-labs/modules/codex)!: remove agentapi, tasks and start logic

[35C4n0r]

Closes #878

What

Major refactor of the coder-labs/codex module to mirror the coder/claude-code v5 changes from #861.

Changes

Structural

• Replace module "agentapi" with module "coder_utils" (registry.coder.com/coder/coder-utils/coder v0.0.1)
• Replace scripts/install.sh with scripts/install.sh.tftpl (Terraform templatefile)
• Delete scripts/start.sh
• Module dir changed from .codex-module to .coder-modules/coder-labs/codex
• Output changed from task_app_id to scripts (ordered list of coder exp sync names)
• Extracted shared test helpers (collectScripts, runScripts) into agentapi/coder-utils-test-helpers.ts

Removed variables

All AgentAPI pass-throughs, boundary, and start-script-only variables:
order, group, report_tasks, subdomain, cli_app, web_app_display_name, cli_app_display_name, install_agentapi, agentapi_version, ai_prompt, continue, enable_state_persistence, codex_system_prompt, enable_boundary, boundary_config_path, boundary_version, compile_boundary_from_source, use_boundary_directly, codex_model

Retained

install_codex (toggle for skipping npm install when CLI is pre-installed)

Renamed

• enable_aibridge -> enable_ai_gateway

Changed

• workdir: now optional (default = null)
• openai_api_key: conditional env var with count, marked sensitive = true
• base_config_toml: heredoc description documenting generated defaults; notes that model_reasoning_effort and workdir trust are only applied in default config
• Default config.toml: stripped sandbox_mode, approval_policy, sandbox_workspace_write, notice.model_migrations
• Install script: removed Node.js/NVM bootstrap (assumes npm pre-installed), sources NVM if present, fails with actionable error if npm missing
• ARG_CODEX_VERSION and ARG_WORKDIR base64-encoded to prevent shell/TOML injection
• Duplicate [model_providers.aibridge] guarded with grep before appending
• Debug header uses user-facing variable names

Tests

• Terraform: 11 pass
• Bun: 15 pass (rewritten to shared collectScripts/runScripts pattern)
• Added: model-reasoning-effort-standalone, ai-gateway-with-custom-base-config, ai-gateway-custom-config-no-duplicate-provider, install-codex-latest, workdir-trusted-project, no-workdir-no-project-section
• Negative assertions on minimal-default-config

Docs

• Migration guide (v4 to v5) in README
• Quoted path in coder_app example
• AI Gateway note about custom base_config_toml requiring manual model_provider

Warning

Breaking change. Drops support for Coder Tasks and Boundary. Keep using v4.x.x if you depend on them.

---

This PR was authored by Coder Agents.

[matifali]

A minor nit, but it looks good to me.

[matifali]

/coder-agents-review

[coder-agents-review]

Solid refactor that cleanly converges codex on the coder-utils pattern, removes substantial dead code (AgentAPI, Boundary, Tasks), and simplifies the install pipeline. The scripts output design and conditional coder_env resources are well-considered. The breaking change is documented and version-bumped correctly.

Severity breakdown: 3 P2, 11 P3, 2 Nit.

The P2s center on: (1) openai_api_key missing sensitive = true (5 reviewers flagged this), (2) ARG_WORKDIR/ARG_CODEX_VERSION passed unencoded through templatefile single-quote assignments, breaking or enabling injection when the value contains ' or ", and (3) zero test coverage for the enable_ai_gateway + base_config_toml combination, which is the most complex split behavior in the install script and the only path requiring manual user action.

Five reviewers independently flagged that enable_ai_gateway = true with a user-provided base_config_toml unconditionally appends [model_providers.aibridge], risking duplicate TOML headers if the user's config already defines that section.

Process note: the PR description lists install_codex as removed, but the final code retains it. The description should be updated to match the delivered code. Two "debug" commits remain unsquashed in the branch; consider squashing before merge.

P3 [DEREM-15] The PR description's "Removed variables" list includes install_codex, but main.tf:45 retains it. The description also claims "ARG_INSTALL toggle (Codex always installs via npm)" was removed, but install.sh.tftpl has ARG_INSTALL. A reviewer trusting the description would miss that these were restored after the initial removal. (Mafu-san)

P3 No test for the default install path. The deleted check-latest-codex-version-works was the only coverage for npm install -g "@openai/codex" (without version pin). The surviving install-codex-version exercises only the pinned branch. The most common real-world path now has zero test coverage. (Bisky) Note: -main.test.ts:78, outside current diff.

Pariston: "I tried to construct a scenario where the stated problem exists but this fix does not help. I could not. The refactor is structurally complete."

Mafu-san: "Despite the turbulent path to get there, the final install_codex function handles NVM, non-NVM, and non-root environments correctly."

🤖 This review was automatically generated with Coder Agents.

[coder-agents-review]

Addendum: findings incorrectly dropped from the initial review.

The following were cut during cross-check with insufficient justification. Reopening them here.

P3 [DEREM-1] main.test.ts:302 — model_reasoning_effort is only tested in combination with enable_ai_gateway, never standalone. The write_minimal_default_config function handles these independently (install.sh.tftpl lines 55-61), so the standalone path is unverified. (Netero)

P3 [DEREM-2] main.tf:129 — The aibridge_config local still uses name = "AI Bridge" while the module consistently calls the feature "AI Gateway" everywhere else (variable names, README, descriptions). The name field is a human-readable label the module controls; the section key [model_providers.aibridge], env var, and API path are protocol-level and correctly unchanged. (Netero)

P3 [DEREM-3] main.tftest.hcl:35 — test_codex_custom_options asserts var.icon == "/icon/custom.svg", which proves Terraform variable assignment works, not that the icon propagates to module.coder_utils. Same pattern in test_codex_with_scripts. The Bun integration tests cover actual behavior, but these tftest assertions add no value on their own. (Netero)

Note [DEREM-4] README.md:51 — The coder_app example uses cd ${local.codex_workdir} without shell quoting. If the resolved path contains spaces, the cd breaks. Users copy-paste README examples. Using cd "${local.codex_workdir}" would be defensive. (Netero)

Note [DEREM-5] install.sh.tftpl:59 — Both model_reasoning_effort and the workdir [projects.] section are only written by write_minimal_default_config, which is skipped when base_config_toml is non-empty. A user setting model_reasoning_effort = "high" alongside a custom base_config_toml gets no error and no effect. The base_config_toml description mentions "AI Gateway sections are still appended" but does not mention that model_reasoning_effort and workdir trust are silently dropped. (Netero)

Note [DEREM-24] README.md — No migration section for v4 to v5 users. The PR description catalogs every removed variable, but a user upgrading an existing template must independently figure out: remove all old variables, add a coder_app or coder_script to start Codex, remove coder_ai_task resources, update the module data path in debugging scripts. A brief "Migrating from v4" section would reduce friction for the breaking change. (Pariston)

🤖 This review was automatically generated with Coder Agents.

[35C4n0r]

@matifali, addressed the comments and tested.

[matifali]

/coder-agents-review

[coder-agents-review]

Re-review blocked. 10 of 22 findings were addressed (thank you), but 12 remain without a substantive response. The panel cannot re-review until each open finding gets one of: a fix, a reasoned defense, or a linked deferral ticket.

Findings with content-free dismissals ("not a concern, do not address" with no reasoning):

• DEREM-9 (P3) Test boilerplate copied verbatim from claude-code. Author: "out of context of pr, do not address." The consequence (maintenance burden across six future module migrations) was not addressed.
• DEREM-12 (P3) minimal-default-config test has one positive assertion, no negative. Author: "not a concern, do not address." The consequence (silent regressions injecting aibridge or sandbox_mode into defaults) was not addressed.
• DEREM-14 (P3) TOML injection via ARG_WORKDIR in project trust section. Author: "Not a concern do not address." Note: DEREM-7 fix (base64-encoding) protects the shell assignment, but ARG_WORKDIR is still interpolated unsanitized into a TOML heredoc at line 78. The TOML injection vector persists.
• DEREM-18 (P3) Debug header prints internal template variable names. Author: "not a concern, do not address."
• DEREM-19 (P3) openai-api-key test spins up a container it never uses. Author: "do not address."

Findings with no response at all (posted in addendum comment):

• DEREM-1 (P3) model_reasoning_effort only tested with enable_ai_gateway, never standalone.
• DEREM-2 (P3) Display name "AI Bridge" while module calls feature "AI Gateway."
• DEREM-3 (P3) Tautological tftest assertions (var == input literal).
• DEREM-4 (Note) Unquoted path in README example.
• DEREM-5 (Note) model_reasoning_effort and workdir silently dropped when base_config_toml provided.
• DEREM-15 (P3) PR description claims install_codex removed but code retains it.
• DEREM-24 (Note) No migration section for v4 to v5 users.

For each: fix it, explain why the current behavior is correct, or file a ticket to defer. A bare "do not address" without engaging the stated consequence does not unblock. This needs a human decision.

🤖 This review was automatically generated with Coder Agents.

[coder-agents-review]

Re-review of the delta since R1. 19 of 22 R1 findings were addressed with code changes; good progress. The PR description was updated to correctly list install_codex as retained. Migration guide and Node.js upgrade warning added.

New severity breakdown: 7 P3 open (3 Netero re-raises from R1 findings that regressed or were incompletely fixed, 4 new panel findings).

DEREM-14 contest (TOML injection via ARG_WORKDIR): rejected unanimously by all 7 panel reviewers. The author's defense conflates shell injection (fixed by base64 at assignment time) with TOML injection (decoded value interpolated raw into a heredoc at line 80). DEREM-25 tracks the active root cause. This needs a human decision: add a workdir validation constraint, or quote/escape the value before TOML interpolation.

DEREM-9/27 (test boilerplate duplication): the author extracted shared helpers in 41fa36c, then reverted in 2aa54d4 with no explanation. The duplication remains. This also needs a human decision: extract again (with claude-code updated too), or accept the duplication.

Mafu-san: "The commit messages identify the finding number, state what was done, and for contested or accepted findings explain the reasoning. This is traceable, deliberate work. The process failures are specific to verification depth and correction persistence, not to the volume or organization of work."

Hisoka on DEREM-14: "A workdir with an embedded newline decodes cleanly into the shell variable, and heredoc expansion writes the newline verbatim into config.toml. The result is a new [malicious] table injected above trust_level."

---

registry/coder-labs/modules/codex/main.tf:7

P3 [DEREM-30] The codex module declares coder >= 2.12 but its dependency module.coder_utils requires >= 2.13. A user who provisions with coder provider 2.12.x passes the codex constraint but fails during coder_utils resolution with a confusing error pointing at the wrong module.

Fix: change to version = ">= 2.13".

🤖

🤖 This review was automatically generated with Coder Agents.

[matifali]

@DevelopmentCats can you give it a test too?

[DevelopmentCats]

@DevelopmentCats can you give it a test too?

Yeah I will retest and post with my results. I ran into some issues before and was trying to find the exact issue, but had issues getting things to launch properly on my end.

[DevelopmentCats]

Hey Jay, did a thorough test of this module and have some findings. Issues #1 and #3 are merge-blockers from my perspective — they're regressions vs both the old codex v4 install script and the merged claude-code v5 precedent. The rest are doc nits.

---

Issue 1: codex binary not on PATH for downstream consumers (Medium — merge blocker)

scripts/install.sh.tftpl — the install_codex() function sources NVM and exports PATH in the current shell, but nothing persists to shell profiles or symlinks into $CODER_SCRIPT_BIN_DIR. Any coder_app command, coder_script, or new shell session after install fails with codex: not found.

The old install.sh at least persisted ~/.npm-global/bin to ~/.bashrc. The merged claude-code v5 (PR #861) does both — add_path_to_shell_profiles AND symlinks into $CODER_SCRIPT_BIN_DIR.

Suggested fix — add these two functions and call ensure_codex_in_path at the end of install_codex():

function add_path_to_shell_profiles() {
  local path_dir="$1"

  for profile in "$HOME/.profile" "$HOME/.bash_profile" "$HOME/.bashrc" "$HOME/.zprofile" "$HOME/.zshrc"; do
    if [ -f "${profile}" ]; then
      if ! grep -q "${path_dir}" "${profile}" 2> /dev/null; then
        echo "export PATH=\"\$PATH:${path_dir}\"" >> "${profile}"
        echo "Added ${path_dir} to ${profile}"
      fi
    fi
  done

  local fish_config="$HOME/.config/fish/config.fish"
  if [ -f "${fish_config}" ]; then
    if ! grep -q "${path_dir}" "${fish_config}" 2> /dev/null; then
      echo "fish_add_path ${path_dir}" >> "${fish_config}"
      echo "Added ${path_dir} to ${fish_config}"
    fi
  fi
}

function ensure_codex_in_path() {
  local CODEX_BIN=""
  if command -v codex > /dev/null 2>&1; then
    CODEX_BIN=$(command -v codex)
  elif [ -x "$HOME/.npm-global/bin/codex" ]; then
    CODEX_BIN="$HOME/.npm-global/bin/codex"
  fi

  if [ -z "${CODEX_BIN}" ] || [ ! -x "${CODEX_BIN}" ]; then
    echo "Warning: Could not find codex binary after install"
    return
  fi

  local CODEX_DIR
  CODEX_DIR=$(dirname "${CODEX_BIN}")

  if [ -n "${CODER_SCRIPT_BIN_DIR:-}" ] && [ ! -e "${CODER_SCRIPT_BIN_DIR}/codex" ]; then
    ln -s "${CODEX_BIN}" "${CODER_SCRIPT_BIN_DIR}/codex"
    echo "Created symlink: ${CODER_SCRIPT_BIN_DIR}/codex -> ${CODEX_BIN}"
  fi

  add_path_to_shell_profiles "${CODEX_DIR}"
}

Then at the end of install_codex(), after the install and version print:

  printf "%s Installed Codex CLI: %s\n" "$${BOLD}" "$(codex --version)"
  ensure_codex_in_path

And also call ensure_codex_in_path in the skip-install branch so pre-baked images also get the symlink:

  if [ "$${ARG_INSTALL}" != "true" ]; then
    echo "Skipping Codex installation as per configuration."
    ensure_codex_in_path
    return
  fi

This mirrors exactly what claude-code v5 does with ensure_claude_in_path.

---

Issue 2: PR description says install_codex removed, but it's retained (Low — doc nit)

I see the PR body now has install_codex under "Retained", but the initial automated review (DEREM-15) flagged an earlier version of the description that listed it under "Removed variables". This may already be fixed — just double-check the description matches the current code state.

---

Issue 3: OPENAI_API_KEY env-var auth broken on Codex ≥ 0.12x (High — merge blocker)

The module sets OPENAI_API_KEY via coder_env and assumes Codex picks it up. With current Codex CLI versions, this results in 401s — the env var alone doesn't work reliably.

The old install.sh had this covered with an add_auth_json() function that wrote {"OPENAI_API_KEY": "..."} directly to ~/.codex/auth.json. This was dropped in the refactor.

Upstream bugs confirming the env-var path is broken:

• openai/codex#20871 — codex login --with-api-key silently fails to write auth.json on Linux
• openai/codex#15151 — env var silently overrides OAuth token, causing 401s

Suggested fix — add an add_auth_json function to install.sh.tftpl and call it at the bottom when AI Gateway is not enabled. This needs a new template variable for the API key:

In main.tf, add ARG_OPENAI_API_KEY to the templatefile call:

  install_script = templatefile("${path.module}/scripts/install.sh.tftpl", {
    ARG_INSTALL                = tostring(var.install_codex)
    ARG_CODEX_VERSION          = var.codex_version != "" ? base64encode(var.codex_version) : ""
    ARG_WORKDIR                = local.workdir != "" ? base64encode(local.workdir) : ""
    ARG_BASE_CONFIG_TOML       = var.base_config_toml != "" ? base64encode(var.base_config_toml) : ""
    ARG_ADDITIONAL_MCP_SERVERS = var.additional_mcp_servers != "" ? base64encode(var.additional_mcp_servers) : ""
    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
    ARG_OPENAI_API_KEY         = var.openai_api_key != "" ? base64encode(var.openai_api_key) : ""
  })

In install.sh.tftpl, add the variable decode and function:

ARG_OPENAI_API_KEY=$(echo -n '${ARG_OPENAI_API_KEY}' | base64 -d)

function add_auth_json() {
  if [ "$${ARG_ENABLE_AI_GATEWAY}" = "true" ] || [ -z "$${ARG_OPENAI_API_KEY}" ]; then
    return
  fi

  local auth_path="$HOME/.codex/auth.json"
  mkdir -p "$(dirname "$${auth_path}")"

  cat << EOF > "$${auth_path}"
{
  "OPENAI_API_KEY": "$${ARG_OPENAI_API_KEY}"
}
EOF
  echo "Seeded auth.json with API key"
}

Then add the call at the bottom of the script:

install_codex
populate_config_toml
setup_workdir
add_auth_json

This restores the behavior from the old install.sh and matches how Codex actually resolves credentials on Linux.

---

Issue 4: AI Gateway + custom base_config_toml doesn't auto-select provider (Low — documented, OK)

When both enable_ai_gateway = true and a custom base_config_toml are set, the script appends [model_providers.aigateway] but doesn't inject model_provider = "aigateway" into the user's custom config. The README's > [!NOTE] block already warns about this. No code fix needed — just flagging it for completeness.

---

Issue 6: coder_app.command re-runs on reconnect — README should warn (Low — doc fix)

The README example at lines 53-64 shows interactive codex in a coder_app, which is fine because the process stays alive. But the obvious adaptation — codex exec "$PROMPT" — exits after one round-trip, and coder_app re-launches on every pane reconnect, spawning parallel processes.

Suggested fix — add a note after the coder_app example:

> [!NOTE]
> The `coder_app` command re-executes on every pane reconnect. This works for interactive `codex` (which stays alive),
> but one-shot commands like `codex exec` will re-run each time. For one-shot prompts, use a `coder_script`
> (runs once at startup) and a `coder_app` that attaches to the existing session (e.g. via tmux/screen).

---

Summary

#	Issue	Severity	Action
1	codex not symlinked / PATH not persisted	Medium	Code fix needed — port ensure_codex_in_path pattern from claude-code v5
2	PR description vs install_codex	Low	Verify PR body is up to date
3	OPENAI_API_KEY env-var auth broken	High	Code fix needed — restore auth.json seeding from old install.sh
4	AI Gateway + custom config	Low	Already documented, no fix
6	coder_app reconnect re-runs	Low	README note

Issues #1 and #3 are regressions vs both the old codex v4 script and the merged claude-code v5 module. Happy to help with the implementation if useful.

---

This comment was generated with the help of Coder Agents on behalf of @DevelopmentCats.

[35C4n0r]

Addressed all findings from the testing review:

Issue #1 (codex not on PATH): Fixed. Ported ensure_codex_in_path and add_path_to_shell_profiles from claude-code v5. The install script now symlinks codex into $CODER_SCRIPT_BIN_DIR and persists the binary directory to all shell profiles (bash, zsh, fish). Called after both the install and skip-install branches.

Issue #2 (PR description): Already fixed in an earlier commit; the PR body lists install_codex under "Retained".

Issue #3 (OPENAI_API_KEY auth broken): Fixed. Restored add_auth_json() from v4. When AI Gateway is not enabled and an API key is provided, the script writes it to ~/.codex/auth.json. The API key is base64-encoded in main.tf and decoded in the template.

Issue #4 (AI Gateway + custom config): Acknowledged, already documented in the README's [!NOTE] block. No code fix needed.

Issue #6 (coder_app reconnect): Fixed. Added a [!NOTE] after the coder_app example explaining that commands re-execute on pane reconnect and recommending coder_script for one-shot prompts.

All 15 bun tests and 11 terraform tests pass.

This comment was generated by Coder Agents on behalf of @35C4n0r.

[DevelopmentCats]

Addressed all findings from the testing review:

Issue #1 (codex not on PATH): Fixed. Ported ensure_codex_in_path and add_path_to_shell_profiles from claude-code v5. The install script now symlinks codex into $CODER_SCRIPT_BIN_DIR and persists the binary directory to all shell profiles (bash, zsh, fish). Called after both the install and skip-install branches.

Issue #2 (PR description): Already fixed in an earlier commit; the PR body lists install_codex under "Retained".

Issue #3 (OPENAI_API_KEY auth broken): Fixed. Restored add_auth_json() from v4. When AI Gateway is not enabled and an API key is provided, the script writes it to ~/.codex/auth.json. The API key is base64-encoded in main.tf and decoded in the template.

Issue #4 (AI Gateway + custom config): Acknowledged, already documented in the README's [!NOTE] block. No code fix needed.

Issue #6 (coder_app reconnect): Fixed. Added a [!NOTE] after the coder_app example explaining that commands re-execute on pane reconnect and recommending coder_script for one-shot prompts.

All 15 bun tests and 11 terraform tests pass.

This comment was generated by Coder Agents on behalf of @35C4n0r.

I will go ahead and test again, but knock on wood we should be good to merge this, this morning.

[DevelopmentCats]

Tested and Approved

[matifali]

a few nits i think we shoudla ddress and then good to merge.