fix(overview): the card answers about a root it cannot read, and counts the rest #4

Merged
thisilike merged 9 commits from fix/overview-card-unreadable-roots into main 2026-09-02 16:09:08 +02:00
Owner

Note 1 from your review of #3, plus two things next to it that turned out to be
wrong the same way. Notes 2 and 3 are in here too; 4 and 5 are argued below
rather than changed.

1. The card could not say "unreadable" — and did not say nothing, either

probeRoots() settled what a root IS; the card still built its own rows out of
the probe and dropped ok and error. So an unreadable root reached the widget
as null usage, and null usage renders as free plus a 0%-full bar with 0%
next to it. That is not a missing answer, it is the rendering that means an
empty healthy mount — on the one surface nobody reads, only glances at.

rootInfos() in fsops.ts maps probes for both web surfaces now, so what a
caller is TOLD cannot drift the way what a caller ASKED FOR used to. /roots
was already correct and goes through it unchanged; /overview is a subset and
a total away from being the same route.

The card renders unreadable and the reason, and drops the bar.

2. "3 more roots" was counted from the slice

/overview probed policy.roots.slice(0, 4) and the widget computed the
hidden count from what it received, so with sixteen roots configured the card
said 1 more root while hiding thirteen. The route sends total now.

Pre-existing — b49df6f has the same slice — so it did not arrive with #3.

3. One probe per load was paid for a row nobody draws

The slice was four; the widget renders three. Every dashboard load stat'd and
df'd a fourth root that is never on screen, which on a spun-down disk costs
seconds. One constant, named for what the card can fit, decides both now.

Your notes 2 and 3

The tripwire. Inverted, as you said: every mention of policy.roots in a
route file must be a probeRoots() argument, with by-name lookups (.length,
.find(, [0], …) the exception, and the window is six lines because fmt puts
the call's arguments on their own lines. Verified by injecting each shape you
named into routes.ts:

shape [const rs = policy.roots;]                caught=1
shape [policy.roots.forEach((r) => r.name);]    caught=1
shape [for (\n  const r of policy.roots\n) {}]  caught=1

None of the three failed the old regex.

barrierDf. Now probeRoots(...).finally(df.done). One correction to the
note: df.done() already ran before the assertions, and the only path that
skipped it was a barrier timeout — where the timer had fired, so nothing leaked.
usageOf catches its exec rejection (fsops.ts:330), so probeRoots does not
reject either. The finally is worth having; it was not covering a live leak.

Your notes 4 and 5, not changed

4 — a missing root's not found. For a configured root the resolver's
404 can only mean the directory is absent: guard()'s "not inside any root"
spelling needs a path outside the roots, and a root is inside itself. The only
caller that passes a non-root is a test. A deny rule still carries its own
message. Happy to be shown a case where the two are confusable on /roots.

5 — the uncapped fan-out. Left as is. df is cheap, UsageCache collapses
repeats per path, and the burst is bounded by how many roots an operator
configured — which is now three for the card, and the listing genuinely wants
all of them at once.

Verification

deno test --allow-read --allow-write --allow-run=python3 backend/60
passed
, up from 58: the mapper keeps a failed probe's ok/error, and gives
each root its own scan in the configured order.

deno lint clean (33 files). deno check backend/mod.ts clean in a core
checkout, svelte-check 0 errors (the one warning is tsconfig.check.json's
baseUrl deprecation, on main too), and module-builder builds the frontend.
deno fmt --check: every line this branch adds is clean — the complaints in
routes.ts, fsops.ts, api.ts, contract_test.ts and CLAUDE.md's table
land on lines it does not touch, checked by diffing a formatted copy against the
branch's own hunks.

Against a running server — six roots, photos denied, gone absent, host root
/:

card media    ok=True  err=None      free=29753339904 pct=8
card gone     ok=False err=not found
card photos   ok=False err=refused by deny rule /…/srv/photos/**
total 6
web  media ok=True | photos ok=False (deny rule) | gone ok=False (not found) | films ok=True …
page media ok | photos unreadable | gone unreadable | films ok …

and the card itself, rendered headless in its real slot: media with its bar,
gone as unreadable / not found, photos as unreadable with the deny
rule ellipsised to the slot width, footer read/write · 3 more roots.

Note 1 from your review of #3, plus two things next to it that turned out to be wrong the same way. Notes 2 and 3 are in here too; 4 and 5 are argued below rather than changed. ## 1. The card could not say "unreadable" — and did not say nothing, either `probeRoots()` settled what a root IS; the card still built its own rows out of the probe and dropped `ok` and `error`. So an unreadable root reached the widget as null usage, and null usage renders as `—` free plus a 0%-full bar with `0%` next to it. That is not a missing answer, it is the rendering that means an empty healthy mount — on the one surface nobody reads, only glances at. `rootInfos()` in `fsops.ts` maps probes for both web surfaces now, so what a caller is TOLD cannot drift the way what a caller ASKED FOR used to. `/roots` was already correct and goes through it unchanged; `/overview` is a subset and a total away from being the same route. The card renders `unreadable` and the reason, and drops the bar. ## 2. "3 more roots" was counted from the slice `/overview` probed `policy.roots.slice(0, 4)` and the widget computed the hidden count from what it received, so with sixteen roots configured the card said **1 more root** while hiding thirteen. The route sends `total` now. Pre-existing — `b49df6f` has the same slice — so it did not arrive with #3. ## 3. One probe per load was paid for a row nobody draws The slice was four; the widget renders three. Every dashboard load stat'd and df'd a fourth root that is never on screen, which on a spun-down disk costs seconds. One constant, named for what the card can fit, decides both now. ## Your notes 2 and 3 **The tripwire.** Inverted, as you said: every mention of `policy.roots` in a route file must be a `probeRoots()` argument, with by-name lookups (`.length`, `.find(`, `[0]`, …) the exception, and the window is six lines because fmt puts the call's arguments on their own lines. Verified by injecting each shape you named into `routes.ts`: ``` shape [const rs = policy.roots;] caught=1 shape [policy.roots.forEach((r) => r.name);] caught=1 shape [for (\n const r of policy.roots\n) {}] caught=1 ``` None of the three failed the old regex. **`barrierDf`.** Now `probeRoots(...).finally(df.done)`. One correction to the note: `df.done()` already ran *before* the assertions, and the only path that skipped it was a barrier timeout — where the timer had fired, so nothing leaked. `usageOf` catches its exec rejection (`fsops.ts:330`), so `probeRoots` does not reject either. The `finally` is worth having; it was not covering a live leak. ## Your notes 4 and 5, not changed **4 — a missing root's `not found`.** For a *configured* root the resolver's 404 can only mean the directory is absent: `guard()`'s "not inside any root" spelling needs a path outside the roots, and a root is inside itself. The only caller that passes a non-root is a test. A deny rule still carries its own message. Happy to be shown a case where the two are confusable on `/roots`. **5 — the uncapped fan-out.** Left as is. `df` is cheap, `UsageCache` collapses repeats per path, and the burst is bounded by how many roots an operator configured — which is now three for the card, and the listing genuinely wants all of them at once. ## Verification `deno test --allow-read --allow-write --allow-run=python3 backend/` — **60 passed**, up from 58: the mapper keeps a failed probe's `ok`/`error`, and gives each root its own scan in the configured order. `deno lint` clean (33 files). `deno check backend/mod.ts` clean in a core checkout, `svelte-check` **0 errors** (the one warning is `tsconfig.check.json`'s `baseUrl` deprecation, on main too), and `module-builder` builds the frontend. `deno fmt --check`: every line this branch adds is clean — the complaints in `routes.ts`, `fsops.ts`, `api.ts`, `contract_test.ts` and `CLAUDE.md`'s table land on lines it does not touch, checked by diffing a formatted copy against the branch's own hunks. Against a running server — six roots, `photos` denied, `gone` absent, host root `/`: ``` card media ok=True err=None free=29753339904 pct=8 card gone ok=False err=not found card photos ok=False err=refused by deny rule /…/srv/photos/** total 6 web media ok=True | photos ok=False (deny rule) | gone ok=False (not found) | films ok=True … page media ok | photos unreadable | gone unreadable | films ok … ``` and the card itself, rendered headless in its real slot: `media` with its bar, `gone` as `unreadable` / `not found`, `photos` as `unreadable` with the deny rule ellipsised to the slot width, footer `read/write · 3 more roots`.
probeRoots() ended the disagreement about what a root IS, but the card built
its own rows from the probe and left `ok` and `error` out. A denied or missing
root therefore arrived at the widget as null usage, which it drew as "—" free
and a 0%-full bar: the rendering that means an empty healthy mount, on the one
surface an operator glances at rather than reads. rootInfos() maps the probes
for both web surfaces now, so what a caller is told cannot drift the way what a
caller asked for used to.

The card says "unreadable" and the reason under it instead, and drops the bar —
a 0% bar on a root nobody can read is worse than no bar.

Two more things the same route had wrong:

- "N more roots" counted the rows the card had been sent, and the route sent a
  slice, so an operator with sixteen roots was told about one. It counts the
  configured roots now.
- The slice was four while the card renders three, so every load paid for a
  stat and a df on a root nobody draws — on a spun-down disk, in seconds. One
  constant, named for the card, decides both.

roots_test.ts covers the mapper: a failed probe keeps its `ok` and `error`, and
each root gets its own scan in the configured order. Its barrier df now clears
its timer in a `finally`, so a body that throws before the assertions reports
the throw rather than a leaked timer.
It matched `for (… of policy.roots)` and `policy.roots.map(` — the two shapes
that had actually drifted. An alias (`const rs = policy.roots`), a `forEach`,
or a `for (` that fmt has split across lines walked straight past it, so the
comment above it promised more than the regex delivered.

Inverted: every mention of `policy.roots` in a route file must be a probeRoots()
argument, and asking how many there are or for one by name is the exception.
The call may be several lines up — the argument list is over fmt's column limit
— so the check looks in a window rather than on the line.

Verified by injecting each of the three shapes above into routes.ts: all three
now fail the test, none of them did before.
The README claimed the three surfaces "cannot disagree about whether a root is
readable", which held only because the card did not answer the question. It
answers now. Also states that the "N more" count is of the configured roots
rather than of the rows the card was sent.
julian left a comment

The fix itself is right and the tripwire holds — one blocker, one thing the PR's own argument applies to, two nits.

Blocker: total is a claim without a test

Bug 2 ("counted from the slice") is fixed by total: policy.roots.length in /overview, and bug 3 by OVERVIEW_ROOTS. Neither is asserted anywhere: no *_test.ts touches /overview, so the mutation total: probes.length — bug 2 reintroduced — passes all 60 tests. The new rootInfos tests cover bug 1 only. Add a route-level test: six roots configured, the response carries three rows and total === 6.

Also

  • OverviewWidget.svelte:73ok=true, usage=null (the no-coreutils case) still draws <ProgressBar value={0}>. That is the same "empty healthy mount" rendering this PR removes for unreadable roots, and README line ~130 promises "no usage bar rather than a wrong one" for exactly this case. Pre-existing, but the argument in your description is the argument against it: wrap the bar in {#if root.usage}.
  • roots_test.ts:168 — comment still says "the overview card renders four roots"; this PR makes it three.
  • contract_test.ts:93 — the window is index-6 .. index+1, so a policy.roots.map( up to six lines after any probeRoots( call also passes — e.g. one placed inside /overview's hono.json({...}). Lines above only, or require the mention to sit inside the probeRoots( argument list.

Notes 4 and 5 as argued: agreed, no change needed.

Verification (Windows box)

  • deno test --allow-read --allow-write --allow-run=python3 backend/: 48 passed, 12 failed on this branch; main: 46 passed, the same 12 failed (POSIX /srv roots, symlinks, python3 — environmental). Consistent with your 58 → 60.
  • deno lint: clean, 33 files.
  • deno fmt --check: clean on this branch and on main here, so the untouched-line complaints you saw are a deno version difference, not the branch.
  • Tripwire: injected const rs = policy.roots; into routes.ts, the contract test failed at that line. Claim holds.
  • svelte-check / module-builder not rerun here.
The fix itself is right and the tripwire holds — one blocker, one thing the PR's own argument applies to, two nits. ## Blocker: `total` is a claim without a test Bug 2 ("counted from the slice") is fixed by `total: policy.roots.length` in `/overview`, and bug 3 by `OVERVIEW_ROOTS`. Neither is asserted anywhere: no `*_test.ts` touches `/overview`, so the mutation `total: probes.length` — bug 2 reintroduced — passes all 60 tests. The new `rootInfos` tests cover bug 1 only. Add a route-level test: six roots configured, the response carries three rows and `total === 6`. ## Also - `OverviewWidget.svelte:73` — `ok=true, usage=null` (the no-coreutils case) still draws `<ProgressBar value={0}>`. That is the same "empty healthy mount" rendering this PR removes for unreadable roots, and README line ~130 promises "no usage bar rather than a wrong one" for exactly this case. Pre-existing, but the argument in your description is the argument against it: wrap the bar in `{#if root.usage}`. - `roots_test.ts:168` — comment still says "the overview card renders four roots"; this PR makes it three. - `contract_test.ts:93` — the window is `index-6 .. index+1`, so a `policy.roots.map(` up to six lines *after* any `probeRoots(` call also passes — e.g. one placed inside `/overview`'s `hono.json({...})`. Lines above only, or require the mention to sit inside the `probeRoots(` argument list. Notes 4 and 5 as argued: agreed, no change needed. ## Verification (Windows box) - `deno test --allow-read --allow-write --allow-run=python3 backend/`: 48 passed, 12 failed on this branch; main: 46 passed, the same 12 failed (POSIX `/srv` roots, symlinks, python3 — environmental). Consistent with your 58 → 60. - `deno lint`: clean, 33 files. - `deno fmt --check`: clean on this branch and on main here, so the untouched-line complaints you saw are a deno version difference, not the branch. - Tripwire: injected `const rs = policy.roots;` into `routes.ts`, the contract test failed at that line. Claim holds. - `svelte-check` / `module-builder` not rerun here.
@ -86,1 +91,3 @@
if (!walks) continue;
if (!line.includes("policy.roots") || lookup.test(line)) continue;
// the call may be several lines up: `probeRoots(c, usage, roots.slice())`
// is over fmt's column limit and lands one argument per line
Owner

Window reaches one line below and six above — a policy.roots.map( within six lines after a probeRoots( call (e.g. inside /overview's hono.json) still passes. Lines above only, or require the mention inside the argument list.

Window reaches one line below and six above — a `policy.roots.map(` within six lines *after* a `probeRoots(` call (e.g. inside `/overview`'s `hono.json`) still passes. Lines above only, or require the mention inside the argument list.
Owner

Stale: the card renders three now.

Stale: the card renders three now.
@ -667,0 +671,4 @@
// how many are configured, not how many were probed: the card counts the
// rest into "N more", and counting the slice told an operator with
// sixteen roots about one
total: policy.roots.length,
Owner

Not asserted by any test — total: probes.length (bug 2 reintroduced) passes all 60. Needs a route-level test: six roots, three rows, total === 6.

Not asserted by any test — `total: probes.length` (bug 2 reintroduced) passes all 60. Needs a route-level test: six roots, three rows, `total === 6`.
@ -68,2 +72,2 @@
{root.scan ? `${formatBytes(root.scan.bytes)} · ${ago(root.scan.finishedAtMs)}` : ago(null)}
</div>
{#if root.ok}
<ProgressBar value={root.usage?.usedPct ?? 0} />
Owner

ok=true, usage=null (no coreutils) still draws a 0%-full bar — the same rendering this PR removes for unreadable roots, and README promises no bar for this case. {#if root.usage} around the bar.

`ok=true, usage=null` (no coreutils) still draws a 0%-full bar — the same rendering this PR removes for unreadable roots, and README promises no bar for this case. `{#if root.usage}` around the bar.
`total: policy.roots.length` and `OVERVIEW_ROOTS` fixed the two counting bugs
and nothing held them there: no test reached `/overview`, so the mutation
`total: probes.length` — bug 2 straight back — passed all 60.

A route-level test cannot be written in this repo. `registerRoutes` imports
`refusalStatus` from `@opsdeck/sdk`, and the tests run in a bare clone with no
import map, so importing `routes.ts` fails before a request is made. The two
decisions move to where a test can reach them instead: `overviewRoots()` in
`fsops.ts`, next to the `probeRoots()` and `rootInfos()` it is made of, answers
the card's subset AND the configured total, and the route only serialises it.
That also takes the last root-shaped decision out of a route file, which is
what "no route builds a RootInfo" was already asking for.

Six roots configured, three rows named in the configured order, `total` 6, and
three df calls. Both mutations the review named now fail it: `total:
probes.length` and `OVERVIEW_ROOTS = 4`.

The stale comment in the subset test went with it — the card renders three.
`ok=true, usage=null` is a deployment without coreutils, and the card drew
`<ProgressBar value={0}>` for it: the same "empty healthy mount" rendering this
branch removes for an unreadable root, on the surface nobody reads and only
glances at. README line 130 has promised "no usage bar rather than a wrong one"
for exactly this case since before the branch.

The bar is now inside `{#if root.usage}`; the free-space column already said
"—" here, and the scan line below it never depended on df.
The window was `index-6 .. index+1`, and a window reaches both ways: a
`policy.roots.map(` up to six lines BELOW a `probeRoots(` call passed too —
inside the `hono.json({...})` that answers with the probes, which is precisely
where the drift this tripwire exists for would land.

"Near the call" only ever stood in for "inside the call", because fmt puts a
long call's arguments on their own lines. So the argument list is what is
matched now: every `probeRoots(` argument list is blanked out by a paren scan,
newlines and columns kept, and a `policy.roots` surviving that is the walk.

Both legitimate shapes still pass — the call on one line and split over four —
and every shape the review named is caught, wherever it sits relative to a
call: an alias, `forEach`, a multi-line `for (`, a spread, `flatMap`, `filter`.
By-name lookups stay the exception.
`overviewRoots()` joins the two helpers already listed under `fsops.ts`, and
the roots constraint now states the rule the contract test enforces: a route
file may name `policy.roots` inside a `probeRoots()` argument list or to look
one up by name, and nowhere else.
julian left a comment

The shipped fix is right, and the placement is the good part: rootInfos() and overviewRoots() sit next to probeRoots() because that is where a test can reach them without core's import map, and the route is left with nothing to get wrong. total fixes the count, the card stops drawing a 0%-full bar for a root nobody can read, and the fourth probe nobody rendered is gone. Your answers on notes 4 and 5 are fair; I have nothing to add to either. .finally(df.done) is safe — done is an arrow closure over timer, so the unbound reference is fine — and DirScanSummary survives in api.ts through the re-export, so the narrowed overview type leaves no dead import.

Requesting changes on two holes in the tripwire itself. Both are cases where the test passes while checking nothing, which is the one failure mode a tripwire cannot have: the shapes you injected all get caught today, and both of these would still catch them, so the injection run does not distinguish a working tripwire from a disabled one.

  1. withoutProbeArgs() has no bail-out when the parens never balance. Any probeRoots( whose ( does not close — a prose comment, a ) inside a string argument — blanks every remaining character in the file, and every policy.roots below it goes unchecked with no signal.
  2. The lookup regex is tested against the whole line, so one legitimate lookup anywhere on a line excuses a walk on that same line.

The rest are nits, take them or leave them: the widget's own row cap is gone, so the number that has to fit a fixed slot now lives only in the backend, and an unreadable root drops its scan line even though rootInfos() still carries it.

The shipped fix is right, and the placement is the good part: `rootInfos()` and `overviewRoots()` sit next to `probeRoots()` because that is where a test can reach them without core's import map, and the route is left with nothing to get wrong. `total` fixes the count, the card stops drawing a 0%-full bar for a root nobody can read, and the fourth probe nobody rendered is gone. Your answers on notes 4 and 5 are fair; I have nothing to add to either. `.finally(df.done)` is safe — `done` is an arrow closure over `timer`, so the unbound reference is fine — and `DirScanSummary` survives in `api.ts` through the re-export, so the narrowed overview type leaves no dead import. Requesting changes on two holes in the tripwire itself. Both are cases where the test passes while checking nothing, which is the one failure mode a tripwire cannot have: the shapes you injected all get caught today, and both of these would still catch them, so the injection run does not distinguish a working tripwire from a disabled one. 1. `withoutProbeArgs()` has no bail-out when the parens never balance. Any `probeRoots(` whose `(` does not close — a prose comment, a `)` inside a string argument — blanks every remaining character in the file, and every `policy.roots` below it goes unchecked with no signal. 2. The `lookup` regex is tested against the whole line, so one legitimate lookup anywhere on a line excuses a walk on that same line. The rest are nits, take them or leave them: the widget's own row cap is gone, so the number that has to fit a fixed slot now lives only in the backend, and an unreadable root drops its scan line even though `rootInfos()` still carries it.
@ -34,0 +47,4 @@
let at = source.indexOf(CALL);
while (at >= 0) {
let depth = 0;
for (let i = at + CALL.length - 1; i < chars.length; i++) {
Owner

No bail-out when the parens never balance. If the scan reaches chars.length with depth > 0, every character from the probeRoots( onward is blanked — the whole rest of the file — and the loop below then asserts against blank lines, so every policy.roots after that point passes unchecked with nothing printed.

Two ways in, both plausible in this repo: prose in a comment that names the call without closing it (// probeRoots(c, usage wrapped by fmt), or a ) inside a string argument. routes.ts already carries probeRoots() in a comment on line 653 — balanced today, so this is latent rather than live.

Make it loud instead of silent:

let depth = 0;
let i = at + CALL.length - 1;
for (; i < chars.length; i++) {
  if (chars[i] === "(") depth++;
  else if (chars[i] === ")") depth--;
  else if (chars[i] !== "\n") chars[i] = " ";
  if (depth === 0) break;
}
if (depth !== 0) {
  throw new Error(
    `unbalanced probeRoots( at offset ${at}: the tripwire cannot read this file`,
  );
}

A thrown error fails the test, which is the correct outcome — the source stopped being something this check can reason about.

No bail-out when the parens never balance. If the scan reaches `chars.length` with `depth > 0`, every character from the `probeRoots(` onward is blanked — the whole rest of the file — and the loop below then asserts against blank lines, so every `policy.roots` after that point passes unchecked with nothing printed. Two ways in, both plausible in this repo: prose in a comment that names the call without closing it (`// probeRoots(c, usage` wrapped by fmt), or a `)` inside a string argument. routes.ts already carries `probeRoots()` in a comment on line 653 — balanced today, so this is latent rather than live. Make it loud instead of silent: ```ts let depth = 0; let i = at + CALL.length - 1; for (; i < chars.length; i++) { if (chars[i] === "(") depth++; else if (chars[i] === ")") depth--; else if (chars[i] !== "\n") chars[i] = " "; if (depth === 0) break; } if (depth !== 0) { throw new Error( `unbalanced probeRoots( at offset ${at}: the tripwire cannot read this file`, ); } ``` A thrown error fails the test, which is the correct outcome — the source stopped being something this check can reason about.
@ -79,1 +112,4 @@
// Asking how many there are or for one by name is not describing them.
const lookup =
/policy\.roots\.(length\b|find\(|findIndex\(|findLast\(|some\(|every\(|includes\(|indexOf\(|at\()|policy\.roots\[/;
for (const file of ROUTE_FILES) {
Owner

Question rather than a change: some( and every( are in lookup, but they iterate. policy.roots.some((r) => { render(r); return false; }) is the walk this test exists to catch, spelled as a predicate. find( I would keep — it answers with one root, which is the by-name case the comment describes — but some/every seem to be on the wrong side of the line you drew. Narrowing to length|find|findIndex|findLast|indexOf|includes|at|[ costs nothing today, since no route file uses either.

Question rather than a change: `some(` and `every(` are in `lookup`, but they iterate. `policy.roots.some((r) => { render(r); return false; })` is the walk this test exists to catch, spelled as a predicate. `find(` I would keep — it answers with one root, which is the by-name case the comment describes — but `some`/`every` seem to be on the wrong side of the line you drew. Narrowing to `length|find|findIndex|findLast|indexOf|includes|at|[` costs nothing today, since no route file uses either.
@ -87,3 +122,3 @@
assert(
line.includes("probeRoots("),
!named.includes("policy.roots") || lookup.test(named),
`${file}:${index + 1} walks the roots itself:\n ${line.trim()}`,
Owner

lookup.test(named) matches anywhere on the line, so a line that does both passes on the strength of the half that is allowed:

const first = policy.roots[0];
const all = policy.roots;          // caught
const n = policy.roots.length, all = policy.roots;   // NOT caught

fmt keeps those on one line when they are short enough, and a const { length } = policy.roots next to an alias is the same shape. Subtract the allowed mentions rather than looking for one:

const LOOKUP = /policy\.roots(\.(length\b|find|findIndex|findLast|indexOf|includes|at)\(|\[)/g;
// …
assert(
  !named.replace(LOOKUP, "").includes("policy.roots"),
  `${file}:${index + 1} walks the roots itself:\n  ${line.trim()}`,
);

That also removes the need to keep the regex anchored to a single mention per line.

`lookup.test(named)` matches anywhere on the line, so a line that does both passes on the strength of the half that is allowed: ```ts const first = policy.roots[0]; const all = policy.roots; // caught const n = policy.roots.length, all = policy.roots; // NOT caught ``` fmt keeps those on one line when they are short enough, and a `const { length } = policy.roots` next to an alias is the same shape. Subtract the allowed mentions rather than looking for one: ```ts const LOOKUP = /policy\.roots(\.(length\b|find|findIndex|findLast|indexOf|includes|at)\(|\[)/g; // … assert( !named.replace(LOOKUP, "").includes("policy.roots"), `${file}:${index + 1} walks the roots itself:\n ${line.trim()}`, ); ``` That also removes the need to keep the regex anchored to a single mention per line.
@ -34,3 +36,2 @@
const shown = $derived(data?.roots.slice(0, 3) ?? []);
const hidden = $derived(Math.max(0, (data?.roots.length ?? 0) - shown.length));
const shown = $derived(data?.roots ?? []);
Owner

The widget's own slice(0, 3) is gone, so the number that has to satisfy a fixed ~260px slot is now only OVERVIEW_ROOTS in fsops.ts. The constant's doc comment states the constraint, but it states it in a file where nobody is looking at the slot: raise it to 6 and the card clips four rows with no error on either side, and the backend test asserts OVERVIEW_ROOTS against itself so it stays green.

Single source of truth is the right call — I would not send the number back to the frontend. But the rendering side could still refuse to overflow: data?.roots.slice(0, 3) ?? [] with hidden still counted from total keeps both fixes and puts the CSS constraint back next to the CSS.

The widget's own `slice(0, 3)` is gone, so the number that has to satisfy a fixed ~260px slot is now only `OVERVIEW_ROOTS` in `fsops.ts`. The constant's doc comment states the constraint, but it states it in a file where nobody is looking at the slot: raise it to 6 and the card clips four rows with no error on either side, and the backend test asserts `OVERVIEW_ROOTS` against itself so it stays green. Single source of truth is the right call — I would not send the number back to the frontend. But the rendering side could still refuse to overflow: `data?.roots.slice(0, 3) ?? []` with `hidden` still counted from `total` keeps both fixes and puts the CSS constraint back next to the CSS.
@ -70,0 +81,4 @@
{:else}
<!-- no bar: a 0%-full one on a root nobody can read says "empty",
which is the opposite of what happened -->
<div class="meta why" title={root.error ?? ""}>
Owner

Dropping the bar here is right. Dropping the .meta scan line with it is a second decision, and I do not think it follows: rootInfos() carries scan for a failed probe, so the data is in hand, and an unreadable root is usually a mount that was fine an hour ago. "14.2 GB · 3h ago" under the error is the sentence an operator wants — it says the root has content and this is a mount problem, not an empty disk.

<div class="meta why" title={root.error ?? ""}>
  {root.error ?? "cannot be read"}
</div>
{#if root.scan}
  <div class="meta">
    {formatBytes(root.scan.bytes)} · {ago(root.scan.finishedAtMs)}
  </div>
{/if}

Two lines where the ok branch has one, so check it still fits three roots in the slot — if it does not, this is not worth the row.

Dropping the bar here is right. Dropping the `.meta` scan line with it is a second decision, and I do not think it follows: `rootInfos()` carries `scan` for a failed probe, so the data is in hand, and an unreadable root is usually a mount that was fine an hour ago. "14.2 GB · 3h ago" under the error is the sentence an operator wants — it says the root has content and this is a mount problem, not an empty disk. ```svelte <div class="meta why" title={root.error ?? ""}> {root.error ?? "cannot be read"} </div> {#if root.scan} <div class="meta"> {formatBytes(root.scan.bytes)} · {ago(root.scan.finishedAtMs)} </div> {/if} ``` Two lines where the ok branch has one, so check it still fits three roots in the slot — if it does not, this is not worth the row.
withoutProbeArgs() blanks the argument list of every probeRoots( call by
scanning forward to the matching paren. A call whose parens never close — prose
in a comment that fmt wrapped, a ) inside a string argument — ran that scan to
the end of the file and blanked every character after it, so every policy.roots
below it was asserted against a blank line and the test passed with nothing
printed. Checking nothing is the one failure mode a tripwire may not have, so
the unbalanced case throws: the source stopped being something this check can
reason about, and that is a failure rather than a pass.
The allowed shapes were looked FOR, with lookup.test(named) matching anywhere
on the line, so a line that did both passed on the strength of the half that is
allowed: `const n = policy.roots.length, all = policy.roots;` is a bare alias
next to a count, and fmt keeps it on one line while it is short. The allowed
mentions are subtracted now and whatever is left over fails, which also drops
the need to keep the pattern to one mention per line.

some( and every( leave the allow-list with it. Both visit every root —
`policy.roots.some((r) => { render(r); return false; })` is the walk this test
exists to catch, spelled as a predicate — where find( answers with one root,
which is the by-name case the comment describes. No route file uses either.
Author
Owner

Four of your five, plus one measurement that changed the answer.

1, 2, 3 — the tripwire holes. All three real, all three fixed. Verified by injecting each into routes.ts and restoring afterwards: an unbalanced probeRoots( now throws unbalanced probeRoots( at offset 27021 instead of blanking the rest of the file; const n = policy.roots.length, all = policy.roots; and policy.roots.some((r) => …) both now fail. Every real route-file line still passes.

One correction on the regex you suggested for 2 — I did not take it verbatim. Its (\.(length\b|…)\(|\[) shape requires policy.roots.length(, so plain policy.roots.length would stop being stripped and the test would fail on backend/ui.ts:347 (rootCount: policy.roots.length,). I kept length\b outside the \( requirement; the anchoring fix you were actually after is in.

5 — the scan line. You were right to ask, and the answer is that it does not fit, so it is out. I measured it rather than argued it. Rendered headless on the real dashboard via the @astral/astral recipe in core/CLAUDE.md:376 — six roots, media healthy, gone scanned then unmounted, photos scanned then covered by a deny rule. The slot is 205px and .od-card > .body is overflow: hidden (HomePage.svelte:56,69, grid-auto-rows: 276px).

rows heights content slot
1 readable + 2 unreadable, no scan line 55.91 / 38.52 / 38.52 205px 205px — fits exactly
1 readable + 2 unreadable, with scan line 55.91 / 58.19 / 58.19 217px 205px — footer cut by 11.95px

The second .meta takes an unreadable row from 38.52px to 58.19px and silently cuts the read/write · 3 more roots footer. Same figures at a 376px card (1440 viewport) and a 360px card (380 viewport). My structural reasoning that the branch could not be taller was wrong: the bar is 17.39px against a meta line's 19.67px, so the readable row was never the ceiling I assumed. Commit dropped; OverviewWidget.svelte and README.md are byte-identical to 738f33f again.

4 — the row cap. Declining, and the measurement above is part of why.

Leaving slice(0, 3) out is deliberate, and putting it back re-creates both halves of what this PR removes. The number would exist twice — OVERVIEW_ROOTS in fsops.ts:491 and a bare 3 in the template — free to disagree exactly the way the three root loops did before probeRoots(). And it fails silently in the direction you are worried about: raise OVERVIEW_ROOTS to 6 and the backend probes six roots, a stat and a df each, seconds apiece on a spun-down disk, so the card can throw three away. That is bug 3 of this PR — the fourth probe nobody rendered — moved to the frontend.

A slice is also not a refusal to overflow. It is the same silent truncation as the clip, minus the cue that something is misconfigured. The constraint is already stated on both sides: fsops.ts:487-490 names the slot the number has to fit, and the widget's own header comment (OverviewWidget.svelte:5-9) says the backend decides how many and why — so an edit that raises the constant reads the reason at the line it is editing. If the real worry is that nobody re-measures the slot, the thing that catches that is a rendering test, not a second constant.

Separately, and not from this PR: three readable roots already measure 212px in that 205px slot and clip the footer by 7.39px — identical on main. OVERVIEW_ROOTS = 3 does not fit the slot it is named for today, which contradicts both its own doc comment and HomePage.svelte's "widgets must fit the slot — never show a scrollbar". Filed as its own issue rather than smuggled in here.

Gates on c6855f5: deno test 61 passed / 0 failed, deno lint clean (33 files), deno check backend/mod.ts exit 0, svelte-check 0 errors, module-builder builds. deno fmt --check wants the same six hunks in contract_test.ts as at 738f33f, offsets shifted only.

Four of your five, plus one measurement that changed the answer. **1, 2, 3 — the tripwire holes. All three real, all three fixed.** Verified by injecting each into `routes.ts` and restoring afterwards: an unbalanced `probeRoots(` now throws `unbalanced probeRoots( at offset 27021` instead of blanking the rest of the file; `const n = policy.roots.length, all = policy.roots;` and `policy.roots.some((r) => …)` both now fail. Every real route-file line still passes. One correction on the regex you suggested for 2 — I did not take it verbatim. Its `(\.(length\b|…)\(|\[)` shape requires `policy.roots.length(`, so plain `policy.roots.length` would stop being stripped and the test would fail on `backend/ui.ts:347` (`rootCount: policy.roots.length,`). I kept `length\b` outside the `\(` requirement; the anchoring fix you were actually after is in. **5 — the scan line. You were right to ask, and the answer is that it does not fit, so it is out.** I measured it rather than argued it. Rendered headless on the real dashboard via the `@astral/astral` recipe in `core/CLAUDE.md:376` — six roots, `media` healthy, `gone` scanned then unmounted, `photos` scanned then covered by a deny rule. The slot is **205px** and `.od-card > .body` is `overflow: hidden` (`HomePage.svelte:56,69`, `grid-auto-rows: 276px`). | rows | heights | content | slot | | --- | --- | --- | --- | | 1 readable + 2 unreadable, no scan line | 55.91 / 38.52 / 38.52 | **205px** | 205px — fits exactly | | 1 readable + 2 unreadable, with scan line | 55.91 / 58.19 / 58.19 | **217px** | 205px — footer cut by 11.95px | The second `.meta` takes an unreadable row from 38.52px to 58.19px and silently cuts the `read/write · 3 more roots` footer. Same figures at a 376px card (1440 viewport) and a 360px card (380 viewport). My structural reasoning that the branch could not be taller was wrong: the bar is 17.39px against a meta line's 19.67px, so the readable row was never the ceiling I assumed. Commit dropped; `OverviewWidget.svelte` and `README.md` are byte-identical to `738f33f` again. **4 — the row cap. Declining, and the measurement above is part of why.** Leaving `slice(0, 3)` out is deliberate, and putting it back re-creates both halves of what this PR removes. The number would exist twice — `OVERVIEW_ROOTS` in `fsops.ts:491` and a bare `3` in the template — free to disagree exactly the way the three root loops did before `probeRoots()`. And it fails silently in the direction you are worried about: raise `OVERVIEW_ROOTS` to 6 and the backend probes six roots, a stat and a df each, seconds apiece on a spun-down disk, so the card can throw three away. That is bug 3 of this PR — the fourth probe nobody rendered — moved to the frontend. A slice is also not a refusal to overflow. It is the same silent truncation as the clip, minus the cue that something is misconfigured. The constraint is already stated on both sides: `fsops.ts:487-490` names the slot the number has to fit, and the widget's own header comment (`OverviewWidget.svelte:5-9`) says the backend decides how many and why — so an edit that raises the constant reads the reason at the line it is editing. If the real worry is that nobody re-measures the slot, the thing that catches that is a rendering test, not a second constant. **Separately, and not from this PR:** three *readable* roots already measure **212px** in that 205px slot and clip the footer by **7.39px** — identical on `main`. `OVERVIEW_ROOTS = 3` does not fit the slot it is named for today, which contradicts both its own doc comment and `HomePage.svelte`'s "widgets must fit the slot — never show a scrollbar". Filed as its own issue rather than smuggled in here. Gates on `c6855f5`: `deno test` 61 passed / 0 failed, `deno lint` clean (33 files), `deno check backend/mod.ts` exit 0, `svelte-check` 0 errors, `module-builder` builds. `deno fmt --check` wants the same six hunks in `contract_test.ts` as at `738f33f`, offsets shifted only.
julian approved these changes 2026-09-02 15:21:01 +02:00
julian left a comment

Both blockers from my last round are closed, and I checked them the way that distinguishes a working tripwire from a disabled one rather than by re-reading the diff. Extracting withoutProbeArgs() and running the shapes against it directly:

both-on-one-line  [const n = policy.roots.length, all = policy.roots;]  caught=1
forEach           [policy.roots.forEach((r) => r.name);]               caught=1
fmt-split-for     [for (\n  const r of policy.roots\n) {}]             caught=1
some              [policy.roots.some((r) => render(r));]              caught=1
legit-args        [probeRoots(\n  c,\n  usage,\n  policy.roots.slice(0, 3),\n)]  caught=0
legit-lookups     [policy.roots[0] / .length / .find(]                 caught=0
prose-comment     [// see the probeRoots( call above]                  THREW

Subtracting the allowed mentions instead of looking for them is the right shape — it is what makes the both-on-one-line case fail, and it drops the one-mention-per-line assumption with it. Blanking the argument list is a better rule than the window it replaces, and the throw means the file can no longer go silently unchecked.

Verification of your numbers, on Linux this time rather than the Windows box that gave me 12 environmental failures last round: deno test --allow-read --allow-write --allow-run=python3 backend/61 passed, 0 failed (your body says 60; 6490b46 and c6855f5 landed after you wrote it). deno lint clean, 33 files. On deno fmt --check I checked the claim rather than took it: intersecting the complained-about line numbers per file with the lines this branch adds gives an empty set for contract_test.ts, fsops.ts and routes.ts, and the one overlap in api.ts (103–104) is the pre-existing cancelScan line shifted down by your hunk. The claim holds. svelte-check and module-builder not rerun here.

On the two nits I left last time: the widget's row cap is still only in the backend, but the card probes the roots it draws and counts the ones configured pins the names to ["r0", "r1", "r2"], so raising OVERVIEW_ROOTS past what the slot fits fails a test rather than silently clipping. That is the guard I was asking for. The scan line for an unreadable root is still dropped — not mentioned either way in your description, so flagging it once more below and then leaving it.

Approving. Everything below is non-blocking; the first is the one I would actually spend a commit on.

Both blockers from my last round are closed, and I checked them the way that distinguishes a working tripwire from a disabled one rather than by re-reading the diff. Extracting `withoutProbeArgs()` and running the shapes against it directly: ``` both-on-one-line [const n = policy.roots.length, all = policy.roots;] caught=1 forEach [policy.roots.forEach((r) => r.name);] caught=1 fmt-split-for [for (\n const r of policy.roots\n) {}] caught=1 some [policy.roots.some((r) => render(r));] caught=1 legit-args [probeRoots(\n c,\n usage,\n policy.roots.slice(0, 3),\n)] caught=0 legit-lookups [policy.roots[0] / .length / .find(] caught=0 prose-comment [// see the probeRoots( call above] THREW ``` Subtracting the allowed mentions instead of looking for them is the right shape — it is what makes the both-on-one-line case fail, and it drops the one-mention-per-line assumption with it. Blanking the argument list is a better rule than the window it replaces, and the throw means the file can no longer go silently unchecked. Verification of your numbers, on Linux this time rather than the Windows box that gave me 12 environmental failures last round: `deno test --allow-read --allow-write --allow-run=python3 backend/` — **61 passed, 0 failed** (your body says 60; `6490b46` and `c6855f5` landed after you wrote it). `deno lint` clean, 33 files. On `deno fmt --check` I checked the claim rather than took it: intersecting the complained-about line numbers per file with the lines this branch adds gives an empty set for `contract_test.ts`, `fsops.ts` and `routes.ts`, and the one overlap in `api.ts` (103–104) is the pre-existing `cancelScan` line shifted down by your hunk. The claim holds. `svelte-check` and `module-builder` not rerun here. On the two nits I left last time: the widget's row cap is still only in the backend, but `the card probes the roots it draws and counts the ones configured` pins the names to `["r0", "r1", "r2"]`, so raising `OVERVIEW_ROOTS` past what the slot fits fails a test rather than silently clipping. That is the guard I was asking for. The scan line for an unreadable root is still dropped — not mentioned either way in your description, so flagging it once more below and then leaving it. Approving. Everything below is non-blocking; the first is the one I would actually spend a commit on.
@ -59,0 +62,4 @@
healthy mount. `rootInfos()` maps them for both web surfaces, and
`overviewRoots()` decides how many the card draws and how many it left out — a
route deciding that counted "N more" from the rows it sent. There are tests
for each, and route files may name `policy.roots` only inside a `probeRoots()`
Owner

"only inside a probeRoots() argument list or to look one up by name" is narrower than what the test allows: policy.roots.length is neither, and both routes.ts and ui.ts rely on it passing. The comment in the test gets the rule right — "asking how many there are or for one by name". Worth matching the wording here, since this line is what someone reads before the test tells them no.

"only inside a `probeRoots()` argument list or to look one up by name" is narrower than what the test allows: `policy.roots.length` is neither, and both `routes.ts` and `ui.ts` rely on it passing. The comment in the test gets the rule right — "asking how many there are or for one by name". Worth matching the wording here, since this line is what someone reads before the test tells them no.
@ -34,0 +41,4 @@
* the arguments says what the window approximated, and every other character
* keeps its line and column.
*/
function withoutProbeArgs(source: string): string {
Owner

This helper is now the load-bearing part of the tripwire, and it has no test of its own. Both route files pass by construction, so a withoutProbeArgs() that blanked nothing would look identical in CI to the one you wrote — the same argument that made the last two holes blockers.

The verification is real but it lives in a commit message. A table of half a dozen sources asserted against the helper directly — the four shapes from your description, the legitimate multi-line argument list, and the unbalanced case asserting the throw — puts it where the next edit re-runs it.

This helper is now the load-bearing part of the tripwire, and it has no test of its own. Both route files pass by construction, so a `withoutProbeArgs()` that blanked *nothing* would look identical in CI to the one you wrote — the same argument that made the last two holes blockers. The verification is real but it lives in a commit message. A table of half a dozen sources asserted against the helper directly — the four shapes from your description, the legitimate multi-line argument list, and the unbalanced case asserting the throw — puts it where the next edit re-runs it.
@ -34,0 +51,4 @@
if (chars[i] === "(") depth++;
else if (chars[i] === ")") depth--;
else if (chars[i] !== "\n") chars[i] = " ";
if (depth === 0) break;
Owner

The unbalanced case throws, but the closes-early case is the other half of the same problem and still passes — a ) inside a string argument drops depth to 0 before the real closing paren, so the tail of the argument list is never blanked:

probeRoots(
  c,
  usage,
  tag("a)b"),
  policy.roots,
);

Run against the helper, this reports routes.ts:5 walks the roots itself — a policy.roots that is a legitimate probeRoots() argument, failing the test that exists to allow it.

It fails safe, which is why this is not a blocker: it is a false positive, so the tripwire never goes quiet the way the unbalanced case used to. But it is a landmine for whoever writes the first probeRoots() argument containing a paren in a string, and the message will point them at the wrong thing.

The cheap version is to skip string, template and comment spans while scanning; a full one is not worth it here.

The unbalanced case throws, but the *closes-early* case is the other half of the same problem and still passes — a `)` inside a string argument drops `depth` to 0 before the real closing paren, so the tail of the argument list is never blanked: ``` probeRoots( c, usage, tag("a)b"), policy.roots, ); ``` Run against the helper, this reports `routes.ts:5 walks the roots itself` — a `policy.roots` that is a legitimate `probeRoots()` argument, failing the test that exists to allow it. It fails safe, which is why this is not a blocker: it is a false positive, so the tripwire never goes quiet the way the unbalanced case used to. But it is a landmine for whoever writes the first `probeRoots()` argument containing a paren in a string, and the message will point them at the wrong thing. The cheap version is to skip string, template and comment spans while scanning; a full one is not worth it here.
@ -459,0 +488,4 @@
* overflow is clipped, never scrolled so the card shows this many and says
* how many it left out.
*/
export const OVERVIEW_ROOTS = 3;
Owner

OVERVIEW_ROOTS is a CSS slot width expressed as a count, and it now lives in the file whose docblock opens "Filesystem work. Everything here takes an already-resolved path". rootInfos() returning an HTTP response shape is the same drift in smaller print.

Your reason is right and I would not move these back into routes.ts: it imports hono and @opsdeck/sdk, so a bare clone cannot load it, and untestable is worse than misfiled. But a new backend/overview.ts has only relative imports too, so it is equally reachable from the tests — and then the card's row count sits in a file named after the card. Worth it the next time something else about the overview needs a home; not worth a commit on its own.

`OVERVIEW_ROOTS` is a CSS slot width expressed as a count, and it now lives in the file whose docblock opens "Filesystem work. Everything here takes an already-resolved path". `rootInfos()` returning an HTTP response shape is the same drift in smaller print. Your reason is right and I would not move these back into `routes.ts`: it imports `hono` and `@opsdeck/sdk`, so a bare clone cannot load it, and untestable is worse than misfiled. But a new `backend/overview.ts` has only relative imports too, so it is equally reachable from the tests — and then the card's row count sits in a file named after the card. Worth it the next time something else about the overview needs a home; not worth a commit on its own.
@ -70,0 +81,4 @@
{:else}
<!-- no bar: a 0%-full one on a root nobody can read says "empty",
which is the opposite of what happened -->
<div class="meta why" title={root.error ?? ""}>
Owner

rootInfos() still carries scan for a root that failed to stat, and the store may well hold a complete scan from before the mount went away — but the {:else} branch drops it, so gone shows the error and nothing about what was last measured there. On a card whose whole job is "how full, and how old is that number", the last known size of a root that just disappeared is the more useful half.

A second line under the error costs the slot height you already spend on healthy rows. Your call — it is a product decision, not a defect.

`rootInfos()` still carries `scan` for a root that failed to stat, and the store may well hold a complete scan from before the mount went away — but the `{:else}` branch drops it, so `gone` shows the error and nothing about what was last measured there. On a card whose whole job is "how full, and how old is that number", the last known size of a root that just disappeared is the more useful half. A second line under the error costs the slot height you already spend on healthy rows. Your call — it is a product decision, not a defect.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
OpsDeck/module_filebrowser!4
No description provided.