fix: harden the app against a simulated QA campaign

[siracusa5]

Summary

Phase 6 hardening from an automated QA campaign: six simulated user personas drove the real app — the console build the Mac app ships as its renderer, served over the engine against isolated sandboxes — clicking through first run, the discrepancy queue, a messy 3,000-note vault, deliberate config corruption, and a keyboard-only pass. Every UI claim was cross-checked against the engine API, so a UI/engine disagreement counted as a finding on its own. Thirty findings were triaged before any code changed; this PR is the resulting single fix pass.

Triage mattered more than the count suggests. Three of the loudest findings were not what they looked like: the console "never updating" was the deliberate pause-on-hidden poll behaviour meeting a QA browser tab that is always hidden; the "engine corrupts files on a reconciled answer" was the compose textarea pre-filling with the value being replaced, with the engine writing exactly what it was handed; and the "unnamed controls" accessibility finding was an artifact of the extraction tool rather than the DOM. Those three are fixed where the fault actually was, and the two accessibility claims that did not survive contact with a real browser were left alone.

Changes

• Provenance — the cascade labelled sections and dissent by lane, so a value won by a second personal-level source read as "Personal" disagreeing with "Personal". Sections, suppressions, and dissent chips name the source; lane colour still comes from the lane.
• Lanes — level buckets were fixed thresholds, so a level-1 source rendered under Company while the Team lane sat empty. Levels rank among those the manifest actually contains, and lane headers list the sources behind them.
• Cascade at size — 3,000 concepts rendered as real DOM across a 726,134px row, with Fit floored at a scale 100x too close and zoom-out floored higher still. Both clamp together; each lane caps at 250 nodes with a "Showing N of M" hand-off to Knowledge.
• Reconciled answers — the compose field starts empty with an explicit "Start from <source>" opt-in, and is refused for list-valued frontmatter on both sides (the engine 400s; the console disables it and says why).
• Discrepancy ledger — a decision indexed under both its canonical and legacy id emitted two resolved rows; the source filter ignored effectiveSource; an active search silently survived a status-tab switch while the empty state implied a cleared queue.
• Search — GET /api/search exposes the ranking search.mjs already served over MCP, and Knowledge calls it debounced, keeping the substring match until ranked results land and falling back silently on an older or unreachable engine.
• Honest source state — a deleted layer folder reported as an empty one ("Add a Markdown note and it appears here" over a folder that no longer exists); skipped dot-entries are counted; lastSuccessAt is tracked so a synced badge stops sitting beside "Last success: Not yet".
• Write safety — PUT /api/file requires the modified timestamp it already documents unless force: true; the guard previously skipped itself whenever a caller omitted the field. No in-repo caller needed changing.
• Keyboard and screen reader — APG roving tabindex on the source-kind radiogroup, focus returning to whatever opened a dialog, the shell inert behind an open dialog, and a per-view document.title.

Known limitation, not addressed here

Discrepancy resolution writes to a git-backed live layer through plain file writes rather than git-core.mjs, leaving the working tree dirty and uncommitted — invisible to teammates and able to fail a later Sync's pull --ff-only. It predates this branch and the right fix is a design decision (exclude live layers from propagation, route the transactional path through commitPaths, or document it), so it is deliberately left for a follow-up rather than settled inside a QA fix pass.

Affected area

• Core engine / MCP / write path
• Console
• Site
• Playground or local demo surfaces
• Docs / specs / contributor workflow
• Packs

Validation

• npm test
• npm --prefix apps/console run typecheck
• npm --prefix apps/console test
• npm --prefix apps/console run build
• npm --prefix apps/site run build — not applicable; no site changes.

npm test passes in CI (engine job green). Locally it stops on two service-test.sh assertions about a github-rest layer pointed at an unreachable repo — pre-existing and environmental rather than from this branch: a worktree at the merge base produces the identical two failures in the same sandbox, while this branch adds 12 passing assertions in that same file (208 vs 196). Every later step the short-circuit skips was run individually and passes. The retrieval eval is unmoved (recall@1 0.895, recall@5 1.000, MRR 0.947). Console: 383 tests across 30 files, typecheck and build clean. Beyond the suites, the fixed build was driven live: /api/search returns ranked hits for body-only terms, compose on a list field 400s, PUT /api/file without modified 400s, deleting a layer folder flips the source to error with the folder named and restoring it recovers on its own, the 3,001-concept cascade renders 251 capped nodes and Fit reaches scale 0.016, and the conflict panel names maya-notes as the effective source.

Compatibility

• Root commands still work, or the PR explains the migration path.
• No root npm dependencies were added.
• Generated files are ignored or intentionally committed.

GET /api/search and the additive skippedHidden / lastSuccessAt fields extend existing payloads without changing index identity or validity, so no source re-indexes on upgrade. PUT /api/file is the one behaviour change for API callers: a request omitting modified now gets a 400 telling it what to send, or force: true to overwrite deliberately.

Sign-off

• Every commit is signed off (git commit -s) per the
  Developer Certificate of Origin.

🤖 Generated with Claude Code

[siracusa5]

Review pass: six defects found and fixed

An adversarial review of this diff found six confirmed defects — three of them regressions this branch introduced. All are fixed in 591e77f and 0a6d71b. Worth recording what they were, because the pattern is the point: fixes written against specs need the same adversarial pass the original findings got, and in two cases the regression test written alongside the fix asserted around the bug rather than at it.

Critical — the search route violated a standing rule in this repo. GET /api/search did a full disk walk, a parse of every concept, and a BM25 build on every request: 1931ms on a 3,000-note vault, 2053ms on repeat (no memoization), ~475MB RSS after six calls — with the console calling it once per typing pause, so the sixth answer landed 7.5s after typing stopped. That is exactly the invariant CLAUDE.md records from the countTokens incident, and search is heavier. It now reads the background index snapshots through a view over already-parsed concepts and memoizes on the identity of the snapshots it read, with the promise stored before it settles so single-flight comes for free. Repeat query: 1405ms → ~2ms. search.mjs is untouched, so the ranking the retrieval eval protects is byte-identical.

The discrepancy dedupe made things worse, not better. It checked what it had already emitted but never what was already live, so a decision indexed under both its canonical and legacy ids emitted a phantom row carrying the canonical id. Base produced two rows with different ids; this branch produced two rows sharing one — in a list the console keys by id and looks up with find(c => c.id === …). A live "reopened" row and a stale "resolved" row became indistinguishable. The test meant to cover it built its fixture with no live conflict, making the broken branch unreachable.

lastSuccessAt claimed a read that never happened. health?.lastSuccessAt ?? entry.lastSuccessAt also fires when a health block reports null — which is precisely how github.mjs says "never built an index" — and that adapter warn-and-continues on API failure, so the index pass still resolves and stamps the entry. An unreachable private repo rendered as "0 concepts · as of <just now> · <403>", on the one row where staleness is the entire point.

Search made the Knowledge list worse before it made it better. The engine matches whole stemmed tokens, so "prim"/"prima"/"primar" return nothing en route to "primary" — and since any engine answer including [] replaced the substring filter, typing a title emptied the list and claimed there were no matches. Engine hits now lead in rank order with substring matches following; the union is empty exactly when both are, so the "no matches in titles or content" message is true when it appears. Stale hits are also cleared on query change, which the comment already claimed.

The bootstrap probe didn't do what its comment said. One probe fires right after the initial read; if the engine is still indexing at that instant — the exact case the comment described — the loop is still parked and nothing resumes it. A hidden page now keeps polling while work is genuinely active, at a slower cadence; an idle hidden tab stays silent, which is the behaviour worth keeping, so it now has its own test.

Two smaller ones: the per-lane cap crashed the entire Cascade on a concept with no layers, where the layout it replaced had merely placed it off-canvas; and lowering the Fit scale floor had also lowered the manual zoom floor, letting the wheel reach a scale where nothing is visible and nothing suggests the way back.

Verification

Console: 383 tests, typecheck and build clean. Engine: full suite green except two service-test.sh assertions about a github-rest layer pointed at an unreachable repo — verified pre-existing, not caused by this branch: a worktree at the merge base (65325a6) produces the identical two failures in this sandbox, while this branch adds 12 passing assertions in that same file (208 vs 196). They pass in CI, which has network. Retrieval eval unmoved (recall@1 0.895, recall@5 1.000, MRR 0.947).

Deliberately left as follow-ups: the lane chips (LayerChip, ConceptDetail) still print Personal/Team/Company while the lane headers now name real sources and levels — consistent, but only half honest; and target_missing is accepted as an acknowledgement reason for any discrepancy kind server-side, gated to broken links only in the UI, which matters because a rule carrying that reason can be promoted to automatic.

[siracusa5]

Second review round: four lenses aimed at the first round's fixes

The fixes in 591e77f/0a6d71b had never been independently reviewed, so four reviewers went at them — engine-deep, console-deep, security, and one driving the real app. They found seven more defects. Three of them were fixes that had solved the reported symptom and created a new one, which is the finding worth reading:

• The zoom floor. Splitting the Fit floor from the manual-zoom floor meant a Fit that legitimately landed at 0.023 was followed by one wheel notch — in either direction — snapping to 0.1 and exploding the canvas around the cursor. That is precisely the bug an earlier commit on this branch had already fixed, and the test whose own wording described it ("zooming out after a Fit… snapped the view back up — the opposite of 'zoom out'") was deleted in the process. Two reviewers caught it independently, one by arithmetic and one in Chrome (Fit 0.0165 → one click → 0.1, a 6× jump).
• lastSuccessAt. Stopping it fabricating a success taught it to disown a real one. A health block reports null both when a source has never been read and when this adapter instance hasn't read it — and adapters are rebuilt on every manifest change while index entries are adopted. Measured: healthy GitHub source, add an unrelated local folder, zero new API calls, still serving three concepts, timestamp flips to null. The durable fact is the index entry; it's now withheld when a health-bearing adapter says it isn't ok (which is what made it untrustworthy) and trusted after that.
• The hidden-tab poll. It keyed off the last successful pass, so an engine dying mid-index left it retrying a corpse forever on a backgrounded tab — 85 failed fetches in ten simulated minutes with no termination condition. It also took the heavy /api/graph + /api/resolve-all path on every hidden poll, which is exactly what that gate exists to prevent.

Two more were honest gaps rather than regressions. /api/search answered from a partial index while reporting only hits and ignoring ?wait= — on a 1,500-doc vault a query 3ms after start returned zero while /api/graph said indexing: true, and 1.5s later the same query returned fifty; someone launching the app on a real vault and typing would read that as an empty vault. And /api/files still read a deleted folder as an empty one, so the file browser offered "add a Markdown note and it appears here" about a folder that no longer existed.

The Escape-focus one is worth its own note, because it is the second time this round that jsdom and Chrome disagreed. A passing jsdom test said focus returned to the opener; in Chrome it landed on <body>. The real cause was a double restore — the shell closed the dialog and restored focus, then the dialog's own Escape handler ran on the same event and restored again from an already-cleared ref, landing on a fallback element or nothing. Both handlers respect defaultPrevented now and the restore is idempotent.

Also fixed: a query with no searchable token answered 500 instead of an empty result; a keystroke changing only whitespace or case re-ran the search and blanked the ranked list; the ranked hits were visually indistinguishable from the substring matches beneath them (they now carry a heading); the TTL cache replayed every walk note except the hidden count; and a concept with no layers wrote a stray NaN into a lane header count.

What the reviews cleared

My own leading suspicion — that the new search memo was an unbounded cache holding a BM25 index per entry — was wrong, and measured to be wrong twice: capped at 200 (500 distinct queries → size 101), old generations dropped rather than accumulated (25 generations × 20 queries → final size 21), only small hit rows retained, rejections evicted, cleared on close, and no stale or removed-source content served (add → query → remove source → re-query returns []). The security pass independently confirmed the no-leak result, and also cleared: /api/search sits behind the same bearer/CSRF gate as its siblings; force: true bypasses only the mtime check, with traversal, symlink escape, unknown-layer and non-text-extension all still enforced; limit validation is solid and an oversized query is rejected before it reaches the handler. The debounce race, the timer scheduling across rapid hide/show cycles, and the discrepancy dedupe were each attacked and held.

One visible product change, and one thing I did not fix

The per-lane cap dropped from 250 to 24, because that is what it costs for a fitted view to clear a scale where a card is still a shape. On a 3,000-concept vault Fit now renders 25 readable nodes at scale 0.2 with "Showing 25 of 3,001 · Browse everything in Knowledge", where before it rendered 251 nodes at 0.0165 — a canvas the live reviewer described as blank apart from one faint dashed line. That is honest and usable, but it is a judgment call about how much a topology view should show, and the real answer for large vaults is wrapping lanes into rows rather than clamping harder. Worth your eye.

Not fixed, flagged deliberately (pre-existing): a composed section body containing ## Heading {#anchor} or a --- block is spliced into the document raw, creating a new anchored section and an embedded frontmatter-looking block in every writable contributor — content an agent later reads as authored structure. Confirmed by exploit. It shipped in #106 and this branch only touches the neighbouring frontmatter guard (frontmatter values are safe — renderScalar escapes them). It belongs with the live-layer git gap in a pass of its own on the write path.

Verification

Engine: full suite green except the two pre-existing service-test.sh assertions about an unreachable github-rest layer, unchanged from the merge base and passing in CI. Console: 395 tests, typecheck and build clean. Every new or changed test was confirmed to fail against the pre-fix code. Retrieval eval unmoved. Beyond the suites: Fit verified at 0.2 with no jump on the next zoom in either direction, /api/search confirmed returning indexing/indexingSources, and a token-free query confirmed answering 200 with no hits.