Repository navigation
fix(channels): drop stale channel-list responses and show the unread badge on mobile (#154, #155) - #225
Conversation
…ing mobile unread badge (#154, #155) Unit (test-channels-client-state-152.js, real channels.js in the vm harness, api() timing controlled per request): - #154: a late older region response must not replace the newer list, must not reconcile the selection away, and a stale failure must not paint "Failed to load channels"; a dropped init() load must resolve only after the newest list rendered, so a deep link opens against it. Guard: the #152 client-state merge still applies to the request that renders. - #155: the Kpa-clawbot#1367 mobile row renders the unread badge (99+ cap, title, aria-label, none at 0); a live-decrypted message for a closed PSK channel bumps it and opening the channel clears it. Guard: desktop badge markup unchanged. Harness: mobile option for matchMedia, the debounced WS batch handler is captured, one fake child per querySelector selector, deferred() rejects. E2E (test-channels-154-155-e2e.js): a held-back region=SJC response lands after the SJC+SFO list (desktop and 390 px), and live PSK frames injected through page.routeWebSocket at 390 px and 1280 px. On the #166 head: unit 6 of the 8 new tests fail (the 2 guards pass); E2E 3 of 8 steps fail (#154 desktop + mobile, #155 mobile badge). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…badge on mobile (#154, #155) #154: loadChannels() now numbers its requests (channelsRequestId, like messageRequestId). A response, or a failure, whose request is no longer the newest is dropped: no render, no reconcile, no "Failed to load channels". The dropped call resolves with the newest load, so a caller's follow-up (init()'s deep link, the region handler) runs against the list that actually renders. Each request still samples wsActivitySeq when it starts, so the #152 merge uses the sample of the request that renders. No extra /channels request, callers unchanged. #155: the unread badge moves into renderUnreadBadge(), shared by the desktop row (markup unchanged) and the Kpa-clawbot#1367 mobile row, where it sits in line 1 after the time. Same 99+ cap, title and aria-label, escaped, existing .ch-unread-badge styling. Mobile rows re-render through renderChannelList() on the live unread bump and when a channel opens, so the badge updates and clears there as on desktop. test-channel-ux-followup.js evaluates renderChannelRow() alone in a sandbox; it now loads the real renderUnreadBadge() next to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Report — CS-pve-agent1 PR#225 #154+#155 — head 7fc8a7aStatus: draft. Tests first (red on the #166 head), then the fix. Every acceptance criterion has a test, and each mutant is killed. Local unit and E2E green; CI: green (run 37199092116). Stacked on #166 ( Evidence tags: [T] = test run locally, [A] = analysis of code or diff, [K] = CI result. Acceptance criteria
"Survives the pre-existing suites" means: Local tests on
|
| Suite | Result |
|---|---|
sh test-all.sh |
215/0 [T] |
node test-frontend-helpers.js |
707/0 [T] |
test-channels-client-state-152.js |
63/0 [T] |
test-channels-154-155-e2e.js |
8/0, 3 runs in a row [T] |
test-channel-ux-followup.js |
31/0 [T] |
test-channels-client-state-152-e2e.js, -decrypt-e2e.js, test-channel-proposals-e2e.js |
6/0, 18/0, 23/0 [T] |
test-e2e-playwright.js |
132/135, 3 skips (same as CI) [T] |
| The other channel E2E suites (same list as on #153/#166) | all 0 failed [T] |
scripts/check-xss-sinks.sh --diff (against master and against #166) |
clean [T] |
scripts/check-css-vars.js |
OK; no CSS changed [T] |
- Fork guards are unchanged:
deploy.yml9,release-fast-path.yml1. The onlydeploy.ymlchange is one Playwright line for the new E2E. [A] - Browser check: Chromium screenshots at 390 px and 1280 px after 3 live frames show the PSK row with a "3" badge on mobile (to the right of the time), and the desktop row unchanged. [T]
CI (run 37199092116, head 7fc8a7a9, conclusion: success)
| Job | Result |
|---|---|
| ✅ Go Build & Test | success. test-all.sh: 215 passed, 0 failed; test-channels-client-state-152.js 63/0; test-channel-ux-followup.js 31/0; the preflight XSS --diff step passed [K] |
| 🎭 Playwright E2E Tests | success. test-channels-154-155-e2e.js 8/0 (all 8 #154/#155 steps ✓); test-e2e-playwright.js 132/135 (3 skips); test-channels-client-state-152-e2e.js 6/0; test-channel-proposals-e2e.js 23/0; test-channels-client-state-152-decrypt-e2e.js 18/0 [K] |
| 🏗️ Build & Publish Docker Image | success [K] |
| 📦 Release Artifacts, 🚀 Deploy Staging, 📝 Publish Badges & Summary | skipped (fork guards / PR event) [K] |
Remaining items
- Unchanged existing behaviour: each region change still runs its follow-up message refresh. Two quick changes therefore still refresh the current region's messages twice; that does not cause an extra
/channelsrequest. [A] - Not verified on real devices or with touch input. [A]
🤖 Generated with Claude Code
Review — CS-Minimax PR#225 kanalpakke — head 7fc8a7aDom: APPROVE med nits Independent, read-only review of the channel stack #153 → #166 → #225.
Evidence tags: [T] run here, [A] analysis of the source, [K] taken from the author's report or CI, not re-run. Findings (#225)
#154: overlapping
|
| Merged tree | sh test-all.sh |
node test-frontend-helpers.js |
test-channels-client-state-152.js |
test-channel-decrypt-m345.js |
test-channel-ux-followup.js |
test-channel-proposals.js |
test-channels-merge-1498-unit.js |
|---|---|---|---|---|---|---|---|
#153 0f293e20 |
215/0 | 707/0 | 42/0 | 24/0 | 30/0 | 32/0 | 12/0 |
#166 37264b07 |
215/0 | 707/0 | 55/0 | 24/0 | 30/0 | 32/0 | 12/0 |
#225 6d4b7760 |
215/0 | 707/0 | 63/0 | 24/0 | 31/0 | 32/0 | 12/0 |
All [T].
E2E
The server was a local Go server built from the #225 merged tree; the PRs change no Go code. It ran on e2e-fixture.db, prepared as in CI: freshen, the inline Kpa-clawbot#1486/Kpa-clawbot#1791 seed from deploy.yml, corescope-migrate, seed-2073, seed-199. Each tree's public/ was served on its own port, and every server was stopped by its port-owner pid.
| Tree | Suite | Result |
|---|---|---|
| #225 | test-channels-154-155-e2e.js |
7/1: only the desktop "opening clears" step fails, 5 of 5 reruns (F1) [T] |
| #225 | test-channels-client-state-152-e2e.js |
6/0 [T] |
| #225 | test-channels-client-state-152-decrypt-e2e.js (own server + ingestor) |
18/0 [T] |
| #225 | test-channel-proposals-e2e.js (own server + ingestor, incl. the #196 auto-approve steps) |
23/0 [T] |
| #225 | test-channel-decrypt-e2e.js, test-channel-issue-1087-e2e.js, test-channel-issue-1111-e2e.js |
15/0, 3/0, 2/0 [T] |
| #225 | test-channels-{list-render,selection-flow,add-modal,ws-batch,share-color,observed-path-hash-size}-e2e.js, test-channels-ws-race-1498-e2e.js |
8/0, 8/0, 8/0, 6/0, 6/0, 7/0, 5/0 [T] |
| #225 | test-channel-{modal,fluid,qr,color-picker}-e2e.js |
15/0, 15/0, 11/0, 10/0 [T] |
| #225 | test-e2e-playwright.js |
stops (fail-fast) at a non-channel test, Version info lives on Perf dashboard (navStats wait), and stops the same way on master. A scratch copy without fail-fast gave 130 passed / 1 failed / 8 skipped on #225 and exactly the same on master: the same single failure, a local environment issue. [T] CI: 132/135. [K] |
| #153 | 152-e2e, 1111, selection-flow, channel-decrypt-e2e |
6/0, 2/0, 8/0, 15/0 [T] |
| #166 | 152-e2e, 1111, selection-flow, channel-decrypt-e2e |
6/0, 2/0, 8/0, 15/0 [T] |
| #166 | #225's test-channels-154-155-e2e.js (before the fix) |
4/4: both #154 steps and the mobile #155 badge step fail as expected, plus F1's desktop step [T] |
Own browser probes (#225 merged tree vs master)
| Probe | #225 | master |
|---|---|---|
| SJC held 2.5 s, then SFO only | SFO list stays; 3 requests for 3 clicks | list replaced by the SJC set |
| PSK conversation open, then region change, then show-encrypted on (desktop) | URL, header ("Review Probe — 0 messages"), selection and the labelled PSK row all kept | conversation closed: "Select a channel", URL #/channels, PSK row gone |
| Unread 3 on the PSK row, then region change, then show-encrypted off | badge stays 3 | not reached; the row was already gone |
All [T]. The show-encrypted toggle was driven the way the customizer does it: the localStorage key plus mc-channels-show-encrypted-changed.
Mutants
Each mutant was applied to a copy of the #225 merged tree and run against test-channels-client-state-152.js, plus the file listed for it.
| # | Issue | Mutant | Result |
|---|---|---|---|
| Mu152a | #152 | no mergeClientChannelState() in the load |
killed (56/7) |
| Mu152b | #152 | unread not carried |
killed (58/5) |
| Mu152c | #152 | reconcile before the merges | killed (51/12) |
| Mu152d | #152 | no mergeUserChannels() in the load |
killed (48/15), incl. the region, show-encrypted and onApproved trigger tests |
| Mu163a | #163 | no stale check in the stored-key loop | killed (60/3) |
| Mu163b | #163 | raw channel part in the cache key | killed (60/3); test-channel-decrypt-m345.js stays 24/0 |
| Mu163c | #163 | no init-time ensureCacheMigrated() |
killed (61/2) |
| Mu163d | #163 | clearChannelCache with the raw prefix |
killed (62/1) |
| Mu154a | #154 | no stale check after the api() await |
killed (60/3), and in the browser race probe (list replaced by SJC) |
| Mu154b | #154 | dropped call resolves at once | killed (62/1, deep-link test) |
| Mu154c | #154 | WS sequence sample replaced by 0 | killed (61/2, clock-skew tests) |
| Mu155a | #155 | no badge on mobile rows | killed (61/2); test-channel-ux-followup.js stays 31/0 |
| Mu155b | #155 | title dropped from the badge |
killed (61/2, incl. the desktop guard) |
| Mu155c | #155 | opening a channel does not clear unread | killed (62/1) |
All [T].
Security, performance and rules (all three)
-
scripts/check-xss-sinks.sh --diff. Exit 0 for each head:- fix(channels): keep client-only state (PSK selection, My Channels, unread) across a channel-list refresh #153 against master;
- fix(channels): leftovers from #153 (#163) #166 against master and against fix(channels): keep client-only state (PSK selection, My Channels, unread) across a channel-list refresh #153;
- fix(channels): drop stale channel-list responses and show the unread badge on mobile (#154, #155) #225 against master and against fix(channels): leftovers from #153 (#163) #166.
Each run used a local
--sharedclone checked out detached at the head. [T] -
Performance.
mergeClientChannelStateis a single pass with aMapkeyed by hash: 300 / 1000 / 3000 channels took 0.09 / 0.26 / 0.92 ms per merge (node, 100 iterations). [T]mergeUserChannels()is O(stored keys × channels) and now runs on every load; it is bounded by the user's handful of PSKs. [A]- The fix(channels): overlapping loadChannels() — an older response can overwrite the newer region's list #154 check is one integer comparison. The fix(channels): mobile channel rows don't show the unread badge #155 badge is O(1) per row inside the existing
renderChannelList()pass; that pass is the existing full-listinnerHTMLpattern, not a new rebuild. [A] - No per-item API calls were added. [A]
-
Fork guards.
deploy.yml9 andrelease-fast-path.yml1 at all three heads and on the fix(channels): drop stale channel-list responses and show the unread badge on mobile (#154, #155) #225 merged tree. Thedeploy.ymldiff against master is three E2E lines plus one comment, all inside the existing Playwright step. [T] -
Commit history.
- No closing keywords in the 3 titles, 3 bodies or any commit message. [T]
- All 29 commits not on master have author and committer
dborup <kontakt@meshview.dk>. [T] - History was only merged, never rebased:
- the old fix(channels): keep client-only state (PSK selection, My Channels, unread) across a channel-list refresh #153 head
dde4fc98and0f88865bare ancestors of3ab81130; 403075e8and3ab81130are ancestors ofc6c09d75;c6c09d75is an ancestor of7fc8a7a9. [T]
- the old fix(channels): keep client-only state (PSK selection, My Channels, unread) across a channel-list refresh #153 head
Not verified
- Real devices and touch input; Playwright viewports only.
- The approval trigger in a browser with a PSK open. It is covered by the unit test (
shared-channel approval (onApproved) keeps the open PSK conversation, killed by Mu152d), and the proposals E2E (23/0) covers approval itself. - Linux font metrics for F1; CI is green there. [K]
- Browsers other than Chromium.
The heads were 3ab81130…, c6c09d75… and 7fc8a7a9… both before and after this review (git ls-remote). All three PRs are still drafts and were not modified.
Generated by Claude Code
…iew F1) page.click() on a channel row aims at the row's centre. Where a desktop My Channels row wraps (font metrics; the unread badge makes the row wider) that centre is the Share button or the colour dot, so the "opening clears the badge" step opened the share dialog or the colour picker instead of the channel (5/5 on macOS Chromium at 1280x800). Both row clicks now go through openChannel(), which clicks .ch-item-name / .ch-row-name and waits for the channel URL. Test only; production code unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Report — CS-pve-agent1 PR#225 runde 2 — head 33e8d1aStatus: F1 fixed in the test only, master merged without conflicts, the fixed E2E passed 5 of 5 runs, local suites are green, CI: green (run 37205823872). Still a draft. Review feedback addressed (commit Evidence tags: [T] = test run locally, [A] = analysis of code or diff, [K] = CI result. F1: E2E click target (should-fix, test only)Cause. I reproduced it on Linux by enlarging
Fix (
F2 and F3
Master merge
Tests on
|
| Suite | Result |
|---|---|
test-channels-154-155-e2e.js |
8/0 in 5 of 5 runs [T] |
sh test-all.sh |
215 passed, 0 failed [T] |
node test-frontend-helpers.js |
707/0 [T] |
test-channels-client-state-152.js |
63/0 [T] |
test-channels-client-state-152-e2e.js, -decrypt-e2e.js |
6/0, 18/0 [T] |
test-channel-proposals-e2e.js |
23/0 [T] |
test-e2e-playwright.js |
132/135, 3 skips (same as CI) [T] |
The other channel E2E suites: test-channel-{decrypt,fluid,issue-1087,issue-1111,modal,qr,color-picker}-e2e.js, test-channels-{add-modal,list-render,observed-path-hash-size,selection-flow,share-color,ws-batch,ws-race-1498}-e2e.js, test-issue-1224-…, test-issue-1367-… |
all 0 failed [T] |
scripts/check-xss-sinks.sh --diff origin/master |
clean [T] |
CI (run 37205823872, head 33e8d1ad, conclusion: success)
| Job | Result |
|---|---|
| ✅ Go Build & Test | success. test-all.sh: 215 passed, 0 failed; test-channels-client-state-152.js 63/0; test-channel-ux-followup.js 31/0; the preflight XSS --diff step passed [K] |
| 🎭 Playwright E2E Tests | success. test-channels-154-155-e2e.js 8/0, with all 8 steps ✓ including desktop "opening the PSK channel clears the badge"; test-e2e-playwright.js 132/135 (3 skips); test-channels-client-state-152-e2e.js 6/0; test-channel-proposals-e2e.js 23/0; test-channels-client-state-152-decrypt-e2e.js 18/0 [K] |
| 🏗️ Build & Publish Docker Image | success [K] |
| 📦 Release Artifacts, 🚀 Deploy Staging, 📝 Publish Badges & Summary | skipped (fork guards / PR event) [K] |
Remaining items
- F2 and F3, as above.
- Only Linux Chromium was run here. The macOS case was reproduced by forcing the wrap, not on a Mac. [A]
- Real devices and touch input were not tested.
🤖 Generated with Claude Code
Relates to #154, #155
Base. #153 and #166 are now merged into master, and master is merged into this branch. The diff against master is only this PR's own changes:
3533318c: the tests;7fc8a7a9: the fix;33e8d1ad: review F1, an E2E click target (test only).#154: overlapping
loadChannels()callsProblem. Every caller (region change, the show-encrypted toggle, shared-channel approval,
init()) awaitedapi('/channels' + qs)and then wrotechannelsunconditionally. The response that arrived last won, not the request that was made last. A delayed SJC response could replace the SJC+SFO list while the pills still showed SJC+SFO.Fix. The fix follows the
messageRequestId/isStaleMessageRequest()pattern:loadChannels()numbers its requests (channelsRequestId).reconcileSelectionAfterChannelRefresh()and no "Failed to load channels".latestChannelsLoad). A caller's follow-up therefore runs against the list that is actually rendered. Examples areinit()'s deep linkselectChannel(routeParam)and the region handler's message refresh. Without this, a deep link whose first load is superseded would open before the newer list exists. It would get the header "Channel x — 0 messages", and the unit test pins this.wsActivitySeqwhen it starts, somergeClientChannelState()runs with the sample of the request that renders./channelsrequests, and the callers are unchanged.#155: unread badge on mobile rows
Problem.
renderChannelRowMobile()(Kpa-clawbot#1367) never renderedch.unread.Fix.
renderUnreadBadge(ch), which the desktop row and the mobile row share. The desktop output is byte-identical; a test pins it..ch-row-line1after the time. It keeps the99+cap,titleandaria-label, and uses the existing.ch-unread-badgeclass. No CSS was changed and no new colours were added. The count and the hash are escaped.renderChannelList()both when live decryption bumps the unread count and when a channel is opened (which resets unread to 0). So the badge updates and clears the same way as on desktop. No separate DOM patch path is needed.test-channel-ux-followup.jsevaluatesrenderChannelRow()alone in a sandbox. It now also loads the realrenderUnreadBadge(), the same way it already loadschannelDisplayName().Tests
Unit tests are in
test-channels-client-state-152.js. They load the realchannels.jsin the existing vm harness, and each/channelsrequest gets its own deferred response.#154: an older region response that lands last…(unit),#154 a late SJC response…(E2E desktop + 390 px, region=SJC held 2.5 s)#154: a dropped older response does not reconcile…#154: a dropped older failure…catch#154: a dropped call resolves only after the newest list rendered…#154: the #152 client-state merge still applies…(guard)99+,title,aria-label, none at 0#155: a mobile row shows the unread badge…(unit), E2E 390 px#155: on mobile a live-decrypted message…(unit, real WS handler + real decrypt); E2E injects live encrypted frames into the page's WebSocket (page.routeWebSocket) at 390 px and 1280 px#155: the desktop badge markup is unchanged(guard), E2E desktop controlpage.click()on a row aims at its centre, and where a desktop My Channels row wraps that centre can be the Share button or the colour dot.sh test-all.shwith the fix(channels): leftovers from #153 (#163) #166 version oftest-channels-client-state-152.js) and is killed by the new tests. M154-1 and M155-1 were also run through the E2E: 2 and 1 steps red.Perf
Not a hot path.
loadChannels()adds one integer comparison per response. The badge is O(1) per row inside the existingrenderChannelList()pass, so there is no new DOM rebuild. Nothing changes in the merge or the reconcile.Not verified
refreshMessages/selectChannel) still runs once per region change, as before, so two quick changes still refresh the messages twice for the current region.🤖 Generated with Claude Code