Skip to content

fix(cli): wire --terminal-reviewer-provider/acp-command through to config#26

Open
stephanbrez wants to merge 1 commit into
Intelligent-Internet:mainfrom
stephanbrez:fix/terminal-reviewer-cli-flags-dropped
Open

fix(cli): wire --terminal-reviewer-provider/acp-command through to config#26
stephanbrez wants to merge 1 commit into
Intelligent-Internet:mainfrom
stephanbrez:fix/terminal-reviewer-cli-flags-dropped

Conversation

@stephanbrez

Copy link
Copy Markdown

Problem

The --terminal-reviewer-provider and --terminal-reviewer-acp-command flags were accepted by click but silently dropped in _resolve_selection: ProviderSelection had no terminal_reviewer fields, so the values never reached env() and were never written to .mcp.json or .codex/config.toml.

HarnessConfig already reads ZENITH_TERMINAL_REVIEWER_PROVIDER and ZENITH_TERMINAL_REVIEWER_ACP_COMMAND with a full terminal_reviewer -> validator -> worker cascade — only the write side was broken.

Fix

  • Add terminal_reviewer and terminal_reviewer_acp_command fields to ProviderSelection, with resolved_terminal_reviewer / resolved_terminal_reviewer_acp_command properties mirroring the existing validation_worker pattern.
  • Update env() to emit the terminal-reviewer vars only when they differ from the validator's (the cascade parent), matching how the validator compares against the worker.
  • Include the terminal reviewer in providers() and the skill dirs so its assets get installed when it's a distinct provider.

Tests

  • terminal-reviewer provider + acp-command land in .mcp.json env.
  • unset flags emit no terminal-reviewer env keys (cascade handles it).
  • three distinct providers (claude/codex/hermes) all write their env vars in a single init.
  • round-trip: a distinct terminal reviewer written by env() is read back intact by HarnessConfig.discover().
  • round-trip: with no explicit terminal reviewer, env() omits the vars and discover() reconstructs the validator as the cascade fallback.

All three CI checks pass locally: ruff check ., mypy src, pytest -q (207 passed, 7 skipped).

Note: issue creation is restricted on this repo, so per CONTRIBUTING.md's fallback for small fixes this PR is filed directly.

🤖 Generated with Claude Code

…nfig

The --terminal-reviewer-provider and --terminal-reviewer-acp-command
flags were accepted by click but silently dropped in _resolve_selection:
ProviderSelection had no terminal_reviewer fields, so the values never
reached env() and were never written to .mcp.json or .codex/config.toml.

HarnessConfig already reads ZENITH_TERMINAL_REVIEWER_PROVIDER and
ZENITH_TERMINAL_REVIEWER_ACP_COMMAND with a full terminal_reviewer ->
validator -> worker cascade — only the write side was broken.

Add terminal_reviewer and terminal_reviewer_acp_command fields to
ProviderSelection, with resolved_terminal_reviewer and
resolved_terminal_reviewer_acp_command properties mirroring the
validation_worker pattern. Update env() to emit the terminal-reviewer
vars only when they differ from the validator's (the cascade parent),
matching the validator's comparison against the worker. Update
providers() and skill dirs to include the terminal reviewer for
asset installation when it's a distinct provider.

Issue creation is restricted on this repository, so a PR is filed
directly per CONTRIBUTING.md's fallback for small fixes.

Tests:
- terminal-reviewer provider + acp-command land in .mcp.json env.
- unset flags emit no terminal-reviewer env keys (cascade handles it).
- three distinct providers (claude/codex/hermes) all write their env
  vars in a single init.
- round-trip: a distinct terminal reviewer written by env() is read
  back intact by HarnessConfig.discover().
- round-trip: with no explicit terminal reviewer, env() omits the vars
  and discover() reconstructs the validator as the cascade fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant