Fix inherited PR evidence and noisy command checks

internal/commandfmt/commandfmt.go

@@ -0,0 +1,126 @@
+// Package commandfmt turns raw agent shell invocations into review-safe
+// command evidence. Hook payloads can contain whole shell scripts; committed
+// shale should carry the check that ran, not heredoc file bodies or local
+// machine paths.
+package commandfmt
+
+import (
+	"path/filepath"
+	"regexp"
+	"strings"
+)
+
+const maxCommandLen = 220
+
+var (
+	homePathRe = regexp.MustCompile(`(?i)(?:/Users|/home)/[^/\s'"` + "`" + `;|)]+`)
+	absPathRe  = regexp.MustCompile(`(^|[\s='":])/(?:[^\s'"` + "`" + `;|)]+)`)
+	splitRe    = regexp.MustCompile(`\s*(?:&&|\|\||;|\n)\s*`)
+	heredocRe  = regexp.MustCompile(`<<-?\s*['"]?([A-Za-z_][A-Za-z0-9_]*)['"]?`)
+)
+
+// ForEvidence returns the command form safe to persist in shale YAML.
+func ForEvidence(repoRoot, raw string) string {
+	return clean(repoRoot, raw)
+}
+
+// ForDisplay returns the command form safe to render for historical evidence
+// that may predate persistence cleanup.
+func ForDisplay(raw string) string {
+	return clean("", raw)
+}
+
+// Classify buckets an agent-invoked command for the "checks recorded" card
+// section. Heuristic by design; "other" is an honest answer.
+func Classify(cmd string) string {
+	c := strings.ToLower(cmd)
+	switch {
+	case containsAny(c, "gitleaks", "semgrep", "trivy", "snyk", "govulncheck", "bandit", "grype"):
+		return "scan"
+	case containsAny(c, "golangci-lint", "eslint", "ruff", "flake8", "pylint", "rubocop", "clippy", "go vet", "staticcheck", "tflint"):
+		return "lint"
+	case containsAny(c, "go test", "pytest", "npm test", "yarn test", "pnpm test", "jest", "vitest", "cargo test", "rspec", "mvn test", "gradle test", "make test"):
+		return "test"
+	case containsAny(c, "go build", "npm run build", "yarn build", "cargo build", "make build", "mvn package", "gradle build", "docker build", "tsc"):
+		return "build"
+	default:
+		return "other"
+	}
+}
+
+func clean(repoRoot, raw string) string {
+	cmd := stripHeredocs(strings.ReplaceAll(raw, "\r\n", "\n"))
+	for _, part := range commandParts(cmd) {
+		part = cleanOne(repoRoot, part)
+		if part != "" && Classify(part) != "other" {
+			return part
+		}
+	}
+	return cleanOne(repoRoot, cmd)
+}
+
+func stripHeredocs(s string) string {
+	var out []string
+	lines := strings.Split(s, "\n")
+	for i := 0; i < len(lines); i++ {
+		line := lines[i]
+		m := heredocRe.FindStringSubmatch(line)
+		if len(m) == 0 {
+			out = append(out, line)
+			continue
+		}
+		end := m[1]
+		for i+1 < len(lines) {
+			i++
+			if strings.TrimSpace(lines[i]) == end {
+				break
+			}
+		}
+	}
+	return strings.Join(out, "\n")
+}
+
+func commandParts(s string) []string {
+	var out []string
+	for _, part := range splitRe.Split(s, -1) {
+		part = strings.TrimSpace(part)
+		if part == "" {
+			continue
+		}
+		if i := strings.Index(part, "|"); i >= 0 {
+			out = append(out, strings.TrimSpace(part[:i]))
+			continue
+		}
+		out = append(out, part)
+	}
+	return out
+}
+
+func cleanOne(repoRoot, s string) string {
+	s = filepath.ToSlash(s)
+	if repoRoot != "" {
+		root := filepath.ToSlash(filepath.Clean(repoRoot))
+		s = strings.ReplaceAll(s, root, ".")
+	}
+	s = homePathRe.ReplaceAllString(s, "~")
+	s = absPathRe.ReplaceAllString(s, "${1}<path>")
+	s = strings.Join(strings.Fields(s), " ")
+	if strings.HasPrefix(s, "cd .") {
+		if i := strings.Index(s, ";"); i >= 0 {
+			s = strings.TrimSpace(s[i+1:])
+		}
+	}
+	if len(s) > maxCommandLen {
+		s = strings.TrimSpace(s[:maxCommandLen-1]) + "…"
+	}
+	return s
+}
+
+func containsAny(s string, subs ...string) bool {
+	for _, sub := range subs {
+		if strings.Contains(s, sub) {
+			return true
+		}
+	}
+	return false
+}

internal/commandfmt/commandfmt_test.go

@@ -0,0 +1,51 @@
+package commandfmt
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestForEvidenceExtractsCheckAndDropsHeredoc(t *testing.T) {
+	raw := `cat > pkg/query_test.go <<'EOF'
+package pkg
+
+const token = "ghp_aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789"
+EOF
+cd /Users/noahkreiger/Documents/fianulabs/core/core; go test ./pkg/criteria -run TestTranslator_NumericPropertyComparison -v 2>&1 | grep -E "PASS|FAIL" | head -25`
+
+	got := ForEvidence("/Users/noahkreiger/Documents/fianulabs/core/core", raw)
+	if got != "go test ./pkg/criteria -run TestTranslator_NumericPropertyComparison -v 2>&1" {
+		t.Fatalf("ForEvidence() = %q", got)
+	}
+	for _, banned := range []string{"package pkg", "ghp_", "/Users/noahkreiger", "grep -E", "head -25"} {
+		if strings.Contains(got, banned) {
+			t.Fatalf("ForEvidence leaked %q in %q", banned, got)
+		}
+	}
+}
+
+func TestForDisplayRedactsHistoricalLocalPaths(t *testing.T) {
+	raw := "cd /Users/noahkreiger/Documents/fianulabs/core/core; go test ./pkg/... 2>&1 | tail -20"
+	got := ForDisplay(raw)
+	if got != "go test ./pkg/... 2>&1" {
+		t.Fatalf("ForDisplay() = %q", got)
+	}
+	if strings.Contains(got, "/Users/noahkreiger") {
+		t.Fatalf("ForDisplay leaked local path: %q", got)
+	}
+}
+
+func TestClassify(t *testing.T) {
+	cases := map[string]string{
+		"go test ./...":     "test",
+		"gitleaks detect":   "scan",
+		"golangci-lint run": "lint",
+		"go build ./...":    "build",
+		"ls -la":            "other",
+	}
+	for in, want := range cases {
+		if got := Classify(in); got != want {
+			t.Errorf("Classify(%q) = %q, want %q", in, got, want)
+		}
+	}
+}

internal/fold/fold.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/provasign/shale/internal/capture"
+	"github.com/provasign/shale/internal/commandfmt"
 	"github.com/provasign/shale/internal/gitx"
 	"github.com/provasign/shale/internal/pricing"
 	"github.com/provasign/shale/internal/redact"
@@ -242,11 +243,15 @@ func foldSession(opts Options, sessionID string) ([]string, error) {
 			}
 
 		case store.KindCommand:
-			cmd, r := eng.Apply(ev.Cmd)
+			rawCmd := commandfmt.ForEvidence(opts.RepoRoot, ev.Cmd)
+			if rawCmd == "" {
+				continue
+			}
+			cmd, r := eng.Apply(rawCmd)
 			redactions += r
 			s.Commands = append(s.Commands, store.Command{
 				Cmd: cmd, ExitCode: ev.ExitCode, At: ev.At.UTC(),
-				Classified: ClassifyCommand(ev.Cmd),
+				Classified: ClassifyCommand(rawCmd),
 			})
 
 		case store.KindPrompt:
@@ -416,26 +421,5 @@ func contains(xs []string, x string) bool {
 // ClassifyCommand buckets an agent-invoked command for the "checks recorded"
 // card section. Heuristic by design; "other" is an honest answer.
 func ClassifyCommand(cmd string) string {
-	c := strings.ToLower(cmd)
-	switch {
-	case containsAny(c, "gitleaks", "semgrep", "trivy", "snyk", "govulncheck", "bandit", "grype"):
-		return "scan"
-	case containsAny(c, "golangci-lint", "eslint", "ruff", "flake8", "pylint", "rubocop", "clippy", "go vet", "staticcheck", "tflint"):
-		return "lint"
-	case containsAny(c, "go test", "pytest", "npm test", "yarn test", "pnpm test", "jest", "vitest", "cargo test", "rspec", "mvn test", "gradle test", "make test"):
-		return "test"
-	case containsAny(c, "go build", "npm run build", "yarn build", "cargo build", "make build", "mvn package", "gradle build", "docker build", "tsc"):
-		return "build"
-	default:
-		return "other"
-	}
-}
-
-func containsAny(s string, subs ...string) bool {
-	for _, sub := range subs {
-		if strings.Contains(s, sub) {
-			return true
-		}
-	}
-	return false
+	return commandfmt.Classify(cmd)
 }

internal/fold/fold_test.go

@@ -228,6 +228,50 @@ func TestRunGitFallbackFillsMissingHookPaths(t *testing.T) {
 	}
 }
 
+func TestRunCleansCommandEvidenceBeforePersistence(t *testing.T) {
+	root := initRepo(t)
+	rawCmd := `cat > pkg/query_test.go <<'EOF'
+package pkg
+
+const token = "ghp_aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789"
+EOF
+cd ` + root + `; go test ./pkg/criteria -run TestTranslator_NumericPropertyComparison -v 2>&1 | grep -E "PASS|FAIL" | head -25`
+	for _, ev := range []store.Event{
+		{Kind: store.KindIntent, At: t0, Title: "Fix query parsing"},
+		{Kind: store.KindCommand, At: t0.Add(time.Minute), Cmd: rawCmd, ExitCode: intPtr(0)},
+		{Kind: store.KindCompletion, At: t0.Add(2 * time.Minute), Note: "fixed"},
+	} {
+		if err := store.AppendEvent(root, "SESSCMD01", ev); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if _, err := Run(Options{RepoRoot: root, Now: now}); err != nil {
+		t.Fatal(err)
+	}
+	s, err := store.Load(filepath.Join(root, ".shale", "SESSCMD01.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(s.Commands) != 1 {
+		t.Fatalf("commands = %+v", s.Commands)
+	}
+	got := s.Commands[0].Cmd
+	want := "go test ./pkg/criteria -run TestTranslator_NumericPropertyComparison -v 2>&1"
+	if got != want || s.Commands[0].Classified != "test" {
+		t.Fatalf("command = %#v, want %q classified as test", s.Commands[0], want)
+	}
+	rawYAML, err := os.ReadFile(filepath.Join(root, ".shale", "SESSCMD01.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, banned := range []string{"ghp_", "package pkg", root, "/Users/", "grep -E", "head -25"} {
+		if strings.Contains(string(rawYAML), banned) {
+			t.Fatalf("committed command evidence leaked %q:\n%s", banned, rawYAML)
+		}
+	}
+}
+
 // Regression: a session with two intent→done arcs must never pair one task's
 // completion with another task's intent (found live: session d38090f4 in the
 // prism repo published the second intent with the first completion note).

internal/gitx/gitx.go

@@ -48,9 +48,11 @@ type FileChange struct {
 }
 
 // FilesChangedSince returns repo-relative paths changed in the window from t
-// to now: commits authored since t on the current branch plus any
-// uncommitted (staged or unstaged) changes. This feeds the `via: git`
-// fallback (ADR D4 tier 3) — honest about being a window, not a touch log.
+// to now: non-merge commits authored on the current branch since t plus any
+// uncommitted (staged or unstaged) changes. It deliberately excludes commits
+// reachable from the default branch/FETCH_HEAD so a branch sync from main
+// does not become this session's evidence. This feeds the `via: git` fallback
+// (ADR D4 tier 3) — honest about being a window, not a touch log.
 func FilesChangedSince(root string, t time.Time) []FileChange {
 	ops := map[string][]string{}
 	add := func(path, op string) {
@@ -66,9 +68,15 @@ func FilesChangedSince(root string, t time.Time) []FileChange {
 		ops[path] = append(ops[path], op)
 	}
 
-	logOut, err := run(root,
-		"log", "--since="+t.UTC().Format(time.RFC3339),
-		"--name-status", "--pretty=format:", "HEAD")
+	args := []string{
+		"log", "--first-parent", "--no-merges",
+		"--since=" + t.UTC().Format(time.RFC3339),
+		"--name-status", "--pretty=format:", "HEAD",
+	}
+	for _, ref := range syncRefs(root) {
+		args = append(args, "^"+ref)
+	}
+	logOut, err := run(root, args...)
 	if err == nil {
 		for _, line := range strings.Split(logOut, "\n") {
 			fields := strings.Split(line, "\t")
@@ -113,6 +121,32 @@ func FilesChangedSince(root string, t time.Time) []FileChange {
 	return changes
 }
 
+func syncRefs(root string) []string {
+	current := Branch(root)
+	candidates := []string{"FETCH_HEAD"}
+	if current != "main" && current != "master" {
+		candidates = append(candidates,
+			"refs/remotes/origin/HEAD",
+			"refs/remotes/origin/main",
+			"refs/remotes/origin/master",
+			"refs/heads/main",
+			"refs/heads/master",
+		)
+	}
+
+	var refs []string
+	seen := map[string]bool{}
+	for _, ref := range candidates {
+		oid, err := run(root, "rev-parse", "--verify", "--quiet", ref+"^{commit}")
+		if err != nil || oid == "" || seen[oid] {
+			continue
+		}
+		seen[oid] = true
+		refs = append(refs, ref)
+	}
+	return refs
+}
+
 // opForStatus maps one git status letter (diff --name-status or porcelain XY)
 // to the spec ops vocabulary. Letters that carry no change ("space", ignored)
 // map to "".