3 Commits

Include renames

Author	SHA1	Message	Date
J3vbandClaude	9eba6969d2	B1-5: ownership moves (RL-09 / L-09, RL-10 / L-10, RL-11 / L-11, RL-13 / L-12) (#1417) ... * refactor: move the protocol schema to protocol/schema.json (RL-09) The WebSocket message-type schema is the one artifact in this repository that neither component owns: `Server/ws/message_types.go` and `Client/src/lib/protocolTypes.ts` are both generated from it, and neither may be hand-edited. It nonetheless lived at `docs/protocol-schema.json` — filed under the directory for prose, whose own README calls it "Reference" material — and its generator lived at `Server/scripts/genprotocol/`, i.e. inside one of the two consumers. Ownership was legible from neither location. The obvious fix — move the generator to the repository root alongside the schema, so the whole tool is at the cross-component boundary — is wrong here. The generator is a Go `package main`, and Go modules are directory-rooted: `Server/go.mod` roots at `Server/`, so a root-level Go program needs a second module or a `go.work`. That second module would sit outside every path filter this repository already has — `golangci-lint` runs with `working-directory: Server/` (ci.yml), `go vet ./...` runs from `Server/` (scripts/run.mjs, .githooks/pre-commit), `.githooks/pre-commit` selects Go files with `^Server/.*\.go$`, `.githooks/pre-push` sets `server_changed` on `^Server/`, setup-go caches on `Server/go.sum`, and dependabot has one gomod block for `/Server`. Six gates would silently stop covering the generator, each failing open. The schema is data and moves freely; the generator is Go and stays where the Go toolchain already runs. Done instead: - `docs/protocol-schema.json` -> `protocol/schema.json`. A new top-level `protocol/` is the cross-component boundary, with a `README.md` naming the two generated consumers, the one command, and the four gates. - `Server/scripts/genprotocol/` -> `Server/cmd/genprotocol/`, the module's conventional home for an executable. This also empties `Server/scripts/` of Go entry points except `seed.go`, which RL-10 moves next. - `Server/cmd/` added to `Server/.dockerignore` and `Server/.air.toml`, which both already excluded `Server/scripts/`. Without this the move would have silently widened the Docker build context and the air watch set. 27 files, 115 insertions, 76 deletions. Two runtime path resolvers re-pointed (`cmd/genprotocol/main.go:41` `-schema` default, `ws/protocol_contract_test.go:67` `filepath.Join`); two git-hook grep patterns (`pre-commit:53`, `pre-push:57`); eight generator call sites across five files (Makefile x2, scripts/run.mjs x2, pre-commit x2, ci-check skill, bughunt-fix.js); two broken relative markdown links (docs/README.md:47, docs/protocol.md:1497); two generated files regenerated, header lines only, zero constants changed; two ledger prose hits plus a `render-ledger.mjs` re-render. No new verify was written: the regenerate-and-diff check is already enforced three times (CI `make protocol-verify`, `.githooks/pre-commit`, `npm run check:server`) and `ws/protocol_contract_test.go` independently checks the schema against the constants a fourth time. Verified: both directions, for both resolvers. With `protocol/schema.json` removed, `go test ./ws/ -run TestProtocol` fails with `reading protocol schema at /home/user/OwnCord/protocol/schema.json: no such file or directory` (two tests) and `go run ./cmd/genprotocol` exits 1 with `read schema: open ../protocol/schema.json: no such file or directory`; with the file restored both pass. So the new path is genuinely resolved, not merely spelled in a comment. The hook patterns were exercised directly: the pre-commit pattern matches `protocol/schema.json` and `Server/cmd/genprotocol/main.go` and no longer matches `docs/protocol-schema.json`; the pre-push pattern matches `protocol/schema.json`. `go run ./cmd/genprotocol` twice in a row leaves `git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts` clean, so the committed outputs are exactly what the generator emits. `go build ./...` and `go vet ./...` pass; `npx prettier --check .`, `npm run typecheck` and `npm run lint` pass; `node .superpowers/render-ledger.mjs --check` reports 348 findings valid. Not included: the four dated `docs/audit-*.md` files, the older `docs/plans/*`, and `CHANGELOG.md` keep the old path — they are point-in-time records, and `.prettierignore` and `scripts/check-doc-counts.mjs` already treat them as deliberately unmaintained. The B1 plan itself keeps its own wording, since it states intent rather than current state. `Server/scripts/` is not deleted: it still holds `seed.go` (RL-10), `k6/`, `toxiproxy/` and two shell scripts. `Server/telemetry/metrics.go:19` declares a scope for a `Server/voice` package that does not exist — spotted here, unrelated to this move, left for RL-13's sweep to carry forward verbatim rather than fixed inside a relocation. No `seed:` Make target was added. Refs RL-09, L-09 * refactor: move the seed tool under Server/cmd/seed (RL-10) `Server/scripts/seed.go` was a `package main` sitting directly in `Server/scripts/`, which made `Server/scripts` itself one of the module's three main packages — a developer tool in the module's build graph under a directory name that says "loose scripts". It also did filesystem work in `func init()`: `os.MkdirAll("data", 0o750)` ran before `flag.Parse()`, so the directory appeared even when the tool immediately refused to run. The audit row (RL-10) claims that `init()` fires "during test discovery". It does not, and the obvious fix aimed at that claim would be aimed at nothing: `Server/scripts/` contains zero `_test.go` files, so Go never builds a test binary there and `go test ./...` never runs the `init()`. The residual defect is narrower and real — an untagged `package main` in the build graph, plus a side effect on a path (`go run ./cmd/seed -h`) that has nothing to do with tests. Done: - `Server/scripts/seed.go` -> `Server/cmd/seed/main.go`, joining `cmd/genprotocol/` from RL-09. `Server/scripts/` now holds shell and JS tooling only (docker-smoke.sh, k6/, toxiproxy/, voice-test.sh) and no Go entry point at all. - The `os.MkdirAll` moved out of `init()` to immediately before `db.Open` in `main()` — the one call that needs the directory, since `db.Open` -> `OpenWithMaxReaders` -> `openFile` creates no intermediate directories. - The package doc comment's usage lines were wrong in two ways, not one: they named `go run scripts/seed.go`, which no longer exists, and they omitted the mandatory `-confirm-dev`, so neither documented command could ever have run. Both corrected, and `seed.go is a standalone tool` became the conventional `Command seed populates ...`. - `Server/CLAUDE.md`'s Layout list now names `cmd/` and states that no Go entry point lives in `scripts/`. Two files, 20 insertions, 17 deletions. `go list` main packages go from `{server, server/cmd/genprotocol, server/scripts}` to `{server, server/cmd/genprotocol, server/cmd/seed}` — the count is unchanged at three, which is the honest framing: this relocates a main package to a conventional path, it does not remove one from the build graph. Verified: both directions, by building the pre-change file and the post-change file and running each in a fresh empty directory. Before, `seed` with no flags exits 1 *and leaves a `data/` directory behind*; `seed -h` exits 0 and also leaves `data/` behind. After, both exit the same way and create nothing — `data/ exists=NO` in each case. The happy path is unchanged: `seed -confirm-dev` in an empty directory creates `data/` at mode 0750, writes `data/chatserver.db`, and reports 4 users / 5 channels / 31 messages; a second run reports 0 new rows, so idempotence survives. The old documented invocation now fails loudly (`go run scripts/seed.go` -> `stat scripts/seed.go: no such file or directory`) and the new one is what the comment says. All four build-tag variants compile, `go vet ./...` passes, `gofmt -l` is clean outside `db/dbgen`, and `npx prettier --check .` passes. Behaviour delta, called out rather than left silent: the two cases above (`-h`, and a missing `-confirm-dev`) no longer create `./data`. That is a change, not a pure relocation. It is the change RL-10 asks for — the remedy text is "remove import/test-time filesystem side effects" — and the alternative that preserves the old behaviour exactly, making the `MkdirAll` the first statement of `main()` before `flag.Parse()`, would keep precisely the side effect the item exists to remove. Not included: `Server/scripts/genprotocol` was moved to `Server/cmd/` by the RL-09 commit rather than here, so the "executable tooling under conventional command ownership" class is closed across the two commits, not this one alone. `filepath.Dir(*dbPath)` was evaluated for the `MkdirAll` and rejected: it would fix a real gap (`-db /elsewhere/x.db` still creates a useless `./data` and does not create `/elsewhere`) but it means creating an arbitrary directory from CLI input, and that is a behaviour change past "shift it out of `init()`" — worth its own item. No `make seed` target was added, and the dated `docs/audit-*.md` rows naming `Server/scripts/seed.go` keep the old path. The findings ledger has zero references to this file, so no re-render was needed. Refs RL-10, L-10 * test: give the cross-stack contracts a named tier (RL-11) `Client/tests/unit/admin-static-channel-perms.test.ts` reads and executes `Server/admin/static/index.html`. Filed under `tests/unit`, nothing about its location or name said it locks a server-owned artifact, so a Go developer editing the admin SPA got a red check called "Client Unit Tests" with no clue why. The register describes this as one file. It is not, and the measured set does not match the description in either direction: - Client -> Server: exactly ONE test crosses by filesystem read, not two. `main-page.test.ts` was named in the plan but only carries a prose comment citing `Server/admin/update_handlers.go:181` at line 1046 — no read, no import, nothing to move. - Server -> Client: the four tests the plan named do not cross. `waf_test.go`/`waf_crs_test.go` set a `User-Agent: OwnCordClient/1.0` literal that appears nowhere under `Client/`; `ws_integration_test.go:289` and `sanitize_content_fuzz_test.go:46` are comments. The real crossing is one the register never named: `Server/updater/updater_test.go:630` does `os.ReadFile` on `Client/src-tauri/tauri.conf.json`. The obvious fixes are both wrong. Moving the invariant "to the owning server test" cannot work: `Server/go.mod` carries no JavaScript engine (no goja, otto, v8go, quickjs, rogchap, duktape), so a Go port could only assert at the text level like `admin/perm_grid_test.go` does — and that is not a substitute. Flipping the guard at `admin/static/index.html:1182` to `targetIsTouchedRole=false` reintroduces OC-0154 in full while leaving every greppable identifier intact, so a text-level test passes on a broken file. Relocating it to the e2e admin journey is worse: that job is `continue-on-error: true` and deliberately unpinned ("requiring it is theatre" — `docs/plans/b0-dev-branch-protection.sh`), so it would convert a blocking, pinned gate into one that is green regardless. And the journey does not cover the invariant today: `grep -Eic "perm|access|role|override|matrix"` over its 142 lines returns 0, so the "if e2e already covers it, delete" branch never fires. Done — one tier, applied to the whole set, defined by artifact coupling and placed by runtime capability: - New `Client/tests/contract/`, holding `server-admin-static-channel-perms.test.ts`. Same directory depth, so `../../../Server/...` still resolves; the body is byte-identical apart from a header naming the owner and the runner. - `Server/updater/tauri_key_contract_test.go` splits the one cross-component Go test out of `updater_test.go` verbatim, same `package updater`. It stays in Go — placement follows capability, and Go parses JSON fine — so only the file name has to declare the crossing. Without this the item would have been "moved one file and declared the class closed". - `npm run test:contract`, and the tier, the membership rule and a blocking/non-blocking table in `docs/contributing.md#testing`, which previously described no tiers at all. - `Client/CLAUDE.md`'s tier list was missing `tests/e2e/admin` and `tests/e2e/native` before this; it now lists all seven and states the rule. `Server/CLAUDE.md` records why the SPA's execution-level invariant is locked from the client tree, so nobody "fixes" it into a regex. - Ledger `OC-0154.fix.test` re-pointed and `FINDINGS.md` re-rendered; `.claude/workflows/bughunt.js` — the workflow that produced OC-0154 — no longer describes the TS test surface as `tests/unit/*.test.ts` only. - Three stale cross-stack pointers of exactly the class this item is about: `tests/e2e/helpers.ts:348,351` and `tests/unit/types.test.ts:13` named `docs/brain/06-Specs/PROTOCOL.md`, which does not exist (`docs/brain/` is a gitignored path); all now name `docs/protocol.md`. 15 files, 125 insertions, 33 deletions. No CI job, workflow, vitest, tsconfig, eslint, knip or stryker change, and no new pinned check — `ci.yml`'s `npx vitest run --coverage` has no path filter and `vitest.config.ts` includes `tests/**/*.test.ts`, so enforcement after the move is bit-identical to enforcement before it. That is deliberate: `dev` pins 11 contexts and a 12th is a branch-protection API write, not something a PR can do, so any new job would be advisory until someone separately changed repository settings — strictly less protection than today. Verified: both directions, and the assertion was not weakened. Flipping `admin/static/index.html:1182` to `const targetIsTouchedRole=false;` makes the moved test fail (`AssertionError: expected 'DELETE' not to be 'DELETE'`); `git checkout` of that file makes it pass again — so the invariant survived the move intact rather than becoming a test that passes anywhere. The split Go test's cross-boundary read is live too: with `Client/src-tauri/tauri.conf.json` moved away, `go test ./updater/` fails with `ReadFile(../../Client/src-tauri/tauri.conf.json): no such file or directory` from `tauri_key_contract_test.go:20`, and passes once restored. The full client suite is 192 files / 5257 tests passing, identical to the count before the move; `npm run typecheck` passes, which proves `tests/contract/` is inside the tsconfig graph and that `tests/types/jsdom.d.ts` still resolves the moved test's `import { JSDOM }`. `npm run lint`, `npx prettier --check .`, `go vet ./...` and `go test ./updater/` all pass. `git grep "tests/unit/admin-static-channel-perms"` finds no survivor outside the B1 plan itself. Not included: nothing was deleted, because no e2e sibling covers OC-0154. `Client/tests/types/jsdom.d.ts` was neither moved nor deleted — it is still the only type source for the moved test's `jsdom` import. `capabilities-scope.test.ts` and `tauri-conf-webview2-args.test.ts` read `src-tauri/` and stay in `tests/unit`: `src-tauri` is inside the `Client` component, so they are not contract tests, and the rule earns that rather than hand-waving it — moving them would have forced repoints of ledger entry OC-0089 and `docs/security.md:64` for no gain. Each gained a one-line header saying why. `Server/admin/perm_grid_test.go` and `emoji_section_test.go` read their own package's embedded asset and are unchanged; they are the text-level complement to the execution-level test, not duplicates. No JS engine was added to `go.mod`, no npm root was created under `Server/`, and no root-level `tests/` tier was created — there is no runner for one and no way to make it blocking from a PR. Separately noticed and NOT fixed here: `docs/contributing.md:221` still says "All ten required checks" while `docs/plans/b0-dev-branch-protection.sh` pins eleven since B1-3 added `Repository Hygiene`, and `docs/plans/hp-0-scorecard-2026-08-25.md:109` is stale the same way — that is the branch-protection item's to fix, not this one's, and one register item per commit. Refs RL-11, L-11 * refactor: rename the Go module to github.com/J3vb/OwnCord/Server (RL-13) `Server/go.mod` declared `github.com/owncord/server` while the public repository is `github.com/J3vb/OwnCord`. Nothing resolves that path — there is no `owncord` GitHub org and no vanity-import host serving go-import metadata for it — so every import line in the tree named a location that does not exist. It compiles because a main module's own path is never fetched, which is exactly why it went unnoticed. The obvious fix — an AST-aware import rewriter (`gomvpkg`, `go mod edit`) — is wrong here, and provably so. Six of the 722 occurrences are not imports at all: `api/main_test.go:20` (a goleak `IgnoreTopFunction` pattern), `telemetry/metrics.go:17-19` (three OTel instrumentation-scope names), `invariants/syncutil_locks.go:73` (a diagnostic message), and `invariants/syncutil_locks_test.go:56` (an import line inside a raw-string Go fixture). An import rewriter touches none of them, and the compiler cannot see any of them either. Done as one scripted substitution over `git ls-files`, anchored on the full `github.com/owncord/server` string. The anchor matters: `owncord-server` is a different identifier — the OTel `service.name` (`config/config.go`, `telemetry/telemetry_otel.go`) and the GHCR image name (`.github/workflows/release.yml`, `docker-compose.yml`) — and a looser pattern would have moved it. It is untouched: 10 occurrences across 9 files, before and after. 350 files, 728 insertions, 728 deletions. 722 occurrences in 344 Go files, plus `go.mod:1`, the `sed` at `Makefile:67`, `Server/CLAUDE.md:3`, `docs/architecture/server.md:5`, and the ledger pair (`findings-ledger.json:3758` plus a `render-ledger.mjs` re-render of `FINDINGS.md`). Zero in any workflow, zero in the Dockerfile, zero in `Server/.golangci.yml` (no `local-prefixes`, `gci`, `importas` or `depguard` rule keys on the module path, so import grouping is not configured anywhere). The plan's blast-radius estimate missed one thing, and it is the one that would have gone red: **gofmt**. `J` (0x4A) sorts before every lowercase letter, so in the 36 files where a module-local import shares a contiguous group with a third-party one, the module's imports must move above `github.com/go-chi/...`. `gofmt -l` was clean before the substitution and listed exactly 36 files after it; `gofmt -w` on those 36 restores it to clean. `gofmt` is an enforced gate — the `formatters` block in `Server/.golangci.yml`, which is S-05 — so a substitution-only commit fails Lint. Verified: both directions, and the line accounting is exact. Every added line in this diff contains the new module path (728) and every removed line contains the old one (728); the count of changed lines containing neither is **zero**, so the gofmt re-sort moved module-path lines only and touched no third-party import. The residual check (`git ls-files -z | xargs -0 grep -n 'github\.com/owncord/server'`) returns exactly two hits, both deliberately out of scope: the RL-13 row in `docs/audit-2026-08-23-repository-layout.md` and the measurement row in this phase's own plan. The compiler-invisible half was proven by reverting *only* `api/main_test.go:20` to the old path on the otherwise-renamed tree: `go build ./...` and `go vet ./api/` both still pass — they see nothing wrong — while `go test ./api/` FAILS, because the runtime function name now carries the new path and goleak stops ignoring `ws.(*Hub).Run.func1`. Restoring the line makes it pass. `go.sum` is byte-identical (no `go mod tidy` was run and none was needed). All four build-tag variants compile; `go vet ./...`, `go vet -tags otel,wazero ./...` and `go vet -tags deadlock ./...` pass; `go test -race ./...` is 16/16 packages green; `go test -tags deadlock ./...` passes; the tag-gated `./plugin/...` (wazero) and `./telemetry/...` (otel) runs pass. `golangci-lint` v2.11.3 — the pinned CI version, rebuilt locally against Go 1.26 because the packaged binary cannot load a 1.26 config — reports **0 issues**. `go run ./cmd/genprotocol` leaves `git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts` clean, so the rename does not reach the generated protocol constants. `npx prettier --check .` and `node .superpowers/render-ledger.mjs --check` pass. Not included: `docs/audit-2026-08-23-repository-layout.md` and `docs/plans/b1-repository-foundation-2026-08-25.md` keep the old path — they are the audit row and the measurement that motivated this change, and rewriting them would erase the record of what was measured. They are why the residual check needs a two-path allowance rather than being empty; that allowance is stated above rather than hidden in a pathspec. `telemetry/metrics.go:19` declares `scopeVoice` for a `Server/voice` package that does not exist; the substitution carried the dead path forward verbatim as `github.com/J3vb/OwnCord/Server/voice` rather than fixing it, because correcting a real observability bug inside a mechanical rename would hide it in a 350-file diff. It needs its own item. No `go.work`, no second module, and no vanity-import host was set up — the new path resolves against the real repository, but nothing imports this module as a library, so `go get` reachability was not exercised either way. Refs RL-13, L-12 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-27 07:11:18 +02:00
J3vbandClaude Opus 5	2a37f386f9	B1-3: repository hygiene gates (RL-19 / L-13, S-05) (#1414) ... * chore(format): one Prettier config at the repository root Every formatting rule in this repository lived under Client/ and covered exactly two globs: Client/src/**/*.ts and Client/tests/**/*.ts. Root Markdown, all of docs/, every YAML and JSON, all CSS, the root scripts and tools/mcp-introspect were formatted by nothing. There was no .editorconfig. The obvious fix -- a second Prettier config at the root for "everything else" -- gives two configs and two ignore files that can silently disagree about the same file. So the root takes ownership instead: config, ignore file and gate move up, and Client/ folds in. Client's inline "prettier" block, its .prettierignore, its format/format:check scripts and its now-unused prettier devDependency are all deleted; knip would have failed client-check on that last one. The .prettierrc.json values are lifted byte-for-byte from Client/package.json, which is what keeps the reformat commit free of client TypeScript churn: 87 tracked files need reformatting and not one of them is under Client/src or Client/tests. .prettierignore carries only what .gitignore does not. Prettier 3 reads the root .gitignore by default, so node_modules/, dist/, coverage/, Client/src/generated/ and docs/security-findings/ need no entry. It does NOT read nested .gitignore files, which is why .remember/ is listed explicitly -- 38 untracked per-machine scratch files were otherwise able to turn a shared gate red. graphify-out/ is listed because its seven files are tracked and .graphify_labels.json is signed byte-for-byte by its .sig, so formatting it would silently invalidate the signature. check:hygiene is registered in scripts/run.mjs and folded into check and release:preflight. It deliberately contains no `gofmt -l` step: gofmt -l prints offenders and still exits 0, so it cannot fail a build. Go formatting is enforced separately. shellcheck and actionlint take their file lists from `git ls-files`, never a filesystem glob -- .claude/worktrees/ holds a gitignored pre-flatten copy of the tree with three .sh files a glob would happily lint. This commit leaves the tree non-conformant on purpose. The reformat is the next commit, so the 87-file diff is reviewable separately from the rule that caused it. Not included: editorconfig-checker. .editorconfig is the editor baseline the audit asked for; Prettier, gofmt and rustfmt already fail CI on the same indentation and newline rules, so a fourth tool checking them again is a gate with no failure mode of its own. Verified: `npx prettier --check .` names 87 tracked files and zero untracked ones; the same command listed 38 .remember/ scratch files before the ignore entry and none after. `node scripts/run.mjs --list` resolves check:hygiene to 8 shell targets and 4 workflow targets. Both package.json files parse. Refs RL-19 / L-13, S-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(format): reformat the tree to the repository Prettier rules Mechanical. This commit is `npx prettier --write .` and nothing else -- the rule that caused it landed in the previous commit so this diff can be reviewed as a transformation rather than as 84 files of hunks. 84 tracked files: 54 Markdown, 7 .mjs, 7 JSON, 6 YAML, 4 .js, 3 CSS, 2 TypeScript (the two Playwright configs at Client's root, which the old Client/src + Client/tests globs never covered). No file under Client/src or Client/tests moves, because .prettierrc.json carries Client's former inline values byte-for-byte. Prettier rewrote 87 files, not 84. The three in .github/ISSUE_TEMPLATE/ had CRLF on disk and differ only in line endings, which .gitattributes (`* text=auto eol=lf`) already normalises, so their committed blobs are unchanged. Worth knowing before someone reconciles the two numbers. The largest single diff is .superpowers/findings-ledger.json at 7976 lines rewritten. That is safe to format: nothing writes the ledger programmatically -- render-ledger.mjs reads it and writes only FINDINGS.md -- so no tool will fight Prettier over its style on the next hunt. FINDINGS.md itself is ignored as generated. Verified: `npx prettier --check .` reports "All matched files use Prettier code style", so the pass is both complete and idempotent. All 7 reformatted JSON files were parsed before and after and compared as values: semantically identical, zero content changes. `node .superpowers/render-ledger.mjs --check` still reports 348 valid findings and leaves FINDINGS.md untouched. `node scripts/check-doc-counts.mjs` still passes its selftest and still agrees on 27 claims across 9 watched documents -- table realignment did not break the patterns it matches on. `node scripts/run.mjs --list` still parses. Refs RL-19 / L-13, S-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(lint): enforce Go formatting in the Server linter S-05: repository-wide Go formatting was not a required gate. The only gofmt enforcement anywhere was .githooks/pre-commit, which is opt-in per clone (`npm run hooks:install`), only sees staged files, and warns-and-skips when gofmt is off PATH. The obvious fix -- a `gofmt -l` step in CI -- does not work: `gofmt -l` prints its offenders and still exits 0, so the step passes no matter what it finds. scripts/run.mjs has the same problem, which is why check:hygiene has no Go step either. So gofmt goes where it can actually fail something: Server/.golangci.yml. The file was already `version: "2"` but had no `formatters:` block at all, so the 19 enabled linters ran with zero formatters. In v2 gofmt/gofumpt/goimports moved out of `linters.enable` into their own section with its own exclusions. Adding it there means the gate reports through the Lint step of "Server Build & Test", which is already pinned as required on dev -- no new job and no new pin. Every tracked .go file is under Server/ (551 of them, one go.mod), so Server-scoped is repository-wide here. One file was genuinely misformatted: a one-space struct field alignment in Server/admin/handlers_users_broadcast_test.go, fixed in the same commit because a single line does not need its own reformat commit. Trap worth recording: `gofmt -l .` on a Windows working tree lists every file that has CRLF on disk, because gofmt normalises line endings. That reported 18 offenders here, 17 of them ghosts. The blobs are all LF -- .gitattributes forces `eol=lf` -- so CI never saw them, and the honest test is to run gofmt over `git show HEAD:<file>` rather than the working copy. Doing that across all 551 tracked Go files found exactly the one real offender above. Verified both directions with golangci-lint v2 locally: `golangci-lint run ./...` reports 0 issues on the formatted tree; appending a misformatted function to Server/auth/constants.go produces 2 gofmt findings; appending the same misformatted function to Server/db/dbgen/admin.sql.go produces 0, so the exclusion holds. Both files restored and verified clean afterwards. Refs RL-19 / L-13, S-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scripts): escape the NUL separator instead of embedding one The `tracked()` helper added earlier in this branch splits `git ls-files -z` output on NUL. The separator was written as a literal NUL byte rather than the two-character JavaScript escape, so scripts/run.mjs became a binary file: `git diff` refused to show it, `grep` reported "Binary file matches" instead of the line, and `* text=auto` in .gitattributes stops normalising line endings for a blob it detects as binary. The code worked -- splitting on a raw NUL and splitting on "\0" are the same operation -- which is exactly why this is worth fixing before it is inherited. A source file that tooling classifies as binary is a file nobody can review. Verified: zero NUL bytes remain, `grep -n "split("` now prints line 50 instead of "Binary file matches", `node scripts/run.mjs --list` still resolves the same 8 shell and 4 workflow targets, and prettier still reports the file clean. * chore(lint): enforce Rust formatting Rust had no formatting gate of any kind: no rustfmt.toml, no `cargo fmt` anywhere in CI, in scripts/run.mjs, in the Makefile or in the git hooks. Clippy was the only Rust gate, and clippy does not check layout. `cargo fmt --all -- --check` now runs in the rust-tests job, ahead of clippy: a formatting failure is cheap to produce and cheap to fix, and there is no reason to spend a clippy pass to surface one. The stable toolchain in that job requested `components: clippy` only, so rustfmt is added there. Only that job. ci.yml has a second, byte-identical `Install Rust` block in tauri-build; it stays clippy-only, because a full desktop build is the wrong place to discover a misplaced brace. No rustfmt.toml. The default profile is the point of a baseline -- a config file here would be a second opinion about style with nothing to say. Client/src-tauri is a single `[package]`, not a workspace, so `--all` is a safeguard against a future member rather than a fan-out today. Verified: `node scripts/run.mjs --list` resolves check:rust to three steps with `cargo fmt --all -- --check` first, `npm run format` now also runs `cargo fmt --all`, and prettier reports ci.yml, run.mjs and the ci-check skill clean. `cargo fmt --all -- --check` currently fails on 13 files -- that is the reformat, and it is the next commit. Refs RL-19 / L-13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(format): reformat the Rust crate to rustfmt defaults Mechanical. This commit is `cargo fmt --all` and nothing else; the gate that demands it landed in the previous commit so this diff is reviewable on its own. 13 of the 17 tracked .rs files, +343/-164. The crate had never been formatted, so the changes are the usual first-run set: aligned trailing comments collapsed to single spaces, single-element slice literals folded onto one line, long method chains broken across lines, closure bodies expanded into blocks. Verified: `cargo fmt --all -- --check` is clean, so the pass is complete and idempotent. `cargo clippy --all-targets -- -D warnings` finishes with no warnings, and `cargo test --lib` reports 115 passed / 0 failed -- identical to before the reformat, which is what "mechanical" has to mean for a commit that touches this much of the crate. Refs RL-19 / L-13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scripts): make the root facade actually run on Windows Adding the first gate that a contributor would run from the repository root exposed two bugs in the facade, both of which made it silently wrong on the platform this project is developed on. 1. Every npm and npx step failed. bin() appends `.cmd` on Windows, but Node refuses to spawn a .cmd or .bat with shell:false -- the CVE-2024-27980 mitigation -- and fails with EINVAL and a *null* exit status. run.mjs only special-cased ENOENT, so the result was `FAILED: npx prettier --check . exited null` with nothing to explain it. check:client has three npm steps and has never been able to run here. Fixed by spawning only the npm shims through a shell. They are concatenated into a single command string rather than passed as an args array, because shell:true plus a separate array is deprecated (DEP0190) and prints a warning on every invocation; no argument in this file contains a space. 2. Every optional() step was skipped, always. onPath() shelled out to `where` on Windows, but where.exe lives in C:\WINDOWS\System32, which a Git Bash PATH does not necessarily contain -- on this machine PATH carries System32\Wbem, System32\WindowsPowerShell\v1.0 and System32\OpenSSH but not System32 itself. The probe could not start, `probe.status === 0` was false, and golangci-lint and sqlc reported as "not installed" while installed. Fixed by resolving against PATH and PATHEXT directly. No subprocess, and no dependency on which directories happen to be on PATH. A spawn error other than ENOENT now reports its code instead of surfacing as a null exit status. Verified: before, `node scripts/run.mjs check:hygiene` died with "exited null" and both optional steps printed SKIP with the tools present on PATH. After, the same command runs prettier, shellcheck and actionlint and prints "check:hygiene: passed", with no deprecation warning. `golangci-lint` is detected by the new onPath where the old one missed it. Refs RL-20 / L-14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(format): ignore build output that nested gitignores hide Prettier honours the root .gitignore and no other. Every build and scratch directory in this repository is ignored by a *nested* one -- Client/.gitignore, .serena/.gitignore, .superpowers/sdd/.gitignore -- so none of them were excluded from the new repository-wide gate. The effect is not subtle. Running `cargo test` once drops roughly 850 formattable files into Client/src-tauri/target/, and the hygiene gate goes from clean to "Code style issues found in 939 files". CI never sees it, because a fresh checkout has no build output; every contributor sees it on their second command. Mirrors the three nested files rather than inventing a list: dist, coverage, playwright-report, test-results, .vite, src-tauri/target and src-tauri/gen from Client/.gitignore, plus .serena/ and .superpowers/sdd/. node_modules needs no entry -- Prettier ignores it by default. Verified: `npx prettier --check .` reports "All matched files use Prettier code style" with a fully populated Client/src-tauri/target/ present on disk, and still names README.md when a misformatted table is appended to it. Refs RL-19 / L-13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(ci): shellcheck, actionlint, and a repository hygiene job The last two gates RL-19 asks for. Neither existed: the shell scripts were never linted, the workflows were never syntax-checked, and .githooks/pre-commit carried hand-written `# shellcheck disable=` directives that nothing had ever read. New `hygiene` job, ubuntu-only and root-scoped, modelled on docs-consistency for the same reason: every gate in it is platform-independent text analysis, and .gitattributes pins eol=lf so a second OS would only re-prove line endings. It runs `npm run check:hygiene` -- the same entry point a contributor runs, not a parallel copy of the commands. shellcheck ships in the runner image. actionlint does not, so it is pinned by version and verified by sha256: an installer script piped from a branch would be the one unverified download in a workflow file that pins every action by commit SHA. Prettier's step moves here from client-check, where it no longer belongs. Both linters found real defects. shellcheck, 3 findings in 8 scripts. Two are SC1125 errors in .githooks/pre-commit: `# shellcheck disable=SC2086 - repo paths contain no spaces` is not a valid directive. Trailing prose makes shellcheck discard the rest of the line, so neither suppression was ever in effect -- and one of the two was written earlier in this same branch, which is a fair demonstration of why the gate is worth having. The prose moves to its own line above. The third is SC2015 in start-server.sh, rewritten as an explicit if. actionlint, 5 findings, all inside `run:` blocks it shellchecks once shellcheck is on PATH. Three SC2015 in load-baseline.yml, rewritten as explicit ifs. Two SC2035 in release.yml, where `sha256sum *` should not become `sha256sum ./*`: the comment four lines above records that ParseChecksumFile exact-matches the last field, so a "./" prefix would strand every deployed server exactly as a "windows/" prefix would. `sha256sum -- *` satisfies the linter and leaves the output bytes identical. Verified all three gates in both directions with shellcheck 0.10.0 and actionlint 1.7.7 on PATH. Passing: `node scripts/run.mjs check:hygiene` prints "check:hygiene: passed" with all three steps run, not skipped. Failing: appending `bait_fn() { cat $1; }` to Server/scripts/voice-test.sh fails on SC2086; changing a runs-on to `ubunt-latest` fails on runner-label; appending a misformatted table to README.md fails prettier. All three files restored and confirmed clean afterwards. actionlint validates the new job in ci.yml itself. Refs RL-19 / L-13, S-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plans): record B1 progress through B1-3 The header still read "B1-0 is complete; B1-1 is the next step" three merged phases later. A plan that misstates where it is costs a reader the same confusion whether it is stale by one phase or three. B1-0 (#1410), B1-1 (#1411), B1-2 (#1412) and B1-3 (this branch) are done; B1-4, dependency automation, is next. Verified: `node scripts/check-doc-counts.mjs` still agrees on 27 claims across 9 watched documents -- this file is one of them -- and prettier reports it clean. * chore(ci): pin Repository Hygiene as a required check on dev The second half of S-05. Its acceptance criterion is "tree is formatted AND a fast required gate fails future drift" -- a check that runs but is not pinned lets a formatting regression merge, so the gate is not a gate until this lands. The name was read off PR #1414 with `gh pr checks` after the job reported `pass` in 26s, not copied out of ci.yml. That order matters: the B0 script records that three pinned names exist in no workflow file at all, and that a required check which never reports blocks every PR forever. Extends the existing script rather than adding a second one, per the B1 plan. Also records, in the "deliberately NOT pinned" list, that Docs & Ledger Consistency reports and passes on a dev PR yet is unpinned. That reads as an oversight from the 2026-08-25 pass rather than a decision, but it belongs to G-04, so it is documented here and not changed. NOT APPLIED YET. Running this script now would pin a check that PR #1413 cannot report -- its branch predates the hygiene job, so the job does not exist in its workflow file and the check would never arrive. Run it after #1414 merges; #1413 needs a rebase onto dev regardless. Verified: shellcheck clean, the embedded JSON parses, and `check:hygiene` passes with prettier, shellcheck and actionlint all running. Refs S-05, RL-14 / G-03. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 18:00:26 +00:00
J3vbandClaude	70473e8e10	B1-2: truth, entry points, and contributor path (#1412) ... * fix(hooks): guard on the command the hook actually runs pre-commit probed one binary and invoked another. The protocol block guarded on `command -v go` and then ran `make protocol-verify`; the sqlc block guarded on `command -v sqlc` and ran `make sqlc-verify`. `make` is not on PATH on a stock Windows box, so a contributor with Go installed but no make had their commit rejected with pre-commit: FAIL: protocol constants are stale — run 'make protocol-generate' in Server/ and stage the result when nothing had been generated and nothing compared. The real cause was `make: not found`, and the advice the message gives fails the same way. Rather than add a `command -v make` guard, inline what the two Makefile targets reduce to — `sqlc generate` / `go run ./scripts/genprotocol` followed by `git diff --exit-code`. Same semantics, one less prerequisite, and it doubles as the make-free equivalent B1-2 asks for. The Makefile targets stay for anyone who prefers them. Also: the protocol block was the only one with no `else`, so a contributor without Go got no check and no notice. It now warns like its two siblings. And gofmt is a separate binary from go, so it is probed separately. Verified both directions with the hook body replayed verbatim: - Go present, make absent -> passes, no staleness claimed. - schema edited without regenerating -> fails, as it must. Refs RL-20 / L-14. * docs: state one branch and PR model Active documents contradicted each other head-on. README.md and docs/contributing.md said branch from `dev` and target `dev`; CLAUDE.md said branch from `main`, PR to `main`. That is R-02, and the 2026-08-19 audit had already recorded it as D-06 without it being resolved. `dev` is the answer, and the repository already behaves that way: B0 made `dev` PR-only with ten required checks enforced on admins, and #1409, #1410 and #1411 all landed there. `main` carries releases. docs/contributing.md becomes the single source of truth. It now states the model, what protection is actually applied, and the two consequences a contributor meets on their first PR — that a self-mergeable PR still cannot merge red, and that Docker and Tauri Full Build report as skipped against `dev` rather than failing. Everywhere else summarises and links here. - CLAUDE.md: corrected, with a link rather than a second copy. - CONTRIBUTING.md: new. GitHub's contributing-guidelines affordance only resolves the root, .github/ or docs/ — `docs/contributing.md` is not a path it finds, so the link never appeared on issues or PRs. - PULL_REQUEST_TEMPLATE.md: names the base branch, which it did not. - bughunt-run skill: reviewed the branch against `origin/main`, which is the wrong base once every PR targets `dev`. README.md already said `dev` and is left as the short summary it should be. Dated audits and the historical remediation plan keep their `main`-era wording — they are records. Refs R-02. * fix(hooks): pick the pre-push base from the nearest integration branch pre-push decided which side's gates to run from `git diff --name-only origin/main...HEAD`. That was right when everything targeted `main`. Once `dev` became the integration branch it stopped being right: a branch cut from `dev` diffed against `main` counts everything on `dev` and not yet on `main` as "changed". Measured on this branch: the old base reported 609 changed files, the new one reports 6. So in practice the hook was running the full server build matrix and the client typecheck plus eslint on every push, whatever the change touched — the file-based narrowing it exists for never engaged. Now it picks whichever of origin/dev, origin/main is nearest, by commits between merge-base and HEAD, skipping a candidate that scores 0. Verified: a branch cut from dev picks origin/dev (2 ahead); dev itself scores 0 against dev and picks origin/main (8 ahead), which is what a dev -> main release PR wants. With no candidate resolvable it falls back to the existing `__all__`, so an unfetched or shallow clone still runs everything. Refs RL-20 / L-14, R-02. * chore(node): one Node source of truth `.nvmrc` and all ten `actions/setup-node` pins said 24; five active documents and the repo's only `engines` block still said 20. A contributor following the docs installed a version CI does not run. Node 24 wins — it is what CI already runs. Every manifest now declares `engines`, and `engine-strict=true` turns a wrong major into a failed install rather than an `EBADENGINE` warning nobody reads. `>=24` rather than `^24` so a Node 26 box keeps working; `Client/.nvmrc` stays the human-facing pin and the docs point at it instead of restating a number. The `.npmrc` is per package root, not one at the top. npm reads the project `.npmrc` from the package directory and does not walk parents — verified with a throwaway package requiring node >=99: with only a parent `.npmrc` npm warned and exited 0; with one in the package directory it failed `notsup`. A single root file would have left `Client/`, the package that matters most, on warnings. Five docs, not the four previously identified — `docs/mcp-introspect.md` also said 20. And `docs/contributing.md` claimed "`.nvmrc` + CI both say Node 20", which was wrong about both. Verified both directions in all three package roots: Node 22 fails `notsup`; Node 24 installs clean and `npm ci` passes in Client/. Refs RL-17 / C-01, ENV-01. * docs: add the documentation landing page `docs/` had 24 top-level files and no index. The root README carried a flat list of 22 links that had drifted: six documents were reachable from nowhere at all — including both 2026-08-23 audits and the test audit — and two entries were labelled "latest" while newer unlinked audits existed. docs/README.md is the index RL-12 asked for. It groups by what a document *is*, because that is what decides whether to trust it: guidance tells you how to do something, reference describes a contract the code implements, audits are dated snapshots nobody updates, plans record intent. Every tracked file under docs/ now appears exactly once, and the audit table says plainly that audit-2026-08-19.md still claims "0 open findings" when the ledger has 38. The root README keeps a short curated list and defers to the index, rather than maintaining a second copy that drifts again. Two fixes while there: `docs/plans/` was linked as a bare directory, unlike its two sibling directory entries, and was annotated "each carries a verified status header" — which docs/plans/README.md:7-9 explicitly contradicts, since a plan's header is exactly the thing that drifts and the index is the authority. Verified: 78 relative links across the new and edited files resolve, and no tracked docs/ file is unreachable from the index. Refs RL-12 / R-06. * feat(scripts): root command facade Entry points existed only inside Server/ (a Makefile) and Client/ (npm scripts). Nothing at the root told a new contributor where to start, and the root package.json had three scripts, none of which built or tested anything. `npm run check` from the root now runs what CI gates on, and check:server / check:client / check:rust run one stack. scripts/run.mjs is dependency-free Node — the shape render-ledger.mjs already uses — so `npm run check` works before `npm install` has. Cross-platform by construction: every step is spawned with an explicit cwd and no shell, so there is nothing to quote and no `cd &&` to behave differently on Windows. npm and npx get their .cmd suffix there. No step shells out to make. The facade orchestrates; it is not a new required path. Each step prints the command and the directory before running it, and those are exactly the commands documented per-stack — so a server contributor can read the output and type them instead, and still never needs Node. Tools CI installs but a contributor may not have (golangci-lint, which has no wrapper in this repo at all; sqlc, pinned by Server/sqlc.version) are skipped with a printed reason rather than failing. Three corrections to the ci-check skill while aligning it: - `make sqlc-verify protocol-verify` replaced by what those targets reduce to, so the documented path does not require make either. - `cargo test` -> `cargo test --lib`, which is what ci.yml actually runs. - "NODE_OPTIONS=--no-experimental-webstorage is mandatory on Node 22+" was false. tests/setup.ts installs the shim, CI runs Node 24 without the flag, and the suite was measured passing without it — 192 files / 5257 tests, identical to the flagged run. Also documents the third RL-20 problem, which needed no code: core.hooksPath is exclusive, so `npm run hooks:install` silently disables any .git/hooks/post-commit — including the one `graphify hook install` writes, which CLAUDE.md tells agents to install. Nothing warned about that. Verified: check:client 5257/192 green, check:rust 123 tests + clippy green, --list prints every command, and the optional-tool skip path reports rather than fails. Refs RL-04 / L-04, RL-20 / L-14. * feat(ci): fail on a document that contradicts the findings ledger G-04's remaining half. The ledger is the source of truth for defect counts, but nothing stopped a planning document from stating a different number and nothing noticed when one did. `render-ledger.mjs --check` cannot help: it validates the JSON schema and returns before rendering, so it never reads FINDINGS.md and cannot see drift at all — and no workflow ran it anyway. scripts/check-doc-counts.mjs counts ledger statuses and compares them to what an allow-list of active documents claims, failing with file, line, claimed value and actual. Wired into ci.yml as a job with no npm ci, since the script imports nothing outside node:, and into the facade as `npm run check:docs` — first in `check`, so a contradicted count does not wait behind ten minutes of -race. The patterns are narrow on purpose. A first attempt matched any "<number> <status>" and flagged nineteen things, all false: "the 45 open P1 rows" (issue-register rows, not ledger findings), "All 8 findings F1-F8" (a different register), "G-05 **refuted**" (an identifier), `">=20"` and `CGO_ENABLED=0` (not counts at all). A check that cries wolf gets ignored, which is the failure G-04 already describes. So a number is only read as a claim in three shapes that cannot mean anything else: an enumeration of two or more "<n> <status>" pairs, a status table row in a table that totals itself, and "<n> records/findings" where the ledger is named within three lines. Fifteen selftest assertions pin both directions, and the job runs them before it runs the check. It reads findings-ledger.json directly rather than importing render-ledger.mjs for `validate`/`render`: that module ends in a bare top-level `await main()` with no import.meta.main guard, so importing it rewrites FINDINGS.md as a side effect. Dated docs/audit-*.md are reported, never failed — they are snapshots nobody maintains. audit-2026-08-19.md does claim zero open findings against 38 open, so b0-baseline's "No plan was found claiming '0 open findings'" holds for docs/plans/ but not for docs/. Not included: a real FINDINGS.md render-drift check. That is RL-07 and belongs with the generated-artifact work, not here. Verified: 27 claims across 9 documents agree; corrupting one count in docs/plans/README.md fails the check naming that line, for both the status and the total. Refs G-04. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-26 10:39:24 +02:00