From fff210dc3943e9015268c11e8aa5f63fdf122b30 Mon Sep 17 00:00:00 2001 From: Tim Erwin Date: Sun, 30 Aug 2026 16:39:21 +0700 Subject: [PATCH] =?UTF-8?q?guard:=20read=20go=20list's=20stdout,=20not=20s?= =?UTF-8?q?tdout+stderr=20=E2=80=94=20fixes=20the=20perpetual=20failures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestVendorSDKsOnlyInTheirAdapters and TestNoGatewayInternalsOutsideGateway have been failing on every branch, blamed on desktop/node_modules. That was the trigger, not the cause. `go list` EXITS 0 and returns a complete, correct package list; the symlink lines are advisory warnings on stderr. The guard helpers used CombinedOutput(), which folded those warning lines into the package list — and each one was then handed back to `go list` as if it were a package path. `.../node_modules@electron/...` contains an `@`, which in module-aware mode reads as path@version and is only valid for `go get`/`go install`, so THAT invocation exited 1. The failure was the helper laundering stderr into data. Fixed at the source: one goListLines helper reading stdout only, used by deps, directImports, and modulePackages. Any `go list` warning is now inert, whatever its cause — an untracked node_modules tree from another branch (desktop/ is not tracked on main at all), a stray symlink, anything future. Genuine failures still fail: non-zero exit is fatal, and an empty package list is now explicitly fatal too, since that would let every guard below pass vacuously — the failure mode this class of test is least able to notice. Verified: full `go test ./...` is green with desktop/node_modules still in place, and the guards still catch a real violation (injecting an anthropic-sdk import into internal/wire fails TestWireIsStdlibOnly as it should). Also fixes two comments still pointing at docs/design/personal-agents.md, which was renamed to autonomous-agents.md in the consolidation. --- internal/agent/autonomy/delegation.go | 2 +- internal/agent/autonomy/runner_exec.go | 2 +- internal/guard/guard_test.go | 63 ++++++++++++++------------ 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/internal/agent/autonomy/delegation.go b/internal/agent/autonomy/delegation.go index ab36f39..9698032 100644 --- a/internal/agent/autonomy/delegation.go +++ b/internal/agent/autonomy/delegation.go @@ -26,7 +26,7 @@ type ExecutionEnvelope struct { // Agent delegation — the user's own already-running, already-logged-in // Chrome, reached through the gateway-owned broker) or BrowserEphemeral // (a fresh, logged-out profile — explicit opt-down only). See - // docs/design/personal-agents.md "Browser broker trust boundary". + // docs/design/autonomous-agents.md "Browser broker trust boundary". BrowserSession string } diff --git a/internal/agent/autonomy/runner_exec.go b/internal/agent/autonomy/runner_exec.go index 8833394..c73b449 100644 --- a/internal/agent/autonomy/runner_exec.go +++ b/internal/agent/autonomy/runner_exec.go @@ -648,7 +648,7 @@ func delegateMode(cs []ConsequenceClass) string { // (Gmail, LinkedIn, an ATS, an internal dashboard) requires being signed in. // "browser:ephemeral" is the explicit opt-down to a fresh, logged-out // profile, for tasks that genuinely don't want the user's session (e.g. -// visiting a site anonymously). See docs/design/personal-agents.md "Browser +// visiting a site anonymously). See docs/design/autonomous-agents.md "Browser // broker trust boundary". func browserModeFor(toolsets []string) string { for _, t := range toolsets { diff --git a/internal/guard/guard_test.go b/internal/guard/guard_test.go index 83f9903..ea130d8 100644 --- a/internal/guard/guard_test.go +++ b/internal/guard/guard_test.go @@ -7,6 +7,7 @@ package guard import ( + "bytes" "os/exec" "strings" "testing" @@ -14,16 +15,29 @@ import ( const modulePrefix = "github.com/memcode-ai/memcode" -// deps returns the transitive import closure of pkg (including pkg itself). -// `go list` keeps the guard honest about TRANSITIVE deps, not just direct ones. -func deps(t *testing.T, pkg string) []string { +// goListLines runs `go list` and returns its STDOUT lines. +// +// Reading stdout only is load-bearing, not tidiness. `go list` writes advisory +// warnings to stderr while still exiting 0 with a complete, correct package +// list — most commonly "warning: ignoring symlink ..." when an untracked +// sibling directory (a node_modules tree from another branch, say) sits in the +// module root. CombinedOutput folds those warning lines into the results, each +// one then gets handed back to `go list` as if it were a package path, and THAT +// invocation fails. The original symptom looked like a desktop/node_modules +// problem; it was really this function laundering stderr into data. +// +// Genuine failures still fail: a non-zero exit is fatal, and so is an empty +// package list, which would otherwise let a guard pass vacuously. +func goListLines(t *testing.T, label string, args ...string) []string { t.Helper() - out, err := exec.Command("go", "list", "-deps", pkg).CombinedOutput() - if err != nil { - t.Fatalf("go list -deps %s: %v\n%s", pkg, err, out) + cmd := exec.Command("go", args...) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("%s: %v\n%s", label, err, stderr.String()) } var ps []string - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + for _, line := range strings.Split(strings.TrimSpace(stdout.String()), "\n") { if line = strings.TrimSpace(line); line != "" { ps = append(ps, line) } @@ -31,6 +45,14 @@ func deps(t *testing.T, pkg string) []string { return ps } +// deps returns the transitive import closure of pkg (including pkg itself). +// `go list` keeps the guard honest about TRANSITIVE deps, not just direct ones. +func deps(t *testing.T, pkg string) []string { + t.Helper() + ps := goListLines(t, "go list -deps "+pkg, "list", "-deps", pkg) + return ps +} + // isStdlib: first path segment contains no dot. func isStdlib(p string) bool { seg := p @@ -96,32 +118,15 @@ var vendorSDKs = map[string]string{ func directImports(t *testing.T, pkg string) []string { t.Helper() - out, err := exec.Command("go", "list", "-f", `{{join .Imports "\n"}}`, pkg).CombinedOutput() - if err != nil { - t.Fatalf("go list %s: %v\n%s", pkg, err, out) - } - var ps []string - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { - if line = strings.TrimSpace(line); line != "" { - ps = append(ps, line) - } - } - return ps + return goListLines(t, "go list "+pkg, "list", "-f", `{{join .Imports "\n"}}`, pkg) } func modulePackages(t *testing.T) []string { - t.Helper() - out, err := exec.Command("go", "list", modulePrefix+"/...").CombinedOutput() - if err != nil { - t.Fatalf("go list: %v\n%s", err, out) + pkgs := goListLines(t, "go list", "list", modulePrefix+"/...") + if len(pkgs) == 0 { + t.Fatal("go list returned no packages — every guard below would pass vacuously") } - var ps []string - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { - if line = strings.TrimSpace(line); line != "" { - ps = append(ps, line) - } - } - return ps + return pkgs } // TestVendorSDKsOnlyInTheirAdapters: a vendor client library imported outside