feat(storage): local-first storage layer — Phase 0#4
Conversation
Lands the remaining non-UI Phase 0 foundation piece: @forumforge/storage, the on-device key/value seam that features persist to (read history, saved posts, local notes and tags, per-site settings, installed adapters — all local-first per docs/PRIVACY.md). - StorageBackend — minimal async get/set/remove/keys contract. Async because the real backends (chrome.storage.local, IndexedDB) are; feature code depends only on this interface so a persistent backend slots in later unchanged. - MemoryStorageBackend — real, unit-tested in-memory backend for tests and non-browser callers. Clones values in and out so stored state can't be mutated through a caller's reference. - Collection<T> — typed, namespaced records keyed by id over any backend; key prefixing (<namespace>:<id>) keeps categories from colliding in the one flat key space a backend exposes. Deliberately no chrome.storage/IndexedDB backend yet — it needs a browser to verify and would be a shallow stub (AGENTS.md), same discipline as the parser PR. Docs (README, ROADMAP, package README) and a .memory lesson updated. Verification: pnpm -r typecheck passes; pnpm test — 45 tests pass (5 files).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe89cec2d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Collection: reject namespaces containing ':' so one collection cannot read keys written under another's prefix (e.g. 'note' vs 'note:archived'). - StorageBackend: document that 'undefined' is rejected on writes, and have MemoryStorageBackend reject it — storing undefined would leave a key that keys() reports but get() cannot distinguish from absence. - Initial Plan.md: sync the canonical Phase 0 checklist with reality (core model, storage layer, and generic extractor are done), matching ROADMAP.md. Adds tests for both new guards. 47 tests pass.
There was a problem hiding this comment.
💡 Codex Review
ForumForge/packages/storage/src/memory.ts
Line 11 in c789a4a
When a caller stores a function value, this condition treats it like a primitive because typeof value === "function", so the backend retains and returns the original mutable function object rather than cloning or rejecting it. Mutating a property on that function after set() therefore mutates the stored value, violating the documented isolation guarantee and behaving differently from the planned serializing backends; include functions in the cloning path so structuredClone rejects them, or reject them explicitly.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Continues Phase 0 — Foundation (now that #3 has merged) by landing the last
non-UI foundation piece:
@forumforge/storage, the on-device key/value seamthat ForumForge features persist to. Everything ForumForge keeps — read history,
saved posts, local notes and tags, per-site settings, installed adapters — stays
on the user's own device by default (see docs/PRIVACY.md).
Real, unit-tested code — no placeholder stubs (per
AGENTS.md).What's included
StorageBackend— the minimal asyncget/set/remove/keyscontract. It is async because the real backends (
chrome.storage.local,IndexedDB) are; feature code depends only on this interface, so a persistent
backend slots in later without touching features.
MemoryStorageBackend— a real in-memory backend for tests, non-browsercallers, and as a default fallback. Values are cloned in and out
(
structuredClone, JSON fallback) so stored state can't be mutated through acaller's reference — the same isolation a serializing backend gives.
Collection<T>— a typed, namespaced set of records keyed by id over anybackend. A backend is one flat key space shared by every feature, so each
collection prefixes keys as
<namespace>:<id>to keep categories (saved posts,notes, read history, settings) from colliding.
Design notes
chrome.storage/ IndexedDB backend yet: it needs a browser toverify meaningfully and would be a shallow stub — which
AGENTS.mdforbids.The seam exists precisely so that backend slots in later with no feature
changes. Same discipline as the parser PR (feat(core, parser): begin Phase 0 — post model + generic extractor #3).
comments, local notes) are Phase 1 features, not Phase 0 — this PR is the
foundation they will consume.
pattern; no runtime dependencies.
Roadmap (Phase 0)
packages/corepackages/parserpackages/storage← this PRDocs updated in the same change:
README.md,ROADMAP.md, a packageREADME.md,and a
.memory/lesson on the storage seam.Verification
pnpm -r typecheck— passes (core + parser + storage)pnpm test— 45 tests pass (5 files; 13 new for storage)pnpm install --frozen-lockfile— passes (lockfile updated for the new importer)Intentionally not done
The browser-dependent Phase 0 items (extension shell, content script, side panel)
are left for follow-up — they need a bundler and a browser to verify, so including
them now would mean shallow stubs.
https://claude.ai/code/session_01CgQPsqXKQpTAVSnp2xRRwv
Generated by Claude Code