Repository navigation
fix(process): preserve Go build settings in managed children - #330
Merged
Merged
Conversation
(cherry picked from commit 8884403)
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
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.
Managed subprocesses currently drop the host’s Go cache and toolchain settings. This prevents Go commands from using preloaded modules in offline environments.
Preserve the captured
GOCACHE,GOMODCACHE,GOPATH,GOROOT,GOTOOLCHAIN, andGOFLAGSsettings, while retaining explicit child overrides and excluding proxy/auth variables from implicit inheritance. Includes regression tests for the environment snapshot, overrides, credential exclusions, and an actual offline module lookup.Extracted only commit 8884403 from #186, preserving Amar Singh’s authorship. The unrelated reasoning/protocol changes from that PR are excluded.
Validation: regression tests failed against unchanged development during triage; the focused process tests pass with the fix under Go 1.27.2 and the race detector. Full CI must pass before merging.
Note
Medium Risk
Expands which host environment variables managed children receive; mitigated by a narrow Go-toolchain allowlist and explicit exclusion of proxy/auth-related
GO*variables.Overview
Managed subprocesses now inherit a snapshot of six Go toolchain/cache variables (
GOCACHE,GOMODCACHE,GOPATH,GOROOT,GOTOOLCHAIN,GOFLAGS) taken whenProcessManagerstarts, sogoin managed children can use preloaded module caches offline. Per-processEnvoverrides still win (e.g.ChildEnvironment/ProcessOptions.Env).The allowlist in
allowedBaseEnvironmentis extended deliberately without inheriting otherGO*settings (e.g.GOPROXY,GOAUTH) that may carry credentials.docs/concurrency.mddocuments this behavior.Tests cover snapshot vs post-snapshot host changes, credential exclusion, override precedence, and an integration test that runs
go list -m allwith downloads disabled using a fake module cache.Reviewed by Cursor Bugbot for commit bee34d8. Bugbot is set up for automated code reviews on this repo. Configure here.