fix(settings): hand back the effect root disposer the store was throwing away

[PathGao]

SettingsStore's constructor calls $effect.root(…) and throws away the disposer it returns.

In the app that is fine — the store is a singleton and lives until the process ends. In tests it is not: every new SettingsStore() leaks about 30 live effects into the file's shared environment.

It has not bitten yet. But settingsPersistence.spec.ts already constructs several stores in one file, and the vitest migration has 25 more files to go. A file that constructs stores and flushes repeatedly would see cross-test writes.

This is the limitation the #615 pilot reported itself — worth knowing before file #10, not after. So it is fixed before the rollout rather than during it.

Interface

SettingsStore.dispose(). The disposer is stashed in a private #disposeEffects and released by a method.

Precedent: observeFoldLayout(root): () => void, whose stop function the caller holds (MarkdownViewer.svelte:1121 keeps it and calls it in its effect teardown). Same contract, adapted to a class — a constructor cannot hand back a second value, so it lives on the instance.

Deliberately not the resetDiagramCache / resetRichContentCache shape: those are module-level seams for module-level state; this state is per-instance.

The app singleton is unchanged — nothing calls dispose() outside tests.

The window listener is covered — confirmed by test, not by reasoning

The storage listener is registered by an $effect inside the root (settings.svelte.ts:404-419), and destroying a root runs its effects' teardowns, so removeEventListener fires. The falsification below shows the listener assertion flipping purely on the presence of dispose().

Nothing in the constructor registers anything outside the root, so no second seam is needed.

Falsified — both halves separately

a disposed store stops writing and stops listening carries two independent assertions, each proven to fail on its own:

1. Write half — remove abandoned.dispose(): a disposed store still writes to localStorage — '12' !== '30'. The abandoned store's effect re-ran on a later flushSync() and overwrote the live store's key.
2. Listener half — with assertion 1 also removed (it sits first and short-circuits): a disposed store is still listening on window — 40 !== 12. The abandoned store folded a storage event into its own field.

The convention is structural, not remembered

createStore() pairs construction with vitest's onTestFinished, and all 29 construction sites route through it — so a new test cannot forget to tear down. AGENTS.md states the rule and that root disposal covers listeners registered inside it.

One honest caveat, not papered over

initOSType() is an async promise the constructor fires. If it resolves after dispose(), it still assigns osType and possibly the three font families on the disposed instance.

That is not an effect leak and cannot write to localStorage — the effects are gone — so it cannot pollute another test. But a test that disposed a store and then read store.editorFont on a later microtask would see it change. Recorded in the commit body; an abort flag would be code for a case nothing exercises.

Verification

• npm test 966 pass / 0 fail
• npm run test:vitest 68 pass / 0 fail (was 67)
• npm run check 769 files / 0 errors / 0 warnings

🤖 Generated with Claude Code