[quality] Reduce Biome lint debt: any, !important, unused imports, and unsafe patterns

[serge-ivo]

Problem

The latest VCQA scan shows lint as one of the weakest categories.

Scan context:

• Command: pnpm vcqa
• Lint score: D 45/100
• 300 lint issues

Dominant findings from the report:

• 182x Unexpected any. Specify a different type.
• 54x Avoid the use of the !important style.
• 14x unused import groups.
• 10x optional-chain simplifications.
• Several hook dependency and static-element interaction warnings.

This overlaps with narrower tickets for hook order and accessibility, but the any and !important counts are large enough to track separately.

Acceptance criteria

• Replace broad any with specific types, unknown plus narrowing, or intentional helper types where appropriate.
• Remove avoidable !important; move remaining cases behind documented design-system exceptions.
• Remove unused imports and no-op suppression comments.
• Do not paper over issues with broad lint disables.
• pnpm lint improves materially.
• pnpm vcqa lint score improves from the current D 45/100 baseline.

Notes

Keep this ticket focused on mechanical lint/type cleanup. React hook-order and accessibility bugs are tracked separately.

[serge-ivo]

Slice: packages/** + workers/mcp/** — done. Ticket stays open for workers/api and store/console.

Landed in 629ba35. Three other agents hold the remaining trees.

The headline number does not describe this slice

pnpm vcqa lint is Biome. Attributing all 307 findings by tree:

category	total	where it actually is
Unexpected any	184	100% workers/api — 0 elsewhere
!important	54	100% store/** HTML (9 pages × 6) — 0 elsewhere
everything else	69	store/console 11, workers/mcp 3, browser-runner 2, sdk 1, agents/coder 1, rest workers/api

packages/** + workers/mcp/** started at 6 findings, 0 errors. Not one of them was an any or an !important. So the two categories the ticket is named after are, in this slice, a legitimate "not applicable" — they are the other agents' work, and the any count in particular is concentrated in one tree.

packages/** + workers/mcp/** now report 0 errors and 0 warnings.

Per-category verdict

any — none to fix; none replaced. Zero any in non-test source across both trees (grep -E '(:|<|\bas\s+)\s*any\b' over non-test .ts/.tsx returns nothing). The 19 in test files are as any on test doubles — a fake MCP server, a registrar map — and already carry written biome-ignore reasons. Inventing an interface for a deliberately partial fake would make the compiler confident about a shape nobody checks, which is worse than the honest cast. Three more grep hits were the English word "any" in prose comments.

Unused imports — all taken. 4 sites: coding/tmux.ts (two whole dead import statements), beforeEach in two MCP test files (both use a per-test setup() helper plus a global afterEach, so it was a leftover), plus an unused u parameter.

!important — 0 in these trees. All 54 are in store/**.

Dead suppression — kept the reason, dropped the directive. use-voice.ts had a useExhaustiveDependencies suppression Biome no longer needs. Deleting it outright would have deleted the reason the deps are [convoOn] — and that rule is the one that once turned value deps into identity deps over … || [] and killed every instance tab in production. It is now a plain comment saying the deps are deliberate and that the fix, if Biome ever changes its mind, is to suppress rather than widen. Same line count, so the file-size ratchet is unmoved.

"Unsafe patterns" — 1 real out of 17. Read individually:

finding	count	verdict
var keyword (error)	2	False. Both are the English word "var" in a JSDoc line (/** A var counts as set…, /** Env var the CLI reads…). Zero var declarations exist in either tree.
loose equality	5	False. Four are the deliberate == null idiom — the one thing === cannot express. The fifth (mcp/src/session.ts:21) is the string literal "==", base64 padding, not an operator at all.
unsafe-json-parse	6	False. Every site is already inside try { JSON.parse(…) } catch { … }. The heuristic appears to want a multi-line try; a one-line one defeats it.
dangerouslySetInnerHTML (error, CWE-79)	2	False. Both are sdk/src/ui-react.ts — the single reviewed boundary, argument branded SafeHtml (a plain string will not typecheck), with safe-html-guard.test.ts asserting it stays the only sink in the repo. The file's own comment predicts this exact scanner hit.
interval-leak	2	False. One is setInterval(…).unref(), a parent-death watchdog that must live for process lifetime and cannot hold the loop open. The other is poll-cadence.ts:62, which contains no setInterval — it is a function named resolvePollMs returning a number.
non-null assertion (this.client!)	2	REAL — fixed. stop() clears client, so teardown racing a slow in-flight action reached these and reported Cannot read properties of undefined (reading 'callTool') at the relay boundary. Now a connected() accessor that names the lifecycle.

Ratio for this slice: 1 real, 16 noise. Worth knowing before anyone treats the scanner's error-severity findings as a work queue.

Nothing touched in the deliberate-swallow class

Spot-checked beyond the known-deliberate list and left every one: cues.ts (a browser refusing to play a chime must not break the voice loop), transcript.ts:72 (fallback parse of an SSE tail — failing means "no speech", which the next two lines handle explicitly), inspect.ts:159 (statSync on a file that can vanish between readdir and stat; size stays undefined, which the type already models as optional).

pnpm lint is now in CI — scoped

This is the real bug behind #326. pnpm lint has never been in ci.yml, so every Biome rule in this repo is advisory. 300 findings accumulating is not a regression anyone was told about; it is what "nobody is watching" looks like.

A gate can only be turned on where the count is already zero, so CI now runs:

pnpm exec biome check --error-on-warnings packages workers/mcp

• --error-on-warnings because every finding these trees ever had was a warning. A gate that ignored warnings would have caught none of them.
• pnpm exec, not npx, so it uses the pinned @biomejs/biome — a new upstream rule cannot turn a green main red without anyone changing this repo.

What stands in the way of widening it (the counts are written into the step so this is a checklist, not archaeology):

• workers/api — ~234, of which 184 are any. This is the ticket's real typing debt and the largest single unlock.
• store/** — ~54 !important across 9 HTML pages, plus the console's hook-dependency and a11y warnings.
• agents/coder — 1.

Add a path to the step the moment a tree reaches zero. Please do not widen it early and then reach for continue-on-error — that is this step with the teeth removed.

Verification

CI green on 629ba35. packages + workers/mcp typecheck clean; 528 unit tests pass; file-size ratchet OK. No CLI version bump: the two browser-runner changes are a dead import and an error string on a path that already threw, so nothing users can observe needs to reach them — the publish workflow correctly skipped both packages as already-published.

[serge-ivo]

Slice: store/** — the 54 !important are gone. store still cannot join the CI gate; the blocking list is below.

Landed in 4961f68 (+ one finding in c61bacd).

All 54 were one copied block, and 45 of them never won anything

Nine pages × six, byte-identical: a bolted-on <style id="m-fix"> mobile-nav drawer. Not 54
decisions — one, made once and pasted.

header nav{display:none!important; … flex-direction:column!important;gap:0!important; … margin:0!important; …}
header.m-open nav{display:flex!important}
header nav a,header nav button{padding:0.8rem 0.25rem!important; …}

Five of the six per page were specificity theatre. header nav is (0,0,2); the only rule it
competes with is each page's own nav{display:flex;gap:…;margin-left:auto} at (0,0,1) — and the
m-fix block comes later in the document. It outranks it twice over. header.m-open nav (0,1,2)
outranks both. The !important did nothing except force itself onward: once display:none is
important, the display:flex that reopens the drawer has to be too.

The sixth was load-bearing, and overriding it was the bug

auth-widget.js is deliberately self-contained — it styles the account chip it injects into
header nav with inline style.cssText, so it works on a page whose CSS it has never seen.
Inline beats any selector, so header nav a{padding:…!important} was the only way to reach it.

It reached it and flattened it. Measured, signed-in, at 390px:

before:  A[account pill]  150×52   padding-top 12.8px  border-bottom 1px   ← the row's, not its own
after:   A[account pill]  135×32   padding-top 2.88px                      ← its own, inside a 58px row

The pill rendered with the drawer row's 12.8px padding replacing its own 2.88px and the row's
border-bottom drawn through the middle of a rounded capsule. The !important was a bug wearing
a fix's clothes.

So the drawer-row rules are > children now — the injected <span> gets the row treatment, the
pill inside keeps its own. Signed-out is byte-identical in geometry (row still 354×46 at x=20; the
padding simply moved from the inner <a> to the wrapper).

width:100% is deliberately not in the span's rule. It was, briefly: the span carries an
inline margin-left:4px, so 100% makes it 4px wider than the flex line, and on the two pages
whose nav sets align-items:center that re-centred every link 2px right. Stretch already sizes it.

Verified by measurement, not by reading. All 8 pages rendered at 390px signed-out and
signed-in, drawer opened, computed style + box diffed for the nav, every child and every
grandchild. The only differences are the two described above.

store/404.html had no <header> and no <nav>

It carried the whole block anyway — plus the hamburger-injection script — matching nothing.
Deleted rather than de-!importanted, along with the pre/table/.container rules for
elements it also does not have.

While in there: its .links row does overflow a 320px phone (three links need ~270px; the
2rem body padding leaves 256px). overflow-x:clip was hiding that rather than fixing it. It wraps
now.

store cannot join the --error-on-warnings gate. Here is exactly what blocks it.

biome check store: 54 → 12 (8 errors, 3 warnings after c61bacd).

file	n	why it is still there
store/console/src/pages/InstanceDetail.tsx	7	2 suppressions/unused + 5 useExhaustiveDependencies. Another agent holds this file right now (the voice/chat delete path) — its line numbers moved twice during this run. Not mine to touch.
store/console/src/tabs/BoardTab.tsx	1	noArrayIndexKey on the board-column draft rows. The index IS the identity while editing (there is no id until save) — a real judgement call, not a rename.
store/console/src/tabs/DataTab.tsx	1	noArrayIndexKey on an audit-trail <ol>. Same: the steps have no stable key, and inventing one would be worse than the honest index.
store/console/src/tabs/SettingsTab.tsx	1	useExhaustiveDependencies
store/console/src/tabs/BehaviourTab.tsx	1	noPrototypeBuiltins on Object.prototype.hasOwnProperty.call(…) — already the correct defensive form; Biome wants Object.hasOwn. Cosmetic, and in a tab I do not own.
store/sw.js	1	fixed in c61bacd — (data && data.url) → data?.url.

The any/!important framing of this ticket does not describe what is left in store: zero of
either. What remains is seven findings in a file under active concurrent edit and four that need
a decision rather than a fix. Add store to the CI step the moment InstanceDetail settles and
someone rules on the two noArrayIndexKey sites — and please not with continue-on-error.

Verification

CI green on 4961f68 and c61bacd. 88 e2e pass (the one failure, /docs/browser-runtime/, needs
pnpm docs:build and predates this work). File-size ratchet green.

[serge-ivo]

store is in the gate (31e41cc, CI green)

pnpm exec biome check --error-on-warnings packages workers/mcp store — 0
errors, 0 warnings across 354 files, running in CI now.

InstanceDetail.tsx was the blocker and its seven findings had one root cause,
which is worth recording because it is a failure mode of suppressions rather
than of the code:

A biome-ignore covers only the line that FOLLOWS it. Both suppressions in
that file had drifted off their statements — one sat two prose lines above it
(so it suppressed a comment), the other inside the useMemo call just before
the dep array, while useExhaustiveDependencies raises its diagnostic on the
call itself. So: 2 × suppressions/unused plus the 5
useExhaustiveDependencies findings the suppressions existed to answer. Both
reasons are now single lines directly above their statements, and
InstanceDetail.test.ts holds them there — a wrapped reason is a suppression
that has moved.

The findings themselves must not be "fixed": those two memos are keyed on
content because surfaces is … || [], and taking the lint's advice is what
left every instance tab dead in production (#309). The advice ships as a
FIXABLE autofix, hence the guard.

The remaining four, judged one at a time:

finding	ruling
BoardTab noArrayIndexKey	real bug. The column editor's rows reorder / are removed from the middle / are appended to. "Move down" swapped the data while the DOM nodes stayed put, so focus and caret stayed on the position — mid-rename, that types into the wrong column. id cannot serve (a new row's id is empty until save slugs it, so two new columns collide on ""), so a client-side uid that is never sent.
DataTab noArrayIndexKey	suppressed with reason. Append-only audit trail on one already-fetched immutable record, in write order. Nothing reorders, inserts, removes or filters it; no row holds state. The index is the identity and a synthetic id would be the index renamed.
BehaviourTab noPrototypeBuiltins	modernisation, taken. Object.prototype.hasOwnProperty.call was already correct; Biome prefers the ES2022 Object.hasOwn. Noted why it is hasOwn and not in — a field set to null is configured.
SettingsTab useExhaustiveDependencies	taken. The focus re-check listed instanceId beside a refreshRunner callback already keyed on it, so one id change re-subscribed the listener twice.

One note for whoever widens this next: --error-on-warnings earned its keep
here. store's last blocker was two dead suppressions, reported as warnings.
They had been suppressing nothing while the findings they answer went
unreported — which is exactly what an advisory lint hides. A gate that ignored
warnings would have passed this tree while it was broken.

Remaining trees: workers/api (the any debt) and agents/coder (1).

[serge-ivo]

workers/api slice — the 184 any, and what they turned out to be

Headline: none of the 184 were in production code. workers/api/src outside
*.test.ts has zero any and already had seven documented biome-ignores for the TS
mixin-constructor idiom, which has no unknown equivalent. All 184 sat in 16 test files.

That reframes the ticket's premise for this tree, because of a second finding:
workers/api/tsconfig.json excludes src/**/*.test.ts, so CI's npx tsc --noEmit has
never read the files holding all 184. They were not typing debt the compiler was being lied
to about — nobody was asking the compiler anything.

The split

	count	what happened
Real holes — a correct type existed and wasn't used	~55	fixed
Correct at a boundary — knowingly-partial test doubles	18 sites	kept, consolidated to 6 documented casts
Fixture strings (noTemplateCurlyInString)	14	kept, suppressed with reasons

Real holes. parse()/safeParse() returning any from JSON.parse → Record<string, unknown> plus a rows() narrower, so .map((x: any) => x.id) becomes rows(items).map((x) => x.id). Fetch mocks typed (u: any, init: any) → RequestInfo | URL / RequestInit,
which are real global types — a mock that types its arguments any cannot catch the case it
exists to model. find(...).reachable under an any was an unchecked index that would throw
past the assertion on a miss; now ?., so a missing row fails the test instead of erroring in
a way that reads like a crash. And (cfgWrite as any).args[0], where Write.args is declared
unknown[] two screens up — the any was standing in for a non-null assertion on .find()
and hid that that was all it was doing.

Kept on purpose. A step/connector tool takes a whole Env of bindings and a four-method
ConnectorClient; the tool under test touches one or two. An interface for that subset is a
second unmaintained shape the compiler would then vouch for, and as unknown as X is the same
claim with the rule switched off. The cast stays — but it is now made once per file, in a
named fake()/fakeEnv() with the reasoning above it, instead of at 18 call sites that each
quietly implied they were the real thing.

Fixture strings. sql.test.ts tests a detector for SQL built by string interpolation, so
it must hand it source text containing ${...} verbatim; two more assert on other modules'
source text. Rewriting them to satisfy the rule would delete the thing under test.

One any was hiding a real bug — but not in the any cluster

The interesting shape (unvalidated data reaching a decision) lives in the ~60
await res.json() as X / JSON.parse(…) as X casts, which noExplicitAny cannot see. The
one doing damage was lib/auth.ts:

if (row?.roles && (JSON.parse(row.roles) as string[]).includes("admin")) return true;
if (row?.github_login && allow.includes(row.github_login.toLowerCase())) return true;

roles is free text; nothing validates it on write. A value that isn't a JSON array — a
truncated blob, {"admin":true}, a bare "admin" — throws in JSON.parse or has no
.includes. Either way control leaves the shared try, the github_login line never runs,
and the catch attributes it to a DB outage. So one bad row silently removed the
ADMIN_ALLOWLIST path — the break-glass mechanism, whose whole job is to work when the normal
route doesn't. Fails closed, so lockout rather than escalation, but lockout of exactly the
person called in when something is wrong. lib/external-usage.ts reads the same column and
already scoped its parse correctly, which is what made this look like an oversight. Fixed with
rolesOf() (parse to unknown, Array.isArray, filter to strings) + 4 regression tests
confirmed to fail against the previous implementation.

Also fixed: fan_out advertised a concurrency knob that nothing implemented. It was in
the JSON schema and the tool description, read into an unused local. Neither mode can honour
it (pages needs the previous cursor; grid only computes cell centres). A dead variable is
harmless; a dead knob in a tool schema is not — the schema is what a model reads to decide
what it controls, so a model setting concurrency: 20 came away believing it tuned throughput.

Also landed (the other ~50 findings in this tree)

14 unused imports; repoById in the Overseer route, dead since #156 moved resolution into
parseDelegationTarget; rowToSummary(env, row) taking an env it never read; 9 optional
chains; 2 x === false ? false : true; a biome-ignore for a style/ rule this repo turns
off entirely.

Can workers/api join the gate?

Not yet — it is one finding away. biome check workers/api reports exactly one warning: a
useOptionalChain at routes/triggers.ts:385, which is in another agent's slice. Everything
else in the tree is at zero. I've left the line alone and updated the CI comment to name it,
so whoever clears it can widen the gate in the same commit. I deliberately did not widen
early — the existing note is right that reaching for continue-on-error afterwards is the gate
with its teeth removed.

Guards: security-invariants.test.ts 17/17 and import-graph.test.ts 15/15 still pass (no
type was extracted to a new module, so no new edges). pnpm test 4873 passing.

Commits: 07e4f96 · 390c729 · b1266cf · b4a1fdf. Leaving this open for the remaining
trees.

[serge-ivo]

Closing: 300 → 0 in the gated trees, and the gate now covers workers/**

pnpm exec biome check --error-on-warnings packages workers store is at zero, and CI's lint step widened to exactly that (.github/workflows/ci.yml). The last blocker was a single useOptionalChain in workers/api/src/routes/triggers.ts, left standing only because the file belonged to another agent's lane.

The gate names workers, not workers/api. host and mcp were already clean, and naming the parent means a new worker is inside the gate from its first commit rather than quietly accruing a backlog nobody is told about — which is how this ticket's 300 happened in the first place.

One tree is still outside: agents/coder (1 finding, a react/useExhaustiveDependencies in the web UI). The CI comment says to add it the moment it reaches zero.

The headline number was misleading three separate times

This is the part worth keeping, because every one of the three dominant findings in the original report meant something different from what it said.

130 "errors" were not code. They were 116 istanbul output files being linted out of a gitignored coverage/ directory. Biome was reading build artifacts. Nothing in the source was wrong; the scan's file selection was. (This is why #290/#295 now guard the quality-scan config in CI — a scan that reads generated output is a scan whose number means nothing.)

The 184 any were all in test files. Every one. And workers/api/tsconfig.json excludes *.test.ts, so CI had never typechecked a single one of them — the any count was measuring a tree the type system was not looking at. The sweep gave a real type to the ones that have one and left a documented cast per file for the partial test doubles, rather than inventing a fake interface to satisfy a rule.

The 54 !important were one copied block × 9 pages — and this is the one that mattered. 45 of the 54 were pure specificity theatre: !important on declarations nothing was competing with. One was hiding a real bug in the auth-widget pill, where the !important was the only thing keeping a genuine cascade conflict from showing. Removing it exposed the bug; the bug got fixed. A count of 54 that reads as "54 instances of one bad habit" was in fact 45 no-ops, 8 duplicates and 1 defect — which is exactly the argument for judging findings one at a time instead of bulk-fixing to a number.

What actually changed the outcome

Not the cleanup — the gate. pnpm lint had never run in CI, so every Biome rule in this repo was advisory, and 300 findings is simply what "nobody is watching" looks like after a year. store/** is the proof: its last blocker was two biome-ignore comments that had drifted off the lines they covered, reported as suppressions/unused — i.e. as warnings. They had been suppressing nothing while the findings they answered went unreported. That is why the CI step runs --error-on-warnings: a gate that ignored warnings would have caught none of this and would protect nothing.