Rows 1, 2, 4, 6, 7, 8, 9 were all closed in the closure table but never struck in the section 6 backlog, making the remaining work look ~4x larger than it is. Only rows 10 (partial) and 12 are still open. Also drops the stale "V1/V2 dispatch" blurb from the architecture index, which contradicted websocket.md after the V1 registry was deleted in #1196. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
26 KiB
OwnCord — Architectural Audit & Spec-Conformance Review
Date: 2026-07-19
Branch: claude/blueprints-architectural-audit-k927qb (audited tree: ddc49f0 = main)
Scope: whole-system architecture mapping + code-by-spec conformance. Read-only — no code or spec changes ship with this audit; the companion blueprint set lives in docs/architecture/.
Relationship to prior audit: successor to audit-2026-04-07.md, which remains the closure tracker for its own findings. Carried-over items are re-verified in §1, not restated.
Finding closure status (maintained; update statuses in place)
Standing rule: every HIGH below gets either a closing commit link or an explicit accepted-risk note before the beta gate. MEDIUMs are folded into the backlog (§6) and closed opportunistically.
| ID | Sev | Finding | Status |
|---|---|---|---|
| A-2026-07-01 | HIGH | announcement channel type: documented in 3 specs and offered by the admin API, but hard-rejected by DB triggers |
RESOLVED 2026-07-19 — implemented end-to-end (D1): migration 016 allows the type; posting requires MANAGE_MESSAGES; specs + client updated |
| A-2026-07-02 | HIGH | Client HTTP path accepts any TLS certificate (allowSelfSigned hardcoded; no TOFU pinning, unlike WS/LiveKit paths) |
CLOSED 2026-07-19 — HTTP TOFU proxy implemented (http_proxy.rs + httpProxy.ts); REST path now cert-pinned, acceptInvalidCerts removed |
| A-2026-07-03 | HIGH | Reference specs (api.md / protocol.md / schema.md) frozen at 2026-04-02; systemic drift incl. whole undocumented subsystems (voice E2EE, plugins) | CLOSED 2026-07-19 — full refresh of api.md/protocol.md/schema.md landed (all §2 fix-spec items); keep-current-per-PR rule now applies |
| A-2026-07-04 | HIGH | Client unit test suite "KNOWN RED" and non-blocking in CI; E2E never gated | CLOSED 2026-07-20 — the "KNOWN RED" premise was stale: suite verified green on main (3261/3261, 114 files) and the annotations in both CLAUDE.md files + ci.yml corrected to "green, must stay green". Root cause of the false premise: on Node 22+, native Web Storage shadows jsdom's localStorage, failing ~478 unrelated tests locally; with NODE_OPTIONS=--no-experimental-webstorage (CI pins Node 20) the suite is fully green. Documented in the client CLAUDE.md and the ci-check skill so it is not re-misdiagnosed. Flipping client-tests to blocking + adding the nightly Playwright gate remain tracked as backlog #10 |
| A-2026-07-05 | MEDIUM | Dead sqlc layer: Server/db/dbgen/ (~3.5k LOC) generated + CI-verified but imported by nothing |
RESOLVED 2026-07-19 — dbgen wired into db.DB; 97 methods across all domains delegate to it (no longer dead). Remaining raw queries (variable IN / FTS / tx) tracked in plans/sqlc-adoption.md |
| A-2026-07-06 | MEDIUM | Three coexisting DB-access styles (raw *db.DB in api/admin/ws, store.Store under service, dead dbgen) |
RESOLVED 2026-07-19 — collapsed to a single sqlc-backed db package: dbgen wired in (D2) and the store seam deleted (D3). The service layer depends on a narrow service.Store interface *db.DB satisfies; ws/plugin similarly. Broadening service-only access above the remaining direct-db handlers is the residual layering work (A-2026-07-06 backlog item 12) |
| A-2026-07-07 | MEDIUM | Channel-visibility logic duplicated across ~4 sites with "must mirror" comments | RESOLVED 2026-07-20 (D9) — all four sites (REST ListVisibleChannels, ws buildReady, replay computeAllowedChannels, hub RefreshChannelVisibility) route through one permissions.Checker predicate (VisibleChannelIDs / HasChannelPerm); REST/WS agreement test added |
| A-2026-07-08 | MEDIUM | Protocol constants on both sides claim generation from docs/protocol-schema.json, which does not exist in the repo |
CLOSED 2026-07-19 — codegen implemented: docs/protocol-schema.json + Server/scripts/genprotocol + make protocol-verify CI gate |
| A-2026-07-09 | MEDIUM | Dual V1+V2 WS dispatch (strangler-fig) still live; two parsers/registries to keep in sync | RESOLVED 2026-07-20 (D10) — the 3 remaining V1 types (chat_command, voice_join, voice_leave) ported to typed V2 handlers; the V1 registry + fallback path deleted. handleMessage has a single dispatch generation. Server-internal only, no wire change |
| A-2026-07-10 | MEDIUM | api.NewRouter god-constructor: builds services, hub, LiveKit, updater, admin, plugins; spawns goroutines; mounts everything |
OPEN |
| A-2026-07-11 | MEDIUM | ws.Hub mega-object with post-construction Set* wiring ("must be called before Run") |
OPEN |
| A-2026-07-12 | MEDIUM | Abandoned SolidJS beachhead still in-tree; docs/client-architecture.md describes the abandoned architecture |
CLOSED 2026-07-19 — beachhead, adapters, build plugin, and Solid deps removed; client-architecture.md retired in favor of architecture/client.md |
| A-2026-07-13 | LOW | Dead schema: sounds table survives soundboard removal (correction 2026-07-19: audit_log_v6 is only a transient rename inside migration 003, not a coexisting table) |
OPEN |
| A-2026-07-14 | LOW | Scattered client constants (#5865F2 ×18, localhost:8443 ×3); 64 timer call sites with manual lifecycle |
OPEN |
| A-2026-07-15 | LOW | docs/plans/security-hardening-remediation.md partly stale (references deleted store/postgres.go) |
OPEN |
Table of Contents
- Carried-over items from audit-2026-04-07
- Spec-conformance matrix
- Server architecture findings
- Client architecture findings
- Process & CI findings
- Prioritized improvement backlog
1. Carried-over items from audit-2026-04-07
Re-verified in today's tree. Statuses below reflect the code, not the prior audit's table; details stay in audit-2026-04-07.md.
| Prior # | Sev | Finding (one-line) | Re-verification (2026-07-19) |
|---|---|---|---|
| 1–5 | CRITICAL | Plugin governance (timeout, storage isolation, ACL, event rate limit, HTTP exfiltration) | Unchanged since prior closure table; plugins still default-disabled (plugins.enabled: false), which is the standing mitigation |
| 6 | HIGH | Server/store/ untested |
RESOLVED 2026-07-19 (D3) — the store/ package is deleted rather than tested. SQLiteStore was a pure pass-through to *db.DB; its event/plugin methods moved into db (event_queries.go, plugin_queries.go). Consumers now depend on narrow interfaces *db.DB satisfies (service.Store, ws.EventStore, plugin.PluginStore), and the former MemStore-based unit tests run against a real in-memory SQLite db — so the code paths that were untested through the seam are now exercised directly |
| 7 | HIGH | Client unit coverage | Suite is large (157 test files) and green; flipping client-tests to blocking is backlog #10 — see A-2026-07-04 |
| 9 | MEDIUM | auth_handler bypasses service layer | Confirmed open — Server/api/router.go:101 passes database *db.DB to MountAuthRoutes while sibling mounts receive svc |
| 10 | MEDIUM | Audit-trail write failures silently ignored | RESOLVED 2026-07-20 (D9) — best-effort audit writes are kept as the convention, but no longer silently discarded: every LogAudit call site now routes through the shared db.WriteAudit helper (db/audit.go), which logs a failed write with actor/action/target context without failing the request. All ~26 call sites across admin/api/ws/service converted from _ = LogAudit(...); pinned by db/audit_test.go |
| 11 | MEDIUM | E2E not in CI | Confirmed open — no Playwright job exists in .github/workflows/ci.yml |
| W3-4 (remediation plan) | LOW | Contradictory upload cache header | Fixed 2026-07-19 — now private, no-cache per the remediation plan's prescription |
2. Spec-conformance matrix
The three reference specs were last meaningfully edited 2026-04-02
(git log on each file); the code is at v1.1.0-alpha.2 with substantial July
2026 changes. Resolution column: fix-spec (doc catches up to code),
fix-code (code is wrong), decide (product decision needed first).
Update 2026-07-19: the spec refresh landed — every fix-spec row below is resolved in the specs; evidence is retained for the record. Item A remains open pending the D1 implementation.
2.1 docs/api.md
| ID | Spec says | Code does | Evidence | Sev | Resolution |
|---|---|---|---|---|---|
| B | GET /api/v1/info returns {name, version} |
version removed (anti-fingerprinting, "C-2") |
Server/api/router.go infoResponse{Name} only |
MEDIUM | fix-spec |
| C | GET /health returns version |
version removed for the same reason |
Server/api/router.go healthResponse{Status, Uptime, OnlineUsers} |
MEDIUM | fix-spec |
| F | — (absent) | Profile surface exists: PATCH /api/v1/users/me, PUT /users/me/password, GET/DELETE /users/me/sessions |
Server/api/profile_handler.go (MountProfileRoutes) |
MEDIUM | fix-spec |
| G | — (absent) | Plugin admin REST surface: /api/v1/admin/plugins (list/enable/disable/uninstall/install-zip) |
Server/api/router.go:250 |
MEDIUM | fix-spec |
2.2 docs/protocol.md
| ID | Spec says | Code does | Evidence | Sev | Resolution |
|---|---|---|---|---|---|
| D1 | — (voice E2EE absent) | Full E2EE signaling: client voice_e2ee_announce/voice_e2ee_offer, server broadcast/relay of both |
Server/ws/message_types.go, Server/ws/voice_e2ee.go; flow in architecture/voice-e2ee.md |
HIGH | fix-spec |
| D2 | — (absent) | voice_speakers server event |
Server/ws/message_types.go (MsgTypeVoiceSpeakers) |
LOW | fix-spec |
| D3 | — (absent) | user_update broadcast on profile change |
Server/ws/message_types.go; wired via MountProfileRoutes broadcaster |
LOW | fix-spec |
| D4 | member_leave appears only in the seq table |
Fully implemented broadcast | Server/ws/message_types.go (MsgTypeMemberLeave) |
LOW | fix-spec |
| E | auth_ok payload = {user, server_name, motd} |
Also carries replay_source (none|buffer|db) |
Server/ws/serve.go auth path; CHANGELOG Phase B |
LOW | fix-spec |
2.3 docs/schema.md
| ID | Spec says | Code does | Evidence | Sev | Resolution |
|---|---|---|---|---|---|
| A | Channel types: text, voice, announcement, dm |
RESOLVED — migration 016 allows announcement; posting gated on MANAGE_MESSAGES (Server/service/message.go) |
Server/migrations/016_announcement_channel_type.sql |
HIGH | done (fix-code, D1) |
| H | Migration history stops at "008", with wrong numbering (two 003_* entries; DM tables labeled 008) |
15 migrations exist, 001–015; real order diverges from 004 onward |
Server/migrations/ directory listing |
MEDIUM | fix-spec |
| I | — (absent) | 9 tables undocumented: login_attempts, settings, emoji, sounds, rate_lockouts, user_blocks, events, plugins, plugin_kv |
Server/migrations/001,011,012,014,015 — see architecture/data-model.md |
MEDIUM | fix-spec |
| J | attachments DDL without uploader_id |
Column added for upload-ownership checks | Server/migrations/010_attachment_uploader.sql |
MEDIUM | fix-spec |
| K | — (sqlc unmentioned) | sqlc.yaml + Server/db/dbgen/ exist (currently dead — A-2026-07-05) |
sqlc.yaml, Server/db/dbgen/ |
LOW | fix-spec (document whichever way A-2026-07-05 resolves) |
Systemic conclusion: drift is not isolated typos — entire subsystems (E2EE signaling, plugins, six migrations) postdate the specs. Item A is the only case where code and spec actively contradict at runtime: an admin can select a channel type the database will refuse to insert. Recommended handling: one coherent spec-refresh PR (backlog item 4) rather than piecemeal edits, plus a decision on A first.
3. Server architecture findings
Verdict: the intended layering (api → service → db, since D3 collapsed the
former store seam) is sound and the service layer respects it; test
discipline is excellent (test LOC ≈ 1.6× source; race + deadlock + mutation
tooling). The findings are about the seams that grew around that design.
| ID | Sev | Area | Evidence | Finding | Recommendation | Effort |
|---|---|---|---|---|---|---|
| A-2026-07-05 | MEDIUM | Data layer | Server/db/dbgen/ (~3.5k LOC), sqlc.yaml, CI sqlc-verify job |
sqlc output is generated, version-pinned, CI-verified — and imported by nothing. Hand-written raw SQL in Server/db/*_queries.go is what runs. |
Decide the Phase-A question: adopt dbgen inside db.DB method bodies, or delete dbgen/ + queries/ + the CI job. Either ends the illusion of a second data layer. |
S |
| A-2026-07-06 | MEDIUM | Layering | All Mount*Routes signatures take database *db.DB alongside svc (Server/api/*_handler.go); Server/admin handlers take *db.DB |
~359 direct database.* calls above the store seam; three access styles coexist. The abstraction exists but cannot be relied on (e.g. for a future backend swap or for test doubles). |
Consolidate incrementally: new handlers service-only; migrate one mount per PR, starting with auth (prior #9). | L |
| A-2026-07-07 | MEDIUM | Correctness risk | Server/ws/serve.go (buildReady, computeAllowedChannels), Server/ws/hub.go (RefreshChannelVisibility), REST handleListChannels |
Channel-visibility filtering implemented ~4× with comments instructing they "must mirror" each other. The recent private-channel fixes (e.g. 2bfe6d6) show this is actively churning — a drift between copies is an information-disclosure bug waiting to happen. |
Resolved 2026-07-20 (D9) — added permissions.Checker.VisibleChannelIDs; all four sites route through it (RefreshChannelVisibility uses the single-channel HasChannelPerm); REST/WS agreement test added. |
M |
| A-2026-07-08 | MEDIUM | Protocol integrity | Server/ws/message_types.go header comment; Client/…/src/lib/protocolTypes.ts header + "Extensions (not in protocol-schema.json…)" comments |
Both sides claim docs/protocol-schema.json is the generated single source of truth. The file does not exist; the two constant sets are maintained by hand and have already grown divergent "extension" entries. |
Either commit a real protocol-schema.json + generator (best: also emits protocol.md tables), or delete the claim and add a cross-language equality test over the two constant sets. |
M |
| A-2026-07-09 | MEDIUM | Real-time | Server/ws/handlers.go (handleMessage V2-then-V1 fallback), dual registration in NewHub (Server/ws/hub.go) |
Strangler-fig V1+V2 dispatch is live: two parsers (lenient/strict), two registries, per-type duplication. | Resolved 2026-07-20 (D10) — ported the last 3 V1 types (chat_command, voice_join, voice_leave) to typed V2 handlers and deleted the V1 registry + handleMessage fallback; a parity guard test locks the single dispatch path shut. |
M/L |
| A-2026-07-10 | MEDIUM | Composition | Server/api/router.go:34 (NewRouter, ~278 lines) |
God-constructor builds rate limiter, TOTP key, storage, services, hub, LiveKit client+process, updater, admin + plugin handlers; spawns goroutines; returns a cleanup closure covering only one of them. Hard to test wiring in isolation; lifecycle ownership is implicit. | Split construction (a Deps/App struct built in main.go) from route mounting (NewRouter(deps)); return a composite io.Closer. |
M |
| A-2026-07-11 | MEDIUM | Real-time | Server/ws/hub.go (SetLiveKit, SetEventPersister, SetPluginRegistry, …) |
Hub is a mega-object wired post-construction via setters that "must be called before Run" — temporal coupling; a missed setter is a nil-deref at runtime, not a compile error. | Move required collaborators into NewHub params (or an options struct validated before Run). Full Hub decomposition is a separate, larger effort (backlog 12). |
S (constructor) / L (decomposition) |
| — | MEDIUM | Layering | Server/ws/hub.go:182 (refreshSettingsLocked) |
Hub runs inline SELECT value FROM settings WHERE key='server_name' instead of using SettingsStore — the only raw SQL in the real-time layer. |
Fixed 2026-07-19 — now uses db.GetSetting; full consolidation folds into A-2026-07-06. |
S |
| — | LOW | Scaling posture | Server/auth/ratelimit.go (documented), in-memory pub/sub + ring buffer, process-local TOTP replay |
Single-instance coupling is structural and documented — this is a deliberate design, not a bug. Recorded here so the constraint stays visible (architecture/system-overview.md D8). | No action now; revisit only if multi-instance ever becomes a goal. | — |
| A-2026-07-13 | LOW | Schema hygiene | sounds table (Server/migrations/001), audit_log + audit_log_v6 (003) |
Dead/duplicated schema: soundboard was removed but its table remains; two audit-log tables coexist after the 003 rebuild. | Add a cleanup migration (drop sounds, finish the audit_log consolidation) next time a migration ships anyway. |
S |
4. Client architecture findings
Verdict: the client's fundamentals are strong — immutable store discipline, TOFU pinning in Rust, keychain credentials with IPC redaction, generation counters against stale listeners, and an unusually deep test/tooling stack (Vitest, dual Playwright suites, Stryker, oxlint + type-checked ESLint, Knip). Findings target structure and one security gap.
| ID | Sev | Area | Evidence | Finding | Recommendation | Effort |
|---|---|---|---|---|---|---|
| A-2026-07-02 | HIGH | Security | src/main.ts (allowSelfSigned: true at API-client construction); src-tauri http plugin built with dangerous-settings |
Every REST call accepts any certificate. The WS and LiveKit paths pin TOFU fingerprints in Rust; the HTTP path — which carries the auth token on every request — does not. An active MITM can capture tokens without triggering the cert-mismatch modal. | FIXED 2026-07-19 — TOFU HTTP proxy (src-tauri/src/http_proxy.rs + src/lib/httpProxy.ts) tunnels REST through a cert-pinned loopback; allowSelfSigned/acceptInvalidCerts and the dangerous-settings feature removed. |
M |
| A-2026-07-12 | MEDIUM | Coherence | src/components/solid/ (154 LOC), src/lib/solidMount.ts, src/lib/solidAdapter.ts, vite-plugin-solid config; CHANGELOG "Solid.js migration (abandoned)" |
The abandoned migration's beachhead, adapters, build plugin, and test deps remain, and docs/client-architecture.md (2026-03-30) still describes a SolidJS client. Two mental models for contributors, one of them false. |
Delete the beachhead + adapters + build plugin; replace client-architecture.md content with a pointer to architecture/client.md or a rewrite. |
S |
| — | MEDIUM | Maintainability | src/lib/livekitSession.ts (1,719 LOC); AccountTab.ts (845), SidebarArea.ts (812), LoginForm.ts (699) |
livekitSession.ts owns the connection state machine, E2EE, track management, reconnection, and diagnostics in one class. It is the highest-risk file to modify in the client. |
Extract E2EE (already has e2eeCrypto.ts as a seam) and track management into collaborators; keep the state machine as the core. Settle for splitting the settings tabs opportunistically. |
M |
| — | MEDIUM | State | src/stores/voice.store.ts imports members/auth stores; auth.store.clearAuth() reaches into leaveVoice() + notification cleanup; business logic in main.ts subscribers |
Cross-store singleton coupling: teardown ordering lives implicitly in import graphs and bootstrap subscribers. | Introduce a thin session-lifecycle module (login/logout orchestration) that calls stores, so stores stop calling each other. | M |
| A-2026-07-14 | LOW | Hygiene | #5865F2 literal ×18 across 11 files (main.ts, ServerPanel.ts, DmSidebar.ts, MemberPickerModal.ts, SidebarArea.ts, …); localhost:8443 ×3; 64 setTimeout/setInterval sites; 12 innerHTML uses |
Scattered magic values and manual timer lifecycles; src/lib/constants.ts holds a single constant. |
Centralize into constants/tokens; adopt a tiny managedTimer(destroyScope) helper so destroy() paths can't leak intervals. |
S |
| — | LOW | Error handling | catch {} swallows in preferences.ts, ws.ts unsubscribe cleanup, disconnectProxy; widespread void-prefixed fire-and-forget |
Mostly deliberate (per lint config), but a handful of swallows hide real failures (e.g. preference persistence silently failing). | Log-at-debug in the swallow sites; keep the pattern otherwise. | S |
5. Process & CI findings
| ID | Sev | Evidence | Finding | Recommendation | Effort |
|---|---|---|---|---|---|
| A-2026-07-04 | HIGH | .github/workflows/ci.yml — client-tests job annotated "KNOWN RED pending reboot-plan P2 triage"; no Playwright job |
The Go side is gated hard (race, deadlock tag, govulncheck, golangci-lint, sqlc-verify, 4-tag build matrix) but the client's 157-file test suite is red and non-blocking, and E2E never runs in CI. For an AI-first workflow where "quality [is] validated primarily through automated checks" (README), the client half of that promise is currently unenforced. | Triage the red suite to green, flip client-tests to blocking, then add at least the web Playwright suite as a nightly non-blocking job (prior #11) before promoting it to a gate. |
M |
| A-2026-07-03 | HIGH | git log on docs/api.md, protocol.md, schema.md (all 2026-04-02) vs code churn through 2026-07-19 |
No process keeps the reference specs current — the July burst (events table, plugins, E2EE, private channels) shipped without touching them. | After the one-time refresh (backlog 4), add a PR-checklist line (mirroring the blueprint maintenance rule in architecture/README.md): protocol/API/schema changes update the matching spec in the same PR. | S |
| A-2026-07-15 | LOW | docs/plans/security-hardening-remediation.md W1-3/W3-5 reference Server/store/postgres.go (deleted) |
The remediation plan predates the Postgres removal; two waves partially target dead code. | Annotate the affected items rather than rewriting the plan. | S |
| — | LOW | ci.yml tauri-build job: if: github.event_name == 'pull_request' && github.base_ref == 'main' |
Full client build (incl. Clippy -D warnings, cargo audit) never runs on push to main — a merge that breaks the native build is caught only at the next PR. |
Add a push-to-main trigger for tauri-build (or a nightly). |
S |
6. Prioritized improvement backlog
Ranked by severity × effort; quick wins float within tier. S/M/L ≈ hours / days / week+.
| # | Item | Finding | Sev | Effort |
|---|---|---|---|---|
announcement channel-type contradiction |
A-2026-07-01 | HIGH | S (decision) | |
db/dbgen sqlc layer |
A-2026-07-05 | MEDIUM | S | |
permissions.Checker.VisibleChannelIDs funnels all four sites; REST/WS agreement test added |
A-2026-07-07 | MEDIUM | M | |
| A-2026-07-03 | HIGH | M | ||
Server/store/ (P4 plan) or test it directlystore/ removed; consumers on narrow *db.DB interfaces; tests on real in-memory SQLite |
prior #6 | HIGH | M | |
ws_proxy.rs)http_proxy.rs + httpProxy.ts; acceptInvalidCerts removed |
A-2026-07-02 | HIGH | M | |
LogAudit errors; fix the contradictory upload Cache-Controldb.WriteAudit helper now covers all 26 call sites repo-wide |
prior #10, W3-4 | MEDIUM | S | |
docs/client-architecture.md |
A-2026-07-12 | MEDIUM | S | |
protocol-schema.json ghostgenprotocol) + make protocol-verify CI gate |
A-2026-07-08 | MEDIUM | M | |
| 10 | PARTIAL 2026-07-20 — Green + blocking client unit suite; nightly Playwright. Suite proven green (3296/3296 on merged main), A-2026-07-04 closed. Still open: flip client-tests to blocking in ci.yml; add the nightly Playwright job. Deferred while GitHub Actions minutes are depleted |
A-2026-07-04 | HIGH | M |
| A-2026-07-09 | MEDIUM | M/L | ||
| 12 | Consolidate DB access behind the service layer (start with auth routes, prior #9); then Hub constructor cleanup and decomposition | A-2026-07-06, -10, -11 | MEDIUM | L |
Blueprints referenced throughout live in docs/architecture/;
update a diagram and its doc in the same PR as any structural change to its
source-of-truth files. Line-number evidence in this report is a snapshot of
commit ddc49f0.