Hermetic web builds, one Node version, and parity in CI (attn-6q7b)

.beads/interactions.jsonl

@@ -812,3 +812,6 @@
 {"id":"int-4a61bc7545c78bd33856bed64a0f7d7e","kind":"field_change","created_at":"2026-08-20T22:47:48.527633Z","actor":"Angus Bezzina","issue_id":"attn-1l2f.4","extra":{"field":"status","new_value":"in_progress","old_value":"open"}}
 {"id":"int-ede33a1c0c39c73a2a09fdf82f07157f","kind":"field_change","created_at":"2026-08-20T22:56:31.837259Z","actor":"Angus Bezzina","issue_id":"attn-1l2f.4","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"deskEnterOpensSelection() in web/src/hosted/app/desk-keys.ts: Enter belongs to the desk only when focus is not on/inside an interactive control (tag, href-bearing <a>, ARIA role, contenteditable); the filter input is the explicit exception. Arrow and '/' gates keep typingInField. 20 cases in desk-keys.test.ts including a pre-fix contrast assertion."}}
 {"id":"int-c912b8333c9cb887f33085ee0b487a88","kind":"field_change","created_at":"2026-08-20T22:56:32.749332Z","actor":"Angus Bezzina","issue_id":"attn-1l2f","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"All four findings fixed and verified. web: 130 test files 0 failures, svelte-check 0 errors, both builds green. rust: 563 lib tests pass, clippy clean. Review E2E unchanged vs baseline (43 PASS / 4 PEND / 13 pre-existing FAIL). Changes are uncommitted on angus/comment-card-redesign pending review; codex re-review of the criterion needs a commit first."}}
+{"id":"int-6c72086930397d90a7623af9b89fdea9","kind":"field_change","created_at":"2026-09-03T23:02:03.869571Z","actor":"Angus Bezzina","issue_id":"attn-6q7b.1","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-e81c0c2ba1299115b3a95da02f9d8c36","kind":"field_change","created_at":"2026-09-03T23:02:04.497331Z","actor":"Angus Bezzina","issue_id":"attn-6q7b.2","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-2e4b2f65fe6de7795ab8ec4d3bc18ea3","kind":"field_change","created_at":"2026-09-03T23:02:05.074798Z","actor":"Angus Bezzina","issue_id":"attn-6q7b.3","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}}

.github/workflows/ci.yml

@@ -54,7 +54,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v4
         with:
-          node-version: "20"
+          node-version: "22"
           cache: npm
           cache-dependency-path: web/package-lock.json
 
@@ -116,7 +116,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v4
         with:
-          node-version: "20"
+          node-version: "22"
           cache: npm
           cache-dependency-path: web/package-lock.json
 
@@ -169,7 +169,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v4
         with:
-          node-version: "20"
+          node-version: "22"
           cache: npm
           cache-dependency-path: web/package-lock.json
 
@@ -208,7 +208,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v4
         with:
-          node-version: "20"
+          node-version: "22"
           cache: npm
           cache-dependency-path: relay/package-lock.json
 
@@ -223,3 +223,68 @@ jobs:
 
       - name: Test
         run: npm test
+
+  parity-e2e:
+    name: Native/Hosted Parity (Ubuntu)
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Install native dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev libayatana-appindicator3-dev xvfb
+
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: "22"
+          cache: npm
+          cache-dependency-path: |
+            web/package-lock.json
+            relay/package-lock.json
+
+      - name: Install dependencies
+        working-directory: web
+        run: npm ci
+
+      # Local mode runs the relay from relay/, so its deps have to be installed
+      # or `npx wrangler` silently fetches whatever is newest from the registry
+      # — a different major than the pinned one, resolved at run time. That is
+      # the non-hermeticity this whole change removes; don't reintroduce it here.
+      - name: Install relay dependencies
+        working-directory: relay
+        run: npm ci
+
+      - name: Install Playwright Chromium
+        working-directory: web
+        run: npx playwright install --with-deps chromium
+
+      - name: Setup Rust
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Cache Rust
+        uses: Swatinem/rust-cache@v2
+        with:
+          shared-key: parity-e2e
+
+      # build.rs embeds web/dist/index.html, so the frontend has to exist
+      # before cargo or the daemon runs a stale UI. Debug build on purpose:
+      # the suite drives the window through --eval/--wait-for, which release
+      # builds strip.
+      - name: Build frontend and owner
+        run: |
+          npm --prefix web run build
+          cargo build --locked
+
+      # Local mode: the script boots its own Miniflare relay, Vite server and
+      # native owner, so this needs no deployed environment and gates the PR
+      # rather than the deploy.
+      #
+      # This suite is the only thing that exercises the owner<->reviewer round
+      # trip end to end. While it lived solely in the dispatch-only deploy
+      # workflow, a real regression in the owner's review rail (attn-6q7b,
+      # fixed in #19) sat on main through two merges and a version bump.
+      - name: Native/hosted parity
+        run: xvfb-run -a scripts/test-hosted-review-e2e.sh

.github/workflows/web-staging-deploy.yml

@@ -32,16 +32,6 @@ jobs:
           cache: npm
           cache-dependency-path: web/package-lock.json
 
-      # web/ pulls `eyecons` straight from GitHub, so `npm ci` has to resolve
-      # that repo's own floating devDependencies from the live registry rather
-      # than from our lockfile. A peer conflict up there (eslint 10 against
-      # plugins that cap at 9) walks npm 10's arborist into a null node and it
-      # dies with "Cannot read properties of null (reading 'edgesOut')". npm 11
-      # resolves the same tree fine. Node 22 still bundles npm 10, so pin the
-      # newer npm here until the git dependency is vendored away.
-      - name: Upgrade npm
-        run: npm install -g npm@11
-
       - name: Install dependencies
         run: npm ci
 

.gitignore

@@ -9,9 +9,6 @@ web/dist/
 web/dist-browser/
 web/playwright-report/
 web/test-results/
-web/src/lib/icons/vscode-generated/
-web/src/lib/vscode-icon-map.generated.ts
-web/src/lib/vscode-icon-packs/
 
 # wrangler dev state
 web/.wrangler/
@@ -28,3 +25,4 @@ web/.wrangler/
 # description — resolves only on the machine that produced it. Quote the finding
 # in the citing document rather than pointing at a path here.
 .impeccable/critique/
+web/.icon-packs/

.nvmrc

@@ -0,0 +1 @@
+22

CLAUDE.md

@@ -113,6 +113,38 @@ tab/profile for the joiner UX. `ATTN_DEV_RELAY_TARGET` and
 `VITE_ATTN_SHARE_ORIGIN` remain available when testing a different explicit
 relay/public-origin pairing.
 
+## Toolchain
+
+Node **22** everywhere — `.nvmrc` pins it, `web/package.json` declares
+`engines.node >=22`, and CI and the staging deploy both run it. The floor comes
+from wrangler 4 in `web/`, which refuses anything older. (The root
+`package.json` stays `>=18`: `attnmd` is the end-user CLI wrapper and its
+consumers should not inherit our build floor.)
+
+`npm ci` in `web/` is hermetic — the lockfile and the npm registry are all it
+needs. Keep it that way: **no `github:` dependencies.** Three icon packs used to
+be exactly that, and because npm resolves a git dependency's own floating
+`package.json` at install time, an unrelated upstream publish could break every
+install here with no change in this repo (attn-6q7b).
+
+### Icon packs
+
+`eyecons`, `catppuccin-vsc-icons` and `vscode-icons` are not usable from npm, so
+their **generated output is committed** under `web/src/lib/icons/vscode-generated/`
+(2,776 SVGs), `web/src/lib/vscode-icon-packs/` and
+`web/src/lib/vscode-icon-map.generated.ts`. The build reads those files and needs
+no pack installed, which is why generation is not part of `build` or `check`.
+
+To change or upgrade a pack, edit the pin in `web/scripts/icon-pack-pins.mjs`,
+then:
+
+```bash
+cd web && npm run refresh:icons   # fetch pinned tarballs, regenerate, commit the diff
+npm run check:icons               # verify the committed tree is self-consistent
+```
+
+`check:icons` never re-runs the generator — it must pass with nothing fetched.
+
 ## Build
 
 ```bash
@@ -266,6 +298,30 @@ scripts/test-apply-e2e.sh
 
 Drives the full owner-side accept/reject pipeline end-to-end: snapshot + UserEdit drift forces the suggestion to REMAP, `apply_ready_verdict` writes the file, the `LocalRevision` journal lands UserEdit + AcceptedSuggestion in order, and a `SuggestionAccepted` (or `SuggestionRejected`) envelope round-trips through the outbox with `resulting_hash` matching the on-disk hash. The Rust E2E cases live in `src/review/apply.rs` as `e2e_*` tests; the bash wrapper also probes the running daemon for the same end-state via the `--eval` bridge (daemon-layer assertions print `PEND` until attn-nnj.8.5 wires the `AcceptSuggestion` command — flip via `ATTN_APPLY_E2E_REQUIRE_DAEMON=1`).
 
+### Native ↔ hosted parity (`scripts/test-hosted-review-e2e.sh`)
+
+The owner↔reviewer round trip end to end: a native owner shares a document, a
+hosted browser reviewer joins through a real invite, and comments have to travel
+both ways and *render* on both sides.
+
+```bash
+scripts/test-hosted-review-e2e.sh          # local: own relay, Vite and owner
+E2E_RELAY_URL=https://relay-staging.attn.sh \
+E2E_WEB_ORIGIN=https://staging.attn.sh \
+  scripts/test-hosted-review-e2e.sh        # against deployed staging
+```
+
+**This runs in CI on every PR** (`parity-e2e` in ci.yml, local mode). It used to
+run only inside the dispatch-only staging deploy, which is how a regression that
+left the owner's review rail empty on uncurated shares survived two merges and a
+version bump (attn-6q7b, fixed in #19).
+
+Local mode owns ports 8787 and 5173 and refuses to start if either is taken —
+a leaked server from an earlier run would otherwise be silently tested against.
+The suite retries in CI only (`playwright.config.ts`), because everything shares
+one machine there; a real regression still fails every attempt. See attn-6q7b.6
+for making local mode deterministic.
+
 ### WebRTC end-to-end test (attn-nnj.7.7)
 
 Two surfaces:

scripts/test-hosted-review-e2e.sh

@@ -43,6 +43,15 @@ OWNER_PID=""
 kill_pid() {
   local pid="$1"
   [ -z "$pid" ] && return 0
+  # Kill descendants first. The relay and web servers are started via `npm run`
+  # / `npx`, which fork the real server as a CHILD — killing only the wrapper
+  # leaves that child holding the port, and the next run then either trips the
+  # port guard above or (before it existed) silently tested against a stale
+  # server from a previous run.
+  local child
+  for child in $(pgrep -P "$pid" 2>/dev/null); do
+    kill_pid "$child"
+  done
   kill "$pid" 2>/dev/null || true
   wait "$pid" 2>/dev/null || true
 }
@@ -63,6 +72,41 @@ wait_http() {
   return 1
 }
 
+# In local mode this script owns its ports. If something else already holds
+# one, we would silently test against a stranger's server: a stale Vite from an
+# earlier run answers on 5173 and serves a DIFFERENT build, which is how a
+# local run once failed 3 of 4 cases for reasons that had nothing to do with
+# the code under test (attn-6q7b). Fail loudly instead.
+port_in_use() {
+  if command -v lsof >/dev/null 2>&1; then
+    lsof -nP -iTCP:"$1" -sTCP:LISTEN >/dev/null 2>&1
+  else
+    # No lsof on the GH Actions runner image; /dev/tcp is a bash builtin.
+    (exec 3<>"/dev/tcp/127.0.0.1/$1") >/dev/null 2>&1
+  fi
+}
+
+require_free_port() {
+  local port="$1" what="$2"
+  if port_in_use "$port"; then
+    echo "port $port is already in use, and this run needs it for the $what." >&2
+    echo "stop whatever is listening there (or set ${3} to another port) and retry." >&2
+    exit 1
+  fi
+}
+
+# Same reasoning as the port guard: `npx wrangler` in relay/ would happily
+# download a wrangler that is not the pinned one and run the relay on it.
+if [ -z "${E2E_RELAY_URL:-}" ] && [ ! -d "$PROJECT_DIR/relay/node_modules" ]; then
+  echo "local mode runs the relay from relay/, whose dependencies are not installed." >&2
+  echo "run: npm --prefix relay ci" >&2
+  echo "(or set E2E_RELAY_URL and E2E_WEB_ORIGIN to test against a deployed environment)" >&2
+  exit 1
+fi
+
+[ -n "${E2E_RELAY_URL:-}" ] || require_free_port "$RELAY_PORT" "local relay" RELAY_PORT
+[ -n "${E2E_WEB_ORIGIN:-}" ] || require_free_port "$WEB_PORT" "hosted web app" WEB_PORT
+
 mkdir -p "$ATTN_HOME" "$SHARE_DIR" "$RELAY_STATE"
 printf '# Hosted review canary\n\n- [ ] Read-only browser task\n\nCiphertext boundary marker: NARWHAL-TEAK-7429.\n\nShared by native, rendered in the hosted reviewer.\n' >"$DOC"
 printf '# Folder sibling canary\n\nSwitching files must switch decrypted document content.\n' >"$SIBLING_DOC"