Skip to content

fix(map): drop the M4-banned caret from a map.js comment - #56

Merged
dborup merged 1 commit into
masterfrom
codex/fix-1648-m4-map-area-comment-glyph
Sep 17, 2026
Merged

dborup merged 1 commit into
masterfrom
codex/fix-1648-m4-map-area-comment-glyph

Conversation

@dborup

@dborup dborup commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Why

test-issue-1648-m4-emoji-scan.js forbids UI-icon characters on every line of the files it scans, comments included, unless the line is tagged EMOJI-OK. For public/map.js that explicitly includes the dropdown caret ▾.

A comment added with the selected-area outline (4d46dba4b, merged in 9f5d594c) quoted the Area dropdown label as "Area: X ▾". That single character made the scan fail, which stops the "Run JS unit tests (packet-filter)" CI step. It went unnoticed while that step was already stopping earlier on the old test-issue-1375-scope-stats-fetch.js failure.

What changes

  • One comment line in public/map.js: "Area: X ▾" becomes "Area: X". The next line already names AreaFilter, so the comment still identifies the control.
  • The UI and program behaviour are unchanged. No code, test, workflow or config is modified.
  • The scanner is not changed or weakened, and no EMOJI-OK tag or allowlist entry is added.

Verification (local, not GitHub CI)

Fresh runs from exact HEAD 52ae9d1c:

  • test-issue-1648-m4-emoji-scan.js: fails on master 12cc30ff with one hit at map.js:681, passes on this branch.
  • test-issue-1648-m2-emoji-scan.js (also scans map.js): passes before and after.
  • node --check public/map.js passes and git diff --check is clean.
  • Negative control, in a scratch copy: restoring the original comment makes the M4 scan fail again with exactly one hit at map.js:681.

Reused from the earlier local verification of this same commit, not re-run:

  • The M4 scan also failed as expected when a forbidden icon was injected into rendered markup (▾ at map.js:162, ✕ in an L.divIcon HTML string at map.js:2587).
  • test-issue-1375-scope-stats-fetch.js passed 6/6 and test-app-api-inflight-cleanup-rejection.js 8/8, both under --unhandled-rejections=strict.
  • A local run of the workflow's JS unit-test step, with the same fail-fast behaviour, reached all 67 of 67 commands; previously it stopped at 55.

Known next baseline failure and limitations

  • Next failure: that local JS-step run still fails on its last command, test-a11y-axe-routes-coverage.js: axe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardriving. It fails identically on unmodified master and is not addressed here, so the CI step is still expected to fail at that point.
  • Not fully green: full CI is not documented as passing. Go tests, the Playwright E2E suite, the full test-all.sh and later CI jobs were not run for this change. test-issue-1648-m6-final-sweep.js (run only by test-all.sh) already fails on master because of public/packet-path-map.js and is unrelated.
  • No browser or staging test: none was done, because the change is a comment only.

Update 2026-09-17: re-verified against current master e17377d8 (after #57 and #58)

The branch is unchanged (still 52ae9d1c on base 12cc30ff); it was not rebased or amended. public/map.js is identical on 12cc30ff and e17377d8, and the computed merge with current master (tree 01a42ed9) changes exactly one line in public/map.js.

The concrete failure: the push CI run for #58 on master (run 35177011746, e17377d8) failed only in "Run JS unit tests (packet-filter)", on the M4 scan hit map.js:681 // … "Area: X ▾". The Go server (-race), ingestor, channel and decrypt suites passed in that run. Playwright, Docker, release, deploy and badges were skipped.

What this PR changes / does not change: unchanged from above — one comment line, no runtime change, scanner/expectations/workflows untouched, no EMOJI-OK tag.

Local results (macOS arm64, Node v25.6.1; isolated git archive exports, not GitHub CI)

Check Baseline e17377d8 Candidate (merge tree 01a42ed9)
node test-issue-1648-m4-emoji-scan.js rc=1, one hit map.js:681 rc=0, PASS: all M4 surfaces icon-free
node --check public/map.js ok ok
git diff --check (branch diff and merge-tree diff) — clean
Workflow JS step (set -e + the 67 node commands, same order) stops at command 55 (M4) passes M4, stops at command 67

Command 67, test-a11y-axe-routes-coverage.js, fails identically on baseline and candidate (axe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardriving). It is a pure source check, pre-existing on master, previously hidden behind the M4 failure, and not fixed here. Run individually, all other 66 commands exit 0 on the candidate (test-a11y-axe-1668-selftest.js prints an intentional [FAIL] STALE ALLOWLIST selftest line but exits 0).

Actual GitHub CI

  • The only CI run for this PR is 34942815482 (2026-09-15, merge of 52ae9d1c into the old base 12cc30ff, Linux). In it the M4 scan passed and the JS step then failed on the same test-a11y-axe-routes-coverage.js assertion; Go suites passed; later steps and jobs were skipped.
  • No CI has run for this PR against e17377d8: no new commit was pushed, and no workflow was re-run manually.
  • Expected CI outcome after merge: the JS step would still fail, now on test-a11y-axe-routes-coverage.js (a11y CI: expand axe route coverage to remaining 7 analytics tabs Kpa-clawbot/CoreScope#1706) instead of M4. This PR alone does not make CI green.

🤖 Generated with Claude Code

…ment

test-issue-1648-m4-emoji-scan.js forbids U+25BE (the dropdown caret) on
any map.js line not tagged EMOJI-OK, comments included. The comment
added with the selected-area outline (4d46dba, merged in 9f5d594)
quoted the Area dropdown label with its caret and broke the scan, which
went unnoticed while the JS CI step stopped earlier on the Kpa-clawbot#1375 test.

The quoted "Area: X" still identifies the control, and the next line
names AreaFilter. Comment-only: no runtime change, scanner unchanged.

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