Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,37 @@ jobs:
shell: pwsh
run: choco install make --version=4.4.1 -y --no-progress

# golangci-lint, AT AN EXACT VERSION, on every leg, because the lint
# target is a step of make ci and make ci is what every leg runs.
# Until this step existed no leg had the linter: the target's skip
# line was printed inside every make ci step, and lint ran nowhere.
# The target now refuses that skip on CI, so a leg this step does not
# reach fails at lint rather than passing it.
#
# THROUGH THE GO TOOLCHAIN rather than a release download. An install
# at an exact module version is checked against the checksum database
# this job's Go is configured with, so the version below pins the
# content and not only the name. The linter is then built by the same
# Go this leg tests with, and that matters in one direction: a linter
# built by an older Go cannot type-check a newer one's standard
# library, and fails on files nobody touched. What it costs is a build
# of the linter on every run of every leg.
#
# THE VERSION MOVES BY HAND. The scheduled update policy bumps action
# references and cannot see a version written in a line of shell.
#
# THE VERSION IT PRINTS is the record of which linter the leg ran: the
# lint target runs whichever golangci-lint is first on PATH, and this
# line says which one that was. Built without cgo, as everything this
# module builds is, so no leg's C toolchain takes part.
- name: install golangci-lint
shell: bash
env:
CGO_ENABLED: "0"
run: |
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.14.0
golangci-lint --version

- name: make ci
shell: bash
run: make ci
Expand Down
45 changes: 45 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# golangci-lint configuration, in the version 2 schema.
#
# LINT GATES, OVER TWO CLASSES OF FINDING AND NO OTHERS: declarations
# nothing uses (unused), and staticcheck's correctness checks (its SA
# family). Every other linter is off, and the reason is a measurement
# rather than a preference.
#
# Measured 2026-10-05 with golangci-lint 2.14.0: the linter's default set
# was run over this tree with every finding printed, and all three CI legs
# reported the same 110.
#
# - errcheck: 93, and every one was false. Each was a write to a
# terminal or a report stream that the program cannot act on when it
# fails, where the exit status already carries the verdict, or a Close
# on something opened only for reading. errcheck cannot tell a Close
# that could lose a write from one that cannot, by the name it reads,
# so there is no narrower form of it to keep.
# - staticcheck's style families (S1, ST and QF): 10. Nine were rewrites
# of code that already reads clearly. The tenth would have done damage:
# it asked for a fmt.Sprintf("%s", value) to become the value itself,
# inside the positive control of a redaction test, where formatting
# through that verb is the very path the control exists to run.
# - unused and the SA family: 7, and all signal. Four dead declarations
# and one deprecated call were fixed. Two lines that exist to prove
# something stay, each with an annotation on the line saying what it
# proves and why the linter is wrong about it.
#
# So the classes that are off produced nothing worth changing and one
# change that would have weakened a test; the classes that are on produced
# nothing else. govet and ineffassign, the rest of the default set,
# reported nothing; vet runs anyway, as its own step of make ci.
#
# WHAT THIS DOES NOT SEE: an unchecked error is not linted at all.
version: "2"

linters:
default: none
enable:
- staticcheck
- unused
settings:
staticcheck:
# The correctness checks only, for the reason above.
checks:
- "SA*"
53 changes: 53 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,51 @@ YAML is a step nobody can run before pushing.
- `make fmt` `make vet` `make test` `make build` — `ci` runs them in that
order, formatting first, so a formatting failure is not discovered
after a five-minute suite.
- **`make lint` GATES, over two classes of finding and no others, and
that is a ruling rather than a default** (2026-10-05). The workflow
installs golangci-lint at an exact version on every leg, through the Go
toolchain, and `lint` is a step of `make ci`, so a finding reds every
leg. On CI an absent linter is a failure, not a skip; on a machine
without it the target skips and says so. `.golangci.yml` enables
`unused` and staticcheck's correctness checks, and nothing else.

**Why those two.** The linter's default set was run over this tree with
every finding printed, and all three legs reported the same 110.
errcheck gave 93, and every one was false: a write to a terminal or a
report stream the program cannot act on when it fails, where the exit
status already carries the verdict, or a Close on something opened only
for reading. staticcheck's style families gave 10: nine rewrites of code
that already reads clearly, and one that would have done damage, asking
for a `fmt.Sprintf("%s", value)` inside a redaction test's positive
control to become the value itself, which would stop the control
running the path it exists for. The two classes kept gave 7, and every
one was signal: four dead declarations and a deprecated call, which
were fixed, and two lines that exist to prove something.

**Why a gate rather than advice.** `lint` runs inside `make ci`, on
every leg of every push. A step there whose exit code decides nothing
is a decoration, by this file's own rule: it spends its runtime on every
run, and while it is green its output is a log nobody reads. Advice is
honest where a person runs the linter and reads it; inside the gate it
is a cost with no consequence.

**The two annotations, and what they protect.** Each is a
`//nolint:<linter> // <reason>` on the line it excuses, so adding a
third is a reviewable change with its reason beside it, never a quiet
widening of the configuration:

- the `%s` of a whole config, through an `any`, in the unknown-fields
leak test. The verb is wrong for the type on purpose: a debug print
through an `any` is how a secret would leak, vet cannot see that path,
and the row exists to prove it does not leak;
- the shared floor for a stall-window leg with nothing recorded.
Nothing uses it today, by design: a refusal tells a caller to pass it,
and a guard polices that name, so deleting it would leave both naming
nothing.

**What it does not see:** an unchecked error is not linted at all.
errcheck cannot tell a Close that could lose a write from one that
cannot by the name it reads, so that class stays with review.
- **`make surface-check` is the one check `ci` cannot carry, and the
reason is the shape of its subject rather than its cost.** There is no
push range in a working copy: a checkout is one state, and that check is
Expand Down Expand Up @@ -380,6 +425,14 @@ YAML is a step nobody can run before pushing.
invites is `--admin`, which would merge past the very queue this
ruleset exists to enforce.

**The queue refuses to enqueue a pull request whose head commit carries
a failing run of a required check, even when another run of that check
on the same commit is green** — observed 2026-10-05: the push-event run
and the pull-request run both report `ci (windows-latest)`, and with the
first red and the second green the enqueue was refused with *"Pull
request has failing required statuses and Pull request Required status
check "ci (windows-latest)" is failing"*.

**The seven required checks, spelled exactly as GitHub names them:**

| required check |
Expand Down
34 changes: 31 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,39 @@ fmt:
exit 1; \
fi

# lint is a no-op until a linter is actually configured for this repo —
# it must never fail ci for the reason "no linter is installed".
# lint runs golangci-lint over the whole module.
#
# ON CI IT IS INSTALLED, SO AN ABSENT LINTER IS A FAILURE. This comment
# used to say the opposite: that lint must never fail ci for the reason
# "no linter is installed". While that held, no workflow installed one,
# every leg printed the skip line below inside its make ci step, and lint
# ran nowhere with nothing to say so but that line. The workflow now
# installs the linter at an exact version on every leg before make ci, so
# on CI there is nothing to tolerate: a skip there would be this target
# reporting that it ran when it did not. CI is told apart the way
# guard-a-branch-to-work-on tells it apart, by a CI variable that is set
# and not empty.
#
# ON A MACHINE WITHOUT IT, it still skips, and says so. The linter is not
# a build dependency of this module, and a contributor has no reason to
# hold it. The skip line is kept word for word because it is what a CI log
# is searched for to show the linter did not run, and a reworded skip
# would make that search find nothing whether or not it ran.
#
# EVERY FINDING IS PRINTED. By default the linter caps how many findings
# it prints per linter and how many share one message, and keeps one
# finding per line, so a new finding can land inside a group it has
# already collapsed and the count it prints does not move. A report that
# drops findings cannot be read as a list of them; these flags turn all
# three reductions off.
lint:
@if command -v golangci-lint >/dev/null 2>&1; then \
golangci-lint run ./...; \
golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 --uniq-by-line=false ./...; \
elif [ -n "$$CI" ]; then \
echo "golangci-lint is not installed, and on CI that is a failure rather than a skip."; \
echo "The workflow installs it at an exact version before make ci runs, so if it"; \
echo "is missing here, that step did not run or did not put it on PATH."; \
exit 1; \
else \
echo "golangci-lint not installed; skipping lint"; \
fi
Expand Down
2 changes: 1 addition & 1 deletion internal/config/unknown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestUnknownFieldsRenderKeysAndNeverValues(t *testing.T) {
// ...any parameter, which is exactly where vet cannot look either.
var asAny any = loaded
renderings := map[string]string{
"%s of the config": fmt.Sprintf("%s", asAny),
"%s of the config": fmt.Sprintf("%s", asAny), //nolint:staticcheck // the wrong verb is the point: a debug print through an any is how a secret would leak, vet cannot see it, and this row proves it does not leak
"%v of the config": fmt.Sprintf("%v", loaded),
"%+v of the config": fmt.Sprintf("%+v", loaded),
"%v of the value": fmt.Sprintf("%v", *loaded),
Expand Down
2 changes: 1 addition & 1 deletion internal/flow/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ const (
// caller passes its own and says why; this value is what a caller
// passes when its leg has nothing measured to reason from, and passing
// it is a statement about the RECORD rather than about the row.
unmeasuredLegFloor = 6 << 20
unmeasuredLegFloor = 6 << 20 //nolint:unused // unused by design: the floor for a leg with nothing recorded, which blockPointFloor's refusal tells a caller to pass and TestNoRowInheritsAFloorItDidNotState polices by this name

// drainTail is how much body is left after the pacing stops, to be
// drained at full speed. It matters for the reason the paced phase
Expand Down
16 changes: 5 additions & 11 deletions internal/guard/failurecontract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ type obligation struct {
where string
field string
expr string
chain string
values []string // resolved possibilities; empty means unresolved
problem string // a structurally forbidden write or address-taking
binding string // reaching definitions for an obligation-ledger expression
Expand Down Expand Up @@ -1251,16 +1250,11 @@ func TestBuildTargetsKeepTheirOwnTypeInfoAndIncludeArm64(t *testing.T) {
}
}

// resolve reduces one argument expression to the finite set of values it
// can carry, or to nothing at all — which is UNRESOLVED and fails.
func resolve(site, field string, e ast.Expr, text func(parsedFile, ast.Node) string,
p parsedFile, kind string) obligation {
return resolveIn(site, field, e, text, p, kind, nil, 0)
}

// resolveIn carries the enclosing function, so a local name can be traced
// to the statement that defined it, and a depth, so a cycle stops rather
// than recursing forever. RECURSION FAILS CLOSED: past the bound the
// resolveIn reduces one argument expression to the finite set of values it
// can carry, or to nothing at all — which is UNRESOLVED and fails. It
// carries the enclosing function, so a local name can be traced to the
// statement that defined it, and a depth, so a cycle stops rather than
// recursing forever. RECURSION FAILS CLOSED: past the bound the
// obligation is unresolved, which is a visible failure.
func resolveIn(site, field string, e ast.Expr, text func(parsedFile, ast.Node) string,
p parsedFile, kind string, within *ast.FuncDecl, depth int) obligation {
Expand Down
34 changes: 0 additions & 34 deletions internal/guard/guard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,40 +846,6 @@ func TestNamedURLConstantsStayUnderTheCeiling(t *testing.T) {
// Guard 3: no private citation in a source comment.
// ---------------------------------------------------------------------

// loadCitationPatterns reads and compiles every pattern in the committed
// manifest. It genuinely reads the file on every run rather than caching
// a copy of its contents in this package: the required proof for this
// guard is that deleting a line from the manifest measurably changes what
// the guard can see, and a guard that read its own hardcoded copy of the
// patterns would fail that proof while still going green on the day
// nobody expects it to.
func loadCitationPatterns(t *testing.T, root string) []*regexp.Regexp {
t.Helper()
path := filepath.Join(root, "scripts", "citation-patterns.txt")
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("reading %s: %v", path, err)
}

declared, err := rulefile.Parse("scripts/citation-patterns.txt", string(data))
if err != nil {
t.Fatal(err)
}
var patterns []*regexp.Regexp
for _, rule := range declared {
re, err := regexp.Compile(rule.Text)
if err != nil {
t.Fatalf("scripts/citation-patterns.txt:%d: %s is not a pattern this guard can "+
"compile: %v", rule.Line, rule.ID, err)
}
patterns = append(patterns, re)
}
if len(patterns) == 0 {
t.Fatal("scripts/citation-patterns.txt contains no patterns — this guard would silently pass")
}
return patterns
}

// loadVendorTerms reads the provider and service vocabulary this
// repository may not name in authored text. Read on every run, no cached
// copy, for the reason every rule file here is: deleting a line has to
Expand Down
17 changes: 0 additions & 17 deletions internal/pack/limits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"os"
"path/filepath"
"reflect"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -1549,22 +1548,6 @@ func wholeFinding(f check.Finding) string {
return strings.Join([]string{f.Message, f.What, f.Why, f.Next}, "\n")
}

// listedInOrder reports the first pair of names that appear in s out of
// the order given, or the empty string when they all agree.
func listedInOrder(s string, want []string) string {
positions := make([]int, len(want))
for i, w := range want {
positions[i] = strings.Index(s, w)
if positions[i] < 0 {
return fmt.Sprintf("%s is missing", w)
}
}
if !sort.IntsAreSorted(positions) {
return fmt.Sprintf("positions %v", positions)
}
return ""
}

func rowFor(t *testing.T, res check.Results, id string) check.Status {
t.Helper()
for _, row := range res.Manifest {
Expand Down
50 changes: 31 additions & 19 deletions tools/leakscan/spawnbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,32 +179,44 @@ func TestEveryGitInvocationGoesThroughTheDeadline(t *testing.T) {
const wrapped = "runInputNow"
const wrapper = "runInput"

// EVERY NON-TEST FILE IN THE DIRECTORY IS PARSED, whatever its build
// constraint, so a caller in a file built only for another platform is
// still seen. A package loader would answer for the host's build alone,
// and would add a dependency this module does not otherwise need.
fset := token.NewFileSet()
pkgs, err := parser.ParseDir(fset, ".", func(fi os.FileInfo) bool {
return !strings.HasSuffix(fi.Name(), "_test.go")
}, 0)
entries, err := os.ReadDir(".")
if err != nil {
t.Fatalf("parsing this package: %v", err)
t.Fatalf("listing this package: %v", err)
}
var files []*ast.File
for _, entry := range entries {
name := entry.Name()
if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
}
file, err := parser.ParseFile(fset, name, nil, 0)
if err != nil {
t.Fatalf("parsing %s: %v", name, err)
}
files = append(files, file)
}

callers := map[string]int{}
scanned := 0
for _, pkg := range pkgs {
for _, file := range pkg.Files {
var enclosing string
ast.Inspect(file, func(n ast.Node) bool {
if fd, ok := n.(*ast.FuncDecl); ok {
enclosing = fd.Name.Name
}
sel, ok := n.(*ast.SelectorExpr)
if !ok || sel.Sel.Name != wrapped {
return true
}
scanned++
callers[enclosing]++
for _, file := range files {
var enclosing string
ast.Inspect(file, func(n ast.Node) bool {
if fd, ok := n.(*ast.FuncDecl); ok {
enclosing = fd.Name.Name
}
sel, ok := n.(*ast.SelectorExpr)
if !ok || sel.Sel.Name != wrapped {
return true
})
}
}
scanned++
callers[enclosing]++
return true
})
}

if scanned == 0 {
Expand Down
Loading