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
4 changes: 3 additions & 1 deletion .claude/rules/build-and-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion .claude/rules/ci.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <core|daemon|mcp> -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
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/ci-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Comment on lines +45 to +47

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1969122. An occurrence scan found the same claim in README.md's quick reference too, so both lines are corrected.


## CI Notes

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 10 additions & 11 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down