Skip to content

feat(home): My Mesh "Node page" link (upstream 2027 port) - #54

Merged
dborup merged 4 commits into
masterfrom
codex/port-upstream-2027-node-page-link
Sep 20, 2026
Merged

dborup merged 4 commits into
masterfrom
codex/port-upstream-2027-node-page-link

Conversation

@dborup

@dborup dborup commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • Ports the My Mesh "Node page →" button from upstream Kpa-clawbot/CoreScope#2027 (reviewed upstream head e5536d20fa8ed4e94b7d2abec481fa382dbf11c7). It appears on the normal card (before Full health / View packets) and on the health-error card (as its only action), and navigates to #/nodes/<encodeURIComponent(pubkey)>. .mnc-actions now wraps on narrow cards.
  • Local keyboard change, not part of upstream: the card's keydown handler called preventDefault() on Enter/Space before ignoring nested .mnc-btn / .mnc-remove targets, which suppressed the buttons' native keyboard activation. The handler now returns early for those targets, so Node page, Full health, View packets and Remove handle Enter/Space themselves without triggering the card's health action. Enter/Space on the card itself still opens health.
  • Registers the self-contained regression test once in the existing "Run Playwright E2E tests" step, directly after test-home-coverage-e2e.js (no BASE_URL, CHROMIUM_REQUIRE=1).

Commits: 027045a1 (feature, keyboard change, test) and eb19b286 (test hardening, CI registration).
Files: public/home.js, public/home.css, test-issue-2027-my-mesh-node-page-e2e.js, .github/workflows/deploy.yml (+1 line).

Correction to the first commit message

027045a1's message overstates backend behaviour. The accurate statement is: the button navigates to the correct route; what the node detail page shows depends on the backend's node lookup (GET /api/nodes/{pubkey}).

  • The mocked-404 test step is frontend evidence only (nodes.js renders its existing "Node not found" state for a 404). It does not show that all channel-only nodes are missing from the backend.
  • UpsertNode is not the only writer to the nodes table; it is the only INSERT path, alongside several UPDATE/DELETE writers.
  • In a local synthetic-backend diagnostic, a node moved to inactive_nodes still returned health 200 while node detail returned 404, so a normal card can also lead to "Node not found".

Regression test (24 scenarios)

test-issue-2027-my-mesh-node-page-e2e.js runs the real app.js, roles.js, home.js (and nodes.js for the 404 step) in headless Chromium with a mocked api():

  • Card contents for normal and error cards; mouse, Enter and Space navigation for both; URL-encoding of the pubkey; Node page never triggers the card's health handler; Enter/Space on the card still opens health; Full health / View packets via mouse and keyboard; Remove via mouse, Enter and Space (keyboard cases assert exactly one removal, no navigation, no health panel); wrapping at 320px without horizontal overflow; re-render without doubled handlers; mocked 404; combined interactions.
  • Any pageerror or console.error on a step's pages fails that step. Playwright's default timeout is 5 s. Exit 0 requires all 24 scenarios to run with zero failures.
  • Playwright's own SIGINT/SIGTERM/SIGHUP handlers are disabled for this test, because they only close the browser and could let an interrupted run exit 0 without a summary. The test instead prints a FAIL — interrupted line (counted as a failure by scripts/aggregate-e2e-pass.sh), closes the browser and exits 128+signal.

Local verification (not GitHub CI)

Fresh, on an isolated export of the merge result of this branch onto current master 24760c3db917e908796045eeb0c4c5a3f98d0bc9 (after #53); merge tree 4721d3a1b7819bac002420b1988251e730b9d03b; macOS, Playwright 1.58.2 Chromium:

  • Merge is conflict-free. Only the four files above differ from master. fix(frontend): handle orphaned api cleanup promise rejection #53's public/app.js, test-app-api-inflight-cleanup-rejection.js and test-all.sh are unchanged, and the workflow YAML equals master's apart from the one added line.
  • CHROMIUM_REQUIRE=1 node test-issue-2027-my-mesh-node-page-e2e.js: 2 runs, 24/24, exit 0, no leftover browser or test processes.
  • node --unhandled-rejections=strict test-app-api-inflight-cleanup-rejection.js: 8/8, exit 0.
  • node --check, YAML parse, git diff --check and the three TestForkGuard* tests pass.

Earlier local runs on this branch (before #53 was merged; not repeated for this PR):

  • 3 normal runs 24/24. With only the keydown guard removed, the 8 keyboard scenarios fail (Node page ×4, Full health ×2, Remove ×2) and 16 pass.
  • SIGTERM / SIGINT / SIGHUP during an unfinished run: exit 143 / 130 / 129 with the FAIL marker, no summary, no leftover processes. The previous version of the test exited 0 in the same situation.
  • Injected console.error and late pageerror were detected; a doubled Remove re-render was detected; a fatal exception after launch exited 1; CHROMIUM_REQUIRE=1 without Chromium exited 1; the registration line under bash -eo pipefail with tee kept failures non-zero.
  • Independent review approved the last commit. Signal probes during chromium.launch() (0–600 ms) and during the final browser.close() (0–120 ms) found no exit 0 combined with a FAIL marker.

Known limitations

  • Staging has not been tested.
  • No GitHub CI result is claimed here; see the checks on this PR.
  • test-home-coverage-e2e.js (existing test, needs a running server) failed its "Full health" step once in 4 local runs on the base without this change; with this change it passed 4 of 4. Treated as a pre-existing flake; not investigated.
  • On the normal path, page.close() and the final browser.close() have no local timeout; they are bounded only by the job timeout (deploy.yml sets none, so GitHub's default applies). After a signal the test forces exit within 5 s.
  • If a signal arrives exactly during the final browser.close(), the exit code relies on Playwright's close ordering rather than an explicit check (probes found no false pass). A rejection from page.close() during an interrupt can exit 1 instead of 128+signal; both are non-zero.
  • The test is not added to test-all.sh.

🤖 Generated with Claude Code

dborup and others added 4 commits September 14, 2026 17:57
…Mesh cards

Ports the relevant part of upstream PR Kpa-clawbot#2027 (Kpa-clawbot/CoreScope,
head e5536d2): adds a "Node page ->"
button to My Mesh cards, on both the normal card and the health-error
card, plus flex-wrap on .mnc-actions so buttons wrap on narrow cards.
Escaping/URL-encoding of the pubkey in navigation matches the existing
Full health/View packets buttons.

Local addition, not in upstream's diff: the card's keydown handler
called e.preventDefault() on Enter/Space before delegating to the
click handler (which already ignores .mnc-btn/.mnc-remove targets).
Because a bubbled keydown's preventDefault() suppresses the native
click-activation of a focused descendant <button>, a keyboard user
tabbed onto any nested action button (Node page, Full health, View
packets, Remove) could not activate it via Enter/Space at all. This
was reproduced with Playwright's native keyboard API (page.keyboard,
not dispatchEvent) against the ported button before applying any fix:
mouse click and card-level Enter/Space worked, but Enter/Space on a
focused nested button did nothing. The fix moves the existing
.mnc-remove/.mnc-btn guard to the top of the keydown handler as an
early return (no preventDefault, no stopPropagation), letting the
browser's native button activation proceed. Enter/Space on the card
itself, and all existing mouse/click behavior, are unaffected.

Also verified, contra upstream's own commit message: a node seen only
in channel messages (never advertised) does NOT resolve on the detail
page in this fork. cmd/ingestor's UpsertNode (the only writer to the
`nodes` table) is called exclusively on the ADVERT branch, so such a
node has no `nodes` row and GET /api/nodes/{pubkey} 404s. Loading the
real public/nodes.js against that case renders the pre-existing
"Node not found" (Kpa-clawbot#1150) state. Navigation itself is always correct;
this is a backend data-availability fact, not a navigation bug.

Adds test-issue-2027-my-mesh-node-page-e2e.js: a Playwright regression
(route-interception pattern, no real server needed, same shape as the
already-merged test-packet-trace-alignment-e2e.js) covering card
content, mouse-click and native-keyboard (Enter/Space) navigation for
both card types, the keyboard fix (fails without it, passes with it),
non-interference with Full health/View packets/Remove, narrow-viewport
wrapping, re-render double-fire safety, the channel-only-node reality
check above, and zero page/console errors. 22/22 passing across 3
clean runs. Not registered in test-all.sh or any workflow -- per this
repo's actual CI wiring, test-all.sh is not invoked by any GitHub
Actions workflow, and Playwright e2e files are registered individually
in the "Playwright E2E Tests" job of .github/workflows/deploy.yml; the
natural registration point would be a new line there
(`CHROMIUM_REQUIRE=1 node test-issue-2027-my-mesh-node-page-e2e.js`,
no BASE_URL needed), left as a proposal rather than made unilaterally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t in CI

Test-only follow-up to 027045a; public/home.js and public/home.css are
unchanged.

test-issue-2027-my-mesh-node-page-e2e.js:
- Adds Enter and Space on a focused Remove: the focused node is removed
  exactly once (one re-render fetch for the remaining card, localStorage
  keeps only the other node), with no navigation and no health panel.
- Every harness page is tracked per step and closed when the step ends;
  any pageerror or console.error it raised fails that step (no filtering).
- Playwright's default timeout is 5000 ms per page, so a missing element
  fails a scenario in seconds instead of 30 s.
- Interrupted or incomplete runs can no longer look green. Playwright's
  own SIGTERM/SIGHUP handlers only close the browser, so a run stuck on a
  pending promise drained its event loop and exited 0 with no summary
  (reproduced). The test now launches Chromium with handleSIGINT,
  handleSIGTERM and handleSIGHUP disabled and owns those signals: it
  prints a "<file>: FAIL — interrupted" line (counted as a failure by
  aggregate-e2e-pass.sh), closes the browser and exits 128+signal.
  process.exitCode = 1 alone was not enough: with a pending timer the
  process ignored SIGTERM for over 20 s. A run also fails unless all 24
  expected scenarios ran.
- Wording: the mocked-404 step only proves nodes.js's frontend handling of
  a 404. This supersedes the broader backend claims in 027045a's message
  and test header (channel-only nodes lacking a nodes row; UpsertNode as
  the only writer): the button navigates to the correct route, and what
  the detail page shows depends on the backend's node lookup.
- CSS comment corrected: the grid layout comes from home.css; style.css
  contributes shared base rules such as box-sizing.

.github/workflows/deploy.yml: one line in the existing "Run Playwright
E2E tests" step, right after test-home-coverage-e2e.js:
  CHROMIUM_REQUIRE=1 node test-issue-2027-my-mesh-node-page-e2e.js 2>&1 | tee -a e2e-output.txt
No BASE_URL (self-contained fixture). Triggers, permissions, jobs, needs,
fork guards and all other registrations are unchanged.

Local verification on an isolated export (macOS, Playwright 1.58.2
Chromium; not CI): 3 normal runs 24/24; keydown guard removed -> 8
keyboard scenarios fail (including Remove Enter/Space), 16 pass;
SIGTERM/SIGINT/SIGHUP during an unfinished run -> exit 143/130/129
within 1 s, no summary, no leftover browser processes; injected
console.error and late pageerror detected in 3 of 3 runs;
CHROMIUM_REQUIRE=1 without Chromium -> exit 1; the registration line
under bash -eo pipefail with tee keeps failures non-zero;
TestForkGuard* pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings the branch up to master c0e7246 so CI runs on current master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ocus

Two review findings on the Kpa-clawbot#2027 port, both introduced or extended by it:

- The two new "Node page ->" buttons interpolated mn.pubkey into data-key
  unescaped, four characters from the sibling aria-label that uses
  escapeAttr(). Real pubkeys are 64 hex chars from the ingestor, so this was
  not reachable, but the port should not add to the pattern. The PR's own
  two lines now escape; the three pre-existing sites are left for a separate
  sweep so this stays a port.

- The keydown guard makes the hover-revealed Remove button (opacity: 0)
  genuinely keyboard-activatable for the first time. A sighted keyboard user
  would have been operating an invisible, unconfirmed destructive control
  with no focus ring (WCAG 2.4.7). It is now revealed whenever it or its card
  holds focus, with a regression test asserting it is visible while focused
  and hidden again afterwards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dborup
dborup merged commit 27470cb into master Sep 20, 2026
11 of 12 checks passed
dborup pushed a commit that referenced this pull request Sep 20, 2026
Brings the branch up to master 27470cb (post #54) so CI runs on current master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dborup pushed a commit that referenced this pull request Sep 20, 2026
Brings the branch up to master dbdf1c0 (post #54, #27) so CI runs on current master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant