kit-206-viewer-reload-hint: The artifact viewer's reload button shows an orange outline when the artifact changed since it was loaded

docs/devteam-dev/tickets/kit-206-viewer-reload-hint/plan.md

@@ -0,0 +1,87 @@
+# kit-206 viewer reload hint: implementation plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:executing-plans (inline, with test-driven-development). Steps use checkbox (`- [ ]`) syntax.
+
+## TL;DR
+
+- `#av-reload` gets the class `stale` (2 px `var(--amber)` outline, title "A newer version is ready: reload to see it") when the stage's artifact has a newer fingerprint than the one its frame loaded.
+- `TS` gains `fpNow` and `mainFpNow`, started equal to `fp`/`mainFp` in `showArtifact`; `tsSet` resets them.
+- Dashboard: after each `/api/data` read (`loadNow`, the raw `d`, not the possibly held `DATA`), the Library rows update them.
+- Viewer tab: every 10 s while visible, one `/api/view` ask in flight for `TS.slug`/`TS.at` (main or a draft only).
+- Reload copies the known newer fingerprints into `fp`/`mainFp` and clears the hint; nothing reloads by itself.
+- One new browser test in `test/dashboard.test.mjs`, two subtests (dashboard stage, viewer tab), light and dark.
+- Risk: one-reload races at open and at reload are accepted (a spurious hint once); each visible viewer tab on main or a draft costs one `artifact describe` per 10 s; the test waits on real polls (up to 45 s). Two tasks, only `index.html` and the test change (plan-1 P3).
+
+**Goal:** the reload button says when the artifact on the stage is behind the one on disk.
+
+**Architecture:** one hint function (`avHint()`) reads `TS` and sets the button; two feeders (the dashboard's data read, the viewer tab's poll) update `TS.fpNow`/`TS.mainFpNow` and call it; the reload handler moves the loaded fingerprints forward and calls it.
+
+**Tech stack:** the dashboard's single-file vanilla JS; `node:test` with the kit's CDP Chrome driver (`test/lib/chrome.mjs`).
+
+**Spec:** `docs/devteam-dev/tickets/kit-206-viewer-reload-hint/spec.md` (read it; this plan argues from it).
+
+## Global constraints
+
+- Files: `docs/devteam/dashboard/index.html`, `test/dashboard.test.mjs`, and this ticket's folder. No file added outside the ticket folder.
+- Colour: `var(--amber)`, the warning token (light `#f5a623`, dark `#ffb224`). No new token.
+- Titles, exact: stale "A newer version is ready: reload to see it"; otherwise "Load the page again" (today's).
+- Viewer poll: 10 s, only while `document.visibilityState === 'visible'`, only `TS.kind === 'artifact'` with `TS.at` not `tree:`, one ask in flight. Its code in `viewerBoot` uses none of the names `route(`, `render(`, `load(`, `loadNow(`, `loadSchemaDocs(`.
+- Never reload the frame except on the button's press.
+- The new test never uses `POLL_FAST`: it waits on the real 10 s polls (spec-2 N1). Waits: dashboard up to 45 000 ms (2 × 10 s + `DATA_WAIT_MS` 20 s + 5 s), viewer tab up to 37 000 ms (2 × 10 s + describe's 12 s + 5 s); met by the first `dashboard-data` run, or `/api/view` ask, that starts after the change.
+- Browser tests run here with Node 22: `/usr/local/bin/node` (the default Node 18 has no WebSocket and skips them).
+
+## Review focus
+
+1. A reload before any check (fpNow still the open-time fp): the hint still comes on a later change. Pinned in Task 1's test.
+2. A held dashboard draw (the owner typing): the check must use the read, not `DATA`. Covered by using `d` in `loadNow`; reviewer reads it.
+3. A switch of stage (another artifact, a preview, close) while a viewer ask is in flight: its reply is dropped. Pinned by the slug/at guard; reviewer reads it.
+4. A tree stage and a preview stage never poll and never show the hint. Pinned in Task 2's test (the existing "nothing else" asks list stays clean for a tree).
+5. Dark mode: outline colour is the dark token. Pinned in Task 1's test.
+
+---
+
+### Task 1: the hint, the reload, and the dashboard's check
+
+**Files:**
+- Modify: `docs/devteam/dashboard/index.html`: CSS near `.av-bar` (~516-535); `TS` (~3543) and `tsSet` (~3554); `showArtifact` (~3569-3570); `openArtifact` (~3647); the `#av-reload` handler (~3669); `loadNow` after `drawOrHold(d)` (~5325).
+- Test: `test/dashboard.test.mjs`, a new top-level test after the viewer test (~2021).
+
+**Interfaces (produced):**
+- `TS.fpNow`, `TS.mainFpNow`: string | null.
+- `avHint(): void`: sets `#av-reload`'s `stale` class and title from `TS` (stale iff `TS.kind === 'artifact' && TS.fp && TS.fpNow && TS.fp !== TS.fpNow`).
+- `avFpSeen(fp: string|null, mainFp: string|null, slug: string, at: string): void`: if the stage shows that `slug`/`at` (and `at` isn't `tree:`), sets each given non-null value on `TS.fpNow`/`TS.mainFpNow`, then `avHint()`.
+
+- [ ] **Step 1: write the failing test** `the viewer's reload button says when a newer version is ready, and the press loads it`, `{ skip: canDrive ? false : 'needs Chrome and a Node with WebSocket', timeout: 180000 }`, using `threadsFixture('reload-hint')`, `landArtifact(F, 'hint-mock', mock('main'))` and the draft written at `F.old/.worktrees/demo-01-draft/docs/devteam/project/artifacts/hint-mock` as `mock('v1')`, where `mock(t)` is `{ 'artifact.json': …title 'Hint mock'…, 'index.html': `<!doctype html><title>${t}</title><p>${t}</p>` }`. The draft `mock('v1')` is written before the dashboard starts, so its first read already lists it. A page with `COUNT_FETCH`, and on each new document a listener, installed only where `document.getElementById('av-frame')` exists (the script also runs in the artifact's own frame), that pushes `e.data.title` of `{ lens: 1, type: 'ready' }` messages whose `e.source` is `#av-frame`'s `contentWindow` into `window.__titles`. The long waits use `soonIn(p, 45000)` and `soonIn(p, 37000)` (the default is 30 s) (plan-1 P5). Helper `amber()` = computed `color` of a probe `<span style="color: var(--amber)">` appended to `#artifact-view`. Subtest **dashboard**:
+  - open `${B}/#artifact/wt/demo-01-draft/hint-mock`; wait for `__titles` to include `'v1'`; assert `#av-reload` has no `stale` and its title is `Load the page again`.
+  - press `#av-reload` (before any change); wait for a second `'v1'`.
+  - write `index.html` as `mock('v2')`; wait up to 45 000 ms for `stale`; assert title `A newer version is ready: reload to see it`, `getComputedStyle(#av-reload).outlineColor === amber()`, outline width `2px`, and `__titles` has no `'v2'` (nothing reloaded by itself).
+  - `Emulation.setEmulatedMedia` `{ features: [{ name: 'prefers-color-scheme', value: 'dark' }] }`: outline colour still equals `amber()`, and `amber()` differs from its light value; set it back to light.
+  - press `#av-reload`: wait for `'v2'` in `__titles`; assert no `stale`, title `Load the page again`, and `TS.fp === TS.fpNow`.
+- [ ] **Step 2: run it, see it fail** — `/usr/local/bin/node --test --test-name-pattern "reload button says" test/dashboard.test.mjs`; expected: times out waiting for `stale`.
+- [ ] **Step 3: implement.** CSS `#av-reload.stale:not(:focus-visible) { outline: 2px solid var(--amber); outline-offset: 1px; }`, so a keyboard focus ring still shows (plan-1 P6). Add `fpNow: null, mainFpNow: null` to `TS` and to `tsSet`'s reset. In `showArtifact`, pass `fpNow: fp, mainFpNow: mainFp` to `tsSet` (new stage), and on the same stage (a re-show, frame not reloaded) keep `fp`/`mainFp` as they are and take the given values only into `fpNow`/`mainFpNow` when non-null (plan-1 P2); call `avHint()` at its end. `avHint()` and `avFpSeen()` beside `showArtifact`. The reload handler: after resetting `src`, if `TS.kind === 'artifact'` copy `fpNow`→`fp` and `mainFpNow`→`mainFp` where non-null, then `avHint()`. `tsSet` also calls `avHint()` so a preview or close clears it. In `openArtifact`, a draft whose own Library row is absent while main's is present opens with main's fp as its `fp` (absence means equal to main; spec-2 N2). In `loadNow` after `drawOrHold(d)` (the raw `d`, so a held draw still feeds the check; spec-2 N4): if `TS.kind === 'artifact'`, `avFpSeen(row(TS.at)?.fp ?? null, row('main')?.fp ?? null, TS.slug, TS.at)` over `d.library?.artifacts`. Comments in the file's voice.
+- [ ] **Step 4: run it, see it pass**, then the whole file: `/usr/local/bin/node --test test/dashboard.test.mjs` through `devteam.mjs gate test`.
+- [ ] **Step 5: commit** `kit-206: the reload button's hint, and the dashboard's check`.
+
+### Task 2: the viewer tab's poll
+
+**Files:**
+- Modify: `docs/devteam/dashboard/index.html`: `viewerBoot` (~5664-5695) and its comment.
+- Test: `test/dashboard.test.mjs`, a second subtest in Task 1's test.
+
+**Interfaces (consumed):** `avFpSeen`, `TS.fpNow`, from Task 1.
+
+- [ ] **Step 1: write the failing subtest** **viewer tab**: write the draft back to `mock('v1')`; go to `${B}/view/artifact/hint-mock?at=demo-01-draft`; wait for `'v1'` in a fresh `__titles`; no `stale`. Write `mock('v3')`; wait up to 37 000 ms for `stale` and the stale title; `__titles` has no `'v3'`. Press: `'v3'` arrives, `stale` off. Then go to `${B}/view/artifact/hint-mock?at=tree:<T>` (T from `landArtifact`), wait (`soonIn(p, 37000)`) until `__asked` holds at least four `/api/threads` (boot's read and three 5 s ticks: over 15 s after both intervals started, so one 10 s tick has passed), and assert `__asked` has exactly one `/api/view?` (boot's) and `#av-reload` has no `stale` (plan-1 P1).
+- [ ] **Step 2: run it, see it fail** (no `stale` in the viewer tab).
+- [ ] **Step 3: implement** in `viewerBoot`, beside the 5 s threads interval (after `await tlRefresh()`, so the test's threads count is the poll's clock), for an artifact: a `let asking = false` and a 10 s `setInterval` that returns unless visible, `TS.kind === 'artifact'`, `TS.at` not `tree:`, and not `asking`; it fetches `/api/view?kind=artifact&slug=&at=` for the `TS.slug`/`TS.at` taken at the ask, and on a 200 calls `avFpSeen(j.fp, j.main_fp, slug, at)`; anything else changes nothing; the fetch aborts after 15 s (`AbortController`; spec-2 N3); `asking` cleared in `finally`. Rewrite the boot comment to name this read (one `artifact describe` per ask, every 10 s while seen, main or a draft only).
+- [ ] **Step 4: run it, see it pass**, then the whole file through the gate, and `/usr/local/bin/node --test test/dashboard.test.mjs` with the existing viewer test's "nothing else" check still green.
+- [ ] **Step 5: commit** `kit-206: the viewer tab checks its artifact's fingerprint`.
+
+After both: verification.md (a row per done-when), then the finish (worker.md §8). The stale `/api/view` comment was told to the supervisor in m-20261115-013705816 and ruled on (below) (plan-1 P4).
+
+## Rulings
+
+- m-20261115-013720113-supervisor (re m-20261115-013705816): spec approved; leaving `devteam.mjs`'s stale "never polled" comment on `/api/view` for kit-106 is fine; the supervisor tells fast-threads.
+
+## From the spec reviews
+
+spec-2 (APPROVE 9d399ce) left N1-N4; they are taken here, not in spec.md, so its approval holds: the test's waits and no `POLL_FAST` (N1), main's fp for an unlisted draft at open (N2), a 15 s abort on the viewer's ask (N3), the check on every read, held or drawn (N4).

docs/devteam-dev/tickets/kit-206-viewer-reload-hint/reviews/final-1.md

@@ -0,0 +1,25 @@
+# Final review, pass 1
+
+- Reviewer: final-reviewer (fresh subagent, claude-opus)
+- Reviewed: 2e02abc6542e52b2e1ae3bae370f2a9ee6b26076 (PR #242)
+
+**Summary.** I read the ticket, spec, plan record, verification, task reviews and PR body, viewed both screenshots, and read the full diff of `docs/devteam/dashboard/index.html` and `test/dashboard.test.mjs` with the code around it: `TS`/`tsSet`, `showArtifact`, `avHint`/`avFpSeen`, `openArtifact`, `closeArtifact`, the `#av-reload` handler, `route`, `loadNow`, `viewerArtifact`/`viewerBoot`, `tArtFpNow` and `tsSubject`. `/usr/local/bin/node --test --test-name-pattern "reload button says" test/dashboard.test.mjs` at this head passed 3/3 (the test and both subtests).
+
+Done-whens:
+1. **The check and the outline: met.** The dashboard takes the fingerprint from the Library rows after each `/api/data` read (the `loadNow` hook). A viewer tab asks `/api/view` every 10 s while visible, for main or a draft only, one ask at a time, aborting after 15 s, dropping a reply when the stage has moved on. The test checks the outline's computed colour equals a probe's `var(--amber)` in light and in emulated dark (the two differ), the 2px width and the title. The screenshots show the outline in both schemes.
+2. **Press clears it, nothing reloads by itself: met.** The frame still shows the old `<title>` when the hint appears and again 3 s later; after the press it shows the new one, `stale` is off and the title is back to "Load the page again". The reload handler copies only fingerprints it knows.
+3. **The dashboard test: met.** Both hosts, a press before the first check, and a tree never asked about. Per the task reviews, the T1-1 and T1-2 mutations and the removed tree guard each fail it.
+4. **CI and no file added: partly evidenced.** No file is added outside the ticket folder; changes are limited to index.html and the test plus records. All 8 test.yml jobs were still pending when I checked; the merge gate covers it.
+
+Behaviour in context:
+- **Thread "changed since" (`tArtFpNow`) and new threads (`tsSubject`).** `TS.fp` stays the fingerprint the frame loaded; before this change it was set only at open (`route()` runs only on the first read and on `hashchange`). "Changed since" works as before, and new threads carry the fingerprint of what is on screen. Reload moves `fp` and `mainFp` forward, matching the frame after the press.
+- **Previews, close, switching stages.** Each goes through `tsSet`, which clears both new fields and calls `avHint`. The reload handler leaves previews alone.
+- **The owner's framed wrapper.** It shows only trees, so it never polls. In the framed case where a draft is gone (a 404, shown "held"), the tab polls and gets 404s, which change nothing.
+- **Code style.** Comments and names match the file. The new poll avoids the names the existing viewer test forbids; that test passed in the whole-file gate run at 7f62d29 and the code is unchanged since. Nothing project-specific.
+
+| Id | Severity | Rule | Where | Finding | Disposition |
+|---|---|---|---|---|---|
+| F1 | Minor | spec acceptance check 3 (flake risk) | test/dashboard.test.mjs, viewer-tab subtest, "no check yet when pressed" | The assertion that only boot's `/api/view?` has been asked depends on the frame loading and the press landing within the first 10 s tick after `tlRefresh`. On a very slow runner it could fail: a flake, not a false pass. | accepted: the load and press take about 1 s against a 10 s window, and a failure is loud; if CI shows it, the fix is to press on the first `ready` |
+| F2 | Minor | spec Edge cases (ruling m-20261115-013720113) | docs/devteam/scripts/devteam.mjs, the `/api/view` comment | The comment still says `/api/view` is "never polled"; the supervisor ruled to leave it for kit-106, outside this ticket's touches. Noted so it is tracked. | ruled (m-20261115-013720113): left for kit-106; the supervisor tells fast-threads |
+
+Verdict: APPROVE 2e02abc6542e52b2e1ae3bae370f2a9ee6b26076

docs/devteam-dev/tickets/kit-206-viewer-reload-hint/reviews/plan-1.md

@@ -0,0 +1,27 @@
+# Plan review, pass 1
+
+- Reviewer: plan-reviewer (fresh subagent, claude-opus)
+- Reviewed: 34ee7d3a4a92f707d3ba4ae86c8a869537d6bd50 (plan.md)
+
+**What I read.** At 34ee7d3: the plan, the spec, spec-1 and spec-2, the ticket, and quality.md §3 and §6; in `index.html` the tokens (~30-45), the viewer CSS (~516-540), `#av-reload` (~779, ~3669), `TS`/`tsSet`/`showArtifact` (~3543-3575), `openArtifact`/`route` (~3638, ~3685), the frame `load` and lens `message` handlers (~5171-5200), `drawOrHold`/`load`/`loadNow` (~5253-5330) and `viewerArtifact`/`viewerBoot` (~5644-5695); in `devteam.mjs` `ARTIFACT_LENS` (`ready` only in answer to `hello`, carrying `document.title`), `draftWorktree`/`draftArtifacts` (a plain directory under `.worktrees/` is a draft), `/api/view` (a 12 s describe; `ARTIFACT_AT` takes worktree names), the artifact route (`no-store`) and `dashboardData`/`DATA_WAIT_MS`; in the tests `soonIn`, `POLL_FAST`, `landArtifact`, `COUNT_FETCH`, the existing viewer test, `threadsFixture`, `chrome.mjs` (`send` is generic, so `Emulation.setEmulatedMedia` works) and `test/browser/dev-links.mjs`, which already clicks `#av-reload` and listens to lens messages from the parent page.
+
+**Coverage.** Every done-when and spec points 1-5; N1-N4 taken in as spec-2 says.
+
+**Interfaces.** `avHint`, `avFpSeen(fp, mainFp, slug, at)` and `fpNow`/`mainFpNow` are used the same way in both tasks.
+
+**Feasibility.** The lens `ready` listener on the parent works (messages from an opaque-origin frame reach the parent; `e.source` compares with `#av-frame.contentWindow`); the `#artifact/wt/<wt>/<slug>` route and a draft folder under `F.old/.worktrees/` work; the computed `outline-color` against a probe and the emulated dark scheme work; the real 10 s polls fit in the 180 s timeout.
+
+**Existing tests.** They keep passing: the viewer test's "nothing else" list allows `/api/view?`; the `viewerBoot` checks (forbidden names, `postMessage`, `location.*`) are not tripped; the fault tests are unaffected.
+
+**What blocks.** P1: the tree check waits a fixed 11 s from navigation, but the poll's interval only starts after boot's awaits, so the check would usually pass even with the `tree:` guard broken.
+
+| Id | Severity | Rule | Where | Finding | Disposition |
+|---|---|---|---|---|---|
+| P1 | Important | spec "The change" 2 (a `tree:` version is never checked); plan Review focus 4; quality.md §3 (missing test with a plausible failure path) | plan Task 2 Step 1; `index.html` `viewerBoot` ~5664-5695 | The 10 s interval is created after `await viewerArtifact()` and, beside the threads interval, after `await tlRefresh()`. `go()` returns at the start of boot, so the interval starts 1-3 s later and its first tick lands after an 11 s assertion: a missing `tree:` guard would still pass. Create the view poll beside the 5 s threads interval and wait for at least four `/api/threads` in `__asked` before asserting one `/api/view?`, or another wait that provably covers one tick. | fixed: the poll sits beside the threads interval; the test waits for four `/api/threads` |
+| P2 | Minor | spec "The change" 1 | plan Task 1 Step 3; `index.html` ~3570 `else Object.assign(TS, { fp, mainFp })` | On a re-show of the same stage the frame is not reloaded, but `TS.fp` is overwritten with `DATA`'s newer fp, so the hint clears while the frame still shows the old page. Keep `fp`/`mainFp` there, or accept it. | fixed: a re-show keeps `fp`/`mainFp`; the given values go only to `fpNow`/`mainFpNow` |
+| P3 | Minor | quality.md §6 (the TL;DR says what could go wrong) | plan TL;DR | No risk bullet. Swap one bullet for the main risk. | fixed: a risk bullet |
+| P4 | Minor | spec Edge cases, last bullet | plan, the "After both" line | No step tells the supervisor about the stale `/api/view` comment. | fixed: told in m-20261115-013705816, ruled in m-20261115-013720113; both in the plan |
+| P5 | Suggestion | none (robustness) | plan Task 1 Step 1 | (a) `addScriptToEvaluateOnNewDocument` also runs in the artifact's frame: guard the listener on `#av-frame`. (b) Write the v1 draft before the dashboard starts. (c) The long waits need `soonIn(p, 45000)`/`soonIn(p, 37000)`; the default is 30 s. | fixed: all three in Task 1 Step 1 |
+| P6 | Suggestion | none (taste, accessibility) | plan Task 1 Step 3 CSS | `#av-reload.stale` overrides the `:focus-visible` ring. Consider `#av-reload.stale:not(:focus-visible)`. | fixed: `:not(:focus-visible)` |
+
+Verdict: CHANGES 34ee7d3a4a92f707d3ba4ae86c8a869537d6bd50