The foundation is here. Finish the failure paths.
A second review, shaped around the product Jordan is actually shipping: a terminal-first client, independent servers, voice, peer-to-peer files and out-of-process plugins. Built from his local source, run, probed and measured on September 17.
The first review judged a GitHub snapshot against a generic chat-app bar. Jordan’s reply set the real bar. This pass re-ran everything against his supplied download and corrects the first page where it was wrong or too loose.
Concord has a substantial V1 foundation. This download is not ready to certify as a release candidate, because the existing behavior and its recovery paths are not finished: voice settings race, a server mute reads as a departure, disconnects can evict a live client, the database runs without its intended safeguards, a ban does not revoke a saved token, and history cannot page.
None of that calls for new features or a rewrite. Preserve the terminal-first client, independent servers, optional federated discovery, peer-to-peer files and out-of-process plugins, and close the twelve findings in sprint order.
Release goal: initial features present and working, performance understood, V1 stable enough to carry future plugins. Everything below is judged against that.
The supplied download was hashed, copied into a separate working copy, built, tested, race-checked and probed. Seven added Go test files reproduce the findings. The production files were not changed.
Source boundary: the download has no git metadata, so no upstream commit is asserted, and it may not include Jordan’s work after its September 7 file timestamps. 195 files are SHA-256 hashed in the evidence manifest. 126 Go files: 93 production files with 50,249 lines, 33 test files with 9,341 lines. Coverage of the original suite: client 7.0%, server 33.5%, database 43.0%, plugins 19.2%, Grapevine hub 0.0%, models 79.0%, protocol 84.6%, themes 3.7%. Coverage is a map of missing evidence, not a release score.
Every V1 area exists in source. The right-hand column is the evidence a release claim still needs. The repository’s own known-issues text is partly stale: retention exemptions and reconnect backoff are implemented and need behavioral tests, not an “unimplemented” label.
| V1 area | Present in source and exercised | Remaining release evidence |
|---|---|---|
| Text chat, nicknames, mentions, channels, roles, pins | Implemented. Unit and WebSocket checks. Local delivery measured. | Pagination, server-side authorization, moderation, reconnect and slow-client correctness. |
| Voice | Opus, WebRTC mesh, capture and playback, device settings, VAD, toggle push-to-talk. | F01 and F02. Real 2, 4 and 8-user calls, device changes, mute and deafen, mixed rates, reconnect. |
| WAN connectivity | STUN default exists. | TURN credentials and relay configuration are not exposed in the inspected ICE builder. Validate difficult NAT before promising general WAN voice. |
| P2P attachments | Manifest storage, direct transfer, checksum, Save As, sender registry. End-to-end test passes. | Cancel, lost peer, stalled transfer, partial-file cleanup, retry, concurrent destinations. |
| Retention and exempt channels | Backend and UI paths exist, including add and remove exemption handlers. | Scheduled prune plus announcement exemption plus restore against a realistic database. |
| Discovery and federation | Opt-in registration, HMAC heartbeat, directory sharing, join token flow. | F11. Multi-hub outage and recovery. Exact wording of what a listing discloses. |
| Plugins | Process supervision, service accounts, chat relay, remote panes, typed configuration. Reference plugin wire tests pass. | F09. Actual Tukan and Mynah, compatibility policy, failure recovery, permission boundaries. |
| Themes, help, ASCII, mouse | Broad implementation. Layout, mouse and theme tests present. | Human terminal and device matrix. No new decorative work is needed for readiness. |
| Release packaging | Three binary targets and a native voice build path. | Native builds, install, upgrade and rollback on each platform, with complete artifacts. |
Open a finding for the behavior, the evidence, the acceptance condition and its source. Priorities are release recommendations for this brief, not severity scores. Line numbers refer to the supplied download, which carries no commit hash.
Behavior: Changing server mute or deafen should keep channel membership and gate audio. The server mute handler emits a voice-state update without a channel ID. The client reads a nil channel ID as leaving, removes the voice state and stops the affected local engine, while the database still holds the membership. Separately, self mute and deafen flags are never applied as capture or playback gates in the voice engine.
Evidence: Reproduced the malformed mute event and the retained database membership. Traced the client leave branch. No physical microphone or speaker validation was performed.
Done when: Mute changes preserve the channel ID and full voice state. Explicit capture and playback gates exist. Mute, deafen, unmute, rejoin and moderation are tested with two real audio clients. Honest peers stop accepting audio from a server-muted peer.
Behavior: UpdateConfig writes the audio config struct under the engine mutex. The frame sender reads input gain, push-to-talk, VAD and the VAD threshold without it. Changing settings during capture touches the same memory concurrently.
Evidence: A device-free race-detector probe reproduced multiple conflicting reads and writes. The repository’s own race suite passes because this path is never exercised.
Done when: The audio path reads a coherent immutable or locked config snapshot, and playback and config reads are checked the same way. Codec ownership stays on the current audio goroutine. The new race probe passes.
Behavior: The connection map is keyed only by user ID. A second connection replaces the first, but unregistering the first removes the replacement. Separately, the hub loop calls unregister, which enqueues an offline presence event into the same 256-slot broadcast queue the loop consumes. If that queue is full, its only consumer is blocked trying to enqueue.
Evidence: Both conditions reproduced with deterministic in-process probes. The queue test fills the queue, proves blocking, then drains one entry to recover. It is a conditional deadlock, not a claim that every disconnect hangs.
Done when: A single-session or multi-session policy is chosen and enforced, and removal compares connection identity. The hub loop can never block on its own queue. Replacement, old-socket closure, full queues and slow consumers are tested. Clients get an explicit reconnect and resync instead of silent loss of durable chat events.
Behavior: The shared production database constructor appends _foreign_keys=on&_journal_mode=WAL, but this driver does not read those settings from that string. A fresh on-disk database opened through the constructor reports foreign keys 0 and journal mode delete. That affects integrity and deletion behavior and invalidates any performance assumption built on WAL.
Evidence: Reproduced through the database constructor. Two original cascade tests are explicitly skipped. Their comments should not be read as proof the problem is confined to tests.
Done when: The driver’s supported per-connection pragmas are configured and asserted on initial and replacement connections. Existing orphan rows and migrations are audited before constraints go on. The skipped cascade tests are restored and green. Backup and restore are tested. Foreign keys are not switched on blindly against existing data.
Behavior: Ban closes the currently indexed connection and marks the member banned, but authentication accepts an existing session without checking ban state. The channel permission path also accepts the banned membership. A saved token can therefore authenticate again and pass the send-messages check.
Evidence: Reproduced token authentication and send authorization after writing the ban and member status. This is independent of trusting the operator: it undermines the owner’s own moderation action.
Done when: Ban and timeout state are enforced at identify and on protected actions, the right sessions are revoked, and every relevant live connection is closed. Verified with a real WebSocket reconnect using a token issued before the ban.
Behavior: A non-admin holding only Manage Roles can update @everyone to include Administrator. Create, update and assign paths check Manage Roles but never bound the granted permissions by the caller’s own authority or protect superior roles. If Manage Roles is meant to equal full administrator authority, the product should say so; today the separate permission controls imply a narrower boundary.
Evidence: Reproduced a non-admin granting Administrator to @everyone. No malicious operator or untrusted plugin is required.
Done when: Grantable permissions and a role hierarchy are defined and enforced on every create, update, assign and remove path. Ordinary edits to @everyone still work. Owner and last-admin protections live on the server, not only in the TUI.
Behavior: The history handler queries whatever channel ID it is given without requiring membership or the channel’s view and history permissions. An authenticated non-member receives its normal messages. Server snapshots, pins, voice join and signaling, file signaling and plugin-pane input need the same authorization pass. A control in the permission editor is not proof it is enforced everywhere.
Evidence: Reproduced non-member history access in a server fixture. The prerequisite is a valid login plus knowledge of the channel ID. This is not an unauthenticated public read.
Done when: Membership, ban state, channel view and action permission are checked through one authorization path at each protocol boundary. Negative tests cover non-members, denied members and mismatched server and channel pairs, while explicitly authorized plugin behavior keeps working.
Behavior: The protocol accepts a Before cursor and the database supports it, but the history handler passes nil to the query. The client asks for a single 200-message window and has no wired older-page request on that path. Retained history beyond that window is not a finished browsing feature.
Evidence: Reproduced a request with before set to the latest message returning the latest message again. Traced the only client history-request construction.
Done when: The cursor is passed and validated, ordering is deterministic by timestamp plus ID for equal timestamps, and older-page loading is wired into the client. Verified across more than 200 messages, page boundaries, deletion and concurrent arrivals without duplicates or gaps.
Behavior: The remote-pane client keeps the last sequence number and discards anything lower. A restarted plugin can start again at zero, leaving an open pane stale until a new enter or a higher sequence arrives. No session or epoch travels with the frame. The manifest parses a minimum Concord version that is never checked. Enable and disable have no already-running guard and mutate the supervisor map without a mutex.
Evidence: Reproduced lower-sequence rejection with an already-open pane. Full crash-and-reopen recovery was not driven. The version and supervisor observations are source-traced, not runtime reproductions.
Done when: Panes carry a session or epoch with a reconnect and re-enter contract. Enable and disable are idempotent and serialized. Compatibility is enforced before launch. Tested with two installs, a crash during an open pane, two administrators toggling, a server restart and an unsupported protocol version.
Behavior: Normal peer-to-peer transfer and integrity verification work. On a failed or closed peer connection, cleanup only removes the peer map entry. Stop closes peers but does not clean active receives, close their partial files or emit a terminal transfer result. The send backpressure wait has no per-transfer cancellation or deadline.
Evidence: A shutdown probe retained an active receive and an open .part file with no terminal event. Mid-transfer network loss and stalled-peer timeouts still need a wire-level test.
Done when: Each transfer has one idempotent completion or cancellation path that closes files and peers, removes or deliberately keeps partial files, frees map entries, unblocks backpressure and reports one final status. Sender-online semantics and hash verification are preserved. Swarming stays future scope.
Behavior: Anyone who can reach hub registration can supply a host and port. Join then sends an HTTP POST to that address before returning connection details. The handler does not restrict private or loopback targets or redirected destinations. That exposes the hub operator’s network boundary even though clients intentionally trust the servers they choose. Per-IP join limiting also trusts any X-Forwarded-For header.
Evidence: A local-only test registered a temporary loopback HTTP service, joined without an account, observed the callback and received host and port. No external system was contacted. This proves the request primitive, not arbitrary internal data exfiltration.
Done when: A public hub has a defined destination policy, validates resolution and redirects, and accepts forwarding headers only from configured proxies. Intentional LAN discovery survives through an explicit private deployment policy. Registration is limited and bodies are bounded. Private and public defaults stay distinct.
Behavior: The voice-enabled build, the plain novoice-tag build and vet all pass here. But CGO_ENABLED=0 go build -tags novoice ./cmd/client fails because the Save As dependency imports Cocoa with no files available under that configuration, and the no-voice Make target uses exactly that combination. The dist recipe builds only server and client, omits the hub, and labels the client without a novoice suffix.
Evidence: Reproduced the exact CGO-free build failure on darwin/arm64. Windows and Linux voice release builds were not executed. The plain novoice build is not broken on this machine because CGO stays enabled.
Done when: A build-tagged dialog fallback exists or the platform tooling requirement is stated. All three intended binaries are produced and smoke-tested on each supported native platform, with voice in the supported client. Dist, install, artifact names, dependencies and instructions agree.
Every number below comes from an M1 Max with a stated workload. None of them establishes a supported user count, a Pi or ten-year-old laptop budget, voice mesh capacity or a full-system footprint. Those need target hardware.
| Workload | Measured result | Boundary |
|---|---|---|
| 10 real loopback WebSocket clients, 100 messages a run, wait for all recipients, 3 runs | p50 0.94–1.04 ms · p95 1.29–1.35 ms · max 1.55–1.67 ms All persisted, no missing checked deliveries. | No login or bcrypt timing, WAN, saturation, slow consumers or long history. |
| In-process fanout, every recipient subscribed and drained, 3 runs | 1 recipient 87 ns · 10 recipients 452 ns · 100 recipients 3.82 µs | Not WebSocket or persistence throughput. |
| History handler, newest 200 of 1,000 stored messages, 3 runs | 5.44–5.57 ms · ~1.14–1.18 MB · ~20,084 allocations a request | One author. Handler, database and JSON, excluding the socket write. |
| Main TUI render at 160×45 with 110 channel rows, 3 runs | 1.04–1.08 ms · ~0.80–0.87 MB a render | Existing synthetic fixture, not a populated real session. |
| Mouse-zone scan of that frame, 3 runs | 0.82–0.85 ms · ~5.57 MB a scan | Separate component. Benchmark allocation rate is not resident memory. |
| Settings and channel-management renders | 0.75–0.85 ms a render | Synthetic fixture. Native terminal paint excluded. |
An allocation profile of the isolated zone-scan benchmark attributes 99.6% of allocation space to the bubblezone scanner. The 7.90 GB total is cumulative churn across iterations, not an application footprint. The main view calls the zone scan on every render, and the typing animation tick reschedules every 100 ms even with nobody typing. For the low-resource goal, look at unnecessary idle rendering and zone allocation before anything else, and measure the real event loop before claiming an idle CPU or RAM cost.
Member lookups are cached, but each message still fetches its author. Batch or cache author retrieval before enlarging the history window; the existing multi-user lookup helper is a starting point. Keep queries bounded, and fix the SQLite settings in F04 before comparing optimization results.
The broadcast benchmark registers 100 users but never joins them to its target channel, so it is not a 100-recipient delivery test. The database insert benchmark reuses the same unique email and ignores errors, so later iterations measure rejected inserts. Voice mesh cost grows as N(N−1)/2 peer pairs with N−1 outgoing audio paths per participant. Set the practical channel cap from measured calls.
Each gate names what ships and what “done” means. Passing checks decide readiness, not a date. No new features, no rewrite.
Ship: exact source commit, supported OS and architecture list, V1 feature manifest.
Done when: Jordan’s current code is reconciled with this download, the relay-or-directory promise is explicit, and Tukan and Mynah fixtures exist.
Ship: F03 to F08.
Done when: new safety regressions pass, the probes are inverted from “defect observed” to “defect prevented”, and the skipped integrity tests are restored.
Ship: F01 and F02 plus an actual audio and network matrix.
Done when: 2, 4 and 8 clients pass join, leave, mute, deafen, push-to-talk, VAD, device change and rejoin with no races, and the advertised network modes work.
Ship: F09 and F10, a stable event and manifest contract.
Done when: the reference and real plugins survive crash, restart and toggle with no orphan processes, and transfer loss or cancel completes cleanly.
Ship: corrected benchmarks, profile-driven changes, a target-hardware run.
Done when: agreed budgets are met on the weakest supported device, with no silent chat loss and no steadily growing resources during a soak.
Ship: F11 if the hub is public, F12, native artifacts, an upgrade and restore rehearsal.
Done when: all three binaries work on each supported OS, a clean install and a restored backup pass, and no release-blocking regression is open.
Acceptance targets to agree before measuring: text delivery p95 under 200 ms on a stated LAN workload, input-to-paint p95 under 50 ms on the weakest supported terminal, no loss of acknowledged messages, stable resources over a two-hour mixed chat, voice, plugin and file run plus a longer overnight idle. These are proposed criteria, not measured passes. Define concurrent clients, history size, voice participants and plugin workload alongside them. A release-ready claim names the exact commit, artifacts, environment, scenarios and known limitations. “No known blocking defects within the tested matrix” is a supportable goal. “No bugs anywhere” is not.
Review boundary: Jordan’s local download as supplied, hashed and copied into a separate working copy. Build, vet, tests and the race detector ran on macOS arm64 with Go 1.27.1. Eleven observation probes, one race probe, a loopback load test and component benchmarks were added in seven test files that live beside the report with full logs and a replay guide. Not done: real audio calls, Windows, Omarchy or Pi devices, hostile NAT, a production soak, a restore of Jordan’s data, a dependency vulnerability scan, or a second model’s independent review. This report supports a sprint, not a release certification. The September 9 review stays available as the earlier snapshot.