chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767)

[cliffhall]

Closes #1785
Closes #1787
Closes #1789
Closes #1790
Closes #1767

Bundles five small, related tidy / quality-gate items into one PR (per request, to reduce ticket sprawl). All prettier-only / gate-coverage / dead-code — no runtime behavior change. npm run ci is green.

What's in it

#1785 — remove dead src/types/navigation.ts

InspectorTab had zero importers and had drifted from the live InspectorTabId in utils/inspectorTabs.ts (different case + missing Network). Deleted. src/types/ now holds only ambient .d.ts stubs, making the "#1776 no-domain-types-in-src/types/" framing exactly true.

#1787 — gate src/hooks/** + test the two untested hooks

src/hooks/ was not in the web coverage include, so its four modules were outside the ≥90 gate silently, and two had no tests. Added src/hooks/**/*.{ts,tsx} to include and wrote dedicated tests:

• useServerJsonImport — 94.4% branches, 100% lines/functions/statements
• useImportClientConfig — 90.6% branches, 100% lines/functions/statements
• (the two already-tested hooks are now gated too, both 100%)

#1789 — format-gate the remaining ungated first-party TS/JS

• Widened clients/{cli,tui,launcher} format/format:check to include their top-level configs (*.{ts,js} / *.{ts,tsx,js}), matching what clients/web does; also widened clients/web to include .storybook and clients/launcher to include its nested scripts/ (the *. top-level glob is non-recursive, so nested dirs are named explicitly).
• Added root format:shared / format:check:shared covering test-servers/src/**, vitest.shared.mts, and the repo-root eslint.config.js.
• Audited all 885 tracked first-party TS/TSX/JS/MJS/CJS/MTS files against the glob set — 0 ungated after this PR.

#1790 — pin prettier exactly

Dropped the ^3.8.1 caret for an exact 3.8.4 in all five package.jsons so the gate's verdict can't shift with an in-range patch bump (3.9.x reformats 10 already-gated files). Lockfiles synced — prettier-only, verified no other dep changed.

#1767 — root-file format and lint gate

Its web-server/ bullet shipped in #1788; its root-files bullet is closed here — both prettier (format:shared) and eslint (lint:shared, via a second files block in the root eslint.config.js scoped to Node globals). Its third bullet — cli/tui __tests__ typecheck — surfaces 76 type errors (mostly partial-mock casts) and is materially larger, so it's split out to #1791 rather than bloating this tidy bundle.

Verification

npm run ci green (validate → coverage ≥90 per-file gate → verify:build-gate → smoke → Storybook, 460 tests). The newly-covered files were prettier --write-normalized (pure formatting).

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 54s —— View job

---

Review — PR #1792

• Read PR diff (git diff origin/v2/main...HEAD)
• Review config / gate changes (chore: format-gate the remaining ungated first-party TS/JS (cli/tui/launcher configs, test-servers/src, vitest.shared.mts) #1789, chore: pin prettier to an exact version (the ^3.8.1 caret lets an in-range bump fail the format gate on untouched files) #1790, Close remaining quality-gate gaps: web/server + root-file format, cli/tui __tests__ typecheck #1767)
• Review new hook tests (test(web): src/hooks/ is outside the coverage gate — two hooks (useServerJsonImport, useImportClientConfig) have no tests #1787)
• Review dead-code removal + docs (refactor(web): remove dead src/types/navigation.ts InspectorTab, fold into utils/inspectorTabs #1785)
• Post review

Verdict: LGTM. The five items land cleanly and the "no runtime behavior change" claim holds — I verified every non-test, non-package.json edit is either prettier-only reformatting or gate/config wiring. CI on this branch is green (run 30186751543). Findings below are all minor/follow-up; nothing blocking.

---

Verified good

#1790 lockfile coherence. This was the one thing that could have silently broken npm ci: dropping the caret without touching the resolved entry. All five lockfiles already resolve node_modules/prettier → 3.8.4, so the exact pin is satisfied by every lockfile as-is. The diffs correctly touch only the devDependencies mirror. ✅

#1785 dead-code removal. src/types/navigation.ts has zero references anywhere in the repo (only prose mentions in specification/, plus the AGENTS.md line this PR updates). Safe delete. ✅

#1787 coverage include. src/hooks/ has exactly four modules and all four now have tests, so adding src/hooks/**/*.{ts,tsx} to include can't have left a straggler below the gate. ✅

Prettier-only reformats. test-servers/src/* and the client eslint.config.js / tsup.config.ts / vitest.config.ts / vitest.shared.mts diffs are quote style plus union-type line wrapping. The leading-pipe form (params.inputSchema as | Record<string, z.ZodType> | undefined) is valid TS and semantically identical. ✅

Timer hygiene in the new tests. useServerJsonImport.test.tsx afterEach does runOnlyPendingTimers() → useRealTimers(), which is the right guard against the #1760 timer-after-teardown class. Neither new test file uses renderWithMantine — correct, since these are pure hooks with no Mantine tree; the AGENTS.md rule targets component rendering. ✅

---

Findings

1. #1789 doesn't quite reach "the remaining ungated first-party TS/JS" — 4 files are still outside every format glob (minor).

I enumerated all 885 tracked TS/JS/MJS files against the post-PR glob set. These fall through:

clients/launcher/scripts/make-executable.js
clients/web/.storybook/main.ts
clients/web/.storybook/preview.tsx
clients/web/.storybook/vitest.setup.ts

clients/web's format is prettier --write src server "*.{ts,js}" — the *. glob is non-recursive, so .storybook/ is missed; and no client's format covers a nested scripts/ dir (the root format:scripts is scoped to the repo-root scripts/**).

This isn't hypothetical: all three .storybook files are currently non-conforming under the newly-pinned prettier 3.8.4 (verified locally — main.ts alone has quoted object keys, single quotes, and non-collapsed arrays). make-executable.js happens to be clean. Since the PR body and the AGENTS.md text this PR writes both frame #1789 as closing the ungated-first-party-JS/TS gap, it'd be worth either widening the two globs (prettier --write src server .storybook "*.{ts,js}" and launcher ... src __tests__ scripts "*.{ts,js}", with a --write pass for the .storybook churn) or stating the exclusion explicitly so the next audit doesn't re-discover it.

Fix this →

2. #1790's premise applies equally to the lint toolchain (optional follow-up).

Pinning prettier exactly so "the gate's verdict can't shift with an in-range patch bump" is right, but validate:core also runs lint:core + the new lint:shared, and eslint (^9.39.4/^9.39.5) and typescript-eslint (^8.56.1/^8.65.0) are still caret ranges — a minor bump there adds rules to configs.recommended and can fail the gate the same way. Not something to expand this PR for; worth a ticket if the exact-pin reasoning is meant to be a policy rather than a prettier-specific fix.

3. useImportClientConfig.test.tsx:267 — asymmetric assertion vs. its own comment.

// Each rename sees the other's target already claimed.
expect(result.current.renameErrors["beta"]).toContain("already in use");

The renameErrors loop (useImportClientConfig.ts:286-290) is symmetric, so renameErrors["alpha"] should also be set. Asserting only beta means a regression that made the check one-directional (e.g. dropping the other.id === conflict.id skip, or only walking forward) could still pass. One extra expect closes it.

Fix this →

4. useImportClientConfig.test.tsx:284 exercises a UI-unreachable branch — say so.

"renames to the original id when the rename target is blank" covers the res.renameTo.trim() || conflict.id fallback at useImportClientConfig.ts:238. That's a legitimate defensive-branch test, but the state it sets up (renameTo: " ") also sets renameErrors["alpha"], so canImport is false and the real UI can never reach runImport() there. A one-line comment ("only reachable programmatically — the UI blocks submit via canImport") stops a future reader from reading it as intended UX.

5. Timer constants duplicated between hook and test (minor maintainability).

useServerJsonImport.test.tsx hardcodes 300 / 1000 / 250, mirroring the module-private VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS. If any is tuned, typeAndFlush silently stops flushing the debounce and the assertions drift rather than failing loudly. Either export the constants or use vi.advanceTimersByTime(...) with values derived from them (or runAllTimers() where the exact boundary doesn't matter).

6. Doc staleness in specification/ (nit).

AGENTS.md was correctly moved to past tense, but specification/v2_ux_interfaces_plan.md:184 still reads "the navigation.ts InspectorTab here is dead and slated for removal", and line 85 still says "Place under clients/web/src/types/" — which now contradicts the settled rule. Same one-line update as AGENTS.md got.

---

Notes (no action needed)

• Extracting sharedRules and adding the second files block scoped to globals.node is the right shape, and self-linting eslint.config.js is a nice touch. The added test-servers/build/** globalIgnores entry is redundant against lint:shared's test-servers/src/** glob, but it's correct defense-in-depth if anyone ever runs a bare eslint . at the root.
• The tui format glob change from the explicit index.ts tui.tsx dev.ts list to *.{ts,tsx,js} is a strict superset (verified against the actual top-level file list), and it picks up eslint.config.js / tsup.config.ts / vitest.config.ts — good.
• I could not run npm run coverage locally (this checkout has no node_modules and installing four clients wasn't warranted for a review), so the per-hook ≥90 numbers in the PR body are taken as CI-verified via the green run above, which does include the coverage gate.
  · chore/tidy-1785-1787-1789-1790

[cliffhall]

Thanks — thorough as always, especially catching that the .storybook/ files were actually dirty under the pinned 3.8.4. Addressed in 1ce3b64:

1 — 4 files still ungated: Fixed. Widened clients/web format/format:check to include .storybook and clients/launcher to include scripts, and reformatted the three .storybook files (they were non-conforming). To make sure the gap is fully closed this time, I audited all 885 tracked TS/TSX/JS/MJS/CJS/MTS source files against pinned prettier — all clean.

3 — asymmetric assertion: Fixed. Now asserts renameErrors["alpha"] and ["beta"], so a one-directional regression in the (symmetric) collision loop fails.

4 — UI-unreachable branch: Added a comment noting the blank-rename test hits the res.renameTo.trim() || conflict.id fallback programmatically, unreachable in the UI (blocked by canImport).

5 — duplicated timer constants: Exported VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS from the hook and use them in the test — no more hardcoded 300/1000/250.

6 — stale spec doc: Un-staled both specification/v2_ux_interfaces_plan.md references (line 85 placement + line 184 "slated for removal") to past tense, matching the AGENTS.md update.

2 — eslint/typescript-eslint also caret: Deliberately not expanding this PR for it, and holding off on a ticket to avoid sprawl. The exact-pin here was the prettier-specific fix from #1790; if we want it to be a policy across the lint toolchain that's a separate decision — happy to file it if you'd like.

npm run ci green. Re-requesting review.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 41s —— View job

---

Review — PR #1792 (round 2)

• Read the follow-up commit (1ce3b64) delta
• Re-audit format-glob coverage (finding 1)
• Verify the test fixes (findings 3, 4, 5)
• Verify doc updates (finding 6)
• Post review

Verdict: LGTM — ship it. All five findings from round 1 are addressed (or, for #2, deliberately deferred with a reasonable rationale). One doc-sync nit below, non-blocking.

---

Re-verification of the round-1 findings

1 — ungated first-party files: independently confirmed closed. I re-ran the audit against the post-fix glob set rather than taking the claim on faith: enumerated all 885 tracked .ts/.tsx/.js/.jsx/.mjs/.cjs/.mts files and matched each against every format/format:check target (root core/**, scripts/**, the format:shared triple, and the four clients' dirs + non-recursive *.{ts,…} top-level globs). 0 ungated. The two glob widenings are exactly the right ones:

• clients/web: src server .storybook "*.{ts,js}" — picks up all three .storybook files (the *. glob is non-recursive, so naming the directory was required).
• clients/launcher: src __tests__ scripts "*.{ts,js}" — picks up scripts/make-executable.js.

And the three .storybook reformats are pure prettier output — quote style, object-key unquoting, array collapse/expansion, JSX attribute wrapping, trailing commas, and two missing final newlines. No semantic change; the // eslint-disable-next-line react-refresh/only-export-components directive and the axe test: "error" config are preserved verbatim. ✅

3 — symmetric assertion. useImportClientConfig.test.tsx:282-283 now asserts both alpha and beta, and I re-read the loop (useImportClientConfig.ts:262-295) to confirm the premise: taken is rebuilt per conflict and the other.id === conflict.id skip makes it genuinely symmetric, so both keys must be populated. A one-directional regression now fails. ✅

4 — UI-unreachable branch documented. The comment at useImportClientConfig.test.tsx:299-302 names the exact branch (res.renameTo.trim() || conflict.id) and why the UI can't reach it (canImport false). ✅

5 — timer constants. VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS are now exported from useServerJsonImport.ts:19,25,31 with doc comments and consumed by the test — no hardcoded 300/1000/250 remain. Exporting non-component values from a hook module doesn't trip react-refresh/only-export-components (no component exports in the file), so no lint churn. ✅

6 — spec doc. Both specification/v2_ux_interfaces_plan.md references are past-tense now (line 85's placement row correctly points at InspectorTabId, line 184's note drops "slated for removal"). ✅

2 — eslint/typescript-eslint carets. Declining to expand scope here is the right call; the policy question is genuinely separable from #1790's prettier-specific fix. No objection to leaving it unfiled.

Also re-verified

• Lockfile coherence after the exact pin: all five package-lock.jsons carry devDependencies.prettier: "3.8.4" and resolve node_modules/prettier → 3.8.4, so npm ci is satisfied everywhere with no re-resolution needed.
• Coverage include completeness: src/hooks/ holds exactly four modules, no index.ts, and all four have co-located tests; **/*.test.{ts,tsx} is in coverage.exclude, so adding src/hooks/**/*.{ts,tsx} gates precisely the four sources.
• .storybook is eslint-covered by clients/web's eslint . (files: ["**/*.{ts,tsx}"], no dot-dir ignore in flat config) — the pre-existing disable directive in preview.tsx is the proof. So the new prettier coverage doesn't leave a lint asymmetry there.

---

Findings

1. AGENTS.md now under-describes the format surface it just widened (nit, doc-sync).

AGENTS.md:222 still reads:

clients/web's format/format:check covers src, server, and its top-level configs (*.{ts,js} …), not just src

That was accurate before 1ce3b64 and is now stale — .storybook is missing from it, and nothing in AGENTS.md records that clients/launcher's glob gained scripts. Since the whole premise of #1789 is "the documented gate surface is the audit trail," this is exactly the line a future auditor would read to conclude .storybook is still ungated. One-line fix in the same sentence, plus a clause for launcher.

Fix this →

2. PR body's #1789 section describes the pre-fix state (nit).

The body lists only the cli/tui/launcher top-level-config widening and format:shared; it doesn't mention .storybook or launcher scripts. Worth a line before merge so the squashed commit message reflects what actually landed (the commit message for 1ce3b64 does cover it, but a squash-merge would use the PR body).

---

Notes (no action needed)

• clients/{web,cli,tui,launcher}/eslint.config.js are prettier-gated but not eslint-gated (each client's files glob is **/*.{ts,tsx} / {ts,mts}), which now contrasts slightly with the root eslint.config.js self-linting via the new lint:shared. Consistent with the documented "the scripts/ gate is prettier-only" stance and not worth churn — noting it only so the asymmetry is a known one rather than a rediscovery.
• I could not run prettier --check, the unit suite, or npm run coverage in this checkout (no node_modules, and installing four clients isn't warranted for a review). The glob/lockfile/logic audits above are static and self-contained; the prettier-clean and ≥90-coverage claims rest on the author's green npm run ci.
  · chore/tidy-1785-1787-1789-1790

[cliffhall]

Both round-2 nits fixed:

1 — AGENTS.md stale: Updated the format-surface sentence (line 222) to include .storybook for web and a clause for clients/launcher (src, __tests__, scripts, top-level configs), noting the non-recursive *. glob is why nested dirs are named explicitly. Swept both docs again — no other stale glob descriptions. (docs-only commit)

2 — PR body: Updated the #1789 section to describe what actually landed (the .storybook / launcher-scripts widenings + the 885-file "0 ungated" audit), so a squash-merge carries the accurate summary.

Thanks for the re-audit against the post-fix glob set — appreciate not taking the "0 ungated" claim on faith.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 21s —— View job

---

Review — PR #1792 (round 3)

• Read the round-2 follow-up commits (1ce3b64, b4d0424)
• Re-verify format-glob coverage claim (0 ungated)
• Re-verify doc sync (AGENTS.md / README.md / specification)
• Review the new hook tests and exported timer constants
• Post review

Verdict: LGTM — ship it. b4d0424 closes the last round-2 nit and I re-ran the format-coverage audit independently: 885 tracked source files, 0 ungated. Nothing blocking. Three follow-up observations below, all about making this PR's one-shot audit durable — none of them belong in this PR.

---

Re-verified

AGENTS.md:222 now matches the globs. The sentence names .storybook for web and adds the launcher clause (src, __tests__, scripts, top-level configs) plus the "*. is non-recursive, so nested dirs are named explicitly" rationale. Checked against the actual scripts — accurate:

gate	glob
root format:shared	test-servers/src/**/*.ts vitest.shared.mts eslint.config.js
clients/web	src server .storybook "*.{ts,js}"
clients/cli	src __tests__ "*.{ts,js}"
clients/tui	src __tests__ "*.{ts,tsx,js}"
clients/launcher	src __tests__ scripts "*.{ts,js}"

0-ungated audit reproduced independently. I enumerated all 885 tracked .ts/.tsx/.js/.jsx/.mjs/.cjs/.mts/.cts files and matched each against the full post-PR glob set (root core/scripts/shared + the four clients' dirs and non-recursive top-level globs). 0 fall through. ✅

.storybook reformats are pure prettier output — quote style, object-key unquoting, array collapse, JSX attribute wrapping, two missing final newlines. The react-refresh/only-export-components directive and test: "error" a11y config survive verbatim. Same for test-servers/src/* and vitest.shared.mts (quote style + leading-pipe union wrapping, semantically identical). ✅

renameErrors symmetry premise re-checked at source. useImportClientConfig.ts:280-291 rebuilds taken per conflict and skips other.id === conflict.id, so both alpha and beta must be populated — the new two-sided assertion at useImportClientConfig.test.tsx:282-283 is load-bearing. ✅

Exported timer constants. VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS are exported at useServerJsonImport.ts:19,25,31 and consumed by the test; no 300/1000/250 literals remain. The file exports no components, so react-refresh/only-export-components doesn't fire. ✅

Coverage include addition is exactly right. src/hooks/ holds precisely four sources (useImportClientConfig, usePaginatedList, useScrollMemory, useServerJsonImport), all four have co-located tests, no index.ts, and **/*.test.{ts,tsx} is in coverage.exclude — so src/hooks/**/*.{ts,tsx} gates the four sources and nothing else. ✅

Root eslint.config.js second block. files: ["eslint.config.js"] has no slash, so it matches only the root file — it can't accidentally pull in clients/*/eslint.config.js and lint them under Node-only globals. languageOptions merges per-key with the parser from tseslint.configs.recommended, so the .js self-lint parses fine, and no-undef stays on for it (tseslint's eslint-recommended overrides are .ts-scoped) with globals.node supplying process et al. Correct as written. ✅

Spec doc + README. Both specification/v2_ux_interfaces_plan.md references are past tense (line 85 points at InspectorTabId, line 184 drops "slated for removal"); README's per-client-scripts paragraph gained the format:shared / lint:shared clause. ✅

---

Follow-up observations (not for this PR)

1. The 885-file audit has no durable guard — it's a point-in-time check. #1789's value is "0 ungated," but nothing keeps it at 0. The repo already has the right pattern for this exact problem: verify:build-gate exists specifically because a gate can silently stop gating. A scripts/verify-format-coverage.mjs that does what I just did in-tree — enumerate git ls-files source extensions, match against the glob set, exit non-zero on any miss — would turn this PR's audit into a standing check, and it's ~30 lines with no new deps.

File a follow-up →

2. The client top-level globs cover a narrower extension set than the root's. Root format:scripts uses *.{mjs,js,cjs,ts}; web/cli/launcher use *.{ts,js} and tui *.{ts,tsx,js}. Every top-level config today is .ts/.js so the audit passes — but a future clients/web/vitest.workspace.mts, a postcss.config.cjs, or a top-level .tsx in web would be ungated on arrival, silently. Widening all four to a uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} is a no-op today and removes the whole class. (This is the mechanism behind #1 — a script catches it, a wider glob prevents it.)

3. src/App.tsx and src/theme/** are still outside the coverage include — the #1787 class is only partly closed. AGENTS.md:258 documents the hazard (and this PR correctly updates it to note hooks was added), so this is known rather than a discovery — but it's worth quantifying since #1787 just proved the pattern:

• src/App.tsx is 4526 lines with a 2243-line App.test.tsx beside it, and is in no include entry — so the single largest module in the web client is tested but not gated.
• src/theme/** is 24 modules with zero tests, and they're not styling-declaration-only: 10 of them carry if (props.variant === …) branches (Card.ts has 7).

Same "whitelist include silently drops a directory" shape #1787 fixed for hooks/. Worth a ticket in its own right rather than expanding this one.

4. eslint asymmetry (noted, no action). The four clients/*/eslint.config.js are prettier-gated but not eslint-gated (each client's files glob is **/*.{ts,tsx}), while the root's now self-lints via lint:shared. Consistent with the documented "the scripts/ gate is prettier-only" stance — recording it so it stays a known asymmetry rather than a future rediscovery.

---

Note on verification limits

This checkout has no node_modules and no cached prettier binary, so I could not run prettier --check, the unit suite, or npm run coverage — installing four clients isn't warranted for a review. Everything above (glob matching, the 885-file enumeration, the renameErrors logic re-read, the prettier-only classification of each reformat diff, the eslint flat-config semantics) is static and self-contained; the prettier-clean and ≥90 per-file claims rest on the author's green npm run ci.
· chore/tidy-1785-1787-1789-1790

[cliffhall]

Thanks for the three rounds — and for independently reproducing the 885-file / 0-ungated audit rather than taking it on faith.

All four round-3 observations are follow-up-shaped (you flagged each as "not for this PR"), and they're good ideas — a standing verify-format-coverage.mjs (obs 1) is the natural durable version of this PR's one-shot audit, the uniform-extension client globs (obs 2) would prevent the class outright, and App.tsx (4526 lines, tested-but-ungated) + theme/** (obs 3) are the same whitelist-drop shape #1787 just fixed for hooks/. I'm holding off on filing them for now to avoid piling on more tickets, but they're captured here in the thread so they're not lost — happy to open issues for any/all if we want them tracked on the board.

Considering the review converged (three rounds, last two "ship it", no outstanding change-requests on this PR).

[cliffhall]

Rather than leave the round-3 observations as follow-ups, handled all three in-place (55ad7ab):

Obs 1 — no durable guard for the audit: Added scripts/verify-format-coverage.mjs, wired into npm run validate (runs first, before validate:core). It parses the format:check globs out of every package.json (single source of truth — no second list), enumerates all tracked source files via git ls-files, and exits non-zero listing any file no glob covers. Negative-tested: dropping .storybook from web's glob makes it fail with the 3 files named. This is the standing version of this PR's one-shot 885-file audit — the format-coverage analog of verify:build-gate.

Obs 2 — narrow extension set: Widened all client + root format globs to a uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs}, so a future top-level .mts/.cjs/.tsx is gated on arrival. No-op today; removes the class.

Obs 3 — App.tsx / theme/ outside include:** Measured it — theme/** was 21/23 files already ≥90 transitively, so I gated the directory and added Paper.test.tsx for the one laggard (Paper.ts, 62%→100% branches). App.tsx (~4.5k lines, ~42% branches) is a genuine dedicated effort, so I converted it from a silent omission into an explicit documented exclusion (comment on the coverage include array + AGENTS.md) rather than cram a low-value test dump. If it should be a tracked board issue instead, happy to file one.

npm run ci green — the new guard reports 885/885 tracked source files gated. Re-requesting review.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 5m 23s —— View job

---

Review — PR #1792 (round 4)

• Read the round-3 follow-up commit (55ad7ab) delta
• Scrutinize scripts/verify-format-coverage.mjs glob-matching logic (ran it; negative-tested it)
• Verify the uniform-glob widening + validate wiring
• Verify the theme/** gate + App.tsx documented exclusion + Paper.test.tsx
• Post review

Verdict: LGTM. The new guard is sound and I verified it empirically rather than by inspection — it runs clean (887/887 tracked source files gated) and it fails correctly when a glob narrows. Three findings below: one doc-sync (real, same class as the r2/r3 nits), one blind spot in the guard I confirmed by experiment, one comment-accuracy nit. None blocking.

⚠️ CI for 55ad7ab was still in progress while I reviewed (run 30203933439); b4d0424 was green. So the theme/** ≥90 numbers rest on your local npm run ci until that run lands — worth a glance before merge, since this is the commit that added a new directory to the gate.

---

Verified — the guard actually guards

Unlike previous rounds, this one is runnable without node_modules (pure node + git), so I exercised it:

Passes on the current tree. verify:format-coverage — OK: all 887 tracked source files are format-gated. (885 in the PR body + the 2 new test files.) ✅

Negative test: narrowing is caught. I removed .storybook from clients/web's format:check glob and re-ran — it named exactly the three files and exited 1:

verify:format-coverage — 3 tracked source file(s) are not covered by any prettier format glob:
  clients/web/.storybook/main.ts
  clients/web/.storybook/preview.tsx
  clients/web/.storybook/vitest.setup.ts

So the guard genuinely reproduces the round-1 finding it was written to prevent. ✅

Glob→RegExp translation is correct for all four shapes actually in use. I traced globToRegExp by hand against each:

shape	example	emitted	correct?
recursive **/	core/**/*.{ts,tsx}	^core\/(?:.*\/)?[^/]*\.(?:ts|tsx)$	✅ ((?:…)? allows zero dirs, matching fast-glob)
non-recursive top-level	clients/web/*.{ts,…}	[^/]* — doesn't cross /	✅ matches prettier's behavior
bare directory	src	f === rel || f.startsWith(rel + "/")	✅ (prettier recurses; every SOURCE_EXTENSIONS member is prettier-supported)
exact file	vitest.shared.mts	exact match	✅

No .prettierignore or prettier config anywhere in the repo (checked git ls-files), so the directory-recursion matchers can't over-claim coverage for a file prettier would actually skip — the one way this class of guard usually gets a false negative. ✅

Failure direction on a new client is safe. MANIFESTS is hardcoded, so adding clients/foo makes its files unmatched → the guard fails loudly and prompts the MANIFESTS update, rather than silently trusting an unread manifest. Right way round. ✅

format ↔ format:check symmetry holds. The guard reads only the --check side, so a --write glob that drifted narrower would leave npm run format unable to fix what CI flags. I diffed all five manifests' write/check pairs — identical in every one, including all four widenings in this commit. ✅

Uniform extension widening is a genuine no-op today — guard still reports 887/887 after it, confirming nothing new was pulled in and nothing dropped. And it's wired where it belongs: first in validate (fails in seconds, before any build), and CI runs npm run validate at main.yml:40, so it's CI-enforced. ✅

theme/** gate math checks out. src/theme/ holds 23 .ts/.tsx sources; **/*.test.{ts,tsx} and **/index.{ts,tsx} are both in coverage.exclude, so the new include entry gates precisely 22 modules (21 component theme files + theme.ts) — no straggler hiding behind the barrel. Paper.test.tsx renders through renderWithMantine (per AGENTS.md), covers default + all three variants, and asserts the classNames branch (paper-code), which is the right shape for a styles/classNames-callback file. ✅

---

Findings

1. The docs didn't follow the code — including the sentence round 2 fixed (moderate; same class as the r2/r3 nits).

This commit changed the glob strings and added a script; AGENTS.md/README weren't updated, and AGENTS.md's own maintenance rule covers exactly this ("when adding new commands … update the relevant sections"). Four concrete staleness points:

• verify:format-coverage is documented nowhere. Not in AGENTS.md, not in the README script table (which does have a verify:build-gate row — its closest sibling, and the pattern you explicitly modeled it on), not in the README scripts/ layout line ("install cascade, smokes, verify-build-gate, pack:verify"). Given the guard's whole purpose is "the documented gate surface is the audit trail," an undocumented guard is a odd gap.
• validate's ordering is now wrong in two places. AGENTS.md:219 — "validate runs the core/ gate first (validate:core)" — and README:143 — "Runs validate:core … first". verify:format-coverage now runs before it.
• AGENTS.md:217 and :220 quote the pre-widening globs: scripts/**/*.{mjs,js,cjs,ts} (now {ts,tsx,mts,cts,js,jsx,mjs,cjs}) and "the shared surface … is test-servers/src/**/*.ts" (now *.{ts,tsx,mts,cts}).
• AGENTS.md:222 re-staled. That's the sentence b4d0424 corrected in round 2; it still says web's top-level configs are covered by *.{ts,js}.

Fix this →

2. The guard trusts a format:check script it never confirms is run — verified blind spot (optional hardening).

prettierCheckArgs() walks Object.values(pkg.scripts) and harvests globs from any script containing prettier --check, with no check that the script is reachable from validate. So the guard proves "a glob exists that would cover this file", not "this file is checked by CI." I confirmed it's exploitable, not theoretical — renaming clients/web's format:check → format:check:UNUSED (nothing invokes it, so web's whole src/server/.storybook surface is no longer checked by anything) still gives:

verify:format-coverage — OK: all 887 tracked source files are format-gated.
exit=0

That's the same "a gate silently stops gating" failure mode the script was written to close, one level up. Cheapest fix that closes it: resolve npm run <name> references from each manifest's validate and only harvest globs from reachable scripts — or, minimally, assert each MANIFESTS entry's validate chain references its own format:check. Not a blocker (the realistic regression is glob narrowing, which is caught), but worth knowing the guarantee's exact edge.

Fix this →

3. App.tsx isn't the only ungated src file — the new comment says it is (nit).

clients/web/vite.config.ts:170-175 reads "src/App.tsx is deliberately NOT listed … Every other src directory is gated", and AGENTS.md now says "The one deliberate, documented exception is src/App.tsx". Two more non-test files fall outside every include entry:

• src/main.tsx — the React bootstrap (createRoot(...).render(...))
• src/index.ts — a #!/usr/bin/env node bin bootstrap that re-exports runWeb

Both are legitimate exclusions — they're the exact analog of clients/cli's src/index.ts, which AGENTS.md already calls out as "the only coverage exclusion." The issue is only that the sentence claiming a single exception is now the thing a future auditor would trust. Naming them as bootstrap exclusions makes the list complete, and it's cheap since you're already touching the comment. (The "every other src directory" phrasing is literally true — the gap is top-level files.)

Fix this →

---

Notes (no action needed)

• SOURCE_EXTENSIONS is a second list, which softly contradicts the header's "no second list to keep in sync" — but the inline comment at L26-28 is honest about it, and the list is a strict superset of every glob's extension set, so a new extension shows up as a loud failure (the file matches no glob) rather than a silent pass. Right trade.
• Tokenizer edge cases, all fail-loud: single-quoted globs ('src/**/*.ts') would keep their quotes and stop matching → false failure, not false pass; a --flag value pair would treat value as a path; […] character classes pass through into the regex unmapped. None occur today and every failure mode is in the safe direction.
• Out of the guard's scope by design: prettier also formats .md/.json/.css/.yml, and root *.md / package.json / .github/workflows/*.yml are gated by nothing. CSS under src is covered incidentally (directory recursion). Not a regression and clearly not this PR's remit — noting it so "0 ungated" is read as "0 ungated TS/JS."
• The guard doesn't ship. Root files allowlist is clients/*/build, clients/web/dist, scripts/install-clients.mjs — so git ls-files can never run outside a checkout in a consumer install. No graceful-degradation needed.
• Self-gated. scripts/verify-format-coverage.mjs is itself covered by format:check:scripts, and (per the documented "the scripts/ gate is prettier-only" stance) not eslint-gated — consistent with the rest of scripts/.
• Handling all three round-3 observations in-place instead of deferring them was the right call for obs 1 and 2 in particular: the guard converts a one-shot audit into an invariant, and the uniform globs remove the class the guard would otherwise have to catch after the fact.
  · chore/tidy-1785-1787-1789-1790

[cliffhall]

All three round-4 findings addressed (0fa0f1b):

2 — guard trusted an unwired format:check (verified blind spot): Hardened verify-format-coverage.mjs to only harvest globs from scripts transitively reachable from validate (following npm run <name> refs within each manifest). Re-ran your exact exploit — renaming clients/web's format:check → format:check:UNUSED now fails the guard (627 files flagged) instead of passing. So it now asserts "checked by CI", not just "a glob exists".

3 — comment claimed App.tsx was the only ungated src file: Added src/main.tsx and src/index.ts (browser createRoot render + bin runWeb re-export — the analog of cli's excluded src/index.ts) to the include-array comment and the AGENTS.md sentence, so the documented exclusion list is complete.

1 — doc staleness: Fixed all four points — documented verify:format-coverage (AGENTS.md + a README script-table row next to verify:build-gate + the scripts/ layout line), corrected the validate ordering in AGENTS.md:219 and README (it runs verify:format-coverage first now), and updated the quoted globs (scripts/** and the shared surface to the uniform extension set; the re-staled line 222). Swept both docs — no stale glob strings or ordering claims remain.

Also noted your CI-timing caveat: the run for this commit is green (coverage gate incl. the new theme/** files passed). npm run ci green locally too.

Re-requesting review.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 28s —— View job

---

Review — PR #1792 (round 5)

• Read the round-4 follow-up commit (0fa0f1b) delta
• Re-verify the hardened verify-format-coverage.mjs (reachability logic + re-run the exploit)
• Verify the doc fixes (all four staleness points from r4 finding 1)
• Verify the completed exclusion list (main.tsx / index.ts)
• Post review

Verdict: LGTM — ship it. All three round-4 findings are addressed, and I re-ran the exploit rather than trusting the claim: the round-4 blind spot is genuinely closed. Two small findings below (one eslint/config asymmetry, one doc overstatement) plus one residual guard edge — none blocking.

⚠️ CI for 0fa0f1b is in progress as I write (run 30204399006). This commit touches only the guard, the vite-config comment, and docs, so the coverage/format gates are unchanged from 55ad7ab — but worth a glance before merge since verify-format-coverage.mjs itself is prettier-gated and I can't run prettier --check here.

---

Round-4 findings — re-verified

Finding 2 (guard trusted an unwired format:check) — closed, verified by re-running the exploit. I reproduced my own round-4 attack against the hardened script:

$ # rename clients/web format:check → format:check:UNUSED, drop it from web's validate
$ node scripts/verify-format-coverage.mjs
verify:format-coverage — 627 tracked source file(s) are not covered by any prettier format glob:
  clients/web/.storybook/main.ts
  clients/web/.storybook/preview.tsx
  clients/web/.storybook/vitest.setup.ts
  clients/web/eslint.config.js
  …

Previously this exited 0. The guard now asserts "checked by CI", not "a glob exists." On the clean tree it still passes — 887/887. ✅

I also traced the reachableScripts BFS for failure direction, which matters more than the happy path:

• Root: validate → verify:format-coverage, validate:core → format:check:{core,scripts,shared}. All three harvested. ✅
• Clients: entry validate → format:check in all four. ✅
• A manifest with no validate, or one whose validate used a form the regex doesn't understand (npm --prefix … run validate, npm-run-all format:check) → zero globs harvested → guard fails loudly. Fails safe in every case I could construct. ✅

Finding 3 (incomplete exclusion list) — closed. clients/web/src has exactly four top-level files (App.tsx, App.test.tsx, index.ts, main.tsx); the comment at clients/web/vite.config.ts:170-179 and the AGENTS.md sentence now name all three non-test ones with their rationale. The list is complete as of this tree. ✅

Finding 1 (doc staleness) — all four points closed. Verified each against the actual scripts: verify:format-coverage is now in AGENTS.md, the README script table (next to verify:build-gate), and the README scripts/ layout line; the validate ordering is corrected in AGENTS.md:217 and README:143; the scripts/** and shared-surface globs are updated; and AGENTS.md:222 (the line round 2 fixed and round 4 found re-staled) now quotes the uniform set. I grepped both docs and specification/ for the old glob strings — none remain. ✅

Also re-verified: format ↔ format:check pairs are still identical in all five manifests after the r3 widening (a write-side glob drifting narrower would leave npm run format unable to fix what CI flags); no tracked source file is .gitignored (prettier 3 honors .gitignore by default, so a tracked-but-ignored file would be a false pass for the bare-directory matchers — there are none); src/theme/ gates exactly 22 modules (21 component files + theme.ts; index.ts/*.test.tsx excluded), and theme/assets/ holds only SVGs. ✅

---

Findings

1. lint:shared's glob was widened but the eslint config's files block wasn't — a .mts/.cts test-server file would be prettier-gated but not lint-gated (minor).

55ad7ab widened the script:

"lint:shared": "eslint \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js"

but eslint.config.js:48 still matches only:

files: ["test-servers/src/**/*.ts", "vitest.shared.mts", "eslint.config.js"],

So a future test-servers/src/foo.mts is handed to eslint by the CLI glob while matching no config object — it ends up with no rules applied (or an "no matching configuration" notice), not rule-gated. And nothing catches it: verify:format-coverage is prettier-only by design, so the lint side has no equivalent guard. One-word fix — mirror the script's extension set in the files array (test-servers/src/**/*.{ts,tsx,mts,cts}). Cosmetic today (every file there is .ts), but it's precisely the silent-narrowing class this PR exists to close.

Fix this →

2. AGENTS.md now claims every format glob uses the uniform extension set — two don't (nit).

AGENTS.md:216 reads:

Every format glob uses the uniform extension set *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} (#1792) so a new-extension file can't slip the gate

But format:check:core is core/**/*.{ts,tsx} and format:check:shared is test-servers/src/**/*.{ts,tsx,mts,cts} — and AGENTS.md:220 correctly quotes both of those narrower forms, so the doc contradicts itself four lines apart. The behavior is fine (the guard turns any resulting gap into a loud failure, which is arguably better than a blanket widen), but the sentence is the one a future auditor would trust. Scope it: "every client format glob," or "the uniform set where the surface can hold any extension; core//shared stay {ts,tsx}/{ts,tsx,mts,cts} and the guard catches any file that falls outside."

Fix this →

3. Residual guard edge, one level up from finding 2 (optional; the realistic case is now covered).

reachableScripts verifies reachability within a manifest, starting from that manifest's own validate. It doesn't verify that the root validate reaches each client's validate. Verified:

$ # remove `&& npm run validate:web` from the root validate
$ node scripts/verify-format-coverage.mjs
verify:format-coverage — OK: all 887 tracked source files are format-gated.

So web's whole surface can be dropped from the root chain and the guard still reports full coverage. This is much less likely than the round-4 case (dropping a whole client from validate is a conspicuous edit that also removes its lint/build/test), so I'd leave it — but if you want the invariant airtight, the cheap version is one assertion: each non-root MANIFESTS entry must be named in the root's reachable script set.

---

Notes (no action needed)

• Paper.test.tsx is the right shape for a styles/classNames-callback theme file: renders through renderWithMantine (per AGENTS.md), covers default + all three variants so both callbacks run on every branch, and asserts the paper-code class rather than just "it rendered." The header comment explains why the branches were uncovered (no single component exercises every variant), which is the useful part for a future reader.
• The guard doesn't ship — the root files allowlist is clients/*/build, clients/web/dist, scripts/install-clients.mjs, so git ls-files can never run in a consumer install. No graceful-degradation needed.
• SOURCE_EXTENSIONS is still a second list, softly against the header's "no second list to keep in sync" — but the inline comment at L26-28 is honest about it and the list is a superset of every glob's extension set, so a new extension surfaces as a loud failure rather than a silent pass. Right trade.
• Out of the guard's remit by design: prettier also formats .md/.json/.css/.yml, and root *.md / package.json / .github/workflows/*.yml are gated by nothing. Read "0 ungated" as "0 ungated TS/JS."
• Handling the round-3 observations in-place rather than deferring them was the right call, and the round-4 hardening is what turns the guard from "a glob exists" into a real invariant. Nice convergence.

Verification limits

This checkout has no node_modules, so I could not run prettier --check, eslint, the unit suite, or npm run coverage. What I did run: scripts/verify-format-coverage.mjs on the clean tree (887/887) and under two mutated-manifest negative tests (pure node + git, no deps), plus git ls-files audits for gitignored-but-tracked sources and root-level files. The prettier-clean and ≥90 coverage claims rest on the in-flight CI run above.
· chore/tidy-1785-1787-1789-1790

[cliffhall]

All three round-5 findings fixed (332eb61):

1 — lint:shared glob vs eslint files block: Widened the root eslint.config.js shared files block to test-servers/src/**/*.{ts,tsx,mts,cts} to match the script glob, so a future .mts/.cts there is lint-gated, not just prettier-gated.

2 — AGENTS.md overstated "every format glob uniform": Scoped the claim to client globs and noted core/ stays {ts,tsx} / shared stays {ts,tsx,mts,cts}, with verify:format-coverage as the backstop — removing the self-contradiction with the line four below it.

3 — residual guard edge (root → client reachability): Added the assertion — the guard now verifies the root validate chain actually invokes each client's validate (cd <dir> && npm run validate). Re-ran your exploit: dropping validate:web from the root chain now fails the guard (the root validate chain does not invoke 1 client validation(s): clients/web), where before it exited 0. Combined with the round-4 per-manifest reachability, the guard now verifies the full root → client validate → format:check chain end to end.

npm run ci green. Re-requesting review.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 59s —— View job

---

Review — PR #1792 (round 6)

• Read the round-5 follow-up commit (332eb61) delta
• Re-verify the airtight guard (root → client reachability) by re-running both exploits
• Verify the eslint files widening and the AGENTS.md scoping fix
• Probe for a residual hole in the guard
• Post review

Verdict: LGTM — ship it. All three round-5 findings are closed, and I re-ran the exploits rather than reading the diff and trusting it: the root→client assertion genuinely fires. One residual (symmetric to the one just fixed, on the write side) that I confirmed by experiment — optional, and not a reason to hold this PR after six rounds.

⚠️ CI for 332eb61 is in progress (run 30204753932). This commit touches the guard, the root eslint files array, and one AGENTS.md sentence — no coverage-surface change from 0fa0f1b — but verify-format-coverage.mjs is itself prettier- and (now) validate-gated, and I can't run prettier --check here, so give it a glance before merge.

---

Round-5 findings — re-verified empirically

Finding 3 (root → client reachability) — closed. Exploit re-run:

$ # remove `&& npm run validate:web` from the root `validate`
$ node scripts/verify-format-coverage.mjs
verify:format-coverage — the root `validate` chain does not invoke 1 client validation(s):

  clients/web (expected `cd clients/web && npm run validate`)
…
exit=1

Previously this exited 0. And the round-4 exploit still fails correctly (renaming web's format:check → format:check:UNUSED and dropping it from web's validate → 627 files flagged, exit 1). Clean tree: 887/887, exit 0. ✅

I also traced clientsUnreachedFromRoot's failure direction, which matters more than the happy path: it reads the root manifest's own validate-reachable command set, so a rename (validate:web → something else), a re-plumb to npm --prefix clients/web run validate, or a missing root validate all produce a loud failure, never a silent pass. Right way round. ✅

Finding 1 (lint:shared glob vs eslint files block) — closed. eslint.config.js:47 is now test-servers/src/**/*.{ts,tsx,mts,cts}, exactly matching the script glob, so a future .mts/.cts there gets sharedRules + tseslint.configs.recommended rather than falling through to "no matching configuration." Brace expansion is supported in flat-config files (minimatch), and the block's globals.node scoping is unchanged. lint:core's core/**/*.{ts,tsx} still matches its files block too — no sibling drift. ✅

Finding 2 (self-contradicting AGENTS.md claim) — closed. Line 217 now scopes the uniform set to client globs, names core/ as {ts,tsx} and shared as {ts,tsx,mts,cts} with the reason, and frames verify:format-coverage as the backstop "regardless of which glob was expected to catch it" — which is the accurate description of what the script actually asserts. That removes the contradiction with line 220 four lines below. I grepped both docs and specification/ for stale glob strings and ordering claims: none remain, and line 222 (the one that re-staled between r2 and r4) is still correct. ✅

---

Findings

1. The guard checks only the --check side — a write-glob narrowing is still silent (verified; optional).

Rounds 4–5 hardened the check side into a real invariant, but prettierCheckArgs() matches prettier --check only, and nothing verifies the root format chain reaches each client's format. Both halves of that are exploitable — I ran them:

$ # clients/web: format → "prettier --write src"   (check side untouched)
verify:format-coverage — OK: all 887 tracked source files are format-gated.   exit=0

$ # root: format → "npm run format:core"           (drops all four clients + scripts + shared)
verify:format-coverage — OK: all 887 tracked source files are format-gated.   exit=0

The consequence is a DX trap rather than a coverage hole: CI still flags the file, but npm run format no longer fixes it — so the documented remedy ("always run the auto-fixer first rather than letting format:check catch it", AGENTS.md:217) silently stops working and the failure looks unfixable. I diffed all five manifests' write/check pairs and they're identical today (including all of this PR's widenings), so this is prevention, not a live bug. Cheap closure, both mirrors of code that already exists: harvest prettier --write args the same way and assert per-manifest set equality with the --check args, plus a clientsUnreachedFromRoot-style assertion against the root format chain (cd <dir> && npm run format).

File a follow-up →

2. The lint side has no equivalent guard, and the r5 fix was a hand-maintained mirror (note, no action).

Finding 1 above's fix in 332eb61 works by keeping eslint.config.js's files array in sync with the lint:* script globs by hand — which is exactly the class of invariant this PR argued should be machine-checked. An eslint CLI glob wider than every config files block yields unlinted files with no error (flat config just applies no rules), so the mirror can re-drift as silently as the prettier globs did. Not worth expanding a six-round tidy PR for, and the scripts/ gate being prettier-only is already documented — recording it so the asymmetry stays known rather than rediscovered.

3. clientsUnreachedFromRoot matches by substring co-occurrence (nit, theoretical).

c.includes(\cd ${dir}`) && /npm run validate/.test(c)requires the two to appear in the *same command string*, not to be causally linked, and thecdmatch isn't segment-anchored — socd clients/webfoo && npm run validatewould satisfydir === "clients/web", and cd clients/web && npm run build && npm run validate:core` would too. Neither is reachable from any plausible edit, and every failure mode I could construct is in the safe direction. Mentioning it only so it isn't mistaken for an exact check.

---

Notes (no action needed)

• No .prettierignore, .prettierrc, or prettier.config.* anywhere in the repo (re-checked via git ls-files), so the bare-directory matchers (src, server, .storybook, scripts, __tests__) can't over-claim coverage for a file prettier would actually skip — the usual false-negative for this class of guard. The one gitignored-but-tracked file (.claude/settings.json) isn't a source extension, so it can't produce a false pass either.
• git ls-files *.ts … extension matching is exact, not suffix-sloppy: *.ts does not match vitest.shared.mts (the char before ts is m, not .), which is why mts/cts are listed separately in SOURCE_EXTENSIONS. The enumeration is what it claims to be.
• A new client fails loudly. MANIFESTS is hardcoded, so clients/foo/src/x.ts matches no matcher → the guard reports it ungated and prompts the MANIFESTS + root-chain update. Correct direction.
• Doc/script agreement re-verified end to end for all five manifests' format:check globs against AGENTS.md:217/220/222 and the README table — accurate as written.
• Six rounds is a lot for a tidy PR, but the trajectory was worth it: chore: format-gate the remaining ungated first-party TS/JS (cli/tui/launcher configs, test-servers/src, vitest.shared.mts) #1789 started as a one-shot 885-file audit and ends as a CI-enforced invariant that verifies the whole root validate → client validate → format:check → glob chain. Finding 1 above is the same idea applied to the write side; genuinely a follow-up, not a gap in what's here.

Verification limits

No node_modules in this checkout, so I could not run prettier --check, eslint, the unit suite, or npm run coverage. What I did run (pure node + git): the guard on the clean tree (887/887) and under four mutated-manifest negative tests — the two round-4/5 exploits (both now fail correctly) and the two write-side probes in finding 1 (both still pass) — plus git ls-files audits for prettier config, ignored-but-tracked sources, and extension matching. The prettier-clean and ≥90 coverage claims rest on the in-flight CI run above.
· chore/tidy-1785-1787-1789-1790

[cliffhall]

Thanks for six rounds. Round 6 verified all round-5 fixes by re-running the exploits — appreciate that.

The remaining items are all follow-up/no-action by your own framing:

• Finding 1 (write-side glob narrowing undetected): Real, but as you note it's prevention not a live bug — all five manifests' --write/--check glob pairs are identical today, and the effect is a DX trap (CI still flags; npm run format wouldn't auto-fix), not a coverage hole. It's the write-side mirror of the check-side invariant this PR built. Holding off on auto-filing to avoid ticket sprawl; captured here in the thread, and I'll open an issue for it if you'd like it tracked on the board.
• Findings 2 (lint side has no equivalent guard) & 3 (substring co-occurrence match): Noted as known, no action.

Considering the review converged — three consecutive "ship it" verdicts (r4/r5/r6), no outstanding change-requests on this PR, and every prior finding re-verified closed.