From 59f7bd95f8d402574fd8ff048dd2d5ee394ebfc6 Mon Sep 17 00:00:00 2001 From: OpenClaw Bot Date: Fri, 15 May 2026 14:01:31 +0000 Subject: [PATCH] test(#1197): extend nil-context gate to ALL cmd/server/*.go (RED) The original gate only scanned store.go and missed routes.go:1428 (handleNodePaths) which still passed nil to pm.resolveWithContext. Extend the static grep to scan every non-test *.go file in cmd/server/. This commit is RED on purpose: routes.go:1428 is the offender that the next commit will fix. --- cmd/server/resolve_context_callsites_test.go | 82 +++++++++++++------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/cmd/server/resolve_context_callsites_test.go b/cmd/server/resolve_context_callsites_test.go index 6481c8fa..44a6b6f2 100644 --- a/cmd/server/resolve_context_callsites_test.go +++ b/cmd/server/resolve_context_callsites_test.go @@ -2,43 +2,74 @@ package main import ( "os" + "path/filepath" "regexp" "strings" "testing" ) // TestAllResolveWithContextCallSitesPassNonNilContext is a static grep gate -// against #1197: every call to pm.resolveWithContext(...) in store.go must -// pass a non-nil context argument. Reverting any one call site to `nil` would -// silently re-introduce the regression #1197 is meant to prevent. +// against #1197: every call to pm.resolveWithContext(...) in production code +// (any non-test *.go file under cmd/server/) must pass a non-nil context +// argument. Reverting any one call site to `nil` would silently re-introduce +// the regression #1197 is meant to prevent. // -// Allowed exceptions: callers that must pass nil are listed explicitly here -// (e.g. unit tests, GetSubpathDetail's user-supplied raw-hop list — currently -// none in production code). +// Scope rationale: the original gate only scanned store.go and missed +// routes.go:1428 (handleNodePaths) which still passed `nil`. Extending the +// scope to all production *.go files in cmd/server/ closes that hole. +// +// Allowed exceptions: callers that must pass nil (currently none in +// production code) should be enumerated in `allowedNilCallers` below. func TestAllResolveWithContextCallSitesPassNonNilContext(t *testing.T) { - body, err := os.ReadFile("store.go") - if err != nil { - t.Fatalf("read store.go: %v", err) + allowedNilCallers := map[string]bool{ + // ":": true, } - src := string(body) - // Match: pm.resolveWithContext(, , ...) capture arg2. - // Allow whitespace, tolerate identifiers/expressions in arg1. - re := regexp.MustCompile(`resolveWithContext\s*\(\s*([^,]+?)\s*,\s*([^,]+?)\s*,`) - matches := re.FindAllStringSubmatchIndex(src, -1) - if len(matches) == 0 { - t.Fatalf("no resolveWithContext call sites found — test scaffold broken") + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatalf("glob *.go: %v", err) } + // Match: resolveWithContext(, , ...) — capture arg2. + re := regexp.MustCompile(`resolveWithContext\s*\(\s*([^,]+?)\s*,\s*([^,]+?)\s*,`) + var offenders []string - for _, m := range matches { - full := src[m[0]:m[1]] - arg2 := strings.TrimSpace(src[m[4]:m[5]]) - if arg2 == "nil" { - // Compute line number for diagnostics. - line := 1 + strings.Count(src[:m[0]], "\n") - offenders = append(offenders, fmtCallSite(line, full)) + totalCallSites := 0 + scannedFiles := 0 + for _, f := range files { + // Skip *_test.go (unit tests legitimately pass nil for fixture-driven + // behavior) and the test scaffold itself. + if strings.HasSuffix(f, "_test.go") { + continue } + body, err := os.ReadFile(f) + if err != nil { + t.Fatalf("read %s: %v", f, err) + } + scannedFiles++ + src := string(body) + matches := re.FindAllStringSubmatchIndex(src, -1) + for _, m := range matches { + totalCallSites++ + full := src[m[0]:m[1]] + arg2 := strings.TrimSpace(src[m[4]:m[5]]) + if arg2 != "nil" { + continue + } + line := 1 + strings.Count(src[:m[0]], "\n") + site := f + ":" + itoa(line) + if allowedNilCallers[site] { + continue + } + offenders = append(offenders, site+" — "+full) + } + } + + if scannedFiles == 0 { + t.Fatalf("no production *.go files scanned — test scaffold broken") + } + if totalCallSites == 0 { + t.Fatalf("no resolveWithContext call sites found across %d files — test scaffold broken", scannedFiles) } if len(offenders) > 0 { t.Fatalf("found %d call site(s) of pm.resolveWithContext that pass nil context "+ @@ -47,12 +78,7 @@ func TestAllResolveWithContextCallSitesPassNonNilContext(t *testing.T) { } } -func fmtCallSite(line int, snippet string) string { - return "store.go:" + itoa(line) + " — " + snippet -} - func itoa(i int) string { - // Avoid pulling strconv into a tiny helper; trivial inline. if i == 0 { return "0" }