fix: resolve false Go/gofmt not-installed warnings on Windows and unify tool detection#58
fix: resolve false Go/gofmt not-installed warnings on Windows and unify tool detection#58Zireael wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/aft/src/format.rs">
<violation number="1" location="crates/aft/src/format.rs:377">
P2: `AFT_DISABLE_WELL_KNOWN_LOOKUP` escape hatch is bypassed on Windows. The early `return` in the `cfg!(windows)` branch means integration tests on Windows cannot suppress the well-known-path fallback. Move the env-var check above the platform branch.</violation>
<violation number="2" location="crates/aft/src/format.rs:1104">
P2: Missing spaces in the rendered string — regression from the original `.join(" ")`. The `\` continuation eats leading whitespace, producing `"installed —ensure"`, `"uses/opt/homebrew/bin"`, and `"macOS).GUI-launched"`. Add a space before each `\`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| return None; | ||
| // On Windows, check common install locations that GUI-launched editors | ||
| // may miss from PATH: Go SDK, Cargo, and user-local Go binaries. | ||
| let candidates = |
There was a problem hiding this comment.
P2: AFT_DISABLE_WELL_KNOWN_LOOKUP escape hatch is bypassed on Windows. The early return in the cfg!(windows) branch means integration tests on Windows cannot suppress the well-known-path fallback. Move the env-var check above the platform branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/format.rs, line 377:
<comment>`AFT_DISABLE_WELL_KNOWN_LOOKUP` escape hatch is bypassed on Windows. The early `return` in the `cfg!(windows)` branch means integration tests on Windows cannot suppress the well-known-path fallback. Move the env-var check above the platform branch.</comment>
<file context>
@@ -372,9 +372,11 @@ fn try_path_lookup(command: &str) -> Option<PathBuf> {
- return None;
+ // On Windows, check common install locations that GUI-launched editors
+ // may miss from PATH: Go SDK, Cargo, and user-local Go binaries.
+ let candidates =
+ well_known_windows_search_paths(command, std::env::var_os("USERPROFILE").as_deref());
+ return try_well_known_path_lookup_in(&candidates);
</file context>
| let candidates = | |
| if std::env::var_os("AFT_DISABLE_WELL_KNOWN_LOOKUP").is_some() { | |
| return None; | |
| } | |
| let candidates = |
| "Install Go from https://go.dev/dl/, or — if it's already installed —\ | ||
| ensure its bin directory is on PATH (Homebrew typically uses\ | ||
| /opt/homebrew/bin on Apple Silicon, /usr/local/bin on Intel macOS).\ | ||
| GUI-launched editors often don't inherit login-shell PATH." |
There was a problem hiding this comment.
P2: Missing spaces in the rendered string — regression from the original .join(" "). The \ continuation eats leading whitespace, producing "installed —ensure", "uses/opt/homebrew/bin", and "macOS).GUI-launched". Add a space before each \.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/format.rs, line 1104:
<comment>Missing spaces in the rendered string — regression from the original `.join(" ")`. The `\` continuation eats leading whitespace, producing `"installed —ensure"`, `"uses/opt/homebrew/bin"`, and `"macOS).GUI-launched"`. Add a space before each `\`.</comment>
<file context>
@@ -1053,13 +1096,17 @@ pub(crate) fn install_hint(tool: &str) -> String {
+ C:\\Go\\bin, C:\\Program Files\\Go\\bin. \
+ GUI-launched editors often don't inherit login-shell PATH."
+ } else {
+ "Install Go from https://go.dev/dl/, or — if it's already installed —\
+ ensure its bin directory is on PATH (Homebrew typically uses\
+ /opt/homebrew/bin on Apple Silicon, /usr/local/bin on Intel macOS).\
</file context>
| "Install Go from https://go.dev/dl/, or — if it's already installed —\ | |
| ensure its bin directory is on PATH (Homebrew typically uses\ | |
| /opt/homebrew/bin on Apple Silicon, /usr/local/bin on Intel macOS).\ | |
| GUI-launched editors often don't inherit login-shell PATH." | |
| "Install Go from https://go.dev/dl/, or — if it's already installed — \ | |
| ensure its bin directory is on PATH (Homebrew typically uses \ | |
| /opt/homebrew/bin on Apple Silicon, /usr/local/bin on Intel macOS). \ | |
| GUI-launched editors often don't inherit login-shell PATH." |
a0c93e8 to
b0dd722
Compare
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/aft/src/format.rs">
<violation number="1" location="crates/aft/src/format.rs:377">
P2: `AFT_DISABLE_WELL_KNOWN_LOOKUP` escape hatch is unreachable on Windows because the Windows branch returns early before the check. Tests on Windows cannot disable the well-known path fallback. Move the env-var check before the `cfg!(windows)` branch.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| return None; | ||
| // On Windows, check common install locations that GUI-launched editors | ||
| // may miss from PATH: Go SDK, Cargo, and user-local Go binaries. | ||
| let candidates = |
There was a problem hiding this comment.
P2: AFT_DISABLE_WELL_KNOWN_LOOKUP escape hatch is unreachable on Windows because the Windows branch returns early before the check. Tests on Windows cannot disable the well-known path fallback. Move the env-var check before the cfg!(windows) branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/format.rs, line 377:
<comment>`AFT_DISABLE_WELL_KNOWN_LOOKUP` escape hatch is unreachable on Windows because the Windows branch returns early before the check. Tests on Windows cannot disable the well-known path fallback. Move the env-var check before the `cfg!(windows)` branch.</comment>
<file context>
@@ -372,9 +372,11 @@ fn try_path_lookup(command: &str) -> Option<PathBuf> {
- return None;
+ // On Windows, check common install locations that GUI-launched editors
+ // may miss from PATH: Go SDK, Cargo, and user-local Go binaries.
+ let candidates =
+ well_known_windows_search_paths(command, std::env::var_os("USERPROFILE").as_deref());
+ return try_well_known_path_lookup_in(&candidates);
</file context>
| let candidates = | |
| // Test-only escape hatch: integration tests that need to assert | |
| // "tool not installed" semantics set AFT_DISABLE_WELL_KNOWN_LOOKUP=1 | |
| // so CI runners with a system tool at a well-known path don't | |
| // silently make those tests pass. Production callers never set this. | |
| if std::env::var_os("AFT_DISABLE_WELL_KNOWN_LOOKUP").is_some() { | |
| return None; | |
| } | |
| let candidates = |
…fy tool detection
- Make resolve_tool_uncached pub(crate) and add Windows well-known search paths
(C:\Go\bin, %USERPROFILE%\.cargo\bin, etc.) to fix false negatives on Windows.
- Delegate configure.rs resolve_tool_uncached to format.rs to eliminate
near-duplicate tool detection logic and ensure well-known path fallback
applies at configure-time too.
- Add cfg!(windows) branch to install_hint("go") with Windows paths.
- Add .qartez/ to .gitignore (local code intelligence cache, not source).
Fixes cortexkit#47
b0dd722 to
9470c38
Compare
AFT has been emitting spurious "not installed" warnings for
goandgofmton Windows 11 even when both tools are correctly installed. Thegofmtwarning also fires on macOS and Linux. This PR fixes four root causes.1.
gofmt --versionexits non-zero (all platforms)try_path_lookupprobes tools via--versionand treats non-zero exit as "not found."gofmtdoes not accept--version— it prints version info to stdout but exits code 1 — so the probe always returns a false negative. The well-known-path fallback now catches this case: it checks file existence viafs::metadatainstead of spawning the tool.2.
try_well_known_path_lookupunconditionally disabled on WindowsThe well-known-path fallback hardcoded
cfg!(windows) { return None; }because its search list was exclusively POSIX (/opt/homebrew/bin,/usr/local/bin, etc.). Windows now has its own search function,well_known_windows_search_paths, covering:C:\Go\bin— Go SDK default install pathC:\Program Files\Go\bin— Go SDK (Program Files variant)%USERPROFILE%\.cargo\bin—cargo installtools%USERPROFILE%\go\bin—go installdefault GOPATH layout3.
install_hint("go")provides macOS-only guidanceThe install hint shown to users mentioned only macOS Homebrew paths. It now has a
cfg!(windows)branch that points to the standard Windows install directories.4. Duplicate tool-detection logic
detect_missing_tools_for_languagesinconfigure.rsmaintained its own copy of the tool-resolution code, missing the well-known-path fallback entirely. It now delegates toformat.rs::resolve_tool_uncached, keeping the resolution logic in one place.Closes #57
Summary by cubic
Fixes false “not installed” warnings for
goandgofmton Windows and makes tool detection consistent across platforms. Also improvesgofmtprobing, adds Windows install guidance, and excludes local cache dirs from git.Bug Fixes
gofmtexiting non-zero for--versionby checking file existence to avoid false negatives on all platforms.C:\Go\bin,C:\Program Files\Go\bin,%USERPROFILE%\.cargo\bin,%USERPROFILE%\go\bin) and updateinstall_hint("go")with Windows guidance.configure.rstoformat.rs::resolve_tool_uncached(nowpub(crate)) for consistent behavior during configure and runtime.Refactors
.qartez/to.gitignoreto ignore local code intelligence cache.Written for commit 9470c38. Summary will update on new commits. Review in cubic