diff --git a/.claude/rules/build-and-migrations.md b/.claude/rules/build-and-migrations.md index a6d2a30..5dd362e 100644 --- a/.claude/rules/build-and-migrations.md +++ b/.claude/rules/build-and-migrations.md @@ -6,7 +6,9 @@ ## Just Targets -`just proto` MUST run before `just build-*`. The `just all` target runs the full dependency chain: proto → build-daemon → build-mcp. Per-module targets (`test-core`, `lint-daemon`, etc.) exist for focused development. +Generated code is a prerequisite of every target that loads a daemon or mcp package — building, testing, linting, scanning, and `just tidy` alike, since production and test files in both modules import `daemon/gen/`. Each of those recipes declares `proto` as a dependency rather than relying on the caller to remember, and `just` runs it once per invocation however many legs ask for it. The `core` recipes deliberately do not, so `just test-core` and its siblings need no protobuf toolchain. Per-module targets (`test-core`, `lint-daemon`, etc.) exist for focused development. + +A recipe that loads Go packages without generated code present does not degrade quietly — it fails to load, naming the missing `daemon/gen/finch/v1` import. Any new recipe touching daemon or mcp wants the same `proto` dependency. ## Embedded Migrations diff --git a/.claude/rules/ci.md b/.claude/rules/ci.md index 99755df..b4f8912 100644 --- a/.claude/rules/ci.md +++ b/.claude/rules/ci.md @@ -16,12 +16,16 @@ This list is the single source of truth for the supplies below — they referenc `just vuln` scans all three Go modules with `govulncheck`. It is deliberately **not** in the list above: the scan takes minutes, and that list feeds the pre-merge and readiness supplies, so including it would impose the cost at every merge on top of CI already running it. It is also not in the pre-push hook, for the same reason. Run it when changing dependencies or when you want the answer before pushing; otherwise let CI be the enforcing surface. -It needs generated proto code, so run `just proto` first. +Its daemon and mcp legs regenerate protobuf code first, declared as a recipe prerequisite, so the scan needs no manual setup. ### What the gate guarantees `govulncheck` fails only on advisories it can **statically reach** from finch's own code. A scan reports a silent tail of advisories in imported packages and required modules that never surface, and static reachability is defeated by reflection and interface dispatch. So a green scan means "no reachable advisory," not "no known-vulnerable dependency." The broader question — is any dependency in the graph known-vulnerable at all — is answered by Dependabot alerts, which cover the full transitive closure but cannot tell you whether the code is reachable. The two are complements; neither alone is coverage. +So when scoping a dependency bump, read both surfaces rather than whichever one raised the alarm. For the scanner, `govulncheck -C -scan module` lists every advisory affecting a module in the graph with no reachability filtering — it lifts the reachability limits described above, not the database-coverage limit described below. Two notes on the form: module mode takes no package pattern, so a trailing `./...` is rejected, and it still loads packages, so run `just proto` first — this is a raw invocation rather than a recipe, so nothing declares that prerequisite on your behalf. For the alerts, `gh api repos/{owner}/{repo}/dependabot/alerts` — `.github/dependabot.yml` configures version updates rather than advisories, so it cannot answer this. + +Each surface can be the only one that sees a given advisory. A Dependabot alert names **one** advisory, and its stated fix version clears that advisory rather than the package — an unalerted sibling in the same package can need a higher version. Going the other way, an advisory with no Go vulnerability database entry is invisible to `govulncheck` in every mode, and Dependabot is the only place it appears. A bump scoped from one surface alone closes some of what it looks like it closed. + The scan covers Go modules only. The Qt app's C++ dependencies (gRPC and protobuf from apt, Qt from the install action) are covered by neither mechanism. ### When an advisory has no available fix diff --git a/.github/workflows/ci-go.yml b/.github/workflows/ci-go.yml index 99696e5..34350c9 100644 --- a/.github/workflows/ci-go.yml +++ b/.github/workflows/ci-go.yml @@ -130,9 +130,10 @@ jobs: - name: Generate protobuf code run: buf generate - # Enforces the go.sum reconciliation CLAUDE.md documents as a manual `just tidy` - # step. Must run after buf generate: without daemon/gen, tidy prunes the gRPC and - # protobuf requirements and reports a spurious diff. Cheap, so it fails fast. + # Enforces the go.sum reconciliation the `just tidy` recipe performs locally. + # Must run after buf generate: without daemon/gen, tidy cannot load the packages + # importing it and fails outright — it looks for the missing package as an external + # module rather than pruning anything. Cheap, so it fails fast. - name: Verify go.mod/go.sum are tidy run: | go -C core mod tidy -diff diff --git a/CLAUDE.md b/CLAUDE.md index 7248be0..086878d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ just all # Build everything just test # Run all tests just lint # Run linters just proto # Regenerate protobuf code -just vuln # Scan Go modules for known vulnerabilities (needs `just proto` first) +just vuln # Scan Go modules for known vulnerabilities ``` ## Architecture @@ -42,7 +42,9 @@ just test-mcp # MCP server only The daemon and mcp modules depend on core via `replace` directives pointing to `../core`. Because they resolve core's full dependency graph through those directives, a dependency change that shifts core's transitive versions (e.g. a Dependabot bump) leaves their `go.sum` stale. Run `just tidy` to reconcile all three modules after any such change. -CI verifies this rather than trusting it: `test-and-lint` runs `go mod tidy -diff` per module, so a missed reconciliation fails the build instead of sitting latent. +`just tidy` regenerates protobuf code first, as a prerequisite declared on the recipe rather than left to the caller. Generated code under `daemon/gen/` is not committed — absent on a fresh clone and after `just clean` — and `go mod tidy` cannot load the packages importing it, so without it the reconciliation fails outright rather than producing anything. See `.claude/rules/build-and-migrations.md` for the rule covering every target with that requirement. + +CI verifies this rather than trusting it: `test-and-lint` runs `go mod tidy -diff` per module — after `buf generate`, for the same reason — so a missed reconciliation fails the build instead of sitting latent. ## CI Notes diff --git a/README.md b/README.md index 974298f..e7d2f88 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ just build-app # Build the Qt desktop app just test # Run all Go tests just test-app # Build and run the Qt app's Qt Quick Test suite just lint # Run golangci-lint on all modules -just vuln # Scan Go modules for known vulnerabilities (run just proto first) +just vuln # Scan Go modules for known vulnerabilities ``` ## Running Locally diff --git a/justfile b/justfile index 1524082..b79dc08 100644 --- a/justfile +++ b/justfile @@ -7,11 +7,11 @@ proto: buf generate # Build the daemon binary -build-daemon: +build-daemon: proto cd daemon && go build -o finch-daemon . # Build the MCP server binary -build-mcp: +build-mcp: proto cd mcp && go build -o finch-mcp . # Build the Qt app @@ -34,11 +34,11 @@ test-core: cd core && go test ./... # Run daemon tests -test-daemon: +test-daemon: proto cd daemon && go test ./... # Run MCP server tests -test-mcp: +test-mcp: proto cd mcp && go test ./... # Run golangci-lint on all Go modules @@ -47,25 +47,24 @@ lint: lint-core lint-daemon lint-mcp lint-core: cd core && golangci-lint run ./... -lint-daemon: +lint-daemon: proto cd daemon && golangci-lint run ./... -lint-mcp: +lint-mcp: proto cd mcp && golangci-lint run ./... # Scan all Go modules for known vulnerabilities. -# Needs generated proto code — run `just proto` first, or the daemon and MCP scans -# fail to load packages (both import the gitignored daemon/gen). # Reports only advisories reachable from finch's own code; CI enforces the same check. +# For the complementary enumerating scan, see `.claude/rules/ci.md`. vuln: vuln-core vuln-daemon vuln-mcp vuln-core: cd core && govulncheck ./... -vuln-daemon: +vuln-daemon: proto cd daemon && govulncheck ./... -vuln-mcp: +vuln-mcp: proto cd mcp && govulncheck ./... # Format all Go code @@ -75,7 +74,7 @@ fmt: cd mcp && go fmt ./... # Reconcile go.mod/go.sum across all Go modules (run after a dependency change) -tidy: +tidy: proto cd core && go mod tidy cd daemon && go mod tidy cd mcp && go mod tidy