Skip to content

fix: resolve agent CLIs outside inherited PATH - #491

Open
shingo-mori-acori wants to merge 4 commits into
fujibee:mainfrom
shingo-mori-acori:codex/fix-cli-fallback-resolution
Open

fix: resolve agent CLIs outside inherited PATH#491
shingo-mori-acori wants to merge 4 commits into
fujibee:mainfrom
shingo-mori-acori:codex/fix-cli-fallback-resolution

Conversation

@shingo-mori-acori

@shingo-mori-acori shingo-mori-acori commented Jul 26, 2026

Copy link
Copy Markdown

Summary

  • keep PATH-resolved wrappers and shims late-bound in the child shell
  • fall back to standard user, Homebrew, and system CLI locations when the parent PATH is incomplete
  • cover macOS, Linux, and Git Bash executable forms

Refs #397

Verification

  • npx --yes bats --print-output-on-failure tests/test_spawn.bats (75/75)
  • npx --yes bats --print-output-on-failure --filter '[Ww]indows fallback' tests/test_spawn.bats (4/4)
  • bash -n scripts/spawn.sh
  • git diff --check origin/main...HEAD
  • GitHub Actions (8/8 checks passed)

Review

  • /simplify findings: nothing found
  • Codex Sol High: no blocker or major findings; verdict ship

@shingo-mori-acori
shingo-mori-acori marked this pull request as ready for review July 26, 2026 04:27
@fujibee

fujibee commented Jul 29, 2026

Copy link
Copy Markdown
Owner

@shingo-mori-acori

Apologies for the long silence — the delay was ours, not a reflection of the PR.

The change itself holds up. Keeping PATH-resolved wrappers and shims late-bound in the child shell is the right shape, the fallback locations are the ones that actually matter across macOS, Linux, and Git Bash, and covering the three executable forms rather than assuming one is the part that makes it portable instead of merely working locally. The added tests exercise the resolution order directly, which is what we would have asked for.

Two housekeeping items, both on us.

The required check names changed after this PR last ran. The bats suite is now split into parallel shards, and the required context is a summary job named bats rather than the per-OS jobs your run reported, so the required check has never reported on this head and GitHub holds the PR as blocked independently of content. We will take that rebase in-house — we do not bounce external PRs back for our own churn — and the branch applies cleanly onto current main, which we verified.

While rebasing we will also restore the executable bit on scripts/spawn.sh. The branch currently carries it as mode 100644 where main has 100755; it reads as incidental rather than intended, and it is easier for us to fix in the same pass than to send you round for one mode change. No action needed from you — flagging it only so the diff changing under you is not a surprise.

On scope, your framing is the correct one and we want it on the record: Refs #397, not Fixes. This resolves CLIs outside the inherited PATH; #397 proposes a cli_wrapper= manifest key as the durable mechanism. Different mechanisms, and #397 stays open after this lands, so nobody later reads this PR as having closed it.

One thing that is genuinely blocking, and it is not about quality. The fallback resolution order is a user-facing contract — once shipped, people depend on which location wins — so it counts as an interface addition here, and interface additions need fujibee to examine the design himself rather than being merged on review alone. That look is still pending on our side. It is not waiting on anything from you, and it is not a judgement on the PR; we would rather tell you where it actually stands than leave it quiet again.

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.

2 participants