diff --git a/README.md b/README.md index 13f1790..37e4e65 100644 --- a/README.md +++ b/README.md @@ -154,9 +154,10 @@ Four gates a repository runs over itself from `task lint`, each failing with wha - `go tool testquality [-allow file] ./...` fails on a test that cannot fail - nothing in it reaches `t.Error`, `t.Fatal`, `t.Skip`, an assertion package, or a helper handed the test - and on one skipped unconditionally. The allowlist is a ratchet: an entry nothing matches fails too. -- `go tool refs [-files 'AGENTS.md ...'] [-allow file]` fails on a repository path or a `task - ` that the prose an agent loads (AGENTS.md and CLAUDE.md, by default) names and the tree - does not have; `-allow` lists the ones named on purpose, ` # why`. +- `go tool refs [-files 'AGENTS.md ...'] [-allow file] [-decisions docs/decisions]` fails on a + repository path, a `task `, an `ADR-NNNN` or a `pkg.Symbol` of a missing `internal/pkg` + that the prose an agent loads (AGENTS.md and CLAUDE.md, by default) names and the tree does + not have; `-allow` lists the ones named on purpose, ` # why`. - `go tool vulngate [govulncheck flags, each -flag=value] ./...` runs govulncheck and fails on a vulnerability the code calls and that has a fix, the standard library's included: the answer is the upgrade, and for the standard library that is go.mod's `toolchain` line. One only imported, diff --git a/cmd/refs/main.go b/cmd/refs/main.go index a6ef041..0561c35 100644 --- a/cmd/refs/main.go +++ b/cmd/refs/main.go @@ -1,7 +1,8 @@ -// Command refs holds a repository's prose to its tree: every repository path and every `task -// ` the files -files names point at must resolve, or be in the allowlist (-allow) with a -// reason. The files are the AGENTS.md and CLAUDE.md an agent loads into every session, by -// default; spin adds its documents. +// Command refs holds a repository's prose to its tree: every repository path, `task `, +// ADR id (under -decisions) and symbol of a missing internal package the files -files names +// point at must resolve, or be in the allowlist (-allow) with a reason. The files are the +// AGENTS.md and CLAUDE.md an agent loads into every session, by default; spin adds its +// documents and storage's code. package main import ( @@ -25,9 +26,10 @@ func main() { pathspecs := flag.String("files", "AGENTS.md CLAUDE.md */AGENTS.md */CLAUDE.md", "the prose to read, as git pathspecs separated by spaces: a tracked file one names is read") taskExe := flag.String("task", taskExeFromEnv(), "the task binary to ask for names") + decisions := flag.String("decisions", "docs/decisions", "the directory an ADR-NNNN resolves in") flag.Parse() - if err := run(".", *allowPath, strings.Fields(*pathspecs), *taskExe); err != nil { + if err := run(".", *allowPath, strings.Fields(*pathspecs), *taskExe, *decisions); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } @@ -40,7 +42,7 @@ func taskExeFromEnv() string { return "task" } -func run(root, allowPath string, pathspecs []string, taskExe string) error { +func run(root, allowPath string, pathspecs []string, taskExe, decisions string) error { allowed, err := allow.Read(allowPath) if err != nil { return fmt.Errorf("refs: %w", err) @@ -65,10 +67,11 @@ func run(root, allowPath string, pathspecs []string, taskExe string) error { } found, err := refs.Check(refs.Options{ - Root: root, - Files: files, - TopDirs: tops, - Tasks: taskLister(root, taskExe), + Root: root, + Files: files, + TopDirs: tops, + Tasks: taskLister(root, taskExe), + Decisions: decisions, }) if err != nil { return err @@ -99,7 +102,7 @@ func run(root, allowPath string, pathspecs []string, taskExe string) error { if len(stale) > 0 || len(unexplained) > 0 { return errors.New("refs: the prose and the tree disagree") } - fmt.Println("OK: every path and task this repository's prose names resolves or is explained.") + fmt.Println("OK: every reference this repository's prose makes resolves or is explained.") return nil } diff --git a/internal/refs/refs.go b/internal/refs/refs.go index 37e0d51..b6ff862 100644 --- a/internal/refs/refs.go +++ b/internal/refs/refs.go @@ -1,5 +1,6 @@ // Package refs decides whether the prose this repository loads into every session still -// points at things that exist: a repository path, and a `task `. +// points at things that exist: a repository path, a `task `, a decision record's ADR id, +// and a `pkg.Symbol` of a package that is gone. // // It was 141 lines of shell, and the shell is why it is here. That script parsed `task // --list-all --json` with a sed expression; when the expression matched nothing it reported @@ -31,7 +32,7 @@ import ( type Finding struct { // File is the prose that makes the reference, relative to the repository root. File string - // Ref is the reference as written: a path, or "task ". + // Ref is the reference as written: a path, "task ", an ADR id, or pkg.Symbol. Ref string } @@ -63,6 +64,9 @@ type Options struct { TopDirs []string // Tasks lists the task names of a directory, relative to Root. Tasks TaskLister + // Decisions is the directory, relative to Root, where `ADR-NNNN` resolves to a file whose + // name starts with it. + Decisions string } // extensions closes the last segment of a path reference. A segment carries no dot otherwise, @@ -95,18 +99,30 @@ func Check(o Options) ([]Finding, error) { } var found []Finding - for _, file := range o.Files { + texts := make([]string, len(o.Files)) + gone := map[string]bool{} + for i, file := range o.Files { body, err := os.ReadFile(filepath.Join(o.Root, file)) if err != nil { return nil, fmt.Errorf("refs: reading %s: %w", file, err) } text := string(body) + texts[i] = text for _, ref := range pathRE.matches(text) { if resolves(o.Root, ref) { continue } found = append(found, Finding{File: file, Ref: ref}) + if pkg, ok := strings.CutPrefix(ref, "internal/"); ok && goPackage.MatchString(pkg) { + gone[pkg] = true + } + } + + for _, id := range adrRE.matches(text) { + if !globResolves(o.Root, path.Join(o.Decisions, id)+"*") { + found = append(found, Finding{File: file, Ref: id}) + } } // A component's file legitimately says "generate is a root task", so its own @@ -123,6 +139,17 @@ func Check(o Options) ([]Finding, error) { } } + // A symbol of a package that is gone, wherever its path was found missing. Scoped to those + // packages because every `x.Y` would drown in json.Unmarshal and t.Fatalf. + if len(gone) > 0 { + symbolRE := symbolPattern(gone) + for i, file := range o.Files { + for _, ref := range symbolRE.matches(texts[i]) { + found = append(found, Finding{File: file, Ref: ref}) + } + } + } + sort.Slice(found, func(i, j int) bool { return found[i].Key() < found[j].Key() }) return dedupe(found), nil } @@ -180,6 +207,19 @@ func pathPattern(tops []string) (matcher, error) { // not a name, so `task --list` yields nothing. var taskRE = matcher{re: regexp.MustCompile("`task ([a-z][A-Za-z0-9:_-]*)"), group: 1} +var adrRE = matcher{re: regexp.MustCompile(`\b(ADR-[0-9]{4})\b`), group: 1} + +// goPackage is the one segment after internal/ that names a package a symbol can be cited from. +var goPackage = regexp.MustCompile(`^[a-z0-9_]+$`) + +func symbolPattern(pkgs map[string]bool) matcher { + names := make([]string, 0, len(pkgs)) + for p := range pkgs { + names = append(names, p) + } + return matcher{re: regexp.MustCompile(`\b((?:` + strings.Join(names, "|") + `)\.[A-Z][A-Za-z0-9_]*)`), group: 1} +} + // resolves is whether the reference names something in the tree. A path git ignores is not in // it even when it is on disk: it is what a build or a run left behind - the dashboard's copy in // cmd/controlplane/web, for one - and reading it as present made the answer depend on whether diff --git a/internal/refs/refs_test.go b/internal/refs/refs_test.go index 8a119ed..6c81713 100644 --- a/internal/refs/refs_test.go +++ b/internal/refs/refs_test.go @@ -252,3 +252,44 @@ func TestABuildOutputIsNotInTheTreeWhetherOrNotItWasBuilt(t *testing.T) { } wants(t, check(t, root, onlyRoot("build")), "cmd/cli/web") } + +// An ADR id resolves to a decision record, and a symbol is a finding only of a package whose +// path is itself missing: matching every `x.Y` would report json.Unmarshal. +func TestADecisionAndASymbolOfAMissingPackage(t *testing.T) { + for _, tc := range []struct { + name, prose string + want []string + }{ + {"a recorded decision", "As ADR-0001 says.\n", nil}, + {"a decision never recorded", "As ADR-0001 and ADR-0002 say.\n", []string{"ADR-0002"}}, + {"a symbol of a missing package", "`internal/wal` is gone, and so is wal.StrictOrder.\n", + []string{"internal/wal", "wal.StrictOrder"}}, + {"a symbol whose package is in the tree", "`internal/db/queries` and queries.Fleet, json.Unmarshal.\n", nil}, + {"a symbol with no missing path beside it", "wal.StrictOrder alone.\n", nil}, + {"another package's symbol beside a missing one", "`internal/wal` is gone, json.Unmarshal is not.\n", + []string{"internal/wal"}}, + {"a nested package is its path alone", "`internal/db/sweep.Run`.\n", []string{"internal/db/sweep"}}, + {"a lowercase name is not a symbol", "`internal/wal` and wal.strict.\n", []string{"internal/wal"}}, + } { + t.Run(tc.name, func(t *testing.T) { + root := tree(t, tc.prose) + if err := os.MkdirAll(filepath.Join(root, "docs/decisions"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "docs/decisions/ADR-0001-one.md"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + found, err := refs.Check(refs.Options{ + Root: root, + Files: []string{"CLAUDE.md"}, + TopDirs: []string{"internal", "docs", "cmd"}, + Tasks: onlyRoot("build"), + Decisions: "docs/decisions", + }) + if err != nil { + t.Fatalf("check: %v", err) + } + wants(t, found, tc.want...) + }) + } +}