test(adapters): live codex install gate — real-cli verification that skips when codex is absent#397
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the Codex adapter surface and adds coverage to catch real-world regressions by exercising the actual codex CLI when it’s available, while also introducing Codex rollout ingest support and installer enhancements needed for reliable Codex project installs.
Changes:
- Add a live “real Codex CLI” gate test (
codex mcp list --json) that installs the adapter into a temp project and validates visibility under trusted vs untrusted projects. - Implement Codex rollout ingestion (
vouch capture ingest-codex, including--latestand--hookmode) with idempotent re-ingest that refreshes a pending proposal in place. - Enhance the adapter installer with
toml_mergesupport and fenced-snippet refresh behavior; expand Codex adapter tiers (T1–T4) and documentation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_install_adapter.py | Adds/extends unit coverage for Codex adapter tiers (T1–T4), toml merge, fenced refresh, skills, hooks merge/idempotency. |
| tests/test_codex_rollout.py | Adds tests for parsing Codex rollouts and ingesting them into review-gated proposals (including hook mode and latest resolution). |
| tests/test_codex_adapter_load_real.py | New live gate that runs the real codex CLI (skips if absent) to validate the installed adapter is accepted/visible. |
| tests/fixtures/codex/rollout-basic.jsonl | Adds a representative Codex rollout fixture with session_meta, tool calls, and outputs. |
| tests/fixtures/codex/rollout-no-meta.jsonl | Adds a negative fixture missing session_meta to assert actionable errors. |
| src/vouch/storage.py | Adds KBStore.update_proposal() to refresh an existing proposal file in place. |
| src/vouch/install_adapter.py | Adds toml_merge install strategy and fenced snippet “refresh” behavior for drifted fences. |
| src/vouch/codex_rollout.py | New Codex rollout parser + ingest logic (dedupe + pending refresh) and hook payload handler. |
| src/vouch/cli.py | Adds vouch capture ingest-codex CLI surface with --latest, --hook, and --codex-home. |
| adapters/codex/README.md | Documents Codex T3 skills and T4 automatic capture behavior. |
| adapters/codex/install.yaml | Expands Codex tiers and switches config install to toml_merge; adds fenced snippet and hooks merge wiring. |
| adapters/codex/hooks.json | Adds Codex Stop hook wiring to vouch capture ingest-codex --hook. |
| adapters/codex/AGENTS.md.snippet | Adds Codex AGENTS.md snippet content for T2 instructions. |
| """Tier-2 e2e: the real Codex CLI reads the installed adapter (#389). | ||
|
|
||
| The unit tests in test_install_adapter.py assert what the installer writes; | ||
| nothing there proves the real codex CLI accepts it — the old T1 silent-skip | ||
| behavior (installer no-op whenever `.codex/config.toml` existed) is exactly | ||
| the class of bug only a live gate catches. This suite closes that gap, |
| def _trust(home: Path, project: Path) -> None: | ||
| # Codex loads project-scoped .codex/ layers only for trusted projects; | ||
| # this is the non-interactive equivalent of answering the trust prompt. | ||
| with (home / "config.toml").open("a", encoding="utf-8") as fh: | ||
| fh.write(f'[projects."{project}"]\ntrust_level = "trusted"\n') | ||
|
|
| def _mcp_servers(env: dict[str, str], cwd: Path) -> list[dict]: | ||
| result = _run_codex(env, cwd, "mcp", "list", "--json") | ||
| assert result.returncode == 0, ( | ||
| f"codex mcp list failed.\nstdout: {result.stdout}\nstderr: {result.stderr}" | ||
| ) | ||
| # Defensive: skip any non-JSON preamble (codex warns on stderr about | ||
| # tmp-dir helper binaries, but keep stdout parsing tolerant anyway). | ||
| payload = result.stdout[result.stdout.index("[") :] | ||
| servers = json.loads(payload) | ||
| assert isinstance(servers, list) | ||
| return servers |
| (codex_dir / "hooks.json").write_text(json.dumps({ | ||
| "hooks": { | ||
| "Stop": [ | ||
| {"hooks": [{"type": "command", "command": "my-own-stop-hook"}]} | ||
| ] | ||
| } | ||
| })) |
| raise CodexRolloutError( | ||
| f"{path.name}: no session_meta record found — this doesn't look " | ||
| f"like a codex rollout, or its schema has drifted; expected the " | ||
| f"first line to be a session_meta with an `id`" | ||
| ) |
00223af to
83e9c6f
Compare
|
addressed the copilot review: |
with `vouch capture ingest-codex` available, codex can self-capture the way claude-code T4 does. the ticket assumed the `notify` setting in config.toml, but codex only honours notify in user-global config — it is a restricted key in project-local layers — and the vouchdev#179 rule forbids touching ~/.codex. codex's hooks system is the project-local equivalent: `.codex/hooks.json` (loaded in trusted projects) fires Stop when a turn completes, with a payload carrying the session id and transcript path. T4 ships that file through json_merge, so an existing user hooks.json is deep-merged into, never overwritten, and re-runs don't duplicate the hook. the handler is a new --hook mode on ingest-codex: read the stop payload from stdin, resolve the session's rollout (transcript_path when present, else by session id in the rollout filename), ingest idempotently, and exit 0 no matter what — the same never-break-the- host rule capture observe follows. stop fires per turn, not per session end, so the finalize policy the ticket left open is resolved as idempotent re-ingest: the dedup guard now refreshes a session's still-PENDING proposal in place when the rollout grew (same id, updated summary, audit-logged), reports an unchanged rollout as a no-op, and never resurrects a proposal a human already decided. storage gains update_proposal, a pure-I/O in-place rewrite of a pending proposal file. wiring only — everything still lands through propose_page; nothing touches approve(). closes vouchdev#388
tests/test_openclaw_plugin_load_real.py set the pattern: exercise the real host cli when it's on PATH, skip cleanly where it isn't. the codex adapter had no equivalent, so regressions in the shipped config only surfaced when a user hit them — the old T1 silent-skip no-op is exactly the class of bug a live gate would have caught. the new suite installs the adapter into a temp project at T4, marks the project trusted inside an isolated CODEX_HOME, and asserts through `codex mcp list --json` that the vouch server is visible: next to a pre-seeded unrelated server on the merge path, alone on the fresh path, and absent for an untrusted project (the config must stay inert where codex says it does). a full-tier check covers the fenced AGENTS.md, the skills, and the Stop hook, and an isolation check pins every written artifact under the temp project. assertions target the observable contract — server listed, config parses, snippet present — not codex internals, so version bumps shouldn't break the suite. no network, no credentials, never touches the real ~/.codex; runs in the normal pytest invocation. closes vouchdev#389
83e9c6f to
f8020a2
Compare
tests/test_openclaw_plugin_load_real.pyset the pattern: a live gate that exercises the real host cli when it's on PATH and skips (not fails) where it isn't. the codex adapter had no equivalent, so regressions in the shipped config only surfaced when a user hit them — the T1 silent-skip no-op that #392 fixes is exactly the class of bug a live gate would have caught.the new
tests/test_codex_adapter_load_real.pyinstalls the adapter into a temp project at the highest shipped tier, marks the project trusted inside an isolatedCODEX_HOME, and asserts through codex itself —codex mcp list --json— that the vouch server is listed. both required live paths are covered: the merge path (a pre-seeded.codex/config.tomlwithmodeland an unrelated[mcp_servers.other]survives, still parses as valid toml, and codex lists both servers side by side) and the fresh-install path (vouch is the only server). a third case pins the trust boundary: for an untrusted project the installed config stays inert, exactly as codex documents.assertions target the observable contract — server listed, config parses, agents snippet present, skills and Stop hook on disk — not codex internals, so codex version bumps shouldn't break the suite. isolation is by construction: every codex invocation runs with a throwaway
CODEX_HOMEand a temp project cwd, the real~/.codexis never in play, andcodex mcp listreads local config only — no network, no credentials. an explicit test also pins every installed artifact under the temp project (the #179 invariant at the live level).verified against the real codex cli 0.142.0 on this machine: all five tests pass in ~0.2s (it's a fast local config read), and the suite runs in the normal pytest invocation with a clean module-level skip where codex is absent — no separate ci job needed.
on scope: because these prs are stacked, github shows the cumulative diff (the ancestor commits from #392–#396 plus this pr's own commit). this branch's own change is a single new test file,
tests/test_codex_adapter_load_real.py; once the ancestors merge intotest, the diff reduces to that one file. merge this last of the code prs.closes #389