diff --git a/free/ci/cmd/main.go b/free/ci/cmd/main.go index aefce08..b4a4e22 100644 --- a/free/ci/cmd/main.go +++ b/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) } diff --git a/free/ci/internal/gate.go b/free/ci/internal/gate.go index c53a064..5a674e6 100644 --- a/free/ci/internal/gate.go +++ b/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. diff --git a/free/ci/internal/gate_helpers.go b/free/ci/internal/gate_helpers.go index 36507d6..5a1d430 100644 --- a/free/ci/internal/gate_helpers.go +++ b/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 { diff --git a/free/ci/internal/gate_helpers_test.go b/free/ci/internal/gate_helpers_test.go new file mode 100644 index 0000000..e755127 --- /dev/null +++ b/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) + } +} diff --git a/free/ci/internal/gate_run.go b/free/ci/internal/gate_run.go index b03d3d8..21ce4c0 100644 --- a/free/ci/internal/gate_run.go +++ b/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 { diff --git a/free/ci/internal/gate_run_test.go b/free/ci/internal/gate_run_test.go new file mode 100644 index 0000000..6eb4d3a --- /dev/null +++ b/free/ci/internal/gate_run_test.go @@ -0,0 +1,203 @@ +// Regression tests for Run()'s pass/fail aggregation — specifically the +// near-empty-run case at the heart of G-015. +// +// WHY these exist: wiring this gate into nself-org/plugins produced exactly +// this real output: +// +// Stacks: node +// secrets:gitleaks PASS (2-4s) +// Overall: PASSED +// +// One gate ran (gitleaks), so the pre-existing "zero gates" guard below never +// fired, and a required merge-gate status check reported PASSED having +// verified zero lines of code. These tests pin the fix: Run() must count only +// gates that actually verified something (Substantive && !Skipped) and must +// refuse to report an unqualified pass when that count is zero, however many +// non-substantive gates ran alongside it. +package internal + +import ( + "os" + "os/exec" + "strings" + "testing" +) + +// TestRun_GitleaksOnlyIsNotASilentPass reproduces the exact G-015 shape: a +// node stack whose only script is unrelated to lint/typecheck/test/build (no +// workspace members either), with gitleaks left enabled. Before this fix, +// Run() would report Passed=true off the strength of the secrets scan alone. +func TestRun_GitleaksOnlyIsNotASilentPass(t *testing.T) { + if _, err := exec.LookPath("gitleaks"); err != nil { + t.Skip("skip: gitleaks binary not found on PATH") + } + if _, err := exec.LookPath("git"); err != nil { + t.Skip("skip: git binary not found on PATH") + } + + root := t.TempDir() + // Mirrors nself-org/plugins' root package.json: a real script, but not + // one of the four the node gate checks for, and no nested member + // packages at all. + writePackageJSON(t, root, `{"version":"1.1.7","scripts":{"ci:local":"echo ok"}}`) + + runGit := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = root + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=nself-ci-test", "GIT_AUTHOR_EMAIL=ci-test@nself.org", + "GIT_COMMITTER_NAME=nself-ci-test", "GIT_COMMITTER_EMAIL=ci-test@nself.org", + ) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + runGit("init", "-q", "-b", "main") + runGit("add", "package.json") + runGit("commit", "-q", "-m", "initial commit") + + result, err := Run(Config{RepoRoot: root, StepTimeout: 30}) + if err != nil { + t.Fatalf("Run() error: %v", err) + } + + if result.Passed { + t.Fatalf("G-015 regression: a run where only gitleaks executed must not pass, got:\n%s", dumpGates(result)) + } + + foundGitleaks, gitleaksPassed := false, false + for _, g := range result.Gates { + if g.Name == "secrets:gitleaks" { + foundGitleaks, gitleaksPassed = true, g.Passed + } + } + if !foundGitleaks { + t.Fatalf("expected a secrets:gitleaks gate to have run, got:\n%s", dumpGates(result)) + } + if !gitleaksPassed { + t.Fatalf("expected the clean fixture repo to pass gitleaks (so the FAILED overall comes from the substantive-check guard, not a secrets false-positive):\n%s", dumpGates(result)) + } + + foundGuard := false + for _, g := range result.Gates { + if g.Name == "gate:no-substantive-checks" { + foundGuard = true + if g.Passed { + t.Errorf("gate:no-substantive-checks must itself be Passed=false") + } + } + } + if !foundGuard { + t.Fatalf("expected a gate:no-substantive-checks entry explaining the failure, got:\n%s", dumpGates(result)) + } +} + +// TestRun_RealScriptCountsAsSubstantive is the positive-path counterpart: once +// a real check runs (here, a trivial passing "test" script), the substantive +// guard must not block a genuine pass. +func TestRun_RealScriptCountsAsSubstantive(t *testing.T) { + pm := "pnpm" + if _, err := exec.LookPath(pm); err != nil { + pm = "npm" + if _, err := exec.LookPath(pm); err != nil { + t.Skip("skip: neither pnpm nor npm found on PATH") + } + } + + root := t.TempDir() + writePackageJSON(t, root, `{"name":"solo","version":"1.0.0","scripts":{"test":"node -e \"process.exit(0)\""}}`) + + result, err := Run(Config{RepoRoot: root, SkipGitleaks: true, StepTimeout: 30}) + if err != nil { + t.Fatalf("Run() error: %v", err) + } + + if !result.Passed { + t.Fatalf("expected a genuine passing script to pass the gate, got:\n%s", dumpGates(result)) + } + + substantiveRan := false + for _, g := range result.Gates { + if g.Name == "node:test" && g.Substantive && !g.Skipped { + substantiveRan = true + } + } + if !substantiveRan { + t.Fatalf("expected node:test to be marked Substantive and not Skipped, got:\n%s", dumpGates(result)) + } +} + +// TestRun_NodeStackWithNoScriptsAnywhereFailsExplicitly covers a bare +// package.json with no scripts, no workspace, and no tsconfig.json: every one +// of the 4 checked scripts (lint/typecheck/test/build) resolves to an +// explicit Skipped gate with its own reason (never a bare silent gap), and +// the run as a whole fails via gate:no-substantive-checks rather than +// reporting PASSED. +// +// This also documents that the pre-existing "zero gates" branch in Run() is +// no longer reachable through the node runner specifically: runNodeGates now +// always emits a Skipped entry per unmatched script instead of an empty +// slice, which is the whole point — a silent gap must announce itself. The +// zero-gates branch still exists as defense-in-depth for a future stack with +// no runner wired in the switch. +func TestRun_NodeStackWithNoScriptsAnywhereFailsExplicitly(t *testing.T) { + root := t.TempDir() + writePackageJSON(t, root, `{"name":"empty","version":"1.0.0"}`) + + result, err := Run(Config{RepoRoot: root, SkipGitleaks: true, StepTimeout: 5}) + if err != nil { + t.Fatalf("Run() error: %v", err) + } + if result.Passed { + t.Fatalf("a repo with no substantive checks anywhere must not pass, got:\n%s", dumpGates(result)) + } + + wantSkipped := map[string]bool{"node:lint": false, "node:typecheck": false, "node:test": false, "node:build": false} + foundGuard := false + for _, g := range result.Gates { + if _, ok := wantSkipped[g.Name]; ok { + if !g.Skipped || g.Output == "" { + t.Errorf("%s: expected Skipped=true with a non-empty reason, got skipped=%v output=%q", g.Name, g.Skipped, g.Output) + } + wantSkipped[g.Name] = true + } + if g.Name == "gate:no-substantive-checks" { + foundGuard = true + } + } + for name, seen := range wantSkipped { + if !seen { + t.Errorf("expected a %s gate entry, got:\n%s", name, dumpGates(result)) + } + } + if !foundGuard { + t.Fatalf("expected gate:no-substantive-checks, got:\n%s", dumpGates(result)) + } +} + +func dumpGates(r *Result) string { + var b strings.Builder + for _, g := range r.Gates { + b.WriteString(" " + g.Name + " passed=") + if g.Passed { + b.WriteString("true") + } else { + b.WriteString("false") + } + b.WriteString(" substantive=") + if g.Substantive { + b.WriteString("true") + } else { + b.WriteString("false") + } + b.WriteString(" skipped=") + if g.Skipped { + b.WriteString("true") + } else { + b.WriteString("false") + } + b.WriteString("\n") + } + return b.String() +} diff --git a/free/ci/internal/gate_runners.go b/free/ci/internal/gate_runners.go index 5ac3ad0..5a4dfc5 100644 --- a/free/ci/internal/gate_runners.go +++ b/free/ci/internal/gate_runners.go @@ -57,78 +57,38 @@ func gitleaksArgs(root, configFlag string, isRepo bool) []string { return args } +// markSubstantive flags each gate as substantive (a real lint/test/build/ +// analyze check) unless runStep already marked it Skipped — e.g. the tool +// binary was missing. A step that never executed a command must not count +// toward "something was verified" just because its caller intended it to. +// G-015 / SPORT: PLUGINS-CI-007 +func markSubstantive(gates []GateResult) { + for i := range gates { + if !gates[i].Skipped { + gates[i].Substantive = true + } + } +} + // runGoGates runs gofmt, go vet, and go test for a Go repo. func runGoGates(root string, timeout int, verbose bool) []GateResult { - return []GateResult{ + gates := []GateResult{ runStep("go:fmt", root, timeout, verbose, "gofmt", "-l", "."), runStep("go:vet", root, timeout, verbose, "go", "vet", "./..."), runStep("go:test", root, timeout, verbose, "go", "test", "-count=1", "-timeout", fmt.Sprintf("%ds", timeout), "./..."), } -} - -// runNodeGates runs pnpm lint, pnpm test, and pnpm build for a Node repo. -// Falls back to npm if pnpm is not present. -func runNodeGates(root string, timeout int, verbose bool) []GateResult { - pm := "pnpm" - if _, err := exec.LookPath("pnpm"); err != nil { - pm = "npm" - } - - pkg := loadPackageJSON(root) - - // A pnpm/npm workspace keeps its real scripts in member packages, not the - // root package.json. Reading only the root made whole repos run ZERO gates - // and still report PASSED. Recurse when the root has no script of its own - // but a member does. - if members := workspaceMembers(root); len(members) > 0 { - var gates []GateResult - for _, script := range []string{"lint", "typecheck", "test", "build"} { - if hasScript(pkg, script) { - gates = append(gates, runStep("node:"+script, root, timeout, verbose, pm, "run", script)) - continue - } - if anyMemberHasScript(members, script) { - // --if-present so members without the script are skipped rather - // than failing the whole recursive run. - gates = append(gates, runStep("node:"+script+" (workspace)", root, timeout, verbose, - pm, "-r", "--if-present", "run", script)) - } - } - if len(gates) > 0 { - return gates - } - } - - var gates []GateResult - if hasScript(pkg, "lint") { - gates = append(gates, runStep("node:lint", root, timeout, verbose, pm, "run", "lint")) - } - if hasScript(pkg, "typecheck") { - gates = append(gates, runStep("node:typecheck", root, timeout, verbose, pm, "run", "typecheck")) - } - if hasScript(pkg, "test") { - gates = append(gates, runStep("node:test", root, timeout, verbose, pm, "run", "test")) - } - if hasScript(pkg, "build") { - gates = append(gates, runStep("node:build", root, timeout, verbose, pm, "run", "build")) - } - - if len(gates) == 0 { - // No scripts found; at minimum run tsc if tsconfig.json exists. - if fileExists(filepath.Join(root, "tsconfig.json")) { - gates = append(gates, runStep("node:tsc", root, timeout, verbose, pm, "exec", "tsc", "--noEmit")) - } - } - + markSubstantive(gates) return gates } // runFlutterGates runs flutter analyze and flutter test. func runFlutterGates(root string, timeout int, verbose bool) []GateResult { - return []GateResult{ + gates := []GateResult{ runStep("flutter:analyze", root, timeout, verbose, "flutter", "analyze"), runStep("flutter:test", root, timeout, verbose, "flutter", "test", "--reporter", "compact"), } + markSubstantive(gates) + return gates } // runRustGates runs cargo clippy (deny warnings) and cargo test for a Rust crate. @@ -138,12 +98,14 @@ func runFlutterGates(root string, timeout int, verbose bool) []GateResult { // Outputs: []GateResult — clippy lint + unit test results // Constraints: clippy --deny warnings; cargo test --all-features; SPORT PLUGINS-CI-004 func runRustGates(root string, timeout int, verbose bool) []GateResult { - return []GateResult{ + gates := []GateResult{ runStep("rust:clippy", root, timeout, verbose, "cargo", "clippy", "--all-targets", "--all-features", "--", "--deny", "warnings"), runStep("rust:test", root, timeout, verbose, "cargo", "test", "--all-features"), } + markSubstantive(gates) + return gates } // runGatewayRoutingCheck verifies that the nself-ai-gateway on staging responds @@ -205,6 +167,7 @@ func runStep(name, root string, timeout int, verbose bool, cmd string, args ...s if _, err := exec.LookPath(cmd); err != nil { gr.Output = fmt.Sprintf("command not found: %s (skipped)", cmd) gr.Passed = true // Skip missing optional tools gracefully. + gr.Skipped = true gr.Elapsed = time.Since(start) return gr } diff --git a/free/ci/internal/gate_runners_node.go b/free/ci/internal/gate_runners_node.go new file mode 100644 index 0000000..8e58e84 --- /dev/null +++ b/free/ci/internal/gate_runners_node.go @@ -0,0 +1,111 @@ +package internal + +// Package internal — gate_runners_node.go +// +// Purpose: Run lint/typecheck/test/build for a Node repo, across the root +// package and any workspace members (declared or implicitly discovered — +// see gate_workspace.go), and explain in the output why any of those four +// checks never ran anywhere. +// Inputs: root string, timeout int, verbose bool +// Outputs: []GateResult — one per script that ran, plus one Skipped entry +// per script that ran nowhere, each carrying the reason in Output. +// Constraints: falls back to npm if pnpm is not on PATH. G-015 — +// nself-org/plugins has 5 nested package.json files and NEITHER a +// pnpm-workspace.yaml NOR a "workspaces" field, so the prior member +// detection found nothing and the gate ran only gitleaks while still +// reporting "Overall: PASSED". +// SPORT: PLUGINS-CI-007 + +import ( + "fmt" + "os/exec" + "path/filepath" +) + +// runNodeGates runs lint/typecheck/test/build for a Node repo. +func runNodeGates(root string, timeout int, verbose bool) []GateResult { + pm := "pnpm" + if _, err := exec.LookPath("pnpm"); err != nil { + pm = "npm" + } + + pkg := loadPackageJSON(root) + ws := detectNodeWorkspace(root) + + var gates []GateResult + ranAny := false + for _, script := range []string{"lint", "typecheck", "test", "build"} { + scriptGates := runNodeScript(pm, root, script, pkg, ws, timeout, verbose) + for _, g := range scriptGates { + if !g.Skipped { + ranAny = true + } + } + gates = append(gates, scriptGates...) + } + + // Nothing declared any of the four scripts anywhere, and there is no + // workspace to recurse into — last resort, run tsc directly if the repo + // at least declares a tsconfig.json. + if !ranAny && ws.kind == workspaceNone && fileExists(filepath.Join(root, "tsconfig.json")) { + gates = append(gates, runStep("node:tsc", root, timeout, verbose, pm, "exec", "tsc", "--noEmit")) + } + + markSubstantive(gates) + return gates +} + +// runNodeScript runs one script name (lint/typecheck/test/build) against the +// root package and, per ws.kind, its workspace members — returning a Skipped +// gate explaining why when the script exists nowhere. +func runNodeScript(pm, root, script string, pkg map[string]interface{}, ws nodeWorkspace, timeout int, verbose bool) []GateResult { + var gates []GateResult + + if hasScript(pkg, script) { + gates = append(gates, runStep("node:"+script, root, timeout, verbose, pm, "run", script)) + } + + switch ws.kind { + case workspaceDeclared: + // A real pnpm/npm workspace understands `-r`/`--workspaces` recursion. + // --if-present so members without the script are skipped rather than + // failing the whole recursive run. + if anyMemberHasScript(ws.members, script) { + gates = append(gates, runStep("node:"+script+" (workspace)", root, timeout, verbose, + pm, "-r", "--if-present", "run", script)) + } + case workspaceImplicit: + // No formal workspace declaration exists, so `pnpm -r` would not + // recurse into these directories at all — run each member on its own. + for _, member := range ws.members { + if !hasScript(loadPackageJSON(member), script) { + continue + } + rel, err := filepath.Rel(root, member) + if err != nil { + rel = member + } + gates = append(gates, runStep(fmt.Sprintf("node:%s (%s)", script, rel), member, timeout, verbose, pm, "run", script)) + } + } + + if len(gates) == 0 { + gates = append(gates, GateResult{ + Name: "node:" + script, + Passed: true, + Skipped: true, + Output: skipReasonForScript(script, ws), + }) + } + + return gates +} + +// skipReasonForScript explains why a given script never ran anywhere, so a +// SKIP entry is never a bare, unexplained line. G-015 / SPORT: PLUGINS-CI-007 +func skipReasonForScript(script string, ws nodeWorkspace) string { + if ws.kind == workspaceNone { + return fmt.Sprintf("skipped: no %q script in package.json and no workspace members found", script) + } + return fmt.Sprintf("skipped: no %q script in package.json or any of %d workspace member(s)", script, len(ws.members)) +} diff --git a/free/ci/internal/gate_workspace.go b/free/ci/internal/gate_workspace.go new file mode 100644 index 0000000..c4b5ad5 --- /dev/null +++ b/free/ci/internal/gate_workspace.go @@ -0,0 +1,174 @@ +package internal + +// Package internal — gate_workspace.go +// +// Purpose: Resolve a Node repo's workspace member packages so the node gate +// can see scripts that live in member packages instead of only the root +// package.json — declared (pnpm-workspace.yaml / package.json +// "workspaces") or, failing that, merely nested with no formal declaration +// at all. +// Inputs: root string — repo root +// Outputs: nodeWorkspace{kind, members} +// Constraints: A declared workspace always wins (respects the author's exact +// glob intent, including exclusions); the implicit walk is a fallback +// only, bounded to implicitMemberMaxDepth and skipping dependency/build/VCS +// directories, so it cannot balloon into a full-repo crawl. +// SPORT: PLUGINS-CI-007 (G-015) +// +// WHY this exists as a fallback at all: nself-org/plugins has 5 nested +// package.json files (e.g. ".workers/plugins-registry", +// "free/feature-flags/sdk-ts") and declares NEITHER a pnpm-workspace.yaml NOR +// a "workspaces" field. The prior workspaceMembers() understood only those +// two formal declarations, found nothing, and the node gate ran zero checks +// while gitleaks alone reported "Overall: PASSED" — the shape of bug G-015 +// closes. + +import ( + "os" + "path/filepath" + "strings" +) + +// workspaceKind distinguishes a formally declared pnpm/npm workspace — which +// supports `pnpm -r` / `npm --workspaces` recursion — from implicitly +// discovered member packages, which must be run one directory at a time +// since no workspace manifest tells the package manager they are related. +type workspaceKind int + +const ( + // workspaceNone: no pnpm-workspace.yaml, no "workspaces" field, and the + // fallback walk found no nested package.json either. + workspaceNone workspaceKind = iota + // workspaceDeclared: pnpm-workspace.yaml or package.json "workspaces" + // names the members explicitly. + workspaceDeclared + // workspaceImplicit: nothing declared a workspace, but nested + // package.json files were found by walking the tree. + workspaceImplicit +) + +// nodeWorkspace is the result of resolving a Node repo's workspace members. +type nodeWorkspace struct { + kind workspaceKind + members []string +} + +// implicitMemberMaxDepth bounds the fallback walk in discoverNestedMembers. +// 4 covers every case seen in nself-org repos (e.g. plugins' +// free//ts/package.json sits 3 directories below the repo root) +// without turning into an unbounded crawl of an entire monorepo. +const implicitMemberMaxDepth = 4 + +// skipNestedDirs are directories never worth descending into while +// discovering implicit workspace members: dependency trees, build output, +// and VCS metadata. Deliberately does NOT skip all dot-directories — plugins' +// own ".workers/plugins-registry" is a real member with real scripts. +var skipNestedDirs = map[string]bool{ + "node_modules": true, ".git": true, "dist": true, "build": true, + "out": true, "coverage": true, ".next": true, ".turbo": true, + ".cache": true, "vendor": true, "target": true, ".venv": true, +} + +// detectNodeWorkspace resolves a Node repo's workspace members. Declared +// patterns (pnpm-workspace.yaml or package.json "workspaces") always take +// priority over the implicit fallback walk. +func detectNodeWorkspace(root string) nodeWorkspace { + if patterns := declaredWorkspacePatterns(root); len(patterns) > 0 { + return nodeWorkspace{kind: workspaceDeclared, members: globMembers(root, patterns)} + } + if members := discoverNestedMembers(root, implicitMemberMaxDepth); len(members) > 0 { + return nodeWorkspace{kind: workspaceImplicit, members: members} + } + return nodeWorkspace{kind: workspaceNone} +} + +// declaredWorkspacePatterns reads pnpm-workspace.yaml globs, or failing that, +// package.json's "workspaces" array. Members outside the repo (sibling +// checkouts referenced via "..") are not ours to gate. +// +// The package.json path was previously dead code: workspaceMembers() read +// loadPackageJSON(root)["workspaces"], but loadPackageJSON only ever returns +// the "scripts" sub-object, so that type assertion could never succeed — a +// "workspaces" field was silently never honoured. This reads the raw file. +func declaredWorkspacePatterns(root string) []string { + if data, err := os.ReadFile(filepath.Join(root, "pnpm-workspace.yaml")); err == nil { + var patterns []string + 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, "- ")), `'"`) + if pat != "" && !strings.HasPrefix(pat, "..") { + patterns = append(patterns, pat) + } + } + return patterns + } + + data, err := os.ReadFile(filepath.Join(root, "package.json")) + if err != nil { + return nil + } + var patterns []string + for _, pat := range extractJSONStringArray(string(data), "workspaces") { + if !strings.HasPrefix(pat, "..") { + patterns = append(patterns, pat) + } + } + return patterns +} + +// globMembers expands declared workspace glob patterns to absolute member +// directories that actually contain a package.json. +func globMembers(root string, patterns []string) []string { + 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 +} + +// discoverNestedMembers walks root — bounded to maxDepth, skipping +// dependency/build/VCS directories — collecting every directory that holds +// its own package.json. Used only as a fallback when no pnpm-workspace.yaml +// or "workspaces" field declares members explicitly; see detectNodeWorkspace. +func discoverNestedMembers(root string, maxDepth int) []string { + var members []string + + var walk func(dir string, depth int) + walk = func(dir string, depth int) { + if depth > maxDepth { + return + } + entries, err := os.ReadDir(dir) + if err != nil { + return + } + for _, e := range entries { + if !e.IsDir() || skipNestedDirs[e.Name()] { + continue + } + sub := filepath.Join(dir, e.Name()) + if fileExists(filepath.Join(sub, "package.json")) { + members = append(members, sub) + } + walk(sub, depth+1) + } + } + walk(root, 1) + return members +}