fix: find a harness on PATH the way the shell does - #20
Merged
Conversation
Since Go 1.19 exec.LookPath returns a binary found through a relative PATH entry together with an error, so a caller testing err != nil refuses to run it. Every harness launcher tested exactly that and reported "not installed or not in PATH" for a binary the operator could launch by name in the same shell. The obvious repair did not work either: LookPath stops at the first match, so appending an absolute entry left the earlier relative one matching first and failing identically. A relative PATH entry is an ordinary thing to keep and bash runs it without comment, so reach now does too. The lookup stays exec.LookPath rather than a hand-rolled PATH walk, because on Windows executability is PATHEXT and not a file mode; only the refusal is dropped. The result is resolved to an absolute path while the working directory is still the one the lookup assumed, since this path is handed to exec and to the seam probe and a relative one names a different file after any chdir. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuwaXDJut6CorENbW1KHPk
nilerr caught the reflex: filepath.Abs only fails when the working directory cannot be determined, and returning the unresolved relative path with a nil error there hands the caller exactly the ambiguity this function exists to remove. There is nothing to resolve a relative entry against in that state, so it is an error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuwaXDJut6CorENbW1KHPk
4 tasks
grok.go arrived with main after this branch was cut, still calling exec.LookPath directly. Left alone it would be the one harness that still refused a binary found through a relative PATH entry — which is the exact case that surfaced this bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuwaXDJut6CorENbW1KHPk
Merged
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.
The bug
Every harness launcher resolves its binary with
exec.LookPathand checkserr != nil. Since Go 1.19, a binary found through a relativePATHentry comes back with the resolved path and an error satisfyingexec.ErrDot, so that check rejects it. reach then prints:about a binary the operator can launch by name in the very same shell. That message is false about the one thing it asserts, and it sends someone off reinstalling a harness that was never missing.
The obvious repair does not work either, which is what makes this worth fixing rather than documenting.
LookPathstops at the first match, so appending an absolute entry leaves the earlier relative one matching first and failing identically:Hit live while testing #12:
grokran fine from zsh, andreach grokinsisted it was not installed, through two rounds of PATH edits.The fix
lookHarnessPathkeepsexec.LookPathand drops only theErrDotrefusal. A relativePATHentry is an ordinary thing for a person to keep, and bash runs what it finds there without comment, so reach does too.Two things are deliberate:
exec.LookPath, not a hand-rolled PATH walk.platform_windows.goalready documents why: on Windows executability isPATHEXT, not a file mode, and re-implementing that is how a search quietly stops finding.cmdwrappers.execand to the seam probe; a path left relative names a different file after anychdir, and "which binary is this, really" is exactly the question those two callers cannot be vague about.Applied to every harness call site —
claude,codex,goose,gemini,crush,kimi, anddoctor— and duplicated intoharnessprobefor the reasoncache.goduplicates theREACH_HOMErule: a probe that resolved a binary differently from the launcher would verify a seam for one binary and hand the operator another.Tests
Three, covering the reported behaviour rather than the implementation: a relative entry resolves to an absolute path; a relative entry sitting ahead of an absolute one still resolves instead of refusing; and a genuinely absent binary still errors, because the refusal being wrong for relative entries does not make "not installed" wrong in general.
go build,go vetandgo test ./...all pass.Not in scope
internal/fileops/helper.golooks up thegotoolchain the same way. It is left alone here: that path is a build-time convenience with a clear failure message, not the user-facing lookup this fixes. Worth a follow-up if you want the policy uniform.🤖 Generated with Claude Code
https://claude.ai/code/session_01PuwaXDJut6CorENbW1KHPk