chore(tools): add commentlint and hold the tree to its two rules

.github/workflows/ci.yaml

@@ -32,6 +32,8 @@ jobs:
           # action fetch a remote version mapping that fails the job on any
           # network blip. Keep in sync with nixpkgs' golangci-lint in flake.nix.
           version: v2.12.2
+      - name: Comments
+        run: go run ./tools/commentlint .
 
   test:
     name: Test (${{ matrix.os }})

AGENTS.md

@@ -6,7 +6,7 @@ This checkout is the tool's own source, not a fleet home itself - there is no `s
 
 ## Rules
 
-- Zero comments by default. Only add one when the WHY is non-obvious: a hidden constraint, a subtle invariant, a workaround for a specific bug. Never restate code, narrate what, add banners, or docstring the obvious.
+- Comments obey two rules `make lint` enforces through `tools/commentlint`: a comment may not open with the identifier it documents, and a comment block may not exceed three lines. CONTRIBUTING.md's "Comments" section owns the bar for writing one at all, the exemptions, and the reasoning.
 - Command output goes through `internal/axi` as TOON and every failure through `cmd/root.go`'s error document; `hand watch`'s event stream is the one exception, and SPECS.md's "Output shape" section owns the contract.
 - Harness/herdr syntax, exit-code enforcement, watch's stdout/errOut split, and first-run prompt handling are commented at point of use (`internal/herdr`, `internal/harness`, `cmd/root.go`, `cmd/precondition.go`, `internal/watcher`, `cmd/teardown.go`, `cmd/prdetect.go`, `cmd/merge.go`, `cmd/launch.go`); SPECS.md's "Exit codes" and each command's spec section own the authoritative tables.
 - `herdr`, `treehouse` and `gh` are faked once, in `internal/faketool`, for every suite; a test declares the fleet it wants rather than writing sh. `internal/faketool/FIDELITY.md` records what the real tools do and `tests/contract` (`make contract`) rechecks that record against them. Extend the shared fake, never hand-write another; SPECS.md's "Testing strategy" owns the rule behind it.

CONTRIBUTING.md

@@ -25,6 +25,29 @@ Without Nix, install those yourself.
 Commits use conventional commits: feat:, fix:, chore:, etc.
 release-please handles versioning and changelogs from these.
 
+## Comments
+
+The default is no comment.
+Add one only for a why the code cannot show: a hidden constraint, a subtle invariant, a workaround for a specific bug.
+Restating code, narrating what, banners, and doc comments on the obvious are noise no linter can catch, so they stay a reviewer's call.
+
+Two rules bound the comments that clear that bar, enforced by `make lint` rather than by a reviewer reading a diff:
+
+1. A comment may not open with the identifier it documents.
+2. A comment block may not exceed three lines.
+
+Consecutive `//` lines are one block, and neither a bare `//` line inside a run nor a blank line above a doc comment breaks it.
+Both are ways of writing six lines of prose in front of one declaration while satisfying a three-line rule, and the blank-line form also drops the first half out of godoc.
+Rule 1 applies wherever Go's doc convention does not: unexported declarations, everything in `_test.go`, and comments inside function bodies.
+An exported declaration's doc comment is required by convention to open with its name, so it is exempt from rule 1, but not from rule 2.
+Exempt from both rules: the package doc comment, directives (`//go:build`, `//go:generate`, `//nolint`, `// #nosec`), and files carrying the generated-code header.
+
+Rule 2 will occasionally be wrong, because a genuinely subtle invariant sometimes needs a fourth line.
+That is accepted: a rule that is right most of the time and mechanically enforced binds harder than one that is right always and enforced never.
+Prose that outgrows three lines belongs in SPECS.md, which is where it is read.
+
+`go run ./tools/commentlint .` runs the check alone and prints one `file:line:column` per violation.
+
 ## Reporting issues
 
 Open a GitHub issue with repro steps, OS, arch, and hand --version.

Makefile

@@ -16,6 +16,7 @@ lint:
 	@output=$$(gofmt -l .); if [ -n "$$output" ]; then echo "Files not formatted:"; echo "$$output"; exit 1; fi
 	go vet ./...
 	golangci-lint run
+	go run ./tools/commentlint .
 
 e2e:
 	go test -tags=e2e -timeout=10m ./tests/e2e/...

SPECS.md

@@ -178,6 +178,9 @@ secondhand/                 # maintainer's in-repo fleet home = repo checkout
       axi.go                # fields, row blocks, --fields selection, truncation hints, help[] lines
     sessionhook/            # ambient context for a supervising session (see "Ambient context")
       sessionhook.go        # install, repoint and report the SessionStart hook entry
+  tools/
+    commentlint/            # the comment check `make lint` and CI run (see "Repo scaffolding")
+      main.go               # walk the tree, report one file:line:column per violation
   go.mod
   go.sum
   AGENTS.md                 # agent instructions (~25 lines of rules)
@@ -2496,6 +2499,7 @@ An existing fleet home has live state on disk, and the import has to meet it wit
   A value the invocation did not supply is not a usage error: the same malformed value read from a `config/` default is a general error (code `1`).
 - `3`: precondition failed, meaning the command refuses because the world is not in the state it requires: unlanded work, red CI, a missing or unmerged PR, a missing brief or report, a task, project or hold that does not exist, an id carrying an open hold (`hand spawn`), a task in the wrong kind or state (already merged, not a completed scout, already claimed by another command), a project name or worktree already taken, a project still referenced by active tasks, a PR that conflicts with one already recorded for a task or belongs to neither the task's project's repo nor its declared upstream (`hand pr`), a PR that `gh pr view` can't confirm exists (`hand pr`), a task branch whose PRs do not resolve to a single usable winner (`hand teardown`), a `no-mistakes`-mode project whose gate is not initialized (`hand spawn`, `hand promote` - see "Gate preflight"), a fleet home that already has a watcher attached (`hand watch`, remedied by `--takeover` - see "One watcher per fleet home").
   Two more apply to every command, since each one resolves a fleet home before it does anything: the working directory has no fleet home at or above it and `HAND_HOME` is unset, or `HAND_HOME` is set to a directory that is not a fleet home. The second refuses rather than falling back to the walk up, because a silent fallback is how an operator dispatches into the wrong fleet.
+  These are signalled to `cmd` as sentinel errors (`cmd/precondition.go`), each carrying only the trailing phrase and wrapped by its caller as `<noun> "<name>" <phrase>`, so one condition renders as one string wherever it surfaces.
 - `4`: no event delivered, only from `hand watch --until-event`: its `--timeout` elapsed, or it was signaled, without a transition. This includes the timeout elapsing anywhere in arming, the herdr reachability probe as well as the per-task probe sweep - the window is over either way, and no one task is at fault. Distinct from `0` because there the exit *is* the event delivery, and from `1` because the watcher itself did not fail (see "Delivering an event to a supervisory agent").
 - `5`: arm-time probe failure, only from `hand watch --until-event`: one named task's herdr pane answered its pre-wait probe with a failure, named on stderr. Distinct from `4` because a specific worker is at fault and can be acted on, and from `0` because nothing was delivered (see "Delivering an event to a supervisory agent").
 - `6`: send undelivered, only from `hand send`: the composer stayed busy for the whole `--wait` bound, so the message never reached the pane. Distinct from `1`, which for `hand send` means the send can never succeed (no such herdr pane, herdr itself erroring) - `6` means the opposite, a transient state a caller can retry, most simply with a longer `--wait`. Not `4` or `5`: those are reserved to `hand watch --until-event`.
@@ -2768,6 +2772,9 @@ Files tracked in the source repo (not generated by `hand init`):
 
 **`.golangci.yaml`:** the tracked file is authoritative - it keeps golangci-lint's default linter set and only sets `run.build-tags: [e2e]`, without which the `//go:build e2e` package in `tests/e2e` is invisible to the linter.
 
+**`tools/commentlint/`:** the tracked source is authoritative - a `go run ./tools/commentlint .` target that `make lint` and the CI workflow both invoke over the whole tree, exiting 1 with one `file:line:column` per violation.
+CONTRIBUTING.md's "Comments" section owns the two rules it checks, their exemptions, and why they are the only two that are machine-checkable.
+
 **`.gitignore`:** the tracked file is authoritative - the built binary, the `hand init` runtime directories, Go and Nix build output, worktree tooling files, and editor/OS cruft.
 
 **`flake.nix`:** the tracked file is authoritative - a `packages.default` derivation building the `hand` binary and a `devShells.default` carrying the Go toolchain.

cmd/fields.go

@@ -7,8 +7,8 @@ import (
 	"github.com/atqamz/secondhand/internal/axi"
 )
 
-// pickFields resolves --fields against cols, defaulting to def. An unknown name
-// is a usage error, not a silently narrower schema header.
+// Resolves --fields against cols, defaulting to def. An unknown name is a usage error, not a silently
+// narrower schema header.
 func pickFields[T any](cols []axi.Column[T], fields, def []string) ([]axi.Column[T], error) {
 	want := fields
 	if len(want) == 0 {
@@ -21,9 +21,8 @@ func pickFields[T any](cols []axi.Column[T], fields, def []string) ([]axi.Column
 	return out, nil
 }
 
-// rejectFieldsWithJSON keeps --fields honest: it narrows the TOON schema
-// header, and silently ignoring it next to --json would hand a caller the full
-// object it asked to narrow.
+// Keeps --fields honest: it narrows the TOON schema header, and silently ignoring it next to --json
+// would hand a caller the full object it asked to narrow.
 func rejectFieldsWithJSON(fields []string, asJSON bool) error {
 	if len(fields) > 0 && asJSON {
 		return &ExitError{Err: fmt.Errorf("--fields applies to the default TOON output, not --json"), Code: 2}

cmd/fleethome_test.go

@@ -6,19 +6,16 @@ import (
 	"testing"
 )
 
-// mkFleetDirs lays down the markers home.Resolve requires to recognize dir as
-// a fleet home, for fixtures that chdir into a bare temp directory without
-// going through hand init. It also neutralizes an ambient HAND_HOME, which
-// would otherwise outrank the fixture and point the command under test at the
-// developer's real fleet.
-//
-// Both marker sets are written, not just state/hand.db: tests that fault the
-// store by turning state/hand.db into a directory would otherwise stop being
-// homes at all, and the command would fail on home resolution before ever
-// reaching the fault under test.
+// Lays down the markers home.Resolve requires to recognize dir as a fleet home, for fixtures that chdir
+// into a bare temp directory without going through hand init.
 func mkFleetDirs(t *testing.T, dir string) {
 	t.Helper()
+	// Neutralizes an ambient HAND_HOME, which would otherwise outrank the fixture and point the command
+	// under test at the developer's real fleet.
 	t.Setenv("HAND_HOME", "")
+	// Both marker sets are written, not just state/hand.db: tests that fault the store by turning
+	// state/hand.db into a directory would otherwise stop being homes at all, and the command would fail on
+	// home resolution before ever reaching the fault under test.
 	for _, sub := range []string{"data", "state"} {
 		if err := os.MkdirAll(filepath.Join(dir, sub), 0o755); err != nil {
 			t.Fatal(err)

cmd/gatepreflight_test.go

@@ -12,34 +12,33 @@ import (
 	"github.com/atqamz/secondhand/internal/state"
 )
 
-// fakeNoMistakesPath writes a fake no-mistakes binary that answers every subcommand with the same
-// text, mirroring the real binary's observed behavior documented in internal/project.GateStatus:
-// `no-mistakes status` exits 0 whether or not the repo is initialized, so the outcome is read from
-// stdout text rather than the exit code. Returns a PATH with the fake binary's directory prepended
-// ahead of the real PATH: the script's own "cat" still needs to resolve, and the fake must win the
-// lookup over any real no-mistakes already on this machine.
+// Writes a fake no-mistakes binary that answers every subcommand with the same text, mirroring the
+// real behavior internal/project.GateStatus documents: `no-mistakes status` exits 0 whether or not the
+// repo is initialized, so the outcome is read from stdout text rather than the exit code.
 func fakeNoMistakesPath(t *testing.T, stdout string) string {
+	// Prepended ahead of the real PATH, not replacing it: the script's own "cat" still needs to resolve,
+	// and the fake must win the lookup over any real no-mistakes already on this machine.
 	return fakeNoMistakesPathExit(t, stdout, 0)
 }
 
-// fakeNoMistakesPathExit is fakeNoMistakesPath with an explicit exit code, for the invocations the
-// real binary refuses non-zero: `no-mistakes runs` exits 1 on both "repo not initialized" and "not
-// in a git repository", where `no-mistakes status` exits 0 printing the same text. GateRunPRs reads
-// the refusal from the text either way, so the fake reproduces the exit code rather than flattening
-// every refusal to 0.
+// fakeNoMistakesPath with an explicit exit code, for the invocations the real binary refuses non-zero:
+// `no-mistakes runs` exits 1 on both "repo not initialized" and "not in a git repository", where
+// `no-mistakes status` exits 0 printing the same text.
 func fakeNoMistakesPathExit(t *testing.T, stdout string, code int) string {
 	t.Helper()
 	bin := t.TempDir()
+	// GateRunPRs reads the refusal from the text either way, so the fake reproduces the exit code rather
+	// than flattening every refusal to 0.
 	script := fmt.Sprintf("#!/bin/sh\ncat <<'EOF'\n%s\nEOF\nexit %d\n", stdout, code)
 	if err := os.WriteFile(filepath.Join(bin, "no-mistakes"), []byte(script), 0o755); err != nil {
 		t.Fatal(err)
 	}
 	return bin + string(os.PathListSeparator) + os.Getenv("PATH")
 }
 
-// fakeHerdrPaneDone fakes only "pane get", answering the pane as done (not busy), enough for
-// promote's precondition check to pass through to gatePreflight without needing the rest of a
-// clean promote's herdr calls, which gatePreflight's refusal preempts.
+// Fakes only "pane get", answering the pane as done (not busy), enough for promote's precondition
+// check to pass through to gatePreflight without needing the rest of a clean promote's herdr calls,
+// which gatePreflight's refusal preempts.
 const fakeHerdrPaneDone = `#!/bin/sh
 cmd="$1 $2"
 case "$cmd" in
@@ -53,10 +52,9 @@ case "$cmd" in
 esac
 `
 
-// setupSpawnHomeGate registers a no-mistakes-mode project and nothing else: gatePreflight fires
-// in spawn before state.Claim, the brief check, or any herdr/treehouse call, so none of those need
-// to exist for these tests. noMistakesPath becomes PATH verbatim, letting each test control
-// exactly whether a fake no-mistakes binary is reachable.
+// Registers a no-mistakes-mode project and nothing else: gatePreflight fires in spawn before
+// state.Claim, the brief check, or any herdr/treehouse call, so none of those need to exist here.
+// noMistakesPath becomes PATH verbatim, letting each test control whether the fake is reachable.
 func setupSpawnHomeGate(t *testing.T, noMistakesPath string) string {
 	t.Helper()
 	home := t.TempDir()
@@ -75,12 +73,9 @@ func setupSpawnHomeGate(t *testing.T, noMistakesPath string) string {
 	return home
 }
 
-// setupPromoteHomeGate mirrors setupPromoteHome but registers a no-mistakes-mode project and
-// skips the worktree/treehouse setup: gatePreflight fires in promote after the report, pane-busy,
-// and brief checks but before worktree.Get, so those three preconditions must be satisfied while
-// nothing past gatePreflight needs to exist. noMistakesPath becomes PATH verbatim except for the
-// fake herdr binary this helper always adds, letting each test control whether no-mistakes is
-// reachable.
+// Mirrors setupPromoteHome but registers a no-mistakes-mode project and skips the worktree/treehouse
+// setup: gatePreflight fires in promote after the report, pane-busy, and brief checks but before
+// worktree.Get, so those three must be satisfied while nothing past gatePreflight needs to exist.
 func setupPromoteHomeGate(t *testing.T, noMistakesPath string) string {
 	t.Helper()
 	useFastLaunchPolling(t)
@@ -110,6 +105,8 @@ func setupPromoteHomeGate(t *testing.T, noMistakesPath string) string {
 		t.Fatal(err)
 	}
 
+	// noMistakesPath becomes PATH verbatim except for the fake herdr binary this helper always adds,
+	// letting each test control whether no-mistakes is reachable.
 	herdrBin := t.TempDir()
 	if err := os.WriteFile(filepath.Join(herdrBin, "herdr"), []byte(fakeHerdrPaneDone), 0o755); err != nil {
 		t.Fatal(err)
@@ -120,10 +117,9 @@ func setupPromoteHomeGate(t *testing.T, noMistakesPath string) string {
 	return home
 }
 
-// TestSpawnRefusesWhenNoMistakesGateNotInitialized stands in for both real histories from
-// atqamz/secondhand#60 (never-initialized project, and a project whose working_path went stale
-// after the fleet home was renamed): both were checked against the real binary and emit the same
-// status text, so one refusal test here covers both.
+// Stands in for both real histories from atqamz/secondhand#60 (a never-initialized project, and one
+// whose working_path went stale after the fleet home was renamed): both were checked against the real
+// binary and emit the same status text, so one refusal test covers both.
 func TestSpawnRefusesWhenNoMistakesGateNotInitialized(t *testing.T) {
 	path := fakeNoMistakesPath(t, "repo not initialized (run 'no-mistakes init' first)")
 	home := setupSpawnHomeGate(t, path)
@@ -169,9 +165,9 @@ func TestSpawnProceedsWhenNoMistakesGateReady(t *testing.T) {
 	}
 }
 
-// TestSpawnSkipGateCheckBypassesRefusalAndWarns pairs the two halves of the escape hatch: the
-// not-initialized gate no longer refuses, and the bypass still announces itself on stderr, which
-// is the only thing that keeps it visible in a transcript.
+// Pairs the two halves of the escape hatch: the not-initialized gate no longer refuses, and the
+// bypass still announces itself on stderr, which is the only thing that keeps it visible in a
+// transcript.
 func TestSpawnSkipGateCheckBypassesRefusalAndWarns(t *testing.T) {
 	path := fakeNoMistakesPath(t, "repo not initialized (run 'no-mistakes init' first)")
 	setupSpawnHomeGate(t, path)

cmd/hold.go

@@ -72,10 +72,9 @@ func newHoldSetCmd() *cobra.Command {
 	return cmd
 }
 
-// The limit kind is refused with its own message rather than falling through to
-// the generic one: it is a real kind hand status renders and hand spawn honors,
-// so an operator who names it deserves to be told who owns it instead of that it
-// does not exist.
+// The limit kind is refused with its own message rather than falling through to the generic one: it is a
+// real kind hand status renders and hand spawn honors, so an operator who names it deserves to be told
+// who owns it instead of that it does not exist.
 func validateHoldKind(kind string) error {
 	switch kind {
 	case state.HoldKindOperator, state.HoldKindBlocked: