feat(nav): show the running CoreScope version in the navigation drawer - #142
Conversation
Unit test (real nav-drawer.js, fake DOM, counting fetch): 1 of 14 pass on master (the narrow-width no-request guard). E2E at 1440x900, 1024x768, 800x900 and 1280x480: 2 of 7 pass on master (the narrow and no-error guards). Relates to #111 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
#111) The drawer gets a footer link "CoreScope <version>" to the dborup/ CoreScope releases page, with commit and build time in its tooltip. /api/health is fetched on the first open that passes the width gate, never at page load, and cached for the page lifetime (failures too), so re-opening adds no requests. A rejected, non-OK, invalid or version-less response, or the server's "unknown" placeholder, keeps the neutral "CoreScope" label. Values go through textContent/title only. The footer does not shrink; the route list above it keeps scrolling. The mobile More sheet is unchanged (no second health fetch). Relates to #111 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
Independent review of
|
| Criterion | Result |
|---|---|
Render CoreScope <version> in the drawer footer |
Met [F]. Routed v3.1.4 renders CoreScope v3.1.4 (dark-theme screenshot at 1024x768). Unit and E2E are green on the head. |
| Link to the fork's releases page | Met [F]. nav-drawer.js:102 points to https://github.com/dborup/CoreScope/releases with target=_blank and rel="noopener noreferrer". Mutant M11 (upstream URL) is caught by unit and E2E. |
| Commit and build time in the tooltip when available | Met [F]. nav-drawer.js:117-121. "unknown" parts are dropped (M13 is caught by unit). |
| Fetch only when opened, after the viewport gate | Met [F]. requestVersion() sits after if (!isWide()) return; at nav-drawer.js:283-284. My probe saw 0 requests at load and 1 after a real touch-pointer swipe open (the onPointerUp → open() path). M2 (call before the gate) and M5 (fetch at build) are both caught. |
| Cached for the page lifetime | Met [F]. versionRequested is set before the fetch (:126). E2E: 1 request after 4 opens. M1 is caught. |
Failed, non-OK or version-less response → neutral CoreScope |
Met [F]. Unit covers 8 cases. My routed browser probe covered HTTP 500 with a version, " UNKNOWN ", and an HTML body: all showed CoreScope with an empty title and no page errors. JSON-parse rejection is handled by the second .then rejection handler (:130). |
Remote values via textContent only |
Met [F]. :115 uses textContent, :121 uses title. There is no innerHTML on the version path. M4 (innerHTML) is caught by unit and E2E. scripts/check-xss-sinks.sh --file public/nav-drawer.js flags only the pre-existing static route href (same finding on master at :118). The new setAttribute('href', RELEASES_URL) is a constant and is not flagged. |
Confirm the /api/health field names |
Met [F]. HealthResponse has json:"version", "commit" and "buildTime" (cmd/server/types.go:621-623). Curl on the local head server returned {version: unknown, commit: unknown, buildTime: unknown}. /api/stats carries the same three fields; the PR does not use it. |
| No drawer layout regression at narrow desktop/tablet widths | Met [F]. E2E passes at 1440x900, 1024x768, 800x900 and 1280x480. My own probe at 1280x220 and 1280x120 keeps the footer at 38px at the drawer bottom while the list shrinks and scrolls. |
Decide mobile More parity separately; no second eager fetch |
Met [F]. bottom-nav.js is untouched. The only new /api/health caller is the drawer. |
| Existing navigation/accessibility tests stay green | Met [F]. See Suites. The focus trap now wraps between the close button and the version link: Shift+Tab from the first focusable lands on the version link and Tab from there returns to close [F]. The drawer stays inert when closed. |
Test-first and mutants
Commit A → head: both test files are byte-identical (diff of A vs head, no changes). [F]
Red on master and A, green on the head [F]:
- Unit
test-issue-111-drawer-version.js: master 1 passed / 13 failed; A 1 / 13; head 14 / 0. - E2E
test-issue-111-drawer-version-e2e.js: master server 2 passed / 5 failed (every "no version link" check fails, plus a timeout on the routed case); head server 7 / 0.
Mutants on the head. Each was run with the new unit test, the new E2E and test-nav-drawer-1064-e2e.js. The originals were restored and verified by shasum against git show 19b7e882:<path> (js 800dc6b5…, css dfa2b584…). [F]
| # | Mutant | Unit | E2E | nav-1064 | Result |
|---|---|---|---|---|---|
| M1 | drop versionRequested = true (no cache) |
9 fail | 4 fail | pass | caught |
| M2 | call requestVersion() before the width gate |
1 fail | 1 fail | pass | caught |
| M3 | drop the "unknown" filter |
2 fail | 4 fail | pass | caught |
| M4 | innerHTML instead of textContent |
4 fail | 1 fail | pass | caught |
| M5 | fetch at DOM build (page load) | 2 fail | 5 fail | pass | caught |
| M6 | drop the r.ok check |
1 fail | pass | pass | caught (unit) |
| M7 | drop the typeof v !== 'string' guard |
crash (unhandled rejection) | pass | pass | caught (unit) |
| M8 | drop the rejection handler | crash (unhandled rejection) | pass | pass | caught (unit) |
| M9 | drop flex-shrink: 0 on the footer |
pass | pass | pass | equivalent: a probe at 1280x220 and 1280x120 shows an identical 38px footer and the same list height with and without it (a flex item's min-height: auto already stops it shrinking below its content) |
| M10 | drop trim() |
1 fail | pass | pass | caught (unit) |
| M11 | upstream releases URL | 1 fail | 4 fail | pass | caught |
| M12 | never set title |
2 fail | 1 fail | pass | caught |
| M13 | unfiltered commit in the tooltip |
1 fail | pass | pass | caught (unit) |
Suites run locally
| Suite | master | head |
|---|---|---|
test-issue-111-drawer-version.js (new) |
1/14 | 14/14 |
test-issue-111-drawer-version-e2e.js (new) |
2/7 | 7/7 |
test-nav-drawer-1064-e2e.js |
11/11 | 11/11 |
test-bottom-nav-1061-e2e.js |
31/31 | 31/31 |
test-gesture-hints-1065-e2e.js |
15/15 | 15/15 |
test-nav-more-floor-1139-e2e.js |
10/10 | 10/10 |
test-issue-1648-m1-icons-e2e.js |
16/16 | 16/16 |
test-privacy-page.js |
34/34 | 34/34 |
test-issue-1648-m1-emoji-scan.js |
pass | pass |
test-issue-1668-m4-per-route.js |
pass | pass |
test-packet-filter.js |
92/92 | 92/92 |
test-aging.js |
19/19 | 19/19 |
test-frontend-helpers.js |
705 pass / 2 fail | 705 pass / 2 fail. Identical failure set (favStar ×2), pre-existing and unrelated |
All counts above are [F]. Go was not touched by the PR, so the Go suites were not run. eslint could not be run: there is no config in the tree, so I rely on the PR text for that [T].
Browser
Own Playwright scenario against the head server [F]:
- 1280x800 with touch: 0
/api/healthrequests at load, then a real touch pointer swipe (down 30 → up 300) opens the drawer and makes exactly 1 request. - The assertions of the existing "Version info lives on Perf dashboard, not in navbar" test still hold with the drawer open: there is no
#navStats .version-badgeor.engine-badge, and the drawer is appended to<body>, outside#navStats, the top nav and any header. The PR's footer does not touch#navStats. - Routed HTTP 500,
" UNKNOWN "and an HTML body: all show the neutralCoreScopelabel, an empty title and no page errors. - A routed long version is ellipsised inside the drawer (see finding 2).
- Screenshots (kept locally with the reviewer, not attached):
shot-1024x768-dark.png: footerCoreScope v3.1.4at the bottom and all routes visible.shot-1280x480-long.png: the ellipsis.shot-1280x220.png
- The light theme shows the pre-existing white drawer header, the same on master as the PR states. It is unrelated.
- Computed colours: the link uses
--nav-text-muted(rgb(203,213,225)) on--nav-bg, and the border uses--borderin both themes. This matches the existing drawer header border.
Performance and security
- One
/api/healthrequest per page lifetime, only after the first wide open. There is no work at load and nothing on a render, ingest or WS path. No perf claim needs proof. [F] - The new state is bounded: one boolean and one element ref. There are no timers or listeners. [F]
- The only DOM sinks are
textContentandtitle. Thehrefis a constant. [F] - CSS uses variables only (
--border,--nav-text-muted,--nav-text,--accent). The rgba values innav-drawer.cssare pre-existing. [F] cmd/serveris not touched. Nomap[string]interface{}was added. [F]- A failed first fetch is cached for the page lifetime, so a transient error leaves the neutral label until reload. That is what the issue asks for. [K]
Not verified
- Real touch devices or tablets. The swipe was simulated with synthetic
PointerEvents. - A real release build's version string. Local builds report
unknown, so the populated case used routed responses. - On a mouse-only desktop the drawer cannot be opened at all (touch/pen only, and no hamburger calls
__navDrawer.open), so those users never see the footer. This is pre-existing drawer design. [K] - The full
test-e2e-playwright.jsrun. It is known to fail fast locally; its version assertion was reproduced in my probe instead. - CI's
check-xss-sinks.sh --diffmode. There is no git in the archive, so I used--filemode on master and head as described above. - The PR's "Not verified" section (touch devices; the real release string) is honest and matches my gaps.
Relates to #111
Plan and design
The user asked for autonomous work, so the plan is written here instead of waiting for sign-off (AGENTS.md rule 5).
Commits:
1ea7dbf3: tests that reproduce the missing footer (red on master).19b7e882: the feature.What the drawer footer does
public/nav-drawer.js) gets a footer with a linkCoreScope <version>pointing tohttps://github.com/dborup/CoreScope/releases(target="_blank",rel="noopener noreferrer").commit <sha> · built <time>when those values are known.flex-shrink: 0at the bottom of the drawer's flex column. The route list above it keeps its own scroll, so no route is covered.--nav-text-muted,--nav-text,--border,--accent). Long versions are truncated with an ellipsis.Fetching
/api/health/api/healthis fetched on the firstopen()that passes the existing width gate (> 768px). It is never fetched at page load, and never at narrow widths where the drawer cannot open.handleHealth→HealthResponse{Version, Commit, BuildTime}(json:"version","commit","buildTime")."unknown"(resolveVersion/resolveBuildTime/resolveCommitincmd/server/main.go), and a local build really reportsversion: "unknown". Those values are treated as missing.CoreScopelabel, never blank,undefinedorunknown:"unknown"textContentandtitle.Mobile "More" sheet
Deliberately not changed. The More sheet at ≤ 768px has no footer today. Adding one would need its own lazy fetch, which the issue asks to decide separately. There is no second health fetch anywhere.
How this differs from upstream
Kpa-clawbot/CoreScope#2068Upstream is read as a reference only; nothing was cherry-picked.
"unknown"is treated as missing. Upstream would renderCoreScope unknown..then) on every open.--nav-text-mutedtoken. Upstream referenced a--nav-mutedtoken that the fork does not define, plus an rgba fallback.Acceptance criteria
CoreScope <version>in the drawer footer"unknown"), E2E test with a routed responseCoreScopetextContentonlyinnerHTMLis written. E2E: the same with<b>/api/healthfield names confirmedTests
d264716ctest-issue-111-drawer-version.js(realnav-drawer.js, fake DOM, countingfetch)test-issue-111-drawer-version-e2e.js(Playwright, local server ontest-fixtures)deploy.ymlunit step andtest-all.sh; the E2E test in the Playwright step next totest-nav-drawer-1064-e2e.js.Existing suites (local, this branch):
test-nav-drawer-1064-e2e.jstest-bottom-nav-1061-e2e.jstest-gesture-hints-1065-e2e.jstest-nav-more-floor-1139-e2e.jstest-privacy-page.jstest-issue-1648-m1-emoji-scan.jstest-issue-1668-m4-per-route.jseslint and
scripts/check-xss-sinks.sh --diff origin/masterare clean.Browser check
v3.1.4: the footer showsCoreScope v3.1.4at the bottom of the drawer and all routes stay visible.Perf
There is one request per page, and only once the drawer is opened. There is no work at page load and nothing on a hot path.
Not verified
unknown, so the populated case was checked with a routed response.Overlap with other open PRs
.github/workflows/deploy.yml(unit step and Playwright step) andtest-all.sh: one line each, next to lines added by PR fix(live): recover the shared WebSocket from silent half-open connections #140 (fix(live): recover the shared WebSocket from silent half-open connections #117), PR fix(packets): empty observer/type selections on Clear Filters #132 (fix(packets): Clear Filters must reset observer and type selection state #121), PR fix(analytics): treat the distance index's 202 as a transient building state #133 (fix(analytics): treat lazy distance-index 202 responses as transient #120), PR fix(rx-coverage): honour configured defaults and save an independent viewport #136 (fix(rx-coverage): honor configured defaults and save an independent viewport #124), PR fix(map): ignore stale async work and keep Path Inspector controls usable #139 (fix(map): ignore stale async work and keep Path Inspector controls usable #123) and PR fix(live): wire every persisted view toggle before Live init awaits #135 (fix(live): wire persisted view toggles before initialization awaits #125).public/nav-drawer.jsorpublic/nav-drawer.css.origin/master, in issue order: the only conflicts are the test-registration lines indeploy.ymlandtest-all.sh, where several PRs add a line after the same anchor. Keep both lines; nothing else overlaps textually.🤖 Generated with Claude Code
https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
Generated by Claude Code