Repository navigation
fix(channels): leftovers from #153 (#163) - #166
Merged
Merged
Conversation
Relates to #152. loadChannels() replaces the channel list with the server snapshot and carries nothing across. These tests drive the real loadChannels(), the real WS path and the real init() handlers (region change, show-encrypted toggle, shared-channel approval) through the existing test hooks, and a Playwright check adds a PSK through the Add Channel modal and switches region at desktop and mobile width. On master, 11 of 15 unit tests and 4 of 6 E2E steps fail with the reported symptoms: the PSK selection is cleared and the URL rewritten to #/channels, My Channels and labels vanish, unread and the user:* preview reset, and a WS update that lands during an in-flight request is lost. The four passing unit tests are guards for the fix (omitted rows are not resurrected, a stale client row does not beat a newer snapshot). Registered in the existing unit and Playwright steps of deploy.yml and in test-all.sh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxHDRtHRCQxS6AzzDXXSJi
Relates to #152. loadChannels() replaced the channel list with the server snapshot and carried nothing across, so a region change, the show-encrypted toggle or a shared-channel approval closed an open PSK conversation, dropped My Channels and reset unread badges and user:* previews. loadChannels() now runs: fresh snapshot (copied, so the api() cache is not mutated) -> mergeApprovedChannels -> mergeUserChannels -> mergeClientChannelState -> sort -> render -> reconcile. The new pure helper carries unread (explicit 0 included), userAdded and userLabel by hash for server rows and user:* rows alike, and never re-creates a row the fresh list lacks, so the region filter keeps working. A label just re-read from storage wins over the old one. Live activity (lastActivityMs, lastSender, lastMessage, messageCount, moved together) is carried only when the WS path stamped the row with a local sequence number newer than the one loadChannels() sampled when its request started. No browser/server time comparison, so clock skew cannot decide it. user:* rows exist only in the tab and always keep theirs. onApproved now just calls loadChannels(): its trailing mergeUserChannels()/render ran after the reconcile had already closed the PSK conversation, and is redundant now. init() keeps its own mergeUserChannels()/render: a no-op after a successful load, but the only thing that lists My Channels when /channels fails (covered by a test). Test follow-ups: wait for the decrypt pass before the E2E pane snapshot, a cross-realm array assertion, and tests for init()'s failure path and for a stored label winning over the previous list's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxHDRtHRCQxS6AzzDXXSJi
A region change while a PSK (user:* or key-matched) channel's decrypt
fetch is still in flight left the message pane stuck on "Decrypting
messages…": the in-flight decrypt's own staleness check discards its
result once the region changes under it, and the region handler's
refreshMessages({regionSwitch:true}) has no REST messages to refetch for
an encrypted row, so it just returned without ever restarting the
decrypt. The same gap could also leave a finished decrypt showing the
previous region's messages.
regionChangeHandler now re-runs selectChannel(selectedHash) for an
encrypted selection instead of refreshMessages() — it redoes the key
lookup and decrypt fetch for the new region without closing the
conversation (selectedHash/URL stay put).
Test: new unit test drives selectChannel() with a real AES-128-ECB +
HMAC-SHA256 encrypted packet (Node's crypto, matching the scheme
documented in channel-decrypt.js) and an in-flight /packets fetch,
switches region mid-flight, and asserts the pane ends up with the
new-region message, not stuck on "Decrypting…" and not showing the old
region's message. Confirmed red by reverting this fix (pane stays stuck
on "Decrypting…").
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
mergeClientChannelState() carried prev.userAdded/prev.userLabel forward from the in-memory previous channel list, and the remove-key handler's server-known-channel branch unmarked userAdded but never cleared userLabel. Removing a key from a server-known channel therefore left a stale userLabel on the in-memory row; the next refresh's merge carried it back onto the fresh row (mergeUserChannels() only (re)derives it for keys still in storage, so a removed key's label was never otherwise overwritten), making the removed label resurface indefinitely. mergeUserChannels() already re-derives both userAdded and userLabel from storage on every load, before mergeClientChannelState() runs — so the merge never needed to carry them from prev at all. Drop that carry-over and make storage the single source of truth; also clear userLabel in the remove handler itself so the UI doesn't show it for one extra render. Extracted the remove-button logic into removeUserChannelKey() so it's directly testable. Tests: a new regression test adds a key+label to a server-known channel, removes it through the real handler, and asserts the label and My Channels membership don't resurrect across two loadChannels() refreshes. Updated the existing mergeClientChannelState pure-helper test, whose assertions encoded the old (buggy) carry-over contract. Confirmed red against two separate mutants: reverting the merge-helper change alone (caught by the pure-helper test) and reverting the remove-handler's label clear alone (caught by the new regression test). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
mergeUserChannels() matches a stored PSK key against a server-known or newly-approved shared channel by name (not just by hash), annotating the existing row with userAdded instead of creating a separate user:* row. If that PSK's conversation was already open under its user:* hash, the hash simply disappears from the fresh channel list on the next refresh (a region change, or a shared-channel approval), even though the conversation is still live — reconcileSelectionAfterChannelRefresh() then treats the missing hash as "channel gone" and closes it: clears the selection, empties messages, and rewrites the URL to #/channels. reconcileSelectionAfterChannelRefresh() now checks, before closing a missing user:* selection, whether a userAdded row with the same PSK name exists in the fresh list; if so it remaps selectedHash to that row and updates the URL via replaceState, without touching messages. Tests: two new regression tests cover both triggers — a region switch that reveals a same-named server channel, and approving a shared channel with the same name as the open PSK — each asserting the selection remaps (not closes), the URL updates, and messages are left alone. Confirmed red by reverting the reconcile change (both tests fail: selection closes to null instead of remapping). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
…utant The mutant "drop _wsSeq from the object literal when processWSBatch() creates a brand-new channel row" survived the existing suite: every _wsSeq regression test exercised a row that already existed in channels (the ch._wsSeq = ++wsActivitySeq branch), never the push() branch for a channel seen for the first time over WS. Add a test: a live message for a channel with no existing row creates one via the WS path while a concurrent loadChannels() is in flight, then an older snapshot for that same channel resolves. Without the new row's _wsSeq stamp, mergeClientChannelState() has no way to tell the freshly live row is newer than the request, so the older snapshot would win. Confirmed red by removing the field from the push() literal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
…nnels-client-state
…region-switch race The existing #152 E2E (test-channels-client-state-152-e2e.js) uses a PSK key that matches no fixture packets, so it never exercises an in-flight client-side decrypt — it can't catch F1 (region switch while decryptAndRender()'s /api/packets fetch is still pending). Add a dedicated E2E test that seeds two genuinely client-decryptable GRP_TXT packets (real AES-128-ECB + HMAC-SHA256, matching the scheme in channel-decrypt.js) directly into a temp copy of the fixture DB, under a PSK key the server never sees — one observed only from an SJC observer, one only from SFO, so a region-scoped /api/packets?region=<R> fetch returns exactly one. It delays the first /packets response ~3s, narrows the region filter mid-decrypt, and asserts the pane ends up showing the new region's message (not stuck on "Decrypting…", not the stale one), with the conversation still open. Runs at desktop and mobile. Since the server refuses to start against an unmigrated DB and writes must not go through the read-only server connection, the test briefly runs the real ingestor (against an in-process fake MQTT broker, no real traffic) to apply schema migrations, stops it, seeds the packets, then starts the read-only server — consistent with the read/write separation invariant. Confirmed red against the pre-F1 code (10 passed, 4 failed — pane stuck on "Decrypting…" and/or showing it instead of the new-region message) and green against the current (fixed) code (14 passed, 0 failed), both independently re-run. Registered in .github/workflows/deploy.yml next to test-channel-proposals-e2e.js (same own-server-and-ingestor pattern). Not added to test-all.sh, which only runs unit tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
A region switch, the show-encrypted toggle, or a shared-channel approval can all trigger loadChannels() -> reconcileSelectionAfterChannelRefresh(), which remaps an open user:* PSK conversation to a same-named row mergeUserChannels() just matched by name (#152 F3, round 2). If a decrypt for the old user:* hash was still in flight at that exact moment, the remap changes selectedHash out from under it: the in-flight fetch's own staleness check (isStaleMessageRequest) discards its result once it resolves, and nothing else restarts the fetch for the remapped hash — the pane is left stuck on "Decrypting messages…" forever. Track whether a decrypt is pending for the current selection (messageLoadPending, set/cleared around decryptAndRender()). When reconcile remaps a selection while that flag is set, it now also calls selectChannel(remapped.hash) to restart loading for the new hash. A quiet remap (no decrypt in flight) still leaves messages untouched, preserving the round-2 F3 invariant. Tests: a new unit test starts a real decrypt (in-flight /packets fetch) on a user:* hash, triggers a remap to a server-known row mid-flight, and asserts the remapped channel's own messages load (not stuck on "Decrypting…"). A second test guards the quiet-remap case. Confirmed red by reverting the reconcile change (pane stays stuck on "Decrypting…"). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
fetchAndDecryptChannel()'s cache key was the channel name alone, with no
region component, even though the fetched/decrypted message set depends
entirely on which observers the current region filter includes. This let
a cache entry primed under one region answer a fetch for a different
region:
- Zero candidates for the current region fell through to `return {
messages: cachedMsgs, empty: true }`, showing a foreign region's
messages instead of an empty list (e.g. All->OAK showed the SJC+SFO
messages cached under "All"; SJC->MRY showed SJC's message).
- The delta path's "0 new candidates since lastTs" short-circuit
(`newCandidates.length === 0`) trusted the cache's count/lastTs across
regions too: a different region with the same candidate count but an
older timestamp returned the old region's cached message outright.
- A stale (superseded) decrypt could still write its results into the
cache after being superseded, since the write sites had no visibility
into request staleness at all.
Fixes: the cache key now includes the region param (sorted, so selection
order doesn't fragment it); a zero-candidate result always renders as an
empty list, never leftover cached content; every cache write goes
through setCacheIfFresh(), which checks the caller-supplied isStale()
(backed by isStaleMessageRequest()) first. Also capped the number of
distinct cache entries (MAX_CACHE_KEYS, channel-decrypt.js) since
region-scoped keys mean one channel can now occupy several, evicting the
least-recently-written ones.
Tests: three new unit tests cover cross-region isolation (All->OAK),
the same-count delta-path pitfall (SJC->MRY), and that a superseded
decrypt's evidence never reaches the cache. A new E2E step in
test-channels-client-state-152-decrypt-e2e.js switches to a traffic-free
region (OAK) after the existing decrypt has fully settled and asserts an
empty pane, confirmed red against the pre-fix code in a real browser.
Confirmed red for the unit tests too, each against its own mutant
(dropping the region from the cache key; dropping the isStale guard in
setCacheIfFresh).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
…e E2E harness Two nits in test-channels-client-state-152-decrypt-e2e.js's process lifecycle helpers: - A thrown error in main()'s try block (e.g. the server process never comes up) left `failed` at 0, since no step() had run yet to increment it. The `finally` block's `if (failed) ... else fs.rmSync(dir, ...)` then deleted the log directory anyway, and the FATAL message from the outer catch pointed at log files that no longer existed. Track a `fatal` flag alongside `failed` so logs are kept for either, then re-throw unchanged so the outer catch's behavior is otherwise identical. - waitFor() polled in fixed 200ms steps for up to the full timeout (20s) even when the callback's own check already knew the watched process had exited — the ingestor/server health checks threw on a dead process, but waitFor just caught that and kept retrying instead of stopping. waitFor now takes an optional `proc` and checks its exitCode itself before each poll, failing immediately instead of waiting out the timeout. No behavior change for the success path; these only affect error reporting and how fast a genuine startup failure is surfaced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
…nnels-client-state
…che entry
N2 region-scoped the client-side decrypt cache ("<channel>|<regions>"),
but removeKey() -> clearChannelCache() still only deleted the bare
"<channel>" entry, so removing a PSK left its decrypted plaintext in
localStorage under every region it had been viewed in.
clearChannelCache(name) now deletes "<name>" (the pre-N2 key) and every
key starting with "<name>|", and leaves channels whose name merely shares
the prefix alone. The key format moves into ChannelDecrypt
(channelCacheKey), so the writer in channels.js and the clearer can't
drift apart again.
Tests: two unit tests (raw blob with legacy/All/SJC/SFO,SJC entries plus a
prefix-sharing channel; real decrypts in three regions removed through
the channel-list handler) and a new decrypt-E2E step asserting that no
localStorage value holds the channel's plaintext after Remove, at desktop
and mobile. All red on 9706722.
Relates to #152
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
… entries The client-side decrypt cache was capped at 50 entries only. One entry can hold ~365 KiB (1000 messages) against a ~5.2M-character localStorage quota shared with the stored keys and labels, so a full cache made storeKey() and saveLabel() fail silently. - The whole cache blob is kept under 1.5M characters; the least recently used entries (last write, or last read this page load) go first. Read stamps are folded in on the next write, so a read never rewrites the blob. - A QuotaExceededError from the cache write evicts the oldest entry and retries; an entry that can never fit drops the blob instead. setItem() stores the whole new blob or keeps the old one, so it is never half-written. - Keys and labels go through setItemMakingRoom(): on a quota failure they evict decrypt cache until the write fits. - Pre-N2 entries (keyed by channel name alone, no "|") are dropped once, marked by corescope_channel_cache_v=2. Each entry is serialised once and joined, so the budget costs no extra stringify. Measured in Node with 12 channels x 1000 messages written in turn: setCache 23.0 -> 14.1 ms, getCache 13.0 -> 5.9 ms on average, since the blob no longer grows past the budget. Tests (all red on 9706722): budget + eviction order, read keeps an entry alive, quota evict-and-retry with a never-fitting entry, storeKey/labels under a full quota, one-time legacy migration. Mutants killed: no eviction, reversed eviction, no quota retry, reads not counted, keys not making room, no migration, migration on every load. Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
N1's "a decrypt is pending" flag was one shared boolean, set only inside decryptAndRender(). That left three races: - S1: decrypt A, a region switch starts decrypt B, A finishes and clears the flag while B still runs; a remap then saw no pending load and left the pane on "Decrypting messages…". - S2: a remap while selectChannel() still awaited computeChannelHash() happened before the flag was set at all. - S3: a superseded selectChannel() resuming after computeChannelHash() still painted "Decrypting messages…" over the newer request's view (e.g. a REST refresh after a region switch revealed a same-named server channel). selectChannel() now sets messageLoadPending to its own request id before its first await and only the owning request clears it (the body moves into loadSelectedChannel(request, ...)). reconcileSelectionAfterChannel- Refresh() restarts loading when the flag equals the current messageRequestId, and decryptAndRender() returns early for a stale request instead of writing "Decrypting messages…". Tests S1, S2 and S3 are red on 9706722. Mutants killed: a global flag cleared by any request (S1), the flag set after the first await (S2), no stale check before "Decrypting…" (S3), no restart on remap (N1, S1, S2). Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
…xed flushes S1's "decrypt B is in flight" precondition counted /packets fetches after a fixed flush(300). Those fetches only start once computeChannelHash()'s Web Crypto digest resolves, which runs off the event loop and can outlast the flushes on a busy machine — seen failing once in about eight runs while mutation-testing R4-4. A settle(cond) helper now flushes until the condition holds or ~2s of real time pass, and S1 uses it for its in-flight preconditions and for the remapped load. Production code is unchanged. Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
… entry When the current region's decrypt fetch finds zero candidates, N2 renders an empty pane but left that region's cache entry in place, so the next visit flashed the outdated history (onCacheHit) before its own fetch answered. fetchAndDecryptChannel() now deletes the "<channel>|<regions>" entry in that case, unless the request is stale by then (a newer request owns the cache). ChannelDecrypt gains deleteCache(key) for removing one entry. Test (red on 9706722): a stale zero-candidate answer leaves the entry, a current one drops it, and the next visit shows nothing outdated while its fetch is in flight. Mutants killed: zero candidates falling back to the cache, no delete, deleting even when stale. Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
…ed region key - The decrypt E2E's waitFor() and stopProcess() only checked exitCode. A child killed by a signal (SIGKILL, SIGSEGV) has exitCode null and signalCode set, so waitFor() polled out its whole timeout and stopProcess() waited forever for an 'exit' that had already fired. Both now use hasExited() (exitCode or signalCode), and the early-exit error names the signal. - The E2E file requires node:sqlite and playwright where they are used, and exports its process helpers, so the unit suite can test them. - Unit tests: waitFor() rejects at once for SIGKILL and SIGSEGV, and stopProcess() returns at once for a signal-killed child (both red on 9706722); SJC,SFO and SFO,SJC share one decrypt cache entry (pins existing behaviour — green on 9706722, red on the unsorted mutant). Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
…channelCacheKey R4-1 moved the decrypt cache key format into ChannelDecrypt (channelCacheKey), which fetchAndDecryptChannel() now calls. The two delta-cache tests in test-channels-observed-path-hash-size.js replace ChannelDecrypt with a partial stub, so they failed with "ChannelDecrypt.channelCacheKey is not a function" (11/11 -> 9/11). The stub now provides it; the tests are unchanged otherwise and pass 11/11. Relates to #152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr
… view (#163) selectChannel() walks the stored keys of an encrypted channel, awaiting computeChannelHash once per key, then paints the Kpa-clawbot#781 lock text. It never checked isStaleMessageRequest(), so a channel or region change during the loop let the lock text land on the newer request's pane. The deep-link lookup of '#'-named channels had the same gap: when its /channels request failed, the catch fell through to "Loading messages…" regardless of who owned the pane. Check isStaleMessageRequest() after each await in the key loop (which also stops hashing keys for a dead request) and before the fall-through write. The /channels lookup branch and decryptAndRender() already checked. Tests: four new cases in test-channels-client-state-152.js, all red on dde4fc9 and on master 727efca.
…ng '|' (#163) Cache keys are "<channel>|<regions>" and clearChannelCache() removed every key starting with "<channel>|", so a channel "#a" also cleared "#a|b"'s entries, and their keys could be confused. The channel part now has '%' and '|' percent-encoded (names without either keep their plain key), so the first '|' of a key is always the separator and the encoding is injective. clearChannelCache() matches that exact prefix and no longer special-cases a bare-name key. Migration: CACHE_VERSION is 3. A blob written before it (pre-#153 bare-name keys or #153's unescaped keys, which can't be told apart for a name with '|') is dropped whole on first use. It is only a cache, and no old plaintext remains. Tests: new #163 item 2 cases (clear "#a" keeps "#a|b"; removeKey; the encoding is injective; both old formats are gone, with and without a version marker), all red on dde4fc9. Three existing tests change with the behaviour: the R4-2 pre-N2 migration test (a region-scoped entry of the old format no longer survives), R4-1 (the legacy bare-name entry is the migration's job now) and two cases in test-channel-decrypt-m345.js that cached under a bare name instead of channelCacheKey().
Every local build of the ingestor binary into the repo root left an untracked file next to the already-ignored corescope-server. Proof: after (cd cmd/ingestor && go build -o ../../corescope-ingestor .) git status shows nothing for the binary. There is no go.mod at the root, so the issue's go build -o corescope-ingestor ./cmd/ingestor does not run from the root; this is the equivalent that writes the same file.
Both wrote and read bare "<channel>" keys, outside the "<channel>|<regions>" format channelCacheKey() produces, and no production code called them. A future caller goes through channelCacheKey() + setCache()/getCache(). Tests: the decrypt E2E's roundtrip step now exercises setCache()/getCache() under channelCacheKey() (same roundtrip coverage, plus a check that nothing lands under the bare name). A unit test asserts the old exports are gone (red on dde4fc9).
) ensureCacheMigrated() ran on the first cache read or write, so old-format decrypted plaintext stayed in localStorage until the channels page used the cache — even for a channel whose key was removed before the upgrade. Call it once when the module initialises. The calls in the cache functions stay as the fallback for a localStorage that isn't available yet; a failed getItem/removeItem no longer marks the migration as done, so that fallback can still run. Tests: loading the module alone (no cache call) drops old-format entries (version marker absent and "2"), red on dde4fc9; a current-format cache survives the load; the lazy fallback runs when localStorage appears after load.
…ests Both Kpa-clawbot#1468 tests snapshotted window._channelsGetStateForTest() as soon as the WS test hook existed. channels.js sets that hook synchronously in init(), before loadChannels() has an /api/channels answer, so `before` could read 0 channels. When the response then landed between the two evaluate() calls, `after` read the full list and the orphan test failed with before=0, after=5 although the orphan message was dropped (CI run 37002943213). With a delay on /api/channels the same race fails on #153's head too, and it also hits the control test: a WS-born channel pushed while the request is in flight is not in the snapshot, and mergeClientChannelState() only enriches rows the snapshot has. waitForChannelListSettled() waits until #chList has rendered the list (no .ch-loading), the count is > 0 and unchanged for 300 ms; both tests take their `before` snapshot only after that. Test-only change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
|
Review feedback addressed (commit CI run 37002943213 failed in Playwright E2E on
🤖 Generated with Claude Code |
This was referenced Oct 3, 2026
Conflicts: - .github/workflows/deploy.yml: master replaced the inline JS unit list with `sh test-all.sh` (#174); take master's step. The #152 unit file now runs through test-all.sh. - test-all.sh: take master's `run` form and keep test-channels-client-state-152.js after test-channels-observed-path-hash-size.js. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Report — CS-pve-agent1 PR#166 master-merge — head c6c09d7Status: the updated #153 branch is merged (it includes master), with no conflicts; local unit and E2E green; CI: green (run 37198612690). Merged master (commit Evidence tags: [T] = test run locally, [A] = analysis of code or diff, [K] = CI result. Conflicts and how they were resolved
Local tests on
|
| Suite | Result |
|---|---|
sh test-all.sh |
215 passed, 0 failed [T] |
node test-frontend-helpers.js |
707/0 [T] |
test-channels-client-state-152.js (with the #163 tests) |
55/0. Also 15 runs on an idle machine: 0 failures [T] |
test-channel-decrypt-m345.js, test-channel-decrypt-e2e.js |
24/0, 15/0 [T] |
test-channels-client-state-152-e2e.js |
6/0 [T] |
test-channels-client-state-152-decrypt-e2e.js |
18/0 [T] |
test-channel-proposals-e2e.js |
23/0 [T] |
test-e2e-playwright.js (both Kpa-clawbot#1468 tests included) |
132/135, 3 skips (same as CI) [T] |
| The other channel E2E suites (same list as on #153) | all 0 failed [T] |
Known red baseline: none found in these suites.
CI (run 37198612690, head c6c09d75, conclusion: success)
| Job | Result |
|---|---|
| ✅ Go Build & Test | success. test-all.sh: 215 passed, 0 failed; test-channels-client-state-152.js 55/0; the preflight XSS --diff step passed [K] |
| 🎭 Playwright E2E Tests | success. test-e2e-playwright.js 132/135 (3 skips; both Kpa-clawbot#1468 tests ✅); 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] |
🤖 Generated with Claude Code
Collaborator
Review — CS-Minimax PR#166 kanalpakke — head c6c09d7Dom: APPROVE Independent, read-only review of head Evidence tags: [T] run here, [A] analysis of the source, [K] taken from the author's report or CI, not re-run. Findings
No defects found. Merge into the stack
#163 items
Tests on this merged tree
Security and rules
Not verifiedI did not repeat the browser reproduction of the item 1 lock-text race. The author reports 6/6 on The head was Generated by Claude Code |
dborup
marked this pull request as ready for review
October 4, 2026 13:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #153 — stacked on dde4fc9; review only the commits after dde4fc9.
Relates to #163.
What this changes
Five small leftovers from the round-4 review of #153, one commit each. Every new test was written first and is red on
dde4fc98; item 3 has no test.e635b28aselectChannel()checksisStaleMessageRequest()after eachcomputeChannelHashawait in the stored-key loop, and before the fall-through "Loading messages…" write. A superseded request can no longer paint the "no decryption key" lock text or "Loading messages…" over a newer view, and stops hashing keys.29d47198%and `728fa982.gitignore: addcorescope-ingestornext tocorescope-server.9fdfb857cacheMessages/getCachedMessagesand their exports.40bd9d3eensureCacheMigrated()once whenChannelDecryptinitialises; the lazy calls stay as a fallback. A failed storage access no longer marks the migration done, so the fallback can still run.Decisions worth a look
encodeURIComponent. Only%and|are encoded, so every existing key without those characters is unchanged and the existing key-literal tests and E2E checks keep working.encodeURIComponenton the whole name would also rewrite:and#.|, so a selective migration could leave plaintext behind. Cost: one re-decrypt per channel after the upgrade.test-channel-decrypt-m345.jscached under a bare name instead ofchannelCacheKey();cacheMessagesroundtrip step became asetCache/getCacheroundtrip underchannelCacheKey(), plus a check that nothing lands under the bare name.go build -o corescope-ingestor ./cmd/ingestordoes not run from the repo root (there is no rootgo.mod). The equivalentcd cmd/ingestor && go build -o ../../corescope-ingestor .leavesgit statusclean with this change.Tests
node test-channels-client-state-152.js: 55 passed, 0 failed. Of the 14 new cases, 13 are red ondde4fc98(the 14th, "a current-format cache survives the load", guards against over-deleting). The four item 1 cases are also red on master727efca0.test-channel*-unitfiles andtest-frontend-helpers.js: same results as master. The known failures are identical there:channel-colors(2),psk-ux(1),ux-followup(1),ux-round2(1), andfavStar(2) infrontend-helpers.152-e2e,152-decrypt-e2e,channel-decrypt-e2e,1111,ws-race-1498,selection-flow,proposals-e2eall pass.channelCacheKey(three item 2 tests), a raw prefix inclearChannelCache(the#a/#a|btest), not dropping the blob on migration, no init-time call (both item 5 load tests), and no storage-failure reset (the lazy-fallback test).scripts/check-xss-sinks.sh --diff dde4fc98andscripts/check-css-vars.js: clean.computeChannelHash, then another channel.dde4fc98: the lock text overwrote the newer view in 6/6 runs. This branch: 0/6.deploy.ymlis untouched; all 9github.repository == 'Kpa-clawbot/CoreScope'guards are still there. No new test files, so nothing to register intest-all.shordeploy.yml.Config / customizer
None. No new configurable values.
🤖 Generated with Claude Code