Remediate all lint findings and make CI green (closes #1) #3

Merged
clawbot merged 1 commits from lint-remediation into main 2026-07-26 20:00:20 +02:00
Collaborator

Follow-up to the STRTA scaffold (landed in PR #2): fixes all ~192 pre-existing golangci-lint findings under the standard .golangci.yml and makes the CI/Docker gate genuinely green, closing #1.

Lint remediation (behavior-preserving)

  • gochecknoglobals (4): replaced package globals with an options struct threaded through the commands (also makes paralleltest safe).
  • err113 (4): package-level sentinel errors, wrapped with %w.
  • errcheck (15): every previously-unchecked error is now checked or explicitly discarded.
  • forbidigo (5): verbose output via fmt.Fprintf(os.Stdout, …).
  • noinlineerr/wsl/nlreturn/gofmt (~142): mechanical, mostly --fix.
  • complexity (cyclop/gocognit/nestif): small behavior-preserving helper extractions.
  • Remainder (mnd, lll, nonamedreturns, revive, modernize, nilnil, thelper, paralleltest): named constants, wrapped lines, doc comments, t.Helper()/t.Parallel(), etc.
  • One justified //nolint:gosec // G304: os.Open(path) where the operator-named path IS the intended input (verified false positive).

Two environment fixes so the tests actually run in CI (not skipped)

  1. xattr namespace: keys moved from berlin.sneak.app.attrsum.* to the user.* namespace (user.berlin.sneak.app.attrsum.checksum / …sumtime). Linux only permits user.*/trusted.*/security.*/system.* xattrs on regular files (the old keys returned EOPNOTSUPP); macOS treats the name as opaque, so behavior there is unchanged. README key table updated. NOTE: this is an intentional, owner-approved on-disk key rename — files summed under the old keys won't be recognized under the new keys.
  2. Docker builder runs make check (and make build) as an unprivileged builder user, so TestPermissionErrors (expecting EACCES on a 0000 file) is meaningful rather than bypassed by root.

Verification

make check and docker build . (in-image make check under the pinned golangci-lint v2.10.1) are both green: all 6 tests pass, 0 issues. make test and make fmt-check pass. Behavior preserved.

Follow-up to the STRTA scaffold (landed in PR #2): fixes all ~192 pre-existing golangci-lint findings under the standard `.golangci.yml` and makes the CI/Docker gate genuinely green, closing #1. ## Lint remediation (behavior-preserving) - `gochecknoglobals` (4): replaced package globals with an `options` struct threaded through the commands (also makes `paralleltest` safe). - `err113` (4): package-level sentinel errors, wrapped with `%w`. - `errcheck` (15): every previously-unchecked error is now checked or explicitly discarded. - `forbidigo` (5): verbose output via `fmt.Fprintf(os.Stdout, …)`. - `noinlineerr`/`wsl`/`nlreturn`/`gofmt` (~142): mechanical, mostly `--fix`. - complexity (`cyclop`/`gocognit`/`nestif`): small behavior-preserving helper extractions. - Remainder (`mnd`, `lll`, `nonamedreturns`, `revive`, `modernize`, `nilnil`, `thelper`, `paralleltest`): named constants, wrapped lines, doc comments, `t.Helper()`/`t.Parallel()`, etc. - One justified `//nolint:gosec // G304`: `os.Open(path)` where the operator-named path IS the intended input (verified false positive). ## Two environment fixes so the tests actually run in CI (not skipped) 1. xattr namespace: keys moved from `berlin.sneak.app.attrsum.*` to the `user.*` namespace (`user.berlin.sneak.app.attrsum.checksum` / `…sumtime`). Linux only permits `user.*`/`trusted.*`/`security.*`/`system.*` xattrs on regular files (the old keys returned `EOPNOTSUPP`); macOS treats the name as opaque, so behavior there is unchanged. README key table updated. NOTE: this is an intentional, owner-approved on-disk key rename — files summed under the old keys won't be recognized under the new keys. 2. Docker builder runs `make check` (and `make build`) as an unprivileged `builder` user, so `TestPermissionErrors` (expecting `EACCES` on a `0000` file) is meaningful rather than bypassed by root. ## Verification `make check` and `docker build .` (in-image `make check` under the pinned golangci-lint v2.10.1) are both green: all 6 tests pass, `0 issues`. `make test` and `make fmt-check` pass. Behavior preserved.
clawbot added 1 commit 2026-07-26 19:56:42 +02:00
Clear all golangci-lint findings and make the CI build green (closes #1)
All checks were successful
check / check (push) Successful in 35s
7f75f2ee72
Fix every golangci-lint finding under the repo's standard .golangci.yml
(from ~192 down to 0 under the pinned v2.10.1) without changing program
behavior:

- gochecknoglobals: replace the verbose/quiet/exclude* package globals with
  an options struct threaded through the command implementations.
- err113: introduce package-level sentinel errors and wrap them with %w.
- errcheck: check or explicitly discard every previously unchecked error
  (bar.Add/Finish, deferred Close, verbose writes).
- forbidigo: route verbose output through os.Stdout instead of fmt.Print*.
- noinlineerr / wsl_v5 / nlreturn / gofmt: split inline error checks and
  normalize whitespace.
- complexity (cyclop/gocognit/nestif): extract small behavior-preserving
  helpers (runOverPaths, countAndBar, walkSkip, clearOne, checkOne,
  missingChecksum, reportCheck).
- mnd/lll/nonamedreturns/revive/modernize/nilnil: named constants, wrapped
  lines, unnamed returns, doc comments, SplitSeq, non-(nil,nil) returns.
- paralleltest/thelper: mark tests parallel (now race-safe with no shared
  globals) and add t.Helper(); probe the real xattr key so the guard is
  accurate.

Also make the tests actually runnable in CI rather than skipping:

- move the xattr keys into the user.* namespace (user.berlin.sneak.app.*),
  which Linux requires for regular-file xattrs; macOS treats the whole
  string as an opaque name, so behavior is unchanged there.
- run the Docker builder's checks as an unprivileged user so the permission
  tests are meaningful (root bypasses file mode bits).
clawbot merged commit 7f75f2ee72 into main 2026-07-26 20:00:20 +02:00
clawbot deleted branch lint-remediation 2026-07-26 20:00:21 +02:00
Author
Collaborator

Merged to main after an independent adversarial review (reviewer separate from the author). The reviewer traced every refactor against the original control flow and found no behavior change (the options-struct threading, the extracted helpers, sentinel errors, and errcheck fixes are all semantics-preserving), confirmed the user.* xattr rename is applied consistently on every read/write/check/clear path with the README updated, verified the non-root Docker build genuinely exercises TestPermissionErrors as builder, and confirmed the single //nolint:gosec // G304 is the only nolint and a real false positive. Re-ran docker build .: 0 issues under the pinned golangci-lint and 6/6 tests pass as non-root. Hygiene and scope clean. Verdict: PASS. attrsum's make check / docker build are now green.

Merged to `main` after an independent adversarial review (reviewer separate from the author). The reviewer traced every refactor against the original control flow and found no behavior change (the `options`-struct threading, the extracted helpers, sentinel errors, and errcheck fixes are all semantics-preserving), confirmed the `user.*` xattr rename is applied consistently on every read/write/check/clear path with the README updated, verified the non-root Docker build genuinely exercises `TestPermissionErrors` as `builder`, and confirmed the single `//nolint:gosec // G304` is the only nolint and a real false positive. Re-ran `docker build .`: `0 issues` under the pinned golangci-lint and 6/6 tests pass as non-root. Hygiene and scope clean. Verdict: PASS. attrsum's `make check` / `docker build` are now green.
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/attrsum#3