2026-07-27 OpenRouter Interceptor Storage and JSON Boundaries

What I set out to do

Find out what the four deliberately-off ESLint rules actually complain about, and then close the two boundaries the answer pointed at.

What I actually did

Probed all four against the current tree. Every count had moved from the estimate recorded a commit earlier, which was itself taken before scripts/ was cleaned, so the numbers in eslint.config.ts were stale evidence. Fixed them.

The breakdown mattered more than the totals:

• strict-boolean-expressions, 188. But 152 are nullable string or nullable boolean, where empty and absent genuinely mean the same thing (if (name), if (match?.[1])). With allowNullableString and allowNullableBoolean it is 36, and 26 of those were any.
• no-unsafe-*, 139. Not the ~173 I had estimated, and I had the cause wrong: I had blamed new Function(), which accounts for one site in a test.
• no-confusing-void-expression, 36. All invalidVoidExprArrow. ignoreArrowShorthand: true is 0, so it is a formatting preference.
• require-await, 21. Three are the experiment’s API methods, which are async because the WebExtension contract says so.

Then the real work: almost none of the 139 was loose code. Two boundaries hand back any by declaration and every any in the extension traced to one of them.

browser.storage.*.get() is typed Promise<{ [key: string]: any }> by @types/firefox-webext-browser. So settings["openrouter_api_key"] was whatever you wrote next to it, and downloaded.length compiled against a boolean. That is exactly the problem messaging.ts had, so it got the same answer: src/storage.ts is one table naming what lives under each key, read through the schema that key maps to, and nothing else touches browser.storage.

JSON.parse is the same by definition. parseJson(text, schema) collapses “is it JSON” and “is it the thing I need” into one question.

Three of the five rules are on repo-wide now with no disables. The other two are 17 findings, every one in a *.test.ts and every one a library declaration.

What was striking

Making every schema total was the decision that paid. The read side answers with a value for undefined and for junk alike, so no caller asks whether a key was set. A theme of “chartreuse” reads back as “system”; a downloaded list that is a string reads back as []. That deleted more branching at the call sites than the parse added.

A guard lost its last caller by being done properly. validateJanitorAIResponse was the parse-then-check shape: background.ts parsed the response to any, asked the guard whether it was a chat payload, then carried on with the same any narrowed. Parsing through the schema does both, so the wrapper had nothing left to do. parse-dont-validate keeps producing this: the check disappears rather than being satisfied.

Two library artefacts were fixable after all, which I had written off in the morning’s report. lib.es5 types the String.replace replacer as (substring: string, ...args: any[]), but the callback’s parameters can just be annotated string, since any is assignable to it. And Array.isArray() narrowing unknown to any[] is avoidable with v.is(v.array(v.unknown()), x), which matters because it had schemas/storage.ts tripping the rule inside the parsing code itself.

I git checkout’d a file with unstaged work in it while testing the ratchet, and silently lost the theme.ts changes. Caught it because the diff stat looked too small. Worth remembering that verifying a rule by breaking a real file means reverting that file, and revert is not scoped to the thing you broke.

The cost is real and I nearly did not look for it. Importing the table pulls valibot and every schema in it, so the two JanitorAI content scripts gained ~20KB each. Accepted, but only after measuring: they run on a page that already loads megabytes of React, and splitting the table by consumer would trade the one auditable boundary for the thing it replaced.

Also found and fixed while in reach: diagnostics.ts was writing the pending-tab-refresh flag by hand instead of calling requestTabRefreshOnReload, which exists for exactly that.

Second pass: the last 17

Went back for the two rules I had written off, and both had an honest answer rather than a disable.

new Function(src) is confined to one helper, src/test-support/eval-expr.ts. Eight bare calls became one, and it carries the only no-unsafe-* disable in the repo. evalExpr(expr, schema) parses with the schema the real reader uses, which is what seven of the eight sites were already doing by hand, so the helper made them shorter as well as safer.

The vitest matchers were the better lesson. Each expect.any / stringMatching / objectContaining had a replacement that reads better than what it replaced:

• errorFrom narrows the Answer union, which is what makes answer.error reachable at all. Matching a regex against a field the type says may not be there was working around the union rather than using it.
• toMatchObject is just the symmetric form of a nested objectContaining.
• One expect.any(Number) was re-checking what the schema parse had already guaranteed. The key set was the only thing left to pin.

Worth noticing that a matcher declared any had been hiding assertions that were partly redundant and partly avoiding a narrowing the code should have done.

I git checkout’d a file with uncommitted work a second time, same trick (break a real file to prove the rule fires, then revert). Same silent loss. The habit to build: verify a ratchet on a scratch file, or stash first, never git checkout on the file you are mid-edit in.

Third pass: the last two rules

Re-measured the three still-off rules. strict-boolean-expressions had moved 188 → 169, and the movement was exactly the any category (26 → 3) — nothing else shifted, which is a clean confirmation of what the storage and JSON work actually touched. The nullable-string count went up by 4, because schema fields default to "" rather than being absent, so a few truthiness tests are now string-typed instead of any-typed. Same tests, better types, and the rule can finally see them.

The three surviving “any” findings are unknown, not any. The rule reports both under one message. All three are the parse work landing: turning any into unknown moved them from invisible to flagged. That is the rule paying rent — unknown in a boolean position is a question you should have to answer.

Fixed all 13 that survived allowNullableString + allowNullableBoolean, and turned both remaining rules on. Two of the 13 were latent bugs rather than style: if (timeout) would have skipped clearTimeout on a handle of 0, and if (tabs[0]?.id) would have treated tab 0 as no tab. Neither can happen in Firefox today, but neither is guarded by anything except luck.

The nicest one: two startup diagnostics logged !!browserAPI.webRequest, which the types make vacuous — the log could only ever print “true”. in is the question it was trying to ask, and the namespace genuinely is absent without the permission. A log that restates its own type is worse than no log.

require-await stays off, and I think permanently: 13 of its 21 are either forced by the WebExtension contract or are async () => ({...}) standing in for a Promise-returning interface, which is just how you write that stub.

Third git checkout near-miss avoided by finally doing it right: copy the file to the scratchpad, break it, run the linter, copy back. Two losses was enough.

Fourth pass: looking for where the principle applies, and finding a dead page

Asked where else “parse, don’t validate” applies. Surveying for it turned up that the options page has been completely inert. options.html declares maxCaptures and retentionDays as <select>; options.ts read them with getEl(id, HTMLInputElement), which throws. That was the third of twenty-one lookups, so nothing after it ran — no setting loaded, Save never wired.

getEl was right. The failure was where the failure went: initOptions is async and started with fireAndForget, so a whole page failing to start showed up as one console line. A parse whose failure only reaches a log is a parse nobody reads. That is the lesson I want to keep from today.

The Playwright specs could not have caught it — they read the built HTML and assert it contains an id, which stays true of markup no code can use. That is the failure mode their own file header warns about, one level up. Fixed by extracting readOptionsControls() and mounting the page’s real markup in a DOM test via Vite ?raw.

Then six more places where a check or assertion stood in for a type:

• A 153-line runtime validator with no callers. message-handler-validator.ts watched for the handler shapes that break Firefox’s reply channel. The router makes those unrepresentable. Parse properly and the validator has nothing to do — the clearest illustration of the principle I have seen in this codebase.
• consolidate kept three parallel maps keyed by the same string, with a ! on every read out of them meaning “present because I put it there”. One Map<string, Group> carries it. This is the principle one level up from values: don’t check the lookup, make it unable to fail.
• A guard the storage parse had made redundant. theme-toggle declared its own Theme alias and an isTheme predicate because sync took a string. It doesn’t any more. The “unrecognised preference” test moved to asThemePreference, where the behaviour actually lives now.
• data-index is the page’s attribute, so parseInt there is a boundary. NaN loses every comparison silently: a row with an unreadable index was neither skipped nor selected, and then poisoned the running maximum.
• The devtools capture modes were written out three times; now derived from one list. Notably the panel still checks rather than parses, deliberately — coercing an unrecognised <select> option to the default would write the wrong mode and look like the user chose it. Parse is not always the answer.
• 29 groups[0]! became first(groups) via test-support/at.ts.

Then Adam corrected me on the one I had defended. I had argued the devtools panel should keep its isCaptureMode predicate rather than parse, because coercing an unrecognised <select> option to the default would write a mode nobody chose. He pointed out the parse can just return null.

He was right, and my reasoning had a hole I should have seen: I was treating “parse” as synonymous with “produce a value”, so a case where I did not want a value looked like a case for a check. A parse is not obliged to invent something. null is a perfectly good parse result, and it is what the rest of the repo already does — parseCard, parseJson, readFromPage all return T | null. Same list, same parse, different answers to absence: storage falls back because it must yield something, the panel ignores because it must not.

The stronger point underneath it: x is T is an unverified claim in exactly the way as is. Nothing checks that a predicate’s body implies the type — one that returns true unconditionally compiles and narrows. I had banned as repo-wide for that reason and left the same claim standing in different clothes. asCaptureMode is a find over the literal list, so the value came out of the list and its type is derived, not asserted. messageOf in errors.ts returns string | null, which is what the narrowing above it actually produced.

Two predicates survive in the experiment’s api.ts, and for a real reason: they check one function exists and stand for a whole Gecko module interface, which a nullable parse cannot express without an assertion.

Two eslint-disables left in the whole repo, both load-bearing.

Fifth pass: what was left in strictness, dead code and lint

Asked the open question — anything else for stricter types, dead code, lint — and measured rather than guessed. Three findings, each structural rather than a list of fixes.

The eslint config named its rules one at a time. Measured against strict-type-checked + stylistic-type-checked, 71 of the 89 were absent, and 45 of those were already at zero findings. So rules this repo plainly wants were off purely because nobody had typed their names. That is the difference between a config that expresses a policy and one that records what somebody remembered. Extending the presets makes “on” the default and every exception a decision with its reason beside it. 194 findings at first measurement, 0 now, with require-await the single deliberate exception.

Five tsconfigs had drifted into three definitions of “strict”. scripts/ lacked noUnusedLocals, noImplicitReturns, noFallthroughCasesInSwitch; the node config lacked those plus exactOptionalPropertyTypes; the privileged experiment — the only code here running with chrome privileges — was the loosest of the five and was not in npm run typecheck at all. A file changed meaning when it moved between them. One tsconfig.base.json now, and three new flags that each measured free first: verbatimModuleSyntax, isolatedModules, erasableSyntaxOnly.

36 triple-slash reference directives were one tsconfig entry, written once per consumer. src/**/* already covered src/types/; only test/tsconfig.json needed it, because its include is relative to test/ and nothing imports an ambient declaration.

What was striking

Removing the directives exposed that the pre-commit hook was answering a different question from the typecheck. tsc-files builds a program out of the staged files alone, so it never sees an ambient .d.ts at all — it reported ten TS7017s that no project has. For a repo with no CI, the hook is the only gate, and it had been an approximation of the gate. Replaced with the real npm run typecheck, all five projects, 3.4 seconds. An approximation of a gate is worse than the gate: it fails on things that are fine and passes things that are not, and you cannot tell which from the output.

no-deprecated was the highest-signal rule of the lot, and I nearly dismissed it as noise. It found getBytesInUse in the options page — deprecated because Firefox does not implement it, in a Firefox-only extension. That one was already feature-detected, so it stays with a disable and a reason. But a rule that tells you an API is deprecated on the one browser you ship to is not a style rule.

.catch((err: Error) => ...) is an as wearing a different hat. Four of them. A rejection carries whatever was thrown; annotating the parameter Error is a claim nothing checks, exactly like the x is T predicates from the fourth pass. use-unknown-in-catch-callback-variable is the rule for it and it was absent from the config for no reason at all.

A class with only static members is a module spelt as a class. TransformationChains had fourteen this.#transformation references handed to reduce, each an unbound-method finding: a method read off its object and called with a different this. Harmless only while none of them uses this. Plain functions cannot have the problem, and raw/display turned out to be dead once I looked.

One return new Promise(...) inside a try was a real escape: without the await the promise leaves the try, and the catch below could never have seen it reject. return-await is the only rule that would ever have said so.

Sixth pass: the last JavaScript

Two .mjs lab generators, ~400 lines. Being .mjs meant no project owned them — not typechecked, not linted, not held to a single rule the rest of the week had turned on. That is the quiet version of the drift problem the tsconfig base fixed: it is not that the settings differed, it is that a file can sit outside every setting.

Converted both to .ts under test/, run by ts-node/esm exactly as scripts/ already runs. test/tsconfig.json includes **/*.ts, so they were covered the moment the extension changed.

The rename was not the point. build-lab did JSON.parse on a diagnostic capture and reached into capture.panelCapture?.slider?.outerHtml — an any four levels into someone else’s format, where a moved field builds a page out of undefined or fails several lines later saying something unrelated. It parses against a schema now, and the schema doubles as the list of fields this generator would need updating for, which is a thing a type can say and a comment cannot.

build-sidebar-lab had tested each tag before replacing it, twice, because String.replace on a non-matching pattern returns the string unchanged — so a renamed tag would have produced a lab page still pointing at the extension’s own bundle. Same silent-success shape as the options page from the fourth pass. Written once as replaceTag now.

The trap worth recording: happy-dom exports its own Node, Text and Element, structurally different from lib.dom’s and identically named. With both in scope, annotating redact(node: Node) picked the DOM one, and node instanceof window.Text became an intersection TypeScript reduces to never. Every error pointed at a property access; none pointed at the import. When a whole object collapses to never, look at where its type came from, not at what you did with it.

No JavaScript is tracked in this repo now. The only .js left is two esbuild bundles, generated and gitignored, which is what allowJs: false in the base config was asserting on faith an hour earlier.

Numbers

• JavaScript files tracked: 2 (~400 lines) → 0
• typescript-eslint rules: 18 named → 89 from the presets, require-await off
• preset findings: 194 → 138 (after dead code) → 0
• tsconfigs: 5 configs, 3 strictness levels → 1 shared base
• triple-slash directives: 37 → 1 (the one that was load-bearing)
• eslint-disables repo-wide: 2 → 6, each with its reason inline
• config files linted: 0 → 3 (eslint.config.ts, vite.config.ts, playwright.config.ts had never been linted)
• 821 unit tests, 279 Playwright / 1 skipped, five projects typecheck clean
• Commits: 5b81701 (dead code + directives + the hook), 32f665b (the tsconfig base), 1f64c22 (the presets and the config files)

Numbers (fourth pass)

• no-unsafe-*: 139 → 0, all five rules on, one disable in the repo
• strict-boolean-expressions: 169 → 0 with two allowances, on
• no-confusing-void-expression: 36 → 0 with ignoreArrowShorthand, on
• 803 unit tests, 279 Playwright / 1 skipped
• tsc --noEmit clean across all four projects; lint, stylelint, build clean
• Non-null assertions in src/: 29 → 0; eslint-disables repo-wide: 31 → 2
• 819 unit tests (up from 787 this morning), 279 Playwright / 1 skipped
• Commits: f7d987e (the boundaries), 07f79fb + f08e619 + b2b9735 (AGENT.md), b4b397b (the eval helper and the last two no-unsafe rules), dd099a1 (the void rule, strict-boolean-expressions, and the 13), 0f36ba3 (the dead options page), dc689b1 (invariants in types)

Seventh pass: the Queries tab moves under the capture rows

A UI change rather than a types one. The sidebar had three tabs — Network Requests, Queries, Playground — and Queries was a flat list of every analysis across every character, each card carrying a “View original capture →” button back to the row it came from. That button is the tell: the tab existed to hold something that belonged somewhere else.

Analyses now render inside the capture row they were run against. Expanding a row gives two panes, System Prompt and Analyses, and the Analyses pane is a carousel over that character’s runs. Several analyses of one character are variations on one answer, read against each other rather than scrolled down; a stacked list of them would push the next capture off the screen.

Asked before building rather than after. “Carousel to go through requests that bundle to the same card” had two readings, and only one was buildable: the background store merges requests per character (upsertCaptureByCharacter), keeping a representative payload and a count, so the individual requests folded into a card no longer exist to page through. Analyses do — they are stored one per run with a captureId. Confirmed which before writing anything.

What was striking

A shared single value beat a per-capture map. detailPane and expandedQueryId are one value each, not maps keyed by capture, because only one row is expanded at a time. A map would have had to answer “which entry is live” from expandedCaptureId anyway, and could disagree with it. The fallback falls out for free: expanding a different character names an analysis that row does not have, so it shows its newest. That is the ordinary path, not an edge case, and it has a test saying so.

check-css found the leftovers I would have missed. Moving the query styles into the component’s shadow stylesheet left .status-badge, .status-complete and .status-error in sidebar.css matching nothing — a page stylesheet cannot reach into a shadow root. The build failed on them, which is exactly the failure mode that check exists for.

The contrast test found a real defect, and it was pre-existing. .status-complete had been dead markup since capture rows stopped badging their common state, so its --success-color text had never been measured. Making it live for the analysis badge measured it at 3.34:1 in light mode, under AA. The palette already documented this exact problem for --highlight-char-text: green is the one hue where the 600/700 step cannot carry small text. Added --success-text: light-dark(#166534, #86efac) and sampled it. I would not have found this by looking — the token was correct-looking in the stylesheet, which is the failure mode sidebar-theme.spec.ts was written for.

Parsing the stored tab was not optional. sessionStorage survives a rebuild, so a session last left on Queries meets a build with no Queries tab and lights up nothing at all — no active button, no active view, an empty sidebar. readStoredTab is a find over the tab list now, same shape as asCaptureMode. Same for the data-tab attribute, which is markup and so also a boundary. parse-dont-validate again, in a place I nearly wrote as a string.

One guard replaced a growing list of exceptions. The row’s click handler toggles the row, and the detail is a child of it, so reading the prompt used to collapse the row out from under you. The old code special-cased the delete button, and every new control in the pane would have needed adding to that list. Treating the whole .capture-detail-inline as not-the-row is one rule that does not need extending — and it fixes the prompt-collapse behaviour as a side effect. Also moved preventDefault() after the guard, so Space inside a select works normally.

The badge class is looked up, not interpolated. status-${query.status} happens to match the CSS today; the day either name is renamed it silently loses its colour. A Record<OpenRouterQuery["status"], string> says which class each status wears, and adding a status will not compile without one.

Numbers

• Tabs: 3 → 2; sidebar.css 1106 → 833 lines (271 moved into the component)
• captures-list.dom.test.ts: 12 → 30 tests
• 842 unit tests, 299 Playwright / 1 skipped, lint + typecheck + build clean
• New palette token: --success-text, and two new samples in the theme spec

Related

parse-dont-validate