chore(lint): add comment noise tooling and enable gocritic

Description

Adds two dependency-free Go tools for keeping comment noise down, wires them into lefthook and CI, enables gocritic, and clears everything both gates report on the current tree.

scripts/comment-overlap flags comments whose words are already carried by the adjacent code, which is a mechanical proxy for "document only what is non-obvious from the code". Doc comments are excluded by position through go/ast Doc fields, so idiomatic godoc is never flagged for repeating its own symbol name. Gated at full token coverage, where the comment demonstrably adds nothing.

scripts/comment-ratio reports how much of a diff is comment rather than code, split into doc and inline. It judges no single comment, so unlike a classifier it cannot be defeated by rewording. Advisory only, because the measured distribution across the last 60 merged MRs has no natural cutoff (inline ratio: median 0.047, p90 0.168).

gocritic is enabled with the diagnostic and style tags, minus four checks that conflict with decisions already in .golangci.yml. It surfaced 171 findings, all fixed here.

Real defects fixed, as opposed to style

  • MCP tools could execute the wrong command. iterCommands yielded a path slice that registerTools captures in a long-lived handler closure, while sibling recursion appended into the same backing array. Past depth two the slice has spare capacity, so a sibling overwrote a registered tool's path. Affects paths like mr note create and project members add.
  • attestation/verify leaked a file descriptor on both error paths, because the close was deferred after them, and dereferenced a nil file when CreateTemp failed.
  • job/artifact held every extracted file's handles open until the whole archive finished, up to the 100k file limit. The entry write is now its own function.
  • artifactregistry/login could panic on a credentials line that matched with fewer submatches than it indexed.
  • gen-docs, git.RunClone, alias/expand and two config paths appended to a slice they did not own.

Reviewer note

The final commit mixes three concerns (syntax-preference fixes, the defects above, and comment deletions) because the pre-commit hooks block incremental commits while the tree is dirty. Roughly 105 of the 171 findings are syntax preference with no behaviour change, ~16 are the defects listed above, and 45 are comment deletions. Happy to split or to narrow the gocritic selection if the style checks are more churn than the team wants.

None. This came out of a discussion about reducing comment noise in the codebase.

How has this been tested?

make test: 5080 tests pass, 9 skipped, identical to the count before these changes. golangci-lint run ./...: 0 issues. comment-overlap gate: 0 findings.

Behaviour was verified by reverting every test file and running the suite against the production changes alone. All 5080 tests passed unmodified, then passed identically with the test-side lint fixes re-applied. So no test was adjusted to accommodate a behaviour change, and no coverage was removed.

Precision of the overlap gate was checked by reading findings rather than trusting the count: 16 of 16 sampled findings at the gated threshold were true positives. Two false positives found at full-tree scale were fixed in the tool rather than by deleting good comments, namely numbered step sequences and block comments annotating an unnamed return.

Merge request reports

Loading
Loading