Skip to content

fix(acp): integrate #31 with robust Codex config parsing - #41

Merged
TueVNguyen merged 3 commits into
mainfrom
integration/codex-acp-config
Sep 6, 2026
Merged

TueVNguyen merged 3 commits into
mainfrom
integration/codex-acp-config

Conversation

@TueVNguyen

Copy link
Copy Markdown
Collaborator

Integrates #31 by @stephanbrez, preserving the contributor’s original commit and authorship.

The original change routes Codex -c overrides through CODEX_CONFIG, which the npm ACP adapter reads, so model and per-role reasoning settings reach workers and terminal reviewers.

This PR also addresses two issues found during review:

  • Parse shell-quoted and unquoted config assignments, including --config, with the last assignment taking precedence.
  • Reject malformed or non-object CODEX_CONFIG instead of silently replacing it with defaults.

Configuration is validated before starting MCP helpers, preventing invalid configuration from leaving helper processes running. Regression tests cover quoting, repeated overrides, invalid JSON, and validation before startup across worker, validator, and terminal-reviewer roles. The existing sandbox and approval policy remains unchanged.

Supersedes #31 for integration purposes, Fixes #27 .

Validation:

  • Python 3.12: 254 tests passed, 7 skipped.
  • Ruff and mypy passed.
  • Git whitespace checks passed.

Live ACP smoke tests were skipped.

stephanbrez and others added 3 commits July 25, 2026 20:03
The npm @agentclientprotocol/codex-acp adapter (v1.1.0+, verified against
1.1.2) does not parse -c overrides from argv — it reads its startup
configuration from the CODEX_CONFIG env var (JSON), parsed at
src/CodexAcpClient.ts and spread unchanged into codex's thread/start
config. The -c flags appended by _augment_acp_command (sandbox_mode,
approval_policy, model_reasoning_effort) were silently discarded — sandbox,
approval policy, and reasoning effort never reached the worker (issue #27).

Build CODEX_CONFIG in _acp_subprocess_env in three layers, each overriding
the previous:

1. User-supplied CODEX_CONFIG (ambient env).
2. -c overrides parsed from the augmented command string, via
   _parse_codex_c_overrides (handles both double- and single-quoted values).
3. Zenith's three safety keys (sandbox_mode, approval_policy,
   model_reasoning_effort) — always win, since sandbox/approval are
   autonomy-safety requirements and effort is the per-role resolved value.

The -c flags remain in argv for -c-honoring codex-acp builds (harmless if
ignored by the npm adapter). _acp_subprocess_env now accepts reasoning_effort
and acp_command; both call sites (run_node, run_terminal_review) thread
role_config.worker_reasoning_effort and the augmented command through.

Additionally, routing all -c overrides through CODEX_CONFIG lets a
user-specified codex model reach the adapter — previously dropped for the
same reason. A model supplied via ZENITH_*_ACP_COMMAND (-c model="...") or
ambient CODEX_CONFIG is preserved (layers 1–2) while zenith's safety keys
still override (layer 3). This model-override support was requested in the
issue #27 comments; it is not part of the original issue but rides on the
same mechanism.

Verified end-to-end against the installed adapter: the adapter's startup
log (app-server.log) shows codexConfig with model, sandbox_mode,
approval_policy, and model_reasoning_effort all received.

Tests:
- _parse_codex_c_overrides extracts model + sandbox from double and single
  quoted -c flags; empty when no flags present.
- CODEX_CONFIG JSON present with correct effort; defaults to xhigh.
- User CODEX_CONFIG merged (preserves ambient model); command-string model
  wins over ambient model.
- Custom model from augmented command preserved alongside zenith's safety
  keys.
- Zenith's safety keys override user-supplied safety values; invalid user
  JSON replaced. Claude path has no CODEX_CONFIG.

Fixes #27.
@TueVNguyen
TueVNguyen merged commit a8d9b57 into main Sep 6, 2026
3 checks passed
DiegoMMF added a commit to DiegoMMF/zenith that referenced this pull request Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codex worker -c overrides never reach the npm codex-acp adapter (sandbox, approval policy, reasoning effort silently dropped)

2 participants