Skip to content

Read every Space through the storage ports - #133

Merged
cxxxxxn (cxxxxxn) merged 9 commits into
microsoft:mainfrom
ultmaster:feat/backend-agnostic-reads
Aug 28, 2026
Merged

Read every Space through the storage ports#133
cxxxxxn (cxxxxxn) merged 9 commits into
microsoft:mainfrom
ultmaster:feat/backend-agnostic-reads

Conversation

@ultmaster

Copy link
Copy Markdown
Contributor

Every production structured read now uses StructuredStore: Space topology through SpaceHandle.read(), single node content through SpaceNodes.read(), a named subset through readMany(), and whole-Space or incremental scans through list() / stream(). CanvasStore stays inside the Disk adapter and its own suites; storage/canvas-store.ts had no importers left and is deleted — the first of the three Phase-4.5 forwarding shims to go.

Second of three change sets, specified in docs/proposals/multi-backend-storage.md §12.7.

Its own commits (e0c6a1579d09481b):

  • Agent-thread targets, World/spatial/prompt context, Portal-Pin routing, snapshot rendering, Interactive Views, executor prestate, external-note filtering, the built-in file tools, the Space GET, search, and the Space preview projection all read through space(canvasId).
  • node-prompt could not stay store-aware. It is a pure merge that took a CanvasStore solely to lazily read a record its caller had not supplied, making a pure function into a synchronous Disk dependency no async port can satisfy. The record became an argument, and each caller reads in the shape its request has — which is what made readMany earn its place rather than merely have one.
  • The last Disk-layout imports are closed. The external-note watcher read the Space record off the file beside nodes/; reveal-nodes assembled the sidecar directory itself; bundle import owned the staging location, the title-derived directory name, the record filename, and the index entry. stageSpaceImport takes the placement and the route keeps the .huabu.zip format. Splitting it separated two constants that were one string for a historical reason: the bundle's record filename is a wire format frozen by every bundle already exported, while Disk's is how a backend files a record today.
  • The guard is now writable as specified — import-level and symbol-level, repo-wide, migrations and tests exempt. The barrel was a way around a path-only check: the preview projection reached the legacy store by name through storage/index.js. resetStorageCache is deliberately not on the symbol list; it reads nothing, and finding it a home is §12.8's business.

Two behaviours moved on purpose, neither implied by the read migration:

  • Two Space directories carrying the same canvasId resolved last-wins; the directory scan now rejects, so a Finder-side duplication is a loud failure rather than a Space that silently resolves to an arbitrary copy.
  • The Space preview read node sidecars strictly and answered 422 when one failed to parse. The port defines a single lenient read for the collection, and the projection already fell back to topology data for an absent record, so a damaged sidecar now renders the way it does when its own Space is opened. The 422 stays for a malformed Space record. docs/architecture/space-preview.md is updated to say so.

Stacking. Builds on #132, which builds on #130; neither branch exists on microsoft/Huabu, so GitHub cannot use them as a base and the diff below carries their commits too. This PR's own work starts at e0c6a157.

pnpm run check passes at this branch's tip.

🤖 Generated with Claude Code

https://claude.ai/code/session_011EKZXxEWwZvAZo8YMaLv4m

Yuge Zhang (ultmaster) and others added 7 commits August 27, 2026 17:23
The thread resolver and RFS node lookup reached the legacy Disk store for
both the Space record and node content. Both now go through
`space(canvasId)`, which makes the resolver chain async up to its already-
async callers: the RFS route, the agent route, and the interactive-view
owner-thread check.

RFS's file → node mapping does not become portable, and should not. Every
backend could mint `nodes/<label>.md` from records, but Disk inverts the
*real* filename, because the file is really there and a user may have
renamed it; the two agree only when nothing has touched the directory from
outside, which is the one case Disk cannot assume. So it moves onto
`diskTree.nodeIdForPath()` — named for the backend that has it, and listed
in the census as disposition B, deferred (§6.4.3). The record and the
sidecar around it come from the ports.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… ports

The World resolvers, the spatial queries, and the prompt assemblers all
reached the legacy synchronous Disk store. They now go through
`space(canvasId)`, which makes each entry point async up to its already-async
caller: the space-query facade, the agent tool executor, and the chat
envelope builder.

Three things worth calling out beyond the mechanical move.

`node-prompt` stops taking a `CanvasStore`. It is a pure merge — the caller's
authored fields win, the stored record fills the gaps — and it took a store
only so it could lazily read a record the caller had not supplied. That made
a pure function into a synchronous Disk dependency no async port can satisfy.
The record is now an argument, so each caller reads what it needs in the
shape that suits its request: `readMany` for a selection the wire already
named, `list` where the work genuinely spans the Space (the outline
describes every node, inspect filters over all of them).

`readWorldTargetCanvasesStrict` walked the Workspace itself, reading
`space.json` files, which made a reference resolver a consumer of the Disk
record layout and cost it the whole Workspace per call. It now reads the ids
it was asked for. Its duplicate-topology check moves to where duplicates are
actually observable — the Disk directory scan, which resolved two
directories claiming one `canvasId` last-wins and now raises. A Finder-side
duplication becomes a loud failure of every catalogue read instead of a Space
that silently resolves to an arbitrary copy.

One behaviour change, deliberately. World reference resolution read source
nodes strictly and rejected on malformed frontmatter, so one hand-edited file
made the entire World view 500. The port's documented stance is the opposite
and has been since phase 4: broken frontmatter is not a read failure,
because a node whose YAML a user broke must stay repairable through the
content PUT. The resolver now follows the port, and the test says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agent Node placement, Task run launching, the SSE snapshot, web previews,
Portal-Pin routing, snapshot rendering, Interactive Views, the executor
prestate, external-note filtering, and the built-in file tools all read
through `space(canvasId)` now. Only `canvas.route.ts` and the search
adapter still hold a legacy store.

Each read is expressed in the shape its request actually has, rather than
whatever the synchronous store made convenient:

- Executor prestate hydration and Interactive View listing scan the Space,
  because they genuinely span it.
- Portal-Pin routing pre-reads exactly the source Spaces the command
  references. The set is knowable without running the passes — a source is
  either named by an update or referenced by a World node — so the memoized
  lazy reads become one batch and the passes stay synchronous.
- Snapshot rendering, `fs-read`, and Interactive View `get` read one node.

`importForeignNodeSources` stops taking a `CanvasStore` altogether: it used
it for one topology read and for `store.canvasId`, both of which it already
had as an argument.

Two places needed care rather than a mechanical await. The external-note
snapshot both reads and prunes pending items, and the caller depends on
registering its listener and taking the snapshot with no await between them,
so the known-node set is now read before that block and passed in. The
built-in file tools' node lookup reads its Space record up front for the same
reason — the lookup itself is called synchronously from the search passes,
and only that half crosses the port. Its directory scan stays put: those
tools are Disk-only, and mapping real filenames to records is what they are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Space GET, single-node content, search, export, reveal, and the
preprocess guard were the last production readers on the legacy Disk store.
With them on `space(canvasId)`, `storage/canvas-store.js` has no importers
left and is deleted — the first of the three Phase-4.5 forwarding shims to go
entirely, rather than just shrink.

Search moves to `SpaceNodes.stream`, which is what that member existed for:
it emits meta-tier matches as each record lands instead of waiting for the
whole scan, and the map it returns still feeds the content tier with no
second read.

Two Disk facts that were reached through the legacy store move onto
`diskTree` rather than disappearing:

- Duplicate sidecars. Only a filesystem can have two files claiming one node
  id, and the read path deliberately reports it as a non-blocking hint while
  a write hard-fails — a user who broke it by hand needs to see the node in
  order to fix it. `hydrateOneNode` now takes the record and the duplicate
  list as arguments and reaches nothing itself.
- The explicit `revalidateNodeForRead` before a single-node read is gone: the
  port's own `read` already reconciles the adapter's cached index before
  answering, so the route was asking for something it now gets by default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ndary

No production module outside `storage/` may name how a backend stores a
Space. Three still did, and each was a different kind of leak rather than an
oversight.

The external-note watcher read the Space record off the file beside `nodes/`.
That path read was equivalent only because Disk keeps the two together, and
"what does this Space contain" is a question every backend answers — so it
goes through the port. Reveal-nodes assembled the sidecar directory itself;
the Disk capability now answers "which folder holds the notes", which is what
having one owner for the layout is for.

Bundle import was the real one. It owned the staging location, the
title-derived directory name, the record filename, and the directory index
entry — all four are placement, and none of them are the `.huabu.zip` format
the route legitimately interprets. `stageSpaceImport` takes them, and the
route keeps unzipping, the manifest, and artifact-URL remapping. Its refusal
off Disk is phrased inline until the capability matrix owns that wording
(§12.8).

Splitting it surfaced something worth naming: the bundle's record filename
and Disk's record filename are the same string for a historical reason, not a
shared one. One is a wire format frozen by every bundle already exported; the
other is how a backend files a record today. They are separate constants now,
because they drift the moment a backend that is not Disk exports a bundle.

With those gone, `storage/paths.js` has one production importer left and it is
a migration — exempt by construction, since rewriting a frozen historical
shape is the one legitimate reason to know a layout that is no longer current.
The neutrality guard is now writable as specified: import-level, repo-wide,
migrations and tests exempt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EKZXxEWwZvAZo8YMaLv4m
The preview scene was the one production reader the migration missed, because
it reaches the legacy Disk store by name through the barrel rather than by
path — which the import guard resolved to `storage/index` and let through. It
now reads topology through `space(canvasId).read()` and node records through
`nodes.list()`, and the guard checks the symbol as well as the path, so the
barrel stops being a way around it.

`resetStorageCache` stays off that symbol list deliberately. It is on the
barrel too and the Workspace routes still call it, but it reads nothing: it is
activation dropping an adapter's caches, which is a composition concern with
its own home to find, not a module learning how a Space is stored.

One behaviour moves, stated rather than absorbed. The route used to read node
sidecars strictly and answer 422 when one failed to parse; the port defines a
single lenient read for the collection — a record it cannot produce is
omitted, a record broken by hand recovers — and the projection already fell
back to topology data for an absent record. A damaged sidecar now renders the
way it does when its own Space is opened, instead of failing the whole
preview. The 422 stays for a malformed Space *record*, which is the damage
that makes the projection meaningless, and now covers a record the port
refuses to produce as well as topology that is not the expected shape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EKZXxEWwZvAZo8YMaLv4m
§12.7 was written as a plan before the work started; it now records what
landed, including the two findings that only appeared while building — that
`node-prompt` could not stay store-aware, and that the barrel let one reader
around a path-level import guard — and the two behaviours that moved on
purpose rather than as side effects.

The architecture doc's list of "remaining compatibility consumers" described
reads that no longer exist. What is left outside the ports is a set of
Disk-shaped capabilities, not a set of readers, which is a different claim and
the one §12.8 acts on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EKZXxEWwZvAZo8YMaLv4m
@ultmaster
Yuge Zhang (ultmaster) force-pushed the feat/backend-agnostic-reads branch from 9d09481 to 50ec4b5 Compare August 27, 2026 09:29
Yuge Zhang (ultmaster) and others added 2 commits August 27, 2026 18:24
The unit suites beside each reader prove it calls the port. None of them
shows that a request still answers the same way now that every read goes
through `space(canvasId)`, so this adds one suite that drives a real temp
Workspace, the Disk adapters, and the Canvas routes with nothing mocked —
34 scenarios over the four node-read shapes, the Space GET, the preview
projection, duplicate Space directories, World references, search, bundle
export/import, the per-node content routes, executor batches, and Workspace
switching. Both behaviours this branch moved on purpose are pinned there:
a duplicated Space directory fails loudly naming both, and a hand-damaged
sidecar renders instead of 422'ing a preview.

Writing it surfaced two things.

**The bundle record filename could be `undefined`.** `space-import.ts`
captured `[SPACE_JSON_FILENAME, 'canvas.json']` at module scope, and
`layout.ts` → `workspace.ts` → the storage barrel → the composition root →
`space-import.ts` is an import cycle. Entered through a Disk module, this
file evaluates while `layout.ts` is still initializing and the array freezes
that constant to `undefined` for the life of the process — every Space
bundle import then answers 500 with nothing in the message to explain it.
The production entry point's own order happens to be safe, which is why
nothing caught it and why no static import in a test can express the failing
case; the regression test resets the module registry and re-enters the cycle
deliberately. Assembling the list per call is the fix.

**`SpaceNodes.list()` documented the opposite of what it does.** The port
said a record the scan cannot retrieve is omitted; microsoft#132 made the scan strict
about reachability and the adapter has said so since, and the adapter is what
runs. The rule the two collection shapes actually keep — strict about
retrievability, lenient about content — is now stated once on the interface
and both shapes are held to it by test, because a caller that could pick the
scan which hides an environmental failure cannot tell a Space that lost a
node from one it merely cannot read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFDPiFypeiho39r8Q7Z9uC
@cxxxxxn
cxxxxxn (cxxxxxn) merged commit e0caebc into microsoft:main Aug 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants