fix(channels): refresh the suggesting tab when its channel is auto-approved (#232)

[dborup-agent]

Relates to #232

Problem

With channelProposals.autoApprove on (#196), a suggested channel is approved at once. The tab that suggested it only updated its status line ("… is already shared with everyone.") and did not list the channel until the next list refresh. onApproved, which runs loadChannels(true), was called only from the admin decision path, not from the suggest poller.

Fix

public/channel-proposals.js:

• The suggest poller now calls the same onApproved path when a request ends approved.
• Both callers go through a new notifyApproved(st, fallbackName), which runs onApproved once per approval. The key is proposal.id + '@' + proposal.reviewedAt:
  • the admin decision and the suggest poller reporting the same approval in one tab refresh once;
  • a proposal that is revoked, re-proposed and approved again keeps its id but gets a new reviewedAt, so it refreshes again.
• The set of seen approvals is per mount() and capped at 64 entries (cleared when full).

public/channels.js: comment only. onApproved itself is unchanged. Since #153, loadChannels() keeps client-only state across the refresh, so an open PSK conversation and unread counts survive.

A suggestion of a name that is already approved also ends approved. The tab then refreshes once, which lists the channel if this tab had not seen it yet.

Performance

The fix adds one Set lookup per final poll result and at most one /api/channels request per approval. That is the same request the admin path already makes, and nothing runs in a render, ingest or WS hot path.

Tests

Tests were committed first (test(...) commit), red on master, then the fix.

Test	What it checks
test-channel-proposals.js (+4)	the poller's approved path calls onApproved once with the name; pending/rejected/error do not; admin decision + poller reporting the same approval → one call, a different approval → another call; re-approval after a revoke (same id, new reviewedAt) → another call
test-channels-client-state-152.js (+1)	the real channels.js and the real ChannelProposals.mount() (new harness option realProposals). An auto-approved suggestion lists the channel with exactly one /channels request and keeps the open PSK conversation (selection, URL, My Channels) and the unread counts on a server row and on the user:* row
test-channel-proposals-e2e.js (+1 step, 1 extended)	real ingestor + server with autoApprove. Page A opens a PSK conversation, then suggests a channel. On the same page (no reload) the channel is listed as shared, the PSK conversation stays open, exactly one /api/channels request is made, and the list refreshes exactly once

The E2E counts refreshes as well as requests because api() coalesces identical in-flight requests. A doubled onApproved would still show as only one network request.

Mutants (each killed):

• poller does not call onApproved: unit, integration and E2E fail;
• onApproved called twice: unit, integration and E2E fail;
• no dedupe: unit fails;
• key ignores reviewedAt: unit fails;
• any final status calls onApproved: unit fails.

Local runs: sh test-all.sh (217 files), node test-frontend-helpers.js, test-channel-proposals-e2e.js and test-channels-client-state-152-decrypt-e2e.js (self-started stacks), and 19 channel E2Es against a local Go server on the CI-migrated e2e-fixture.db. All green. scripts/check-xss-sinks.sh --diff origin/master is clean, and no CSS or colors were changed.

No merge, deploy or configuration change is included.

🤖 Generated with Claude Code

[dborup-agent]

Rapport — CS-pve-agent1 PR#238 #232 — head 8f41334

Status: Ready for review as a draft. All acceptance criteria are covered by red-then-green tests, every mutant is killed, and CI passes (E2E green on attempt 2 after an unrelated timing flake in the #180 packets test).

Evidence legend: [T] = test run (local or CI), [A] = analysis of code/logs, [K] = known from issue/prior PRs.

Acceptance criteria

#	Criterion	Test	Mutant(s) killed	Evidence
1	With auto-approve on, the suggested channel appears in the suggesting tab's list without a reload	test-channel-proposals-e2e.js "autoApprove shares a new browser suggestion…" (same page, no reload, [data-shared="true"] row); test-channels-client-state-152.js "#232: an auto-approved suggestion lists the channel…"; test-channel-proposals.js "suggest poller: an auto-approved suggestion calls onApproved exactly once"	M1 poller does not call onApproved (unit, integration and E2E all fail)	[T] red on master, green on head; CI 24/24
2	An open PSK conversation and unread counts survive that refresh	integration test: selection, URL, My Channels, unread 3 on a server row, unread 4 on the user:* row; E2E: PSK conversation added through the Add modal stays open (URL, header, selected row in My Channels)	M1 (the channel never appears, so the step fails before these checks)	[T]; [K] the merge itself is #153's
3	One /channels call per approval; no double refresh when the admin path and the poller both hit	unit "admin decision and suggest poller reporting the same approval refresh only once"; integration (exactly +1 request, still +1 after settling); E2E (1 network request and 1 refresh, counted through invalidateApiCache('/channels'))	M2 onApproved called twice (unit, integration and E2E fail); M3 no dedupe (unit)	[T]; [A] api() coalesces in-flight requests (public/app.js:157), so the E2E also counts refresh invocations, not only requests. M2 survived the request-only check, which is why the refresh counter was added
4	A re-approval after a revoke (same proposal id) still refreshes	unit "a proposal approved again after a revoke refreshes again"	M4 key ignores reviewedAt (unit)	[T]; [A] cmd/ingestor/channel_proposals.go resets a revoked row to pending with the same id
5	Pending, rejected and failed suggestions do not refresh	unit "suggest poller: pending, rejected and failed suggestions do not call onApproved"	M5 any final status calls onApproved (unit)	[T]
6	Unit test for the poller's approved path, and an E2E step that checks the same page	see rows 1–3	—	[T]

CI per job (run 37275433123)

Job	Attempt 1	Attempt 2
✅ Go Build & Test	pass (incl. test-all.sh 217/217, Preflight XSS --diff gate, eslint)	pass
🎭 Playwright E2E Tests	fail: test-issue-180-packets-url-modal-e2e.js, "reload shows 2 packets, Clear showed 3"; fail-fast stopped before the channel E2Es	pass (#180 12/12, test-channel-proposals-e2e.js 24/24, test-channels-client-state-152-decrypt-e2e.js 18/18)
🏗️ Build & Publish Docker Image	skipped	pass (local build only; GHCR login/push skipped by the fork guard)
📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges	skipped	skipped

Attempt 1 failure analysis [A]: the fixture was freshened at 07:24:39 and the #180 test ran at 07:38:05, about 13.5 min later. Packets were crossing the 15-minute default window that Clear restores between the Clear and the reload. That test and the packets page are not touched by this PR. Locally the same test passed 3/3 against this branch [T]. Only the failed job was rerun.

Local verification [T]

• sh test-all.sh: 217 files, 0 failed; node test-frontend-helpers.js: 707 passed.
• test-channel-proposals.js 36/36; test-channels-client-state-152.js 64/64.
• Self-starting E2Es on the CI-migrated e2e-fixture.db (freshen, seed SQL, corescope-migrate, seeds 2073/199): test-channel-proposals-e2e.js 24/24 and test-channels-client-state-152-decrypt-e2e.js 18/18.
• 19 channel E2Es against a local Go server on the same fixture, all green; the server was stopped by its pid file and the port was verified free.
• bash scripts/check-xss-sinks.sh --diff origin/master clean; git diff --check clean; no CSS or colors changed; fork guards unchanged (deploy.yml 9, release-fast-path.yml 1).

Residuals

• test-issue-180-packets-url-modal-e2e.js has a time-window flake (the fixture ages across the 15-minute default window during the CI run). It is out of scope here and worth its own issue.
• A suggestion of a name that is already approved also ends approved, so the suggesting tab refreshes once. That costs one request and lists the channel if this tab had not seen it. Intentional, and documented in the PR description.
• The per-tab set of seen approvals is capped at 64 and cleared when full. In the unlikely case of more than 64 approvals in one tab, a duplicate report could cause one extra refresh. It cannot cause a missed one.

[adminopenclaw8-sketch]

Review — CS-Minimax PR#238 autoapprove-refresh — head 8f41334

Dom: APPROVE med nits

Independent read-only review of the head above, rebased in scratch onto origin/master 3878d7ea (merge-tree 9354401e, clean). Evidence legend: [T] = test run by me, [A] = analysis of code/logs, [K] = known from the issue, earlier PRs or the author's report.

Findings

#	Severity	Finding	Evidence
F1	nit (test gap, pre-existing)	Nothing pins that unmount() cancels the suggest poller. My mutant M4 drops state.suggestPoller.cancel() from unmount() and survives both test-channel-proposals.js and test-channels-client-state-152.js. The code is correct today: channels.js destroy → ChannelProposals.unmount() → both pollers cancel(), which bumps generation, so a late response is dropped. The integration harness stubs unmount to a no-op (test-channels-client-state-152.js:261). Now that the poller's approved path triggers a list refresh, a one-line unit test (mount, suggest, unmount before the first tick, assert no status fetch and no onApproved) would be cheap.	[T] M4; [A] public/channel-proposals.js unmount(), public/channels.js:1988
F2	nit (documented)	With autoApprove off, suggesting a name that is already approved now causes one /api/channels refresh in that tab, where master made none. The ingestor returns the existing approved row (found && existing.Status != revoked → return existing), so the poller sees approved. This is the only behaviour change outside auto-approval. It is harmless (one request, the list gains the channel if this tab had missed it) and the PR description says so. I'm listing it because the brief asked for "no behaviour change with autoApprove off".	[A] cmd/ingestor/channel_proposals.go submit path; PR body
F3	nit / follow-up (pre-existing, not reproduced)	invalidateApiCache('/channels') clears only the TTL cache, not api()'s _inflight map. If a /channels request is already in flight when onApproved runs (a region change, the encrypted toggle, or a second approval within one round-trip), the refresh coalesces onto the older request and can render without the newly approved channel until the next refresh. This affected the admin path before this PR too. The PR adds a second trigger but does not widen the window much, because a status poll waits at least one poll delay (≥ 1 s) and an approval needs a status poll. Worth a follow-up issue (e.g. api(path, {bust:true}) bypassing _inflight, or an in-flight generation); not a blocker.	[A] public/app.js api() / invalidateApiCache()

No blocking findings.

Answers to the review brief

1. Core: the poller's approved calls the same onApproved path. Yes. The suggest poller's onUpdate calls notifyApproved(st, state.suggestName) when st.status === 'approved'. onAdminDecision now goes through the same notifyApproved, which calls the page's onApproved (invalidateApiCache('/channels') + loadChannels(true), unchanged) [A].

• The channel appears on the same page without a reload: test-channel-proposals-e2e.js "autoApprove shares a new browser suggestion…" waits for #chList .ch-item[data-hash=…][data-shared="true"] on page A [T].
• The open PSK conversation and the unread counts survive. The E2E checks the URL, the header and the selected row in My Channels. The integration test checks selection, URL, My Channels, and unread on a server row and on the user:* row, through the real channels.js and the real mount() [T]. The survival itself is fix(channels): keep client-only state (PSK selection, My Channels, unread) across a channel-list refresh #153's mergeClientChannelState() / mergeUserChannels() order in loadChannelsFor(), which this PR does not touch [A][K].

2. Exactly one /channels call per approval.

• Manual admin approval in the same tab. The admin poller reports approved → one refresh. The suggest poller for a non-auto suggestion stops at pending (it only reschedules on queued), so it cannot report that later admin approval [A]. If the same tab later suggests the same, already-approved name, the dedupe key (id@reviewedAt) matches and there is no second refresh. The unit test "admin decision and suggest poller reporting the same approval refresh only once" covers this, and M9 (no dedupe) kills it [T].
• Several quick approvals. Each distinct approval refreshes once; the unit test's "a different approval still refreshes" covers this [T]. Two admin decisions in quick succession: adminPoller.start() cancels the previous poll, so only the last one reports. The /channels it then fetches comes after both commits and lists both. That is unchanged from master [A]. The F3 caveat applies to overlapping refreshes.
• channelsRequestId (fix(channels): drop stale channel-list responses and show the unread badge on mobile (#154, #155) #225). onApproved calls loadChannels(true), which takes ++channelsRequestId; an older in-flight load returns latestChannelsLoad without rendering. A refresh overlapping a region change therefore cannot render out of order [A]. Within a single approval the E2E counts both network requests (1) and refresh invocations (1, via a wrapped invalidateApiCache), so a doubled onApproved that api() would coalesce is still caught. My M2 (double call) fails exactly that assertion [T].
• Revoke → re-approve. The ingestor resurrects a revoked row with the same id as pending, even with autoApprove on, and a later approve sets a new reviewed_at. So the key changes and the tab refreshes again [A]. The unit test covers this, and M5 (key = name only) kills it [T].

3. Poller lifecycle. createPoller.tick() reschedules only on queued. approved, pending, rejected and a 404 end polling; only network errors back off and retry, capped at MAX_POLL_ATTEMPTS. My M3 (also reschedule on approved) fails the unit test's "exactly 2 status requests" assertion and the integration test [T]. Navigation: unmount() cancels both pollers, and onUpdate also returns early on !state. No timer leak by analysis [A]. The test gap is F1.

4. No behaviour change with autoApprove off or on rejection. pending, rejected and error never call onApproved: the unit test, plus my M6 (also on pending), killed [T]. With autoApprove off, a new suggestion ends pending and the page behaves exactly as on master. The one exception is an already-approved name (F2) [A].

5. Security / UI.

• No new HTML sinks. The suggest status still uses textContent, and the name passed to onApproved is not rendered (channels.js ignores the argument) [A].
• bash scripts/check-xss-sinks.sh --diff origin/master exits 0, and git diff --check is clean [T].
• No CSS or colour changes, and no per-item API calls. The new per-result work is one Set lookup; the set is per mount() and capped at 64 [A].

6. Rules.

• Fork guards: deploy.yml 9, release-fast-path.yml 1, and no workflow files are touched [T].
• No closing keywords in the PR body or the commit messages ("Relates to fix(channels): auto-approved channel does not appear in the suggesting tab until the next list refresh #232"), and closingIssuesReferences is empty [T].
• Both commits (76a2b6f6 test-first, 8f41334e fix) have author and committer dborup <kontakt@meshview.dk>, with a Co-Authored-By trailer [T].

CI

Run 37275433123 on this head [T]:

• Attempt 1: the Playwright job failed on test-issue-180-packets-url-modal-e2e.js alone ("reload shows 2 packets, Clear showed 3"), and fail-fast stopped the job before the channel E2Es.
• Attempt 2: green, with test-channel-proposals-e2e.js 24/24 and test-channels-client-state-152-decrypt-e2e.js 18/18 in the log.
• That test and the packets page are not touched here, so I agree with the time-window-flake reading [A]; I did not reproduce it myself.
• Small correction to the report: attempt 2 has a new job id for "Go Build & Test" too, so it was rerun as well, not only the failed E2E job. Both passed, so this has no effect.

Tests run (merged tree, Go server/ingestor/migrate built from it) [T]

Suite	Result
sh test-all.sh	217 files, 0 failed
node test-frontend-helpers.js	707 passed
test-channel-proposals-e2e.js (self-started server + ingestor, CI-prepared fixture)	24/24
test-channels-client-state-152-decrypt-e2e.js (self-started)	18/18
19 channel E2Es from deploy.yml against a local Go server on e2e-fixture.db (freshened, CI seed SQL, corescope-migrate, seeds 2073 and 199)	all green (2–15 steps each)

The local server was stopped by its pid file and the port was verified free afterwards.

Mutants (own) [T]

Mutant	Unit (36)	Integration (64)	Proposals E2E
M1 poller does not call notifyApproved	killed (3)	killed	killed (row never appears)
M2 onApproved called twice	killed (3)	killed	killed (refresh count 2)
M3 poller keeps polling after approved	killed (1)	killed	—
M4 unmount() does not cancel the suggest poller	survives	survives	— (F1)
M5 dedupe key = name only (ignores id/reviewedAt)	killed (1)	survives	—
M6 pending also refreshes	killed (1)	survives	—
M7 onApproved drops invalidateApiCache('/channels')	survives	survives	killed (TTL cache hides the row)
M8 admin path no longer calls notifyApproved	killed (1)	survives	—
M9 no dedupe check	killed (2)	survives	—

M7 shows that the E2E is what guards the cache invalidation; the unit and integration harnesses do not model api()'s TTL cache.

Not verified

• Manual browser session against a running instance (only the Playwright runs above).
• F3 was not reproduced; it is by code reading only.
• The attempt-1 fix(packets): leaving with the SlideOver open rewrites the new page's URL; Clear Filters on a detail URL re-filters on reload #180 flake was not reproduced locally.
• Go test suites were not re-run locally; this PR touches no Go code, and CI's Go job is green.
• No staging or production checks, by design.