Feat/action flag fix

.gavel.yaml

@@ -1,9 +1,46 @@
-commit: {}
+commit:
+  gitignore:
+  - .bin/
+  - .cache
+  - .claude
+  - .container-sandbox.yaml
+  - .DS_Store
+  - .DS_Store?
+  - .env
+  - .env.local
+  - .gavel/
+  - .gingko/
+  - .idea/
+  - .jscpd.json
+  - .playwright-mcp/
+  - .shell/
+  - .Spotlight-V100
+  - .task
+  - .tmp/
+  - .Trashes
+  - .vitest/
+  - .vscode/
+  - '*.log'
+  - '*.test'
+  - node_modules/
+  - dist/
+  - '*.tmp'
+  - .metadata_never_index
+  - .tmp
+  - .agents/
+  - .sandbox.yaml
+  - nohup.out
+  - skills-lock.json
+  linkedDeps: {}
 fixtures: {}
 lint:
   ignore:
   - file: .shell/checkout/8d512cac74b933d1f8d97629ab79966828d2e0789c66a463d8a78e7d40ccc42f/**
-secrets: {}
+  - file: linters/betterleaks/testdata/betterleaks-output.json
+    source: betterleaks
+secrets:
+  configs:
+  - .betterleaks.toml
 ssh: {}
 verify:
   checks:

action.yml

@@ -122,10 +122,24 @@ runs:
         set +e
         rm -f "${JSON_FILE}" "${GAVEL_LOG_FILE}"
 
+        # --show-passed is only defined on `gavel test` / `gavel lint`. Other
+        # subcommands (e.g. `fixtures`) reject unknown flags and fail at parse
+        # time. Detect the first non-flag word in $GAVEL_ARGS and only append
+        # the test-specific flag for those subcommands.
+        first_subcmd=""
+        for tok in $GAVEL_ARGS; do
+          case "$tok" in
+            -*) ;;
+            *) first_subcmd="$tok"; break ;;
+          esac
+        done
+        extra_flags=("--no-progress" "--no-color" "--format" "json=${JSON_FILE}")
+        case "$first_subcmd" in
+          test|lint) extra_flags=("--show-passed" "${extra_flags[@]}") ;;
+        esac
+
         # shellcheck disable=SC2086
-        gavel $GAVEL_ARGS \
-          --no-progress --no-color --show-passed \
-          --format "json=${JSON_FILE}" \
+        gavel $GAVEL_ARGS "${extra_flags[@]}" \
           2> >(tee "${GAVEL_LOG_FILE}" >&2)
         code=$?
         set -e

cmd/gavel/summary.go

@@ -231,9 +231,25 @@ func ensureSource(sources map[string]*sourceCounts, name string) *sourceCounts {
 	return sc
 }
 
+// writeCountsTable emits a markdown table containing only sources that have
+// failures or skips. Sources with 100% passing tests are intentionally
+// omitted — their pass count and duration are rolled into the totals row
+// emitted by writeTotals so the PR comment stays short for the common
+// "all green" case. When every source is clean, the table is omitted
+// entirely and writeTotals carries the headline.
 func writeCountsTable(b *strings.Builder, sources map[string]*sourceCounts) {
-	rows := make([]*sourceCounts, 0, len(sources))
+	var rows []*sourceCounts
+	var passingOnly, passingDuration int
+	var hiddenSources int
+	totalPassing := 0
 	for _, sc := range sources {
+		totalPassing += sc.passed
+		if sc.failed == 0 && sc.skipped == 0 {
+			passingOnly += sc.passed
+			passingDuration += int(sc.duration)
+			hiddenSources++
+			continue
+		}
 		rows = append(rows, sc)
 	}
 	sort.Slice(rows, func(i, j int) bool {
@@ -245,12 +261,22 @@ func writeCountsTable(b *strings.Builder, sources map[string]*sourceCounts) {
 	})
 
 	b.WriteString("## Gavel summary\n\n")
+	if len(rows) == 0 {
+		// Nothing to break out — totals row alone tells the story.
+		return
+	}
 	b.WriteString("| Source | Pass | Fail | Skip | Duration |\n")
 	b.WriteString("|---|---:|---:|---:|---:|\n")
 	for _, sc := range rows {
 		fmt.Fprintf(b, "| %s | %d | %d | %d | %s |\n",
 			escapePipe(sc.name), sc.passed, sc.failed, sc.skipped, formatDuration(sc.duration))
 	}
+	if hiddenSources > 0 {
+		// One collapsed row covers every all-passing source so a reader
+		// can still see "we ran more than what's listed".
+		fmt.Fprintf(b, "| _%d more passing source(s)_ | %d | 0 | 0 | %s |\n",
+			hiddenSources, passingOnly, formatDuration(time.Duration(passingDuration)))
+	}
 	b.WriteString("\n")
 }
 

cmd/gavel/summary_test.go

@@ -84,16 +84,30 @@ func TestBuildCompactSummary(t *testing.T) {
 
 	out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200})
 
-	// Counts table by source — expect one row per test package AND one per linter.
+	// Counts table by source — only sources that have failures or skips
+	// should appear. The serve package has failures, the verify package
+	// has a skipped test, and golangci-lint reports a violation, so all
+	// three keep their row. Sources that are 100% passing (e.g. the
+	// clean gofmt linter below) are folded into the passing-only summary
+	// row + Totals line.
 	if !strings.Contains(out, "github.com/flanksource/gavel/serve") {
 		t.Errorf("expected serve package row in counts table, got:\n%s", out)
 	}
 	if !strings.Contains(out, "github.com/flanksource/gavel/verify") {
-		t.Errorf("expected verify package row in counts table, got:\n%s", out)
+		t.Errorf("expected verify package row (has a skipped test) in counts table, got:\n%s", out)
 	}
 	if !strings.Contains(out, "golangci-lint") {
 		t.Errorf("expected golangci-lint row in counts table, got:\n%s", out)
 	}
+	// gofmt is a passing-only linter — must not appear as its own row.
+	if strings.Contains(out, "| lint: gofmt |") {
+		t.Errorf("passing-only linter must be folded into the passing summary row, got:\n%s", out)
+	}
+	// The collapsed-passing row should mention the gofmt linter's pass count
+	// (1) and the "more passing source(s)" text.
+	if !strings.Contains(out, "more passing source(s)") {
+		t.Errorf("expected collapsed passing-source row, got:\n%s", out)
+	}
 
 	// Passing rows present, but no per-test listing for passing tests in the detail section.
 	if strings.Contains(out, "initializes a bare git repo") {
@@ -280,8 +294,11 @@ func TestBuildCompactSummaryPrefersRealResultsOverCrashField(t *testing.T) {
 		Error: "partial error that should be ignored",
 	}
 	out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200})
-	if !strings.Contains(out, "pkg/x") {
-		t.Errorf("expected normal results to render, got:\n%s", out)
+	// pkg/x is 100% passing so its row is collapsed into the totals; we
+	// only need to confirm the normal counts path ran (Totals line) and
+	// that the crash-stub path did not render.
+	if !strings.Contains(out, "**Totals:** 1 passed") {
+		t.Errorf("expected normal Totals line for the single passing test, got:\n%s", out)
 	}
 	if strings.Contains(out, "Gavel crashed") {
 		t.Errorf("normal results must not fall through to crash path, got:\n%s", out)
@@ -329,6 +346,85 @@ func TestRunSummaryReadsJSONFile(t *testing.T) {
 	}
 }
 
+// TestBuildCompactSummaryCollapsesAllPassingSources exercises the
+// "PR comment must stay short" behaviour: when every source is 100%
+// passing, the per-source table disappears entirely and the Totals
+// line carries the full headline (count + duration). Regressing to
+// the old behaviour (one row per package) blows up PR comments on
+// large monorepos.
+func TestBuildCompactSummaryCollapsesAllPassingSources(t *testing.T) {
+	input := gavelResultJSON{
+		Tests: []parsers.Test{
+			{Package: "pkg/a", Children: parsers.Tests{
+				{Package: "pkg/a", Name: "TestOne", Passed: true, Duration: 10 * time.Millisecond},
+				{Package: "pkg/a", Name: "TestTwo", Passed: true, Duration: 20 * time.Millisecond},
+			}},
+			{Package: "pkg/b", Children: parsers.Tests{
+				{Package: "pkg/b", Name: "TestThree", Passed: true, Duration: 30 * time.Millisecond},
+			}},
+		},
+	}
+	out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200})
+
+	// No per-package rows when every source is clean.
+	if strings.Contains(out, "| pkg/a |") {
+		t.Errorf("all-passing pkg/a must not appear as its own row, got:\n%s", out)
+	}
+	if strings.Contains(out, "| pkg/b |") {
+		t.Errorf("all-passing pkg/b must not appear as its own row, got:\n%s", out)
+	}
+	// No table headers either — the table itself collapses.
+	if strings.Contains(out, "| Source |") {
+		t.Errorf("counts table must collapse when every source is clean, got:\n%s", out)
+	}
+	// Totals line still carries the headline.
+	if !strings.Contains(out, "**Totals:** 3 passed · 0 failed · 0 skipped") {
+		t.Errorf("expected Totals line with aggregate counts, got:\n%s", out)
+	}
+}
+
+// TestBuildCompactSummaryCollapsesPassingAlongsideFailing checks the
+// mixed case: failing sources keep their own row, but every all-passing
+// source is folded into a single trailing summary row whose pass count
+// matches the sum of the hidden sources.
+func TestBuildCompactSummaryCollapsesPassingAlongsideFailing(t *testing.T) {
+	input := gavelResultJSON{
+		Tests: []parsers.Test{
+			{Package: "pkg/fail", Children: parsers.Tests{
+				{Package: "pkg/fail", Name: "TestBoom", Failed: true, Message: "boom"},
+			}},
+			{Package: "pkg/clean1", Children: parsers.Tests{
+				{Package: "pkg/clean1", Name: "TestOne", Passed: true, Duration: 100 * time.Millisecond},
+				{Package: "pkg/clean1", Name: "TestTwo", Passed: true, Duration: 100 * time.Millisecond},
+			}},
+			{Package: "pkg/clean2", Children: parsers.Tests{
+				{Package: "pkg/clean2", Name: "TestThree", Passed: true, Duration: 50 * time.Millisecond},
+			}},
+		},
+	}
+	out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200})
+
+	// Failing source keeps its dedicated row.
+	if !strings.Contains(out, "| pkg/fail |") {
+		t.Errorf("failing pkg/fail row must be present, got:\n%s", out)
+	}
+	// Clean sources do not appear by name…
+	if strings.Contains(out, "pkg/clean1") {
+		t.Errorf("clean pkg/clean1 must not appear by name, got:\n%s", out)
+	}
+	if strings.Contains(out, "pkg/clean2") {
+		t.Errorf("clean pkg/clean2 must not appear by name, got:\n%s", out)
+	}
+	// …but a single collapsed row reports the count of hidden sources
+	// AND the sum of their pass counts (2 + 1 = 3).
+	if !strings.Contains(out, "_2 more passing source(s)_") {
+		t.Errorf("expected '2 more passing source(s)' collapse row, got:\n%s", out)
+	}
+	if !strings.Contains(out, "| _2 more passing source(s)_ | 3 | 0 | 0 |") {
+		t.Errorf("expected collapsed row with summed pass count, got:\n%s", out)
+	}
+}
+
 func longMultilineStderr() string {
 	var sb strings.Builder
 	// 8 lines of 250 chars each — must be truncated to 5 lines of ≤200 chars.

cmd/gavel/test.go

@@ -544,7 +544,11 @@ var testCmd *cobra.Command
 
 func init() {
 	testCmd = clicky.AddNamedCommand("test", rootCmd, testrunner.RunOptions{}, runTests)
-	testCmd.Flags().SetInterspersed(false)
+	// Allow flags and positional package paths to interleave so callers (e.g. the
+	// flanksource/gavel composite action) can append flags after user-supplied
+	// paths. Use `--` to terminate flag parsing when forwarding flags to the
+	// underlying runner via the `gavel test ./pkg -- --focus X` idiom.
+	testCmd.Flags().SetInterspersed(true)
 	testCmd.Flags().BoolVar(&testDurationFlags.Detach, "detach", false,
 		"With --ui, fork a detached UI server and exit. The child serves until --auto-stop (default 30m) or --idle-timeout (default 5m) fires.")
 	testCmd.Flags().DurationVar(&testDurationFlags.AutoStop, "auto-stop", 0,

cmd/gavel/test_framework_subcmds.go

@@ -24,7 +24,7 @@ func registerTestFrameworkSubcommands() {
 			return runTests(opts)
 		})
 		sub.Short = fmt.Sprintf("Run only %s tests", fw)
-		sub.Flags().SetInterspersed(false)
+		sub.Flags().SetInterspersed(true)
 		if err := sub.Flags().MarkHidden("framework"); err != nil {
 			panic(fmt.Sprintf("hide --framework on %s subcommand: %v", name, err))
 		}

cmd/gavel/ui_serve.go

@@ -6,8 +6,10 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/flanksource/clicky"
@@ -94,7 +96,14 @@ func runUIServe(opts UIServeOptions) (any, error) {
 			return nil, fmt.Errorf("load results %v: %w", opts.ResultsFiles, err)
 		}
 	} else {
+		root, err := resolveGavelRoot()
+		if err != nil {
+			listener.Close() //nolint:errcheck
+			return nil, err
+		}
+		srv.SetGavelDir(root)
 		srv.MarkDone()
+		logger.Infof("Indexing snapshots from %s", filepath.Join(root, ".gavel"))
 	}
 
 	addr := listener.Addr().(*net.TCPAddr)
@@ -315,6 +324,28 @@ func firstNonLoopbackIPv4() string {
 	return ""
 }
 
+// resolveGavelRoot returns the git root of the current working directory and
+// validates that .gavel/ exists inside it. Used by `gavel ui serve` (no args)
+// to discover the snapshot directory to index.
+func resolveGavelRoot() (string, error) {
+	out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output()
+	if err != nil {
+		return "", fmt.Errorf("not inside a git repository: %w", err)
+	}
+	root := strings.TrimSpace(string(out))
+	if root == "" {
+		return "", fmt.Errorf("git rev-parse --show-toplevel returned empty")
+	}
+	gavelDir := filepath.Join(root, ".gavel")
+	if _, err := os.Stat(gavelDir); err != nil {
+		if os.IsNotExist(err) {
+			return "", fmt.Errorf("no %s directory found in %s — run `gavel test` to populate it, or pass a snapshot path explicitly", ".gavel", root)
+		}
+		return "", err
+	}
+	return root, nil
+}
+
 // writeURLFile writes url to path atomically (write to tempfile in same dir,
 // fsync, rename). Atomicity matters because a wrapping shell script may be
 // polling the file concurrently.