test(codex): stop two migration tests spawning the real codex CLI - #5
Merged
Merged
Conversation
`migrate()` documents `discover_plugins` as "when True (default), query `plugin/list` against the live codex CLI ... Set False to skip the subprocess spawn (for tests or restricted environments)". Two tests never opt out. They assert config.toml RENDERING, not plugin discovery, but leave the parameter at its default, so each run spawns the real codex CLI, which shells out to `git ls-remote https://github.com/openai/plugins.git HEAD` -- a live network call to a third-party repository from a unit test. Every other test in the file already gets this right: the ones that pass `discover_plugins=True` monkeypatch `_query_codex_plugins` first. These two just missed the switch. Both tests PASS either way, which is why it went unnoticed -- the same silent shape as the update-flow leak fixed in eff88d7. A suite that reaches the network is non-hermetic (fails offline, depends on a third party's repo staying reachable) and slower for no benefit. Verified with a logging `git` shim that records every git invocation including those from grandchild processes: before: 1 call -- `ls-remote https://github.com/openai/plugins.git HEAD` after: 0 calls 20 passed in both cases.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two tests in
tests/hermes_cli/test_codex_runtime_plugin_migration.pyspawn the real codex CLI on every run, which shells out togit ls-remote https://github.com/openai/plugins.git HEAD— a live network call to a third-party repository, from a unit test.Why it happens
migrate()already documents the switch:Both tests assert
config.tomlrendering, not plugin discovery, but neither passes the parameter, so it defaults toTrue.Every other test in the file already handles this correctly: the ones that genuinely exercise discovery pass
discover_plugins=Trueand monkeypatch_query_codex_pluginsfirst. These two simply missed the switch.Why it matters
The tests pass either way, which is why this went unnoticed — the same silent shape as the update-flow leak fixed in
eff88d7a8. The costs are real though:Verification
Measured with a logging
gitshim that records every invocation, including from grandchild processes:ls-remote https://github.com/openai/plugins.git HEADRun in a throwaway clone with an isolated venv — never in an install or a runtime checkout.