fix: script runner misidentifies 'codex' in model names as the codex runtime binary#397
Conversation
…runtime binary (microsoft#396) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@microsoft-github-policy-service agree [company="Helix, Inc"] |
|
@ryan-niemes-helix the command you issued was incorrect. Please try again. Examples are: and |
There was a problem hiding this comment.
Pull request overview
Fixes a script-runner command transformation bug where codex substrings inside Copilot --model values could cause the command to be rewritten as a codex exec ... invocation, and adds a regression test.
Changes:
- Tightens the
codexdetection regex in_transform_runtime_commandto avoid matchingcodexas a substring (e.g.,gpt-5.3-codex). - Adds a unit test ensuring Copilot commands with
--model gpt-5.3-codexare not mis-transformed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/apm_cli/core/script_runner.py |
Adjusts codex pattern matching to reduce false positives during runtime command transformation. |
tests/unit/test_script_runner.py |
Adds a regression test covering codex appearing inside a Copilot model name. |
| if re.search(r"(^|\s)codex\s+.*" + re.escape(prompt_file), command): | ||
| match = re.search( | ||
| r"codex\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command |
There was a problem hiding this comment.
The updated codex detection still matches codex anywhere in the command (as long as it’s preceded by whitespace). That means a copilot command like copilot --model codex -p hello-world.prompt.md would still hit this codex branch and be rewritten to codex exec ..., because codex -p hello-world.prompt.md satisfies the regex. Since this section is explicitly for commands without env-var prefixes, the codex transform should only trigger when codex is the leading command token (optionally after leading whitespace), and the extraction regex should be aligned to that same anchored shape.
| if re.search(r"(^|\s)codex\s+.*" + re.escape(prompt_file), command): | |
| match = re.search( | |
| r"codex\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command | |
| # Only trigger when 'codex' is the leading command token (optionally after whitespace) | |
| if re.search(r"^\s*codex\s+.*" + re.escape(prompt_file), command): | |
| match = re.search( | |
| r"^\s*codex\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command |
| def test_transform_runtime_command_copilot_with_codex_in_model_name(self): | ||
| """Test copilot command with --model flag containing 'codex' substring is not misidentified.""" | ||
| original = "copilot --allow-all-tools --model gpt-5.3-codex -p hello-world.prompt.md" | ||
| result = self.script_runner._transform_runtime_command( | ||
| original, "hello-world.prompt.md", self.compiled_content, self.compiled_path | ||
| ) | ||
| assert result == "copilot --allow-all-tools --model gpt-5.3-codex" |
There was a problem hiding this comment.
This regression test covers the substring case (gpt-5.3-codex), but the current codex regex would still misidentify copilot if the model value is exactly codex (e.g., copilot --model codex -p hello-world.prompt.md). Adding a test for that case will prevent future regressions and will fail with the current implementation until the codex detection is tightened to only match when codex is the actual command token.
Closes #396
Summary
codexregex in_transform_runtime_commandwith(^|\s)so it only matchescodexas a standalone command token, not as a substring of flag values like--model gpt-5.3-codexcopilot --allow-all-tools --model gpt-5.3-codex -p fix-issue.prompt.mdTest plan
test_transform_runtime_command_copilot_with_codex_in_model_namepasses🤖 Generated with Claude Code