fix: prefer APM-managed runtimes over system PATH and warn on codex/GitHub Models incompatibility#1356
Conversation
…itHub Models incompatibility Scenario A: _detect_installed_runtime used shutil.which() which finds system PATH stubs (e.g. gh copilot shim) before checking ~/.apm/runtimes/. Add find_runtime_binary() utility that checks APM-managed binaries first, falling back to PATH. Replace all runtime detection sites across script_runner, codex_runtime, copilot_runtime, and MCP integrators. Binary path is resolved at subprocess execution time, not command generation time, to preserve downstream parser compatibility. Scenario B: apm runtime setup codex installs v0.118.0 which requires wire_api=responses, but GitHub Models only exposes Chat Completions (no /responses endpoint). Add a clear compatibility warning in setup-codex.sh and setup-codex.ps1 that fires when the resolved version is >= 0.116. Fixes microsoft#605 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam
added
the
panel-review
There was a problem hiding this comment.
Pull request overview
This PR attempts to make runtime resolution prefer APM-managed binaries and adds Codex/GitHub Models compatibility warnings for newer Codex releases.
Changes:
- Adds
find_runtime_binary()to resolve~/.apm/runtimesbefore PATH. - Replaces selected runtime availability/probing calls with the new resolver.
- Adds Codex >= 0.116 compatibility warnings in Bash and PowerShell setup scripts, plus unit tests for the resolver.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/runtime/utils.py |
Adds shared runtime binary resolution helper. |
src/apm_cli/core/script_runner.py |
Uses the resolver for runtime detection and subprocess execution. |
src/apm_cli/runtime/codex_runtime.py |
Uses the resolver for Codex availability and execution. |
src/apm_cli/runtime/copilot_runtime.py |
Uses the resolver for Copilot availability. |
src/apm_cli/integration/mcp_integrator.py |
Uses the resolver for MCP runtime filtering. |
src/apm_cli/integration/mcp_integrator_install.py |
Uses the resolver in MCP install fallback/runtime probes. |
scripts/runtime/setup-codex.sh |
Resolves latest into CODEX_VERSION and prints compatibility warning. |
scripts/runtime/setup-codex.ps1 |
Resolves latest into $Version and prints compatibility warning. |
tests/unit/runtime/test_runtime_utils.py |
Adds unit tests for runtime binary resolution behavior. |
tests/unit/runtime/__init__.py |
Adds package marker for runtime unit tests. |
tests/unit/test_script_runner.py, tests/unit/test_runtime_windows.py, tests/unit/test_codex_runtime.py |
Updates mocks/tests to use the new resolver. |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | Clean utility extraction; RuntimeManager contains a diverged APM-first lookup (no X_OK check, no .exe suffix) that is a live gate -- must unify with find_runtime_binary. |
| CLI Logging Expert | 0 | 2 | 2 | Warning content is accurate and actionable, but bypasses log_warning/Write-WarningText helpers used everywhere else in the same files -- consistency gap, not a correctness regression. |
| DevX UX Expert | 0 | 1 | 2 | PATH priority fix is clean; codex warning fires post-download (any warning beats silent 404); warning text is copy-paste ready with two clear remediation options. |
| Supply Chain Security Expert | 0 | 3 | 1 | find_runtime_binary() bypasses path_security guards at the one user-controlled call-site; new user-writable binary substitution surface; scope silently expanded from Windows-only to all platforms. |
| OSS Growth Hacker | 0 | 2 | 1 | Two adoption wins ship without CHANGELOG; hardcoded rust-v0.115.0 version pin will rot as codex releases progress. |
| Doc Writer | 2 | 1 | 1 | runtime-compatibility.md and runtime.md describe GitHub Models as working default for codex -- factually incorrect for the default-installed version (0.118.0); live misinformation on the primary onboarding surface. |
| Test Coverage Expert | 0 | 2 | 1 | Unit tier well-satisfied (11/11); no integration-with-fixtures test exercises APM binary priority through an actual subprocess -- core user promise certified at mock tier only. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Doc Writer] (blocking-severity) Add caution callout to runtime-compatibility.md and update runtime.md table: codex >= 0.116 does not support GitHub Models; APM default install (0.118.0) is above threshold -- Blocking severity, live documentation correctness regression on the primary onboarding surface -- new users will follow the doc and hit the exact 404 the warning tries to prevent.
- [Supply Chain Security Expert] (blocking-severity) Add validate_path_segments(name) and ensure_path_within() at the top of find_runtime_binary() before constructing apm_path; restrict find_runtime_binary() calls in script_runner.py to a known allowlist of APM-managed runtime names -- script_runner.py:580 passes shlex-split user apm.yml input directly; path_security.py already exists for this pattern -- bypassing it is a concrete regression of APM's security contract.
- [Python Architect] Replace inline APM-first lookup in RuntimeManager.is_runtime_available() and list_runtimes() with delegation to find_runtime_binary(); fixes missing is_file+X_OK and .exe suffix gaps at the mcp_integrator_install.py gate -- RuntimeManager is a live gate in mcp_integrator_install.py; the diverged implementation will report non-executable directories or dangling symlinks as available, and silently fails on Windows.
- [CLI Logging Expert] Replace bare echo/Write-Host calls in setup-codex.sh and setup-codex.ps1 warning blocks with log_warning/Write-WarningText; extract LAST_COMPAT_VERSION constant with maintenance comment -- Two independent personas (cli-logging-expert and devx-ux-expert) flagged the same inconsistency; consistency gap will silently diverge as logging helpers evolve; hardcoded version pin will rot.
- [Test Coverage Expert] Add integration-with-fixtures test: APM binary installed in ~/.apm/runtimes/ -> find_runtime_binary() called -> subprocess launched with APM-managed path (tests/integration/test_runtime_binary_priority_e2e.py) -- The core user promise -- "apm-installed binary always wins over system PATH" -- is only certified at unit tier with mocks; missing on a behavior-change surface is a regression-trap gap that will not survive the next refactor.
Architecture
classDiagram
direction LR
class find_runtime_binary {
<<PureFunction>>
+find_runtime_binary(name: str) str | None
}
note for find_runtime_binary "Priority chain:\n1. ~/.apm/runtimes/<name>[.exe] (is_file + X_OK)\n2. shutil.which(name)"
class ScriptRunner {
<<IOBoundary>>
+run_script(script, env) CompletedProcess
}
class CodexRuntime {
+execute_prompt(prompt) None
+is_available() bool
}
class CopilotRuntime {
+is_available() bool
}
class RuntimeManager {
<<Strategy>>
+runtime_dir: Path
+is_runtime_available(name) bool
+list_runtimes() dict
}
note for RuntimeManager "Parallel inline APM-first lookup\n(diverged from find_runtime_binary)\nDoes NOT check os.access or .exe suffix"
class MCPIntegrator {
+_detect_installed_runtime() list
}
class MCPIntegratorInstall {
+install(scope) list
}
ScriptRunner ..> find_runtime_binary : resolves binary
CodexRuntime ..> find_runtime_binary : resolves binary
CopilotRuntime ..> find_runtime_binary : resolves binary
MCPIntegrator ..> find_runtime_binary : availability check
MCPIntegratorInstall ..> find_runtime_binary : availability check
MCPIntegratorInstall ..> RuntimeManager : is_runtime_available gate
RuntimeManager ..> find_runtime_binary : should delegate (does not yet)
class find_runtime_binary:::touched
class ScriptRunner:::touched
class CodexRuntime:::touched
class CopilotRuntime:::touched
class MCPIntegrator:::touched
class MCPIntegratorInstall:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([CLI command / adapter call]) --> B{caller type}
B -->|script_runner.py:580| C[find_runtime_binary name]
B -->|codex_runtime.py:42| C
B -->|copilot_runtime.py:172| C
B -->|mcp_integrator.py:840| C
B -->|mcp_integrator_install.py:191| C
C --> D{"[FS] ~/.apm/runtimes/name.exe\nWindows only\nis_file + X_OK?"}
D -->|yes| E[return apm_path_exe]
D -->|no| F{"[FS] ~/.apm/runtimes/name\nis_file + X_OK?"}
F -->|yes| G[return apm_path]
F -->|no| H[[shutil.which name\nsystem PATH scan]]
H --> I{found?}
I -->|yes| J[return system path]
I -->|no| K[return None]
B -->|mcp_integrator_install.py:195\nmanager gate| L[RuntimeManager.is_runtime_available]
L --> M{"[FS] self.runtime_dir/binary\nexists only - no X_OK check"}
M -->|yes| N[return True]
M -->|no| O[[shutil.which binary_name\nno .exe suffix on Windows]]
O --> P[return bool]
style C fill:#fff3b0,stroke:#d47600
style L fill:#ffdddd,stroke:#cc0000
sequenceDiagram
participant CLI
participant ScriptRunner
participant find_runtime_binary
participant FS as ~/.apm/runtimes
participant PATH as shutil.which
CLI->>ScriptRunner: run script with runtime binary
ScriptRunner->>find_runtime_binary: find_runtime_binary("codex")
find_runtime_binary->>FS: is_file(codex[.exe]) + X_OK?
alt APM-managed binary found
FS-->>find_runtime_binary: path
find_runtime_binary-->>ScriptRunner: ~/.apm/runtimes/codex
else fallback
FS-->>find_runtime_binary: not found
find_runtime_binary->>PATH: shutil.which("codex")
PATH-->>find_runtime_binary: system path or None
find_runtime_binary-->>ScriptRunner: system path or None
end
ScriptRunner->>ScriptRunner: replace args[0] with resolved path
ScriptRunner->>ScriptRunner: subprocess.run(actual_command_args)
Recommendation
Hold for two pre-merge fixes: (1) update runtime-compatibility.md and reference/cli/runtime.md to reflect that codex >= 0.116 is incompatible with GitHub Models -- the default-installed version is 0.118.0, so these pages are live misinformation today; (2) apply validate_path_segments() and ensure_path_within() in find_runtime_binary() and restrict the script_runner.py call-site to a known allowlist. Both fixes are small and well-scoped. Once those land, the remaining recommended findings (RuntimeManager delegation, logging helper consistency, integration test, CHANGELOG entries, version pin extraction) are excellent material for a fast follow-up PR and should be tracked as issues, not blockers. The functional core is correct and the adoption story is strong -- this PR should ship this sprint, just not in its current form.
Full per-persona findings
Python Architect
-
[recommended] RuntimeManager duplicates APM-managed-first lookup instead of delegating to find_runtime_binary at
src/apm_cli/runtime/manager.py:285
manager.py lines 279-286 and 318-323 implement the same APM-managed-first / system-PATH-fallback logic asfind_runtime_binary, but with two divergences: (1) it checks only.exists()rather than.is_file() and os.access(X_OK), so a non-executable directory entry or dangling symlink would be reported as installed; (2) it never tries the Windows.exesuffix. The PR migrates every caller of binary detection in the adapter and integrator layers but leaves the Manager's own internal lookup untouched. Sincemanager.is_runtime_available()is used as the primary gate inmcp_integrator_install.pythe divergence is live, not dead code.
Suggested: Replace the shutil.which fallback block in list_runtimes and is_runtime_available withfrom .utils import find_runtime_binary. -
[nit] find_runtime_binary called twice where result could be stored at
src/apm_cli/runtime/manager.py:285
manager.py lines 285-286 call shutil.which(binary_name) twice in the fallback branch.
Suggested: Storepath = shutil.which(binary_name)once, then deriveinstalled = path is not None. -
[nit] Design patterns note
Strategy / Priority Chain pattern used correctly in find_runtime_binary. Collapsing RuntimeManager inline lookup into find_runtime_binary is the only structural win available; no new pattern introduction warranted.
CLI Logging Expert
-
[recommended] setup-codex.sh uses bare
echofor the warning instead oflog_warningatscripts/runtime/setup-codex.sh
Every other warning-level message in setup-codex.sh useslog_warning. The new block callsechodirectly, bypassing the helper that applies consistent prefixing, color, and any future routing changes.
Suggested: Replace bare echo calls with log_warning for the lead line and log_info for continuation lines. -
[recommended] setup-codex.ps1 uses
Write-Hostfor the warning instead ofWrite-WarningTextatscripts/runtime/setup-codex.ps1
The rest of setup-codex.ps1 usesWrite-WarningTextfor warnings andWrite-Info/Write-Successfor other levels. The new block callsWrite-Hostwith hardcoded-ForegroundColor Yellow, duplicating whatWrite-WarningTextalready does and bypassing any future changes to that helper.
Suggested: Use Write-WarningText for the lead line and Write-Info for continuation lines, removing the hardcoded -ForegroundColor Yellow. -
[nit]
[!] WARNING:prefix is redundant -- the helper already provides the symbol atscripts/runtime/setup-codex.sh
If log_warning/Write-WarningText already prepends [!], the message text should start with the fact, not repeat the severity word.
Suggested: Drop "WARNING: " from the message string once the helper call is in place. -
[nit] Pre-existing emoji in setup-codex.sh line 108 violates ASCII-only rule (exposed in PR diff context) at
scripts/runtime/setup-codex.sh
A non-ASCII emoji and the word DEBUG appear in a log_info call. Not introduced by this PR but visible in the diff context.
Suggested: Remove the line entirely or replace with a clean ASCII log_info message if it has operational value.
DevX UX Expert
-
[recommended] Warning fires after download completes, not before -- user pays the install cost before learning the version is incompatible with their provider at
scripts/runtime/setup-codex.sh
When --version is omitted the script resolves the latest tag, downloads the binary, writes config, and only then emits the warning. The version number is available the moment latest_tag is resolved, which is before the download block. Checking codex_minor at that point and exiting early saves bandwidth and avoids writing a broken configuration to disk.
Suggested: Move the version compatibility check to immediately after CODEX_VERSION is resolved and before the download URL is constructed. If GitHub Models is detected as the provider, offer to abort rather than proceeding silently. -
[nit] Warning uses bare echo while the rest of the script uses log_info / log_success helpers -- inconsistent output style at
scripts/runtime/setup-codex.sh
Every other user-facing line in the shell script uses logging helpers. The bash side should use a log_warning helper so the output style matches.
Suggested: Replace bare echo lines with log_warning (define if absent) so the warning renders with the same prefix/style as surrounding messages. -
[nit] Recovery command hardcodes
rust-v0.115.0-- will become stale as codex releases progress atscripts/runtime/setup-codex.sh
No mechanism to update the string automatically. The string lives in two separate script files.
Suggested: Extract to a named variable LAST_COMPAT_VERSION=rust-v0.115.0 at the top of each script with a comment: "# MAINTENANCE: update this when the last GitHub-Models-compatible version changes".
Supply Chain Security Expert
-
[recommended] find_runtime_binary() bypasses path_security guards despite one user-controlled call-site at
src/apm_cli/runtime/utils.py
script_runner.py:580 passes actual_command_args[0] which is shlex-split from an apm.yml scripts: entry -- a project-controlled but potentially attacker-supplied string. A name like '../../evil' would construct ~/.apm/runtimes/../../evil; if that file is executable, find_runtime_binary returns it and subprocess.run executes it. The project's own path_security.py exists precisely to guard this pattern, and bypassing it is a concrete regression relative to APM's stated security contract.
Suggested: Addvalidate_path_segments(name, context='runtime binary name')at the top of find_runtime_binary() to reject traversal sequences. After constructing apm_path, callensure_path_within(apm_path.resolve(), apm_runtimes.resolve()). -
[recommended] User-writable ~/.apm/runtimes/ is now preferred over integrity-verified system PATH binaries at
src/apm_cli/runtime/utils.py
Any malicious same-user process (npm postinstall hook, pip install side-effect) can drop an executable named 'codex', 'copilot', 'gemini', or 'claude' into ~/.apm/runtimes/ and have it silently win over the system-installed version. This is a new binary-substitution surface that did not exist before this PR.
Suggested: At minimum, log a warning when a managed binary is selected over a PATH entry that also exists. Longer-term, verify content-hash against lockfile; assert runtimes directory is not group/world-writable. -
[recommended] script_runner.py: binary resolution scope silently expanded from Windows-only to all platforms at
src/apm_cli/core/script_runner.py
Pre-PR the Windows guard was intentional: handle .cmd/.ps1 shell wrappers without shell=True. Post-PR the code performs ~/.apm/runtimes/ probing for every runtime command on every platform. Any command token from an apm.yml script that matches a filename in ~/.apm/runtimes/ will be silently redirected.
Suggested: Restrict find_runtime_binary() in _execute_runtime_command() to a well-known allowlist of APM-managed runtime names ('copilot', 'codex', 'gemini', 'claude') rather than probing for any arbitrary actual_command_args[0]. -
[nit] Path.is_file() follows symlinks; no symlink-escape guard applied at
src/apm_cli/runtime/utils.py
An attacker with write access to ~/.apm/runtimes/ can place a symlink to a malicious binary elsewhere. Path.is_file() returns True for symlinks pointing to files.
Suggested: Replace apm_path.is_file() with apm_path.resolve().is_file() and wrap in ensure_path_within(apm_path.resolve(), apm_runtimes.resolve()).
OSS Growth Hacker
-
[recommended] CHANGELOG not updated -- missed upgrade and story beat
Both fixes directly improve the first-run experience. CHANGELOG entries are APM's primary mechanism for converting existing users into sharers and upgraders -- skipping them for user-facing fixes leaves adoption value on the table.
Suggested: Add two entries under [Unreleased]: one for PATH priority fix, one for the codex warning with migration path. -
[recommended] Hardcoded
rust-v0.115.0version pin in warning message will rot and become misinformation
The string lives in two separate script files with no shared constant. Any future update requires a manual two-file edit that is easy to miss. A user following a cached error message six months from now will get a stale command.
Suggested: Extract the compatible version floor into a single named constant in each script with a maintenance comment above it. -
[nit] Warning copy says "if you are using GitHub Models" -- but GitHub Models is APM's default codex provider
The conditional phrasing may cause users to skip reading the warning because they assume it does not apply to them. Since APM configures GitHub Models as the default provider, the majority of users ARE affected.
Suggested: Rephrase to: "APM configures GitHub Models as the default codex provider. GitHub Models does not expose the /responses endpoint and will return 404 with codex >= v0.116. Your options:"
Auth Expert -- inactive
No auth files changed (auth.py, token_manager.py, credential resolution untouched); binary path preference change does not alter token injection -- setup_runtime_environment still sets the same env vars regardless of which binary path is resolved.
Doc Writer
-
[blocking] runtime-compatibility.md describes Codex + GitHub Models as working without caveat, but the default-installed codex version (0.118.0) is now incompatible with GitHub Models at
docs/src/content/docs/integrations/runtime-compatibility.md
The Codex section presents GitHub Models as the happy path. Since the default pinned version is rust-v0.118.0 (above the 0.116 threshold), a user following this doc will install a version that cannot use GitHub Models, with no warning in the doc. This is a correctness regression in the documentation.
Suggested: Add a caution callout in the Codex section: codex >= 0.116 is not compatible with the GitHub Models API; useapm runtime setup codex --version <older-version>to pin a compatible release or configure an alternative provider after install. -
[blocking] reference/cli/runtime.md lists Codex default config as "GitHub Models via global ~/.codex/config.toml" -- incorrect for codex >= 0.116 at
docs/src/content/docs/reference/cli/runtime.md
The Supported runtimes table states the default config for codex is GitHub Models. This is now wrong for the version APM installs by default (0.118.0). A user consulting the reference before running setup will have inaccurate expectations.
Suggested: Update the codex row: "GitHub Models (free) for codex < 0.116. Versions >= 0.116 are incompatible with GitHub Models; APM warns during setup." Also reference the --version flag as the escape hatch. -
[recommended] CHANGELOG.md missing entries for both user-facing behavior changes at
CHANGELOG.md
APM follows Keep a Changelog. Neither the binary resolution priority change nor the codex warning appears in [Unreleased].
Suggested: Add two entries under [Unreleased] > Changed/Fixed describing the binary resolution preference and the codex >= 0.116 compatibility warning. -
[nit] runtime-compatibility.md Codex troubleshooting section advises restarting terminal for PATH changes -- less relevant now that APM prefers its own runtime directory at
docs/src/content/docs/integrations/runtime-compatibility.md
The troubleshooting entry "Ensure PATH is updated (restart terminal)" predates the binary-resolution preference change. APM now resolves the managed binary at ~/.apm/runtimes/codex directly.
Suggested: Update to note that APM resolves the managed binary directly; PATH staleness only affects system-installed codex binaries used as fallback.
Test Coverage Expert
-
[recommended] No integration-level test exercises APM-managed binary priority when an actual subprocess is launched at
tests/integration/test_runtime_binary_priority_e2e.py
All unit tests mock find_runtime_binary at the boundary -- they do not exercise the full flow: APM binary installed in ~/.apm/runtimes/ -> find_runtime_binary() called -> subprocess launched with the APM-managed path. The user promise "apm-installed binary always wins over system PATH" is only certified at unit tier. Grepped tests/integration/ for find_runtime_binary, apm_managed, runtimes.*priority -- no match.
Proof (missing):tests/integration/test_runtime_binary_priority_e2e.py::test_apm_managed_binary_preferred_over_system_path-- proves: When a user installs codex via apm runtime setup codex, subsequent apm run calls use the APM-managed binary even if a different codex is on system PATH [multi-harness-support,secure-by-default] -
[recommended] Shell script GitHub Models incompatibility warning has no automated test -- manual-only at
tests/integration/test_runtime_smoke.py
Shell script behavior is classified as manual-only in the tier matrix, so this is not a blocking gap. However, if this warning regresses silently, a user configuring Codex with GitHub Models gets no guidance. Grepped tests/ for "responses endpoint" and "GitHub Models.*404" -- no match.
Proof (manual at manual-only):tests/integration/test_runtime_smoke.py::test_codex_runtime_setup (existing)-- proves: setup-codex.sh runs without error and installs the binary, but does NOT assert the GitHub Models incompatibility warning text is emitted -
[nit] Unit tests for find_runtime_binary are comprehensive and well-structured; 11/11 tests present at
tests/unit/runtime/test_runtime_utils.py
Verified by reading tests/unit/runtime/test_runtime_utils.py in full. All 11 claimed tests are present and exercise APM priority, PATH fallback, None return, non-executable skip, Windows .exe suffix, multi-name, permission fallback, missing-dir handling, and string return type.
Proof (passed at unit):tests/unit/runtime/test_runtime_utils.py::TestFindRuntimeBinary (11 tests)-- proves: APM-managed binary in ~/.apm/runtimes/ is preferred over system PATH; fallback to PATH when absent; None when neither exists; non-executable skipped [secure-by-default,devx]
assert result == str(apm_binary) # test_apm_managed_binary_takes_priority
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by PR Review Panel for issue #1356 · ● 4M · ◷
github-actions
Bot
removed
the
panel-review
…#605) B2 — Path traversal security in find_runtime_binary(): - Reject names containing '/' or '\' (explicit separator guard) - Call validate_path_segments() to catch '..', '.', URL-encoded traversal sequences, and empty segments (absolute paths) - Wrap APM-binary file checks with ensure_path_within() so symlinks pointing outside ~/.apm/runtimes/ are silently skipped - Import PathTraversalError, ensure_path_within, validate_path_segments from apm_cli.utils.path_security B1 — Detection order confirmed correct (APM runtimes before PATH) B3 — Docs: add Codex v0.116+ / GitHub Models compatibility caveat to docs/src/content/docs/integrations/runtime-compatibility.md R1/R2 — setup-codex.sh and .ps1: update warning text to mention wire_api=chat as the fix for GitHub Models compatibility R3 — setup-codex.sh: replace raw echo '[!] WARNING' block with log_warning calls; setup-codex.ps1: replace Write-Host -Yellow block with Write-Warning calls R4 — Extract LAST_COMPAT_VERSION_MINOR=115 constant (sh) and $LastCompatVersionMinor = 115 (ps1) to eliminate magic numbers R5 — CHANGELOG: add three entries under [Unreleased] ### Added Tests: add TestFindRuntimeBinaryPathSecurity class (7 new tests) covering traversal, separator, absolute-path, URL-encoded, and symlink-escape scenarios; all 8515 unit tests pass
Panel feedback addressed in
|
| Finding | Status | Fix |
|---|---|---|
| [B] Path traversal guard missing at user-controlled call-site | Fixed | Added validate_path_segments() + ensure_path_within() before runtime binary resolution |
| [B] Docs misinformation in runtime-compatibility.md | Fixed | Corrected version guidance and wire_api requirements |
[R] setup-codex.sh warning doesn't mention wire_api=chat |
Fixed | Warning now explicitly states wire_api=chat requirement for pre-0.116 |
| [R] setup-codex.ps1 same issue | Fixed | Write-Warning updated with same guidance |
| [R] Logging helpers for consistent output | Fixed | Added log_warning (sh) / Write-Warning (ps1) pattern |
[R] LAST_COMPAT_VERSION constant not extracted |
Fixed | Extracted to module-level constant |
| [R] Missing CHANGELOG entries | Fixed | Added entries under [Unreleased] ### Fixed and ### Added |
| [N] Docs caveat for codex/GitHub Models | Fixed | Added runtime-compatibility.md page to Starlight docs |
All Copilot review threads resolved. CI green, lint clean.
Description
Fixes both scenarios reported in #605:
Scenario A -- PATH priority
_detect_installed_runtimeusedshutil.which()which finds system PATH stubs (e.g.gh copilotshim) before checking~/.apm/runtimes/.Fix: New
find_runtime_binary()utility insrc/apm_cli/runtime/utils.pythat checks~/.apm/runtimes/<name>first (APM-managed, executable), then falls back toshutil.which(). Handles Windows.exesuffix. Replaced all runtime detection sites:src/apm_cli/core/script_runner.py--_detect_installed_runtime+ binary resolution at subprocess execution timesrc/apm_cli/runtime/codex_runtime.py--is_available()+execute_prompt()src/apm_cli/runtime/copilot_runtime.py--is_available()src/apm_cli/integration/mcp_integrator.py-- runtime fallback probessrc/apm_cli/integration/mcp_integrator_install.py-- runtime availability checksshutil.which("code")for VS Code detection is intentionally untouched.Scenario B -- codex/GitHub Models incompatibility
apm runtime setup codexinstalls v0.118.0 which requireswire_api=responses, but GitHub Models only exposes Chat Completions (no/responsesendpoint).Fix: Added a clear compatibility warning in
setup-codex.shandsetup-codex.ps1that fires when the resolved version is >= 0.116, including for--version latest.Fixes #605
Type of change
Testing
8508 tests pass. 11 new tests for
find_runtime_binary()utility. Lint clean (ruff check+ruff format --check).