Check that prose names identifiers that exist

Makefile

@@ -12,7 +12,7 @@ PREFIX ?= $(HOME)/.local/bin
 # none of which should end up anywhere a backup would find them.
 DEMO_DIR ?= /tmp/deck-demo
 
-.PHONY: help build run test race vet fmt tidy install uninstall reinstall watch clean demo
+.PHONY: help build run test race vet fmt tidy hooks install uninstall reinstall watch clean demo
 
 help: ## list the targets
 	@grep -E '^[a-z-]+:.*?## ' $(MAKEFILE_LIST) \
@@ -39,6 +39,17 @@ fmt: ## gofmt the tree
 tidy: ## resolve deps + write go.sum
 	go mod tidy
 
+hooks: ## install scripts/commit-msg where git will actually look for it
+	@dir=$$(git rev-parse --git-path hooks); \
+	common=$$(git rev-parse --git-common-dir)/hooks; \
+	mkdir -p "$$dir"; \
+	ln -sf "$$(pwd)/scripts/commit-msg" "$$dir/commit-msg"; \
+	echo "installed: $$dir/commit-msg"; \
+	if [ "$$dir" != "$$common" ]; then \
+	  echo "note: core.hooksPath is shared, so this now runs for every repository."; \
+	  echo "It exits immediately in one with no Go files."; \
+	fi
+
 install: ## build and copy to $(PREFIX), creating it if needed
 	@mkdir -p $(PREFIX)
 	go build -o $(PREFIX)/$(APP) .

internal/coord/jobs_test.go

@@ -81,7 +81,17 @@ func TestAnalyseRefusesTheSameThingsWorkDoes(t *testing.T) {
 // synchronous answer would outlast the calling agent's tool timeout, which
 // belongs to that agent rather than to us.
 func TestAnalyseReturnsBeforeItFinishes(t *testing.T) {
-	c := analysable(t, agent.ClaudeRun{}, nil)
+	// The reviewer is held open until the assertions are done. A stub that
+	// returns immediately lets the run reach JobDone before State is read, so
+	// the test failed on whichever machine happened to schedule it that way —
+	// under -race on a loaded runner, often enough to be noticed. Holding the
+	// run open is what makes "still running" a fact rather than a gamble.
+	release := make(chan struct{})
+	defer close(release)
+	c := analysableWith(t, func(_ context.Context, _, _ string) (agent.ClaudeRun, error) {
+		<-release
+		return agent.ClaudeRun{}, nil
+	})
 
 	start := time.Now()
 	job, err := c.Analyse("me", "wily-crane-bbbb", "")

internal/gittest/gittest.go

@@ -31,6 +31,14 @@ func Repo(t *testing.T) string {
 	Run(t, dir, "init", "-q", "-b", "main")
 	Run(t, dir, "config", "user.name", "test")
 	Run(t, dir, "config", "user.email", "test@example.test")
+	// Hooks are isolated for the same reason the identity is: a fixture must
+	// not inherit the machine. A developer's global core.hooksPath applies to
+	// every repository including these throwaway ones, and a hook that refuses
+	// a commit leaves the fixture half-built — the file staged but unrecorded,
+	// which a later checkout then carries onto the wrong branch. Pointing at a
+	// path with no hooks in it is what makes the suite say the same thing here
+	// as it does in CI.
+	Run(t, dir, "config", "core.hooksPath", "/dev/null")
 	return dir
 }
 

internal/naming/naming_test.go

@@ -32,9 +32,16 @@ func TestSessionIsUnique(t *testing.T) {
 		}
 		seen[n] = true
 	}
-	// 50 adjectives x 50 animals x 32^4 suffixes is a large space; a couple of
-	// birthday collisions in 2000 draws would still be suspicious.
-	if collisions > 0 {
+	// 50 adjectives x 50 animals x 32^4 suffixes is 2.6 billion names, so the
+	// birthday chance of one collision in 2000 draws is about 1 in 1300. This
+	// asserted zero and therefore failed roughly that often, which teaches a
+	// reader to re-run a red suite rather than read it.
+	//
+	// One is tolerated, two is not: the chance of a second is about 1 in 3
+	// million, while a generator that had lost its suffix would draw from 2500
+	// names and collide about 620 times here. The threshold still separates
+	// those two cases by a wide margin.
+	if collisions > 1 {
 		t.Errorf("%d collisions in %d draws", collisions, draws)
 	}
 }

prose_test.go

@@ -0,0 +1,246 @@
+package main
+
+// Prose is the one place in this repository with no compiler. An identifier
+// spelled wrong in code fails the build; the same mistake in a doc comment, a
+// README table or an architecture note passes every gate and is read as fact by
+// the next person. This test closes that gap for the half of it a machine can
+// judge: whether a name written in prose exists at all.
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"testing"
+)
+
+// camelCase is the shape that distinguishes a Go identifier from an English
+// word without needing a dictionary. sessionLine matches; "focus" and "Session"
+// do not, and neither does ordinary prose — which is what keeps the check quiet
+// enough to be worth having. Single-word identifiers are out of reach for the
+// same reason, and that is the accepted cost.
+//
+// Every capital must be followed by lowercase. That is what separates a Go name
+// from an acronym: this repository writes several of those often, and a check
+// that flagged them would be turned off within a week.
+var camelCase = regexp.MustCompile(`\b[a-z][a-z0-9]*(?:[A-Z][a-z0-9]+)+\b`)
+
+// backticked pulls the code spans out of markdown. A name outside them is
+// English, and a name inside one is a claim about this tree.
+var backticked = regexp.MustCompile("`([^`]+)`")
+
+// miss is one name, in one file, that resolves to nothing.
+type miss struct{ file, name string }
+
+// scanProse reports every name written in prose under root that is not an
+// identifier somewhere in root's Go sources.
+//
+// It reads names out of the syntax tree rather than out of the file text. Text
+// would include the comments themselves, so a misspelling written twice would
+// vouch for itself and the check would pass on exactly the case it exists to
+// catch.
+//
+// Taking a root rather than walking the working directory is what lets the
+// planted-misspelling test run the real scanner over a fixture, instead of the
+// discrimination being something a human checked once and wrote down.
+func scanProse(t *testing.T, root string) []miss {
+	t.Helper()
+
+	var goFiles, mdFiles []string
+	err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() && (d.Name() == ".git" || d.Name() == "assets") {
+			return filepath.SkipDir
+		}
+		switch filepath.Ext(path) {
+		case ".go":
+			goFiles = append(goFiles, path)
+		case ".md":
+			mdFiles = append(mdFiles, path)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("walk %s: %v", root, err)
+	}
+	// Without this the whole check passes by finding nothing — a walk that
+	// starts in the wrong place, or a skip rule that grows too broad, would
+	// turn into a green test rather than a red one.
+	if len(goFiles) == 0 {
+		t.Fatalf("no Go sources under %s; the walk is wrong, not the tree", root)
+	}
+
+	fset := token.NewFileSet()
+	inCode := map[string]bool{}
+	comments := map[string][]string{}          // file -> comment text
+	inLiterals := map[string]map[string]bool{} // file -> names inside its own string literals
+
+	for _, path := range goFiles {
+		f, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
+		if err != nil {
+			t.Fatalf("parse %s: %v", path, err)
+		}
+		inLiterals[path] = map[string]bool{}
+		ast.Inspect(f, func(n ast.Node) bool {
+			switch v := n.(type) {
+			case *ast.Ident:
+				inCode[v.Name] = true
+			case *ast.BasicLit:
+				// A name a file only ever writes into a string is still a name
+				// that file's comments may explain: the SVG attributes ansisvg
+				// emits are not Go symbols, and its comments discuss them by
+				// name. Scoped to the file, so one literal cannot vouch for
+				// prose anywhere else in the tree.
+				for _, name := range camelCase.FindAllString(v.Value, -1) {
+					inLiterals[path][name] = true
+				}
+			}
+			return true
+		})
+		for _, g := range f.Comments {
+			comments[path] = append(comments[path], g.Text())
+		}
+	}
+
+	// A name that prefixes a real identifier is naming a family rather than a
+	// symbol — "workingCopy indices" heads workingCopyWorktree and
+	// workingCopyDir. Accepting it keeps the check honest about what it can
+	// know, and costs little: a misspelling is not a prefix of what it meant.
+	prefixes := func(name string) bool {
+		for known := range inCode {
+			if len(known) > len(name) && strings.HasPrefix(known, name) {
+				return true
+			}
+		}
+		return false
+	}
+
+	// A name is reported once per file that spells it, so a fix lands in one
+	// place rather than being chased through a list of repeats.
+	var missing []miss
+	seen := map[miss]bool{}
+	note := func(file, name string) {
+		m := miss{file, name}
+		if inCode[name] || inLiterals[file][name] || seen[m] || prefixes(name) {
+			return
+		}
+		seen[m] = true
+		missing = append(missing, m)
+	}
+
+	for path, texts := range comments {
+		for _, text := range texts {
+			for _, name := range camelCase.FindAllString(text, -1) {
+				note(path, name)
+			}
+		}
+	}
+
+	for _, path := range mdFiles {
+		body, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatalf("read %s: %v", path, err)
+		}
+		for _, span := range backticked.FindAllStringSubmatch(string(body), -1) {
+			// A qualified name arrives as ui.coordArgs or gitx.Diff; the
+			// package half is not declared in this module's own trees, so only
+			// the selector is checked.
+			for _, name := range camelCase.FindAllString(span[1], -1) {
+				note(path, name)
+			}
+		}
+	}
+
+	sort.Slice(missing, func(i, j int) bool {
+		if missing[i].file != missing[j].file {
+			return missing[i].file < missing[j].file
+		}
+		return missing[i].name < missing[j].name
+	})
+	return missing
+}
+
+// TestProseNamesIdentifiersThatExist is the regression for a class of defect
+// this repository kept producing: a comment or a document naming a function
+// that is not there.
+//
+// What it cannot judge is whether a claim about a real symbol is true —
+// "sessionLine draws no marker" names something that exists and was wrong
+// anyway. That half stays a reading job.
+//
+// It reads every markdown file on disk, not only the tracked ones, so an
+// uncommitted note is checked here and not in CI. That is deliberate: working
+// instructions go stale the same way published ones do, and a local-only
+// failure is still a true one.
+func TestProseNamesIdentifiersThatExist(t *testing.T) {
+	for _, m := range scanProse(t, ".") {
+		t.Errorf("%s names %q, which is not an identifier anywhere in this module",
+			m.file, m.name)
+	}
+}
+
+// TestProseScanReportsAPlantedMisspelling proves the scanner discriminates.
+//
+// Without it the test above is a green light of unknown value: it passes on a
+// clean tree whether or not it can see anything. This runs the real scanner
+// over a fixture whose comment names a function that does not exist.
+//
+// The fixture is written to a temporary directory rather than committed. A
+// committed one would put the misspelling into this module's own sources, where
+// it would resolve — and the commit-msg hook, which greps the tree rather than
+// parsing it, would then treat the typo as a real symbol forever.
+func TestProseScanReportsAPlantedMisspelling(t *testing.T) {
+	dir := t.TempDir()
+	const sample = `package sample
+
+// jumpToVerandah is named here and declared nowhere.
+func Real() {}
+`
+	// A markdown fixture too, because the two halves read different things —
+	// one parses comments out of a syntax tree, the other pulls code spans out
+	// of text. A single fixture would leave whichever half broke still green.
+	const notes = "# Sample\n\nThe `openTheVerandah` helper does the thing.\n"
+
+	if err := os.WriteFile(filepath.Join(dir, "sample.go"), []byte(sample), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "notes.md"), []byte(notes), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	var got []string
+	for _, m := range scanProse(t, dir) {
+		got = append(got, m.name)
+	}
+	sort.Strings(got)
+	if want := "jumpToVerandah,openTheVerandah"; strings.Join(got, ",") != want {
+		t.Fatalf("scan reported %v, want both planted names", got)
+	}
+}
+
+// TestCamelCaseMatchesNamesNotAcronyms covers the matcher every other check is
+// built on, including the rule that made it usable. An acronym is not a Go
+// name, and a check that reported one on every run is a check somebody deletes.
+func TestCamelCaseMatchesNamesNotAcronyms(t *testing.T) {
+	for _, tc := range []struct {
+		in   string
+		want []string
+	}{
+		{"jumpToSession moves the cursor", []string{"jumpToSession"}},
+		{"sessionLine and cursorMarker", []string{"sessionLine", "cursorMarker"}},
+		{"resolved on macOS only", nil},
+		{"the sRGB ramp", nil},
+		{"focus and Session on their own", nil},
+	} {
+		got := camelCase.FindAllString(tc.in, -1)
+		if strings.Join(got, ",") != strings.Join(tc.want, ",") {
+			t.Errorf("on %q: got %v, want %v", tc.in, got, tc.want)
+		}
+	}
+}

scripts/commit-msg

@@ -0,0 +1,41 @@
+#!/bin/sh
+# Refuse a commit message that names a Go identifier this tree does not have.
+#
+# TestProseNamesIdentifiersThatExist does this for comments and documents, and
+# cannot do it here: go test never sees a commit message. The rule is the same
+# one — a name written in prose is a claim, and an unchecked claim is read as
+# fact by whoever runs git log next.
+#
+# Install with `make hooks`. Warns rather than blocks, because a message may
+# legitimately name a symbol the commit deletes.
+
+msg="$1"
+root=$(git rev-parse --show-toplevel)
+
+# A repository with no Go in it has no identifiers to check, and every camelCase
+# word in its messages would be reported. Leaving quietly here is what makes the
+# hook safe to install in a core.hooksPath shared by every repository you touch.
+[ -n "$(git -C "$root" ls-files '*.go' | head -n 1)" ] || exit 0
+
+# The same shape the Go test matches: lowercase start, every capital followed by
+# lowercase, so macOS and sRGB are left alone.
+names=$(sed 's/^#.*//' "$msg" |
+	grep -oE '\b[a-z][a-z0-9]*([A-Z][a-z0-9]+)+\b' | sort -u)
+
+missing=""
+for name in $names; do
+	# Look in the tracked Go sources only. A name that appears nowhere in them
+	# is either misspelled or describes code that is no longer here.
+	if ! git -C "$root" grep -qw --cached -- "$name" -- '*.go' 2>/dev/null &&
+		! git -C "$root" grep -qw -- "$name" -- '*.go' 2>/dev/null; then
+		missing="$missing $name"
+	fi
+done
+
+if [ -n "$missing" ]; then
+	echo "commit-msg: these names are in the message but not in any .go file:" >&2
+	for name in $missing; do echo "  $name" >&2; done
+	echo "Check the spelling, or say it in words if it is not a symbol." >&2
+fi
+
+exit 0