fix(ci): widen node workspace detection, refuse a silent pass (G-015)

free/ci/cmd/main.go

@@ -363,11 +363,16 @@ func printResults(r *internal.Result) {
 
 	for _, g := range r.Gates {
 		mark := "PASS"
-		if !g.Passed {
+		switch {
+		case g.Skipped:
+			mark = "SKIP"
+		case !g.Passed:
 			mark = "FAIL"
 		}
 		fmt.Printf("  %-30s  %s  (%s)\n", g.Name, mark, g.Elapsed.Round(time.Millisecond))
-		if !g.Passed && g.Output != "" {
+		// Always show WHY for a skip or a failure — a silent SKIP/PASS line
+		// is exactly the shape of the G-015 bug this gate now refuses to be.
+		if (g.Skipped || !g.Passed) && g.Output != "" {
 			for _, line := range strings.SplitAfter(g.Output, "\n") {
 				fmt.Print("    ", line)
 			}

free/ci/internal/gate.go

@@ -53,6 +53,20 @@ type GateResult struct {
 	Passed  bool
 	Output  string
 	Elapsed time.Duration
+	// Substantive is true when this gate actually verified repo code — a
+	// lint, typecheck, test, build, or static-analysis step that ran for
+	// real. Meta checks (secrets scan, gateway routing, eval) and steps that
+	// never executed (missing binary/script — see Skipped) are false, so
+	// Run() can tell "every real check was skipped" from "everything was
+	// verified" even though both cases have Passed=true. G-015 / SPORT:
+	// PLUGINS-CI-007
+	Substantive bool
+	// Skipped is true when this entry represents a check that did NOT
+	// execute (missing script, missing binary, absent env var). Output
+	// always carries the reason. A skipped gate never fails the run by
+	// itself and never counts toward Substantive coverage. G-015 / SPORT:
+	// PLUGINS-CI-007
+	Skipped bool
 }
 
 // Result is the overall gate run result.

free/ci/internal/gate_helpers.go

@@ -87,6 +87,38 @@ func extractJSONObject(json, key string) map[string]string {
 	return result
 }
 
+// extractJSONStringArray extracts a top-level JSON string-array field (e.g.
+// "workspaces": ["packages/*", "apps/*"]) using the same simple string
+// parsing as extractJSONObject — no external JSON library. Handles only a
+// flat array of string literals, which is the shape package.json
+// "workspaces" always takes in practice.
+func extractJSONStringArray(json, key string) []string {
+	search := `"` + key + `"`
+	idx := strings.Index(json, search)
+	if idx < 0 {
+		return nil
+	}
+	start := strings.Index(json[idx:], "[")
+	if start < 0 {
+		return nil
+	}
+	start += idx + 1
+	end := strings.Index(json[start:], "]")
+	if end < 0 {
+		return nil
+	}
+	block := json[start : start+end]
+
+	var items []string
+	for _, part := range strings.Split(block, ",") {
+		part = strings.Trim(strings.TrimSpace(part), `"'`)
+		if part != "" {
+			items = append(items, part)
+		}
+	}
+	return items
+}
+
 // isGitRepo reports whether root is inside a git checkout.
 //
 // Purpose:     Decide whether gitleaks can scan tracked content (respecting
@@ -108,62 +140,6 @@ func isGitRepo(root string) bool {
 	}
 }
 
-// workspaceMembers returns the directories of a pnpm/npm workspace's member
-// packages, or nil when root is not a workspace.
-//
-// Purpose:     Let the Node gates see scripts that live in member packages
-//              rather than the root package.json.
-// Inputs:      root string — repo root
-// Outputs:     []string — absolute member directories containing a package.json
-// Constraints: Handles pnpm-workspace.yaml globs and package.json "workspaces".
-//              Skips node_modules. Glob depth is whatever the pattern states.
-func workspaceMembers(root string) []string {
-	var patterns []string
-
-	if data, err := os.ReadFile(filepath.Join(root, "pnpm-workspace.yaml")); err == nil {
-		for _, line := range strings.Split(string(data), "\n") {
-			line = strings.TrimSpace(line)
-			if !strings.HasPrefix(line, "- ") {
-				continue
-			}
-			pat := strings.Trim(strings.TrimSpace(strings.TrimPrefix(line, "- ")), `'"`)
-			// Members outside the repo (sibling checkouts) are not ours to gate.
-			if pat != "" && !strings.HasPrefix(pat, "..") {
-				patterns = append(patterns, pat)
-			}
-		}
-	}
-
-	if len(patterns) == 0 {
-		if ws, ok := loadPackageJSON(root)["workspaces"].([]interface{}); ok {
-			for _, w := range ws {
-				if str, ok := w.(string); ok {
-					patterns = append(patterns, str)
-				}
-			}
-		}
-	}
-
-	seen := map[string]bool{}
-	var members []string
-	for _, pat := range patterns {
-		matches, err := filepath.Glob(filepath.Join(root, pat))
-		if err != nil {
-			continue
-		}
-		for _, m := range matches {
-			if strings.Contains(m, "node_modules") || seen[m] {
-				continue
-			}
-			if fileExists(filepath.Join(m, "package.json")) {
-				seen[m] = true
-				members = append(members, m)
-			}
-		}
-	}
-	return members
-}
-
 // anyMemberHasScript reports whether at least one workspace member defines the
 // named script, so a gate is only added when it will actually do something.
 func anyMemberHasScript(members []string, script string) bool {

free/ci/internal/gate_helpers_test.go

@@ -0,0 +1,163 @@
+// Regression tests for Node workspace member discovery.
+//
+// WHY these exist: nself-org/plugins declares neither a pnpm-workspace.yaml
+// nor a package.json "workspaces" field, yet has 5 nested package.json files
+// with real lint/typecheck/test/build scripts. The old workspaceMembers()
+// only understood the two formal declarations, found nothing, and the node
+// gate silently ran zero checks while gitleaks alone reported "PASSED"
+// (G-015). These tests pin the fallback discovery that fixes it, plus the
+// package.json "workspaces" field path, which the old code referenced via
+// loadPackageJSON(root)["workspaces"] — a type assertion that could never
+// succeed, because loadPackageJSON only ever returns the "scripts" object.
+package internal
+
+import (
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+)
+
+func writePackageJSON(t *testing.T, dir, content string) {
+	t.Helper()
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("mkdir %s: %v", dir, err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte(content), 0o644); err != nil {
+		t.Fatalf("write package.json in %s: %v", dir, err)
+	}
+}
+
+// TestDetectNodeWorkspace_ImplicitDiscoveryMatchesPluginsShape reproduces the
+// exact nself-org/plugins layout: a root package.json with only an unrelated
+// script ("ci:local"), no pnpm-workspace.yaml, no "workspaces" field, and
+// nested package.json files at varying depths (mirrors ".workers/plugins-registry"
+// and "free/feature-flags/sdk-ts"). Before the fix this produced workspaceKind
+// == workspaceNone and zero members; it must now find both.
+func TestDetectNodeWorkspace_ImplicitDiscoveryMatchesPluginsShape(t *testing.T) {
+	root := t.TempDir()
+	writePackageJSON(t, root, `{"version":"1.0.0","scripts":{"ci:local":"echo ok"}}`)
+	writePackageJSON(t, filepath.Join(root, ".workers", "plugins-registry"),
+		`{"name":"registry","scripts":{"typecheck":"tsc --noEmit","test":"node --test"}}`)
+	writePackageJSON(t, filepath.Join(root, "free", "feature-flags", "sdk-ts"),
+		`{"name":"sdk-ts","scripts":{"build":"tsc","test":"jest"}}`)
+
+	ws := detectNodeWorkspace(root)
+
+	if ws.kind != workspaceImplicit {
+		t.Fatalf("expected workspaceImplicit (no formal declaration, nested package.json present), got kind=%d members=%v", ws.kind, ws.members)
+	}
+	if len(ws.members) != 2 {
+		t.Fatalf("expected 2 implicit members, got %d: %v", len(ws.members), ws.members)
+	}
+	wantA := filepath.Join(root, ".workers", "plugins-registry")
+	wantB := filepath.Join(root, "free", "feature-flags", "sdk-ts")
+	if !slices.Contains(ws.members, wantA) {
+		t.Errorf("expected member %s, got %v", wantA, ws.members)
+	}
+	if !slices.Contains(ws.members, wantB) {
+		t.Errorf("expected member %s, got %v", wantB, ws.members)
+	}
+}
+
+// TestDiscoverNestedMembers_SkipsDependencyAndBuildDirs asserts the fallback
+// walk never descends into node_modules/dist/.git — a repo where every real
+// package.json lives inside node_modules (typical after `pnpm install`) must
+// not report thousands of vendored packages as workspace members.
+func TestDiscoverNestedMembers_SkipsDependencyAndBuildDirs(t *testing.T) {
+	root := t.TempDir()
+	writePackageJSON(t, filepath.Join(root, "node_modules", "some-dep"), `{"name":"some-dep"}`)
+	writePackageJSON(t, filepath.Join(root, "dist"), `{"name":"build-output"}`)
+	writePackageJSON(t, filepath.Join(root, "real-app"), `{"name":"real-app","scripts":{"test":"true"}}`)
+
+	members := discoverNestedMembers(root, implicitMemberMaxDepth)
+
+	if len(members) != 1 {
+		t.Fatalf("expected exactly 1 real member (node_modules/dist must be skipped), got %d: %v", len(members), members)
+	}
+	if members[0] != filepath.Join(root, "real-app") {
+		t.Errorf("expected real-app, got %v", members)
+	}
+}
+
+// TestDiscoverNestedMembers_RespectsMaxDepth ensures the walk does not surface
+// a package.json buried deeper than the configured bound, keeping the
+// fallback a scoped discovery rather than a full-repo crawl.
+func TestDiscoverNestedMembers_RespectsMaxDepth(t *testing.T) {
+	root := t.TempDir()
+	deep := filepath.Join(root, "a", "b", "c", "d", "e", "f")
+	writePackageJSON(t, deep, `{"name":"too-deep"}`)
+
+	members := discoverNestedMembers(root, 2)
+
+	if len(members) != 0 {
+		t.Fatalf("expected 0 members beyond maxDepth, got %v", members)
+	}
+}
+
+// TestDetectNodeWorkspace_DeclaredPnpmWorkspaceWins asserts a real
+// pnpm-workspace.yaml still takes priority over the implicit walk and is
+// resolved via its own glob semantics, unchanged from before this fix.
+func TestDetectNodeWorkspace_DeclaredPnpmWorkspaceWins(t *testing.T) {
+	root := t.TempDir()
+	writePackageJSON(t, root, `{"name":"root"}`)
+	if err := os.WriteFile(filepath.Join(root, "pnpm-workspace.yaml"), []byte("packages:\n  - 'packages/*'\n"), 0o644); err != nil {
+		t.Fatalf("write pnpm-workspace.yaml: %v", err)
+	}
+	writePackageJSON(t, filepath.Join(root, "packages", "a"), `{"name":"a","scripts":{"test":"true"}}`)
+
+	ws := detectNodeWorkspace(root)
+
+	if ws.kind != workspaceDeclared {
+		t.Fatalf("expected workspaceDeclared, got kind=%d", ws.kind)
+	}
+	if len(ws.members) != 1 || ws.members[0] != filepath.Join(root, "packages", "a") {
+		t.Fatalf("expected [packages/a], got %v", ws.members)
+	}
+}
+
+// TestDeclaredWorkspacePatterns_PackageJSONWorkspacesField pins the fix for a
+// previously-dead code path: the old workspaceMembers() read
+// loadPackageJSON(root)["workspaces"], but loadPackageJSON only ever returns
+// the "scripts" sub-object, so that type assertion could never succeed —
+// package.json "workspaces" was silently never honoured. This must now work.
+func TestDeclaredWorkspacePatterns_PackageJSONWorkspacesField(t *testing.T) {
+	root := t.TempDir()
+	writePackageJSON(t, root, `{"name":"root","workspaces":["apps/*","libs/*"]}`)
+
+	patterns := declaredWorkspacePatterns(root)
+
+	if !slices.Contains(patterns, "apps/*") || !slices.Contains(patterns, "libs/*") {
+		t.Fatalf("expected [\"apps/*\" \"libs/*\"], got %v", patterns)
+	}
+}
+
+// TestDetectNodeWorkspace_NoPackageAnywhereIsNone asserts a plain single
+// package (no nested package.json, no declaration) still resolves to
+// workspaceNone rather than spuriously discovering itself.
+func TestDetectNodeWorkspace_NoPackageAnywhereIsNone(t *testing.T) {
+	root := t.TempDir()
+	writePackageJSON(t, root, `{"name":"solo","scripts":{"test":"true"}}`)
+
+	ws := detectNodeWorkspace(root)
+
+	if ws.kind != workspaceNone {
+		t.Fatalf("expected workspaceNone for a repo with no nested members, got kind=%d members=%v", ws.kind, ws.members)
+	}
+}
+
+// TestSkipReasonForScript_NamesTheScriptAndMemberCount asserts a skip reason
+// is never a bare, unexplained line — it must name the script and say
+// whether a workspace was even considered.
+func TestSkipReasonForScript_NamesTheScriptAndMemberCount(t *testing.T) {
+	none := skipReasonForScript("lint", nodeWorkspace{kind: workspaceNone})
+	if !strings.Contains(none, `"lint"`) || !strings.Contains(none, "no workspace members found") {
+		t.Errorf("workspaceNone reason missing detail: %q", none)
+	}
+
+	withMembers := skipReasonForScript("build", nodeWorkspace{kind: workspaceImplicit, members: []string{"a", "b", "c"}})
+	if !strings.Contains(withMembers, `"build"`) || !strings.Contains(withMembers, "3 workspace member") {
+		t.Errorf("workspaceImplicit reason missing member count: %q", withMembers)
+	}
+}

free/ci/internal/gate_run.go

@@ -112,6 +112,42 @@ func Run(cfg Config) (*Result, error) {
 		return res, nil
 	}
 
+	// 8. Reject a run where every gate that DID execute was non-substantive.
+	//
+	// G-015: (1) above only caught the case of literally zero gates. Wiring
+	// this gate into nself-org/plugins showed a second, worse shape of the
+	// same bug — gitleaks ran (1 gate, len(res.Gates) != 0) while the
+	// detected "node" stack contributed nothing, because its scripts live in
+	// nested member packages the old detector never looked at. The result
+	// printed "Overall: PASSED" having verified zero lines of code. A gate
+	// meant to be a required merge check must never look identical whether
+	// it checked everything or checked nothing, so this counts only gates
+	// that actually ran a lint/test/build/analyze step (Substantive &&
+	// !Skipped) and refuses to pass when that count is zero, regardless of
+	// how many meta/skipped entries are present.
+	substantive := 0
+	for _, g := range res.Gates {
+		if g.Substantive && !g.Skipped {
+			substantive++
+		}
+	}
+	if substantive == 0 {
+		res.Passed = false
+		res.Gates = append(res.Gates, GateResult{
+			Name:   "gate:no-substantive-checks",
+			Passed: false,
+			Output: fmt.Sprintf(
+				"Detected stack(s) %s produced zero lint/test/build/analyze checks — "+
+					"only non-code checks ran (e.g. secrets scan). See the SKIP entries "+
+					"above for which checks were skipped and why.\n"+
+					"Passing this would report \"Overall: PASSED\" without verifying any code.",
+				strings.Join(res.Stack, "+"),
+			),
+		})
+		res.Elapsed = time.Since(start)
+		return res, nil
+	}
+
 	res.Passed = true
 	for _, g := range res.Gates {
 		if !g.Passed {