refactor(state): fail-closed decoders for persisted domain records

[BorisTyshkevich]

Part of the ADR-0004 vanilla-shell investment track (see docs/ADR-0004-ui-shell.md); scoped from the 2026-08-03 architecture review.

Goal

Give the five persisted-domain reads in src/state.ts that currently trust localStorage verbatim a fail-closed decoder each, following the pattern decodeStoredSavedQueries already establishes — drop malformed data to a safe default, never throw, never let a corrupt or hand-edited value reach the UI with the wrong shape.

Context

src/state.ts:221–227 decodes savedQueries through a real decoder:

// decodeStoredSavedQueries fails closed: `ok: false` carries diagnostics and
// ...
const decodeStoredSavedQueries = decodeStoredSavedQueriesUntyped as (value: unknown) =>

Five other persisted reads in the same file have no equivalent and say so explicitly in their own comments — each is a raw as cast straight from read.loadJSON:

Field	Site	Comment
varValues	src/state.ts:696	"The as trusts the localStorage shape verbatim — no decoder exists today (unlike savedQueries)."
filterActive	src/state.ts:706	"The as trusts the localStorage shape verbatim — no decoder exists today."
varRecent	src/state.ts:717	"The as trusts the localStorage shape verbatim — no decoder exists today."
varRecentDisabled	src/state.ts:722	"The as trusts the localStorage shape verbatim — no decoder exists today."
history	src/state.ts:738	"The as trusts the localStorage shape verbatim — no decoder exists today."

Corrupt, truncated, or hand-edited localStorage for any of these five keys decodes to a wrongly-typed object/array that reaches the Workbench UI as-is (variable strip, optional-block filters, MRU dropdowns, executed-query history) — it fails open, not closed. This is the same defect family as the already-closed #570 (NaN-unsafe clamp on the four numeric geometry preferences editorPct/sideSplitPct/cellDrawerPx/docPanePx), just on the object/array-shaped side of persisted state rather than the numeric side; #570's decoders never touched these five keys because they go through read.loadJSON + as, not clamp.

#570's current state: CLOSED (reason: Not Planned, closed 2026-08-03, no linked replacement or comment). This issue does not reopen #570 — it is independent, narrower-scoped work addressing the object/array-shaped reads #570 never covered. If #570's numeric-clamp defect is later judged worth resuming, it should be filed/reopened separately; this issue does not carry that scope.

Deliverables

For each of the five fields, add a small fail-closed decoder module (or function, matching the granularity decodeStoredSavedQueries uses) that:

• validates the persisted shape (e.g. varValues/filterActive are plain string/boolean-keyed records; varRecent matches RecentMap's versioned shape; varRecentDisabled is a boolean; history is an array of well-formed HistoryEntry objects);
• drops malformed top-level values or malformed individual entries rather than throwing;
• falls back to the field's existing documented default ({}, emptyRecentMap(), false, []) when the whole value is unusable;
• never surfaces a wrongly-typed value to the UI.

Replace each as cast at the five sites above with a call to its decoder.

Tests

• One test per field with a non-conforming stored value (wrong top-level type, wrong entry shape, truncated JSON already handled by loadJSON's own parse-failure path, and at least one "individual entry malformed, rest kept" case where the field is a collection) asserting the decoder falls back to the documented default rather than propagating the bad shape.
• Existing "reads + clamps persisted prefs" coverage in tests/unit/state.test.ts continues to pass unmodified in intent for well-formed input.
• npm test (coverage gate) and tsc --noEmit pass.

Acceptance criteria

1. varValues, filterActive, varRecent, varRecentDisabled, and history are each read through a fail-closed decoder, not a raw as cast.
2. Each decoder falls back to the field's existing documented default on malformed input and never throws.
3. Malformed individual entries in a collection-shaped field are dropped without discarding the whole collection where that's a meaningful distinction (e.g. history array, varRecent map).
4. npm test (coverage gate) and tsc --noEmit pass.

Non-goals

• Touching the four numeric geometry preferences (editorPct, sideSplitPct, cellDrawerPx, docPanePx) or clamp's NaN-safety — that was #570's scope, now closed as Not Planned; this issue does not resume it.
• Any behavior change to well-formed persisted data — this is a decode-time safety net, not a feature or schema change.
• Migrating savedQueries's existing decodeStoredSavedQueries pattern itself.

Inherited from #586 (phase 1 of #593) — a lenient width parser

Surfaced by ChatGPT review pass 2 of PR #596 and verified. Folded here rather than fixed in #586
because it is exactly this issue's subject: fail-closed decoding of a persisted value.

#586 replaced a || chain with firstValidPx (src/state.ts:645) so a corrupt canonical width
no longer blocks a valid legacy fallback and no longer survives as NaN. That fixed the NaN
path, but the validator is parseInt, which is lenient:

Stored value	parseInt(raw, 10)	Effect
"420px"	420	accepted; happens to recover the intent
"1e3"	1	accepted as 1, then clamped to the 320 floor
"0x10"	0	accepted as 0, then clamped to 320
"12abc"	12	accepted as 12, then clamped to 320

So a genuinely corrupt canonical asb:rightInspectorPx still wins over a perfectly valid
asb:docPanePx/asb:cellDrawerPx, silently yielding the floor instead of the user's real saved
width. #586's own comment is honest about this ("the first candidate that parses to a finite
number, whatever its magnitude"), so the defect is a contract mismatch with the fail-closed
migration rule, not a lying comment.

Fix shape consistent with this issue: validate the complete trimmed string before conversion and
take the first fully valid finite candidate, preserving the documented precedence
(rightInspectorPx → docPanePx → cellDrawerPx → 480) among valid values only. Existing tests
use distinct values 500/420/560 and must keep passing.

Extra acceptance for this inherited item

• firstValidPx (or its successor) rejects trailing junk, exponent form, hex form and
  whitespace-only values instead of coercing them.
• Regression cases cover a corrupt canonical value with a valid legacy fallback in each
  position.

Related

• createState decodes a corrupt editorPct/sideSplitPct/cellDrawerPx/docPanePx to NaN #570 — same defect family (fail-open persisted reads), CLOSED as Not Planned 2026-08-03; not reopened by this issue.
• core/library-codec.ts's decodeStoredSavedQueries — the pattern this issue extends.
• ADR-0004 (docs/ADR-0004-ui-shell.md)

[BorisTyshkevich]

Phase 3 of umbrella #593.

[BorisTyshkevich]

Inherited from #587 (phase 2 of #593, PR #600) — one part of this issue's subject already landed there.

#587 needed a fail-closed decoder for asb:sidePanel as part of its side-panel registry, because its registry id ('library') differs from the persisted key ('saved') and that bridge has to be decoded at the state-load boundary. So src/core/side-panels.ts's decodeSidePanelKey now validates that one key, and state.ts applies it once in createState.

Worth recording precisely, because the pre-existing defect was real and is now closed:

• Before refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 there was no bridge and no validation at all — state.ts did a raw read.loadStr(KEYS.sidePanel, 'saved'), and consumers only ever compared === 'saved' / === 'history'. An unrecognised stored value therefore fell through to the History body while leaving neither tab visually active.
• After refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 any value that is not exactly 'history' decodes to the documented default, reached by round-tripping through the id vocabulary rather than by whichever branch a bare === 'saved' comparison happens to take.
• The registry id 'library' is never itself persisted (the Library tab writes exactly 'saved'), asserted negatively at the raw saveStr seam — so reverting refactor(shell): a side-panel registry so adding a navigation panel is a one-file change #587 needs no data rollback.

So this issue should not re-implement asb:sidePanel's decoder. Its remaining scope is the other persisted values, including the one folded in from #586's review:

• firstValidPx validates with parseInt, so "1e3" → 1 and "0x10" → 0 still beat a valid legacy key. (Recorded at the refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586 gate; still open.)

Everything else in this issue's acceptance criteria is unaffected. decodeSidePanelKey is a usable precedent for the shape the rest should take: unknown in, a closed union out, no partial parse.

[BorisTyshkevich]

Closing as completed via PR #601 (merged 812eaa4), phase 3 of the #593 refactor umbrella.

All 4 acceptance criteria met:

1. varValues, filterActive, varRecent, varRecentDisabled, history each decode through a fail-closed decoder in src/core/state-codec.ts, not a raw as cast.
2. Each decoder falls back to the field's documented default on malformed input and never throws.
3. Malformed individual entries in collection-shaped fields (history array, varRecent map) are dropped without discarding the whole collection.
4. npm test and tsc --noEmit pass — independently re-verified by the coordinator at the merged head.

Plus the inherited #586 item: firstValidPx now requires a complete trimmed optionally-signed decimal integer, rejecting '1e3'/'0x10'/'12abc', and confirms the parseInt result is finite.

ChatGPT three-pass review: pass 1 and 2 found real, verified findings (prototype-mutation hazards in decodeStoredRecentMap and the write-side enforceTotalCap/recordRecent/visibleRecents/clearRecent in src/core/recent-values.ts; an unclamped-Infinity width bug), all fixed; pass 3 certified clean at head 812eaa4. Full detail in the #593 ship-log comment.

No follow-ups filed from this phase's own scope.