J3vbandClaude Fable 5
1fe3df7962
* refactor(protocol): retire reserved voice_speakers and member_leave (S-15) Neither type was ever emitted by the server; B2-1 clears them from the schema before the epoch-1 wire fixtures are captured, so the frozen epoch does not carry two dead message types. Client: dropped the dead `ws.on(MEMBER_LEAVE)` / `ws.on(VOICE_SPEAKERS)` dispatcher handlers, the `MemberLeavePayload` type and both `ServerMessage` union members, and the tests that only exercised those WS paths. `removeMember` (member_ban) and `setSpeakers` (LiveKit ActiveSpeakers) stay live and keep their direct unit tests. * test(ws): capture the epoch-1 wire fixtures alpha.4 is the last client on the pre-epoch wire and B2-2 adds a protocol epoch to the auth handshake next, so record what epoch 1 actually looks like while it is still observable. TestEpoch1Fixtures drives eleven journeys through the ws package's in-process hub harness (full migrations, real hub, httptest WebSocket server) and compares each journey's per-connection frame sequence with a transcript under protocol/fixtures/epoch-1/: - fresh-connect, auth-failure, ping - chat-send-fanout, chat-edit-delete, reaction-add-remove - typing, mark-read, dm-send - resume-replay (last_seq + buffer-tier replay burst) - voice-join-e2ee-leave (join, both voice_state forms, announce and offer relay, leave) Volatile values are replaced by typed placeholders before both writing and comparison -- any key that is id/seq/last_seq, ends in _id (except channel_id and role_id) or _at, is timestamp/ts/last_seen, or contains token, becomes "<class:json-type>" so a field that changes type is still a diff, while everything else is compared verbatim. Regenerate with: go test ./ws -run TestEpoch1Fixtures -update Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(client): pin the epoch-1 auth frame contract Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(updater): pin the epoch-1 manifest and client-update shapes * test(ws): harden the epoch-1 fixtures against trailing frames and absent optionals Every journey now ends with the ping/pong barrier on each recorded connection, so a frame the server emits after the last read fails as `expected "pong", got "X"` instead of going unrecorded; auth-failure asserts the StatusPolicyViolation close instead, its socket being gone. alice carries a display name, avatar, about text, custom status, identity public key and an announce signature (bob carries none), so every optional field is frozen in its present form as well as its absent one — a rename or a retype of display_name or identity_public_key now moves a fixture. The typing journey focuses the channel on "a" before typing: without the subscription registerNow only makes for a focused client, its ping/pong proved nothing about excludeUserID. Comment fixes: the escaped placeholder form MarshalIndent would write, the real (headroom) reason for the raised read limit, a note that bare id and active_channel_id are normalised by design, and a .prettierignore line saying these fixtures are verified by the Go comparison, not by git diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(protocol): document the epoch-1 fixtures * docs(protocol): match the epoch-1 wire where the fixtures contradicted the doc - voice_state: note the unsequenced relay-to-joiner form (table row + section) - auth_error: correct the example message and list the real rejection messages - member_join/presence connect example: add the seq both frames actually carry - chat_message.user: document display_name - voice_join reply order: state that the joiner's own voice_state broadcast is not ordered against the other three frames * test(ws): freeze the null forms of auth_ok and member_join user fields buildAuthOK emits display_name/about/custom_status/avatar as always-present nulls, but only alice — who has all four set — ever authenticated on a recorded connection, so the fixtures froze those fields in their populated form alone. A rename, a retype or a dropped null would have moved nothing, on the very frame B2-2 edits. fresh-connect now records bob's handshake too, on a second connection: his auth_ok carries the four nulls, his member_join carries avatar null with display_name and identity_public_key omitted, and alice — idle by then, so her reads stay in hub order — records the same pair as an already-connected observer sees it. In voice-join-e2ee-leave bob answers alice's signed announce with a legacy unsigned one, which freezes the absent form of signature next to her present one. The auth frames the test writes now carry the correlation id the real client stamps on every frame (ws.ts send()); normalisation renders it <id:string>. expectClosed also asserts the close reason ("authentication failed"), not just code 1008 — HP-2 asks for both. Comment precision, no behaviour change: the barrier guarantee now states that pong may overtake a pending LOW-priority frame (writePump) and that no journey is affected because every barrier is sent on an idle connection; the ping-budget ceiling is six connections, not four; the typing journey cites handleChannelFocusV2 rather than registerNow as the subscribe site; resume-replay's b barrier explains why moving it past the resume would be a flake, not a fix; and the header notes that normalisation hides that chat_send_ok.id echoes the request id. The client contract test's cited range for ws.ts's send() call is 441-453, not 441-454. Regenerate with: go test ./ws -run TestEpoch1Fixtures -update Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(protocol): shape-not-value fixture rule, voice_max_video default, auth close code The fixture rule read "a fixture may only change with an epoch bump", which is false for the seeded values the transcripts record verbatim: role permission masks and colours, motd, server_name, voice_max_video, the voice_config preset. A migration that changes a default mask diffs fresh-connect.json, and the README told the author to revert a change that never touched the wire. Split the rule along shape versus value. A key set, a JSON type, a key appearing or disappearing, or per-connection frame order is a protocol change and earns fixtures/epoch-<n+1>/. A seeded default value is a seed change: regenerate in the same PR and read the diff frame by frame. Normalising those values is explicitly not the answer — a placeholder over a mask or over an enum such as voice_config.threshold_mode would hide the drift the fixtures exist to catch. Two wire facts corrected against the fixtures: - voice_max_video on an unconfigured channel is 25, not 0 (migration 004 is DEFAULT 25); the doc listed it among the zero values. - auth_error is followed by a close with code 1008 (policy violation) and reason "authentication failed" (serve.go:128), which the doc left as "closes the connection". Also: voice_speakers moves from discord-parity's "still dead" list to "came off the list" — it was retired earlier on this branch. And Kick says sessions are revoked and sockets drop on the next sweep, which is what ForceLogout does (moderation.go:236); it does not cut sockets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(protocol): fixture rule covers enum vocabularies; header comment precision Re-check minors from the whole-branch review: a fixed-vocabulary value the client switches on is shape, not a seeded value; drop the false 'new default channel' clause (the seed asserts channel ids); the low-priority frame is presence_update, not connect presence. * docs(plans): record B2-1 evidence for HP-2 (PR #1435, fixture commit SHAs) * docs(protocol): additive changes stay within an epoch; bump only for what old clients cannot process Codex review on #1435: B2-2 keeps protocol_epoch = 1 while adding auth/ready/auth_error fields, which the previous wording would have called a break to revert. An epoch is a compatibility boundary, not a snapshot: additive keys regenerate in the same PR and are documented; removals, renames, retypes, dropped frames and reordering bump the epoch. The plan's B2-1 evidence records the refinement and hands B2-2/B2-4 the open questions (additive-tolerant replay of the epoch-1 transcript; epoch 0 vs 1 naming). * test(client): compare auth-frame key sets order-independently Codex review on #1435: Object.keys preserves insertion order, so a harmless property reorder in ws.ts would fail the pin. Key order has no wire meaning; the Go fixtures already compare with sorted keys. Sort both sides. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
33 KiB
OwnCord — Repo Health Audit
Date: 2026-08-19
Branch: claude/repo-health-audit-s0xnyo (audited tree: eacba10 = main)
Scope: full-repo health check — prior-finding closure verification, dynamic
checks (build/test/boot), doc/code drift across the three reference specs, and
a static sweep focused on the WebSocket hub, reconnect/sync handling, and the
REST/WS boundary. Read-only; no code changes ship with this audit.
Relationship to prior audits: successor to the 2026-08-04 pair
(security, docs & coverage),
which remain the closure trackers for their own findings. Every item those
audits left open is re-verified in §2.
Method: ten parallel analysis passes (3 prior-finding verification, 3
doc-drift, 4 static-sweep), every BROKEN/FRAGILE candidate then re-traced by an
independent adversarial verifier instructed to refute it (7 confirmed, 1
downgraded, 0 refuted). Every dynamic-check number in §3 comes from an actual
run in this session. Rank legend: BROKEN = fails or states falsehoods now
· FRAGILE = works today, likely to break · DEBT = cleanup, no user
impact.
1. Executive summary
The code is in the best shape any of the six audits has found it: every suite passes, the server boots clean, the bug-hunt ledger is fully closed (188 fixed / 2 declined / 1 duplicate / 0 open), and the heavily-audited subsystems — hub locking, replay tiers, permission funnels, plugin sandbox — all verified sound again (§7).
What has decayed is the documentation, again. Fifteen days after the
2026-08-04 spec refresh, the three reference docs carry ten newly wrong
statements (§4): schema.md misses two migrations and documents the exact FK
behavior migration 030 was written to remove; protocol.md claims DM chat
events are unsequenced when they are stamped, ring-buffered and replayable,
and documents five rate limits as "None" that are all enforced;
api.md tells non-admins they can call an admin-only endpoint and documents
three error codes the server never emits. The keep-docs-current-per-PR rule
(A-2026-07-03) is not holding on its own.
On the code side the sweep found five FRAGILE items and no BROKEN ones — two silent-failure gaps in auth persistence, a role-change/handshake race that can leave a socket on stale subscriptions, dead-on-arrival client replay-dedup machinery, and one block-enforcement asymmetry (blocking a user does not evict them from a live 1:1 DM voice call) that continues the exact defect family the 2026-08-04 security review closed. One client unit test is a load-sensitive timeout flake. Everything else is debt.
2. Prior-finding closure verification
Every item left open by a prior audit, re-verified against eacba10.
2.1 Carried findings
| Item | Sev | Verdict 2026-08-19 | Evidence |
|---|---|---|---|
| A-2026-07-10 router god-constructor | MED | PARTIALLY FIXED | NewRouter is down to 171 lines (Server/api/router.go:39-209) with 7 extracted helpers (routerTOTPKey :213 … routerMetricsRoutes :412), but remains the composition root: builds limiter/services/hub, spawns go hub.Run() (:161), and the returned cleanup (:204-206) still covers only limiterStopCh |
A-2026-07-11 Hub Set* post-construction wiring |
MED | STILL OPEN, hazard mitigated | 6 setters remain (ws/hub_events.go:40,48,60,70, ws/hub_livekit.go:10,48); three are now atomic stores safe post-Run, the other three are guarded by rejectIfRunning (ws/hub.go:721-728) — a missed setter is no longer a race, but the temporal coupling stands |
| A-2026-07-14 scattered client constants | LOW | STILL OPEN, grew | #5865F2 ×19 across 12 files (was 18/11); localhost:8443 ×3; setTimeout/setInterval at 114 call-site lines across 43 non-test files (was 64); src/lib/constants.ts still holds only 2 constants |
| 2026-04-07 #9 auth handler bypasses service layer | MED | STILL OPEN | Server/api/router.go:117 passes database *db.DB to MountAuthRoutes while siblings get svc; handlers call database.* directly (auth_handler.go:166,201,209,483,572,584,673,757) |
| 2026-04-07 #5 plugin HTTP exfiltration | accepted | ACCEPTED RISK INTACT | All four mitigations re-verified: defaults off + empty allowlist (config/config.go:351-357), empty allowlist denies all (plugin/host_http.go:140-158), WASI-only runtime with no host imports (plugin/sandbox_wazero.go:100-113), inert EventSink.Dispatch + no production Subscribe (plugin/host_events.go:122-131) |
| A-2026-07-06 residual: direct DB above service layer | MED | IMPROVED ~50% | ~170 direct call sites in non-test api/admin/ws (was ~359), 30 files; top offenders ws/voice_join.go (14), admin/handlers_channel_perms.go (13), ws/hub_broadcast.go (11) |
channelCanSend copy (permission-middleware plan disclosure) |
MED | STILL OPEN | Server/ws/serve_ready.go:142-160 still hand-mirrors MessageService.checkSendPermission (service/message_perms.go:78-102); guarded by the mirror test ws/can_send_test.go but synced by hand |
| audit-2026-08-04 §5: backup restore hardcodes DB path; 500-path serves a closed DB | obs | FIXED | admin.SetDatabasePath wired from main.go:337 (pinned by handlers_backup_test.go:553-577); every restore failure branch now rolls back the safety copy and/or restarts (handlers_backup.go:276-329) — no branch leaves a closed DB serving |
T-2026-07-25-11 residual: HandleLiveKitHealthForTest duplicate |
LOW | STILL OPEN (test-only) | All 7 legacy callers unmigrated (api/middleware_test.go:1026,1049,1075, api/coverage_push_test.go:327,350,373,642); production handler is single-sourced |
T-2026-07-25 §4: logctx.WithGroup nesting decision |
LOW | STILL OPEN by design | Behavior unchanged (logctx.go:52-54), pinned by test; no production code opens a logger-level group; doc comment defers explicitly (note: stray word "ponytail" at logctx.go:49) |
T-2026-07-25 backlog #8: gofmt drift in storage/storage.go |
LOW | FIXED — but recurred elsewhere | gofmt -l Server/ is clean except one new drifted file, admin/handlers_users_broadcast_test.go (§6); no gofmt gate exists in CI or .golangci.yml, which is why it drifted in |
T-2026-07-25-17/-18/-19 (main.go/seed untested; MainPage.ts/main.ts excluded; no Go/Rust coverage floor) |
LOW | UNCHANGED, documented | vitest excludes still carry written justifications (+1 justified entry: noise-suppression.ts); the rest are recorded accepted positions |
T-2026-07-25-20 ws flake under -coverpkg |
LOW | CAN'T VERIFY | Not reproduced this session (plain go test used); stays on watch |
DC-14 reserved protocol entries (voice_speakers, member_leave) |
P3 | Resolved 2026-08-28 — dropped in B2-1 (S-15); was "STILL OPEN, deliberate" | Constants + schema entries exist, zero non-test emit sites; consistently documented "Reserved" in protocol.md:1506,1508; a protocol rev for the owner to schedule |
admin-e2e soak graduation |
P2 | STILL PENDING | ci.yml:311 continue-on-error: true, graduation criterion recorded 2026-08-15 (~30 consecutive green main runs, ci.yml:302-307) |
2026-07-19 §5: tauri-build never runs on push to main |
LOW | STILL TRUE | ci.yml:440-443 gates on pull_request + base_ref == main; only other desktop build is the v*-tag release workflow |
livekitSession.ts monolith |
MED | STILL OPEN, grew | 1809 LOC (was 1719); satellites roomEventHandlers.ts/livekitDiagnostics.ts were added without shrinking the core. AccountTab.ts 1185, SidebarArea.ts 848, LoginForm.ts 784 |
| TODO inventory | — | 3 remain | ws/voice_e2ee.go:167 (key-holder TOCTOU, documented accepted), ws/deps.go:271 (generics), SidebarArea.ts:659 (H16 DOM thrash). The update_handlers.go container TODO is resolved (503 CONTAINER_DEPLOYMENT via updater.RunningInContainer, update_handlers.go:49-53) |
2.2 CI gates and pins (all verified holding)
.nvmrc = 20 matches all six setup-node pins; knip, client-e2e,
client-e2e-parity genuinely blocking (no continue-on-error, no || true);
-tags wazero ./plugin/... and -tags otel ./telemetry/... run in CI
(ci.yml:87-89); rust-tests runs on every event;
tests/e2e/E2E-ISSUES.md matches the live Playwright configs (293 tests vs
the doc's dated 291 — normal growth, counts are labeled as a run snapshot).
2.3 Plan statuses (docs/plans/)
| Plan | Verdict | Notes |
|---|---|---|
| security-hardening-remediation | complete, verified | W2-4/W3-3 closed with the named locking tests present |
| channel-visibility-unification, http-tofu-proxy, permission-middleware-consolidation, v2-dispatch-migration, audit-2026-07-19-decisions | shipped, nothing reopened | only cosmetic line-number drift in citations |
| security-scan-2026-07-22-remediation | shipped; header stale in the good direction | F3 follow-up 3 (getIdentityPin fail-open) is marked open but was fixed (identity.ts:158-175, tri-state lookup citing DC-08) |
| sqlc-adoption | accurate | residual raw queries (variable IN, FTS, tx) still deliberately raw as listed |
| tauri-capability-narrowing | DNS-rebinding follow-up still open | embeds.ts:8 still fetches user-posted URLs from the renderer via @tauri-apps/plugin-http; capabilities/default.json still grants https://* (:33,:36); no Rust og-meta command exists |
| infrastructure-roadmap | shipped except 2 named leftovers | TOTP/partial-auth persister seam (no persist code in auth/totp.go) and published capacity numbers (the load-baseline workflow exists to produce them) |
| discord-parity | phases 1-6 shipped; named leftovers open | role hoist/mentionable + @RoleName mentions, categories as real entities; its still-dead-code list is one item stale (sounds table was dropped by migration 029) |
| bug-detection-improvements | header stale — partially implemented | header says "not implemented" but Tier 1a (make fuzz, Server/Makefile:43-48) and Tier 2 (5 ESLint rules, delivered 2026-08-08 per its own body) exist; Tiers 3-4 open |
| slash-commands | design-only, as stated | no plugin_commands migration, no autocomplete surface |
3. Dynamic checks (this session, at eacba10)
Environment: Linux container, 4 CPU / 15 GB; go 1.24.7 host +
GOTOOLCHAIN=auto; Node 22.22.2 (CI pins 20), npm 10.9.7.
| Check | Command | Result |
|---|---|---|
| Go vet | cd Server && go vet ./... |
PASS |
| Go tests | go test -count=1 ./... |
PASS — 15 test packages ok (ws 54s, db 14.5s) |
| Client typecheck | npm run typecheck |
PASS |
| Client lint | npm run lint |
PASS — exit 0 with 25 non-blocking no-underscore-dangle warnings, all in livekitSession.ts:1656-1759 (delegating-method section) |
| Client unit suite | npm test |
5044/5045 — one timeout failure, shown to be a load flake (§5, F-5); passes 3/3 in isolation |
| Server build | go build . |
PASS |
| Server boot | run binary in empty dir | CLEAN — first-run config + certs generated, expected WARNs only; /health 200 {"status":"ok","uptime":1,"online_users":0}; /admin 200 serving the SPA (145,663 bytes); LiveKit auto-download + managed start; clean shutdown on SIGTERM |
| Client build | npm run build |
PASS (chunk-size warnings only) |
| Protocol codegen gate | go run ./scripts/genprotocol diffed against committed output |
CLEAN — byte-identical Go + TS |
| sqlc gate | make sqlc-verify |
SKIPPED — sqlc binary absent in this container; CI covers it |
| knip | npx knip |
exit 0 — zero unused files/exports/deps (4 cosmetic config hints) |
| Go module hygiene | go mod tidy -diff |
CLEAN — empty diff |
4. New findings — BROKEN (all documentation)
No code-level BROKEN findings. All ten are reference-doc statements that are false against the running code; a reader following them writes wrong clients or wrong ops procedures. All were independently re-verified.
schema.md
| ID | Finding | Evidence | Fix |
|---|---|---|---|
| B-01 | Migration table ends at 029; attachments.message_id documented as ON DELETE CASCADE, the exact behavior migration 030 removed (ON DELETE SET NULL, to stop stranding uploaded files); 031 undocumented |
schema.md:44-74, :420 vs migrations/030_attachments_unlink_on_message_delete.sql:25 (+ header lines 1-16), 031_sessions_expiry_index.sql |
Add 030/031 rows; flip :420 to SET NULL with the unlink semantics |
| B-02 | Indexes table lists 3 dropped indexes (idx_sessions_token, idx_invites_code, idx_channel_overrides_channel_role) and omits 7 live ones (idx_channel_overrides_role, idx_attachments_message, idx_messages_pinned, idx_api_tokens_user, idx_users_avatar, idx_channels_dm_group, idx_sessions_expires_at) |
schema.md:719,723,728 vs migrations/019_perf_indexes.sql, 020_drop_redundant_indexes.sql:6-7, 018:27, 027:28, 028:29, 031:13; the doc's own migration rows (:64-65) contradict its index table |
Rewrite the table from cumulative migration state |
| B-03 | "The connection pool is pinned to a single connection" — false for file-backed (production) DBs, which run a 1-writer + up-to-64-reader pool split | schema.md:27 vs db/db.go:223 (writer.SetMaxOpenConns(1)), :239-244 (reader pool max(4, NumCPU), database.max_readers) |
Describe the writer/reader split and the knob |
protocol.md
| ID | Finding | Evidence | Fix |
|---|---|---|---|
| B-04 | DM chat_message/chat_edited/chat_deleted/reaction_update documented as unsequenced and excluded from the ring buffer; they are seq-stamped, ring-buffered AND persisted (tier-1 replayable) |
protocol.md:98,263-264,1489-1494 vs ws/emit.go:23-26 → hub_broadcast.go:923-935 (nextSeq + replayBuf.Push + persistEvent); emit sites handlers_chat.go:102,123,146, handlers_reaction.go:47 |
Fix the seq tables + reconnection section |
| B-05 | plugin_broadcast documented "Has seq? No"; it rides the sequenced, replayable channel-broadcast path |
protocol.md:1524 vs ws/event.go:347-357, hub_broadcast.go:939-964 |
Flip the row to Yes |
| B-06 | RATE_LIMITED WS errors claimed to "include retry_after in seconds"; no WS error payload ever carries it (only REST 429 headers do) |
protocol.md:1399 vs ws/messages.go:332-358, ws/event.go:7-12; repo-wide grep negative |
Delete the clause (or implement the field) |
| B-07 | Five c2s types documented rate limit "None" are all limited: channel_focus 5/s, mark_read 5/s, call_decline 1/3s, chat_command 5/s, ping 2/s |
protocol.md:1461,1462,1478,1479,1480,1535 vs ws/handlers_presence.go:19-22,107,135, handlers_call.go:15-18,75-77, handlers_command.go:29-36,49-51 (comment cites OC-0091 — limiter added, doc never updated), handlers_ping.go:14-16 |
Document all five incl. silent-drop vs RATE_LIMITED behavior |
| B-08 | E2EE prose says "Both E2EE message types are rate limited at 5 per second", contradicting the doc's own table (offers 64/s) and the code; the inner 5/s-per-target offer cap is undocumented | protocol.md:1112 vs :1433,1437-1439; ws/voice_e2ee.go:16-24,213-223 |
Fix the prose; document the per-target inner cap |
api.md
| ID | Finding | Evidence | Fix |
|---|---|---|---|
| B-09 | GET /api/v1/diagnostics/connectivity documented "any authenticated user"; code requires ADMINISTRATOR (H-8) — a non-admin following the doc gets 403. Also: limiter is per-IP not "per user"; livekit_url is sanitized to host:port, not the documented full URL |
api.md:2472,2473,2485 vs api/router.go:153-158, middleware.go:228-229, diagnostics_handler.go:54-58 |
Fix auth line, limiter scope, example |
| B-10 | Error-code table documents SERVER_ERROR / INTERNAL / TOO_LARGE — none ever emitted by REST (real 500 code is INTERNAL_ERROR, ×60+ sites; oversize upload is 400 BAD_REQUEST); STORAGE_ERROR 507 missing entirely. A client switching on documented codes never matches |
api.md:57-58,119,181,229,310 vs greps (zero SERVER_ERROR/TOO_LARGE emissions), upload_handler.go:115-146, storage/storage.go:172, ws/errors.go:6 |
Replace with INTERNAL_ERROR, correct the 413 row, add STORAGE_ERROR |
5. New findings — FRAGILE
F-1 — Blocking a user does not evict them from a live 1:1 DM voice call.
BlockUser commits with zero side effects (service/block.go:24-54;
api/dm_handler.go:83-88 takes no evictor). The only block re-checks for a
user already in voice are client-volunteered (ws/voice_join.go:136-141 join
gate; :579-590 token refresh). The minute-sweep — whose own invariant
comment says "Revocation must evict a live session, not merely block the next
join" (ws/hub_sweep.go:167) — re-checks only role CONNECT_VOICE
(hub_sweep.go:185,328-354), which has no DM/block branch; LiveKit validates
tokens at join only, so a client that stops calling voice_token_refresh
keeps its SFU session indefinitely. Contrast: CloseDM evicts via
dmVoiceEvictor (api/dm_handler.go:236-242). This is the same
guard-asymmetry family as A-2026-08-03 (block check on rings), one sink over.
Adversarially verified: CONFIRMED.
Fix: have handleBlockUser evict via the existing dmVoiceEvictor for
the pair's shared 1:1 DM channel (mirroring handleCloseDM), or add a
DM-block re-check to the sweep.
F-2 — Role reassignment racing a WS handshake leaves the socket on
old-role subscriptions. The handshake resolves permissions from the
auth-time c.user snapshot (ws/serve.go:124-132; computeAllowedChannels
via GetRoleByID(user.RoleID) at :670, used by both :306 and :736), and
nothing revalidates handshake-time subscriptions (the OC-0024 recheck covers
only applySetChannelID; the sweep re-checks CONNECT_VOICE only). The
demotion's live revocation, revokeUnreadableChannels
(hub_broadcast.go:797-879), early-returns if the user is not yet in
h.clients (:818-822) and its Unsubscribe no-ops on a replaced client via
the pubsub identity guard (pubsub.go:133-134) — so a role change landing in
the handshake window is never applied to that socket, which then receives
events for channels the new role cannot see until the next reconnect.
RefreshChannelVisibility handles both hazards explicitly (hub_broadcast.go:443-444);
the role path predates that pattern. Adversarially verified: CONFIRMED.
Fix: re-read the user row inside reconnectPrecheck/handleFreshConnect,
and make revokeUnreadableChannels re-resolve the live client before
sendMsg/Unsubscribe, mirroring RefreshChannelVisibility.
F-3 — Auth lockout persistence failures are silent. All three lockout
writes discard errors with no log: _ = r.store.UpsertLockout(...)
(auth/ratelimit.go:172), _ = ...DeleteLockout (:234),
_ = ...CleanupExpiredLockouts (:283) — while the load path got exactly this
fix as ledger finding OC-0061 (the file's only slog call, :88). The doc
comment at :162-164 says "The persist write must land once the lockout is
decided", yet a failed write leaves zero trace and the lockout evaporates on
restart. Adversarially verified: CONFIRMED (not an OC-0061 duplicate — this
is the symmetric write-path gap).
Fix: mirror OC-0061 — slog.Warn on all three error paths.
F-4 — Session-cap eviction failure is invisible (observability gap;
downgraded from a stronger claim). CreateSession discards the H-6
eviction error (db/auth_queries.go:244 _ = d.q.EvictOldestSessions(...))
with no log anywhere in the chain. The verifier established the cap is not
durably disabled — the DELETE trims to the newest 24 so the next successful
login self-heals, and a persistent DB failure also fails the subsequent
InsertSession — so the residue is: a transient failure lets one session
exceed the cap, and no failure of a security control is ever logged.
Adversarially verified: DOWNGRADED to this statement.
Fix: slog.Warn on eviction error only.
F-5 — One client unit test is a load-sensitive timeout flake.
tests/unit/message-list.test.ts › "does not report success when
renderWindow's own >30-in-2s breaker drops the rebuild" performs 30
synchronous full renderWindow rebuilds of a 100-row jsdom list inside the
default 5s vitest timeout. In this session's full run (concurrent with go test on a 4-CPU box) it timed out — 5044/5045 — then passed 3/3 in isolation
(~4.2s per whole-file run). Same class as the open T-2026-07-25-20 watch item.
Fix: give that test an explicit larger timeout (or shrink the fixture) so
CI-under-load cannot produce a spurious red.
F-6 — Client replay-dedup machinery can never engage (dead safety net +
misleading tests). The server always writes auth_ok before the replay
burst (ws/serve.go:531,537-544), but the client creates replayDedup in the
socket-open handler and clears it when auth_ok is processed
(ws.ts:463-465, :358) — so the dedup set and isReplaying() (ws.ts:788)
are inactive for every real replayed frame; the dispatcher's gates at
dispatcher.ts:580,592 never fire, and its own comment at :612-618 admits
the timing. The pinning tests inject replay frames in an order a
spec-compliant server never produces (ws-reconnect.test.ts:409-497).
Meanwhile the server can legitimately duplicate a voice frame on resume (the
voice supplement is re-read outside seqMu, serve.go:263-271,553-556), which
is what the machinery was presumably for. Today's no-op behavior is actually
correct for unread counts, making this inert-but-misleading rather than
harmful. Adversarially verified: CONFIRMED.
Fix: either delete the inert machinery (and fix comments + tests) or move
the clear to end-of-burst — decide, don't leave it half-true.
6. New findings — DEBT
Grouped; every item carries its evidence at the cited lines.
Docs/comments
- D-01
protocol.mderror-code table missingBAD_PAYLOADandNOT_KEY_HOLDER(emitted atws/voice_e2ee.go:119-199). - D-02
protocol.mdready/member_join field lists incomplete:voice_statesomitsusername/speaking/camera/screenshare,rolesomitsposition/is_default,member_joinomits its top-levelstatusfield (serve_ready.go:365,db/models.go:99-106,234-245,ws/messages.go:56-67). - D-03
api.md: body-cap exemption list wrong (3 exempt prefixes, not 1 —api/constants.go:177-184);identity_public_keyaccepted byPATCH /users/mebut undocumented (profile_handler.go:29-37,100-114); plugin endpoints return plain-text errors contradicting the "all errors use the JSON envelope" claim, and theX-Plugin-Runtimeheader is undocumented (plugins_handler.go:51-136);/health's 503 degraded state +reasonfield undocumented (router.go:446-453,509-544); metrics/LiveKit endpoints have their own CIDR keys with admin fallback, not "admin CIDRs" (config/config.go:219-233);updates/applycan 409RESTART_PENDING/UPDATE_IN_PROGRESS, absent from its table (admin/restart.go:104-112). - D-04 Stale comments:
buildReady's claims "no slow_mode … voice_* extras" contradicted byreadyChannelPayloadsdirectly above (serve_ready.go:312-315vs:216-224);tsconfig.e2e.json:2-3+ci.yml:155-156say "47 spec files + the three playwright configs" (52 and four); stray word "ponytail" inlogctx.go:49. - D-05 Three plan headers understate shipped work (bug-detection-improvements, security-scan-2022-remediation F3-3, discord-parity's sounds-table row) — all drift in the direction that misdirects planning toward finished work.
- D-06 Branch-flow docs contradict practice: README:230-232 and
contributing.md:152-153say branch fromdev, PR todev, "dev is merged to main for releases"; rootCLAUDE.md:63says branch frommain, PR tomain. Practice matches CLAUDE.md — the 5 most recent merged PRs (#1390-#1394) arebase:main, and the sync direction is inverted (commit40cba79mergesmainintodev;devis 4 graph-refresh commits ahead, 0 behind).
Dead code / hygiene (deadcode + knip corroborated)
- D-07
Hub.hasChannelPermis unreachable in production; its doc comment claims the sweep calls it, but the sweep useshasChannelPermChecked(ws/handlers.go:286-292vshub_sweep.go:185,320-354); the F5 guarantee test pins the dead copy (voice_perm_stale_test.go). - D-08
UserService.SetCustomStatus/ClearCustomStatus: zero production callers; "Called on logout" comment false (logout writes the DB directly,auth_handler.go:584); duplicates the length validationHandlePresenceUpdateowns (service/user.go:201-215vschannel.go:183). - D-09 Production-unused hub APIs:
PubSub.PublishHigh(pubsub.go:181),Hub.BroadcastToAllLow(hub_broadcast.go:907),EventRingBuffer.EventsSince(ringbuffer.go:43), and theHub.Register/UnregisterclientEvents queue (hub.go:446-453, whoseRunbranch is test-only and would fail closed for a future production caller);TopicsForClient's "debugging and tests" doc is stale — it has a production caller (hub_broadcast.go:850). - D-10 gofmt drift:
admin/handlers_users_broadcast_test.go(one misaligned field); no gofmt/gofumpt gate exists in CI or.golangci.yml. - D-11 25 non-blocking
no-underscore-danglelint warnings accumulated inlivekitSession.ts:1656-1759.
Boundary asymmetries (all privileged-actor or minor; the next members of the audit-2026-08-04 defect family — worth sweeping in one pass)
- D-12
typing_startrequires only READ_MESSAGES on guild channels while the send it advertises requires SEND_MESSAGES (announcement: MANAGE_MESSAGES) — a send-muted member can still broadcast typing (service/channel.go:133vsmessage_perms.go:93-99; the DM branch is fully gated). - D-13
getPermChannelanswers a DM id with 400 "DM channels do not support permission overrides" while its siblinggetAdminChanneldeliberately answers 404 to avoid confirming which ids are DMs (A-2026-08-02) — same perimeter, one oracle left (admin/handlers_channel_perms.go:40-44vshandlers_channels.go:44-61). - D-14 Admin channel create/patch has no length cap or sanitation on
name/topic/category (only the 1 MiB body cap); the group-DM writer of the
same column caps at 100 runes and its comment claims a channel-name parity
that does not exist (
admin/handlers_channels.go:104-116,171-183vsservice/dm.go:173-175,234-237). - D-15 Invite create/revoke are the only privileged mutation family with no
audit row and no log line — an operator cannot see who minted an invite
(
service/invite.go:29-77; every sibling family audits: roles, bans, emoji, settings, channels, purge, auth). - D-16
channel_focus's read-state write is called "the load-bearing half" in its own comment, then discarded unlogged (service/channel.go:275-286); a persistent failure means unread badges never clear with zero trace.
7. Verified clean (read end-to-end and found sound)
Recorded so future audits don't re-spend the effort. Hub dispatch loop (panic
breaker, stop idempotence, register/replacement ordering); every seqMu
critical section (no lock held across real I/O; shed frames never burn a seq);
lock ordering across seqMu/h.mu/ps.mu/rb.mu/c.mu/keyHolderMu with
no reverse edge; pubsub index hygiene and identity guards; three-tier client
queues + writePump anti-starvation; topic rate limiter and pruner/persister
lifecycle (no goroutine leaks found); ring-buffer tier boundaries (both edges
fall conservative to full ready) and the closed lost-update window
(register-inside-seqMu); cold-tier contiguity probes including the
correct off-by-one on the prune gap; replay authorization incl. the OC-0206
watermark re-check under seqMu; seq reseeding across restarts; client
backoff/generation discipline (OC-0219) and optimistic-send reconciliation
across reconnects; REST/WS message-family parity (send/edit/delete/reaction/
pin/purge all funnel through the same service gates); REST-mutation fan-out
ordering (post-commit, context.WithoutCancel, watermark bumps); admin
users/bans/roles/override families (hierarchy, audit rows, invalidation
ordering); plugin sandbox mitigations (§2.1); protocol type inventory
(27 c2s / 39 s2c identical across schema, Go, TS, and doc tables) and
generated-code gates (protocol codegen byte-identical; go mod tidy clean;
knip zero findings; no TS zombie modules; Rust unwraps confined to test
modules); E2EE section of protocol.md including the eacba10 epoch change —
fully documented and accurate.
8. Verdict
MUST fix (before the next feature work lands on top):
- The ten BROKEN reference-doc statements (§4) — one spec-refresh PR, same shape as 2026-08-04's C1. Docs are the record everything else keys off, and two of the ten are security-relevant reader traps (B-09 auth claim, B-07 undocumented limits).
- F-1 block/voice eviction — it is the live continuation of the guard-asymmetry family the security review closed; small, testable fix.
- F-3 + F-4 + D-16 silent-failure logging — three
slog.Warnlines restoring visibility to security/UX controls.
CAN wait (scheduled, not urgent): F-2 (handshake race — real but narrow window; needs a careful hub change + deadlock-tag run), F-5 (test timeout bump), F-6 (delete-or-fix decision), all §6 DEBT including the D-12..D-15 asymmetry sweep and the D-06 contributing-docs correction, and every §2 carried architecture item (service-layer consolidation, hub constructor, client constants, monoliths).
Already done (no action): everything in §2 marked FIXED/intact — backup
restore, CI gates, .nvmrc, plugin mitigations, DC-08, W2-4/W3-3, protocol
codegen and ledger hygiene — plus §7's verified-clean list. The bug-hunt
ledger has zero open findings.
9. Roadmap
9.1 Fix order (every BROKEN + FRAGILE, dependencies first, smallest verifiable step first)
- F-5 test-timeout bump (one line in one test) — protects the CI signal every later step relies on.
- §4 B-01..B-10 spec-refresh PR for schema.md/protocol.md/api.md, folding in D-01..D-05 while in the files — pure docs, zero code risk, ends the "docs are the record" debt before code changes add more drift.
- F-3 + F-4 + D-16 logging PR (three
slog.Warnsites, mirrors the shipped OC-0061 pattern) — trivial, independently testable. - F-1 block → DM-voice eviction (reuses the existing
dmVoiceEvictorseamCloseDMalready exercises; pin with a test like A-2026-08-03's) — smallest security-adjacent code fix. - F-2 handshake/role-change race — hub-internal, needs the
ci-checkdeadlock pass; do after 4 so the voice-eviction test infrastructure is fresh. - F-6 replay-dedup decision (delete vs repair) — last because it needs an owner call; either branch also fixes the misleading tests/comments.
9.2 Alpha exit — what stands between here and a stable beta
(each item backed by a finding above or an existing plan)
- Graduate the two non-blocking CI legs:
admin-e2e(soak criterion already written inci.yml:302-307) and atauri-buildpush-to-main or nightly trigger (2026-07-19 §5, still true) — after which every surface is gated. - Add a gofmt gate (D-10 proved the gap; one CI line).
- Close the DNS-rebinding follow-up (tauri-capability-narrowing): move
link-preview fetches behind a Rust command and drop the
https://*capability — the last open item from the F3 security track. - Finish the two infrastructure-roadmap leftovers: TOTP/partial-auth persister seam, and publish the capacity numbers the load-baseline workflow exists to produce.
- Start the service-layer consolidation at
MountAuthRoutes(2026-04-07 #9, the designated first step of backlog 12), then the hub constructor cleanup (A-2026-07-11 S-effort half). - Unify
channelCanSendonto the permission checker — the last hand-mirrored visibility/permission copy. - Protocol rev decision (owner): emit-or-drop
voice_speakers/member_leave(DC-14), and wire the plugin wire types into the client or document desktop non-support (D-02's sibling). DC-14 was decided 2026-08-28: dropped in B2-1 (S-15). - Docs process hardening: the 15-day re-drift (§4) says the per-PR rule alone doesn't hold; add the docs-check line to CI or the PR template checklist with teeth (a grep-able "docs reviewed" gate), plus the D-06 contributing-flow correction so new contributors aim at the right branch.
9.3 Ideas — opinion, not findings
- Slash-commands Phase A (plan exists, design-only today) — first user-visible plugin payoff.
- Role hoist/mentionable +
@RoleNamementions (discord-parity leftover). - Categories as real entities (discord-parity leftover).
- Linux ARM64 server binary + prebuilt Docker image (README platform-matrix gap).
- macOS PTT support (currently a stub, noted in discord-parity's dead-code list).