feat(execbroker): scope environment updates

[MeteorsLiu]

Broker Formula environment operations within the active execbroker scope so Formula code and build commands observe the same isolated environment.

The implementation includes:

• Add scoped LookupEnv, Unsetenv, Clearenv, Environ, and ExpandEnv operations alongside Getenv and Setenv.
• Merge broker implementations into ixgo’s complete os and syscall packages with RegisterPackage, preserving all unrelated package exports.
• Route gsh Sys.Getenv, Sys.Environ, and Sys.ExpandEnv through the broker while keeping command execution through execbroker.Run.
• Add Formula, gsh, package-merge, nested-scope, goroutine-isolation, and process-environment regression coverage.

This gives Formula code and its command helpers one scope-aware environment without mutating the LLAR process environment.

[codecov]

Codecov Report

❌ Patch coverage is 88.49558% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/execbroker/execbroker.go	87.37%	8 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

[fennoai]

Review: scoped Getenv/Setenv in execbroker

Solid, well-tested change. The new Getenv/Setenv mirror the existing scope patterns (Println, rewrite), locking is correct, nested-scope save/restore works, and the tests cover goroutine-locality, nesting, validation, and process-env isolation. No blocking correctness bugs found.

The one issue worth attention before merge is a scope-isolation gap: this PR now routes os.Setenv from .gox formula scripts through the broker, but two formula hook paths are not wrapped in a broker scope — see the inline comment on formula.go. The remaining findings are minor accuracy/perf notes.

Minor notes not filed inline:

• The five migrated callers (x/autotools, x/cmake, x/pkgconfig) discard the new Setenv error with _ =. Keys are compile-time constants so validateEnv won't realistically fail, but a NUL in an inherited value would become a silent no-op. A brief comment noting the deliberate discard would match the care elsewhere.
• Getenv (execbroker.go:71-82) releases the read lock on two branches; folding to a single RUnlock before the os.Getenv fallback (as Println does) removes the double-unlock maintenance hazard.
• envValue scans back-to-front (last-wins, matching OS duplicate-key resolution) while setEnv scans front-to-back; a one-line comment on the asymmetry would help future readers.

[fennoai]

[P3] Getenv doc/behavior: a non-nil scope Env masks the process environment

The comment says "Without a scope, it has the same behavior as os.Getenv," which is accurate. But when a scope exists with a non-nil scope.Env that does not contain key, envValue returns "" and Getenv returns "" without consulting os.Getenv (line 75-78). A variable present in the real process environment is then reported as empty.

Currently masked in production because the only Do scope (build.go) starts with Env nil and the first Setenv copies the full os.Environ(). But since formulas now route os.Getenv here, a partial-Env scope would diverge from real os.Getenv. Either document this ("with a scope, only variables present in the scope environment are visible") or fall back to os.Getenv when the key is absent from a non-nil scope.Env.

[fennoai]

[P3] os.Environ() allocates the full env under the write lock

os.Environ() copies the entire process environment while scopeMu.Lock() is held, blocking every other goroutine's Getenv/Setenv/rewrite/Println. It only runs on the first scoped write per scope, so impact is bounded — but the allocation could be done outside the lock (or snapshotted at Do() time) to keep the global mutex critical section minimal.

[fennoai]

Review (follow-up): scoped Getenv/Setenv in execbroker

My earlier review's two most important inline findings were dropped by the submitter's 80-char title limit, so I'm re-posting them here. This corrects that — the headline item is a scope-isolation gap for formula-invoked os.Setenv.

[P1] os.Setenv exposed to formulas, but Filter/OnRequire hooks run without a broker scope — internal/formula/formula.go:87

This registers os.Setenv as an external for formula scripts so scoped writes stay isolated from the real process environment. That isolation only holds while a scope is active for the current goid: execbroker.Setenv falls through to the real os.Setenv when no scope exists (execbroker.go:97-100).

But Filter and OnRequire run via runFormulaHook in internal/modules/load.go:300 and :332 without an enclosing execbroker.Do(...) — only OnBuild/OnTest in internal/build/build.go:308 establish a scope. So a formula calling os.setenv("PATH", ...), LD_PRELOAD, GIT_SSH_COMMAND, http_proxy, etc. inside onRequire/filter will mutate the real llar process environment, affecting subsequently spawned commands (e.g. vcs/git during dependency resolution).

Fix: wrap the Filter/OnRequire execution in an execbroker.Do(...) scope (as build.go does), or make the no-scope branch of Setenv fail closed for formula-exposed calls rather than delegating to the process environment.

[P3] Setenv doc: "first scoped write copies the process environment" is unconditional but code isn't — internal/execbroker/execbroker.go:84

The copy only happens when scope.Env == nil (line 101-103). If the scope was created with a non-nil/partial Env (Do(Scope{Env: []string{...}})), the first Setenv mutates/appends the existing slice instead. Suggest qualifying: "…copies the process environment if the scope has no environment set yet."

---

(The other two findings — Getenv partial-scope masking, and os.Environ() under the write lock — are attached as inline comments on the prior review.)