Repository navigation
fix(go): keep go list diagnostics out of the default output - #11816
Conversation
Version resolution falls back to `go list` when GOPROXY=direct, and the module-path walk tries a package's parents before it finds the module. Every miss made `go` write a complaint to stderr, and duct's `.read()` captures only stdout — so a valid setup printed those lines while mise's own per-candidate message was already behind `debug!`. Route the three `go list` calls through `crate::cmd::cmd_read_async`, which pipes stderr and folds it into the error. The call sites already log their failures at debug, so go's output now appears under `--verbose` instead.
📝 WalkthroughWalkthroughThe Go backend now performs module metadata queries asynchronously, logs recoverable query failures at debug level, and preserves contextual errors for latest-version lookup. The direct-installation end-to-end test verifies that default output excludes ChangesGo module query handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR moves three Go module-resolution commands to the asynchronous command helper so routine
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security defects identified. The helper preserves the prior full-environment, argument, exit-status, trimming, and stdout behavior needed by Go resolution while intentionally capturing stderr, and the isolated e2e test verifies both clean output and successful pseudo-version resolution. Important Files Changed
Reviews (1): Last reviewed commit: "fix(go): keep go list diagnostics out of..." | Re-trigger Greptile |
Closes #5189.
What is left of the problem
Most of that report is already fixed. Version resolution goes through
$GOPROXYnow, so on a default setupgois never spawned and none of the reporter's three cases produce output. Measured on v2026.8.2 with their exact examples:The
GOPROXY=directfallback is the part that still leaks, and it reproduces on one of the reporter's own tools:Splitting the streams locates it — the version is the only thing on stdout, and those two lines come from
goitself:Note this is a valid setup producing that noise:
_list_remote_versionswalks the package path and its parents, so misses are the normal path to a hit, not a fault.Cause
Three
go listcalls used duct's.read(), which captures stdout and inherits stderr. mise's own per-candidate message was already demoted todebug!, so the subprocess output was the only thing still reaching the terminal.The change
Those three calls now use
crate::cmd::cmd_read_async, which already exists for exactly this shape — its doc comment says to use it "for backends that pass a full env fromdependency_env()", which is whatgo_list_env()builds.gem.rsis the existing caller.It maps onto the duct call one-for-one —
env_clear().envs(env)for.full_env(), same trailing-newline trim — with two differences that matter here: stderr is piped rather than inherited, and a non-zero exit produces an error whose message carries that stderr. Since the call sites already route their failures intodebug!, go's explanation lands behind--verboseinstead of on stdout, which is what the issue asks for.The one place it can still surface at default verbosity is the existing summary
warn!in_list_remote_versions, which fires only when every candidate failed — a genuine error where go's reason is the useful part.Scope
go installis untouched. That path usesCmdLineRunnerand its output is something the user wants to see.src/backend/go.rsplus one e2e assertion.go listis now dropped entirely rather than debug-logged. In practice that is download-progress chatter. If you would rather keep it at debug level, that is a small addition, but it means not reusing the shared helper..read()is blocking and was being called fromasync fn, so therun_with_timeout_asyncwrapper around_list_remote_versionscould not actually interrupt it.cmd_read_asyncusestokio::process. I have not measured any speed difference and am not claiming one.Tests
e2e/backend/test_go_installalready exercises the parent-path walk underGOPROXY=direct. Added anassert_not_containsfor"go: module"against that same command.It is placed before the existing
assert_matcheson purpose: mise caches resolved versions to disk, so running it second would skipgoentirely and the assertion would pass without testing anything.The match string avoids a false positive on mise's own
go:github.com/...output, which has no space after the colon.No unit tests — all three call sites are a
goinvocation with no separable pure logic.Summary by CodeRabbit
go listmodule-walk errors in the default output.