diff --git a/CLAUDE.md b/CLAUDE.md index fcc52ad..ac91ef0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,8 +54,15 @@ carries `-count=1` and sets `MAVEN_ONNX_LIB`. Without that variable the four make build # all 11 binaries. make build-web for one (web/waked/poll/caldav skip CGO) make test # go test -race across ./internal/... ./cmd/... with CGO env set make t PKG=./internal/router/eval/ RUN='TestONNX' V=1 # V=1 for -v, RACE=0 to drop -race +make analyze # staticcheck, deadcode and govulncheck. Not in `test`: all three need the network ``` +**The static gates pass against a baseline, not against zero** +(`scripts/analyzers/*.baseline`, reasoning in `docs/workflow.md`). A fix must +delete its baseline entry, because the gate also fails on an entry whose finding +is gone. **`make audit` is a git-grep inventory, not analysis.** Do not cite it +as a reachability check. + ## The daemons Eleven binaries under `cmd/`, wired socket-to-socket over `internal/ipc`, not diff --git a/Makefile b/Makefile index 031ea7c..90ed983 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ PIPER_BIN := $(shell pwd)/deps/piper/piper PIPER_MODEL := $(shell pwd)/models/tts/ru_RU-irina-medium.onnx PIPER_ESPEAK := $(shell pwd)/deps/piper/espeak-ng-data -.PHONY: t audit simulate stt-fixtures test-stt-golden all build build-stt build-tts build-daemon build-client build-waked build-web build-poll build-caldav clean test fmt-check vet run-stt run-tts run-web download-embedder deps-go deps-sentinel deps-vuln vuln tidy eval-router eval-reach eval-recall eval-phrasing eval-models build-gpud +.PHONY: t audit simulate stt-fixtures test-stt-golden all build build-stt build-tts build-daemon build-client build-waked build-web build-poll build-caldav clean test fmt-check vet run-stt run-tts run-web download-embedder deps-go deps-sentinel deps-vuln vuln deps-lint lint deadcode analyze tidy eval-router eval-reach eval-recall eval-phrasing eval-models build-gpud all: build @@ -119,6 +119,46 @@ vuln: deps-vuln CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" LD_LIBRARY_PATH="$(shell pwd)/deps/lib" \ PATH="$(shell pwd)/deps/go/go/bin:$$PATH" GOTOOLCHAIN=local $(GOVULNCHECK) ./... +# lint and deadcode — the other two analyzers the 2026-08-10 audit asked for +# (V-694). They are not part of `test` for the same reason `vuln` is not: they +# install over the network, and they are slow enough that a change to one Go +# file should not pay for them. +# +# Neither reports zero, so neither fails on its own output. The accepted set +# lives in scripts/analyzers/*.baseline and scripts/analyzer-gate.sh decides. +# What is new fails, and so does a baseline entry whose finding is gone. +# +# deadcode runs with -test, so a test file is a root. Without it the report is +# 172 lines, most of internal/router/eval, and none of it is a mistake. +STATICCHECK_VERSION := v0.7.0 +DEADCODE_VERSION := v0.48.0 +STATICCHECK := $(shell pwd)/deps/bin/staticcheck +DEADCODE := $(shell pwd)/deps/bin/deadcode + +deps-lint: deps-sentinel + @mkdir -p deps/bin + GOTOOLCHAIN=local GOBIN=$(shell pwd)/deps/bin \ + $(GO) install honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) + GOTOOLCHAIN=local GOBIN=$(shell pwd)/deps/bin \ + $(GO) install golang.org/x/tools/cmd/deadcode@$(DEADCODE_VERSION) + +# Both load the packages, so both carry the CGO env `test` carries. Without it +# the four CGO daemons do not load and the analyzer reports a build error +# instead of a finding -- which analyzer-gate.sh fails on rather than filters. +ANALYZER_ENV = CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" \ + LD_LIBRARY_PATH="$(shell pwd)/deps/lib" \ + PATH="$(shell pwd)/deps/go/go/bin:$$PATH" GOTOOLCHAIN=local + +lint: deps-lint + @$(ANALYZER_ENV) $(STATICCHECK) ./... | scripts/analyzer-gate.sh staticcheck + +deadcode: deps-lint + @$(ANALYZER_ENV) $(DEADCODE) -test ./... | scripts/analyzer-gate.sh deadcode + +# Every static gate in one command. Not `check`, because it is not the thing to +# run before a commit: vuln reads the network and all three are slow. +analyze: lint deadcode vuln + # Run the tidy the sentinel makes possible. Not part of `test`: it rewrites # go.mod, and a build target that edits the module file is a surprise. # vendor/ is committed, so a tidy that drops a requirement must be followed by diff --git a/docs/caveats/CLAUDE.md b/docs/caveats/CLAUDE.md index 8bbe8f6..46f9e49 100644 --- a/docs/caveats/CLAUDE.md +++ b/docs/caveats/CLAUDE.md @@ -21,10 +21,11 @@ caveat is the pointer between them plus the trigger. ## Index Every entry below came from the 2026-08-10 deep audit -(`docs/evals/2026-08-10-repo-audit.md`). Two of the twenty findings are fixed -and have no entry. The unauthenticated mavgpud proxy was V-673. The 20 reachable -advisories in the toolchain and `x/text` were V-682, which left the analyzers -entry below behind under its own id. +(`docs/evals/2026-08-10-repo-audit.md`), except the last, which came from wiring +the gate the audit asked for. Three of the twenty findings are fixed and have no +entry. The unauthenticated mavgpud proxy was V-673. The 20 reachable advisories +in the toolchain and `x/text` were V-682. The missing analyzers were V-694, and +what they now report is the baseline entry under V-701. | limit | severity | | --- | --- | @@ -44,6 +45,6 @@ entry below behind under its own id. | [baselineGrammars is mirrored by hand](invariants.md#grammars) | medium | | [Committed absolute paths pin the build to this box](config.md#paths) | medium | | [The env example omits deployed variables](config.md#secrets) | medium | -| [staticcheck and deadcode are not wired into a make target](dependencies.md#analyzers) | medium | +| [The analyzers pass against a baseline, not zero](dependencies.md#baseline) | medium | | [Domain packages depend on store and IPC types](layering.md#dtos) | low | | [Eleven symbols are unreachable](layering.md#deadcode) | low | diff --git a/docs/caveats/dependencies.md b/docs/caveats/dependencies.md index 3c61aa1..45136c6 100644 --- a/docs/caveats/dependencies.md +++ b/docs/caveats/dependencies.md @@ -1,14 +1,13 @@ # Dependencies -## staticcheck and deadcode are not wired into a make target [#694] {#analyzers} +## The analyzers pass against a baseline, not against zero [#701] {#baseline} -Costs: two of the three analyzers the 2026-08-10 audit asked for are missing. -Neither is installed on this box and no target runs them. `make audit` is a git-grep -inventory over loc, todo, stubs, docs, tests and gaps. **Do not read it as a -static-analysis gate.** `make vuln` is the third one and it is wired (V-682): -govulncheck is pinned in the Makefile, installed into `deps/bin` and run over -`./...`. It reads the published database over the network, so it stays out of -`make test`. -Revisit when: the next dead-code claim needs checking. `deadcode` has a finding -waiting for it in [layering.md](layering.md#deadcode). -Workaround: none. Read a reachability claim as unverified until one of them runs. +Costs: `make lint` and `make deadcode` are wired and green (V-694), but green +means "nothing new since 2026-08-11". The accepted set is 19 staticcheck +findings and 13 unreachable symbols, listed with a reason each in +`scripts/analyzers/*.baseline`. Three of the unreachable symbols must stay: +[layering.md](layering.md#deadcode). One accepted staticcheck finding is V-687. +Revisit when: V-701 sweeps the baseline, or a fix deletes an entry. The gate +fails on an entry whose finding is gone, so the deletion is not optional. +Workaround: none needed. Reachability claims are checkable now. Read the +baseline before trusting that a target reporting clean means the tree is clean. diff --git a/docs/workflow.md b/docs/workflow.md index 084ec48..a8180fe 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -1,6 +1,6 @@ # Session workflow: the five stores and the guards -*Last verified: 2026-08-09 @ a9b480a* +*Last verified: 2026-08-11 @ 557f5a3* How a session starts, where each kind of writing belongs, and what the hooks refuse. `CLAUDE.md` carries the commands. This file carries the reasoning. @@ -78,3 +78,40 @@ blocks further edits past 600 changed lines on a `task/` branch. budget read high. `--no-verify` exists. Using it means saying why in the commit body. + +## Static gates + +Three analyzers, one target each, and `make analyze` for all three. The +2026-08-10 audit asked for them because none was installed on the box and +`make audit` is a git-grep inventory, not analysis. Do not read `make audit` as +a gate. + +- `make vuln`, govulncheck over `./...` (V-682). +- `make lint`, staticcheck over `./...` (V-694). +- `make deadcode`, deadcode with `-test` over `./...` (V-694). + +None of the three joins `make test`. All three install over the network, and +`test` has to pass on a box with no route out. `vuln` reads the advisory +database at run time as well. Run `make analyze` before a dependency or +toolchain bump lands, and before calling a symbol unreachable. + +Each tool is pinned in the Makefile beside `GO_VERSION`. A gate that moves on +its own is not a gate. Each installs into `deps/bin`, because a tool is not a +dependency of the module. + +**staticcheck and deadcode pass against a baseline, not against zero.** The +accepted findings live in `scripts/analyzers/*.baseline`, one line each. A key +holds file, check id and message, never a line number. A line number goes stale +on the next edit above it. The output then reports moved findings as new ones, +and the reader learns to skip it. + +`scripts/analyzer-gate.sh` gives the verdict. A finding absent from the baseline +fails. So does a baseline entry whose finding is gone, which is what stops the +accepted set from outliving the repo. Deleting the entry is part of each fix. + +`deadcode` runs with `-test` because a test is a caller. Without the flag the +report is 172 lines, most of `internal/router/eval`, none of it a mistake. + +A baseline entry carries the reason it stays. Three reasons appear. Another task +owns the finding. The check cannot see through a false positive. A cosmetic +finding waits for a sweep. diff --git a/scripts/analyzer-gate.sh b/scripts/analyzer-gate.sh new file mode 100755 index 0000000..78eea1f --- /dev/null +++ b/scripts/analyzer-gate.sh @@ -0,0 +1,97 @@ +#!/usr/bin/env bash +# analyzer-gate.sh — turn an analyzer's output into a pass/fail verdict. +# +# The 2026-08-10 audit asked for staticcheck, govulncheck and deadcode +# (V-694). govulncheck needed no gate of this shape because it already +# reported zero after the toolchain bump. The other two do not: staticcheck +# reports 20 findings today and deadcode reports 11 unreachable symbols, and +# three of those eleven are deliberate. A target that fails on the first run +# is not a gate, it is a target nobody runs. So the accepted set is written +# down, and only what is NOT in it fails. +# +# staticcheck ./... | scripts/analyzer-gate.sh staticcheck +# deadcode -test ./... | scripts/analyzer-gate.sh deadcode +# +# The baseline is keyed on file, check id and message, never on line number. +# A key carrying a line number goes stale on the next edit above it and then +# reports moved findings as new ones, which trains the reader to ignore it. +# The cost of dropping the line is that two identical findings in one file +# share one key, so the second is accepted with the first. That is the right +# way round: the same check firing twice on the same file is one thing to fix. +# +# A baseline entry with no finding left also fails. Fixing something and +# leaving its entry behind is how the accepted set stops describing the repo. +# The fix is one line: delete the entry the failure names. +# +# Reads stdin, writes a report, never writes a file. + +set -uo pipefail +cd "$(dirname "$0")/.." || exit 1 + +tool="${1:?usage: analyzer-gate.sh }" +baseline="scripts/analyzers/$tool.baseline" +[ -f "$baseline" ] || { printf 'analyzer-gate: no baseline at %s\n' "$baseline" >&2; exit 2; } + +# Normalise to "\t\t". Anything that does not parse is an +# analyzer error, not a finding, and it fails without consulting the baseline. +# staticcheck: path.go:12:34: message (SA1234) +# deadcode: path.go:12:34: unreachable func: Symbol +found=$(mktemp) || exit 2 +malformed=$(mktemp) || exit 2 +trap 'rm -f "$found" "$malformed"' EXIT + +while IFS= read -r line; do + [ -n "$line" ] || continue + case "$tool" in + staticcheck) + if [[ "$line" =~ ^([^:]+):[0-9]+:[0-9]+:\ (.*)\ \(([A-Z]+[0-9]+)\)$ ]]; then + # SA1019 ends its message with a space. Trim, so no baseline entry + # depends on trailing whitespace surviving an editor. + msg="${BASH_REMATCH[2]}" + printf '%s\t%s\t%s\n' "${BASH_REMATCH[1]}" "${BASH_REMATCH[3]}" "${msg%"${msg##*[![:space:]]}"}" >>"$found" + else + printf '%s\n' "$line" >>"$malformed" + fi + ;; + deadcode) + if [[ "$line" =~ ^([^:]+):[0-9]+:[0-9]+:\ unreachable\ func:\ (.*)$ ]]; then + printf '%s\tunreachable\t%s\n' "${BASH_REMATCH[1]}" "${BASH_REMATCH[2]}" >>"$found" + else + printf '%s\n' "$line" >>"$malformed" + fi + ;; + *) printf 'analyzer-gate: unknown tool %s\n' "$tool" >&2; exit 2 ;; + esac +done + +if [ -s "$malformed" ]; then + printf '%s: the analyzer said something that is not a finding:\n' "$tool" >&2 + sed 's/^/ /' "$malformed" >&2 + exit 1 +fi + +accepted=$(mktemp) || exit 2 +trap 'rm -f "$found" "$malformed" "$accepted"' EXIT +grep -v '^[[:space:]]*\(#\|$\)' "$baseline" | sort -u >"$accepted" +sort -u "$found" -o "$found" + +new=$(comm -23 "$found" "$accepted") +gone=$(comm -13 "$found" "$accepted") +status=0 + +if [ -n "$new" ]; then + printf '%s: %d finding(s) not in %s:\n' "$tool" "$(printf '%s\n' "$new" | wc -l)" "$baseline" + printf '%s\n' "$new" | sed 's/^/ /' + printf 'Fix it, or add the line to the baseline with the reason it stays.\n' + status=1 +fi + +if [ -n "$gone" ]; then + printf '%s: %d baseline entry/entries no longer found:\n' "$tool" "$(printf '%s\n' "$gone" | wc -l)" + printf '%s\n' "$gone" | sed 's/^/ /' + printf 'Delete them from %s.\n' "$baseline" + status=1 +fi + +[ "$status" -eq 0 ] && printf '%s: clean against %d accepted finding(s)\n' "$tool" "$(wc -l <"$accepted")" +exit "$status" diff --git a/scripts/analyzers/deadcode.baseline b/scripts/analyzers/deadcode.baseline new file mode 100644 index 0000000..b96f7c1 --- /dev/null +++ b/scripts/analyzers/deadcode.baseline @@ -0,0 +1,32 @@ +# deadcode — the unreachable symbols this repo accepts today. +# +# Keyed "\t unreachable \t", tab separated, no line numbers. +# Generated from the first gated run on 2026-08-11 and edited by hand since. +# `make deadcode` fails on anything absent here and on any entry left behind +# after its symbol is deleted. +# +# The gate runs with -test, so a test file counts as a root. Without it the +# whole of internal/router/eval is unreachable and the report is 172 lines of +# fixtures nobody wrote by mistake. +# +# Eleven of these are V-686, from the 2026-08-10 audit. Three of the eleven +# must stay and the audit says why: HisGender is a documented seam tied to +# V-399, AudioDuration should call internal/audio rather than be deleted, and +# CountWord is a safe delete. Read docs/caveats/layering.md#deadcode before +# removing any of them. +cmd/mavwaked/vad.go unreachable AudioDuration +cmd/mavwaked/vad.go unreachable PCMToF32 +internal/crawl/watch.go unreachable Watcher.Watches +internal/phraser/confirm.go unreachable IsC +internal/phraser/eval/checks.go unreachable HisGender +internal/phraser/plural.go unreachable CountWord +internal/update/update.go unreachable WithClock +internal/voice/errors.go unreachable jsonMarshal +internal/voice/errors.go unreachable jsonUnmarshal +internal/webauthn/cbor.go unreachable cborValue.At +internal/worker/server.go unreachable Server.SetSynthesizer + +# Two test helpers the audit did not count, because it listed production +# symbols only. A helper no test calls is dead the same way. +cmd/mavend/replier_llm_test.go unreachable assertStub +cmd/mavwaked/vad_test.go unreachable frameRMSQuick diff --git a/scripts/analyzers/staticcheck.baseline b/scripts/analyzers/staticcheck.baseline new file mode 100644 index 0000000..a8f9614 --- /dev/null +++ b/scripts/analyzers/staticcheck.baseline @@ -0,0 +1,46 @@ +# staticcheck — the findings this repo accepts today. +# +# Keyed "\t\t", tab separated, no line numbers. +# Generated from the first gated run on 2026-08-11 and edited by hand since. +# `make lint` fails on anything absent here and on any entry left behind after +# its finding is fixed, so emptying this file is done one line at a time. +# +# The sweep that empties it is V-701, which carries the judgement on each +# entry. What follows is the short reason only. + +# V-687. The dedupe check runs after the phraser has already been paid. +cmd/mavend/tick_digest.go SA4006 this value of deduped is never used + +# V-686, the eleven unreachable symbols the 2026-08-10 audit listed, seen from +# the other side. Three of them must stay: docs/caveats/layering.md#deadcode. +cmd/mavend/replier_llm_test.go U1000 func assertStub is unused +cmd/mavwaked/vad_test.go U1000 func frameRMSQuick is unused +cmd/mavweb/handlers_test.go U1000 field signalErr is unused +internal/voice/errors.go U1000 func jsonMarshal is unused +internal/voice/errors.go U1000 func jsonUnmarshal is unused + +# False positives, checked. The code is right and the check cannot see why. +# RenderICal is called twice because rendering twice is the assertion. The +# morning loop reads the first rune after the hedge and breaks on purpose. The +# task_phrases line is prose about //go:embed and the real directive is below it. +internal/calendar/ical_render_test.go SA4000 identical expressions on the left and right side of the '!=' operator +internal/morning/plan_test.go SA4004 the surrounding loop is unconditionally terminated +internal/router/task_phrases.go SA9009 ineffectual compiler directive due to extraneous space: "// go:embed, so the single-binary deploy is unchanged: the JSON is compiled into" + +# At EOF the wake loop trims partial and returns, so audio past one frame is +# dropped. Harmless where it sits, misleading to read. V-701. +cmd/mavwaked/main.go SA4006 this value of partial is never used + +# Cosmetic and mechanical. V-701 sweeps them. +cmd/mavweb/voiceproxy.go ST1013 should use constant http.StatusMethodNotAllowed instead of numeric literal 405 +cmd/mavweb/voiceproxy.go ST1013 should use constant http.StatusServiceUnavailable instead of numeric literal 503 +internal/ipc/client.go S1016 should convert r (type chatResp) to ChatReply instead of using struct literal +internal/ipc/server.go S1016 should convert reply (type ChatReply) to chatResp instead of using struct literal +internal/memory/behavior_test.go S1011 should replace loop with obs = append(obs, habitHistory("calendar_event_20260804_standup", time.Tuesday, 10, 0, 3, now)...) +internal/memory/behavior_test.go S1011 should replace loop with obs = append(obs, habitHistory("cooldown:water", time.Tuesday, 9, 0, 3, now)...) +cmd/mavend/continuation_test.go SA1012 do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use + +# Deprecated since Go 1.25. Replacing it means rewriting both guards on +# golang.org/x/tools/go/packages, which is a decision and not a sweep. +internal/ipc/maperr_test.go SA1019 parser.ParseDir has been deprecated since Go 1.25 and an alternative has been available since Go 1.11: ParseDir does not consider build tags when associating files with packages. For precise information about the relationship between packages and files, use golang.org/x/tools/go/packages, which can also optionally parse and type-check the files too. +internal/phraser/persona_floor_test.go SA1019 parser.ParseDir has been deprecated since Go 1.25 and an alternative has been available since Go 1.11: ParseDir does not consider build tags when associating files with packages. For precise information about the relationship between packages and files, use golang.org/x/tools/go/packages, which can also optionally parse and type-check the files too.