fix(web): session-generation guards for the non-message stores (F38)

[Calmingstorm]

Finding F38 — a settled request can write the previous account's data into the next session

Only messageStore guarded its post-await continuations against an identity boundary. serverStore, dmStore, unreadStore, and permissionStore each await an API call and then set(...) with no guard. logout() aborts in-flight requests via the shared sessionAbort signal, but a request that already resolved at the boundary cannot be aborted, and a resolution that lands after the AppPage effect-cleanup reset writes the old account's data into the new session.

Odin design-reviewed the approach before implementation (chose the shared-generation option).

Fix

New src/api/session.ts — a narrow session-lifecycle API, not exported mutable state:

• captureSessionGeneration() / isSessionGenerationCurrent(gen) / invalidateSession()
• invalidateSession() advances the generation synchronously at every identity boundary (logout and account replacement in authStore) before aborting requests, disconnecting the socket, or resetting stores. Token refresh (same identity) does not advance it.
• client.ts's token-refresh guard now uses this one generation instead of its own private epoch.

Every async action in the four stores captures the generation at entry and, after every await (and in catch), bails before any side effect — so a stale continuation cannot write the store, set/clear an error, clear a new session's loading state, fire a second request in a multi-await action, or return an old result as success (createDM returns undefined in that case).

messageStore is intentionally not migrated: its local epoch is entangled with fetch ownership, journal protection, and reset — a store-local reset epoch and an auth-session generation solve related-but-different problems. (Can be reconciled in a later, separate change.)

Tests

session.test.ts (generation semantics) and sessionGuards.test.ts cover Odin's required cases, all mutation-verified: held read / held mutation / held rejection cannot touch the new session; a new-session request commits while an older one settles afterward and the old one cannot clear the new loading state; a multi-await action stops after the first await; createDM returns undefined; and logout advances the generation before abort and reset run.

340 web tests across 37 files, plus the Go regression suite and migrations 014/015 for the server-side invariants this PR carries: commit-ordered message sequence, a mention badge computed as a projection of (mentions, read watermark), generation-stable channel-create subscription, and lock-captured server deletion.seq / read_states.last_read_seq) as the unread watermark tied to write and acknowledgment. test / typecheck / build / lint all green.

---

Round-35 addendum — the concurrency-correctness arc (client + server)

Later review rounds evolved this PR from client session guards into a full client+server correctness effort under one theme: stop guessing what the database actually committed. Round-35 resolves the four round-34 blockers:

1. Monotonic acks. The ack now returns the committed ReadState (watermark + computed mention count); the client applies it monotonically (higher lastReadSeq wins regardless of completion order) and derives the unread flag from the winning watermark's count — so two acks completing out of order regress neither the watermark nor the flag.
2. CHANNELS_STALE reconciliation. A revocation emits CHANNELS_STALE; the client refetches the affected server's channels and the lineage reconciles away any CHANNEL_CREATE leaked in the create-vs-revoke window. granted/revoked are tracked from real hub-mutation results, so the event fires iff a subscription was actually removed.
3. Atomic mention + anchor. The mention row and its read_states anchor commit in one transaction; the NOTIFICATION is contingent on that committed, visible state — a partial failure emits nothing.
4. Channel-create delivery under authorization churn (rewritten in round 36 — see below).

Round-36 update — subscription model rewrite + equal-watermark ack

Round 35's blocker 4 was patched as a "degraded per-user floor"; round 36 replaced it wholesale after that floor accreted holes (stale-create-after-CHANNELS_STALE, phantom-after-delete, and no live delivery). The floor is deleted. SubscribeAuthorizedStable now reconciles the hub subscription to the authorized set, re-reading until consistent (generation fast-path → two agreeing authorized reads → bounded best-effort), keeps the subscription (no rollback), and the caller always delivers via BroadcastToChannel (the live hub set under the hub lock). This collapses the three failure modes into the stable-path guarantee already accepted: no leak (a revocation's synchronous unsubscribe excludes the member; the persistent-leak interleaving is impossible by happens-before — the channel commit precedes every create authz-read), no phantom (a deleted channel's live set is empty and the existence read short-circuits the broadcast), and real live delivery (the subscription persists). The primitive returns no recipient set, so no caller can deliver to a stale slice.

Round-36 blocker 1 (equal-watermark ack regressing the mention count): the client apply tie-break is now >=, so on an equal watermark the existing state wins — an ack cannot regress the count at a watermark it did not advance.

Every round ran a two-reviewer adversarial pre-review sweep on the full diff before Odin; the server sweep proved the persistent-leak interleaving impossible, and both cleared. Mutation-verified Go pins: TestChurnConvergesWithLiveDelivery, TestRevokedDuringReconcileNotSubscribed, TestDeletedDuringReconcileNoCreate, TestSubscribeAuthorizedStableConvergesUnderGenChurn, TestRevocationEmitsChannelsStale, TestMentionAnchorFailureCommitsNoMention.

[Calmingstorm]

Reviewed Bastion PR #22 at 7e4322437ebc033debf47902d58fc4facef83b21.

The store-side generation checks are directionally right, but I found three blockers.

1. Login/registration completions do not own the generation they started under

authStore.login() waits for apiLogin() and only then calls invalidateSession() (web/src/stores/authStore.ts:43-63). That makes a stale completion declare itself the newest identity instead of proving it still belongs to the current lifecycle. A deterministic held-response probe reproduced:

1. Start login A.
2. Resolve its API promise, then synchronously call logout() before promise continuations drain.
3. Drain the continuation.
4. Login A persists its tokens and sets isAuthenticated=true, resurrecting the session that was just ended.

The same omission allows an older concurrent login to replace a newer one or an older rejected login to write an error into the newer session. Capture the generation at auth-action entry, check it after the await and in catch, and only then establish the new identity boundary. A stale auth completion must not resolve as a valid login to a caller that will navigate.

register() (authStore.ts:67-84) has neither that ownership check nor invalidateSession() at all, despite registration returning tokens and establishing a new authenticated identity. My probe confirmed a successful registration leaves the previously captured generation current. Login and registration need the same identity-boundary contract and deterministic logout/concurrent-auth regressions.

2. A stale refresh rejection can log out the newer account

The refresh success path checks generationAtRefresh, but the rejection path does not (web/src/api/client.ts:178-208). I reproduced:

1. Hold account A's refresh.
2. Log out A and establish account B's tokens.
3. Reject A's old refresh with 401.
4. The old catch calls clearTokens() and onAuthFailure(), deleting B's tokens and ending B's session.

Generation ownership must cover the refresh failure continuation before it clears tokens, invokes auth failure, or affects queued work. Add a regression using a held old refresh and a newer authenticated session.

3. Two of the four store integrations are not regression-pinned

The new tests exercise server-store and DM-store guards, but never import unreadStore or permissionStore. I removed every session-generation import/capture/check from both unreadStore.ts and permissionStore.ts; all 156 tests still passed. Those are two load-bearing production connections that can disappear without detection.

Add held old-session success regressions for both stores (at minimum fetchReadStates/ackChannel and fetchPermissions) proving they cannot write into the next session. The guards themselves look correct; the issue is that the claimed four-store fix is only pinned for two stores.

Validation

• 156/156 committed web tests passed
• Test TypeScript check passed
• Production build passed
• ESLint: zero errors, two pre-existing warnings
• git diff --check passed
• npm audit remains 17 findings / 0 critical
• All GitHub jobs green; PR mergeable
• Three deterministic review probes failed as described and were removed
• Mutation probe was restored; checkout is clean

PR #22 is not ready to merge. The shared generation is the right authority, but the auth operations that create/end identities must obey it too. Otherwise the gate is sound and the gatekeeper occasionally arrives from yesterday.

[Calmingstorm]

Reviewed Bastion PR #22 round two at 5a40154c98188295014419133608cd336dcdeece.

The five reported changes are present and mutation-sensitive, but I found three remaining session-boundary blockers.

1. loadFromStorage() remains outside the generation boundary

web/src/stores/authStore.ts:126-173 performs an async apiGetMe() and then either writes freshUser or calls logout() without capturing/checking the shared generation.

Two deterministic held-request probes reproduced both directions:

• Start stored-session validation, call logout(), then resolve the old validation: the old user is written back into the logged-out auth store.
• Start stored-session validation for account A, complete a newer login for B, then reject A's validation: the old catch calls logout() and ends B's valid session.

Capture the generation for the stored-session attempt and guard both its post-await success and rejection continuation. A stale validation must neither write the user nor log out the current identity. This is also one of the identity boundaries named by session.ts itself, so leaving the bootstrap path unowned defeats the shared authority at startup.

2. Superseded login/register still resolve as successful operations

The post-await guards at authStore.ts:56/68/82/94 correctly prevent stale store writes, but they return normally. Production callers then treat that as successful authentication:

await login(...)
navigate('/app')

A real LoginForm probe started login A, superseded it with a newer session boundary, then resolved A. The auth store stayed unauthenticated, but the form still navigated from /login to /app.

There is a second lifecycle consequence: login() sets isLoading=true, while logout() does not reset isLoading; a held login superseded by logout returns through the guard and leaves auth loading permanently true. My probe ended with isLoading === true.

A stale auth operation needs an explicit non-success outcome—a recognized session-cancelled rejection or a boolean/result that callers check before navigating. Logout must also restore auth loading state. Pin the behavior through the real login and registration caller paths, not only the store write.

3. Refresh coordination captures ownership too late and is shared across generations

The interceptor captures generationAtRefresh only when a 401 is handled (client.ts:164), not when the original request is sent. Therefore an account-A request can finish with 401 after account B becomes current; the interceptor then captures B's generation, reads B's refresh token, and retries A's original request under B's credentials.

A deterministic probe sent an account-A POST, advanced the session to B before returning its 401, and observed:

• refresh called with account B's refresh token;
• the original account-A POST retried and succeeded as B.

The global isRefreshing/failedQueue has the inverse cross-generation bug as well. I held refresh A, advanced to B, then made a B request return 401. B queued behind A; when stale refresh A rejected, processQueue(refreshError) rejected B with stale-a-refresh instead of letting B perform its own refresh.

Attach the generation to each request when it is dispatched and reject a stale response before refresh/retry. Refresh single-flight state and queued requests must also be generation-owned; a new-session request cannot queue behind, resume from, or be rejected by an old-session refresh.

Confirmed fixed

I independently mutation-checked all five claimed safeguards:

• stale refresh-rejection guard;
• unread read-state guard;
• permission guard;
• login post-await guard;
• registration entry invalidation.

Removing each one failed its targeted regression.

Validation

• 162/162 committed web tests passed
• Test TypeScript check passed
• Production build passed
• ESLint: zero errors, two pre-existing warnings
• git diff --check passed
• npm audit remains 17 findings / 0 critical
• All GitHub jobs green; PR mergeable
• Six deterministic review-only probes failed as described and were removed
• Exact review checkout is clean

PR #22 is not ready to merge. The store guards now hold; bootstrap auth, caller-visible auth completion, and refresh ownership still have ways to carry yesterday's session into today's account.

[Calmingstorm]

Reviewed Bastion PR #22 round three at f9543516af2864579342123e7bcc3f6d9fd2b828.

The three reported mechanisms are present and their committed cases pass, but I found four remaining blockers.

1. Request ownership is stamped after the request is issued

web/src/api/client.ts:101-117 stamps __sessionGeneration inside Axios's request interceptor. That interceptor is asynchronous by default, so the identity boundary can pass after apiClient.get() returns but before the request is stamped or linked to sessionAbort.

I reproduced this deterministically:

1. Install account A tokens.
2. Call apiClient.get('/old').
3. Immediately invalidate A and install account B tokens, without waiting for the adapter.
4. Let /old return 401.
5. Actual: the old request is stamped as B, its Authorization header is populated from B, and refresh is called with B's refresh token.

The committed stale-request regression waits until the adapter has started, which guarantees the interceptor has already stamped the request and misses this boundary. Capture ownership synchronously at request creation—e.g. a synchronous request interceptor or an explicit request wrapper/config stamp—and pin the immediate call → boundary sequence.

2. A stale refresh can still settle the current refresh's queue

failedQueue and processQueue() at client.ts:129-143 are global rather than refresh-owner scoped. When B encounters A's stale refresh, B correctly drops A's queue and starts refresh B. Refresh A remains physically in flight, however.

I reproduced:

1. Hold refresh A.
2. Advance to B.
3. B1 receives 401 and starts held refresh B.
4. B2 receives 401 and queues behind refresh B.
5. Reject refresh A.
6. Actual: refresh A's unconditional processQueue(refreshError) rejects B2 with A's stale failure. Refresh B can later succeed, but B2 is already dead.

The same cross-owner drain exists in the stale-success branch at lines 214-216. Scope queues to a refresh generation/owner ID, and allow a refresh settlement to process only its own queue. The finally ownership guard protects isRefreshing; it does not protect failedQueue.

3. F38 still omits commandStore

web/src/stores/commandStore.ts:16-27 is another per-user store with an async continuation and no session guard. resetAllStores() explicitly clears it on logout, but an old response can write afterward. Its server-ID equality check is not a session boundary because two users can share the same server.

A held-response probe reproduced:

1. Account A starts command fetch for shared server s.
2. Reset/logout.
3. Account B fetches s and commits new-command.
4. A's old response settles.
5. Actual: old-command replaces B's command list.

Apply the shared generation to fetchCommands() and add the same-ID/new-session regression. featureStore appears server-global and is not reset as per-user state; commandStore plainly is.

4. Registration supersession remains unpinned

The implementation's post-await registration guard at authStore.ts:86 is required, but the tests cover only that registration invalidates at entry. I removed that guard; all 166 committed tests and test type-checking still passed. A stale registration would then authenticate after a newer boundary.

Add the registration counterpart to the login regression: hold registration, supersede it, resolve it, assert SessionSupersededError, no tokens/user commit, and no caller navigation. This was part of the previous login/register blocker and needs mutation-sensitive coverage for both paths.

Confirmed fixed

• loadFromStorage() now guards both success and rejection after its captured boundary.
• Superseded login rejects rather than fulfilling, and logout clears auth loading.
• A request that was already stamped before the boundary does not refresh afterward.
• A current request starts a new refresh instead of directly queuing behind an already-stale refresh.
• Unread and permission guards are now regression-pinned.

Validation

• 166/166 committed web tests passed
• Test TypeScript check passed
• Production build passed
• ESLint: zero errors, two existing warnings
• git diff --check passed
• npm audit remains 17 findings / 0 critical
• All GitHub jobs green; PR mergeable
• Three deterministic review probes failed as described
• Registration mutation left the committed suite green
• All probes/mutations were removed; exact review checkout is clean

PR #22 is not ready to merge. Generation ownership now reaches the obvious continuations; it still arrives one microtask late at request creation and remains shared too broadly at refresh settlement.

[Calmingstorm]

Reviewed Bastion PR #22 round four at 3c082b7899ce15cb148911066c2358eb865a894b.

The four reported round-three fixes are present: request interception is synchronous, stale refresh A no longer drains replacement refresh B's queue, commandStore is generation-guarded, and registration supersession is pinned. Two blockers remain.

1. Old-session requests already queued behind a refresh can remain pending forever

web/src/api/client.ts:181-189 moves concurrent 401s off the Axios transport and onto the standalone failedQueue promise. When that refresh later belongs to a stale generation:

• the success path returns at lines 220-221 without settling its queued requests;
• the catch path deliberately skips processQueue() at lines 233-237;
• the finally path also leaves refresh ownership untouched at lines 253-259.

That avoids corrupting refresh B's queue, but it abandons refresh A's waiters. abortInFlightRequests() does not settle them because they are no longer waiting on the aborted adapter request.

A deterministic probe performed this sequence:

1. Account A request starts held refresh A.
2. A second account-A request receives 401 and queues behind A.
3. The session is invalidated and in-flight HTTP is aborted.
4. Refresh A rejects.
5. The refresh leader rejects, but the queued request remains pending (timeout rather than rejected).

The queue needs refresh/session ownership rather than one unowned global array. On an identity boundary or stale refresh settlement, reject only the entries belonging to that ended refresh/generation; never touch a replacement refresh's entries. Add a regression that queues an A request, ends A with no replacement 401, settles refresh A, and proves the queued request rejects promptly and the next session starts with clean refresh state.

2. Production DM creation bypasses the guarded store action and can still write across sessions

The new dmStore.createDM() guard is not used by the principal production creation paths:

• web/src/components/dm/NewDMDialog.tsx:75-78
• web/src/components/user/UserContextMenu.tsx:22-25
• web/src/components/user/UserProfileCard.tsx:34-39

Each calls apiCreateDM() directly and then selects the returned DM without checking the originating generation. NewDMDialog additionally calls fetchDMs() only after the old create resolves, so that fetch captures the new generation and does not validate the create's ownership.

A rendered NewDMDialog probe held apiCreateDM, invalidated the session, reset the DM store, then resolved the old create. The component called the new session's fetchDMs() and set selectedDMId to dm-old.

Route these production paths through the guarded store action and treat its undefined superseded result as a no-op, or apply equivalent operation ownership at each caller. Add at least one production-wiring regression; the current store-only test proves an action no caller uses for these paths.

Validation

Passed:

• 170/170 web tests across 25 files, with no unhandled errors
• test TypeScript check
• production build
• ESLint with only the two existing warnings
• diff checks
• npm audit baseline: 17 findings / 0 critical
• all GitHub jobs green; PR reports mergeable

Both deterministic review probes were removed, and the exact checkout is clean.

PR #22 is not ready to merge. The session generation now owns ordinary store continuations, but refresh waiters and the actual DM creation UI still have routes around the boundary.

[Calmingstorm]

Reviewed Bastion PR #22 round five at 99ef0af70a3cd466136a90eef3a99924748d1de8.

The generation-tagged queue fixes the reported stale-refresh-vs-replacement-refresh collision, and the production DM entry points now route through dmStore.createDM. Three session-boundary blockers remain.

1. Session invalidation still does not settle queued refresh waiters

web/src/api/client.ts:190-199 moves concurrent 401s from Axios onto the standalone failedQueue. The new generation tags let a refresh settle only its own waiters, but the queue is drained only when code later enters another 401 path or the bare Axios refresh settles (client.ts:185-187, 230-247).

invalidateSession() plus abortInFlightRequests() does not touch this queue. The queued request is no longer waiting on the abortable adapter request, and the bare axios.post refresh has neither a session abort signal nor a timeout.

A deterministic probe did the following:

1. Start refresh A and hold its bare-Axios request.
2. Queue a second A request behind it.
3. Invalidate A and abort in-flight requests.
4. Leave the held refresh unsettled.
5. Actual: the queued request remained pending past the timeout.

The committed regression explicitly rejects the refresh after invalidation, so it proves eventual stale-settlement drainage, not invalidation drainage. Every waiter belonging to the ended generation must be rejected at the identity boundary even if its refresh never settles. Do that without touching replacement-generation waiters.

2. A queued old-session request can still retry with the new account's credentials

A queue entry is generation-tagged only for processQueue; its continuation at client.ts:194-199 does not verify ownership before retrying. The synchronous request interceptor then unconditionally overwrites __sessionGeneration and Authorization from the current session (client.ts:101-116).

I reproduced this ordering:

1. A request starts refresh A; a second A request queues behind it.
2. Refresh A succeeds and resolves the queued waiter.
3. Before that waiter's .then(...) runs, the session advances to B and B's tokens are installed.
4. The old queued waiter resumes and calls apiClient(originalRequest).
5. Actual: the old request is sent with Authorization: Bearer b-access and is re-stamped as generation B.

The request's original generation must remain immutable across retries, and the queued continuation must recheck that ownership immediately before retrying. A settled refresh does not grant its waiters immunity from a boundary occurring between queue resolution and promise continuation.

3. DM workflows guard the create await, not the complete operation

The production routes no longer call raw apiCreateDM, but operation ownership still ends too early.

NewDMDialog

At web/src/components/dm/NewDMDialog.tsx:78-82, the dialog:

1. awaits the guarded create;
2. awaits fetchDMs();
3. unconditionally selects the earlier dm.id and closes.

If the session changes during step 2, fetchDMs() captures the new generation and returns normally, after which the dialog selects the old account's DM. A rendered held-fetch probe reproduced selectedDMId === "dm-old" after reset. The committed test changes generation only while the create itself is pending and misses this second await.

openDirectMessage

web/src/stores/dmActions.ts:17-20 similarly assumes a defined result remains current after await createDM(). A deterministic microtask boundary after the store's guarded write but before the helper continuation caused it to clear the new session's server selection and select dm-old.

Capture operation generation at each workflow's entry and recheck it after every await and before every subsequent side effect. A guarded inner action does not transfer its ownership proof to an outer continuation.

Validation

Passed on the exact head after removing review probes:

• 175/175 committed web tests across 27 files
• Test TypeScript check
• Production build
• ESLint with only the two existing warnings
• git diff --check
• npm audit baseline: 17 findings / 0 critical
• All GitHub jobs green; PR reports mergeable
• Review probes removed; checkout clean and still at the exact reviewed commit

The reported round-four fixes are directionally correct, but PR #22 is not ready to merge. The queue now knows whose funeral it is attending; it still waits for the corpse to file paperwork, and one waiter can leave wearing the next account's credentials.

[Calmingstorm]

Reviewed PR #22 round six at 244c871c93b4541ca156b707ad1891bb846c1874. The three reported round-five fixes are present: invalidation rejects generation-tagged queue followers synchronously, queue continuations recheck ownership, and both DM workflows now own their complete sequence. Three blockers remain.

1. Session invalidation still cannot settle the request that owns a hung refresh

onSessionInvalidated() drains only failedQueue (web/src/api/client.ts:167-172). The first request that received the 401 is not in that queue; it directly awaits the bare axios.post refresh at client.ts:245-249.

I reproduced this deterministically:

1. Request A gets 401 and becomes the refresh leader.
2. The bare refresh promise never settles.
3. invalidateSession() runs.
4. A remains pending rather than rejecting.

The new boundary fixes followers but still leaves one prior-session request/workflow parked forever for every hung refresh. Race the refresh owner against a generation-invalidation promise or otherwise explicitly settle the leader at the boundary, while retaining the late-result generation guards. Add a regression for leader starts hung refresh -> invalidate -> leader rejects promptly.

2. Guarded store mutations still fulfill as successful when superseded

serverStore.createServer() and createChannel() return Promise<void> and simply return when their generation becomes stale. Their production callers therefore cannot distinguish a committed mutation from a discarded old-session completion.

A rendered CreateServerDialog probe showed:

1. Hold apiCreateServer().
2. Invalidate the session.
3. Resolve the old request.
4. The store correctly does not insert the server, but CreateServerDialog treats the fulfilled action as success, clears the form, and calls onOpenChange(false).

ChannelList has the same shape for createChannel(). This is the value-returning rule in another costume: superseded actions must return an explicit stale/cancelled result or reject with the recognized session-superseded error, and callers must not execute success UI after that outcome. Guard caller catch/finally state as appropriate too.

3. Other production workflows still bypass generation ownership and can mutate the new session

The DM bypass is fixed, but the same class remains in direct component API paths. Two deterministic examples:

• ChannelItem.handleDelete() (web/src/components/channel/ChannelItem.tsx:51-59) awaits apiDeleteChannel() and then calls removeChannel() without ownership. Holding the old delete, invalidating, loading a same-ID channel for the new session, then resolving the old request removes the new session's channel.
• CreateServerDialog.handleJoin() (web/src/components/server/CreateServerDialog.tsx:69-75) calls raw apiJoinViaInvite(), then starts fetchServers() and selectServer() after it resolves. If the boundary lands during the join, those guarded store calls capture the new generation and accept the old workflow's server ID. InvitePage has the same sequence plus navigation.

There are additional direct channel/category workflows with post-await global-store writes in ChannelItem, ChannelList, and UnifiedSidebar. Audit production API call sites that mutate per-user stores or continue multi-step account-owned workflows; route them through guarded actions/helpers or capture one generation for the entire workflow and recheck after every await and before every side effect. Add same-ID channel deletion and held invite-join regressions.

Validation otherwise passed:

• 179/179 committed web tests across 27 files
• Test TypeScript check
• Production build
• ESLint with only the two existing warnings
• Diff checks
• npm audit: 17 total / 0 critical overall, 7 production / 0 critical
• All GitHub jobs green; PR mergeable
• All deterministic probes removed; exact checkout clean

The PR body still reports 156 tests and should be refreshed, non-blocking.

PR #22 is not ready to merge. The shared generation is sound, but ownership still ends at store boundaries in places where the actual workflow keeps walking.

[Calmingstorm]

Reviewed Bastion PR #22 round seven at 5db65dfc4db97fabb53b0e2976fd5fce296ddbb5.

The reported refresh-leader race is fixed: invalidation now settles the leader directly, the listener is removed in finally, and stale refresh settlement remains generation-gated. The first-stage stale create/create-channel cases and the listed invite/channel workflow guards are also present. I found three remaining session-ownership blockers.

1. User-account mutations can still overwrite the next session

web/src/components/user/UserSettingsDialog.tsx:130-161 performs profile and avatar mutations and then writes directly to both useAuthStore and persistent storage without generation ownership. ChangeEmailForm has the same problem at lines 279-288.

A deterministic rendered probe reproduced the cross-account write:

1. Start account A's profile save and hold apiUpdateProfile().
2. Invalidate the session and install account B in authStore.
3. Resolve A's request.
4. Actual: authStore.user changes from B back to A, the old user is persisted, and the old dialog runs its success close.

Profile save, avatar upload, and email change need one workflow generation captured at entry and checked after the await before store/storage/UI writes; stale catch/finally paths must not affect the current session either. This is the exact F38 failure, now against the authentication store itself.

2. An old member-list read can repopulate global presence after reset

web/src/components/member/MemberList.tsx:23-33 still calls apiGetMembers() and then writes every returned status into the global presenceStore with no generation check.

I held the member request, advanced the session, reset presence, and then resolved the old response. Actual: the new session ended with:

{ "old-member": "away" }

Unmounting the component does not cancel that promise callback or its direct global-store writes. Own this fetch with the session generation and guard its success, error, and loading continuations. The production workflow audit should include other component-level reads that write shared per-user stores, not only mutations.

3. The superseded-mutation rejection contract still has holes

serverStore.createServer() checks ownership after apiCreateServer(), but then performs a second await at serverStore.ts:146:

await get().selectServer(server.id)

selectServer() silently returns when superseded, and createServer() never rechecks afterward. A deterministic probe let creation resolve, held the nested channel fetch, invalidated the session, and then resolved it. Actual: createServer() fulfilled with undefined instead of rejecting with SessionSupersededError.

The dialog's additional generation check currently prevents its success UI, but the guarded store action no longer provides the explicit non-success contract claimed by this fix. Check ownership after the nested await and reject stale completion.

The stated “success and failure arms” behavior is also absent from leaveServer() and deleteServer(): both await the API outside a try/catch, so an old-session rejection propagates the raw transport error rather than SessionSupersededError. Add held second-await and held rejection regressions; the current tests cover only a successful first API response becoming stale.

Validation

Passed on the exact head after removing all review probes:

• 185/185 committed web tests across 29 files
• Test TypeScript check
• Production build
• ESLint: zero errors, two existing warnings
• git diff --check
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• All three GitHub jobs green; PR mergeable
• Review probes removed; exact checkout clean

The newly added component tests emit React act(...) warnings, non-blocking.

PR #22 is not ready to merge. The shared generation now reaches much farther, but old account work can still write the auth and presence stores, and one guarded mutation can still report stale completion as success.

[Calmingstorm]

Reviewed Bastion PR #22 round eight at 9dd5f1ccb407e4c6e440e7bc9161c51e540110f5.

The three reported round-seven fixes are present and their new tests are mutation-sensitive: profile/avatar/email writes are generation-gated; MemberList and MessageInput member reads reject stale results; and the nested createServer plus stale leaveServer/deleteServer failure contracts are corrected. I found four remaining session-ownership blockers.

1. Starting login/register does not tear down the identity it supersedes

web/src/stores/authStore.ts:48-69 and 79-96 call invalidateSession() at entry, but do not abort the old requests, disconnect the old WebSocket, clear old tokens, or reset old per-user stores. Those operations still exist only in logout() at lines 106-124.

This produces two concrete failures:

• Old cache survives successful replacement. I seeded account A's messageStore, completed a direct login as B, and B became authenticated while A's messages remained in the store.
• Work started during the held login is mislabeled as new-session work while using A's bearer token. I held B's login, then called the guarded fetchServers(). The synchronous interceptor stamped the request with the post-invalidation generation but attached Authorization: Bearer account-a-access; the response then passed the generation guard and committed A's server data.

The old WebSocket remains active through the same interval and successful replacement never synchronously clears its handlers/data. A new-identity attempt needs an atomic transition: invalidate first, then synchronously abort/disconnect/reset and remove the old credentials before any new-generation work can be admitted. If failed login is intended to leave A logged in, then it is not an identity boundary and needs attempt ownership separate from the active-session generation; the current hybrid grants the new generation to the old identity.

2. Account deletion still belongs to no session

web/src/components/user/UserSettingsDialog.tsx:472-480 awaits apiDeleteAccount() and then unconditionally calls clearTokens() and redirects.

A rendered held-response probe reproduced:

1. Begin account A deletion.
2. Advance the session and install account B's credentials.
3. Resolve A's deletion.
4. Actual: B's access/refresh tokens are cleared and the stale workflow attempts to navigate B to /login.

Its stale rejection and finally paths also write A's error/loading state into the surviving dialog. Capture one generation at workflow entry and guard success, catch, and finally. On current-session success, end the authenticated session through the real logout/reset boundary rather than only clearing storage and hard-redirecting.

3. Account-operation continuation coverage remains incomplete

The round-eight change guards profile/avatar/email store writes and errors, but not all continuations:

• ProfileTab.handleSubmit() always clears isSaving in finally at line 152.
• ChangeEmailForm always clears it at line 304.
• FileReader.onload at line 160 can publish account A's avatar preview after the boundary.
• ChangePasswordForm at lines 346-357 has no generation ownership at all: stale success/failure, field/dialog changes, loading, and its delayed success callback all cross the boundary.

The approved F38 model was explicit: check ownership after every await and in catch/finally before every UI side effect. These paths still violate it. Add held-success and held-failure regressions for password/delete, plus a loading-ownership regression where newer work is active when older finally settles.

4. MemberList guards data but not loading ownership

web/src/components/member/MemberList.tsx:30-39 captures a generation for the member/presence writes, but the unconditional .finally(() => setIsLoading(false)) remains outside it.

I reproduced:

1. Start member request A and hold it.
2. Advance the session and start member request B for the same server ID.
3. Settle A while B remains pending.
4. Actual: the spinner disappears although B is still active.

Guard finally by generation/request ownership, and pin the list state itself as well as the global presence write. The current committed test asserts only the latter.

Validation

Passed on the exact head after removing all review probes:

• 192/192 committed web tests across 31 files
• Test TypeScript check
• Production build
• ESLint: zero errors, two existing warnings
• git diff --check
• npm audit: 17 total / 0 critical
• All three GitHub jobs green; PR mergeable
• Four deterministic review probes failed as described
• Review probes removed; checkout clean at the exact reviewed commit

The PR body still reports 185 tests, non-blocking.

PR #22 is not ready to merge. The guards now protect more completions, but the identity transition itself still allows old credentials to operate under the new generation, and several account/UI continuations remain unowned.

[Calmingstorm]

Reviewed Bastion PR #22 round nine at c0acf649f5bc27e287732b65b2f8961b2dd008d0.

The four reported fixes are present: login/register now perform the full teardown before creating the auth request; stale account-operation continuations are generation-gated; and MemberList gives the newest request ownership of data, presence writes, and loading. Two blockers remain.

1. Invalid login is misclassified as session supersession

The server deliberately returns 401 for invalid credentials (server/internal/api/auth.go:163-171). apiLogin() uses the shared apiClient, whose response interceptor treats every first 401 as an expired protected request (web/src/api/client.ts:182-242). Round nine has correctly removed the old refresh token before login starts, so this path now:

1. sees no refresh token;
2. calls onAuthFailure();
3. production AuthFailureBridge calls logout(), advancing the generation again;
4. authStore.login() sees its generation is stale and throws SessionSupersededError instead of setting the invalid-credentials error.

I reproduced this through the real interceptor with an adapter returning the server's login 401: the auth-failure handler ran and authStore.error remained empty. A wrong password therefore silently returns the user to the login page with no feedback.

Exclude unauthenticated auth routes such as /auth/login from refresh handling (or use a client without the protected-route interceptor), and add a real-interceptor regression proving an invalid login remains owned by the attempt and surfaces its server/fallback error.

The new teardown tests mock apiLogin/apiRegister and only prove teardown occurred before the held response settles. They would still pass if request creation moved before logout(). Please pin the load-bearing order through the real request adapter too: the login/register request must be created after the old token, socket, stores, and generation are torn down.

2. Successful account deletion does not perform an identity transition

DeleteAccountForm checks generation ownership, but success only calls clearTokens() and assigns window.location.href (web/src/components/user/UserSettingsDialog.tsx:497-507). It does not invalidate the session, clear authStore, abort unrelated requests, disconnect the WebSocket, or reset per-user stores.

A deterministic component probe completed a successful deletion and observed all of the following still true before navigation:

• the captured session generation remained current;
• authStore.isAuthenticated remained true with the deleted user;
• the old account's server data remained loaded.

A hard navigation is not a substitute for the synchronous identity boundary this PR establishes, especially if navigation fails or under the Tauri HashRouter. Run the full logout teardown (or an equivalent explicit identity-ending transition) before routing, and regression-test generation/auth/store/socket teardown—not only the stale account-A completion case.

Validation

Passed:

• 198/198 committed web tests
• Test TypeScript check
• Production build
• ESLint with only the two existing warnings
• git diff --check
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• All GitHub jobs green; PR mergeable

Both deterministic review probes failed as described. Probe files were removed; the exact checkout is clean.

PR #22 is not ready to merge.

[Calmingstorm]

Reviewed Bastion PR #22 round ten at 6d44f08b95cb5aa0ac0e0183404b9ad3c83a18d0.

The two reported round-nine fixes are present: auth-endpoint 401s bypass refresh, and a successful account deletion now runs the full logout/reset transition. The real-interceptor login/register ordering tests also pin teardown before request creation.

Three blockers remain.

1. loadFromStorage() has session ownership but no attempt ownership

web/src/stores/authStore.ts:137-188

Every bootstrap captures the same session generation. Two overlapping validations can therefore both remain authoritative. This is not merely theoretical: the app is rendered under StrictMode (main.tsx:9-12), while App calls loadFromStorage() from a mount effect (App.tsx:62-66).

I reproduced deterministically:

1. Start bootstrap A and hold its apiGetMe().
2. Start bootstrap B under the same generation.
3. Resolve B successfully with the current user.
4. Reject older A with a transient error.
5. Actual: A calls logout(); isAuthenticated becomes false and B's user is cleared.

The generation check cannot distinguish these attempts because no identity boundary occurred. Make bootstrap single-flight or add latest-attempt ownership, and allow only the owning attempt to persist the user or log out. Pin both completion orders, including newer-success/older-failure.

2. closeDM() still produces an unhandled rejection at logout

web/src/stores/dmStore.ts:52-63, called fire-and-forget from DMList.tsx:99-103 and UnifiedSidebar.tsx:340-344.

The comment says stale close failures return silently because callers are fire-and-forget, but the generation check occurs after the unguarded await:

await apiCloseDM(channelId)
if (!isSessionGenerationCurrent(generation)) return

If logout aborts the request, the await rejects before the guard. Both production callers discard the promise, so this becomes an unhandled rejection. A probe that started closeDM(), invalidated the session, and rejected the held request failed because the store promise rejected instead of settling silently.

Catch the rejection, return when the captured generation is stale, and define the current-session failure behavior separately. Add a regression through a real fire-and-forget caller or otherwise pin that logout cancellation cannot leak an unhandled rejection.

3. Secret-returning server-settings workflows remain outside session ownership

WebhooksTab and IntegrationsTab still publish responses without generation checks:

• webhook create/regenerate: ServerSettingsDialog.tsx:920-950 calls setRevealed(wh), exposing the one-time plaintext webhook token;
• bot create/regenerate: ServerSettingsDialog.tsx:1095-1127 calls setTokenModal(...), exposing the bot token;
• their list/delete continuations are unguarded as well.

I held apiRegenerateWebhookToken(), invalidated the session, then resolved the old request. The rendered component displayed:

https://cfg.example/api/v1/webhooks/w1/whk_old_session

Component-local state is not cleared by resetAllStores(), and relying on React routing/unmount timing is not an identity boundary. Capture the workflow generation and check it after every await before list changes, one-time token reveal, errors, or pending-state cleanup. Add held-response regressions for webhook and bot create/regenerate, especially the plaintext-token paths.

Validation otherwise passed:

• 202/202 web tests across 31 files
• test TypeScript check
• production build
• ESLint with the two existing warnings
• git diff --check
• npm audit at 17 findings / 0 critical
• all GitHub jobs green; PR reports mergeable

All review probes were removed; the exact checkout is clean.

PR #22 is not ready to merge. The auth boundary itself is finally sound. A few continuations are still trying to crawl across it after death, as software does when denied a proper burial.

[Calmingstorm]

Reviewed Bastion PR #22 round eleven at e2581b73adf5905685c4d587d9c8fa29e607b966.

The three reported fixes are present, and their committed regressions pass. The review found four remaining blockers:

1. loadFromStorage() recency is still not owned by every invocation, and App initialization is not recency-owned.

   • authStore.ts:157-161 returns on absent/corrupt tokens before incrementing loadFromStorageSeq. An older held validation can therefore still settle afterward and commit the old user.
   • Separately, App.tsx:62-66 attaches setIsInitialized(true) to every invocation. Under the real StrictMode double effect, the older validation can settle while the newer validation is still pending and expose protected routing early. I reproduced both deterministically: the first left isAuthenticated === true after the newer no-token invocation, and the rendered StrictMode probe mounted /app while the newer validation was unresolved.

   Every invocation must claim recency at entry, including no-token/corrupt-token paths, and startup initialization must only complete for the latest admitted validation. The no-token/corrupt-token outcome should also perform an authoritative unauthenticated transition rather than only clearing persisted tokens.

2. Webhook/bot list effects are still unguarded across the identity boundary.

   • ServerSettingsDialog.tsx:909-918 applies old apiGetWebhooks/apiGetChannels results after a session change.
   • ServerSettingsDialog.tsx:1098-1103 does the same for apiGetBots.

   Deterministic held-response probes rendered an old-session OLDHOOK and OLDBOT into the still-mounted component after invalidateSession(). The create/regenerate guards protect new one-time secrets, but the sibling read paths still publish old account integration data.

3. Bot token regeneration is not single-flight or recency-ordered.
   handleRegenerate() at ServerSettingsDialog.tsx:1139-1147 allows concurrent clicks and checks only session generation. Two rotations in the same session can settle newest-first, oldest-last; the older response then replaces the modal with a token the newer rotation has already invalidated. My held two-request probe ended by displaying the obsolete first token. Disable repeated submissions or assign per-operation ownership so only the newest-started rotation may reveal its token.

4. closeDM() failure is recorded in state that production never renders.
   dmStore.ts:62-64 sets error: 'Failed to close conversation', but neither DMList, UnifiedSidebar, nor another production consumer selects/displays dmStore.error. A same-session close failure remains silent; the new test only inspects the store directly. Surface a toast or render the error, and pin that real production path.

Validation otherwise passed:

• 209/209 committed web tests across 31 files
• Test TypeScript check
• Production build
• ESLint with only the two existing warnings
• Diff checks
• npm audit: 17 total / 0 critical
• All GitHub jobs green; PR mergeable
• Five deterministic review-only probes failed as described, were removed, and the exact checkout is clean

PR #22 is not ready to merge.

[Calmingstorm]

Reviewed Bastion PR #22 round 12 at 041eaa614f85f1d7bbc80d03acbfe89fac2e6713.

The list-effect ownership guards and visible closeDM() failure path work, but I found three blockers.

1. An owning no-token invocation leaves a superseded provisional identity authenticated

loadFromStorage() writes stored credentials and cached user into Zustand and sets isAuthenticated: true before awaiting apiGetMe() (web/src/stores/authStore.ts:188-200). A later invocation that owns the no-token/corrupt-token outcome calls only clearTokens() and returns true (:169-172). It does not clear the in-memory auth state or per-user stores.

Deterministic interleaving:

1. Invocation A reads account-A credentials, publishes provisional authenticated state, and blocks in apiGetMe().
2. Persistent credentials disappear.
3. Invocation B claims recency, observes no valid credentials, clears storage, and returns true.
4. App accepts B as the owner and completes startup.
5. The store still contains A's user, access token, refresh token, and isAuthenticated: true; AppPage can consequently fetch and connect its WebSocket with that stale identity.

A review probe expected the owner to leave { user: null, accessToken: null, refreshToken: null, isAuthenticated: false }; actual state remained fully authenticated as A. The new committed test at sessionGuards.test.ts:236-252 currently codifies the unsafe result by expecting isAuthenticated to remain true.

The owning no-token/corrupt-token path must perform in-memory identity teardown as well as persistent-token cleanup, while the superseded invocation remains unable to restore anything. Add the held-A/no-token-B regression with the unauthenticated state assertion.

2. App's rejection path bypasses startup ownership

web/src/App.tsx:67-71 honors the owned boolean only on fulfillment, but every rejection executes setIsInitialized(true).

Under StrictMode, if older invocation A rejects unexpectedly while newer owning invocation B is still validating, A immediately mounts routing. A rendered review probe reproduced exactly that: B remained pending, but A's rejection removed the loading gate and mounted /app.

loadFromStorage() handles expected validation failures internally, but unexpected rejection remains possible around platform storage or teardown. “Never hang startup” cannot mean “let a superseded attempt open the gate.” Make the operation total with an ownership-bearing outcome, or otherwise retain ownership on the rejection path. Add a StrictMode regression where A rejects while B is pending and assert routing stays gated until B settles.

3. Bot rotation ordering assumes request-start order equals server commit order

IntegrationsTab allows concurrent rotations and uses regenSeqRef to display only the response from the request started last (web/src/components/server/ServerSettingsDialog.tsx:1161-1172). That does not identify the token committed last by the server.

For two requests R1 then R2, the server can process R2's UPDATE bots SET ... first and R1's update second. R1's token is then the valid credential, but the client suppresses R1's response and displays R2's now-invalid token. The committed test assumes the second-started request necessarily invalidates the first; HTTP concurrency provides no such ordering guarantee.

A rendered probe confirmed two clicks start two destructive rotations while the first is unresolved. Serialize rotation like the webhook path, or have the server provide a monotonic committed rotation version and reconcile against that. Add a regression proving a second rotation cannot begin while the first is pending if serialization is chosen.

Validation

Passed:

• 214/214 committed web tests across 31 files
• Test TypeScript check
• Production build
• ESLint with 0 errors / 2 warnings
• git diff --check
• npm audit baseline: 17 findings / 0 critical
• All GitHub jobs green; PR reports mergeable

All three deterministic review probes failed as described. They were removed, and the exact checkout is clean.

PR #22 is not ready to merge. The owner token now exists; two paths still walk around it, and one rotation counter is attempting to impose causality on the network. The network has declined.

[Calmingstorm]

Reviewed exact head 9eb8068ec3f363de2304ed55a58c468adaa178ca.

The three reported changes are present: the owning no-token path clears in-memory auth fields, loadFromStorage() no longer rejects into an App-level ownership bypass, and bot rotations are serialized. The committed suite and all local gates pass. I still found three F38 blockers.

1. The owning no-token outcome clears auth state without ending the provisional session

web/src/stores/authStore.ts:173-184 calls clearTokens() and clears four auth fields, but it does not invalidate the session generation, abort transport, disconnect the socket, or reset identity-owned stores.

That leaves work started under the provisional identity generation-authoritative even though the owning startup result is now “no session.” This is not theoretical: I reproduced a provisional loadFromStorage() entering token refresh, then an owning no-token invocation clearing auth. When the old refresh resolved, it wrote the previous credentials back to storage:

expected accessToken: null
received accessToken: "a-access-refreshed"

A held generation-guarded store request likewise still sees its captured generation as current and can commit after the no-token owner returns.

The owning no-session result needs the same identity teardown contract as logout—generation advance first, then abort/disconnect/reset/clear—while still returning the owning startup result. Add a real-interceptor refresh regression and a held-store regression. The committed no-token test currently verifies only auth fields and that the superseded validation does not call resetAllStores; it does not pin the actual session boundary.

2. The new totalizing outer catch can initialize routing while leaving a provisional identity authenticated

web/src/stores/authStore.ts:223-225 turns every unexpected startup failure into return owns(). If an older invocation already published provisional account A and the newer owning invocation encounters a synchronous storage/platform failure before it clears or replaces that state, it returns true; App then completes initialization while A remains authenticated in memory.

A deterministic probe made storage.getItem() throw for the owning invocation. loadFromStorage() resolved true, but:

isAuthenticated === true
user.id === "u1"

“Total” must mean the owning failure produces a safe, explicit unauthenticated teardown, not merely a boolean fulfillment. A superseded failure should still return false and commit nothing.

3. The production API-call ownership sweep remains incomplete

Several still-mounted identity-owned workflows still call the API directly without any generation/recency ownership. A concrete reproduction is MembersTab in web/src/components/server/ServerSettingsDialog.tsx:650-680:

• the initial apiGetMembers / apiGetRoles continuation writes unconditionally;
• kick/ban mutate local member state after an unguarded await;
• timeout/assign/remove perform a second member fetch without checking ownership after the first await.

I held the initial member/role request, advanced the session generation, then resolved it. The old member rendered and the old request cleared loading in the surviving tab. Similar unguarded identity-owned paths remain in the same dialog’s roles, bans, and audit tabs, and in InviteDialog.

Please finish the direct-call audit rather than relying on component unmount timing. At minimum pin a held old-session success against each shared workflow family, plus a multi-await action proving no second request or local write occurs after the boundary.

Validation

Passed locally:

• 215/215 web tests across 31 files
• test TypeScript check
• production build
• ESLint with only the two existing warnings
• git diff --check
• npm audit: 17 total / 0 critical; production 7 / 0 critical
• GitHub Go, Lint, and Web jobs all green; PR reports mergeable

The three deterministic review probes failed as described. They were removed afterward, and the exact checkout is clean.

Non-blocking: the PR body still reports 198 tests; the head has 215.

PR #22 is not ready to merge. The startup owner now clears the visible identity, but it has not yet killed the session that identity’s work still belongs to.

[Calmingstorm]

Reviewed round 14 at exact head 6959241f51effb955e6b29ef0e91459853b16e0e.

The owning no-token and owning-catch paths now perform the full teardown, and the newly guarded Roles/Members/Bans/Audit/Invite list cases work. The production ownership sweep is still incomplete, however. I found four deterministic blockers:

1. Server Settings Channels can publish an old session/server's list.

   • web/src/components/server/ServerSettingsDialog.tsx:347-352
   • apiGetChannels(serverId) unconditionally writes channels and clears isLoading; unlike the neighboring tabs, it has neither generation nor request-recency ownership.
   • A held account-A response resolved after invalidateSession() rendered old-session-channel and removed the spinner.
   • This also permits an older serverId request to overwrite a newer one.

2. New-DM user search has neither session nor query ownership.

   • web/src/components/dm/NewDMDialog.tsx:43-54
   • A held account-A apiSearchUsers() response resolved after invalidation and rendered @old-session-user.
   • A slower old query can likewise replace a newer query, and its finally can clear the newer search's loading state.

3. Message search has the same stale-session/out-of-order defect.

   • web/src/components/search/SearchDialog.tsx:54-68
   • A held old-session apiSearch() response rendered old-session-search-result after invalidation.
   • This component is mounted in the production MessageList; use generation plus per-query sequence ownership for success, failure, and finally.

4. Pinned Messages has unowned reads and mutation continuation.

   • web/src/components/message/PinnedMessages.tsx:19-43
   • The initial fetch, event-driven refresh, and unpin completion have no generation/channel-recency guard.
   • A held old-session fetch rendered old-session-pinned-message and cleared loading after invalidation. Switching channels or an event refresh can produce the same stale overwrite within one session.

I pinned all four with deterministic review-only component probes; all four failed. The probes were removed and the exact checkout is clean.

Validation otherwise passed:

• 221/221 committed web tests across 32 files
• test TypeScript check
• production build
• ESLint: 0 errors / 2 existing warnings
• diff checks
• npm audit: 17 total / 0 critical; production 7 / 0 critical
• all GitHub jobs green; PR mergeable

PR #22 is not ready to merge. The central generation authority is now coherent, but several ordinary production reads can still carry the previous account across the boundary. The dead continue to send HTTP responses. Very on-brand for distributed systems, less so for authentication.

[Calmingstorm]

Round 27 review — changes requested

Reviewed exact head eac595e8271464638ac5d74e6ba55ca7da1f4c22.

The round-26 arithmetic bridges are gone: mention and ack writes now trigger newer authoritative read-state fetches, held acks are reset-owned, a close/reopen collision asks the server, and a late message no longer clears a guild-channel deletion tombstone. Those changes fix the five reported orderings. Four blocking cases remain.

1. A later reopen can be erased by the close/reopen deciding fetch

When proof of life arrives during closeDM(), the close starts an ordinary fetchDMs() (web/src/stores/dmStore.ts:112-120). A second MESSAGE_CREATE can then prove the DM reopened after that fetch took its closed snapshot but before the response commits. noteChannelAlive() only bumps an epoch and asserts against tombstones (dmStore.ts:144-153); it does not supersede the in-flight fetch. Because the DM is still in the local list, the production handler considers it known and does not start another fetch (web/src/stores/wsStore.ts:71-79). The older empty snapshot therefore commits at dmStore.ts:58-68 and removes the now-open DM.

A deterministic probe produced:

server final truth: reopened dm-race
client dmChannels: []

Proof arriving during the deciding fetch must supersede/restart that decision, or the fetch must validate the channel's alive epoch before committing.

2. If the close wins, the deciding fetch leaves a dangling DM selection

The direct close path clears selectedDMId (dmStore.ts:122-129), but the collision path delegates to fetchDMs(), whose successful commit only writes dmChannels and isLoading (dmStore.ts:68). When the authoritative response omits the selected DM, selectedDMId remains its ID.

The probe ended with:

dmChannels: []
selectedDMId: dm-selected

MessageList and MessageInput still derive their active channel from that stale selection (web/src/components/message/MessageList.tsx:61-71, MessageInput.tsx:54-55). Reconcile selection whenever the authoritative DM list commits, and pin the close-wins case with the DM selected.

3. A delayed pre-ack notification permanently resurrects the local unread flag

Counts are corrected by the new authoritative follow-up, but unreadChannels is explicitly local and fetches never reconcile it (unreadStore.ts:34-35, 61-76). Valid ordering:

1. The server commits a mention and queues NOTIFICATION.
2. The user acks the channel; the ack response and its follow-up fetch settle at count 0 and clear the flag.
3. The older notification is delivered afterward.
4. The WebSocket handler unconditionally calls markUnread() and incrementMention() (wsStore.ts:149-158).
5. Its follow-up restores the count to 0, but nothing clears unreadChannels.

The deterministic probe ended with mentionCount = 0 and isUnread = true. HTTP responses and WebSocket delivery remain separate clocks. The unread flag also needs server/event watermark ownership; server-owned mention counts alone cannot order this case.

4. A failed server-side channel delete now hides a live channel for the session

The backend still broadcasts CHANNEL_DELETE before executing SQL (server/internal/api/channels.go:350-365). If that delete fails, round 27's client installs a tombstone, removes the channel, and deliberately ignores later messages as proof of life (wsStore.ts:59-69). Every authoritative snapshot still contains the live channel, so omission retirement never fires and reconciliation filters it indefinitely (lineage.ts:127-143).

Tracking broadcast-after-commit as a follow-up does not make this reviewed head safe: this PR introduces the persistent tombstone while the pre-commit broadcaster remains live. Land the server ordering fix before/with this client behavior, or provide a reliable failed-delete recovery path.

Regression coverage

The store-level DM tests do not exercise the changed production MESSAGE_CREATE wiring. Removing noteChannelAlive() or the unknown-DM fetchDMs() call from wsStore.ts leaves all 313 committed tests green. Add a handler-level regression that dispatches a real MESSAGE_CREATE for a closed/unknown DM and proves the reopened row is committed. It should also cover the later-proof-during-deciding-fetch ordering above.

Validation

Passed on the exact reviewed commit:

• 313/313 committed web tests across 37 files;
• test TypeScript check;
• production build;
• ESLint: 0 errors / 2 existing warnings;
• git diff --check;
• production npm audit: 7 findings / 0 critical;
• GitHub Go, Lint, and Web jobs green; PR mergeable.

Three deterministic review-only probes failed as described in blockers 1–3. They were removed, and the exact checkout is clean.

PR #22 is not ready to merge. The counts finally ask the server what happened. The DM resolver and unread flag still occasionally accept testimony from the wrong moment in time.

[Calmingstorm]

Round 28 review — changes requested

Reviewed exact head 909f8b1738621477d75909f1aee5e67c113d3c69.

The durable DM-presence evidence, DM-selection reconciliation, server-side delete ordering, and production handler coverage are present. Three blocking cases remain.

1. Post-commit channel-delete fanout can miss clients or deliver duplicates

ChannelHandler.Delete now correctly deletes first, but then calls broadcastToServer() (server/internal/api/channels.go:350-375). That helper queries the surviving channel rows and broadcasts the same event once through each channel (channels.go:42-55). WebSocket subscriptions are per channel, not per server.

I reproduced both failure modes with the real PostgreSQL/Redis/WebSocket harness:

• Connect while the server has only its default channel, then create a second channel. The client receives CHANNEL_CREATE through the default channel, but channel creation does not subscribe existing sockets to the new channel. Delete the default channel. The post-delete query sees only the new channel, to which that socket is not subscribed, so the client receives 0 CHANNEL_DELETE events.
• Connect after a server has three channels, then delete one. The query sees two surviving channels and the same socket is subscribed to both, so it receives 2 identical CHANNEL_DELETE events.

The committed regression has exactly one surviving subscribed channel, so it happens to observe one event and masks both cases. Fan out once per unique server member/client, or capture and deduplicate the relevant recipients before deletion and broadcast only after the delete succeeds. Add both topologies to the real-WebSocket regression.

2. Durable DM evidence overwrites fresher fetched metadata with a stale local row

For a known DM, noteChannelAlive() captures the current local object and journals upsertDM(known) (web/src/stores/dmStore.ts:165-172). During reconciliation, that application runs after the fetched snapshot, and upsertDM gives every field from known precedence (dmStore.ts:40-43). The message event proves only that the DM exists; it does not make the cached recipients or lastMessage fields fresher.

A deterministic probe started a DM fetch, delivered known-DM evidence while the cached preview was m-old, and resolved the fetch with authoritative preview m-new. The committed result was still m-old.

Journal presence/order without replacing a snapshot row's fields—for example, retain the fetched row when present and insert the known row only when absent. The current production-handler pin checks only the ID, so extend it to prove fresher fetched metadata survives replay.

3. The delayed NOTIFICATION path still permanently resurrects unread state

Round 27's failing ordering involved the server's separate NOTIFICATION event. Round 28 passes MESSAGE_CREATE.createdAt into markUnread() (web/src/stores/wsStore.ts:45-59), but NOTIFICATION still calls markUnread(channelId) without any event identity or timestamp (wsStore.ts:151-160). That means it creates no flagRaisedAt entry (web/src/stores/unreadStore.ts:166-179), and an authoritative read-state fetch cannot retire the flag (unreadStore.ts:92-101).

The original valid ordering therefore still fails:

1. MESSAGE_CREATE is delivered and then acknowledged.
2. The ack and authoritative fetch commit count 0 and clear unread.
3. The older, separately queued NOTIFICATION arrives afterward.
4. Its count follow-up returns authoritative 0, but isUnread(channel) remains true forever.

A deterministic store probe confirmed exactly that: timestamp-less markUnread, followed by authoritative mentionCount: 0 / a newer lastReadAt, left the flag set. The commit message explicitly defers this path, but it is the production path behind the round-27 blocker. Put a stable message identity or server timestamp on NOTIFICATION, propagate it through the production handler, and pin the cross-event ordering.

Validation

Passed on the exact reviewed commit:

• 321/321 committed web tests across 37 files;
• web test TypeScript check, production build, and ESLint (0 errors / 2 existing warnings);
• full Go integration suite against local PostgreSQL/Redis;
• full Go race suite against local PostgreSQL/Redis;
• Go vet and gofmt check;
• git diff --check;
• npm audit: 17 total / 0 critical, 7 production / 0 critical;
• GitHub Go, Lint, and Web jobs green; PR mergeable.

Five deterministic review assertions across three probes failed as described. All probe files were removed, and the exact checkout is clean.

PR #22 is not ready to merge. The delete now happens before the obituary, but the obituary is being mailed once per surviving room—and occasionally to none of the people who knew the deceased.

[Calmingstorm]

Round 29 review — changes requested

Reviewed exact head 650423c898950a6d4491bfda9120090c9d6caf7c.

The round-28 duplicate/missed-delete reproductions are fixed for ordinary members, the DM aliveness replay now preserves fresher fetched content, and NOTIFICATION.createdAt is wired through the production handler. Two blockers remain.

1. Per-member channel fanout bypasses ViewChannel and leaks hidden-channel events

server/internal/api/channels.go:47-60 now sends every channel lifecycle event directly to every row in server_members. That recipient set is broader than the authorized realtime set: the channel list and WebSocket subscriptions deliberately exclude channels from members without ViewChannel.

I reproduced this with the real PostgreSQL/Redis/WebSocket harness:

1. Join a member to a server.
2. Remove ViewChannel from the default role and let subscription reconciliation complete.
3. Create, update, and delete channels as the owner.

The revoked member received all three events:

member without ViewChannel received 1 CHANNEL_CREATE event
member without ViewChannel received 1 CHANNEL_UPDATE event
member without ViewChannel received 1 CHANNEL_DELETE event

CHANNEL_CREATE contains the full channel object, and production wsStore routes it to serverStore.addChannel(), so this is not merely harmless invalidation traffic: a hidden channel can be installed in the selected server's UI. CHANNEL_UPDATE also exposes the full metadata over the socket.

Keep the exactly-once property, but preserve the authorized intended recipient set, not raw server membership. For delete, capture/deduplicate that set before deleting the row and notify it once after commit. Create/update/reorder likewise need view-authorized recipients. Add a real WebSocket regression with a server member whose ViewChannel has been revoked.

2. createdAt is not a causal watermark because bots may override it

The unread fix assumes Message.createdAt and ReadState.lastReadAt are server-minted values whose ordering reflects server mutation order. That is false for messages:

• messages.go:470-479 permits bots to supply createdAt.
• messages.go:503-513 persists that supplied display timestamp as msg.CreatedAt.
• The same value is now placed in NOTIFICATION, and both production handlers pass it to markUnread().
• unreadStore.ts:166-172 drops an event whenever that value is at or before the prior ack's lastReadAt.

A real API probe acknowledged a message, then successfully created a new bot mention after that ack with createdAt = ack.lastReadAt - 1 hour. The server returned 201, and the new notification carried the earlier timestamp. Production therefore classifies a post-ack event as already read. For a backdated non-mention bot message, there is no mention badge to compensate, so the new message can produce no unread indication at all.

Use a non-overridable causal identity/watermark—such as a server insertion sequence/time distinct from display createdAt, or stable event/read identities—and pin the post-ack/backdated-message case through server payload and production client wiring. A user-controlled presentation timestamp cannot order acknowledgements.

Validation

Passed at the exact reviewed head:

• 323/323 web tests across 37 files
• Test TypeScript check
• Production build
• ESLint: 0 errors / 2 existing warnings
• Full Go -race suite with local PostgreSQL/Redis
• Targeted round-29 Go regressions
• gofmt and go vet
• Diff checks
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable

The two deterministic review probes were removed; the exact checkout is clean.

PR #22 is not ready to merge. Exactly-once delivery is now exactly once to too many people, and the purported server clock can still be supplied by a bot. Distributed systems do appreciate irony.

[Calmingstorm]

Reviewed Bastion PR #22 round thirty at 0e61b682e3fb62167596b8cbbb96ee0f72338ddf.

The raw-membership leak is fixed in the ordinary non-racing path, and bot-supplied presentation timestamps no longer directly drive the web client's unread clock. I found three blockers.

1. Creating a channel still does not subscribe already-connected authorized clients to it

ChannelHandler.Create() inserts the channel and calls the new per-user broadcastToServer() (server/internal/api/channels.go:217-234), but nothing adds those users' live clients to the new channel's Hub subscription. WebSocket subscriptions are otherwise established from the channel rows that exist at connect/join time, or reconciled after role changes.

I reproduced through the real HTTP/PostgreSQL/Redis/WebSocket harness:

1. Owner creates a server and member joins.
2. Member connects its socket.
3. Owner creates a new channel; member receives exactly one CHANNEL_CREATE through BroadcastToUser.
4. Owner posts a message in the new channel.
5. Member receives zero MESSAGE_CREATE events because its socket was never subscribed to the new channel ID.

The direct event makes the channel visible in client state while its realtime stream is dead until reconnect. Channel creation needs to synchronously reconcile/subscribe every currently authorized connected user to the new channel, with a regression proving the subsequent channel broadcast arrives. That subscription path also needs to compose safely with a concurrent permission revocation rather than reinstall revoked access.

2. Delete fanout uses authorization captured too early and bypasses a later revocation

Delete() captures authorizedMemberIDs() before the row mutation (channels.go:385-388), then broadcasts directly to those user IDs after deletion (channels.go:390-416). BroadcastToUser does not consult channel subscriptions. A member can therefore lose ViewChannel, be synchronously unsubscribed, and still receive the later delete event from the stale captured list.

A deterministic integration probe held the channel row lock so deletion stopped after its recipient query, revoked the member's ViewChannel, then released the delete. The revoked socket received one CHANNEL_DELETE after revocation.

With today's server-scoped visibility, there is no row-dependent reason to preserve that stale audience: resolve current authorization after the successful delete and immediately before fanout. If future per-channel overrides require pre-delete state, preserve that state transactionally but still intersect it with current membership/role authorization at delivery, or use an equivalent authorization epoch. Add the revocation-during-delete regression; the current test covers only a member revoked before the operation starts.

3. Emission timestamps are not causal read watermarks

The new eventAt and notification createdAt are generated with application-process time.Now() at broadcast (server/internal/api/messages.go:550-558, 829-840), while acknowledgements store PostgreSQL NOW() at ack execution (server/internal/api/readstate.go:72-75). The client compares these values as if they proved whether the ack covered the message (web/src/stores/wsStore.ts:51/163, web/src/stores/unreadStore.ts:166-174). They do not.

Two histories are indistinguishable to the client:

• A bot creates a genuinely post-ack message with a backdated presentation timestamp. It should become unread.
• A pre-ack message is committed, the user acknowledges it, and its WebSocket broadcast/notification is delayed until afterward. The fresh emission time is post-ack even though the message was covered, so it resurrects unread. The authoritative follow-up cannot clear the flag because lastReadAt < emissionTime.

The notification path has an especially direct window: mention count is committed, an ack can commit, then time.Now() is sampled and the notification is emitted. Separate application and database clocks add skew on top, but even perfectly synchronized clocks cannot solve the causal ambiguity. Also, lastReadAt describes when the ack request ran, not which later/earlier message events its supplied messageId covered.

Use a stable server-owned message/read identity or causal watermark generated with the message write and recorded from the acked message when acknowledging it; carry that same non-overridable value in both MESSAGE_CREATE and NOTIFICATION. A database-generated event sequence or message event timestamp paired with an acked-message watermark would work. Pin both sides: a backdated message truly committed after an ack raises unread, while a message covered by an ack but delivered afterward does not.

Validation

• 324/324 web tests across 37 files
• Test TypeScript check and production build
• ESLint: 0 errors / 2 existing warnings
• Full Go integration/race suite against local PostgreSQL and Redis
• Targeted six Go regressions passed
• Go vet and gofmt passed
• git diff --check passed
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable
• Two deterministic integration probes failed as described, were removed, and the exact checkout is clean

PR #22 is not ready to merge. The recipient query now asks who is authorized, but one path asks too early and another never installs the subscription. The timestamp fix answers when a packet left, not whether its message had already been read.

[Calmingstorm]

Reviewed Bastion PR #22 round thirty-one at exact head d4c1ef07476aebedea376ef1785aab25f7c2cfc3.

The round-30 directions are materially better: new-channel subscriptions are installed, delete authorization is queried after commit, and a database watermark is the right replacement for wall-clock inference. The implementation still does not establish the claimed invariants.

Changes requested

1. messages.seq is allocation order, not commit/visibility order

Migration 014 uses a plain PostgreSQL sequence as the causal order (server/migrations/014_message_seq.up.sql:20-25). nextval() is assigned during INSERT, not at commit, and concurrent transactions can commit in the opposite order.

I reproduced this deterministically against the real PostgreSQL harness:

1. Transaction A inserts a message and receives seq=1, but remains uncommitted.
2. The real send endpoint inserts and commits seq=2.
3. The user acknowledges message 2, storing last_read_seq=2.
4. Transaction A commits afterward.

The newly visible post-ack message therefore has seq=1 <= last_read_seq=2; the web client drops its late MESSAGE_CREATE as already read. Explicit-transaction writers such as upload and bulk import make this an application-reachable ordering, not merely a synthetic SQL curiosity.

The order must be consistent with visibility/commit for a channel. For example, serialize message writes and acknowledgements on a per-channel database row/advisory lock before allocating the watermark, or use another committed per-channel ordering scheme. A bare sequence does not provide commit order.

2. The seq contract omits three persisted MESSAGE_CREATE writers

Only normal send and bulk import scan seq. These persisted-message paths still construct models.Message without it and broadcast the legacy raw payload:

• server/internal/api/uploads.go:93-105,170-174
• server/internal/api/webhooks.go:511-524,539-543
• server/internal/api/interactions.go:674-700

A real WebSocket probe through webhook execution received a persisted MESSAGE_CREATE with seq=0/omitted. The client silently falls back to time ordering, reintroducing the exact race this migration is meant to remove. Audit every persisted message insertion and make the response/event contract uniform. The attachment route is especially important because its explicit transaction can assign created_at/seq, remain uncommitted during file work, and become visible only after a concurrent ack.

3. A stale ack still erases newer mention state and regresses read metadata

server/internal/api/readstate.go:72-79 protects only last_read_seq with GREATEST. Every ack still unconditionally writes:

• last_message_id = $3
• last_read_at = NOW()
• mention_count = 0

Real-harness reproduction:

1. Acknowledge old message 1.
2. Message 2 mentions the user; authoritative mention_count becomes 1 and message 2's seq is above the watermark.
3. Repeat/deliver the stale ack for message 1.
4. last_read_seq correctly stays put, but mention_count becomes 0 even though that ack did not cover message 2.

Likewise, acknowledging a newer message and then an older one leaves last_read_seq at the newer value while regressing last_message_id and advancing fallback last_read_at. The entire read-state mutation must be conditioned/reconciled on the causal watermark. Exact mention reconciliation needs a stable mention identity/watermark or persisted mention events; a scalar count cannot determine how many mentions lie above an arbitrary ack watermark.

4. The optimistic web ack deletes the known watermark

web/src/stores/unreadStore.ts:149-164 replaces the channel's ReadState but omits lastReadSeq. During the follow-up fetch—and indefinitely if it fails—the client forgets even the previously committed watermark.

A deterministic store probe started with lastReadSeq=90, completed ackChannel(), held its follow-up fetch, then delivered delayed event seq=85. The channel became unread because the optimistic write had removed 90. Preserve the known watermark at minimum; preferably have ack return the committed ReadState/watermark so the client immediately knows the newly covered seq rather than depending on a second request.

5. Subscription lifecycle remains raceable

There are two independently reproduced races:

• Create vs permission revocation: channels.go:242-249 queries authorized recipients, then separately calls SubscribeUser. If a role update revokes ViewChannel and completes its reconciliation between those operations, the stale create continuation re-subscribes the revoked member. With a deterministic seam at that point, the revoked socket received a later MESSAGE_CREATE. This needs the same revocation-generation/revalidation discipline used by ConnectClient, not a one-time authorization snapshot.
• Delete vs connection admission: Hub.RemoveChannel() deletes the map but does not advance revGen. A connecting client can read the channel before deletion, deletion can commit and call RemoveChannel, and then ConnectClient can install its stale snapshot afterward, recreating the deleted channel's subscriber set. A unit probe reproduced it. Channel removal must invalidate in-flight connection snapshots.

The new server-create regression is also not pinning its named path: TestServerCreateSubscribesConnectedOwner creates another channel through ChannelHandler.Create and sends to that new channel. Removing the new ServerHandler.Create subscription block still leaves this test green because channel creation performs the subscription. Send to the default channel created inside server creation before invoking any other subscribing path.

Validation

Passed on the exact reviewed head before adding probes:

• 326/326 committed web tests
• Web test type-check and production build
• ESLint: 0 errors / 2 existing warnings
• Full Go integration/race suite against local PostgreSQL and Redis
• Go vet and gofmt
• Diff checks
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable

Deterministic review probes for all failures above were removed. The checkout is clean at the exact reviewed commit.

PR #22 is not ready to merge. The new watermark is database-owned, but it is not yet commit-ordered, universally emitted, or applied atomically to the state it is supposed to govern.

[Calmingstorm]

Round 32 review — changes requested

Reviewed exact head c53e7326773dc63286a1f08b7079c0a06ca2b9c4.

The five round-31 fixes are materially present: every persisted message writer now uses the per-channel insert lock and emits seq; stale/duplicate acknowledgements no longer rewrite read-state fields; the optimistic client ack preserves lastReadSeq; channel removal advances reconciliation; and the server-create regression now exercises the default channel. The full committed suites pass. Five blocking cases remain.

1. A fresh older ack still erases mentions strictly above its watermark

This is the disclosed residual, and the real harness confirms it. The strict gate in server/internal/api/readstate.go:79-85 rejects stale/duplicate acks relative to the current last_read_seq, but an ack that legitimately advances from no watermark to message seq=1 still unconditionally sets mention_count=0 even when a mention at seq=2 was already counted by messages.go:863-869.

Deterministic reproduction:

1. Create message 1.
2. Create message 2 mentioning the member; authoritative mention_count becomes 1.
3. Freshly ack message 1.
4. Read state ends as last_read_seq=1, mention_count=0.

This is not merely a stale-retry case; the new read watermark and mention count disagree about server truth. TestMentionIncrementRespectsWatermark covers the opposite order—ack the same mention before its delayed increment—and therefore does not pin this case. Mention accounting needs identities/watermarks scoped to message seq so an ack clears only mentions it covers. The acknowledged follow-up issue does not make the causal state introduced by this PR safe to merge.

2. A covered delayed NOTIFICATION is not a full no-op

The production handler calls three independent continuations (web/src/stores/wsStore.ts:155-173):

1. markUnread(...), which correctly returns early for seq <= lastReadSeq;
2. incrementMention(...), unconditionally when mentionCount is present;
3. browser notification display, also unconditionally.

Because markUnread() returns void, its rejection of a covered event cannot stop the other two. A deterministic handler-level probe began at lastReadSeq=90, mentionCount=0, delivered NOTIFICATION seq=85, and ended with isUnread=false but mentionCount=1; a hidden tab would also show the stale browser notification. The authoritative fetch can eventually repair the badge, but it can fail or remain pending, and covered events were explicitly meant to be complete no-ops.

Make the watermark decision own the whole notification workflow (for example, return an accepted/covered result and gate the optimistic count plus browser notification), then pin the production handler with a seq payload. The current committed handler tests exercise only the timestamp fallback.

3. Channel-event authorization still has a post-check TOCTOU

ChannelHandler.Create performs its second authorization query at server/internal/api/channels.go:259-263, then directly sends to that captured slice at 269-272. Direct BroadcastToUser bypasses the now-current channel subscriptions (channels.go:94-103). A revocation can therefore commit and fully reconcile after the second query but before delivery; the stale confirmed slice still receives CHANNEL_CREATE.

I added a review-only seam immediately after the second query, completed a real role revocation through the HTTP API, then resumed creation. The revoked socket received exactly one CHANNEL_CREATE. The committed test only revokes before the second query, so it misses the remaining window.

The same query-to-direct-delivery shape exists for update/delete through broadcastToServer. Channel-scoped delivery needs a structural authorization check at the actual hub delivery—such as intersecting the current authorized set with live channel subscriptions under the hub lock—or equivalent serialization. Another query followed by another gap is not closure.

4. Create racing delete can resurrect a deleted channel in the hub and emit a phantom create

Channel creation inserts the row in an autocommit statement (channels.go:230-241) and only afterward subscribes users and broadcasts (243-274). A concurrent delete can observe and delete that committed row, broadcast CHANNEL_DELETE, and call RemoveChannel() (423-457) before the create continuation runs.

A deterministic real-harness probe deleted the new channel immediately after its insert. The create request then:

• returned 201 for a row that no longer existed;
• recreated its hub subscriber set via SubscribeUser();
• emitted one CHANNEL_CREATE after the delete.

The client can therefore resurrect the dead channel, and the hub retains a subscriber set for a nonexistent row. Creation must own the row through subscription/event publication or revalidate existence/generation before every post-insert side effect; the create and delete lifecycle need one serializable ordering.

5. Server deletion can omit a member who joins after the recipient snapshot

ServerHandler.Delete snapshots memberIDs and channel IDs outside a transaction at server/internal/api/servers.go:542-550, then separately deletes at 552. A join can commit in between. The cascade removes that newly committed membership, but the user is absent from the captured recipients and receives no SERVER_DELETE.

A deterministic seam after the snapshot completed a real invite join before allowing deletion to continue. The joined socket received zero deletion events even though its join succeeded before the server was deleted. Its client can retain the just-joined server with no correcting event.

Serialize joins against deletion and capture the recipient/channel sets inside the deleting transaction while holding the server row lock. Then either the join commits first and is included, or deletion owns the row and the join cannot succeed.

Validation

Passed on the exact reviewed head:

• 330/330 web tests across 37 files;
• web test TypeScript check and production build;
• ESLint: 0 errors / 2 existing warnings;
• full Go integration/race suite against local PostgreSQL and Redis;
• Go vet and gofmt;
• git diff --check;
• npm audit: 17 total / 0 critical, 7 production / 0 critical;
• GitHub Go, Lint, and Web jobs green; PR mergeable.

Five deterministic review failures were reproduced across server and production-handler probes. All probes were removed; the exact checkout is clean.

PR #22 is not ready to merge. The sequence now respects commit order. The remaining failures happen one layer above it: what an ack covers, whether a covered event is truly ignored, and which lifecycle transition actually won.

[Calmingstorm]

Round 33 review — changes requested

Reviewed exact head c0cd7633536ec9e1d302153212af59f6b767a254. The mention projection and server-delete row lock fix the reported round-32 cases, and the stable subscription loop closes the seams covered by the committed tests. Three blockers remain.

1. ackChannel() still optimistically zeroes a newer mention when the server ack is a no-op

web/src/stores/unreadStore.ts:122-179

The server's watermark gate correctly makes an ack of the already-read message a no-op, but the client cannot tell: apiAckChannel() returns no read state. The local dedupe at lines 127-133 is bypassed whenever a newer mention/unread flag exists, even if messageId still equals the stored lastMessageId. After the unchanged ack returns, the untouched predicate passes and lines 157-176 set mentionCount: 0 and clear unread.

Deterministic committed-code probe:

1. Seed c with lastMessageId=m1, lastReadSeq=1, mentionCount=0.
2. Handle a mention for seq=2 (incrementMention() + markUnread()), but keep the rendered list's last message at m1.
3. Call ackChannel(c, m1) and hold/fail the authoritative follow-up fetch.
4. Observed mentionCount become 0 and isUnread become false, although the server ack was a strict no-op and the derived server count remains 1.

The test failed at the count assertion: expected 1, received 0.

Do not optimistically clear unless the response proves the advanced watermark. The clean contract is for ack to return the committed ReadState/watermark and apply that result; otherwise preserve newer local badge/flag state until the authoritative fetch settles. Pin the identical/stale-ack response with a newer mention and a failed or held follow-up.

2. Mention persistence failure still emits a phantom badge and browser notification

server/internal/api/messages.go:855-901

The new model makes mentions the authoritative source, but processMentions() logs and continues when either the mention-row insert or read-state-row insert fails, then unconditionally broadcasts NOTIFICATION. The client optimistically increments and triggers a fetch; that fetch has no mention row to return, so the visible badge/notification was emitted for state the server never committed.

I installed a temporary BEFORE INSERT ON mentions trigger that raises, then posted @member phantom through the real HTTP/PostgreSQL/Redis/WebSocket harness. The message returned 201, the server logged the forced mention write failure, the authoritative count was 0, yet the member received one NOTIFICATION. The probe failed exactly on that event count.

Treat persistence as part of producing the notification: insert/ensure atomically, and broadcast only after both succeed. At minimum, stop that recipient's path on either error. Add a deterministic DB-failure regression proving no notification is sent when the authoritative mention cannot be recorded.

3. SubscribeAuthorizedStable is not actually atomic with authorization changes or delivery

server/internal/realtime/hub.go:290-310, server/internal/api/channels.go:257-296

The generation loop still has an unguarded gap after ReconcileGen() == gen and before broadcastToServer(...). Authorization can change in that interval. The role mutation commits, reconciles/unsubscribes, and bumps the generation, but SubscribeAuthorizedStable() has already returned its captured confirmed slice; channel create then directly sends CHANNEL_CREATE to that now-revoked user via BroadcastToUser.

A temporary seam immediately after SubscribeAuthorizedStable() returned and before the subErr/broadcast branch revoked ViewChannel through the real role API. The member was synchronously unsubscribed from the channel, but still received exactly one CHANNEL_CREATE from the stale confirmed slice. The probe expected zero and failed.

The reverse transition is also lost: grant ViewChannel during the in-loop test seam. Role reconciliation subscribes the member, but because a grant does not bump revGen, the create pass returns the old recipient set and emits no CHANNEL_CREATE; my real harness probe received zero announcements even though the member immediately received subsequent messages.

The invariant needs a single ordered authorization/subscription/delivery operation, or authorization revisions bumped for every visibility change plus a final delivery guarded by the same revision/lock. A sampled generation followed by an unlocked direct-user broadcast cannot provide the claimed stable result. Pin both post-stability revocation and grant-during-create.

Validation

Passed otherwise:

• 333/333 web tests across 37 files
• Web test TypeScript check
• Production build
• ESLint: 0 errors / 2 existing warnings
• Full Go integration suite against PostgreSQL and Redis
• Full Go race suite against PostgreSQL and Redis
• Go vet, gofmt, and diff checks
• npm audit: 17 total / 0 critical; production 7 / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable
• All review probes removed; exact checkout clean

PR #22 is not ready to merge. The database projection is the right model. The client still guesses what an ack committed, mention delivery can outrun its own persistence, and the subscription loop labels a snapshot stable before the event using it has actually been delivered.

[Calmingstorm]

Reviewed Bastion PR #22 round 34 at 6c0ee9867cbfe8df0ab5feaf5709925c320e5daa.

The three round-33 changes are present, but four blockers remain:

1. Concurrent ack responses can regress the client to an older committed watermark.

   unreadStore.ackChannel() applies each returned read state and claims the lineage in response-arrival order (web/src/stores/unreadStore.ts:130-190). The server watermark gate protects database commit order, but an earlier response does not magically include a later commit.

   Deterministic store probe:

   • ack m1 commits first but its response is delayed;
   • ack m2 commits later and its response arrives first, installing lastReadSeq=2, mentionCount=0;
   • the delayed m1 response then installs lastReadSeq=1, mentionCount=1.

   Final client state was seq 1, not seq 2. Ack completions need monotonic read-state reconciliation by lastReadSeq or request/commit recency, not bare completion order.

2. Live-subscription delivery is atomic with hub mutations, but not with authorization commits.

   Role permission changes commit in PostgreSQL before reconcileServerSubscriptions() runs (server/internal/api/roles.go:445-470; the remove/assign paths have the same split). A handler can be descheduled after the DB commit but before it updates the hub. During that window, CHANNEL_CREATE/single-channel CHANNEL_UPDATE still use the stale live subscription set (server/internal/api/channels.go:315, :427).

   I reproduced the real state transition by committing a ViewChannel revocation after the create stability pass and before delivery, without yet running hub reconciliation. The revoked member received exactly one CHANNEL_CREATE. Locking BroadcastToChannel only serializes against UnsubscribeUser; it cannot see an authorization change that has committed but has not reached that call yet. The authorization commit, reconciliation generation, and delivery need one ordering protocol.

3. Mention persistence and its read-state anchor are not atomic.

   processMentions() commits the mentions row, then separately inserts the read_states anchor (server/internal/api/messages.go:881-904). If the second statement fails, the code skips notification but leaves a committed mention that the read-state list cannot currently expose; a later successful anchor can make that old mention appear without its corresponding notification.

   A PostgreSQL trigger forcing only the anchor insert to fail produced:

   HTTP message result: 201
   mentions rows:        1
   read_states rows:     0
   

   Persist the mention and anchor in one transaction, then emit NOTIFICATION only after that transaction commits.

4. The bounded channel-subscription stabilization failure returns success with a permanently unusable live channel.

   On ErrConnectUnstable, channel creation keeps the committed row, calls RemoveChannel(), and returns 201 (server/internal/api/channels.go:285-298). The comment claims the unknown-message fallback heals this, but a client removed from the hub channel cannot receive that first message, and a channel-list fetch does not reinstall hub subscriptions.

   Forcing all eight stability passes to move the generation yielded:

   create channel:       201
   send message:         201
   connected owner saw:  0 MESSAGE_CREATE events
   

   The failure path must leave authorized connected clients subscribed, explicitly reconcile them, or roll back/otherwise report the failed realtime installation. Returning a healthy 201 after removing every subscription is not a degraded success.

Confirmed improvements:

• Ack now returns a transactionally read server state instead of an empty response.
• A failed mention-row insert emits no phantom notification.
• Delivery through the live channel set excludes members after hub reconciliation has completed.
• Genuine grants advance the reconciliation generation.

Validation:

• 336/336 web tests
• Web test TypeScript check
• Production build
• ESLint: 0 errors / 2 existing warnings
• Full Go integration/race suite against local PostgreSQL and Redis
• Go vet and diff checks
• npm audit: 17 total / 0 critical, 7 production / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable
• Four deterministic probes failed as described, then were removed
• Exact checkout clean at the reviewed commit

PR #22 is not ready to merge. The database is finally returning authoritative state. The client and hub still occasionally replace “authoritative” with “whichever continuation got scheduled last,” because apparently causality needed another on-call rotation.

[Calmingstorm]

Reviewed Bastion PR #22 round 35 at exact head df34d6714322cda2568b49f5698f5decde0b9ad9.

The atomic mention/anchor transaction is correct, and higher-watermark ack responses no longer regress lower ones. The new best-effort path also closes the specific persistent-subscription leak covered by its committed test. Four blockers remain.

1. Equal-watermark ack responses can still regress the authoritative mention projection

web/src/stores/unreadStore.ts:173-178 compares only lastReadSeq, and the tie goes to the response settling now:

existing.lastReadSeq > committed.lastReadSeq ? existing : committed

Two concurrent duplicate acks can both return lastReadSeq=5 while carrying projections observed at different database moments. An earlier transaction can read mentionCount=0, commit, and have its HTTP response delayed; a mention above seq 5 then commits; a duplicate ack reads mentionCount=1 and reaches the client first. The delayed equal-watermark response then wins the tie and restores mentionCount=0/clears unread.

A deterministic store probe reproduced exactly that:

1. start two ackChannel(c1, m5) calls;
2. resolve the later request first with {lastReadSeq: 5, mentionCount: 1};
3. resolve the older request last with {lastReadSeq: 5, mentionCount: 0};
4. final state incorrectly becomes count 0, unread false.

The accepted watermark + count tuple needs lineage. Use per-channel ack request ownership/in-flight deduplication for equal watermarks, or return a server-owned projection/version that orders equal-watermark snapshots. Watermark comparison alone cannot order them.

2. The degraded floor can deliver stale CHANNEL_CREATE after CHANNELS_STALE

At server/internal/api/channels.go:372-378, the fallback captures authorized IDs and then calls BroadcastToUser without any generation or authorization fence around delivery.

A deterministic real HTTP/PostgreSQL/Redis/WebSocket probe forced non-convergence, then revoked ViewChannel after that final ID query but before the loop sent events. The revocation completed reconciliation and emitted CHANNELS_STALE; afterward the stale ID slice delivered one CHANNEL_CREATE to the revoked member. Because the stale event comes after the compensating refetch signal, the client has no later correction and retains the hidden channel metadata.

CHANNELS_STALE only heals create-before-revoke ordering. It cannot repair revoke/stale/create ordering. The final authorization decision and delivery need one fence/versioned protocol rather than an unfenced captured slice.

3. The degraded floor can emit a phantom create after a successful delete

The existence check at server/internal/api/channels.go:363-369 is separated from the final recipient query and direct fanout at :372-378.

A deterministic integration probe deleted the channel successfully after floorExists returned true but before the final fanout. The connected member received CHANNEL_DELETE, the row was confirmed absent, and then received one later CHANNEL_CREATE for that deleted channel. Unlike the stable path, direct per-user delivery does not intersect the hub set that RemoveChannel() cleared.

Existence must remain fenced through delivery. A check-before-fanout is not enough; deletion can commit in that gap.

4. Non-convergence still leaves authorized connected members unable to receive messages

When the final guarded pass moves, server/internal/api/channels.go:347-350 rolls back the speculative subscriptions. The degraded floor at :370-378 sends only a one-time CHANNEL_CREATE; it does not restore a live subscription.

A real integration probe forced this path without changing authorization. The authorized connected member received exactly one CHANNEL_CREATE, but a subsequent message sent to that channel produced zero MESSAGE_CREATE events. The client now knows the row exists, but it remains unusable until reconnect. That is still the round-34 failure mode; an announcement is not a subscription, and the unknown-channel fallback cannot run when messages never reach the socket.

The fallback needs to leave every still-authorized connected client with a validated live subscription, or continue/retry reconciliation until it can do so safely. One-time direct delivery is not a functional floor.

Validation

Passed on the exact head:

• 339/339 web tests across 37 files
• web test TypeScript check
• production TypeScript/Vite build
• ESLint: 0 errors / 2 existing warnings
• full Go integration/race suite against PostgreSQL and Redis
• Go vet and gofmt checks
• diff checks
• production npm audit: 7 findings / 0 critical
• GitHub Go, Lint, and Web jobs green; PR mergeable

Review probes were removed and the exact checkout is clean.

PR #22 is not ready to merge. The normal path is increasingly disciplined. The non-convergence floor still steps outside that discipline, which is where distributed systems keep the knives.

[Calmingstorm]

Reviewed round 36 at cbd7873, then pushed the requested fix directly to fix/web-session-guards in commits 859524f and 791da4b. Current reviewed head: 791da4b04be147fdf369391829568f4a28a077bd.

The round-36 rewrite fixed the degraded-floor failure class, but one remaining blocker in each model still needed a server-owned ordering primitive:

1. Equal-watermark read-state projections still had no reliable order. Keeping the existing value on >= prevents a stale response from regressing a newer one, but also rejects a genuinely newer equal-watermark response. A mention can commit above unchanged lastReadSeq between reads. I added migration 016 with read_states.projection_revision; committed mention INSERT/DELETE and advancing acks increment it under the read-state row lock, API responses expose it, and the client uses it as the tie-breaker with a pre-migration watermark fallback.

2. Hub reconciliation was still not serialized with authorization/deletion commits, and only removed users subscribed by that invocation. A pre-existing stale subscription could survive an authoritative read. I changed reconciliation to replace the entire live channel set with the authorized connected-user set. Migration 017 adds a per-server PostgreSQL advisory fence to every commit that changes membership, roles, or deletes channels/servers. Channel creation holds the matching shared session lock through the authoritative existence/authz read, exact hub reconcile, and synchronous live-set dispatch. This closes the DB-commit-to-hub-mutation gap rather than hoping the generation counter observes it.

Added regression coverage for the equal-watermark projection revision and exact removal of pre-existing unauthorized subscriptions.

Validation at 791da4b:

• 341/341 web tests
• Web test type-check and production build
• ESLint: 0 errors / 2 existing warnings
• Full Go suite with -race against PostgreSQL and Redis
• Go vet, gofmt, and diff checks
• Production npm audit: 7 findings / 0 critical
• GitHub Go, Lint, and Web jobs all green
• PR is mergeable; checkout clean

I am leaving final merge approval to your review of the pushed changes, as requested. The loop has now been broken with database ordering rather than another round of interpretive archaeology.