test(ui): the node-test count guard checks every claim occurrence, not the first match (issue #95)

CHANGELOG.md

@@ -1,7 +1,18 @@
 # Changelog
 
+<!-- House rule for the terminal-status coverage claim: writing the phrase
+"Pinned by N `node --test` cases" (with a real number) declares the TOTAL case
+count of internal/ui/testdata/terminal_status.test.mjs, and the guard
+(TestTerminalStatusChangelogCount) requires EVERY occurrence in this file to
+equal the current total — so when the count grows, bump every occurrence.
+For subset counts use different phrasing (e.g. "fourteen new #83 cases").
+(issue #95) -->
+
 ## Unreleased
 
+- **test(ui): the `node --test` case-count guard now checks EVERY occurrence of the coverage-claim phrase instead of the first match (issue #95)** — `TestTerminalStatusChangelogCount` located the claim with `FindSubmatch`, which returns the first "Pinned by N `node --test` cases" in the file; since entries are prepended in reverse chronological order, the anchor was positional — any future entry using the phrase and landing above the #80 entry would silently hijack the guard (a stale number above plus drift in the real claim would still pass, and entries quoting subset counts carried no constraint at all). Of the issue's two proposals this takes the stricter one: every occurrence of the phrase must equal the derived total (`grep -c '^test('`), making the claim position-independent — the phrase now means "the file's total case count" wherever it appears, and subset counts keep their distinct phrasing (e.g. #83's "fourteen new #83 cases"). Verified by injection: a stale claim above the anchor fails, and a correct claim above plus a corrupted #80 claim fails — the exact silent-failure mode the issue named. Pinned by `TestTerminalStatusChangelogCount`.
+
+
 - **fix(worktree): the delete refusal now says WHAT is dirty, warns about the gitignored files force would destroy, and offers a force exit (issue #102)** — reported on v0.5.1-beta.3: deleting a worktree was refused with the flat "worktree has uncommitted or untracked changes; delete is refused", yet the user had confirmed the workspace clean beforehand — in fact 133 tracked source files (two whole top-level directories) had vanished BEFORE the delete attempt, and the honest-but-undetailed refusal presented a damaged workspace as an everyday "you have changes" prompt, with no force exit short of a manual `git worktree remove --force` the daemon never noticed (its state record survived). Three fixes, matching the issue's P1–P3 (the file disappearance itself was investigated in the issue and is NOT a myworktree bug): **(P1)** `Manager.Delete` is now `Delete(id, force)` and the refusal is a structured `DirtyWorktreeError` — counts by category (deleted/modified/untracked always, added/renamed when nonzero; exactly one bucket per porcelain line by delete > rename > add > modify priority, so the buckets always sum to the total), up to 5 first paths, and the FULL `git status --porcelain` output; the refusal wording is now e.g. "… refused: 133 entries (133 deleted, 0 modified, 0 untracked); first paths: firmware/CMakeLists.txt, …". **(P2)** the force exit exists everywhere the delete does: `POST /api/worktrees/delete` takes `force`, the MCP `worktree_delete` verb takes `force`, and the CLI grew `myworktree worktree delete --force <id>`; force passes `--force` to `git worktree remove`. **(P3)** the refusal separately counts the gitignored entries (`git status --porcelain --ignored`, the "!!" lines the blocking check never sees — 89 gitignored files, 64 of them single-copy, would have silently vanished in the report) and warns that force destroys them unrecoverably. API: the refusal answers **409** with the contract code `worktree_dirty` + `message` (one-line summary for text clients) + the structured `dirty` breakdown. UI: `deleteWorktree` matches the code and opens a dialog with the summary, a collapsible full git status output, the gitignored-file danger warning, and a two-step-confirm Force delete button that resends with `force: true`. The issue's P4 side observation (state.json's recorded branch can lag a worktree-internal `git switch`) needed no change: `Manager.List` already re-resolves the live branch per worktree for display, and the stored value is only a fallback. Pinned by `TestDeleteDirtyRefusalCarriesDetails` / `TestDeleteForceRemovesDirtyWorktree` (real-git integration), `TestDirtyWorktreeErrorSummarizePorcelain` / `TestCountIgnoredPorcelain` / `TestDirtyWorktreeErrorMessage` (unit), `TestHandleWorktreeDeleteDirtyRefusalIsStructured` / `TestHandleWorktreeDeleteForceDeletes` (API contract), and `TestWorktreeDirtyDeleteDialog` (UI anchors). Contract documented in `docs/API.md`. Review follow-ups (two independent reviews, both verified locally before fixing): **(R1)** `myworktree worktree delete <id> --force` silently dropped the flag — Go's `flag` parsing stops at the first positional — so the CLI now scans for `--force`/`-force` manually and rejects extra positionals with a usage error; **(R2)** `force` no longer gates on `git status` — the probe only builds the refusal, and on a damaged worktree (corrupt index) it fails while `git worktree remove --force` still succeeds, so force skips it entirely (pinned by `TestDeleteForceSkipsStatusProbe`); **(R3)** the MCP `worktree_delete` dirty refusal now answers the same 409 + `worktree_dirty` + structured `dirty` breakdown as the HTTP API (was a flat 400; pinned by `TestHandleMCPWorktreeDeleteDirtyRefusalIsStructured`); **(R4)** a worktree whose only at-risk contents are gitignored files still deletes without a refusal, but the delete now returns the destroyed gitignored count — surfaced as `ignored_destroyed` in the API/MCP success responses, a post-delete dashboard notice, and a CLI stderr note (pinned by `TestDeleteCleanWorktreeReportsIgnoredDestroyed` / `TestHandleWorktreeDeleteCleanReportsIgnoredDestroyed`); **(R5)** both status probes run with `-uall` so untracked/ignored directories count file-by-file instead of collapsing to one entry, and with `-c core.quotePath=false` so non-ASCII paths stay literal (pinned by `TestDeleteUntrackedDirCountsFilesIndividually` / `TestDeleteRefusalKeepsNonASCIIPathsLiteral`); the carried `porcelain` payload is capped at 200 lines with a truncation marker. UI nits from the same round: a failed force delete shows the body's human `message` instead of the raw `worktree_dirty` code, and Esc closing the dialog resets its dangling state via a `cancel` listener. Second review round (both reviews confirmed every first-round fix and found no new defect; their remaining nits): `ignored_destroyed` is now present ONLY when counted — a forced success omits the field rather than answering a 0 that read as "none destroyed" while meaning "not counted"; the CLI's hand-rolled scan also honors the `--force=<bool>` spelling and the `--` terminator, with a garbage value failing loudly (`TestRunWorktreeDeleteForceEqualsForms` / `_InvalidForceValueFails` / `_DashDashTerminator`); and the doc-comment/pluralization nits around the 200-line porcelain cap are fixed (`TestCapDirtyPorcelain`).
 - **fix(terminal): the TEXT `{"type":"ping"}` heartbeat is now opt-in via a handshake capability, so stale pre-refresh pages no longer paint it into the terminal every 10 s (issue #98)** — reported on v0.5.1: SOME terminal tabs showed a literal `{"type":"ping"}` line every 10 s while others on the same daemon never did; refreshing an affected tab made it stop. The split was each page's LOAD TIME, not the instance: #83 had introduced the TEXT heartbeat and the client's `parseTTYControlMessage` whitelist in the same commit, so a tab opened on v0.5.0 (whose whitelist has only `ready`/`resize`) and never refreshed ran old JS against the v0.5.1 daemon, and every heartbeat text frame fell through to the paint-everything branch. The root cause is a design orientation, not the ping frame itself: any non-whitelisted text frame is terminal output, so EVERY future server control frame would leak the same way onto stale pages — and already-open tabs never re-fetch JS, so no Cache-Control header can reach them. Fix (the issue's preferred direction): the TEXT heartbeat now rides behind an explicit client capability. The tty/ws handshake accepts `caps=<comma-list>`; the server sends the `{"type":"ping"}` text frame ONLY when the list carries `ping` as a WHOLE token (`ttyClientHasCap` — `pinger` is not a match), and `connectTTY` declares `&caps=ping,sync` (its whitelist already handles `ping`/`pong`/`sync`). A client that does not opt in — the stale v0.5.0 page, any non-browser client — degrades to the pre-#83 behaviour it always had: the RFC 6455 ping still flows to everyone (`onmessage` never fires for control frames, so nothing is ever painted), only the observable text mirror is withheld. An old daemon ignores the unknown parameter, so the version mix is safe in both directions, and the client-probe `{"type":"ping"}` → `{"type":"pong"}` round trip is unchanged (a client that sends a probe understands the answer). The same leak class had a second, lower-rate carrier, closed in the same pass: the `{"type":"sync"}` offset echo (#87) post-dates the v0.5.0 whitelist too, so an un-refreshed v0.5.0 page painted it into the terminal once per connect; the echo now rides behind `caps=sync` OR an inferred opt-in — any client presenting an explicit `since` cursor receives it, because the cursor parameter and the sync whitelist shipped in the same fix (#87; v0.5.0's client never sends `since` on this endpoint at all, so presenting one provably implies parsing the other). The bare caps gate alone would have frozen every pre-caps v0.5.1 page's cursor — the `handshakeSynced` latch never sets without the echo — and re-replayed output on every reconnect until a manual refresh, neither bounded to a tick nor self-healing on the live paths (the poll's `ensureTerminalLiveTransport` deliberately never re-runs `loadLog`); review round 2 caught this before merge. The residual degradation shrinks to a pre-caps page whose cursor is still the unknown/zero sentinel: it sends no `since`, gets no echo, and its reconnects re-replay the ≤64KB tail until a loadLog re-pin or refresh lands a cursor — the pre-#87 behaviour that page shipped with. In `since=-2` mode the echo is the ONLY cursor handoff (the replay is empty by design), and the explicit-`since` inference keeps it flowing there too. The `caps` parameter itself may be repeated (`?caps=pong&caps=ping` — every value is scanned, not `url.Values.Get`'s first only), and matching stays whole-token and case-sensitive (`caps=PING` opts nothing in). Pinned by `TestHandleInstanceTTYWS_HeartbeatTextPingRequiresOptIn` (no caps / `caps=pong` / `caps=pinger` / `caps=PING` receive zero text pings over 6+ heartbeat ticks while the RFC pings keep flowing; `caps=resize,ping` and the repeated `caps=pong&caps=ping` opt in), `TestHandleInstanceTTYWS_SyncFrameRequiresOptIn` (no caps / `caps=ping` / `caps=SYNC` receive the binary replay but never a sync frame; `caps=sync` and the repeated `caps=pong&caps=sync` receive it — as do explicit-`since` clients via the inferred #87-era opt-in: `since=5`, `since=-2`, even unparsable), the `caps=ping,sync` dials in `TestHandleInstanceTTYWS_HeartbeatEmitsPingControlFrames` / `_ResponsivePeerSurvivesReadDeadline`, and the two-sided source anchors in `TestTerminalStatusLivenessHeartbeatContract`. Contract documented in `docs/API.md` (TTY WS). Upgrade note for the release: tabs opened before the upgrade still need ONE refresh to pick up the whitelisted client — say so in the release notes.
 - **fix(ui): switching to an instance-less worktree left the previous worktree's dsh-web iframe on screen, covering the empty state (issue #101)** — with a dsh-web instance active on worktree A, selecting a worktree B with ZERO instances kept A's iframe visible; because `#dsh-panel` sits after `#empty-state` in the DOM, it painted over the "No instances" hint, the UI read as B while keystrokes still reached A's session — an invitation to run commands in the wrong worktree. Three defects combined, all required: (A) `renderWorkspace()`'s no-active-instance branch hid only `#opencode-panel` / `#reasonix-panel` and forgot `#dsh-panel` — whose CSS keeps it full-size until `[hidden]` is set — while the only line hiding it lived in `selectInstance()`, unreachable from the worktree-switch path; (B) `selectWorktree()` never called the previous renderer's `deactivate()` — `maybeDestroyInactiveStoppedSession`'s terminal-bucket guard returns early for web instances — so the panel stayed visible AND the old instance's ready/scope polling kept running in the background after the switch; (C) `selectInstance(null)` early-returns on the empty-worktree path (`selectWorktree` had already nulled `activeInst`), so its panel-hiding lines never execute there — correct de-duplication, but it meant the path could no longer be relied on for cleanup. Fix: one `hideWebPanels()` helper hides ALL three panels as the single source of truth — shared by `renderWorkspace` (the empty branch AND the non-web active branch, which also lacked the dsh hide) and `selectInstance` — so the next web kind cannot be forgotten by one of the call sites; and `selectWorktree` now deactivates the previous instance's renderer unconditionally (deliberately WITHOUT `selectInstance`'s `prevRenderer !== renderer` guard: a worktree switch is always a full leave, never a same-renderer re-activation), which also stops the leaked polling. opencode-web / reasonix were only accidentally correct on this path and are now covered by the same helper; `selectInstance(null)`'s early return (defect C) is left alone by design. Pinned by `TestEmptyWorktreeSwitchHidesAllWebPanels` — asset-text assertions in the style of `TestWebRenderersStoppedSwitchHidesAllFrames` covering the helper's three panel ids, both `renderWorkspace` call sites, the shared-helper call in `selectInstance`, and the guard-less `deactivate()` in `selectWorktree`. Review follow-ups (second round): the leave cleanup is now a shared `deactivateInstanceRenderer()` helper — kind-agnostic and unconditional, skipping an unresolvable id (concurrent deletion) instead of falling back to `'pty'` and deactivating the wrong renderer — used by BOTH worktree-switch entry points: `selectWorktreeByID` (the create/import flows) previously set `activeWT` and fired an async `refresh()` without deactivating the previous renderer or clearing `activeInst`, so the old worktree's web iframe stayed visible — and kept polling — over the new worktree until the refresh round-trip finished; it now deactivates, clears the selection and re-renders synchronously before refreshing. The test pins the deactivate BEFORE the `state.activeWT = id;` / `render();` sequencing in both entry points (position, not mere presence — a deactivate after render() flashes the empty state over the still-visible iframe), and the function slicer derives its end-marker indentation from the declaration line instead of a hardcoded 8 spaces, so a re-indented script block fails loudly rather than silently extending every slice to end-of-file.

RELEASING.md

@@ -48,7 +48,7 @@ One commit, `chore(release): prepare vX.Y.Z`, touching **only**:
 
 | File | Change |
 |---|---|
-| `CHANGELOG.md` | Rename `## Unreleased` → `## vX.Y.Z (YYYY-MM-DD)` with a one-line release summary under it; keep an empty `## Unreleased` placeholder at the top. **Audit the delta for PRs that shipped without a CHANGELOG entry** (`git log --oneline <prev-tag>..develop` / merged PR list) and add the missing entries in house style (bold title — em-dash — detail — `Pinned by TestName`). |
+| `CHANGELOG.md` | Rename `## Unreleased` → `## vX.Y.Z (YYYY-MM-DD)` with a one-line release summary under it; keep an empty `## Unreleased` placeholder at the top. **Audit the delta for PRs that shipped without a CHANGELOG entry** (`git log --oneline <prev-tag>..develop` / merged PR list) and add the missing entries in house style (bold title — em-dash — detail — `Pinned by TestName`). House rule (issue #95, also in an HTML comment at the top of the file): the phrase "Pinned by N `node --test` cases" declares the TOTAL case count of `internal/ui/testdata/terminal_status.test.mjs` and EVERY occurrence must equal it — when the count grows, bump them all; use different phrasing for subset counts. |
 | `README.md` | Bump every `vA.B.C` string: download URLs, example tarball names, `-v` pin example, "current recommended public release" wording. |
 | `README.zh-CN.md` | Same sweep as `README.md`. |
 | `scripts/install.sh` | Bump the `-v` example version strings in the header/usage comments. |

internal/ui/terminal_status_test.go

@@ -36,6 +36,18 @@ func TestTerminalStatusHandling(t *testing.T) {
 // a file holding 27) — a hand-maintained count in prose is a claim nobody
 // checks. Derive it here instead: count the cases, then require the entry to
 // say the same thing.
+//
+// Issue #95: EVERY occurrence of the claim phrase must equal the derived
+// count, not just the first match. The previous first-match lookup silently
+// re-anchored the guard to whichever entry happened to sit highest in the
+// file (entries are prepended in reverse chronological order), so a new
+// entry quoting the phrase with a stale number would both hijack the
+// constraint and leave the real claim unchecked. Requiring every occurrence
+// to match makes the phrase position-independent — it now means "the file's
+// total case count" wherever it appears (subset counts use different
+// phrasing, e.g. #83's "fourteen new #83 cases"). Of the issue's two
+// proposals this was chosen over per-entry anchoring as the simpler
+// invariant to keep unambiguous as entries accumulate.
 func TestTerminalStatusChangelogCount(t *testing.T) {
 	src, err := os.ReadFile("testdata/terminal_status.test.mjs")
 	if err != nil {
@@ -50,13 +62,18 @@ func TestTerminalStatusChangelogCount(t *testing.T) {
 	if err != nil {
 		t.Fatalf("read CHANGELOG.md: %v", err)
 	}
-	claimed := regexp.MustCompile(`Pinned by (\d+) ` + "`" + `node --test` + "`" + ` cases`).FindSubmatch(changelog)
-	if claimed == nil {
+	claims := regexp.MustCompile(`Pinned by (\d+) `+"`"+`node --test`+"`"+` cases`).FindAllSubmatchIndex(changelog, -1)
+	if len(claims) == 0 {
 		t.Fatal("CHANGELOG.md no longer says \"Pinned by N `node --test` cases\"")
 	}
-	if got, err := strconv.Atoi(string(claimed[1])); err != nil || got != cases {
-		t.Fatalf("CHANGELOG.md claims %s `node --test` cases but testdata/terminal_status.test.mjs has %d",
-			claimed[1], cases)
+	for _, loc := range claims {
+		num := changelog[loc[2]:loc[3]]
+		// Report the line so a multi-occurrence violation needs no grep.
+		line := 1 + strings.Count(string(changelog[:loc[0]]), "\n")
+		if got, err := strconv.Atoi(string(num)); err != nil || got != cases {
+			t.Errorf("CHANGELOG.md:%d claims %s `node --test` cases but testdata/terminal_status.test.mjs has %d — EVERY occurrence of the phrase must state the file's total (issue #95)",
+				line, num, cases)
+		}
 	}
 }