From 9eba6969d2d829b9ea5092329b1dd949588c3cac Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Thu, 27 Aug 2026 07:11:18 +0200 Subject: [PATCH] B1-5: ownership moves (RL-09 / L-09, RL-10 / L-10, RL-11 / L-11, RL-13 / L-12) (#1417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .claude/skills/ci-check/SKILL.md | 2 +- .claude/skills/protocol-change/SKILL.md | 8 +- .claude/workflows/bughunt-fix.js | 2 +- .claude/workflows/bughunt.js | 4 +- .githooks/pre-commit | 6 +- .githooks/pre-push | 2 +- .github/workflows/ci.yml | 2 +- .superpowers/FINDINGS.md | 8 +- .superpowers/findings-ledger.json | 8 +- CLAUDE.md | 2 +- Client/CLAUDE.md | 7 +- Client/package.json | 1 + Client/src/lib/protocolTypes.ts | 4 +- ...server-admin-static-channel-perms.test.ts} | 8 ++ Client/tests/e2e/helpers.ts | 6 +- Client/tests/types/jsdom.d.ts | 6 +- Client/tests/unit/capabilities-scope.test.ts | 3 + .../unit/tauri-conf-webview2-args.test.ts | 2 + Client/tests/unit/types.test.ts | 2 +- Server/.air.toml | 2 +- Server/.dockerignore | 3 +- Server/.golangci.yml | 2 +- Server/CLAUDE.md | 11 ++- Server/Makefile | 8 +- Server/admin/admin.go | 6 +- Server/admin/admin_handler_test.go | 6 +- Server/admin/api.go | 10 +-- Server/admin/api_edge_cases_test.go | 4 +- Server/admin/api_test.go | 10 +-- Server/admin/backup_maintenance.go | 2 +- Server/admin/backup_maintenance_test.go | 2 +- Server/admin/channels_archive_voice_test.go | 4 +- Server/admin/export_test.go | 4 +- Server/admin/handlers_backup.go | 2 +- Server/admin/handlers_backup_test.go | 6 +- Server/admin/handlers_channel_perms.go | 4 +- Server/admin/handlers_channel_perms_test.go | 6 +- .../admin/handlers_channel_user_perms_test.go | 6 +- Server/admin/handlers_channels.go | 2 +- Server/admin/handlers_channels_test.go | 4 +- Server/admin/handlers_roles.go | 4 +- Server/admin/handlers_roles_test.go | 8 +- Server/admin/handlers_settings.go | 2 +- Server/admin/handlers_tokens.go | 4 +- Server/admin/handlers_users.go | 6 +- Server/admin/handlers_users_atomic_test.go | 2 +- Server/admin/handlers_users_broadcast_test.go | 4 +- Server/admin/harvest_s5_roles_test.go | 6 +- Server/admin/helpers.go | 2 +- Server/admin/hub_wiring_test.go | 2 +- Server/admin/logstream.go | 8 +- Server/admin/logstream_apitoken_test.go | 4 +- Server/admin/logstream_atomic_test.go | 2 +- Server/admin/logstream_test.go | 4 +- Server/admin/main_test.go | 2 +- Server/admin/middleware.go | 6 +- Server/admin/middleware_and_spawn_test.go | 6 +- Server/admin/middleware_coverage_test.go | 4 +- Server/admin/middleware_db_error_test.go | 2 +- Server/admin/multihandler_test.go | 2 +- Server/admin/perm_gates_test.go | 8 +- Server/admin/perm_grid_test.go | 2 +- Server/admin/restart_guard_test.go | 4 +- Server/admin/setup_clientip_test.go | 4 +- Server/admin/setup_handler.go | 8 +- Server/admin/setup_handler_test.go | 2 +- Server/admin/setup_limiter_reap_test.go | 4 +- Server/admin/setup_owner_orphan_test.go | 2 +- Server/admin/setup_wizard.go | 6 +- Server/admin/setup_wizard_test.go | 6 +- Server/admin/types.go | 2 +- Server/admin/update_handlers.go | 2 +- Server/admin/update_handlers_test.go | 6 +- Server/api/auth_handler.go | 8 +- .../api/auth_handler_delete_broadcast_test.go | 4 +- Server/api/auth_handler_test.go | 6 +- Server/api/avatar_handler_test.go | 12 +-- Server/api/channel_around_test.go | 2 +- Server/api/channel_authz_test.go | 4 +- Server/api/channel_handler.go | 6 +- Server/api/channel_handler_test.go | 8 +- Server/api/channel_reaction_users_test.go | 4 +- Server/api/client_update.go | 2 +- Server/api/client_update_test.go | 4 +- Server/api/constants.go | 2 +- Server/api/coverage_push_test.go | 6 +- Server/api/diagnostics_handler.go | 4 +- Server/api/diagnostics_handler_test.go | 10 +-- Server/api/dm_group_handler_test.go | 2 +- Server/api/dm_handler.go | 6 +- Server/api/dm_handler_block_context_test.go | 8 +- Server/api/dm_handler_presence_test.go | 8 +- ..._handler_rename_participant_lookup_test.go | 8 +- Server/api/dm_handler_test.go | 8 +- Server/api/dm_handler_visibility_ctx_test.go | 6 +- Server/api/emoji_handler.go | 6 +- Server/api/emoji_handler_test.go | 12 +-- Server/api/export_test.go | 4 +- Server/api/filestore.go | 2 +- Server/api/gif_handler.go | 8 +- Server/api/gif_handler_test.go | 8 +- Server/api/invite_handler.go | 6 +- Server/api/invite_handler_test.go | 10 +-- Server/api/livekit_proxy_ws_test.go | 6 +- Server/api/livekit_ratelimit_test.go | 2 +- Server/api/main_test.go | 4 +- Server/api/metrics_handler_test.go | 2 +- Server/api/middleware.go | 6 +- Server/api/middleware_test.go | 8 +- Server/api/plugins_handler.go | 2 +- Server/api/plugins_handler_test.go | 4 +- Server/api/profile_handler.go | 8 +- .../api/profile_handler_identity_key_test.go | 8 +- Server/api/profile_handler_test.go | 8 +- Server/api/router.go | 28 +++---- .../router_delete_account_broadcast_test.go | 8 +- Server/api/router_livekit_process_test.go | 8 +- Server/api/router_test.go | 6 +- Server/api/router_totp_key_fatal_test.go | 6 +- Server/api/totp_handler.go | 4 +- Server/api/totp_handler_test.go | 2 +- Server/api/upload_handler.go | 10 +-- Server/api/upload_handler_test.go | 12 +-- Server/auth/helpers.go | 2 +- Server/auth/helpers_test.go | 4 +- Server/auth/main_test.go | 2 +- Server/auth/password_test.go | 2 +- Server/auth/ratelimit.go | 2 +- Server/auth/ratelimit_cleanup_test.go | 2 +- Server/auth/ratelimit_persist_test.go | 2 +- Server/auth/ratelimit_test.go | 2 +- Server/auth/resolve.go | 2 +- Server/auth/resolve_test.go | 4 +- Server/auth/session_test.go | 2 +- Server/auth/tls.go | 2 +- Server/auth/tls_test.go | 4 +- Server/auth/totp.go | 2 +- Server/auth/totp_encrypt_test.go | 2 +- Server/auth/totp_test.go | 2 +- Server/{scripts => cmd}/genprotocol/main.go | 12 +-- Server/{scripts/seed.go => cmd/seed/main.go} | 37 ++++----- Server/config/config_test.go | 2 +- Server/config/save_test.go | 2 +- Server/db/account.go | 2 +- Server/db/account_test.go | 2 +- Server/db/admin_queries.go | 2 +- Server/db/admin_queries_test.go | 2 +- Server/db/apitoken_queries.go | 2 +- Server/db/apitoken_queries_test.go | 2 +- Server/db/attachment_queries.go | 2 +- Server/db/audit_test.go | 2 +- Server/db/audit_writer_test.go | 2 +- Server/db/auth_queries.go | 2 +- Server/db/auth_queries_test.go | 2 +- Server/db/backup_test.go | 2 +- Server/db/block_queries.go | 2 +- Server/db/channel_queries.go | 2 +- Server/db/channel_queries_test.go | 2 +- .../db/channel_user_override_queries_test.go | 2 +- Server/db/coverage_boost_test.go | 2 +- Server/db/db.go | 4 +- Server/db/db_test.go | 2 +- Server/db/dm_group_queries_test.go | 2 +- Server/db/dm_queries.go | 2 +- Server/db/emoji_queries.go | 2 +- Server/db/emoji_queries_test.go | 2 +- Server/db/errors_test.go | 2 +- Server/db/event_queries_test.go | 2 +- Server/db/lockout_queries.go | 2 +- Server/db/mappers.go | 2 +- Server/db/mention_queries_test.go | 2 +- Server/db/message_queries.go | 2 +- Server/db/message_queries_test.go | 2 +- Server/db/migrate_test.go | 2 +- Server/db/migrate_upgrade_test.go | 4 +- Server/db/migrated_db_test.go | 2 +- Server/db/models_test.go | 2 +- Server/db/open_shared_test.go | 2 +- Server/db/pool_test.go | 2 +- Server/db/profile_queries.go | 2 +- Server/db/role_queries.go | 2 +- Server/db/session_expiry_test.go | 4 +- Server/db/status_test.go | 2 +- Server/db/voice_queries.go | 2 +- Server/db/voice_queries_test.go | 2 +- Server/go.mod | 2 +- Server/invariants/syncutil_locks.go | 2 +- Server/invariants/syncutil_locks_test.go | 2 +- Server/logctx/logctx.go | 2 +- Server/main.go | 22 +++--- Server/main_test.go | 10 +-- Server/permissions/permissions_test.go | 2 +- Server/plugin/dbstore_test.go | 2 +- Server/plugin/pluginstore.go | 2 +- Server/plugin/registry_zip_rollback_test.go | 2 +- Server/restart.go | 4 +- Server/restart_test.go | 2 +- Server/scripts/k6/ws-load.js | 2 +- .../service/archived_channel_readonly_test.go | 2 +- Server/service/block.go | 2 +- Server/service/block_test.go | 2 +- Server/service/channel.go | 8 +- .../service/channel_focus_writeskip_test.go | 4 +- Server/service/channel_presence_test.go | 4 +- Server/service/channel_test.go | 4 +- Server/service/channel_user_override_test.go | 4 +- Server/service/datastore.go | 2 +- Server/service/dm.go | 6 +- Server/service/dm_test.go | 2 +- Server/service/emoji.go | 4 +- Server/service/emoji_test.go | 4 +- Server/service/harvest_s5_test.go | 4 +- Server/service/invite.go | 4 +- Server/service/mentions.go | 4 +- Server/service/mentions_test.go | 4 +- Server/service/message.go | 4 +- Server/service/message_around_test.go | 2 +- Server/service/message_crud.go | 8 +- Server/service/message_crud_test.go | 6 +- Server/service/message_perms.go | 4 +- Server/service/message_perms_test.go | 4 +- Server/service/message_purge.go | 6 +- Server/service/message_purge_test.go | 4 +- Server/service/message_query.go | 4 +- Server/service/message_reaction_users_test.go | 4 +- Server/service/message_reactions.go | 6 +- Server/service/message_reactions_test.go | 4 +- Server/service/message_test.go | 4 +- Server/service/moderation.go | 6 +- Server/service/moderation_test.go | 4 +- Server/service/permission.go | 8 +- Server/service/permission_stats_test.go | 4 +- Server/service/permission_test.go | 4 +- Server/service/profile_fields_test.go | 4 +- Server/service/require_channel_access_test.go | 4 +- Server/service/role.go | 6 +- Server/service/role_test.go | 4 +- Server/service/seed_test.go | 2 +- Server/service/service.go | 4 +- Server/service/user.go | 6 +- Server/service/user_postcommit_ctx_test.go | 2 +- .../service/user_postcommit_readerror_test.go | 2 +- Server/service/user_test.go | 2 +- Server/storage/storage_test.go | 2 +- Server/telemetry/metrics.go | 6 +- Server/telemetry/noop_test.go | 2 +- Server/telemetry/telemetry_default.go | 2 +- Server/telemetry/telemetry_otel.go | 2 +- Server/telemetry/telemetry_otel_test.go | 2 +- Server/telemetry/telemetry_test.go | 2 +- Server/token_cli.go | 6 +- Server/updater/tauri_key_contract_test.go | 37 +++++++++ Server/updater/updater.go | 4 +- Server/updater/updater_test.go | 23 ------ Server/ws/authz_test.go | 6 +- Server/ws/can_send_test.go | 4 +- Server/ws/channel_flags_ready_test.go | 2 +- .../ws/channel_visibility_agreement_test.go | 10 +-- Server/ws/client.go | 4 +- Server/ws/coverage_boost2_test.go | 4 +- Server/ws/coverage_chat_test.go | 2 +- Server/ws/coverage_helpers_test.go | 10 +-- Server/ws/coverage_misc_test.go | 2 +- Server/ws/coverage_voice_lifecycle_test.go | 4 +- Server/ws/coverage_voice_test.go | 6 +- Server/ws/deps.go | 10 +-- Server/ws/dm_group_call_test.go | 4 +- Server/ws/dm_handlers_test.go | 4 +- Server/ws/event.go | 2 +- Server/ws/event_persister.go | 4 +- Server/ws/event_persister_test.go | 2 +- Server/ws/event_presence_test.go | 2 +- Server/ws/event_pruner_test.go | 2 +- Server/ws/eventstore.go | 2 +- Server/ws/export_test.go | 2 +- Server/ws/handler_focus_revoke_race_test.go | 10 +-- Server/ws/handler_v2_channel_focus_test.go | 8 +- Server/ws/handler_v2_mark_read_test.go | 4 +- Server/ws/handler_v2_migration_test.go | 2 +- Server/ws/handler_v2_ping_test.go | 2 +- Server/ws/handler_v2_voice_e2ee_offer_test.go | 2 +- Server/ws/handler_v2_voice_token_test.go | 6 +- Server/ws/handlers.go | 6 +- Server/ws/handlers_call.go | 4 +- Server/ws/handlers_chat.go | 4 +- Server/ws/handlers_command.go | 4 +- Server/ws/handlers_command_gate_test.go | 8 +- Server/ws/handlers_command_test.go | 4 +- Server/ws/handlers_ping.go | 2 +- Server/ws/handlers_presence.go | 4 +- Server/ws/handlers_reaction.go | 2 +- Server/ws/handlers_test.go | 10 +-- Server/ws/handlers_voice.go | 2 +- Server/ws/harvest_s4_internal_test.go | 4 +- Server/ws/harvest_s5_internal_test.go | 8 +- Server/ws/hub.go | 14 ++-- Server/ws/hub_broadcast.go | 6 +- Server/ws/hub_broadcast_test.go | 4 +- Server/ws/hub_channel_meta_test.go | 6 +- Server/ws/hub_emoji_test.go | 4 +- Server/ws/hub_events.go | 2 +- Server/ws/hub_refresh_visibility_race_test.go | 4 +- Server/ws/hub_roles_test.go | 4 +- Server/ws/hub_sweep.go | 8 +- Server/ws/hub_sweep_oc_findings_test.go | 2 +- Server/ws/hub_sweep_test.go | 2 +- Server/ws/hub_test.go | 6 +- Server/ws/hub_visibility_watermark_test.go | 2 +- Server/ws/livekit.go | 2 +- Server/ws/livekit_client_test.go | 4 +- Server/ws/livekit_process.go | 4 +- Server/ws/livekit_test.go | 6 +- Server/ws/livekit_webhook_joined_test.go | 2 +- Server/ws/load_soak_test.go | 8 +- Server/ws/mentions_ready_test.go | 2 +- Server/ws/message_types.go | 4 +- Server/ws/messages.go | 4 +- Server/ws/messages_test.go | 2 +- Server/ws/oc_0006_video_stream_count_test.go | 4 +- Server/ws/oc_0012_cleanup_keyholder_test.go | 2 +- .../oc_0023_screenshare_video_limit_test.go | 4 +- .../ws/oc_0058_member_unban_broadcast_test.go | 2 +- .../oc_0090_deleted_channel_audience_test.go | 2 +- ...e_join_getchannelvoicestates_error_test.go | 6 +- .../ws/oc_0211_session_recheck_dberr_test.go | 2 +- ...19_voice_join_rollback_unsubscribe_test.go | 6 +- .../ws/oc_0222_reconnect_status_order_test.go | 4 +- .../ws/oc_0237_service_error_internal_test.go | 2 +- Server/ws/oc_0252_0269_0272_test.go | 2 +- ...oc_0267_voice_join_rollback_leaver_test.go | 8 +- Server/ws/oc_0276_voice_e2ee_resync_test.go | 2 +- .../ws/oc_0298_apply_connect_status_test.go | 2 +- .../ws/oc_0299_refresh_snapshot_role_test.go | 2 +- Server/ws/origin_test.go | 2 +- Server/ws/perm_cache_test.go | 10 +-- Server/ws/presence_invisible_test.go | 4 +- Server/ws/protocol_contract_test.go | 28 +++---- Server/ws/pubsub.go | 2 +- Server/ws/reconnect_active_channel_test.go | 2 +- Server/ws/reconnect_db_test.go | 6 +- .../reconnect_fallback_channel_leak_test.go | 4 +- Server/ws/reconnect_interior_gap_test.go | 4 +- Server/ws/reconnect_pruned_prefix_test.go | 4 +- Server/ws/reconnect_visibility_race_test.go | 4 +- Server/ws/reconnect_voice_supplement_test.go | 2 +- Server/ws/registry.go | 2 +- Server/ws/ringbuffer.go | 2 +- Server/ws/ringbuffer_test.go | 2 +- Server/ws/role_reassign_handshake_test.go | 4 +- Server/ws/serve.go | 10 +-- Server/ws/serve_auth.go | 4 +- .../serve_failed_handshake_teardown_test.go | 4 +- .../ws/serve_handshake_write_deadline_test.go | 4 +- Server/ws/serve_pumps.go | 2 +- Server/ws/serve_pumps_reconnect_race_test.go | 6 +- Server/ws/serve_ready.go | 4 +- Server/ws/serve_ready_dm_status_test.go | 2 +- .../serve_reconnect_double_teardown_test.go | 4 +- Server/ws/serve_test.go | 6 +- Server/ws/topic_rate_limiter.go | 2 +- Server/ws/voice_audience_test.go | 2 +- Server/ws/voice_controls.go | 4 +- Server/ws/voice_dm_access_test.go | 2 +- Server/ws/voice_e2ee.go | 2 +- Server/ws/voice_e2ee_test.go | 2 +- Server/ws/voice_handlers_test.go | 10 +-- Server/ws/voice_join.go | 8 +- Server/ws/voice_join_token_race_test.go | 6 +- Server/ws/voice_moderation.go | 6 +- .../ws/voice_moderation_deafen_race_test.go | 2 +- Server/ws/voice_moderation_test.go | 10 +-- Server/ws/voice_perm_stale_test.go | 4 +- Server/ws/voice_rate_limits_test.go | 2 +- Server/ws/wait_helpers_test.go | 2 +- Server/ws/ws_integration_test.go | 6 +- docs/README.md | 16 ++-- docs/architecture/server.md | 2 +- docs/architecture/websocket.md | 4 +- docs/contributing.md | 78 +++++++++++++++---- docs/protocol.md | 4 +- protocol/README.md | 36 +++++++++ .../schema.json | 2 +- scripts/run.mjs | 4 +- 383 files changed, 987 insertions(+), 853 deletions(-) rename Client/tests/{unit/admin-static-channel-perms.test.ts => contract/server-admin-static-channel-perms.test.ts} (91%) rename Server/{scripts => cmd}/genprotocol/main.go (92%) rename Server/{scripts/seed.go => cmd/seed/main.go} (92%) create mode 100644 Server/updater/tauri_key_contract_test.go create mode 100644 protocol/README.md rename docs/protocol-schema.json => protocol/schema.json (97%) diff --git a/.claude/skills/ci-check/SKILL.md b/.claude/skills/ci-check/SKILL.md index 88146adb..6c6f9459 100644 --- a/.claude/skills/ci-check/SKILL.md +++ b/.claude/skills/ci-check/SKILL.md @@ -37,7 +37,7 @@ golangci-lint run # CI pins v2.11.3 # Generated output must not be stale. These are what `make sqlc-verify` and # `make protocol-verify` reduce to — make is not on PATH on a stock Windows box. sqlc generate && git diff --exit-code db/dbgen -go run ./scripts/genprotocol && git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts +go run ./cmd/genprotocol && git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts ``` Add `-tags wazero` to `go vet`/`go test` when you touched `plugin/`. diff --git a/.claude/skills/protocol-change/SKILL.md b/.claude/skills/protocol-change/SKILL.md index 8d59b720..776cae6e 100644 --- a/.claude/skills/protocol-change/SKILL.md +++ b/.claude/skills/protocol-change/SKILL.md @@ -1,12 +1,12 @@ --- name: protocol-change -description: Add or change a WebSocket message type in OwnCord. Use before editing docs/protocol-schema.json, Server/ws/message_types.go, or Client/src/lib/protocolTypes.ts. +description: Add or change a WebSocket message type in OwnCord. Use before editing protocol/schema.json, Server/ws/message_types.go, or Client/src/lib/protocolTypes.ts. --- # protocol-change -`docs/protocol-schema.json` is the source of truth. Both constant files are -generated from it by `Server/scripts/genprotocol/`. +`protocol/schema.json` is the source of truth. Both constant files are +generated from it by `Server/cmd/genprotocol/`. **The schema holds message-type NAMES only.** Route by what you are changing — most payload work never touches it, and sending a field change through the @@ -23,7 +23,7 @@ forwards the message raw, there is nothing to add. If it **re-serialises**, an older server drops unknown JSON fields — so a field the server must forward is NOT backward compatible with older servers. -1. Edit `docs/protocol-schema.json`. +1. Edit `protocol/schema.json`. 2. Run `make protocol-generate` from `Server/`. 3. Commit **both** outputs — `Server/ws/message_types.go` and `Client/src/lib/protocolTypes.ts`. One run regenerates the diff --git a/.claude/workflows/bughunt-fix.js b/.claude/workflows/bughunt-fix.js index 7a38f9b8..85eb90ec 100644 --- a/.claude/workflows/bughunt-fix.js +++ b/.claude/workflows/bughunt-fix.js @@ -506,7 +506,7 @@ const GATE_COMMANDS = { ` make sqlc-verify protocol-verify # generated output must not be stale. If make is not on PATH, ` + `run the equivalent commands directly instead: ` + `"sqlc generate && git diff --exit-code db/dbgen" and ` + - `"go run ./scripts/genprotocol && git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts" ` + + `"go run ./cmd/genprotocol && git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts" ` + `- a non-empty diff in either means generated code is stale and the gate fails`, rust: `From Client/src-tauri:\n` + ` cargo test\n` + ` cargo clippy --all-targets -- -D warnings`, diff --git a/.claude/workflows/bughunt.js b/.claude/workflows/bughunt.js index 1f84fb77..c90df999 100644 --- a/.claude/workflows/bughunt.js +++ b/.claude/workflows/bughunt.js @@ -120,7 +120,7 @@ Method: structural reference is evidence of coupling, not of a bug; open the cited file and confirm. 2. For every candidate, grep for ALL callers before judging - a guard may already live upstream. 3. Check whether an existing test already locks the behavior you think is wrong. If a test asserts it, - it is intended behavior, not a bug. Test files are *_test.go and tests/unit/*.test.ts. + it is intended behavior, not a bug. Test files are *_test.go, tests/unit/*.test.ts and tests/contract/*.test.ts. 4. Report EVERY finding you can prove - there is no cap. The quality bar stays: zero findings is a valid, respectable answer, and each finding needs file, line, and a concrete repro. @@ -200,7 +200,7 @@ const SURFACE_LENSES = [ `an entity when events arrive out of order; read-state that can mark unread messages read, or lose an unread ` + `count, across a reconnect; an async handler whose await lets stale state be written after a newer update ` + `(last-write-wins race); a route guard bypassable by a rapid navigation sequence.\n` + - `Check tests/unit/ before reporting - much of this behavior is already test-locked.`, + `Check tests/unit/ and tests/contract/ before reporting - much of this behavior is already test-locked.`, }, ]; diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3459467e..23a036d2 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -50,11 +50,11 @@ if printf '%s\n' "$staged" | grep -qE '^Server/(db/queries/|migrations/|sqlc\.ya fi # Protocol schema changed -> regenerated Go + TS constants must be in the same commit. -if printf '%s\n' "$staged" | grep -qE '^(docs/protocol-schema\.json|Server/scripts/genprotocol/)'; then +if printf '%s\n' "$staged" | grep -qE '^(protocol/schema\.json|Server/cmd/genprotocol/)'; then if command -v go >/dev/null 2>&1; then - (cd Server && go run ./scripts/genprotocol \ + (cd Server && go run ./cmd/genprotocol \ && git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts) \ - || fail "protocol constants are stale — run 'go run ./scripts/genprotocol' in Server/ and stage the result" + || fail "protocol constants are stale — run 'go run ./cmd/genprotocol' in Server/ and stage the result" else printf 'pre-commit: WARNING: go not installed; skipping the protocol-constants check. CI will run it.\n' >&2 fi diff --git a/.githooks/pre-push b/.githooks/pre-push index 277f26a5..f0d0deab 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -54,7 +54,7 @@ if [ "$changed" = "__all__" ]; then else if printf '%s\n' "$changed" | grep -q '^Server/'; then server_changed=1; fi if printf '%s\n' "$changed" | grep -q '^Client/'; then client_changed=1; fi - if printf '%s\n' "$changed" | grep -q '^docs/protocol-schema\.json'; then + if printf '%s\n' "$changed" | grep -q '^protocol/schema\.json'; then server_changed=1 client_changed=1 fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b580953..ac0d4e06 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,7 +64,7 @@ jobs: run: make sqlc-install sqlc-verify # Protocol message-type constants (Go + TS) must never drift from - # docs/protocol-schema.json — the single source of truth. + # protocol/schema.json — the single source of truth. - name: Verify generated protocol constants (make protocol-verify) if: matrix.os == 'ubuntu-latest' run: make protocol-verify diff --git a/.superpowers/FINDINGS.md b/.superpowers/FINDINGS.md index e4d70def..7d89554a 100644 --- a/.superpowers/FINDINGS.md +++ b/.superpowers/FINDINGS.md @@ -4722,7 +4722,7 @@ Server/api/auth_handler.go:470 user, err := database.GetUserByUsername(r.Context bluemonday v1.0.27 sanitize.go:417-443 — case html.TextToken: default: buff.WriteString(token.String()) // x/net/html TextToken.String() == EscapeString(Data) -**Suggested fix:** In Server/admin/setup_handler.go:175 use the same fixpoint sanitizer as registration: `req.Username = strings.TrimSpace(service.SanitizeText(req.Username))` (Server/service/message.go:214). Server/admin already imports github.com/owncord/server/service (admin.go:12) and service does not import admin, so there is no cycle. One line, in the one place setup canonicalizes the username. +**Suggested fix:** In Server/admin/setup_handler.go:175 use the same fixpoint sanitizer as registration: `req.Username = strings.TrimSpace(service.SanitizeText(req.Username))` (Server/service/message.go:214). Server/admin already imports github.com/J3vb/OwnCord/Server/service (admin.go:12) and service does not import admin, so there is no cycle. One line, in the one place setup canonicalizes the username. **Fixed:** `29619536` · test `Server/admin/setup_handler_test.go` · revert-proof pass @@ -4742,7 +4742,7 @@ saveChannelPerms writes the quick "Can access" toggles first (PUT allow=0/deny=0 **Suggested fix:** In saveChannelPerms, record the role IDs the quick-toggle loop actually wrote and skip the matrix step when the selected target is one of them — one guard in the one function: collect `const touched=new Set()` in the loop (add role.role_id on each PUT/DELETE), then wrap the matrix block in `if(path && !(permTargetPath().indexOf('/permissions/')>-1 && touched.has(tid)))`. Cleanest variant: give the quick checkbox an onchange that patches the in-memory role.allow/role.deny in state.permChannel and calls renderPermMatrix(), so the matrix always reflects the pending toggle instead of the stale snapshot. -**Fixed:** `69258a51` · test `Client/tests/unit/admin-static-channel-perms.test.ts` · revert-proof pass +**Fixed:** `69258a51` · test `Client/tests/contract/server-admin-static-channel-perms.test.ts` · revert-proof pass ### OC-0155 — medium — Room-key offer pacing budget is per-call, so two back-to-back rotations blow the server's per-second offer cap and strand peers on a dead key @@ -6306,7 +6306,7 @@ Server/ws/voice_e2ee.go:270-272 (direct publish, bypasses h.broadcast) Server/ws/hub_broadcast.go:64-72 documents that publishing straight to pub/sub "would reintroduce exactly that kind of reordering". -**Suggested fix:** Carry the leaver's join instance in the broadcast and make the client's leave handling instance-conditional. finishVoiceLeave already holds `oldJoinToken` (voice_leave.go:56), so add it to voiceLeavePayload/buildVoiceLeave (via the protocol-change skill, since docs/protocol-schema.json is the source of truth) and record each peer's join token from voice_state in the client. Then guard the top of handleParticipantLeft: if the payload's join token is not the one currently recorded for that peer, ignore the event entirely — one guard in the shared function covers the delete, the retirement and the election at once. A client-only stopgap that removes the permanent half of the damage is to skip the `retirePeerKey` call at :1213-1215 whenever the peer is still present in `voiceStore.voiceUsers.get(channelId)`, which leaves the peer re-announceable instead of permanently blocked. +**Suggested fix:** Carry the leaver's join instance in the broadcast and make the client's leave handling instance-conditional. finishVoiceLeave already holds `oldJoinToken` (voice_leave.go:56), so add it to voiceLeavePayload/buildVoiceLeave (via the protocol-change skill, since protocol/schema.json is the source of truth) and record each peer's join token from voice_state in the client. Then guard the top of handleParticipantLeft: if the payload's join token is not the one currently recorded for that peer, ignore the event entirely — one guard in the shared function covers the delete, the retirement and the election at once. A client-only stopgap that removes the permanent half of the damage is to skip the `retirePeerKey` call at :1213-1215 whenever the peer is still present in `voiceStore.voiceUsers.get(channelId)`, which leaves the peer re-announceable instead of permanently blocked. **Fixed:** `ccd9f39b69b202dc2858c8b02e97104e67f81aec` · test `Client/tests/unit/livekit-e2ee.test.ts` · revert-proof pass @@ -8025,7 +8025,7 @@ The server deliberately withholds a mention badge for an @here from every reader **Evidence:** mentions.ts:124-127 `export function highlightsCurrentUser(content, info) { if (info?.mentionsEveryone === true) return true; ... }` — no way to distinguish @here from @everyone. dispatcher.ts:634-649 `const isMention = highlightsCurrentUser(payload.content, {mentions: payload.mentions, mentionsEveryone: payload.mentions_everyone}); ... if (isMention) incrementMention(payload.channel_id, isDetached);`. Server/service/mentions.go:187 `if set.HereOnly && (db.BroadcastStatus(r.Status) == db.StatusOffline || (s.online != nil && !s.online(r.UserID))) { continue }`. Server/ws/messages.go:83 `MentionsEveryone bool \`json:"mentions_everyone"\`` is the only mention-scope field on the wire. -**Suggested fix:** Stop collapsing the two tokens on the wire, then apply the server's rule once on the client's replay path. (1) Add a `mentions_here` bool to chatMessagePayload/chatEditedPayload (Server/ws/messages.go:83 and :149) sourced from mentionSet.HereOnly (plumb it alongside MentionsEveryone through service/message.go's SendResult/EditResult and ws/handlers_chat.go), regenerating docs/protocol-schema.json -> message_types.go/protocolTypes.ts via the protocol-change skill. (2) In Client/src/lib/dispatcher.ts, hoist the existing `isReplayFrame` computation (currently dispatcher.ts:686-690) above the unread/mention block at 634-649 and gate the badge in that one place: treat a frame as a mention only when `payload.mentions.includes(me)` or (`payload.mentions_everyone && !(payload.mentions_here && isReplayFrame)`). That mirrors applyMentionCounts exactly — a here-only mention delivered in the reconnect burst is by definition one the reader was disconnected for — and leaves live delivery, @everyone, and direct mentions untouched. No change to mentions.ts's highlightsCurrentUser is needed for highlight rendering; only the badge increment must distinguish the two. +**Suggested fix:** Stop collapsing the two tokens on the wire, then apply the server's rule once on the client's replay path. (1) Add a `mentions_here` bool to chatMessagePayload/chatEditedPayload (Server/ws/messages.go:83 and :149) sourced from mentionSet.HereOnly (plumb it alongside MentionsEveryone through service/message.go's SendResult/EditResult and ws/handlers_chat.go), regenerating protocol/schema.json -> message_types.go/protocolTypes.ts via the protocol-change skill. (2) In Client/src/lib/dispatcher.ts, hoist the existing `isReplayFrame` computation (currently dispatcher.ts:686-690) above the unread/mention block at 634-649 and gate the badge in that one place: treat a frame as a mention only when `payload.mentions.includes(me)` or (`payload.mentions_everyone && !(payload.mentions_here && isReplayFrame)`). That mirrors applyMentionCounts exactly — a here-only mention delivered in the reconnect burst is by definition one the reader was disconnected for — and leaves live delivery, @everyone, and direct mentions untouched. No change to mentions.ts's highlightsCurrentUser is needed for highlight rendering; only the badge increment must distinguish the two. **Fixed:** `6f6d0ae` · test `Client/tests/unit/dispatcher.test.ts` · revert-proof self-reported diff --git a/.superpowers/findings-ledger.json b/.superpowers/findings-ledger.json index d7f4beac..696e7b82 100644 --- a/.superpowers/findings-ledger.json +++ b/.superpowers/findings-ledger.json @@ -3755,7 +3755,7 @@ "test": "Server/admin/setup_handler_test.go", "revertProof": "pass" }, - "suggestedFix": "In Server/admin/setup_handler.go:175 use the same fixpoint sanitizer as registration: `req.Username = strings.TrimSpace(service.SanitizeText(req.Username))` (Server/service/message.go:214). Server/admin already imports github.com/owncord/server/service (admin.go:12) and service does not import admin, so there is no cycle. One line, in the one place setup canonicalizes the username.", + "suggestedFix": "In Server/admin/setup_handler.go:175 use the same fixpoint sanitizer as registration: `req.Username = strings.TrimSpace(service.SanitizeText(req.Username))` (Server/service/message.go:214). Server/admin already imports github.com/J3vb/OwnCord/Server/service (admin.go:12) and service does not import admin, so there is no cycle. One line, in the one place setup canonicalizes the username.", "fixedDate": "2026-08-19" }, { @@ -3775,7 +3775,7 @@ "confidence": "high", "fix": { "commit": "69258a51", - "test": "Client/tests/unit/admin-static-channel-perms.test.ts", + "test": "Client/tests/contract/server-admin-static-channel-perms.test.ts", "revertProof": "pass" }, "suggestedFix": "In saveChannelPerms, record the role IDs the quick-toggle loop actually wrote and skip the matrix step when the selected target is one of them — one guard in the one function: collect `const touched=new Set()` in the loop (add role.role_id on each PUT/DELETE), then wrap the matrix block in `if(path && !(permTargetPath().indexOf('/permissions/')>-1 && touched.has(tid)))`. Cleanest variant: give the quick checkbox an onchange that patches the in-memory role.allow/role.deny in state.permChannel and calls renderPermMatrix(), so the matrix always reflects the pending toggle instead of the stale snapshot.", @@ -5134,7 +5134,7 @@ "test": "Client/tests/unit/livekit-e2ee.test.ts", "revertProof": "pass" }, - "suggestedFix": "Carry the leaver's join instance in the broadcast and make the client's leave handling instance-conditional. finishVoiceLeave already holds `oldJoinToken` (voice_leave.go:56), so add it to voiceLeavePayload/buildVoiceLeave (via the protocol-change skill, since docs/protocol-schema.json is the source of truth) and record each peer's join token from voice_state in the client. Then guard the top of handleParticipantLeft: if the payload's join token is not the one currently recorded for that peer, ignore the event entirely — one guard in the shared function covers the delete, the retirement and the election at once. A client-only stopgap that removes the permanent half of the damage is to skip the `retirePeerKey` call at :1213-1215 whenever the peer is still present in `voiceStore.voiceUsers.get(channelId)`, which leaves the peer re-announceable instead of permanently blocked.", + "suggestedFix": "Carry the leaver's join instance in the broadcast and make the client's leave handling instance-conditional. finishVoiceLeave already holds `oldJoinToken` (voice_leave.go:56), so add it to voiceLeavePayload/buildVoiceLeave (via the protocol-change skill, since protocol/schema.json is the source of truth) and record each peer's join token from voice_state in the client. Then guard the top of handleParticipantLeft: if the payload's join token is not the one currently recorded for that peer, ignore the event entirely — one guard in the shared function covers the delete, the retirement and the election at once. A client-only stopgap that removes the permanent half of the damage is to skip the `retirePeerKey` call at :1213-1215 whenever the peer is still present in `voiceStore.voiceUsers.get(channelId)`, which leaves the peer re-announceable instead of permanently blocked.", "fixedDate": "2026-08-20" }, { @@ -6426,7 +6426,7 @@ "lens": "flow-message", "finder": "opus", "confidence": "medium", - "suggestedFix": "Stop collapsing the two tokens on the wire, then apply the server's rule once on the client's replay path. (1) Add a `mentions_here` bool to chatMessagePayload/chatEditedPayload (Server/ws/messages.go:83 and :149) sourced from mentionSet.HereOnly (plumb it alongside MentionsEveryone through service/message.go's SendResult/EditResult and ws/handlers_chat.go), regenerating docs/protocol-schema.json -> message_types.go/protocolTypes.ts via the protocol-change skill. (2) In Client/src/lib/dispatcher.ts, hoist the existing `isReplayFrame` computation (currently dispatcher.ts:686-690) above the unread/mention block at 634-649 and gate the badge in that one place: treat a frame as a mention only when `payload.mentions.includes(me)` or (`payload.mentions_everyone && !(payload.mentions_here && isReplayFrame)`). That mirrors applyMentionCounts exactly — a here-only mention delivered in the reconnect burst is by definition one the reader was disconnected for — and leaves live delivery, @everyone, and direct mentions untouched. No change to mentions.ts's highlightsCurrentUser is needed for highlight rendering; only the badge increment must distinguish the two.", + "suggestedFix": "Stop collapsing the two tokens on the wire, then apply the server's rule once on the client's replay path. (1) Add a `mentions_here` bool to chatMessagePayload/chatEditedPayload (Server/ws/messages.go:83 and :149) sourced from mentionSet.HereOnly (plumb it alongside MentionsEveryone through service/message.go's SendResult/EditResult and ws/handlers_chat.go), regenerating protocol/schema.json -> message_types.go/protocolTypes.ts via the protocol-change skill. (2) In Client/src/lib/dispatcher.ts, hoist the existing `isReplayFrame` computation (currently dispatcher.ts:686-690) above the unread/mention block at 634-649 and gate the badge in that one place: treat a frame as a mention only when `payload.mentions.includes(me)` or (`payload.mentions_everyone && !(payload.mentions_here && isReplayFrame)`). That mirrors applyMentionCounts exactly — a here-only mention delivered in the reconnect burst is by definition one the reader was disconnected for — and leaves live delivery, @everyone, and direct mentions untouched. No change to mentions.ts's highlightsCurrentUser is needed for highlight rendering; only the badge increment must distinguish the two.", "fix": { "commit": "6f6d0ae", "test": "Client/tests/unit/dispatcher.test.ts", diff --git a/CLAUDE.md b/CLAUDE.md index 2cec670b..ff183c46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ CI fails on drift, and the next generator run silently discards your edit. | Generated | Source of truth | Workflow | | ---------------------------------------------------------------------- | ----------------------------------------------- | -------------------------------------------------------------- | | `Server/db/dbgen/` | `Server/db/queries/*.sql`, `Server/migrations/` | `db-change` skill | -| `Server/ws/message_types.go` **and** `Client/src/lib/protocolTypes.ts` | `docs/protocol-schema.json` | `protocol-change` skill | +| `Server/ws/message_types.go` **and** `Client/src/lib/protocolTypes.ts` | `protocol/schema.json` | `protocol-change` skill | | `Client/src/generated/` | `tauri-typegen` | CI patches known typegen bugs — see `.github/workflows/ci.yml` | ## Bug-hunt ledger diff --git a/Client/CLAUDE.md b/Client/CLAUDE.md index 21b48b2e..83e151cf 100644 --- a/Client/CLAUDE.md +++ b/Client/CLAUDE.md @@ -9,8 +9,13 @@ Rust backend in `src-tauri/` for native APIs only. LiveKit handles voice/video. `src/pages/`, `src/components/` UI - `src/lib/protocolTypes.ts` and `src/generated/` are generated — see the root CLAUDE.md -- `tests/unit`, `tests/integration` (vitest, jsdom) · `tests/e2e` (Playwright) · +- `tests/unit`, `tests/integration`, `tests/contract` (vitest, jsdom) · + `tests/e2e`, `tests/e2e/admin`, `tests/e2e/native` (Playwright) · `tests/browser` (vitest browser mode) +- A test whose assertions read, import or execute a **`Server/`-owned** + artifact belongs in `tests/contract`, not `tests/unit` — `src-tauri/` is + part of this component, so reading it is an ordinary unit test. The rule + is in [docs/contributing.md](../docs/contributing.md#testing) ## Gotchas diff --git a/Client/package.json b/Client/package.json index 2aa7d88d..18ecad58 100644 --- a/Client/package.json +++ b/Client/package.json @@ -15,6 +15,7 @@ "test": "vitest run", "test:unit": "vitest run tests/unit", "test:integration": "vitest run tests/integration", + "test:contract": "vitest run tests/contract", "test:e2e": "playwright test", "test:e2e:prod": "npm run build && playwright test --config playwright.config.prod.ts", "test:e2e:native": "playwright test --config playwright.config.native.ts", diff --git a/Client/src/lib/protocolTypes.ts b/Client/src/lib/protocolTypes.ts index f526f66c..cc877e63 100644 --- a/Client/src/lib/protocolTypes.ts +++ b/Client/src/lib/protocolTypes.ts @@ -1,7 +1,7 @@ -// Code generated by scripts/genprotocol from docs/protocol-schema.json; DO NOT EDIT. +// Code generated by cmd/genprotocol from protocol/schema.json; DO NOT EDIT. // // Shared WebSocket protocol message type constants — single source of truth -// for both Server (Go) and Client (TypeScript). Edit docs/protocol-schema.json +// for both Server (Go) and Client (TypeScript). Edit protocol/schema.json // and run `make protocol-generate` in Server/. // // Usage: import { MessageType } from "@lib/protocolTypes"; diff --git a/Client/tests/unit/admin-static-channel-perms.test.ts b/Client/tests/contract/server-admin-static-channel-perms.test.ts similarity index 91% rename from Client/tests/unit/admin-static-channel-perms.test.ts rename to Client/tests/contract/server-admin-static-channel-perms.test.ts index 455804fa..d15fa0a8 100644 --- a/Client/tests/unit/admin-static-channel-perms.test.ts +++ b/Client/tests/contract/server-admin-static-channel-perms.test.ts @@ -1,3 +1,8 @@ +// CONTRACT TEST. The artifact under test is owned by Server/admin; the runner +// lives here because placement follows capability, not ownership — the Go +// module carries no JavaScript engine, so nothing under Server/ can execute +// this SPA. See docs/contributing.md#testing for the membership rule. +// // Loads the real Server/admin/static/index.html (the Go admin panel's // single-file SPA) into a scripted jsdom window and drives its inline // channel-permissions logic directly, the same way a browser would. @@ -5,6 +10,9 @@ // There is no bundler or module system for this file — it is one inline //