From d9b428f8e59b81cf3599666f0409c995464ad9c0 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 14:12:07 +0800 Subject: [PATCH 1/9] =?UTF-8?q?docs(storage):=20plan=20Phase=204.5=20?= =?UTF-8?q?=E2=80=94=20move=20storage-owned=20layout=20inside=20the=20boun?= =?UTF-8?q?dary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §12.2.2 moved `paths.ts` and `canvas-dirs.ts` out of `storage/` on the grounds that they serve "non-storage domains". That reason is inverted: a storage detail with consumers outside `storage/` describes a leak rather than earning a home outside the boundary. Record the correction against §12.2.2 and add §12.5 as Phase 4.5, to run before the SQLite phase so consumers are not migrated twice. The census is smaller than the raw import count implies — one production site outside `storage/` reads a storage-owned path. The problem is that `workspace/disk/` is a mixed module whose name asserts the substrate: it holds the Disk record layout, the blob layout, a `space.json`-derived index, pure naming logic, and genuine workspace concerns, all at once. Also records the path families that map to no port and no table — prompt logs, ACP sessions, per-Space memory — which would otherwise split one Space's state across two substrates with nobody deciding it. Co-Authored-By: Claude Opus 5 (1M context) --- docs/proposals/multi-backend-storage.md | 132 +++++++++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index fee56f0f5..e405fabba 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -893,6 +893,14 @@ the legacy class import; the other two preserve physical-Disk capability imports. Lower-fanout imports are updated directly. No forwarding file may contain logic, and no new call site may import one. +> **Superseded in part by §12.5.** The row moving `paths.ts` and +> `canvas-dirs.ts` to `modules/workspace/disk/` is justified above by those +> files serving "non-storage domains". That reason is inverted: a storage +> detail with consumers outside `storage/` is describing a leak, not earning a +> home outside the boundary. The move was right for the files that describe +> the Workspace _as a place_ and wrong for the files that describe _how the +> Disk backend stores Spaces_. Phase 4.5 separates the two. + #### 12.2.3 Compatibility boundary and blast-radius budget In Phase 2, `storage/compatibility/canvas.ts` owns the legacy @@ -1665,7 +1673,129 @@ bearing: change-review records and Tasks are not history, whatever Disk's arrives, the group comes back — and `events` is where it was before, so nothing else has to move. -### 12.5 Later phases — provisional +### 12.5 Phase 4.5 — storage-owned layout moves inside the boundary — **planned** + +Phase 5 adds a second structured backend. Before it does, the layout knowledge +that belongs to the _Disk_ backend has to stop living outside `storage/`. +Otherwise every later backend inherits a module named `disk` as the ambient +description of where Spaces are, and each one pays to migrate the same callers +again. + +The rule this phase restores: **outside `storage/`, nothing knows how Spaces +are stored.** A domain may know a Space is _materialized_ somewhere — that is a +declared capability with real consumers — but not that its record is +`space.json`, nor that its events are a JSONL file. + +#### 12.5.1 What the census shows + +Measured on `6b43798a`. Non-storage **production** files importing +`modules/workspace/disk/`: + +| File | Symbols | Kind | +| --------------------------------- | ----------------------------------------- | ----------------- | +| `agent/acp/service.ts` | `canvasAcpNamespace` | materialization | +| `agent/agent-thread.service.ts` | `canvasAcpNamespace` | materialization | +| `agent/memory/analyzer.ts` | `canvasMemoryPath`, `workspaceMemoryPath` | materialization | +| `agent/memory/analyzer.ts` | `chatDir` | **storage-owned** | +| `agent/node-ref.ts` | `toSafeFilename` | pure naming | +| `canvas/canvas.route.ts` | `toSafeFilename` | pure naming | +| `canvas/external-watcher.ts` | `registerSpaceDirHandleOwner` | materialization | +| `preprocessing/stages/project.ts` | `normalizeForCompare` | pure naming | +| `workspace-prepare.ts` | `ensureWorldCanvasOnDisk` | materialization | + +Six test files add `nodesDir`, `changesPath`, `canvasRoot`, and +`withSpaceDirHandlesReleased`. + +This is a smaller consumer leak than the raw import count suggests: most +`workspace/disk/` imports come from `storage/` itself, which is the permitted +direction. Exactly **one** production site outside `storage/` reads a +storage-owned path (`chatDir`). The problem is therefore not mass violation by +consumers — it is that the module they import is _mixed_, and its name and +location assert an answer the port layer exists to keep open. + +#### 12.5.2 `paths.ts` holds three populations, not one + +| Population | Members | Belongs to | +| ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | +| Disk structured layout | `SPACE_JSON_FILENAME`, `canvasJsonPath`, `nodesDir`, `nodeFilePath`, `historyDir`, `chatDir`, `changesPath`, `tasksPath`, `eventsPath`, `deltaLogPath` | `storage/backends/disk/` | +| Blob layout | `ARTIFACTS_DIR_NAME`, `artifactsDir`, `artifactPath` | `storage/backends/disk/` (blob) | +| Workspace-as-a-place | `canvasRoot`, `settingDir`, `userSkillsDir`, `workspaceMemoryPath`, `canvasMemoryDir`, `canvasMemoryPath`, `memoryStatePath`, `canvasAcpNamespace`, `WORLD_CANVAS_DIR_NAME` | `modules/workspace/` | + +`canvas-dirs.ts` is not ambiguous at all: it builds its index by reading +`space.json` from every directory. It is Disk structured-backend state that +currently lives outside the storage boundary, and it is the reason a SQLite +profile would silently fall back to id-named artifact directories (§13). + +`naming.ts` is misfiled in a third way — it is pure string logic with no I/O, +already re-exported rather than owned. Phase 5 extracts it to `utils/naming.ts` +as a side effect of needing it in a second backend; that extraction belongs +here instead, where it is the point rather than a side effect. + +#### 12.5.3 Path families with no owner + +Cross-referencing the families above against the Phase 5 schema surfaces a gap +this phase must record even though it does not close it. Six families map to +tables — spaces, nodes, events, changes, tasks, delta log. These do not map to +anything, and no port describes them: + +- `chatPromptLogPath` — per-thread prompt logs +- `acpSessionsPath` — ACP session state +- `canvasMemoryDir`, `canvasMemoryPath`, `memoryStatePath` — per-Space memory + +Under any non-Disk structured profile they remain files with no owner, which +would split one Space's state across two substrates without anyone deciding +that. Phase 4.5 assigns each family an owner — port, materialization +capability, or explicitly Disk-only — so that Phase 5's non-selectability has a +written reason rather than an accident. + +#### 12.5.4 Materialization becomes declared, not ambient + +The consumers marked _materialization_ above cannot be served by a structured +port and should not be. An ACP agent, a file watcher, and RFS need a real +directory; that is a product requirement, not a leak. + +What is wrong today is that they get it by reading an ambient layout module. +The port layer already has the right shape one level down — `BlobScope` +exposes `materialize()` for "consumers that genuinely need a real filename" +(§7). This phase gives Space trees the same treatment: an explicit capability a +consumer depends on by name and a profile can decline to offer, rather than a +path helper that is always simply there. + +#### 12.5.5 Scope + +In: + +- Relocate the Disk structured and blob layout families, and `canvas-dirs.ts`, + into `storage/backends/disk/`. +- Extract pure naming to `utils/naming.ts` (moved out of Phase 5). +- Keep the Workspace-as-a-place families in `modules/workspace/`, without a + `disk` segment asserting the substrate. +- Introduce the Space materialization capability and move the four + materialization consumers onto it. +- Move `agent/memory/analyzer.ts` off `chatDir` and onto the change/log port. +- Assign an owner to every §12.5.3 family; implement none of them. +- Extend the module-boundary test to fail when a non-storage file imports a + storage-owned layout symbol — the guard that stops this recurring. + +Out: + +- Any SQLite work, schema, or adapter. +- Changing the blob scope's _identity_ (title-derived vs `canvasId`). This + phase records the decision point; §13 owns the hazard. +- Agenetes persistence, RFS/file-tool contracts, import/export, product UI. + +#### 12.5.6 Sequence and verification + +The moves preserve symbol names and runtime logic, as in §12.2.2. Tests move +with their subjects. Verification is `pnpm run check` plus the extended +boundary test; behavior parity is asserted by the existing Disk suites, which +must pass unchanged — a diff that alters a Disk test's expectations is out of +scope by definition. + +Phase 5 rebases onto this and drops its `utils/naming.ts` extraction, its +`workspace/disk/naming.ts` shim, and the corresponding roadmap edits. + +### 12.6 Later phases — provisional 5. Add one new adapter at a time — SQLite, then Postgres, then Azure Blob — running the same contract suites, migration fixtures, failure injection, From 001a78afcb2f0afaefa391909d6b44d692325698 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 15:22:53 +0800 Subject: [PATCH 2/9] docs(storage): sharpen Phase 4.5 ownership test from symbol-level review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the "place vs storage" split with the operative question: is the symbol still useful, unchanged, once the structured backend is SQLite? Applied per symbol it sorts into four groups rather than three, and forces two corrections. `WORLD_CANVAS_DIR_NAME` is a directory name, so it is Disk's — SQLite encodes World as a column with its own reserved key. `canvasRoot` cannot simply move: its body resolves through an index built from `space.json`, so it is the materialization anchor and has to be re-founded rather than relocated. Reading the implementations also corrects §12.5.3. `chatPromptLogPath` is a debug artifact its own comment says the app never reads, not unowned durable state; `acpSessionsPath`/`canvasAcpNamespace` are Agenetes' own store reached through a namespace root, so they belong to the agent domain. The finding underneath both: `.history/` conflates the Disk structured tier with per-Space state owned by other domains. Records the flat `modules/workspace/` decision and the dependency direction it settles. Co-Authored-By: Claude Opus 5 (1M context) --- docs/proposals/multi-backend-storage.md | 134 +++++++++++++++--------- 1 file changed, 83 insertions(+), 51 deletions(-) diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index e405fabba..1b2d44445 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1713,69 +1713,101 @@ storage-owned path (`chatDir`). The problem is therefore not mass violation by consumers — it is that the module they import is _mixed_, and its name and location assert an answer the port layer exists to keep open. -#### 12.5.2 `paths.ts` holds three populations, not one - -| Population | Members | Belongs to | -| ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | -| Disk structured layout | `SPACE_JSON_FILENAME`, `canvasJsonPath`, `nodesDir`, `nodeFilePath`, `historyDir`, `chatDir`, `changesPath`, `tasksPath`, `eventsPath`, `deltaLogPath` | `storage/backends/disk/` | -| Blob layout | `ARTIFACTS_DIR_NAME`, `artifactsDir`, `artifactPath` | `storage/backends/disk/` (blob) | -| Workspace-as-a-place | `canvasRoot`, `settingDir`, `userSkillsDir`, `workspaceMemoryPath`, `canvasMemoryDir`, `canvasMemoryPath`, `memoryStatePath`, `canvasAcpNamespace`, `WORLD_CANVAS_DIR_NAME` | `modules/workspace/` | +#### 12.5.2 The test that decides ownership + +A symbol belongs outside `storage/` only if it is **still useful, unchanged, +when the structured backend becomes SQLite**. Applied one symbol at a time, +that question sorts `paths.ts` into four groups — not the three an eyeball +reading suggests. + +| Outcome under a SQLite structured profile | Members | Belongs to | +| ---------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------- | +| Meaningless — the state became a table | `SPACE_JSON_FILENAME`, `canvasJsonPath`, `nodesDir`, `nodeFilePath`, `historyDir`, `changesPath`, `tasksPath`, `eventsPath`, `deltaLogPath`, `WORLD_CANVAS_DIR_NAME` | `storage/backends/disk/` | +| Still useful — belongs to the _other_ axis | `ARTIFACTS_DIR_NAME`, `artifactsDir`, `artifactPath` | `storage/backends/disk/` (blob) | +| Still useful — concept survives, body does not | `canvasRoot`, `chatDir` | re-founded on the materialization capability | +| Still useful, untouched | `settingDir`, `userSkillsDir`, `workspaceMemoryPath` | `modules/workspace/` | + +Two corrections the test forces against a looser reading: + +- `WORLD_CANVAS_DIR_NAME` is a _directory name_. SQLite encodes World as + `is_world = 1` with its own reserved collision key, and the portable concept + already exists as `SpaceRepository.worldId()`. The constant is Disk's. +- `canvasRoot` cannot simply move. Its body resolves through `canvasDirName()`, + which reads an index built from `space.json`, so under SQLite it silently + falls back to id-named directories. It is the materialization anchor and has + to be re-founded on something that does not consult the structured backend — + which is also the fix for the blob-scope hazard in §13. `canvas-dirs.ts` is not ambiguous at all: it builds its index by reading `space.json` from every directory. It is Disk structured-backend state that -currently lives outside the storage boundary, and it is the reason a SQLite -profile would silently fall back to id-named artifact directories (§13). - -`naming.ts` is misfiled in a third way — it is pure string logic with no I/O, -already re-exported rather than owned. Phase 5 extracts it to `utils/naming.ts` -as a side effect of needing it in a second backend; that extraction belongs -here instead, where it is the point rather than a side effect. - -#### 12.5.3 Path families with no owner - -Cross-referencing the families above against the Phase 5 schema surfaces a gap -this phase must record even though it does not close it. Six families map to -tables — spaces, nodes, events, changes, tasks, delta log. These do not map to -anything, and no port describes them: - -- `chatPromptLogPath` — per-thread prompt logs -- `acpSessionsPath` — ACP session state -- `canvasMemoryDir`, `canvasMemoryPath`, `memoryStatePath` — per-Space memory - -Under any non-Disk structured profile they remain files with no owner, which -would split one Space's state across two substrates without anyone deciding -that. Phase 4.5 assigns each family an owner — port, materialization -capability, or explicitly Disk-only — so that Phase 5's non-selectability has a -written reason rather than an accident. +currently lives outside the storage boundary. `space-dir-handles.ts` looks +substrate-specific but fails the test for the same reason — it exists so +Windows can rename a Space _directory_ safely, and under SQLite there is no +such rename. + +`naming.ts` is misfiled in a different way: pure string logic with no I/O, +already re-exported rather than owned. It passes the test trivially (a second +backend needs the identical rules) but has no business behind a `disk` +segment. Phase 5 extracts it to `utils/naming.ts` as a side effect of needing +it twice; that extraction belongs here, where it is the point. + +Because the residue that survives the test is three setting helpers and +`getWorkspacePath()` itself — none of it filesystem-specific — the target is a +**flat `modules/workspace/`** with no substrate segment. + +#### 12.5.3 `.history/` conflates two populations + +The directory holds the Disk structured backend's files — `events.jsonl`, +`tasks.json`, `delta-log.jsonl`, `.changes.json` — and, sharing the +same parent for no stated reason beyond travelling together in export bundles, +per-Space state owned by other domains: + +| Family | Owner under the test | +| -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `acpSessionsPath`, `canvasAcpNamespace` | Agent domain. It is Agenetes' own store — the namespace hands the ACP driver `storage.root` and the driver persists there. Survives the switch; its address inside `.history/` is the accident. | +| `chatPromptLogPath` | Debug artifact. Written only under `HUABU_DEBUG_PROMPT` and, per its own comment, never read by the app. Needs a location, not an owner. | +| `canvasMemoryDir`, `canvasMemoryPath`, `memoryStatePath` | Agent domain, materialized. AI-private Markdown an agent reads and writes as files. | + +Under SQLite the first population evaporates and the second still needs a +home. Phase 4.5 assigns each an owner so that the split is a decision rather +than a leftover; it implements none of them beyond the relocation. #### 12.5.4 Materialization becomes declared, not ambient -The consumers marked _materialization_ above cannot be served by a structured -port and should not be. An ACP agent, a file watcher, and RFS need a real -directory; that is a product requirement, not a leak. +The consumers above cannot be served by a structured port and should not be. +An ACP agent, a file watcher, and RFS need a real directory; that is a product +requirement, not a leak. + +What is wrong today is that they get it by reading an ambient layout module +whose name asserts the substrate. The port layer already has the right shape +one level down — `BlobScope` exposes `materialize()` for "consumers that +genuinely need a real filename" (§7). This phase gives Space trees the same +treatment: an explicit capability a consumer depends on by name and a profile +can decline to offer, rather than a path helper that is always simply there. -What is wrong today is that they get it by reading an ambient layout module. -The port layer already has the right shape one level down — `BlobScope` -exposes `materialize()` for "consumers that genuinely need a real filename" -(§7). This phase gives Space trees the same treatment: an explicit capability a -consumer depends on by name and a profile can decline to offer, rather than a -path helper that is always simply there. +Note the dependency direction this settles. `modules/workspace/` owns _which_ +directory is active; `storage/` owns what the Disk backend puts inside it. +Consumers needing a Space's real directory ask `storage/`, so the current +`storage → workspace/disk` edge inverts to `workspace ← storage` plus an +explicit capability — instead of every domain reaching into a shared layout. #### 12.5.5 Scope In: -- Relocate the Disk structured and blob layout families, and `canvas-dirs.ts`, - into `storage/backends/disk/`. -- Extract pure naming to `utils/naming.ts` (moved out of Phase 5). -- Keep the Workspace-as-a-place families in `modules/workspace/`, without a - `disk` segment asserting the substrate. -- Introduce the Space materialization capability and move the four - materialization consumers onto it. -- Move `agent/memory/analyzer.ts` off `chatDir` and onto the change/log port. -- Assign an owner to every §12.5.3 family; implement none of them. -- Extend the module-boundary test to fail when a non-storage file imports a - storage-owned layout symbol — the guard that stops this recurring. +1. Extract pure naming to `utils/naming.ts` and delete `workspace/disk/naming.ts` + outright — no shim, since every call site is updated in the same step. +2. Relocate the Disk structured and blob layout families, plus + `canvas-dirs.ts`, `name-index.ts`, `space-dir-handles.ts`, and + `world-canvas.ts`, into `storage/backends/disk/`. +3. Leave `settingDir`, `userSkillsDir`, and `workspaceMemoryPath` in a flat + `modules/workspace/`, with no substrate segment. +4. Introduce the Space materialization capability, re-found `canvasRoot` on it, + and move the ACP, watcher, memory, and World-bootstrap consumers onto it. +5. Move `agent/memory/analyzer.ts` off `chatDir`, and relocate the agent-owned + families of §12.5.3 into the agent domain. +6. Extend the module-boundary test to fail when a non-storage file imports a + storage-owned layout symbol — the guard that stops this recurring. Out: From a73057e6bee19ea715b0727877a043437c838001 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 15:25:52 +0800 Subject: [PATCH 3/9] refactor(server): move pure naming rules out of the Disk namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `workspace/disk/naming.ts` held no filesystem access — only string rules — while its path asserted a substrate. Every consumer that needed a safe filename or a comparison key had to import through a module named `disk` to get logic that has nothing to do with disks. Move the rules to `utils/naming.ts` and update all 15 call sites. The module is deleted outright rather than left as a forwarding shim: every importer is updated in the same commit, so a shim would only preserve the wrong direction for the next person to follow. Logic is unchanged. The file is byte-identical to the extraction Phase 5 performs as a side effect of needing the rules in a second backend, so that branch rebases onto this with the file already in place. Phase 4.5, step 1 of 6 (proposal §12.5.5). Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/modules/agent/node-ref.ts | 2 +- .../agent/tools/handlers/fs-read.test.ts | 2 +- .../server/src/modules/canvas/canvas.route.ts | 2 +- .../modules/preprocessing/stages/project.ts | 2 +- .../src/modules/remote_fs/rfs.route.test.ts | 2 +- .../backends/disk/cache-boundaries.test.ts | 2 +- .../backends/disk/legacy/canvas-store.ts | 2 +- .../backends/disk/space-repository.test.ts | 2 +- .../storage/backends/disk/space-repository.ts | 2 +- .../storage/backends/disk/space-title.ts | 2 +- .../backends/disk/structured-store.test.ts | 2 +- .../modules/storage/compatibility/canvas.ts | 2 +- .../storage/compatibility/parity.test.ts | 2 +- .../src/modules/workspace/disk/canvas-dirs.ts | 6 ++++- .../src/modules/workspace/disk/name-index.ts | 2 +- .../workspace/disk => utils}/naming.ts | 26 ++++++------------- 16 files changed, 27 insertions(+), 33 deletions(-) rename apps/server/src/{modules/workspace/disk => utils}/naming.ts (69%) diff --git a/apps/server/src/modules/agent/node-ref.ts b/apps/server/src/modules/agent/node-ref.ts index 10398c700..9521fb140 100644 --- a/apps/server/src/modules/agent/node-ref.ts +++ b/apps/server/src/modules/agent/node-ref.ts @@ -32,7 +32,7 @@ import { nodeRevisionOf } from '@huabu/shared/canvas-engine'; -import { toSafeFilename } from '../workspace/disk/naming.js'; +import { toSafeFilename } from '../../utils/naming.js'; import type { CanvasNodeType, WireNodeRef } from '@huabu/shared'; diff --git a/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts index 39b5e0bfe..bfd76b60f 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts @@ -8,8 +8,8 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { handleRead } from './fs-read.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { getCanvasStore } from '../../../storage/index.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; import { setWorkspacePath } from '../../../workspace.js'; interface ReadResult { diff --git a/apps/server/src/modules/canvas/canvas.route.ts b/apps/server/src/modules/canvas/canvas.route.ts index 09948f8aa..367a15558 100644 --- a/apps/server/src/modules/canvas/canvas.route.ts +++ b/apps/server/src/modules/canvas/canvas.route.ts @@ -43,6 +43,7 @@ import { WorldReferenceResolutionError, } from './world-reference-resolver.js'; import { MAX_UPLOAD_BYTES } from '../../upload-limits.js'; +import { toSafeFilename } from '../../utils/naming.js'; import { ARTIFACT_URL_REGEX } from '../artifact/utils.js'; import { getPreprocessDispatcher, getProfile } from '../preprocessing/index.js'; import { stripOfficeparserPreamble } from '../preprocessing/loaders/office-strip.js'; @@ -63,7 +64,6 @@ import { type UpdateNodeOutcome, } from '../storage/index.js'; import { canvasRoot, nodesDir, SPACE_JSON_FILENAME } from '../storage/paths.js'; -import { toSafeFilename } from '../workspace/disk/naming.js'; import { getWorkspacePath } from '../workspace.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; diff --git a/apps/server/src/modules/preprocessing/stages/project.ts b/apps/server/src/modules/preprocessing/stages/project.ts index c40a58321..9c96f311e 100644 --- a/apps/server/src/modules/preprocessing/stages/project.ts +++ b/apps/server/src/modules/preprocessing/stages/project.ts @@ -8,7 +8,7 @@ * from the outputs of all previous stages. */ -import { normalizeForCompare } from '../../workspace/disk/naming.js'; +import { normalizeForCompare } from '../../../utils/naming.js'; import { isLabelProtected } from '../label-policy.js'; import type { diff --git a/apps/server/src/modules/remote_fs/rfs.route.test.ts b/apps/server/src/modules/remote_fs/rfs.route.test.ts index f85b5c789..5bbd1f4f3 100644 --- a/apps/server/src/modules/remote_fs/rfs.route.test.ts +++ b/apps/server/src/modules/remote_fs/rfs.route.test.ts @@ -48,6 +48,7 @@ vi.mock('../agent/agenetes/drivers.js', () => ({ })); import rfsRoutes from './rfs.route.js'; +import { toSafeFilename } from '../../utils/naming.js'; import { agentNodeService } from '../agent/agent-node.service.js'; import { agentThreadResolver } from '../agent/agent-thread-resolver.js'; import { @@ -62,7 +63,6 @@ import { } from '../task/run-completion.service.js'; import { RunLaunchError, runLauncher } from '../task/run-launcher.js'; import { taskService } from '../task/task.service.js'; -import { toSafeFilename } from '../workspace/disk/naming.js'; import { canvasRoot } from '../workspace/disk/paths.js'; import { setWorkspacePath } from '../workspace.js'; diff --git a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts index e683c67b0..8cfe0e140 100644 --- a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts +++ b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts @@ -21,11 +21,11 @@ import { } from './legacy/canvas-store-cache.js'; import { NODE_TOMBSTONE_TTL_MS } from './legacy/node-tombstones.js'; import { DiskStructuredStore } from './structured-store.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { refreshCanvasDirIndex, registerCanvasDir, } from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; import { canvasRoot, SPACE_JSON_FILENAME, diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts index 483a31f19..6d484dfbc 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts @@ -43,6 +43,7 @@ import { parseFrontmatter, toFrontmatter, } from '../../../../../utils/markdown-frontmatter.js'; +import { toSafeFilename } from '../../../../../utils/naming.js'; import { patchCanvasDirTitle, refreshCanvasDirIndex, @@ -52,7 +53,6 @@ import { unregisterCanvasDir, } from '../../../../workspace/disk/canvas-dirs.js'; import { NameIndex } from '../../../../workspace/disk/name-index.js'; -import { toSafeFilename } from '../../../../workspace/disk/naming.js'; import { canvasJsonPath, canvasRoot, diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts index d8dd7b3ae..b4576a5da 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts @@ -24,8 +24,8 @@ vi.mock('../../../workspace.js', () => ({ import { resetStorageCache } from './legacy/canvas-store-cache.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; import { WORLD_CANVAS_DIR_NAME } from '../../../workspace/disk/paths.js'; import { describeSpaceRepositoryContract } from '../../ports/contracts/space-repository.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.ts b/apps/server/src/modules/storage/backends/disk/space-repository.ts index 29e242313..fdf35780c 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -26,6 +26,7 @@ import { titleVisibleAtDirectory, } from './space-title.js'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../../utils/fs.js'; +import { normalizeForCompare } from '../../../../utils/naming.js'; import { isWorldCanvasId, listAllCanvasDirEntries, @@ -35,7 +36,6 @@ import { requireWorldCanvasId, suggestCanvasDir, } from '../../../workspace/disk/canvas-dirs.js'; -import { normalizeForCompare } from '../../../workspace/disk/naming.js'; import { canvasJsonPath, SPACE_JSON_FILENAME, diff --git a/apps/server/src/modules/storage/backends/disk/space-title.ts b/apps/server/src/modules/storage/backends/disk/space-title.ts index fba4a023e..1cfffe995 100644 --- a/apps/server/src/modules/storage/backends/disk/space-title.ts +++ b/apps/server/src/modules/storage/backends/disk/space-title.ts @@ -6,7 +6,7 @@ import { normalizeForCompare, toSafeFilename, -} from '../../../workspace/disk/naming.js'; +} from '../../../../utils/naming.js'; /** * Whether `filename` is `base` carrying an allocation suffix (` (2)`, ` (3)`). diff --git a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts index 8bc6343ef..a26c1ffd3 100644 --- a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts @@ -18,8 +18,8 @@ import { resetStorageCache, } from './legacy/canvas-store-cache.js'; import { DiskStructuredStore } from './structured-store.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; import { tasksPath } from '../../../workspace/disk/paths.js'; import { describeSpaceLogsContract } from '../../ports/contracts/space-logs.contract.js'; import { describeStructuredStoreContract } from '../../ports/contracts/structured-store.contract.js'; diff --git a/apps/server/src/modules/storage/compatibility/canvas.ts b/apps/server/src/modules/storage/compatibility/canvas.ts index c24a12a0c..9121ed129 100644 --- a/apps/server/src/modules/storage/compatibility/canvas.ts +++ b/apps/server/src/modules/storage/compatibility/canvas.ts @@ -21,13 +21,13 @@ import { existsSync } from 'node:fs'; import path from 'node:path'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../utils/fs.js'; +import { toSafeFilename } from '../../../utils/naming.js'; import { listCanvasDirEntries, refreshCanvasDirIndex, registerCanvasDir, suggestCanvasDir, } from '../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../workspace/disk/naming.js'; import { canvasJsonPath, SPACE_JSON_FILENAME, diff --git a/apps/server/src/modules/storage/compatibility/parity.test.ts b/apps/server/src/modules/storage/compatibility/parity.test.ts index 175bf5919..6f7d6ac54 100644 --- a/apps/server/src/modules/storage/compatibility/parity.test.ts +++ b/apps/server/src/modules/storage/compatibility/parity.test.ts @@ -25,8 +25,8 @@ vi.mock('../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { toSafeFilename } from '../../../utils/naming.js'; import { refreshCanvasDirIndex } from '../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../workspace/disk/naming.js'; import { getCanvasStore, resetStorageCache, diff --git a/apps/server/src/modules/workspace/disk/canvas-dirs.ts b/apps/server/src/modules/workspace/disk/canvas-dirs.ts index 88aa94775..e89211c5c 100644 --- a/apps/server/src/modules/workspace/disk/canvas-dirs.ts +++ b/apps/server/src/modules/workspace/disk/canvas-dirs.ts @@ -11,9 +11,13 @@ import { existsSync, readdirSync, renameSync, statSync } from 'node:fs'; import path from 'node:path'; import { NameIndex, type NameIndexResult } from './name-index.js'; -import { dedupeName, normalizeForCompare, toSafeFilename } from './naming.js'; import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './paths.js'; import { readJsonStrict, sanitizeId } from '../../../utils/fs.js'; +import { + dedupeName, + normalizeForCompare, + toSafeFilename, +} from '../../../utils/naming.js'; import { getWorkspacePath } from '../../workspace.js'; export interface CanvasDirEntry { diff --git a/apps/server/src/modules/workspace/disk/name-index.ts b/apps/server/src/modules/workspace/disk/name-index.ts index 099e76a02..dedc01665 100644 --- a/apps/server/src/modules/workspace/disk/name-index.ts +++ b/apps/server/src/modules/workspace/disk/name-index.ts @@ -19,7 +19,7 @@ import { dedupeArtifactFilename, dedupeName, normalizeForCompare, -} from './naming.js'; +} from '../../../utils/naming.js'; export interface NameIndexEntry { /** Stable identifier — never written to disk as a filename. */ diff --git a/apps/server/src/modules/workspace/disk/naming.ts b/apps/server/src/utils/naming.ts similarity index 69% rename from apps/server/src/modules/workspace/disk/naming.ts rename to apps/server/src/utils/naming.ts index c7b33c434..0218b837d 100644 --- a/apps/server/src/modules/workspace/disk/naming.ts +++ b/apps/server/src/utils/naming.ts @@ -1,15 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -/** - * Filesystem-safe naming primitives. Pure functions; no I/O. - * - * Server-only: dedup helpers depend on `node:path`, and the - * single-name `toSafeFilename` rule is also confined here so the web - * bundle never has to apply it (it never sends `nodes/.md` - * paths to the server — the server enriches refs into `AgentNodeRef` - * with the pre-computed filename before any prompt rendering). - */ +/** Pure naming primitives shared by storage adapters and filesystem views. */ import path from 'node:path'; @@ -20,12 +12,9 @@ const WIN_RESERVED_RE = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i; export const MAX_FILENAME_LENGTH = 120; /** - * Turn a free-form display name into a filesystem-safe filename. - * - * Replaces only `\ / : * ? " < > |` and ASCII control characters with - * `_`. Spaces, hyphens, dots, parentheses and other characters are - * preserved verbatim — the LLM expects this because the rule is - * documented in `apps/server/src/prompt/skills/space/SKILL.md`. + * Normalize a logical label into the collision key used by today's Disk + * layout. SQL adapters use the same pure rule so title and label allocation + * remain portable without importing a filesystem backend. */ export function toSafeFilename( name?: string | null, @@ -36,8 +25,9 @@ export function toSafeFilename( safe = safe.replace(/^[.\s]+|[.\s]+$/g, ''); if (!safe) return fallback; if (WIN_RESERVED_RE.test(safe)) safe = `_${safe}`; - if (safe.length > MAX_FILENAME_LENGTH) + if (safe.length > MAX_FILENAME_LENGTH) { safe = safe.slice(0, MAX_FILENAME_LENGTH); + } return safe; } @@ -52,7 +42,7 @@ export function dedupeName(base: string, existing: Iterable): string { for (const name of existing) taken.add(normalizeForCompare(name)); if (!taken.has(normalizeForCompare(base))) return base; let i = 2; - while (taken.has(normalizeForCompare(`${base} (${i})`))) i++; + while (taken.has(normalizeForCompare(`${base} (${i})`))) i += 1; return `${base} (${i})`; } @@ -71,6 +61,6 @@ export function dedupeArtifactFilename( } if (!stemTaken.has(normalizeForCompare(stem))) return filename; let i = 2; - while (stemTaken.has(normalizeForCompare(`${stem} (${i})`))) i++; + while (stemTaken.has(normalizeForCompare(`${stem} (${i})`))) i += 1; return `${stem} (${i})${ext}`; } From 19e8d8e9ca91c566b6f514e5859a1c552c99276d Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 15:41:25 +0800 Subject: [PATCH 4/9] refactor(storage): move the Disk layout inside the storage boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `workspace/disk/paths.ts` described two unrelated things: where the Disk backend keeps a Space, and where the workspace keeps things that have nothing to do with a backend. Sorting them by the §12.5.2 test — is this still useful once the structured backend is SQLite? — splits cleanly. The Disk record and blob layout moves to `backends/disk/layout.ts`, together with `canvas-dirs.ts` and `name-index.ts`. Those two had zero consumers outside `storage/` already; they were filed under a `disk` segment in the workspace module while being, in fact, Disk structured-backend state built by parsing `space.json`. What stays behind is the residue that survives the test: `setting/`, user skills, user memory, and the per-Space state other domains own. Those anchor on a new `spaceDirectory()` capability exported by the composition root, so they no longer resolve through the Disk name index — the Space counterpart to `BlobScope.materialize()` (§12.5.4). The shim importer list shrinks by fourteen: most consumers turned out to want workspace-owned paths and now say so directly. Five entries join it as relocations of couplings that already existed, and the four remaining storage-owned reads outside the boundary — `chatDir`, `nodesDir`, `changesPath`, `artifactsDir` — are recorded in the boundary test rather than hidden, for step 5 to remove. Behavior is unchanged: every path resolves to the byte-identical location, and no Disk test expectation moved. Phase 4.5, step 2 of 6 (proposal §12.5.5). Co-Authored-By: Claude Opus 5 (1M context) --- .../src/modules/agent/acp/threads.route.ts | 2 +- apps/server/src/modules/agent/agent.route.ts | 2 +- .../server/src/modules/agent/agent.service.ts | 4 +- .../agent/conversation/prompt/debug-prompt.ts | 3 +- .../src/modules/agent/memory/analyzer.test.ts | 7 +- .../src/modules/agent/memory/analyzer.ts | 2 +- apps/server/src/modules/agent/memory/read.ts | 5 +- .../src/modules/agent/memory/sandbox.ts | 2 +- .../src/modules/agent/memory/trigger.ts | 6 +- .../src/modules/agent/skills.route.test.ts | 2 +- .../agent/tools/handlers/fs-write.test.ts | 2 +- .../modules/agent/tools/handlers/fs-write.ts | 2 +- .../modules/canvas/canvas-content-cas.test.ts | 2 +- .../src/modules/canvas/canvas-search.test.ts | 2 +- .../src/modules/canvas/canvas-search.ts | 2 +- .../src/modules/canvas/canvas.route.test.ts | 2 +- .../src/modules/remote_fs/rfs.route.test.ts | 9 +- .../storage/backends/disk/blob-store.ts | 2 +- .../backends/disk/cache-boundaries.test.ts | 10 +- .../backends}/disk/canvas-dirs.ts | 8 +- .../backends}/disk/canvas-dirs.world.test.ts | 4 +- .../disk/canvas-persistence-transaction.ts | 6 +- .../backends/disk/layout.test.ts} | 16 +- .../modules/storage/backends/disk/layout.ts | 140 ++++++++++++++++++ .../disk/legacy/canvas-store-cache.ts | 2 +- .../backends/disk/legacy/canvas-store.ts | 10 +- .../backends}/disk/name-index.ts | 2 +- .../storage/backends/disk/space-logs.ts | 2 +- .../storage/backends/disk/space-nodes.test.ts | 4 +- .../storage/backends/disk/space-record.ts | 4 +- .../backends/disk/space-repository.test.ts | 4 +- .../storage/backends/disk/space-repository.ts | 23 ++- .../storage/backends/disk/space-tasks.ts | 2 +- .../storage/backends/disk/space-write.test.ts | 4 +- .../backends/disk/storage-recovery.test.ts | 4 +- .../backends/disk/structured-store.test.ts | 4 +- .../server/src/modules/storage/canvas-dirs.ts | 2 +- .../modules/storage/compatibility/canvas.ts | 6 +- .../compatibility/delete-canvas.test.ts | 4 +- .../storage/compatibility/parity.test.ts | 2 +- apps/server/src/modules/storage/index.ts | 3 +- .../modules/storage/module-boundaries.test.ts | 25 +++- apps/server/src/modules/storage/paths.ts | 14 +- apps/server/src/modules/storage/storage.ts | 20 +++ .../src/modules/workspace/disk/paths.ts | 139 ++++------------- .../modules/workspace/disk/world-canvas.ts | 5 +- .../migrations/migrate-acp-sessions.ts | 2 +- apps/server/src/prompt/skills/loader.ts | 2 +- 48 files changed, 325 insertions(+), 206 deletions(-) rename apps/server/src/modules/{workspace => storage/backends}/disk/canvas-dirs.ts (97%) rename apps/server/src/modules/{workspace => storage/backends}/disk/canvas-dirs.world.test.ts (97%) rename apps/server/src/modules/{workspace/disk/paths.test.ts => storage/backends/disk/layout.test.ts} (70%) create mode 100644 apps/server/src/modules/storage/backends/disk/layout.ts rename apps/server/src/modules/{workspace => storage/backends}/disk/name-index.ts (99%) diff --git a/apps/server/src/modules/agent/acp/threads.route.ts b/apps/server/src/modules/agent/acp/threads.route.ts index 3c62ed126..e5983797a 100644 --- a/apps/server/src/modules/agent/acp/threads.route.ts +++ b/apps/server/src/modules/agent/acp/threads.route.ts @@ -50,7 +50,7 @@ import { buildReachbackEnv } from './reachback-env.js'; import { getExternalAgentRuntimeConfig } from './runtime-config.js'; import { resolveBindingRecipe } from './service.js'; import { renderExternalAgentSystemPreamble } from '../../../prompt/external-agent/system-preamble.js'; -import { canvasAcpNamespace } from '../../storage/paths.js'; +import { canvasAcpNamespace } from '../../workspace/disk/paths.js'; import { agenetes, EXTERNAL_DRIVER_KIND, diff --git a/apps/server/src/modules/agent/agent.route.ts b/apps/server/src/modules/agent/agent.route.ts index 3cc1005fc..0a7134024 100644 --- a/apps/server/src/modules/agent/agent.route.ts +++ b/apps/server/src/modules/agent/agent.route.ts @@ -31,7 +31,7 @@ import { import { buildChatEnvelope } from '../agent/conversation/envelope.js'; import { buildHistoryFromTurns } from '../agent/conversation/transcript/history.js'; import { getLLMModel } from '../agent/llm.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; +import { canvasAcpNamespace } from '../workspace/disk/paths.js'; import type { ControlMsg, Namespace } from '@agenetes/protocol'; import type { diff --git a/apps/server/src/modules/agent/agent.service.ts b/apps/server/src/modules/agent/agent.service.ts index bfb49c89f..77d3bba8f 100644 --- a/apps/server/src/modules/agent/agent.service.ts +++ b/apps/server/src/modules/agent/agent.service.ts @@ -18,8 +18,6 @@ * themselves and pull the relevant `tool_result` payload. */ -import { loadAgent, type AgentId } from '../../prompt/index.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; import { agenetes, INTERNAL_DRIVER_KIND, @@ -28,6 +26,8 @@ import { } from './agenetes/drivers.js'; import { createChatSubmission } from './agenetes/handle.js'; import { buildHuabuPiWorkloadSpec } from './agenetes/pi-driver.js'; +import { loadAgent, type AgentId } from '../../prompt/index.js'; +import { canvasAcpNamespace } from '../workspace/disk/paths.js'; import { renderInternalAgentInputs } from './conversation/prompt/build-prompt.js'; import { dumpAssembledPrompt } from './conversation/prompt/debug-prompt.js'; import { type ToolScope } from './tools/index.js'; diff --git a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts index a84292af4..acd3a3c9d 100644 --- a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts +++ b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts @@ -17,7 +17,8 @@ import { appendFileSync } from 'node:fs'; import { mkdirp } from '../../../../utils/fs.js'; -import { chatDir, chatPromptLogPath } from '../../../storage/paths.js'; +import { chatDir } from '../../../storage/paths.js'; +import { chatPromptLogPath } from '../../../workspace/disk/paths.js'; import type { Context } from '@earendil-works/pi-ai'; import type { FastifyBaseLogger } from 'fastify'; diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index 5cd69230d..f4179dabb 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -19,9 +19,14 @@ vi.mock('../../storage/index.js', () => ({ getStructuredStore: vi.fn() })); vi.mock('../../workspace/disk/paths.js', () => ({ canvasMemoryPath: (canvasId: string) => `${physicalState.root}/${canvasId}/.memory/space.md`, + workspaceMemoryPath: () => `${physicalState.root}/setting/user.md`, +})); +// `chatDir` is Disk record layout, so it moved inside the storage boundary +// (proposal §12.5.2). The chat digest still reads it directly — the last +// storage-owned path this module touches, tracked as §12.5.5 step 5. +vi.mock('../../storage/paths.js', () => ({ chatDir: (canvasId: string) => `${physicalState.root}/${canvasId}/.history/chat`, - workspaceMemoryPath: () => `${physicalState.root}/setting/user.md`, })); import { runAgent } from '../agent.service.js'; diff --git a/apps/server/src/modules/agent/memory/analyzer.ts b/apps/server/src/modules/agent/memory/analyzer.ts index 46cb5e40a..21c96a6af 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -36,9 +36,9 @@ import { type CanvasFile, type SpaceHandle, } from '../../storage/index.js'; +import { chatDir } from '../../storage/paths.js'; import { canvasMemoryPath, - chatDir, workspaceMemoryPath, } from '../../workspace/disk/paths.js'; diff --git a/apps/server/src/modules/agent/memory/read.ts b/apps/server/src/modules/agent/memory/read.ts index a719e8255..8d6968dbb 100644 --- a/apps/server/src/modules/agent/memory/read.ts +++ b/apps/server/src/modules/agent/memory/read.ts @@ -15,7 +15,10 @@ import { existsSync, readFileSync } from 'node:fs'; -import { workspaceMemoryPath, canvasMemoryPath } from '../../storage/paths.js'; +import { + workspaceMemoryPath, + canvasMemoryPath, +} from '../../workspace/disk/paths.js'; /** * Read the user memory body. diff --git a/apps/server/src/modules/agent/memory/sandbox.ts b/apps/server/src/modules/agent/memory/sandbox.ts index dfbf1ebc6..397bbe1a2 100644 --- a/apps/server/src/modules/agent/memory/sandbox.ts +++ b/apps/server/src/modules/agent/memory/sandbox.ts @@ -31,7 +31,7 @@ import { userSkillsDir, canvasMemoryDir, canvasMemoryPath, -} from '../../storage/paths.js'; +} from '../../workspace/disk/paths.js'; /** Thrown by every resolver below on out-of-sandbox attempts. */ export class MemorySandboxError extends Error { diff --git a/apps/server/src/modules/agent/memory/trigger.ts b/apps/server/src/modules/agent/memory/trigger.ts index 843e855aa..908ddca71 100644 --- a/apps/server/src/modules/agent/memory/trigger.ts +++ b/apps/server/src/modules/agent/memory/trigger.ts @@ -26,11 +26,11 @@ import { existsSync } from 'node:fs'; import { atomicWriteJson, mkdirp, readJson } from '../../../utils/fs.js'; import { createKeyedMutex } from '../../../utils/keyed-mutex.js'; +import { spaceDirectory } from '../../storage/index.js'; import { memoryStatePath, canvasMemoryDir, - canvasRoot, -} from '../../storage/paths.js'; +} from '../../workspace/disk/paths.js'; /** Op-count threshold that triggers a memory analysis pass. */ export const OP_THRESHOLD = 50; @@ -86,7 +86,7 @@ export function writeMemoryState(canvasId: string, state: MemoryState): void { // file. Same hazard for any in-flight memory worker that calls // `markAnalyzed` post-delete. Skip the write when the canvas root // is gone; losing one bookkeeping write is harmless. - if (!existsSync(canvasRoot(canvasId))) return; + if (!existsSync(spaceDirectory(canvasId))) return; mkdirp(canvasMemoryDir(canvasId)); atomicWriteJson(memoryStatePath(canvasId), state); } diff --git a/apps/server/src/modules/agent/skills.route.test.ts b/apps/server/src/modules/agent/skills.route.test.ts index 897e0d4bd..15871a370 100644 --- a/apps/server/src/modules/agent/skills.route.test.ts +++ b/apps/server/src/modules/agent/skills.route.test.ts @@ -30,7 +30,7 @@ import { invalidateSkillCache, type LoadedSkill, } from '../../prompt/skills/loader.js'; -import { userSkillsDir } from '../storage/paths.js'; +import { userSkillsDir } from '../workspace/disk/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { SkillCatalogueEntry } from '@huabu/shared'; diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts index 644fb0e64..7ea06862e 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts @@ -30,7 +30,7 @@ import { canvasMemoryPath, userSkillsDir, workspaceMemoryPath, -} from '../../../storage/paths.js'; +} from '../../../workspace/disk/paths.js'; import { setWorkspacePath } from '../../../workspace.js'; interface ParsedResult { diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.ts index f9413c225..30671a195 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.ts @@ -36,7 +36,7 @@ import { canvasMemoryDir, settingDir, userSkillsDir, -} from '../../../storage/paths.js'; +} from '../../../workspace/disk/paths.js'; import { resolveLongTermPath, resolveUserSkillPath, diff --git a/apps/server/src/modules/canvas/canvas-content-cas.test.ts b/apps/server/src/modules/canvas/canvas-content-cas.test.ts index eab47243b..bb0b55b5f 100644 --- a/apps/server/src/modules/canvas/canvas-content-cas.test.ts +++ b/apps/server/src/modules/canvas/canvas-content-cas.test.ts @@ -30,7 +30,7 @@ import { getStorage, setStorageForTesting, } from '../storage/index.js'; -import { nodesDir } from '../workspace/disk/paths.js'; +import { nodesDir } from '../storage/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { BlobScope, BlobStore } from '../storage/index.js'; diff --git a/apps/server/src/modules/canvas/canvas-search.test.ts b/apps/server/src/modules/canvas/canvas-search.test.ts index cf878d9cd..084ba04f4 100644 --- a/apps/server/src/modules/canvas/canvas-search.test.ts +++ b/apps/server/src/modules/canvas/canvas-search.test.ts @@ -43,7 +43,7 @@ vi.mock('../agent/agenetes/drivers.js', () => ({ }, })); -vi.mock('../storage/paths.js', async (importActual) => ({ +vi.mock('../workspace/disk/paths.js', async (importActual) => ({ ...((await importActual()) as Record), canvasAcpNamespace: (canvasId: string) => ({ name: canvasId, root: '' }), })); diff --git a/apps/server/src/modules/canvas/canvas-search.ts b/apps/server/src/modules/canvas/canvas-search.ts index a3eab3ab0..117c4dce2 100644 --- a/apps/server/src/modules/canvas/canvas-search.ts +++ b/apps/server/src/modules/canvas/canvas-search.ts @@ -44,7 +44,7 @@ import { import { agenetes } from '../agent/agenetes/drivers.js'; import { chatEnvelopeFromSubmission } from '../agent/agenetes/handle.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; +import { canvasAcpNamespace } from '../workspace/disk/paths.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; import type { AgentTurn } from '@agenetes/protocol'; diff --git a/apps/server/src/modules/canvas/canvas.route.test.ts b/apps/server/src/modules/canvas/canvas.route.test.ts index 1620b1d90..4cacdc019 100644 --- a/apps/server/src/modules/canvas/canvas.route.test.ts +++ b/apps/server/src/modules/canvas/canvas.route.test.ts @@ -37,7 +37,7 @@ import { getStructuredStore, resetStorageCache, } from '../storage/index.js'; -import { changesPath } from '../workspace/disk/paths.js'; +import { changesPath } from '../storage/paths.js'; import { withSpaceDirHandlesReleased } from '../workspace/disk/space-dir-handles.js'; import { setWorkspacePath } from '../workspace.js'; diff --git a/apps/server/src/modules/remote_fs/rfs.route.test.ts b/apps/server/src/modules/remote_fs/rfs.route.test.ts index 5bbd1f4f3..7fcd331b3 100644 --- a/apps/server/src/modules/remote_fs/rfs.route.test.ts +++ b/apps/server/src/modules/remote_fs/rfs.route.test.ts @@ -56,14 +56,17 @@ import { agentThreadService, } from '../agent/agent-thread.service.js'; import * as selectableProfiles from '../agent/selectable-agent-profile.js'; -import { getCanvasStore, resetStorageCache } from '../storage/index.js'; +import { + getCanvasStore, + resetStorageCache, + spaceDirectory, +} from '../storage/index.js'; import { RunCompletionError, runCompletionService, } from '../task/run-completion.service.js'; import { RunLaunchError, runLauncher } from '../task/run-launcher.js'; import { taskService } from '../task/task.service.js'; -import { canvasRoot } from '../workspace/disk/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { FixedAgentNodeTarget } from '../agent/agent-thread-resolver.js'; @@ -156,7 +159,7 @@ describe('GET /api/rfs/:canvasId/skill', () => { it('returns only the bundled root guide without authorization', async () => { seedNote('c1', 'node-1', 'Anchor', 'content'); writeFileSync( - join(canvasRoot('c1'), 'skill.md'), + join(spaceDirectory('c1'), 'skill.md'), '# Private Space Override', 'utf8', ); diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 50919956b..88dd8eb6d 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -26,8 +26,8 @@ import { import path from 'node:path'; import { pipeline } from 'node:stream/promises'; +import { artifactsDir } from './layout.js'; import { renameOverWithRetry } from '../../../../utils/fs.js'; -import { artifactsDir } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { createBlobLease, normalizeBlobName } from '../../ports/blob.js'; diff --git a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts index 8cfe0e140..41903fb0a 100644 --- a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts +++ b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts @@ -14,6 +14,8 @@ import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex, registerCanvasDir } from './canvas-dirs.js'; +import { canvasRoot, SPACE_JSON_FILENAME } from './layout.js'; import { forgetCanvasStore, getCanvasStore, @@ -22,14 +24,6 @@ import { import { NODE_TOMBSTONE_TTL_MS } from './legacy/node-tombstones.js'; import { DiskStructuredStore } from './structured-store.js'; import { toSafeFilename } from '../../../../utils/naming.js'; -import { - refreshCanvasDirIndex, - registerCanvasDir, -} from '../../../workspace/disk/canvas-dirs.js'; -import { - canvasRoot, - SPACE_JSON_FILENAME, -} from '../../../workspace/disk/paths.js'; import { setWorkspacePath } from '../../../workspace.js'; import type { diff --git a/apps/server/src/modules/workspace/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts similarity index 97% rename from apps/server/src/modules/workspace/disk/canvas-dirs.ts rename to apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index e89211c5c..1780e1065 100644 --- a/apps/server/src/modules/workspace/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -10,15 +10,15 @@ import { existsSync, readdirSync, renameSync, statSync } from 'node:fs'; import path from 'node:path'; +import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; import { NameIndex, type NameIndexResult } from './name-index.js'; -import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './paths.js'; -import { readJsonStrict, sanitizeId } from '../../../utils/fs.js'; +import { readJsonStrict, sanitizeId } from '../../../../utils/fs.js'; import { dedupeName, normalizeForCompare, toSafeFilename, -} from '../../../utils/naming.js'; -import { getWorkspacePath } from '../../workspace.js'; +} from '../../../../utils/naming.js'; +import { getWorkspacePath } from '../../../workspace.js'; export interface CanvasDirEntry { id: string; diff --git a/apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts similarity index 97% rename from apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts rename to apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts index 1f54b9666..e71c32fd4 100644 --- a/apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts @@ -15,7 +15,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const workspaceState = vi.hoisted(() => ({ path: '' })); -vi.mock('../../workspace.js', () => ({ +vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); @@ -29,7 +29,7 @@ import { renameCanvasDirOnDisk, suggestCanvasDir, } from './canvas-dirs.js'; -import { CanvasStore } from '../../storage/index.js'; +import { CanvasStore } from '../../index.js'; function writeCanvas( root: string, diff --git a/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts b/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts index 098377144..064af388d 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts @@ -27,13 +27,9 @@ import { } from 'node:fs'; import path from 'node:path'; +import { canvasJsonPath, deltaLogPath, nodesDir } from './layout.js'; import { repairJsonLinesTail } from '../../../../utils/fs.js'; import { parseFrontmatter } from '../../../../utils/markdown-frontmatter.js'; -import { - canvasJsonPath, - deltaLogPath, - nodesDir, -} from '../../../workspace/disk/paths.js'; interface FileSnapshot { path: string; diff --git a/apps/server/src/modules/workspace/disk/paths.test.ts b/apps/server/src/modules/storage/backends/disk/layout.test.ts similarity index 70% rename from apps/server/src/modules/workspace/disk/paths.test.ts rename to apps/server/src/modules/storage/backends/disk/layout.test.ts index 9f39762f5..711784c48 100644 --- a/apps/server/src/modules/workspace/disk/paths.test.ts +++ b/apps/server/src/modules/storage/backends/disk/layout.test.ts @@ -7,11 +7,11 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { refreshCanvasDirIndex } from './canvas-dirs.js'; -import { canvasRoot } from './paths.js'; -import { setWorkspacePath } from '../../workspace.js'; +import { refreshCanvasDirIndex, registerCanvasDir } from './canvas-dirs.js'; +import { canvasRoot } from './layout.js'; +import { setWorkspacePath } from '../../../workspace.js'; -describe('Disk Workspace paths', () => { +describe('Disk layout', () => { let workspacePath: string; beforeEach(() => { @@ -38,4 +38,12 @@ describe('Disk Workspace paths', () => { expect(() => canvasRoot(canvasId)).toThrow(/Invalid canvasId/); }, ); + + it('rejects an indexed directory that escapes the active Workspace', () => { + registerCanvasDir('canvas-a', '../escape', null); + + expect(() => canvasRoot('canvas-a')).toThrow( + /escapes the active Workspace/, + ); + }); }); diff --git a/apps/server/src/modules/storage/backends/disk/layout.ts b/apps/server/src/modules/storage/backends/disk/layout.ts new file mode 100644 index 000000000..fe2a400ac --- /dev/null +++ b/apps/server/src/modules/storage/backends/disk/layout.ts @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Where the Disk backend puts a Space. + * + * Every path here answers "how does *this* backend store that", so none of it + * survives a switch to a structured backend that keeps the same state in + * tables — which is the test that moved it inside the storage boundary + * (proposal §12.5.2). Nothing outside `storage/` may depend on these names. + * + * Layout under `//`: + * + * space.json topology; carries the stable canvasId + * nodes/.md per-node markdown (id in frontmatter) + * .artifacts/ raw uploads (hidden dir) + * .history/ + * chat/.changes.json pending change-review records + * events.jsonl + * tasks.json + * delta-log.jsonl + * + * `.history/` also hosts state this backend does not own — ACP sessions and + * the debug prompt log — which the agent domain addresses through the + * materialization capability instead (§12.5.3). + */ + +import path from 'node:path'; + +import { canvasDirName } from './canvas-dirs.js'; +import { sanitizeId } from '../../../../utils/fs.js'; +import { getWorkspacePath } from '../../../workspace.js'; + +/** + * The directory backing a Space. + * + * Resolved through {@link canvasDirName} rather than the canvasId, because + * Disk files a Space under its title and that name moves on rename. This is + * also the materialization anchor the rest of the app reaches by way of + * `storage`'s `spaceDirectory()`. + */ +export function canvasRoot(canvasId: string): string { + const safeId = sanitizeId(canvasId, 'canvasId'); + const workspaceRoot = path.resolve(getWorkspacePath()); + const resolved = path.resolve(workspaceRoot, canvasDirName(safeId)); + if (!resolved.startsWith(`${workspaceRoot}${path.sep}`)) { + throw new Error(`Canvas path escapes the active Workspace: "${canvasId}"`); + } + return resolved; +} + +/** + * On-disk topology filename. Agent- and user-visible (L1), so it uses the + * Space vocabulary; the TypeScript type of its contents stays `CanvasFile` + * (L2 internal). See migrate-canvas-to-space.ts for the legacy rename. + */ +export const SPACE_JSON_FILENAME = 'space.json'; +export const WORLD_CANVAS_DIR_NAME = '.world'; + +export function canvasJsonPath(canvasId: string): string { + return path.join(canvasRoot(canvasId), SPACE_JSON_FILENAME); +} + +export function nodesDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), 'nodes'); +} + +export function nodeFilePath(canvasId: string, filename: string): string { + const base = path.basename(filename); + if (!base || base === '.' || base === '..') { + throw new Error(`Invalid node filename: "${filename}"`); + } + return path.join(nodesDir(canvasId), base); +} + +/** Hidden directory holding raw uploaded files keyed by artifactId. */ +export const ARTIFACTS_DIR_NAME = '.artifacts'; + +export function artifactsDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), ARTIFACTS_DIR_NAME); +} + +export function artifactPath(canvasId: string, filename: string): string { + const base = path.basename(filename); + if (!base || base === '.' || base === '..') { + throw new Error(`Invalid artifact filename: "${filename}"`); + } + return path.join(artifactsDir(canvasId), base); +} + +/** + * The hidden per-Space tier. Shared with non-storage owners today; see the + * module note above and §12.5.3. + */ +export const HISTORY_DIR_NAME = '.history'; + +export function historyDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), HISTORY_DIR_NAME); +} + +export function chatDir(canvasId: string): string { + return path.join(historyDir(canvasId), 'chat'); +} + +/** + * Pending change-review records for an ACP thread (the "what the agent + * changed" card). A mutable sidecar — entries are removed on accept / + * revert — so it lives apart from the append-only `.turns.jsonl` log. + */ +export function changesPath(canvasId: string, threadId: string): string { + return path.join( + chatDir(canvasId), + `${sanitizeId(threadId, 'threadId')}.changes.json`, + ); +} + +export function tasksPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'tasks.json'); +} + +export function eventsPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'events.jsonl'); +} + +/** + * Append-only delta log for headless executor batches (M2). + * + * One JSONL line per `POST /api/canvas/:canvasId/execute` call that + * actually mutated state. Lines carry the canvas version, run id, + * originator, applied commands, and the resulting structural deltas + * (see `shared/canvas-engine/delta.ts`). Used by M3 broadcast / replay + * and as the persistence anchor for `space.json`'s monotonic version + * counter. + * + * Lives next to `events.jsonl` so the entire `.history/` tier travels + * together in canvas export bundles. + */ +export function deltaLogPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'delta-log.jsonl'); +} diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts index 96bcb0582..5a4ee1d61 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts @@ -24,8 +24,8 @@ import path from 'node:path'; import { CanvasStore } from './canvas-store.js'; import { sanitizeId } from '../../../../../utils/fs.js'; -import { refreshCanvasDirIndex } from '../../../../workspace/disk/canvas-dirs.js'; import { getWorkspacePath } from '../../../../workspace.js'; +import { refreshCanvasDirIndex } from '../canvas-dirs.js'; const MAX_CACHE = 16; const cache = new Map(); diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts index 6d484dfbc..d020fd9bd 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts @@ -44,6 +44,8 @@ import { toFrontmatter, } from '../../../../../utils/markdown-frontmatter.js'; import { toSafeFilename } from '../../../../../utils/naming.js'; +import { getWorkspacePath } from '../../../../workspace.js'; +import { assertSpaceMutationAllowed } from '../../../space-lifecycle-admission.js'; import { patchCanvasDirTitle, refreshCanvasDirIndex, @@ -51,8 +53,7 @@ import { renameCanvasDirOnDisk, isWorldCanvasId, unregisterCanvasDir, -} from '../../../../workspace/disk/canvas-dirs.js'; -import { NameIndex } from '../../../../workspace/disk/name-index.js'; +} from '../canvas-dirs.js'; import { canvasJsonPath, canvasRoot, @@ -62,9 +63,8 @@ import { eventsPath, nodeFilePath, nodesDir, -} from '../../../../workspace/disk/paths.js'; -import { getWorkspacePath } from '../../../../workspace.js'; -import { assertSpaceMutationAllowed } from '../../../space-lifecycle-admission.js'; +} from '../layout.js'; +import { NameIndex } from '../name-index.js'; import { readValidCanvasFile } from '../space-record-validation.js'; import { titleVisibleAtDirectory } from '../space-title.js'; diff --git a/apps/server/src/modules/workspace/disk/name-index.ts b/apps/server/src/modules/storage/backends/disk/name-index.ts similarity index 99% rename from apps/server/src/modules/workspace/disk/name-index.ts rename to apps/server/src/modules/storage/backends/disk/name-index.ts index dedc01665..672bc51da 100644 --- a/apps/server/src/modules/workspace/disk/name-index.ts +++ b/apps/server/src/modules/storage/backends/disk/name-index.ts @@ -19,7 +19,7 @@ import { dedupeArtifactFilename, dedupeName, normalizeForCompare, -} from '../../../utils/naming.js'; +} from '../../../../utils/naming.js'; export interface NameIndexEntry { /** Stable identifier — never written to disk as a filename. */ diff --git a/apps/server/src/modules/storage/backends/disk/space-logs.ts b/apps/server/src/modules/storage/backends/disk/space-logs.ts index f6a93262f..322966b84 100644 --- a/apps/server/src/modules/storage/backends/disk/space-logs.ts +++ b/apps/server/src/modules/storage/backends/disk/space-logs.ts @@ -26,13 +26,13 @@ import { type CanvasChangeRecord, } from '@huabu/shared/canvas-engine'; +import { changesPath, eventsPath } from './layout.js'; import { readDiskSpaceRecord } from './space-record.js'; import { atomicWriteJson, readJsonLinesStrict, readJsonStrict, } from '../../../../utils/fs.js'; -import { changesPath, eventsPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts index 80af0b56d..b3bb4604e 100644 --- a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts @@ -16,6 +16,8 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { nodesDir } from './layout.js'; import { getCanvasStore, resetStorageCache, @@ -23,8 +25,6 @@ import { import { DiskSpaceNodes } from './space-nodes.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { nodesDir } from '../../../workspace/disk/paths.js'; import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceNodesContract } from '../../ports/contracts/space-nodes.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-record.ts b/apps/server/src/modules/storage/backends/disk/space-record.ts index 5fe26384c..3a1737129 100644 --- a/apps/server/src/modules/storage/backends/disk/space-record.ts +++ b/apps/server/src/modules/storage/backends/disk/space-record.ts @@ -11,9 +11,9 @@ import path from 'node:path'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { canvasJsonPath } from './layout.js'; import { readValidCanvasFile } from './space-record-validation.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { canvasJsonPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import type { CanvasStore } from './legacy/canvas-store.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts index b4576a5da..dd7ab747d 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts @@ -21,12 +21,12 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { WORLD_CANVAS_DIR_NAME } from './layout.js'; import { resetStorageCache } from './legacy/canvas-store-cache.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; import { toSafeFilename } from '../../../../utils/naming.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { WORLD_CANVAS_DIR_NAME } from '../../../workspace/disk/paths.js'; import { describeSpaceRepositoryContract } from '../../ports/contracts/space-repository.contract.js'; import type { CanvasFile } from '../../../canvas/persistence-types.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.ts b/apps/server/src/modules/storage/backends/disk/space-repository.ts index fdf35780c..80384135d 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -15,6 +15,16 @@ import path from 'node:path'; +import { + isWorldCanvasId, + listAllCanvasDirEntries, + listCanvasDirEntries, + refreshCanvasDirIndex, + registerCanvasDir, + requireWorldCanvasId, + suggestCanvasDir, +} from './canvas-dirs.js'; +import { canvasJsonPath, SPACE_JSON_FILENAME } from './layout.js'; import { forgetCanvasStore, getCanvasStore, @@ -27,19 +37,6 @@ import { } from './space-title.js'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../../utils/fs.js'; import { normalizeForCompare } from '../../../../utils/naming.js'; -import { - isWorldCanvasId, - listAllCanvasDirEntries, - listCanvasDirEntries, - refreshCanvasDirIndex, - registerCanvasDir, - requireWorldCanvasId, - suggestCanvasDir, -} from '../../../workspace/disk/canvas-dirs.js'; -import { - canvasJsonPath, - SPACE_JSON_FILENAME, -} from '../../../workspace/disk/paths.js'; import { withSpaceDirHandlesReleased } from '../../../workspace/disk/space-dir-handles.js'; import { getWorkspacePath } from '../../../workspace.js'; import { diff --git a/apps/server/src/modules/storage/backends/disk/space-tasks.ts b/apps/server/src/modules/storage/backends/disk/space-tasks.ts index 2aa7bded4..35e91e34e 100644 --- a/apps/server/src/modules/storage/backends/disk/space-tasks.ts +++ b/apps/server/src/modules/storage/backends/disk/space-tasks.ts @@ -14,9 +14,9 @@ import { type TaskStoreSnapshot, } from '@huabu/shared'; +import { tasksPath } from './layout.js'; import { readDiskSpaceRecord } from './space-record.js'; import { atomicWriteJson, readJsonStrict } from '../../../../utils/fs.js'; -import { tasksPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-write.test.ts b/apps/server/src/modules/storage/backends/disk/space-write.test.ts index 7c2c7155f..55f1e3773 100644 --- a/apps/server/src/modules/storage/backends/disk/space-write.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-write.test.ts @@ -13,6 +13,8 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { nodesDir } from './layout.js'; import { getCanvasStore, resetStorageCache, @@ -20,8 +22,6 @@ import { import { DiskSpaceRepository } from './space-repository.js'; import { createDiskSpaceWrite } from './space-write.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { nodesDir } from '../../../workspace/disk/paths.js'; import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceWriteContract } from '../../ports/contracts/space-write.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts b/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts index 893c2fbec..26bac81f9 100644 --- a/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts +++ b/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts @@ -25,13 +25,13 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { changesPath, eventsPath } from './layout.js'; import { getCanvasStore, resetStorageCache, } from './legacy/canvas-store-cache.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { changesPath, eventsPath } from '../../../workspace/disk/paths.js'; import { canvasBlobs, createStorage, diff --git a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts index a26c1ffd3..72278d483 100644 --- a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts @@ -13,14 +13,14 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { tasksPath } from './layout.js'; import { getCanvasStore, resetStorageCache, } from './legacy/canvas-store-cache.js'; import { DiskStructuredStore } from './structured-store.js'; import { toSafeFilename } from '../../../../utils/naming.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { tasksPath } from '../../../workspace/disk/paths.js'; import { describeSpaceLogsContract } from '../../ports/contracts/space-logs.contract.js'; import { describeStructuredStoreContract } from '../../ports/contracts/structured-store.contract.js'; diff --git a/apps/server/src/modules/storage/canvas-dirs.ts b/apps/server/src/modules/storage/canvas-dirs.ts index 3ce194125..5f8d94485 100644 --- a/apps/server/src/modules/storage/canvas-dirs.ts +++ b/apps/server/src/modules/storage/canvas-dirs.ts @@ -10,4 +10,4 @@ * site may import it (enforced by the module-boundary test). */ -export * from '../workspace/disk/canvas-dirs.js'; +export * from './backends/disk/canvas-dirs.js'; diff --git a/apps/server/src/modules/storage/compatibility/canvas.ts b/apps/server/src/modules/storage/compatibility/canvas.ts index 9121ed129..87b830510 100644 --- a/apps/server/src/modules/storage/compatibility/canvas.ts +++ b/apps/server/src/modules/storage/compatibility/canvas.ts @@ -22,17 +22,17 @@ import path from 'node:path'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../utils/fs.js'; import { toSafeFilename } from '../../../utils/naming.js'; +import { getWorkspacePath } from '../../workspace.js'; import { listCanvasDirEntries, refreshCanvasDirIndex, registerCanvasDir, suggestCanvasDir, -} from '../../workspace/disk/canvas-dirs.js'; +} from '../backends/disk/canvas-dirs.js'; import { canvasJsonPath, SPACE_JSON_FILENAME, -} from '../../workspace/disk/paths.js'; -import { getWorkspacePath } from '../../workspace.js'; +} from '../backends/disk/layout.js'; import { getCanvasStore } from '../backends/disk/legacy/canvas-store-cache.js'; import { deleteSpace } from '../storage.js'; diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index 06db5ea7c..cbcda58cc 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -14,9 +14,9 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { executeOnServer } from '../../canvas/canvas-executor.js'; -import { refreshCanvasDirIndex } from '../../workspace/disk/canvas-dirs.js'; -import { artifactPath, canvasJsonPath } from '../../workspace/disk/paths.js'; import { DiskBlobStore } from '../backends/disk/blob-store.js'; +import { refreshCanvasDirIndex } from '../backends/disk/canvas-dirs.js'; +import { artifactPath, canvasJsonPath } from '../backends/disk/layout.js'; import { resetStorageCache } from '../backends/disk/legacy/canvas-store-cache.js'; import { DiskStructuredStore } from '../backends/disk/structured-store.js'; import { getCanvasStore } from '../index.js'; diff --git a/apps/server/src/modules/storage/compatibility/parity.test.ts b/apps/server/src/modules/storage/compatibility/parity.test.ts index 6f7d6ac54..e0f177db7 100644 --- a/apps/server/src/modules/storage/compatibility/parity.test.ts +++ b/apps/server/src/modules/storage/compatibility/parity.test.ts @@ -26,7 +26,7 @@ vi.mock('../../workspace.js', () => ({ })); import { toSafeFilename } from '../../../utils/naming.js'; -import { refreshCanvasDirIndex } from '../../workspace/disk/canvas-dirs.js'; +import { refreshCanvasDirIndex } from '../backends/disk/canvas-dirs.js'; import { getCanvasStore, resetStorageCache, diff --git a/apps/server/src/modules/storage/index.ts b/apps/server/src/modules/storage/index.ts index 6663dd732..37446c833 100644 --- a/apps/server/src/modules/storage/index.ts +++ b/apps/server/src/modules/storage/index.ts @@ -28,7 +28,7 @@ export { getWorldCanvasId, isWorldCanvasId, requireWorldCanvasId, -} from '../workspace/disk/canvas-dirs.js'; +} from './backends/disk/canvas-dirs.js'; export { withCanvasMutex, updateNode } from '../canvas/write-coordinator.js'; export type { UpdateNodeOptions, @@ -53,6 +53,7 @@ export { getStructuredStore, initStorage, setStorageForTesting, + spaceDirectory, storageHealth, } from './storage.js'; export type { SpaceDeleteOutcome, Storage } from './storage.js'; diff --git a/apps/server/src/modules/storage/module-boundaries.test.ts b/apps/server/src/modules/storage/module-boundaries.test.ts index 560abae88..ca1e1a926 100644 --- a/apps/server/src/modules/storage/module-boundaries.test.ts +++ b/apps/server/src/modules/storage/module-boundaries.test.ts @@ -185,7 +185,20 @@ describe('storage dependency direction', () => { ); const nonAdapter = importers.filter((f) => !inLayer(f, 'backends')); - expect(nonAdapter).toEqual(['modules/storage/storage.ts']); + // `storage.ts` selects the backend. The rest reach a *named* Disk module + // because the Disk layout and its directory index moved inside the + // boundary in Phase 4.5 (§12.5.2): the barrel re-exports the Disk World + // helpers, the two shims forward Disk-capability imports, and the + // compatibility facade is Disk-coupled by construction. Each entry + // disappears as its consumers move onto ports and the materialization + // capability (§12.5.5 step 5). + expect(nonAdapter).toEqual([ + 'modules/storage/canvas-dirs.ts', + 'modules/storage/compatibility/canvas.ts', + 'modules/storage/index.ts', + 'modules/storage/paths.ts', + 'modules/storage/storage.ts', + ]); }); }); @@ -325,6 +338,16 @@ describe('root forwarding shims', () => { 'modules/remote_fs/rfs.route.ts', 'modules/remote_fs/skill.ts', 'prompt/skills/loader.ts', + // Phase 4.5 relocations, not new couplings. Each of these already read + // a Disk-owned path; it read it from `workspace/disk/paths.js`, which + // this phase emptied of Disk layout (§12.5.2). The same call site now + // names the shim instead. Fourteen entries left this list in the same + // change, because their symbols turned out to be workspace-owned. + 'modules/agent/memory/analyzer.test.ts', + 'modules/canvas/canvas-content-cas.test.ts', + 'modules/canvas/canvas.route.test.ts', + 'modules/workspace/disk/world-canvas.ts', + 'modules/workspace/migrations/migrate-acp-sessions.ts', ], }; diff --git a/apps/server/src/modules/storage/paths.ts b/apps/server/src/modules/storage/paths.ts index 76dd0395b..5f829c8d6 100644 --- a/apps/server/src/modules/storage/paths.ts +++ b/apps/server/src/modules/storage/paths.ts @@ -2,12 +2,14 @@ // Licensed under the MIT license. /** - * @deprecated Forwarding shim — the Workspace layout owns these now. + * @deprecated Forwarding shim — the Disk backend owns its layout now. * - * Import from `modules/workspace/disk/paths.js` instead. This file exists - * only so the many existing physical-Disk capability imports keep resolving - * while they migrate; it must never contain logic, and no new call site may - * import it (enforced by the module-boundary test). + * Import from `storage/backends/disk/layout.js` if you are inside the storage + * module; everyone else wants `spaceDirectory()` from `storage/index.js` or + * the workspace-owned paths in `modules/workspace/disk/paths.js`. This file + * exists only so the remaining physical-Disk capability imports keep + * resolving while they migrate; it must never contain logic, and no new call + * site may import it (enforced by the module-boundary test). */ -export * from '../workspace/disk/paths.js'; +export * from './backends/disk/layout.js'; diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index 24ad60b9d..c11a93e0b 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -28,6 +28,7 @@ import { getWorkspacePath, } from '../workspace.js'; import { DiskBlobStore } from './backends/disk/blob-store.js'; +import { canvasRoot } from './backends/disk/layout.js'; import { DiskStructuredStore } from './backends/disk/structured-store.js'; import { parseStorageProfile, @@ -330,6 +331,25 @@ export async function storageHealth(): Promise { return Promise.all([storage.structured.health(), storage.blobs.health()]); } +/** + * The real directory backing a Space — the materialization capability. + * + * Some consumers genuinely need a filesystem path rather than a record: an + * ACP agent needs a working directory, the external watcher needs something + * to watch, RFS exposes a tree. That is a product requirement, not a leak + * (proposal §12.5.4), and it is the Space-level counterpart to + * `BlobScope.materialize()`. + * + * It lives in the composition root because only this module may ask a named + * backend where anything is. Every profile selectable today materializes, so + * this resolves unconditionally; a backend that stores Spaces without a + * directory would refuse here rather than hand back a path that does not + * exist. + */ +export function spaceDirectory(canvasId: string): string { + return canvasRoot(canvasId); +} + /** * Swap the active storage, returning a restore function. * diff --git a/apps/server/src/modules/workspace/disk/paths.ts b/apps/server/src/modules/workspace/disk/paths.ts index d31355ca8..4dad391c1 100644 --- a/apps/server/src/modules/workspace/disk/paths.ts +++ b/apps/server/src/modules/workspace/disk/paths.ts @@ -2,86 +2,53 @@ // Licensed under the MIT license. /** - * Storage paths. + * Workspace paths that outlive the storage backend. + * + * What remains here passes the §12.5.2 test: it is still meaningful once the + * structured backend keeps Spaces in tables. Two populations qualify. + * + * - Workspace-level, no canvasId: `setting/` and the user memory file. + * Untouched by a backend switch. + * - Per-Space state owned by *other* domains — memory, ACP sessions, the + * debug prompt log — which need a materialized directory but not the Disk + * record layout. They anchor on `spaceDirectory()` from the storage + * facade, so they no longer consult the Disk name index (§12.5.4). + * + * The Disk record and blob layout moved to `storage/backends/disk/layout.ts`. * * Layout under `/`: * * setting/ user-owned, cross-Space * user.md user memory (preferences) * skills//SKILL.md user / memory-agent authored skills - * / name = sanitised Space title - * space.json carries the stable canvasId - * nodes/.md per-node markdown (id in frontmatter) - * .artifacts/ raw uploads (hidden dir) + * / * .memory/ Space-scoped memory (AI-private) * space.md Space memory body * state.json memory worker bookkeeping * .history/ - * chat/.turns.jsonl finalized turns (append-only) - * chat/.active.json in-progress turn (partial) - * events.jsonl * acp-sessions.json per-thread ACP sessionId map (optional) + * chat/.prompt.log debug dump, opt-in * - * Naming convention: anything prefixed with `.` is hidden / AI-private - * (`.artifacts`, `.history`, `.memory`); anything without the prefix is - * user-visible (`nodes/`, `setting/`). + * Naming convention: anything prefixed with `.` is hidden / AI-private; + * anything without the prefix is user-visible. */ import path from 'node:path'; -import { canvasDirName } from './canvas-dirs.js'; -import { sanitizeId } from '../../../utils/fs.js'; +import { spaceDirectory } from '../../storage/index.js'; import { getWorkspacePath } from '../../workspace.js'; import type { Namespace } from '@agenetes/protocol'; - -export function canvasRoot(canvasId: string): string { - const safeId = sanitizeId(canvasId, 'canvasId'); - const workspaceRoot = path.resolve(getWorkspacePath()); - const resolved = path.resolve(workspaceRoot, canvasDirName(safeId)); - if (!resolved.startsWith(`${workspaceRoot}${path.sep}`)) { - throw new Error(`Canvas path escapes the active Workspace: "${canvasId}"`); - } - return resolved; -} - /** - * On-disk topology filename. Agent- and user-visible (L1), so it uses the - * Space vocabulary; the TypeScript type of its contents stays `CanvasFile` - * (L2 internal). See migrate-canvas-to-space.ts for the legacy rename. + * The `.history/` tier is named by the Disk backend, which owns most of what + * is in it. The families below sit there only because they were written next + * to it; the duplicated literal keeps that colocation visible as the accident + * it is, rather than binding this module to the backend's layout (§12.5.3). */ -export const SPACE_JSON_FILENAME = 'space.json'; -export const WORLD_CANVAS_DIR_NAME = '.world'; - -export function canvasJsonPath(canvasId: string): string { - return path.join(canvasRoot(canvasId), SPACE_JSON_FILENAME); -} - -export function nodesDir(canvasId: string): string { - return path.join(canvasRoot(canvasId), 'nodes'); -} +const LEGACY_HISTORY_DIR_NAME = '.history'; -export function nodeFilePath(canvasId: string, filename: string): string { - const base = path.basename(filename); - if (!base || base === '.' || base === '..') { - throw new Error(`Invalid node filename: "${filename}"`); - } - return path.join(nodesDir(canvasId), base); -} - -/** Hidden directory holding raw uploaded files keyed by artifactId. */ -export const ARTIFACTS_DIR_NAME = '.artifacts'; - -export function artifactsDir(canvasId: string): string { - return path.join(canvasRoot(canvasId), ARTIFACTS_DIR_NAME); -} - -export function artifactPath(canvasId: string, filename: string): string { - const base = path.basename(filename); - if (!base || base === '.' || base === '..') { - throw new Error(`Invalid artifact filename: "${filename}"`); - } - return path.join(artifactsDir(canvasId), base); +function legacyHistoryDir(canvasId: string): string { + return path.join(spaceDirectory(canvasId), LEGACY_HISTORY_DIR_NAME); } // ─── Memory module paths ─────────────────────────────────────────────────── @@ -89,7 +56,7 @@ export function artifactPath(canvasId: string, filename: string): string { // Two scopes: // - User memory (`/setting/user.md`): // cross-Space user preferences / profile. User-editable. -// - Space memory (`/.memory/`): hidden, +// - Space memory (`/.memory/`): hidden, // AI-private working notes for *this* Space. The leading `.` puts // it in the same hidden tier as `.history/` and `.artifacts/`. @@ -102,7 +69,7 @@ export function workspaceMemoryPath(): string { export const WORKING_MEMORY_DIR_NAME = '.memory'; export function canvasMemoryDir(canvasId: string): string { - return path.join(canvasRoot(canvasId), WORKING_MEMORY_DIR_NAME); + return path.join(spaceDirectory(canvasId), WORKING_MEMORY_DIR_NAME); } /** Working memory body for a canvas. */ @@ -140,26 +107,6 @@ export function userSkillsDir(): string { return path.join(settingDir(), 'skills'); } -export function historyDir(canvasId: string): string { - return path.join(canvasRoot(canvasId), '.history'); -} - -export function chatDir(canvasId: string): string { - return path.join(historyDir(canvasId), 'chat'); -} - -/** - * Pending change-review records for an ACP thread (the "what the agent - * changed" card). A mutable sidecar — entries are removed on accept / - * revert — so it lives apart from the append-only `.turns.jsonl` log. - */ -export function changesPath(canvasId: string, threadId: string): string { - return path.join( - chatDir(canvasId), - `${sanitizeId(threadId, 'threadId')}.changes.json`, - ); -} - /** * Human-readable debug dump of the assembled prompt sent to the agent, * one block per turn with strong turn separators. Append-only, written @@ -168,36 +115,12 @@ export function changesPath(canvasId: string, threadId: string): string { */ export function chatPromptLogPath(canvasId: string, threadId: string): string { return path.join( - chatDir(canvasId), + legacyHistoryDir(canvasId), + 'chat', `${sanitizeId(threadId, 'threadId')}.prompt.log`, ); } -export function tasksPath(canvasId: string): string { - return path.join(historyDir(canvasId), 'tasks.json'); -} - -export function eventsPath(canvasId: string): string { - return path.join(historyDir(canvasId), 'events.jsonl'); -} - -/** - * Append-only delta log for headless executor batches (M2). - * - * One JSONL line per `POST /api/canvas/:canvasId/execute` call that - * actually mutated state. Lines carry the canvas version, run id, - * originator, applied commands, and the resulting structural deltas - * (see `shared/canvas-engine/delta.ts`). Used by M3 broadcast / replay - * and as the persistence anchor for `space.json`'s monotonic version - * counter. - * - * Lives next to `events.jsonl` so the entire `.history/` tier travels - * together in canvas export bundles. - */ -export function deltaLogPath(canvasId: string): string { - return path.join(historyDir(canvasId), 'delta-log.jsonl'); -} - /** * ACP session persistence — maps each Huabu thread on this canvas * to the live ACP `sessionId` returned by `session/new`, so we can @@ -210,7 +133,7 @@ export function deltaLogPath(canvasId: string): string { * an external agent. */ export function acpSessionsPath(canvasId: string): string { - return path.join(historyDir(canvasId), 'acp-sessions.json'); + return path.join(legacyHistoryDir(canvasId), 'acp-sessions.json'); } /** @@ -225,6 +148,6 @@ export function acpSessionsPath(canvasId: string): string { export function canvasAcpNamespace(canvasId: string): Namespace { return { name: canvasId, - storage: canvasId ? { root: historyDir(canvasId) } : undefined, + storage: canvasId ? { root: legacyHistoryDir(canvasId) } : undefined, }; } diff --git a/apps/server/src/modules/workspace/disk/world-canvas.ts b/apps/server/src/modules/workspace/disk/world-canvas.ts index 5982cb842..c2f41e26a 100644 --- a/apps/server/src/modules/workspace/disk/world-canvas.ts +++ b/apps/server/src/modules/workspace/disk/world-canvas.ts @@ -6,8 +6,11 @@ import path from 'node:path'; import { createId } from '@huabu/shared'; -import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './paths.js'; import { atomicWriteJson, readJson, sanitizeId } from '../../../utils/fs.js'; +import { + SPACE_JSON_FILENAME, + WORLD_CANVAS_DIR_NAME, +} from '../../storage/paths.js'; import type { CanvasFile } from '../../canvas/persistence-types.js'; diff --git a/apps/server/src/modules/workspace/migrations/migrate-acp-sessions.ts b/apps/server/src/modules/workspace/migrations/migrate-acp-sessions.ts index 0353268e9..e128bfcf2 100644 --- a/apps/server/src/modules/workspace/migrations/migrate-acp-sessions.ts +++ b/apps/server/src/modules/workspace/migrations/migrate-acp-sessions.ts @@ -45,7 +45,7 @@ import { agentMetadataSchema } from '@agenetes/protocol'; import { parseMigratableV3Records } from './legacy/acp-sessions-v3.js'; import { readJson } from '../../../utils/fs.js'; -import { SPACE_JSON_FILENAME } from '../disk/paths.js'; +import { SPACE_JSON_FILENAME } from '../../storage/paths.js'; import type { AcpWorkloadSpec } from '../../agent/agenetes/drivers.js'; import type { AgentStateSnapshot, Namespace } from '@agenetes/protocol'; diff --git a/apps/server/src/prompt/skills/loader.ts b/apps/server/src/prompt/skills/loader.ts index 804693832..4001a298a 100644 --- a/apps/server/src/prompt/skills/loader.ts +++ b/apps/server/src/prompt/skills/loader.ts @@ -49,7 +49,7 @@ import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { userSkillsDir } from '../../modules/storage/paths.js'; +import { userSkillsDir } from '../../modules/workspace/disk/paths.js'; import { getWorkspacePath } from '../../modules/workspace.js'; import { getLogger } from '../../utils/logger.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; From a0959cee6eebc3be73ecd0769f486635069562d9 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 15:55:24 +0800 Subject: [PATCH 5/9] refactor(storage): remove a dead chat digest, add BlobScope.owns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of the storage-owned reads outside the boundary turned out to need neither a port nor a relocation. `readChatDigest` scanned `.history/chat/*.json` for a `{ messages: [] }` shape that two migrations retired: turns moved to `.turns.jsonl` and then into the Agenetes Tier-2 store under `chat_v2/`. The only live `.json` writer left in that directory is the change-review sidecar, whose payload is an array with no `messages` key, so every file failed the guard — the reader had returned nothing in any migrated workspace. Its test pointed `chatDir` at a directory that does not exist, so the branch never ran there either. Serving it with a `SpaceChats` port would have handed storage authority over data it does not own — threads and turns belong to the agent runtime, as the legacy store says outright — and obliged every future backend to model Agenetes' turn log. Removed instead; whether the memory agent should see turns at all is filed as a follow-up against `agenetes.history()`, the call canvas-search already uses. `latestChatTs` and its cursor stay: they are the resume point a reinstated digest would need. `import-node-src.ts` asked whether a path was already inside `.artifacts/` by rebuilding the Disk artifacts directory. `BlobScope.owns()` lets the scope answer for itself — pure, synchronous, and `false` on a backend that stores nothing locally. It is covered in the shared blob contract, so both answers are pinned for every adapter. Also corrects the §12.5.1 census, which was built by searching for `workspace/disk/` and so missed every consumer reaching the same symbols through the `storage/paths.js` shim: six production files outside the boundary read a storage-owned symbol, not one. Phase 4.5, step 5 of 6 (proposal §12.5.5, findings in §12.5.7). Co-Authored-By: Claude Opus 5 (1M context) --- .../src/modules/agent/memory/analyzer.test.ts | 7 - .../src/modules/agent/memory/analyzer.ts | 171 +++--------------- .../modules/artifact/artifact.route.test.ts | 1 + .../src/modules/artifact/send-blob.test.ts | 1 + .../src/modules/canvas/import-node-src.ts | 16 +- .../storage/backends/disk/blob-store.ts | 5 + .../compatibility/delete-canvas.test.ts | 2 + apps/server/src/modules/storage/ports/blob.ts | 15 ++ .../ports/contracts/blob-store.contract.ts | 20 ++ apps/server/src/modules/storage/storage.ts | 3 + docs/proposals/multi-backend-storage.md | 62 ++++++- 11 files changed, 129 insertions(+), 174 deletions(-) diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index f4179dabb..06faa8f69 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -21,13 +21,6 @@ vi.mock('../../workspace/disk/paths.js', () => ({ `${physicalState.root}/${canvasId}/.memory/space.md`, workspaceMemoryPath: () => `${physicalState.root}/setting/user.md`, })); -// `chatDir` is Disk record layout, so it moved inside the storage boundary -// (proposal §12.5.2). The chat digest still reads it directly — the last -// storage-owned path this module touches, tracked as §12.5.5 step 5. -vi.mock('../../storage/paths.js', () => ({ - chatDir: (canvasId: string) => - `${physicalState.root}/${canvasId}/.history/chat`, -})); import { runAgent } from '../agent.service.js'; import { runAnalysisPass } from './analyzer.js'; diff --git a/apps/server/src/modules/agent/memory/analyzer.ts b/apps/server/src/modules/agent/memory/analyzer.ts index 21c96a6af..89061aee7 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -8,8 +8,8 @@ * The worker calls {@link runAnalysisPass}. We: * * 1. Build the system prompt from `prompt/agents/memory/AGENT.md`. - * 2. Assemble a compact context bundle from backend-owned Space records and - * logs plus the remaining Disk-owned chat and memory surfaces. + * 2. Assemble a compact context bundle from backend-owned Space records + * and logs, plus the memory surfaces. * 3. Run the sub-agent against that context. The agent's only way * to affect the world is via the `fs_write` tool, whose handler * routes by virtual path into the writers in `./writers.ts`. @@ -24,11 +24,8 @@ * mutations on the same disk targets apply in declared order. */ -import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; -import path from 'node:path'; +import { existsSync, readFileSync } from 'node:fs'; -import { runAgent } from '../agent.service.js'; -import { readMemoryState } from './trigger.js'; import { loadAgent, listSkills } from '../../../prompt/index.js'; import { getStructuredStore, @@ -36,11 +33,11 @@ import { type CanvasFile, type SpaceHandle, } from '../../storage/index.js'; -import { chatDir } from '../../storage/paths.js'; import { canvasMemoryPath, workspaceMemoryPath, } from '../../workspace/disk/paths.js'; +import { runAgent } from '../agent.service.js'; import type { MemoryLogger } from './index.js'; import type { WriteResult } from './writers.js'; @@ -54,8 +51,6 @@ import type { Context, Message } from '@earendil-works/pi-ai'; */ const MAX_NODES_IN_SNAPSHOT = 60; const MAX_EVENTS_IN_DIGEST = 100; -const MAX_CHAT_TURNS_IN_DIGEST = 12; -const MAX_THREAD_SCAN = 6; /** * Run one memory analysis pass. @@ -65,12 +60,10 @@ const MAX_THREAD_SCAN = 6; * rejections are *not* errors — they come back as `ok:false` tool * results which we surface in the returned summary. * - * The returned `latestChatTs` is the maximum message timestamp the - * pass scanned (independent of which were summarised into the - * prompt). The worker persists it as `lastSeenThreadCursor` via - * {@link markAnalyzed} so subsequent passes only look at strictly - * newer turns — without it the chat digest would re-include the - * same messages every threshold crossing. + * `latestChatTs` is always `null` since the chat digest was removed; see + * {@link ContextBundle}. The worker still persists it as + * `lastSeenThreadCursor`, which is the resume point a reinstated digest + * would need. */ export type AnalysisPassResult = | { @@ -174,10 +167,18 @@ interface ContextBundle { messages: Message[]; summary: string; /** - * Max message timestamp scanned by the chat digest, or `null` when - * no new turns were seen. Carries to the worker so it can persist - * `lastSeenThreadCursor` and the next pass only looks at strictly - * newer turns. + * Always `null`. The chat digest that produced it read + * `/.history/chat/*.json` for a `{ messages: [] }` shape that two + * migrations retired — turns moved to `.turns.jsonl` and then into the + * Agenetes Tier-2 store under `chat_v2/`, so the only files left matching + * that glob are change-record arrays with no `messages` key. The reader + * had therefore returned nothing for some time, in production and in a + * test that pointed it at a non-existent directory. + * + * The field and its `lastSeenThreadCursor` plumbing survive the removal + * because they are the resume point any reinstated digest needs; the + * turns themselves belong to the agent runtime and are read through + * `agenetes.history()`, not through storage. See proposal §12.5.7. */ latestChatTs: number | null; } @@ -207,18 +208,6 @@ async function assembleContext( }); parts.push(`${snapshot.nodeCount} nodes`); - const state = readMemoryState(canvasId); - const chat = readChatDigest(canvasId, state.lastSeenThreadCursor); - if (chat) { - messages.push({ - role: 'user', - content: `[SYSTEM Chat digest since ${ - state.lastSeenThreadCursor ?? 'start' - }]\n${chat.text}`, - timestamp: Date.now(), - }); - parts.push(`${chat.turns} chat turns`); - } const eventRows = await handle.events.read(MAX_EVENTS_IN_DIGEST); const events = readEventsDigest(eventRows); if (events) { @@ -247,7 +236,7 @@ async function assembleContext( return { messages, summary: parts.join(', ') || '(empty)', - latestChatTs: chat?.latestTs ?? null, + latestChatTs: null, }; } @@ -293,116 +282,6 @@ function summariseNode(node: unknown): string { return `- [${type}] ${id} "${label.slice(0, 60)}"${pos}`; } -interface ChatDigest { - text: string; - turns: number; - /** - * Max `timestamp` seen across every message that passed the `since` - * filter, regardless of whether it landed in the digest body. The - * worker persists this as the next pass's `lastSeenThreadCursor` - * so the chat digest monotonically advances. - */ - latestTs: number | null; -} - -/** - * Pull a digest of recent chat turns from `/.history/chat/`. - * - * Strategy: - * - List every thread file, sorted by `mtime` descending. - * - Walk up to {@link MAX_THREAD_SCAN} threads, scanning each - * message in turn. For each message: - * - drop turns older than `since` (the bookkeeping's - * `lastSeenThreadCursor`); - * - track `latestTs` = max(`timestamp`) of every survivor, - * so the caller can advance the cursor even when the - * digest body itself was capped; - * - skip system / non-user / non-assistant rows; - * - emit up to {@link MAX_CHAT_TURNS_IN_DIGEST} into the body. - * - For each emitted turn, render the role + the first ~200 chars - * of the content (or `[tool: name]` for assistant turns that - * only carried tool calls). - */ -function readChatDigest( - canvasId: string, - since: number | null, -): ChatDigest | null { - const dir = chatDir(canvasId); - if (!existsSync(dir)) return null; - let files: string[]; - try { - files = readdirSync(dir); - } catch { - return null; - } - const threads = files - .filter((f) => f.endsWith('.json')) - .map((f) => path.join(dir, f)) - .map((p) => ({ path: p, mtime: safeMtime(p) })) - .filter((t) => t.mtime !== null) - .sort((a, b) => (b.mtime ?? 0) - (a.mtime ?? 0)) - .slice(0, MAX_THREAD_SCAN); - - const lines: string[] = []; - let turns = 0; - let latestTs: number | null = null; - for (const thread of threads) { - let ctx: { messages?: unknown[] } | null; - try { - ctx = JSON.parse(readFileSync(thread.path, 'utf8')) as { - messages?: unknown[]; - }; - } catch { - continue; - } - if (!ctx?.messages || !Array.isArray(ctx.messages)) continue; - for (const m of ctx.messages) { - if (!m || typeof m !== 'object') continue; - const msg = m as { - role?: string; - content?: unknown; - timestamp?: number; - }; - const ts = typeof msg.timestamp === 'number' ? msg.timestamp : null; - if (since !== null && ts !== null && ts <= since) continue; - - // Advance latestTs for every message that survived the `since` - // filter — not just the ones we end up emitting. That way the - // cursor still advances when MAX_CHAT_TURNS_IN_DIGEST has been - // reached, and we don't re-scan the same prefix next pass. - if (ts !== null && (latestTs === null || ts > latestTs)) { - latestTs = ts; - } - - const role = msg.role; - if (role !== 'user' && role !== 'assistant') continue; - if (turns >= MAX_CHAT_TURNS_IN_DIGEST) continue; - const text = digestMessageContent(msg.content); - if (text.startsWith('[SYSTEM')) continue; - lines.push(`${role}: ${text}`); - turns++; - } - } - if (turns === 0 && latestTs === null) return null; - return { text: lines.join('\n'), turns, latestTs }; -} - -function digestMessageContent(content: unknown): string { - if (typeof content === 'string') return content.slice(0, 200); - if (!Array.isArray(content)) return ''; - const parts: string[] = []; - for (const block of content) { - if (!block || typeof block !== 'object') continue; - const b = block as { type?: string; text?: string; name?: string }; - if (b.type === 'text' && typeof b.text === 'string') { - parts.push(b.text); - } else if (b.type === 'toolCall' && typeof b.name === 'string') { - parts.push(`[tool: ${b.name}]`); - } - } - return parts.join(' ').slice(0, 200); -} - interface EventsDigest { text: string; count: number; @@ -462,11 +341,3 @@ function readFileSafe(file: string): string { return ''; } } - -function safeMtime(file: string): number | null { - try { - return statSync(file).mtimeMs; - } catch { - return null; - } -} diff --git a/apps/server/src/modules/artifact/artifact.route.test.ts b/apps/server/src/modules/artifact/artifact.route.test.ts index 3010fbec7..26edea2e4 100644 --- a/apps/server/src/modules/artifact/artifact.route.test.ts +++ b/apps/server/src/modules/artifact/artifact.route.test.ts @@ -99,6 +99,7 @@ function installDeleteBlock(canvasId: string): { hasMany: (names) => delegate.hasMany(names), list: () => delegate.list(), materialize: (name) => delegate.materialize(name), + owns: (absolutePath: string) => delegate.owns(absolutePath), async deleteAll() { if (ref.canvasId === canvasId) { started.resolve(); diff --git a/apps/server/src/modules/artifact/send-blob.test.ts b/apps/server/src/modules/artifact/send-blob.test.ts index cded24eaf..b0ab28794 100644 --- a/apps/server/src/modules/artifact/send-blob.test.ts +++ b/apps/server/src/modules/artifact/send-blob.test.ts @@ -50,6 +50,7 @@ function fakeScope(blobs: Record): BlobScope { async materialize() { return null; }, + owns: () => false, async deleteAll() {}, }; } diff --git a/apps/server/src/modules/canvas/import-node-src.ts b/apps/server/src/modules/canvas/import-node-src.ts index e960dafaf..88ddac586 100644 --- a/apps/server/src/modules/canvas/import-node-src.ts +++ b/apps/server/src/modules/canvas/import-node-src.ts @@ -41,7 +41,6 @@ import { toPhysicalRel, } from '../agent/tools/handlers/fs-sandbox.js'; import { canvasBlobs } from '../storage/index.js'; -import { artifactsDir } from '../storage/paths.js'; import type { CanvasStore } from '../storage/index.js'; @@ -266,17 +265,10 @@ async function resolveImportedSrc( return null; } - // Already inside `.artifacts/` (or a bare artifact key that resolves there) - // — nothing to import. This inspects the real local filesystem, as the - // whole local-import branch does; only the write below goes through the - // blob port. - const artifactsRoot = artifactsDir(canvasId); - if ( - absPath === artifactsRoot || - absPath.startsWith(artifactsRoot + path.sep) - ) { - return null; - } + // Already a managed blob (or a bare artifact key that resolves there) + // — nothing to import. The scope answers for its own storage rather than + // this module rebuilding the Disk artifacts path. + if (canvasBlobs(canvasId).owns(absPath)) return null; // A bare key like `art_abc.png` resolves under the canvas root but has no // file on disk there — leave it so the web resolver builds the artifact URL. diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 88dd8eb6d..437cc6935 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -228,6 +228,11 @@ class DiskBlobScope implements BlobScope { return createBlobLease(blobPath(dir, info.name), async () => {}); } + owns(absolutePath: string): boolean { + const dir = this.#resolveDir(); + return absolutePath === dir || absolutePath.startsWith(dir + path.sep); + } + async deleteAll(): Promise { await rm(this.#resolveDir(), { recursive: true, force: true }); } diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index cbcda58cc..e6e341a69 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -114,6 +114,7 @@ class OrderRecordingBlobStore implements BlobStore { scope.hasMany(names), list: (): Promise => scope.list(), materialize: (name: string) => scope.materialize(name), + owns: (absolutePath: string): boolean => scope.owns(absolutePath), deleteAll: async (): Promise => { seen.push(existsSync(canvasJsonPath(ref.canvasId))); await scope.deleteAll(); @@ -180,6 +181,7 @@ class ControllableBlobStore implements BlobStore { scope.hasMany(names), list: (): Promise => scope.list(), materialize: (name: string) => scope.materialize(name), + owns: (absolutePath: string): boolean => scope.owns(absolutePath), deleteAll: async (): Promise => { this.deleteCalls += 1; this.deleteStarted.resolve(); diff --git a/apps/server/src/modules/storage/ports/blob.ts b/apps/server/src/modules/storage/ports/blob.ts index c710d0f05..022e5abfb 100644 --- a/apps/server/src/modules/storage/ports/blob.ts +++ b/apps/server/src/modules/storage/ports/blob.ts @@ -115,6 +115,21 @@ export interface BlobScope { /** A temporary real path for this blob, or null when absent. */ materialize(name: string): Promise; + /** + * Whether an absolute local path is already stored by this scope. + * + * The question a local-import path asks before copying a file in: a + * source that is *already* a managed blob needs no import. Answering it + * by rebuilding the scope's directory is what made callers reach for the + * Disk layout, so the scope answers for itself. + * + * Pure and synchronous — a path comparison, never I/O, and it says + * nothing about whether the blob exists. A backend that stores nothing + * locally always returns `false`, which is the correct answer there: no + * local path it was handed can already be one of its blobs. + */ + owns(absolutePath: string): boolean; + /** * Remove every blob in this scope. * diff --git a/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts b/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts index 9623fcd8f..79332627a 100644 --- a/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts +++ b/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts @@ -286,6 +286,26 @@ export function describeBlobStoreContract( ); }); + it('claims a materialized path and disclaims an unrelated one', async () => { + const blobs = await scope(); + await blobs.put('owned.txt', Buffer.from('mine')); + + // A backend that hands out a real path must recognize it again; + // one that materializes nothing locally owns no local path at all. + // Both are correct, and the two answers must not be mixed within a + // single implementation — that is what this pins. + const lease = await blobs.materialize('owned.txt'); + if (lease) { + expect(blobs.owns(lease.path)).toBe(true); + await lease.release(); + } + + expect(blobs.owns('/definitely/not/this/scope/owned.txt')).toBe(false); + // Absence is not the question `owns` answers: a path that would belong + // to the scope still belongs to it before anything is written there. + expect(typeof blobs.owns('/tmp/never-written.txt')).toBe('boolean'); + }); + it('deleteAll empties the scope and is idempotent', async () => { const blobs = await scope(); await blobs.put('i1.txt', Buffer.from('x')); diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index c11a93e0b..3f1ec8330 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -320,6 +320,9 @@ export function canvasBlobs(canvasId: string): BlobScope { materialize(name: string): Promise { return delegate.materialize(name); }, + owns(absolutePath: string): boolean { + return delegate.owns(absolutePath); + }, deleteAll(): Promise { return delegate.deleteAll(); }, diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index 1b2d44445..a80598b10 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1706,12 +1706,26 @@ Measured on `6b43798a`. Non-storage **production** files importing Six test files add `nodesDir`, `changesPath`, `canvasRoot`, and `withSpaceDirHandlesReleased`. -This is a smaller consumer leak than the raw import count suggests: most +**Corrected.** The table above was built by searching for `workspace/disk/`, +which misses every consumer that reaches the same symbols through the +deprecated `storage/paths.js` shim. Counting both paths, the production files +outside `storage/` that read a **storage-owned** symbol are six, not one: + +| File | Symbol | Resolution | +| ------------------------------------------- | --------------------------------- | ------------------------------------------ | +| `canvas/canvas.route.ts` | `nodesDir`, `SPACE_JSON_FILENAME` | import/export bundle format; own constant | +| `canvas/external-watcher.ts` | `SPACE_JSON_FILENAME` | Disk-only feature; declare materialization | +| `canvas/world-target-access.ts` | `SPACE_JSON_FILENAME` | catalogue read; `SpaceRepository.list()` | +| `canvas/import-node-src.ts` | `artifactsDir` | `BlobScope.owns()` (§12.5.7) | +| `agent/conversation/prompt/debug-prompt.ts` | `chatDir` | writes its own debug log; own path | +| `agent/memory/analyzer.ts` | `chatDir` | dead code; removed (§12.5.7) | + +Even at six this is a smaller leak than the raw import count suggests: most `workspace/disk/` imports come from `storage/` itself, which is the permitted -direction. Exactly **one** production site outside `storage/` reads a -storage-owned path (`chatDir`). The problem is therefore not mass violation by -consumers — it is that the module they import is _mixed_, and its name and -location assert an answer the port layer exists to keep open. +direction. The problem is not mass violation by consumers — it is that the +module they import is _mixed_, and its name and location assert an answer the +port layer exists to keep open. Note also that only one of the six wants a new +port capability; the rest are dead, misfiled, or already served. #### 12.5.2 The test that decides ownership @@ -1827,6 +1841,44 @@ scope by definition. Phase 5 rebases onto this and drops its `utils/naming.ts` extraction, its `workspace/disk/naming.ts` shim, and the corresponding roadmap edits. +#### 12.5.7 Findings from step 5, and one follow-up + +Working the six consumers of §12.5.1 individually produced four different +answers, only one of which was a missing port capability. + +**`BlobScope.owns(absolutePath)` — added.** `import-node-src.ts` asked whether +a resolved path was already inside `.artifacts/`, and answered it by +rebuilding the Disk artifacts directory. The scope can answer for itself. Pure +and synchronous — a path comparison, never I/O — and a backend that stores +nothing locally returns `false`, which is correct there rather than a +degradation. It sits on the blob axis, so it constrains no structured backend. +Covered in the shared blob contract, so both answers are pinned for every +adapter. + +**The memory analyzer's chat digest — removed, not ported.** `readChatDigest` +scanned `/.history/chat/*.json` for a `{ messages: [] }` shape. Two +migrations retired it: `migrate-chat-threads` renamed `.json` to +`.json.bak` and wrote `.turns.jsonl`; `migrate-chat-turns` folded those into +the Agenetes Tier-2 store under `chat_v2/`. The only live `.json` writer left +in that directory is the change-review sidecar, whose payload is an array with +no `messages` key, so every file failed the guard. The reader had returned +nothing in any migrated workspace, and its own test pointed it at a directory +that does not exist, so nothing caught it. + +Adding a `SpaceChats.list()` port to serve it would have been the wrong repair +twice over: it would give the storage ports authority over data storage does +not own — `legacy/canvas-store.ts` states plainly that threads and turns +belong to the agent runtime — and would oblige every future structured backend +to model Agenetes' turn log, which is the two-authorities hazard in §13. + +**Follow-up (open):** decide whether the memory agent should see conversation +turns at all, and if so reinstate the digest against `agenetes.history()` — +the call `canvas-search.ts` already uses for exactly this data. That is a +behavior change and deliberately outside this phase, whose parity rule is that +no working behavior moves. The `latestChatTs` field and its +`lastSeenThreadCursor` plumbing survive the removal because they are the +resume point such a digest would need. + ### 12.6 Later phases — provisional 5. Add one new adapter at a time — SQLite, then Postgres, then Azure Blob — From 9813b959bcade173fcba96604713aa0c12d71ab8 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 16:01:27 +0800 Subject: [PATCH 6/9] refactor(canvas): ask the ref, not the port, whether it is an artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces `BlobScope.owns(absolutePath)`, added one commit ago and wrong. `materialize()` runs port → filesystem: the port renders a real path as a service, and any backend satisfies it by spooling a temp copy. `owns()` ran the other way, handing the port a path in a vocabulary only a local backend can interpret. The `false` a remote backend would return is not a neutral implementation but an admission that the question is meaningless there — so the fix removed a Disk-layout import from one consumer by moving the Disk assumption into the port, where it would constrain every future backend. That is the mistake this phase exists to correct. The caller never needed storage. `toPhysicalRel` already maps the virtual `artifacts/` prefix onto the hidden directory, so "is this ref already an artifact?" is a question about the ref. `isArtifactsRel` answers it in `fs-sandbox.ts`, which owns that mapping, resolving against a synthetic root so no workspace, canvas directory, or filesystem is involved. Writing its test surfaced a limitation worth pinning rather than fixing: the virtual alias applies as a prefix, so `nodes/../artifacts/x` is not treated as an artifact. The pre-existing check behaved identically, and widening it would change which files the import hook copies. Phase 4.5, step 5 of 6 (proposal §12.5.7). Co-Authored-By: Claude Opus 5 (1M context) --- .../agent/tools/handlers/fs-sandbox.test.ts | 55 +++++++++++++++++++ .../agent/tools/handlers/fs-sandbox.ts | 19 +++++++ .../modules/artifact/artifact.route.test.ts | 1 - .../src/modules/artifact/send-blob.test.ts | 1 - .../src/modules/canvas/import-node-src.ts | 9 +-- .../storage/backends/disk/blob-store.ts | 5 -- .../compatibility/delete-canvas.test.ts | 2 - apps/server/src/modules/storage/ports/blob.ts | 15 ----- .../ports/contracts/blob-store.contract.ts | 20 ------- apps/server/src/modules/storage/storage.ts | 3 - docs/proposals/multi-backend-storage.md | 41 +++++++++++--- 11 files changed, 111 insertions(+), 60 deletions(-) create mode 100644 apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts new file mode 100644 index 000000000..4f5617e0c --- /dev/null +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { describe, expect, it } from 'vitest'; + +import { isArtifactsRel, toPhysicalRel } from './fs-sandbox.js'; + +/** + * `isArtifactsRel` decides whether a node `src` already points at an + * artifact, which is what stops the import hook copying a file that is + * already stored. It answers from the ref alone — no workspace, no canvas + * directory, no backend — so these cases need no fixture. + */ +describe('isArtifactsRel', () => { + it('accepts both the virtual and physical spellings', () => { + expect(isArtifactsRel(toPhysicalRel('artifacts/pic.png'))).toBe(true); + expect(isArtifactsRel(toPhysicalRel('.artifacts/pic.png'))).toBe(true); + // A bare key resolves into the artifacts dir via the same map. + expect(isArtifactsRel(toPhysicalRel('artifacts'))).toBe(true); + }); + + it('rejects refs outside the artifacts directory', () => { + expect(isArtifactsRel(toPhysicalRel('nodes/foo.md'))).toBe(false); + expect(isArtifactsRel(toPhysicalRel('upload/pic.png'))).toBe(false); + expect(isArtifactsRel(toPhysicalRel('space.json'))).toBe(false); + // A sibling whose name merely starts with the directory name is not + // inside it — the reason this is a segment-wise test, not a prefix one. + expect(isArtifactsRel(toPhysicalRel('.artifacts-evil/pic.png'))).toBe( + false, + ); + }); + + it('collapses traversal rather than matching on the literal prefix', () => { + // Reaches the hidden dir the long way round; a bare prefix test would + // miss it and the hook would import a file that is already stored. + expect(isArtifactsRel(toPhysicalRel('nodes/../.artifacts/pic.png'))).toBe( + true, + ); + // Leaves it again, so it is an ordinary local file. + expect(isArtifactsRel(toPhysicalRel('.artifacts/../nodes/foo.md'))).toBe( + false, + ); + }); + + it('aliases the virtual prefix only at the start of the ref', () => { + // `toPhysicalRel` rewrites `artifacts/` as a prefix, so a mid-path + // occurrence stays literal and resolves to `/artifacts/…`, which is not + // the hidden directory. Pinned because it is a limitation, not a + // decision: the pre-existing check behaved the same way, and widening it + // would change which files the import hook copies. + expect(isArtifactsRel(toPhysicalRel('nodes/../artifacts/pic.png'))).toBe( + false, + ); + }); +}); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts index bad4173aa..85e5700b1 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts @@ -72,6 +72,25 @@ const VIRTUAL_PREFIX: ReadonlyArray = [ ['upload/', '.upload/'], ]; +/** + * Whether a canvas-relative ref denotes something already under `.artifacts/`. + * + * A question about the *ref*, not about storage: a node `src` that already + * points at an artifact needs no import, whatever backend holds the bytes. + * Resolving against a synthetic root keeps it that way — no workspace, no + * canvas directory, no filesystem — while still collapsing any `..` that + * would slip past a bare prefix test, the same normalization + * {@link safeResolve} relies on. + * + * Takes the *physical* form, so pass {@link toPhysicalRel} output. + */ +export function isArtifactsRel(physicalRel: string): boolean { + const [, artifactsPhysical] = VIRTUAL_PREFIX[0]; + const root = path.resolve('/', artifactsPhysical); + const target = path.resolve('/', physicalRel); + return target === root || target.startsWith(root + path.sep); +} + /** * Rewrite a request path's virtual prefix (`artifacts/`, `upload/`) to its * hidden on-disk counterpart. Idempotent: an already-physical `.upload/…` diff --git a/apps/server/src/modules/artifact/artifact.route.test.ts b/apps/server/src/modules/artifact/artifact.route.test.ts index 26edea2e4..3010fbec7 100644 --- a/apps/server/src/modules/artifact/artifact.route.test.ts +++ b/apps/server/src/modules/artifact/artifact.route.test.ts @@ -99,7 +99,6 @@ function installDeleteBlock(canvasId: string): { hasMany: (names) => delegate.hasMany(names), list: () => delegate.list(), materialize: (name) => delegate.materialize(name), - owns: (absolutePath: string) => delegate.owns(absolutePath), async deleteAll() { if (ref.canvasId === canvasId) { started.resolve(); diff --git a/apps/server/src/modules/artifact/send-blob.test.ts b/apps/server/src/modules/artifact/send-blob.test.ts index b0ab28794..cded24eaf 100644 --- a/apps/server/src/modules/artifact/send-blob.test.ts +++ b/apps/server/src/modules/artifact/send-blob.test.ts @@ -50,7 +50,6 @@ function fakeScope(blobs: Record): BlobScope { async materialize() { return null; }, - owns: () => false, async deleteAll() {}, }; } diff --git a/apps/server/src/modules/canvas/import-node-src.ts b/apps/server/src/modules/canvas/import-node-src.ts index 88ddac586..e201d42fc 100644 --- a/apps/server/src/modules/canvas/import-node-src.ts +++ b/apps/server/src/modules/canvas/import-node-src.ts @@ -38,6 +38,7 @@ import { import { getLogger } from '../../utils/logger.js'; import { safeResolve, + isArtifactsRel, toPhysicalRel, } from '../agent/tools/handlers/fs-sandbox.js'; import { canvasBlobs } from '../storage/index.js'; @@ -265,10 +266,10 @@ async function resolveImportedSrc( return null; } - // Already a managed blob (or a bare artifact key that resolves there) - // — nothing to import. The scope answers for its own storage rather than - // this module rebuilding the Disk artifacts path. - if (canvasBlobs(canvasId).owns(absPath)) return null; + // Already an artifact ref (or a bare artifact key, which `toPhysicalRel` + // maps into `.artifacts/`) — nothing to import. Asked of the ref rather + // than of a resolved path, so no storage layout is involved. + if (isArtifactsRel(physicalRel)) return null; // A bare key like `art_abc.png` resolves under the canvas root but has no // file on disk there — leave it so the web resolver builds the artifact URL. diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 437cc6935..88dd8eb6d 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -228,11 +228,6 @@ class DiskBlobScope implements BlobScope { return createBlobLease(blobPath(dir, info.name), async () => {}); } - owns(absolutePath: string): boolean { - const dir = this.#resolveDir(); - return absolutePath === dir || absolutePath.startsWith(dir + path.sep); - } - async deleteAll(): Promise { await rm(this.#resolveDir(), { recursive: true, force: true }); } diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index e6e341a69..cbcda58cc 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -114,7 +114,6 @@ class OrderRecordingBlobStore implements BlobStore { scope.hasMany(names), list: (): Promise => scope.list(), materialize: (name: string) => scope.materialize(name), - owns: (absolutePath: string): boolean => scope.owns(absolutePath), deleteAll: async (): Promise => { seen.push(existsSync(canvasJsonPath(ref.canvasId))); await scope.deleteAll(); @@ -181,7 +180,6 @@ class ControllableBlobStore implements BlobStore { scope.hasMany(names), list: (): Promise => scope.list(), materialize: (name: string) => scope.materialize(name), - owns: (absolutePath: string): boolean => scope.owns(absolutePath), deleteAll: async (): Promise => { this.deleteCalls += 1; this.deleteStarted.resolve(); diff --git a/apps/server/src/modules/storage/ports/blob.ts b/apps/server/src/modules/storage/ports/blob.ts index 022e5abfb..c710d0f05 100644 --- a/apps/server/src/modules/storage/ports/blob.ts +++ b/apps/server/src/modules/storage/ports/blob.ts @@ -115,21 +115,6 @@ export interface BlobScope { /** A temporary real path for this blob, or null when absent. */ materialize(name: string): Promise; - /** - * Whether an absolute local path is already stored by this scope. - * - * The question a local-import path asks before copying a file in: a - * source that is *already* a managed blob needs no import. Answering it - * by rebuilding the scope's directory is what made callers reach for the - * Disk layout, so the scope answers for itself. - * - * Pure and synchronous — a path comparison, never I/O, and it says - * nothing about whether the blob exists. A backend that stores nothing - * locally always returns `false`, which is the correct answer there: no - * local path it was handed can already be one of its blobs. - */ - owns(absolutePath: string): boolean; - /** * Remove every blob in this scope. * diff --git a/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts b/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts index 79332627a..9623fcd8f 100644 --- a/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts +++ b/apps/server/src/modules/storage/ports/contracts/blob-store.contract.ts @@ -286,26 +286,6 @@ export function describeBlobStoreContract( ); }); - it('claims a materialized path and disclaims an unrelated one', async () => { - const blobs = await scope(); - await blobs.put('owned.txt', Buffer.from('mine')); - - // A backend that hands out a real path must recognize it again; - // one that materializes nothing locally owns no local path at all. - // Both are correct, and the two answers must not be mixed within a - // single implementation — that is what this pins. - const lease = await blobs.materialize('owned.txt'); - if (lease) { - expect(blobs.owns(lease.path)).toBe(true); - await lease.release(); - } - - expect(blobs.owns('/definitely/not/this/scope/owned.txt')).toBe(false); - // Absence is not the question `owns` answers: a path that would belong - // to the scope still belongs to it before anything is written there. - expect(typeof blobs.owns('/tmp/never-written.txt')).toBe('boolean'); - }); - it('deleteAll empties the scope and is idempotent', async () => { const blobs = await scope(); await blobs.put('i1.txt', Buffer.from('x')); diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index 3f1ec8330..c11a93e0b 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -320,9 +320,6 @@ export function canvasBlobs(canvasId: string): BlobScope { materialize(name: string): Promise { return delegate.materialize(name); }, - owns(absolutePath: string): boolean { - return delegate.owns(absolutePath); - }, deleteAll(): Promise { return delegate.deleteAll(); }, diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index a80598b10..84e22ad13 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1716,7 +1716,7 @@ outside `storage/` that read a **storage-owned** symbol are six, not one: | `canvas/canvas.route.ts` | `nodesDir`, `SPACE_JSON_FILENAME` | import/export bundle format; own constant | | `canvas/external-watcher.ts` | `SPACE_JSON_FILENAME` | Disk-only feature; declare materialization | | `canvas/world-target-access.ts` | `SPACE_JSON_FILENAME` | catalogue read; `SpaceRepository.list()` | -| `canvas/import-node-src.ts` | `artifactsDir` | `BlobScope.owns()` (§12.5.7) | +| `canvas/import-node-src.ts` | `artifactsDir` | ref-level `isArtifactsRel` (§12.5.7) | | `agent/conversation/prompt/debug-prompt.ts` | `chatDir` | writes its own debug log; own path | | `agent/memory/analyzer.ts` | `chatDir` | dead code; removed (§12.5.7) | @@ -1846,14 +1846,37 @@ Phase 5 rebases onto this and drops its `utils/naming.ts` extraction, its Working the six consumers of §12.5.1 individually produced four different answers, only one of which was a missing port capability. -**`BlobScope.owns(absolutePath)` — added.** `import-node-src.ts` asked whether -a resolved path was already inside `.artifacts/`, and answered it by -rebuilding the Disk artifacts directory. The scope can answer for itself. Pure -and synchronous — a path comparison, never I/O — and a backend that stores -nothing locally returns `false`, which is correct there rather than a -degradation. It sits on the blob axis, so it constrains no structured backend. -Covered in the shared blob contract, so both answers are pinned for every -adapter. +**No new port capability — a rejected design, recorded because the reasoning +generalizes.** `import-node-src.ts` asked whether a resolved path was already +inside `.artifacts/`, and answered by rebuilding the Disk artifacts directory. +The first fix added `BlobScope.owns(absolutePath): boolean`, letting the scope +answer for itself. That was wrong, in the same way this phase exists to +correct. + +`materialize()` runs port → filesystem: the port _renders_ a real path as a +service, and any backend can satisfy it by spooling a temp copy. +`owns(absolutePath)` runs the other way — it hands the port a path in a +vocabulary only a local backend can interpret and asks it to adjudicate. The +`false` a remote backend would return is not a neutral implementation but an +admission that the question is meaningless there. The net effect would have +been to remove a Disk-layout import from one consumer by moving the Disk +assumption into the port, where it constrains every future backend. + +**Test for the next such proposal:** a port method must be answerable by every +backend in the port's own vocabulary. If one backend's honest implementation +is a constant, the method is describing that backend, not the port. + +The caller never needed storage at all. `toPhysicalRel` already maps the +virtual `artifacts/` prefix onto the hidden directory, so "is this ref already +an artifact?" is a question about the _ref_, answered by `isArtifactsRel` in +`fs-sandbox.ts` — the module that owns virtual↔physical ref mapping. +Resolving against a synthetic root keeps it free of workspace, canvas +directory, and filesystem, while still collapsing `..`. + +Writing its test surfaced a limitation worth pinning: the virtual alias is +applied as a prefix, so `nodes/../artifacts/x` is not treated as an artifact. +The pre-existing check behaved identically, so the test records the behavior +rather than widening it. **The memory analyzer's chat digest — removed, not ported.** `readChatDigest` scanned `/.history/chat/*.json` for a `{ messages: [] }` shape. Two From 9a1e1f80b387de7be5bae2e70cc2b5076fcfb181 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Fri, 14 Aug 2026 16:09:48 +0800 Subject: [PATCH 7/9] refactor(storage): finish moving the Disk substrate out of the workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes Phase 4.5 steps 3, 4 and 6. `space-dir-handles.ts` and `world-canvas.ts` join the rest of the Disk layout under `backends/disk/`. Both fail the §12.5.2 test for the same reason: releasing directory handles exists so Windows can rename a Space *folder*, and the World bootstrap writes one — neither has anything to do once Spaces live in tables. `workspace/disk/paths.ts` becomes `workspace/paths.ts`, and the `disk/` segment is gone, because what remains there describes the workspace as a place. The five consumers still resolving `canvasRoot` through the shim now call `spaceDirectory()`. The two Disk capabilities the application genuinely needs are re-exported from the facade, so app code asks for a capability instead of naming a backend. Three new boundary guards pin the outcome: the workspace module has no substrate segment, imports no backend, and names no Disk layout symbol — the last one catching a reintroduction even if the import path looks innocent. The reach-into-backends rule now exempts tests, matching the exemption the composition-root rule already made and for the same reason: a production file naming an adapter has bound the app to a backend, while a test naming one is choosing its subject. Two test mocks needed following rather than rewriting. `canvas.route` mocked the facade for a Disk rename failure, which no longer intercepts the adapter's own import; it now mocks the module under test. `external-watcher` stubbed the facade wholesale, which would have replaced the real handle registry the assertions depend on; its factory now delegates those two members so both sides share one instance. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/modules/agent/acp/service.ts | 2 +- .../agent/acp/service.workload-spec.test.ts | 2 +- .../src/modules/agent/acp/threads.route.ts | 2 +- .../src/modules/agent/agent-thread.service.ts | 2 +- apps/server/src/modules/agent/agent.route.ts | 2 +- .../server/src/modules/agent/agent.service.ts | 2 +- .../agent/conversation/prompt/debug-prompt.ts | 2 +- .../src/modules/agent/memory/analyzer.test.ts | 2 +- .../src/modules/agent/memory/analyzer.ts | 2 +- apps/server/src/modules/agent/memory/read.ts | 2 +- .../src/modules/agent/memory/sandbox.ts | 4 +- .../src/modules/agent/memory/trigger.ts | 5 +- .../src/modules/agent/skills.route.test.ts | 2 +- .../agent/tools/handlers/fs-sandbox.ts | 5 +- .../agent/tools/handlers/fs-write.test.ts | 5 +- .../modules/agent/tools/handlers/fs-write.ts | 2 +- .../src/modules/canvas/canvas-search.test.ts | 2 +- .../src/modules/canvas/canvas-search.ts | 2 +- .../src/modules/canvas/canvas.route.test.ts | 24 +++--- .../server/src/modules/canvas/canvas.route.ts | 7 +- .../modules/canvas/external-watcher.test.ts | 17 ++++- .../src/modules/canvas/external-watcher.ts | 2 +- .../src/modules/canvas/external.route.ts | 4 +- .../modules/canvas/import-node-src.test.ts | 9 ++- .../remote_fs/interactive-view.rfs.test.ts | 2 +- apps/server/src/modules/remote_fs/skill.ts | 4 +- .../backends}/disk/space-dir-handles.ts | 0 .../storage/backends/disk/space-nodes.test.ts | 2 +- .../storage/backends/disk/space-repository.ts | 2 +- .../storage/backends/disk/space-write.test.ts | 2 +- .../backends}/disk/world-canvas.test.ts | 0 .../backends}/disk/world-canvas.ts | 9 +-- .../server/src/modules/storage/canvas-dirs.ts | 2 +- apps/server/src/modules/storage/index.ts | 17 +++++ .../modules/storage/module-boundaries.test.ts | 76 ++++++++++++++++++- apps/server/src/modules/storage/paths.ts | 2 +- apps/server/src/modules/workspace-prepare.ts | 2 +- .../src/modules/workspace/{disk => }/paths.ts | 6 +- apps/server/src/prompt/skills/loader.ts | 4 +- docs/proposals/multi-backend-storage.md | 12 ++- 40 files changed, 184 insertions(+), 68 deletions(-) rename apps/server/src/modules/{workspace => storage/backends}/disk/space-dir-handles.ts (100%) rename apps/server/src/modules/{workspace => storage/backends}/disk/world-canvas.test.ts (100%) rename apps/server/src/modules/{workspace => storage/backends}/disk/world-canvas.ts (90%) rename apps/server/src/modules/workspace/{disk => }/paths.ts (97%) diff --git a/apps/server/src/modules/agent/acp/service.ts b/apps/server/src/modules/agent/acp/service.ts index df217379b..2437b4659 100644 --- a/apps/server/src/modules/agent/acp/service.ts +++ b/apps/server/src/modules/agent/acp/service.ts @@ -31,7 +31,7 @@ import { getProfileSessionPreferences } from './profile-session-preferences.js'; import { getProfile as getLegacyProfile } from './profile-store.js'; import { buildReachbackEnv } from './reachback-env.js'; import { renderExternalAgentSystemPreamble } from '../../../prompt/external-agent/system-preamble.js'; -import { canvasAcpNamespace } from '../../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../../workspace/paths.js'; import { agenetes, EXTERNAL_DRIVER_KIND, diff --git a/apps/server/src/modules/agent/acp/service.workload-spec.test.ts b/apps/server/src/modules/agent/acp/service.workload-spec.test.ts index 799ca15d9..f9aede8df 100644 --- a/apps/server/src/modules/agent/acp/service.workload-spec.test.ts +++ b/apps/server/src/modules/agent/acp/service.workload-spec.test.ts @@ -41,7 +41,7 @@ vi.mock('./reachback-env.js', () => ({ buildReachbackEnv: () => ({ REACHBACK: '1' }), })); -vi.mock('../../workspace/disk/paths.js', () => ({ +vi.mock('../../workspace/paths.js', () => ({ canvasAcpNamespace: (canvasId: string) => `/canvases/${canvasId}/acp`, })); diff --git a/apps/server/src/modules/agent/acp/threads.route.ts b/apps/server/src/modules/agent/acp/threads.route.ts index e5983797a..ea773135a 100644 --- a/apps/server/src/modules/agent/acp/threads.route.ts +++ b/apps/server/src/modules/agent/acp/threads.route.ts @@ -50,7 +50,7 @@ import { buildReachbackEnv } from './reachback-env.js'; import { getExternalAgentRuntimeConfig } from './runtime-config.js'; import { resolveBindingRecipe } from './service.js'; import { renderExternalAgentSystemPreamble } from '../../../prompt/external-agent/system-preamble.js'; -import { canvasAcpNamespace } from '../../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../../workspace/paths.js'; import { agenetes, EXTERNAL_DRIVER_KIND, diff --git a/apps/server/src/modules/agent/agent-thread.service.ts b/apps/server/src/modules/agent/agent-thread.service.ts index 3bdcaf825..1ab52dab2 100644 --- a/apps/server/src/modules/agent/agent-thread.service.ts +++ b/apps/server/src/modules/agent/agent-thread.service.ts @@ -18,7 +18,7 @@ import { readWorkspaceMemory } from './memory/index.js'; import { planSkillDispatch } from './skill-model-routing.js'; import { acquireAgentTurn, waitForAgentTurnRelease } from './turn-lease.js'; import { loadAgent } from '../../prompt/index.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { HuabuSubmission } from './agenetes/handle.js'; import type { ChatEnvelope } from './conversation/envelope.js'; diff --git a/apps/server/src/modules/agent/agent.route.ts b/apps/server/src/modules/agent/agent.route.ts index 0a7134024..597a2fb53 100644 --- a/apps/server/src/modules/agent/agent.route.ts +++ b/apps/server/src/modules/agent/agent.route.ts @@ -31,7 +31,7 @@ import { import { buildChatEnvelope } from '../agent/conversation/envelope.js'; import { buildHistoryFromTurns } from '../agent/conversation/transcript/history.js'; import { getLLMModel } from '../agent/llm.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { ControlMsg, Namespace } from '@agenetes/protocol'; import type { diff --git a/apps/server/src/modules/agent/agent.service.ts b/apps/server/src/modules/agent/agent.service.ts index 77d3bba8f..c5e9b3845 100644 --- a/apps/server/src/modules/agent/agent.service.ts +++ b/apps/server/src/modules/agent/agent.service.ts @@ -27,7 +27,7 @@ import { import { createChatSubmission } from './agenetes/handle.js'; import { buildHuabuPiWorkloadSpec } from './agenetes/pi-driver.js'; import { loadAgent, type AgentId } from '../../prompt/index.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import { renderInternalAgentInputs } from './conversation/prompt/build-prompt.js'; import { dumpAssembledPrompt } from './conversation/prompt/debug-prompt.js'; import { type ToolScope } from './tools/index.js'; diff --git a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts index acd3a3c9d..2c9f5ef55 100644 --- a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts +++ b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts @@ -18,7 +18,7 @@ import { appendFileSync } from 'node:fs'; import { mkdirp } from '../../../../utils/fs.js'; import { chatDir } from '../../../storage/paths.js'; -import { chatPromptLogPath } from '../../../workspace/disk/paths.js'; +import { chatPromptLogPath } from '../../../workspace/paths.js'; import type { Context } from '@earendil-works/pi-ai'; import type { FastifyBaseLogger } from 'fastify'; diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index 06faa8f69..aa70f8e16 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -16,7 +16,7 @@ vi.mock('../../../prompt/index.js', () => ({ listSkills: vi.fn(), })); vi.mock('../../storage/index.js', () => ({ getStructuredStore: vi.fn() })); -vi.mock('../../workspace/disk/paths.js', () => ({ +vi.mock('../../workspace/paths.js', () => ({ canvasMemoryPath: (canvasId: string) => `${physicalState.root}/${canvasId}/.memory/space.md`, workspaceMemoryPath: () => `${physicalState.root}/setting/user.md`, diff --git a/apps/server/src/modules/agent/memory/analyzer.ts b/apps/server/src/modules/agent/memory/analyzer.ts index 89061aee7..6d70595e9 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -36,7 +36,7 @@ import { import { canvasMemoryPath, workspaceMemoryPath, -} from '../../workspace/disk/paths.js'; +} from '../../workspace/paths.js'; import { runAgent } from '../agent.service.js'; import type { MemoryLogger } from './index.js'; diff --git a/apps/server/src/modules/agent/memory/read.ts b/apps/server/src/modules/agent/memory/read.ts index 8d6968dbb..c8776b05f 100644 --- a/apps/server/src/modules/agent/memory/read.ts +++ b/apps/server/src/modules/agent/memory/read.ts @@ -18,7 +18,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { workspaceMemoryPath, canvasMemoryPath, -} from '../../workspace/disk/paths.js'; +} from '../../workspace/paths.js'; /** * Read the user memory body. diff --git a/apps/server/src/modules/agent/memory/sandbox.ts b/apps/server/src/modules/agent/memory/sandbox.ts index 397bbe1a2..0085b4e43 100644 --- a/apps/server/src/modules/agent/memory/sandbox.ts +++ b/apps/server/src/modules/agent/memory/sandbox.ts @@ -31,7 +31,7 @@ import { userSkillsDir, canvasMemoryDir, canvasMemoryPath, -} from '../../workspace/disk/paths.js'; +} from '../../workspace/paths.js'; /** Thrown by every resolver below on out-of-sandbox attempts. */ export class MemorySandboxError extends Error { @@ -101,7 +101,7 @@ export function resolveUserSkillPath(id: string): string { /** * Resolve the absolute Space memory file path. Throws if the resolved path * escapes the canvas's `.memory/` root (a defensive check — the path - * computation in `workspace/disk/paths.ts` already constrains the result, + * computation in `workspace/paths.ts` already constrains the result, * but going through `ensureUnderRoot` keeps the invariant explicit). */ export function resolveWorkingMemoryPath(canvasId: string): string { diff --git a/apps/server/src/modules/agent/memory/trigger.ts b/apps/server/src/modules/agent/memory/trigger.ts index 908ddca71..10ed258a0 100644 --- a/apps/server/src/modules/agent/memory/trigger.ts +++ b/apps/server/src/modules/agent/memory/trigger.ts @@ -27,10 +27,7 @@ import { existsSync } from 'node:fs'; import { atomicWriteJson, mkdirp, readJson } from '../../../utils/fs.js'; import { createKeyedMutex } from '../../../utils/keyed-mutex.js'; import { spaceDirectory } from '../../storage/index.js'; -import { - memoryStatePath, - canvasMemoryDir, -} from '../../workspace/disk/paths.js'; +import { memoryStatePath, canvasMemoryDir } from '../../workspace/paths.js'; /** Op-count threshold that triggers a memory analysis pass. */ export const OP_THRESHOLD = 50; diff --git a/apps/server/src/modules/agent/skills.route.test.ts b/apps/server/src/modules/agent/skills.route.test.ts index 15871a370..6c9bdd32c 100644 --- a/apps/server/src/modules/agent/skills.route.test.ts +++ b/apps/server/src/modules/agent/skills.route.test.ts @@ -30,7 +30,7 @@ import { invalidateSkillCache, type LoadedSkill, } from '../../prompt/skills/loader.js'; -import { userSkillsDir } from '../workspace/disk/paths.js'; +import { userSkillsDir } from '../workspace/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { SkillCatalogueEntry } from '@huabu/shared'; diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts index 85e5700b1..fcf5abd4f 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts @@ -30,8 +30,7 @@ import { readFileSync, readdirSync, statSync, type Dirent } from 'node:fs'; import path from 'node:path'; import { parseFrontmatter } from '../../../../utils/markdown-frontmatter.js'; -import { getCanvasStore } from '../../../storage/index.js'; -import { canvasRoot } from '../../../storage/paths.js'; +import { getCanvasStore, spaceDirectory } from '../../../storage/index.js'; // ─── Always-skipped directory names ───────────────────────────────────────── @@ -143,7 +142,7 @@ export function safeResolve(canvasId: string, rel: string): string { ) { throw new Error(`Invalid canvasId: ${canvasId}`); } - const root = canvasRoot(canvasId); + const root = spaceDirectory(canvasId); // Accept the clean virtual prefixes (`upload/`, `artifacts/`) as aliases // for their hidden on-disk dirs so agents can reference either form. const target = path.resolve(root, toPhysicalRel(rel)); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts index 7ea06862e..5f17d9ae1 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts @@ -30,7 +30,7 @@ import { canvasMemoryPath, userSkillsDir, workspaceMemoryPath, -} from '../../../workspace/disk/paths.js'; +} from '../../../workspace/paths.js'; import { setWorkspacePath } from '../../../workspace.js'; interface ParsedResult { @@ -51,7 +51,8 @@ beforeEach(() => { setWorkspacePath(tmp); // `canvasRoot(canvasId)` falls back to `/` when the // canvas-dir index has no entry for the id (see `canvasDirName` in - // `workspace/disk/canvas-dirs.ts`). We just need the directory to exist so + // `storage/backends/disk/canvas-dirs.ts`). We just need the directory to + // exist so // writes can land in it. mkdirSync(join(tmp, canvasId), { recursive: true }); }); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.ts index 30671a195..501d8527a 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.ts @@ -36,7 +36,7 @@ import { canvasMemoryDir, settingDir, userSkillsDir, -} from '../../../workspace/disk/paths.js'; +} from '../../../workspace/paths.js'; import { resolveLongTermPath, resolveUserSkillPath, diff --git a/apps/server/src/modules/canvas/canvas-search.test.ts b/apps/server/src/modules/canvas/canvas-search.test.ts index 084ba04f4..9677f057b 100644 --- a/apps/server/src/modules/canvas/canvas-search.test.ts +++ b/apps/server/src/modules/canvas/canvas-search.test.ts @@ -43,7 +43,7 @@ vi.mock('../agent/agenetes/drivers.js', () => ({ }, })); -vi.mock('../workspace/disk/paths.js', async (importActual) => ({ +vi.mock('../workspace/paths.js', async (importActual) => ({ ...((await importActual()) as Record), canvasAcpNamespace: (canvasId: string) => ({ name: canvasId, root: '' }), })); diff --git a/apps/server/src/modules/canvas/canvas-search.ts b/apps/server/src/modules/canvas/canvas-search.ts index 117c4dce2..17e0754d6 100644 --- a/apps/server/src/modules/canvas/canvas-search.ts +++ b/apps/server/src/modules/canvas/canvas-search.ts @@ -44,7 +44,7 @@ import { import { agenetes } from '../agent/agenetes/drivers.js'; import { chatEnvelopeFromSubmission } from '../agent/agenetes/handle.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; import type { AgentTurn } from '@agenetes/protocol'; diff --git a/apps/server/src/modules/canvas/canvas.route.test.ts b/apps/server/src/modules/canvas/canvas.route.test.ts index 4cacdc019..bbfeac7a8 100644 --- a/apps/server/src/modules/canvas/canvas.route.test.ts +++ b/apps/server/src/modules/canvas/canvas.route.test.ts @@ -21,15 +21,22 @@ vi.mock('../storage/index.js', async (importOriginal) => { }; }); -vi.mock('../workspace/disk/space-dir-handles.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - withSpaceDirHandlesReleased: vi.fn(actual.withSpaceDirHandlesReleased), - }; -}); +// Mocked at the module the Disk repository imports, not at the facade: these +// cases force a Space-directory rename to fail, which is Disk behavior, and a +// facade mock would not intercept the adapter's own import. +vi.mock( + '../storage/backends/disk/space-dir-handles.js', + async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + withSpaceDirHandlesReleased: vi.fn(actual.withSpaceDirHandlesReleased), + }; + }, +); import canvasRoutes from './canvas.route.js'; +import { withSpaceDirHandlesReleased } from '../storage/backends/disk/space-dir-handles.js'; import { createCanvas, deleteCanvas } from '../storage/compatibility/canvas.js'; import { canvasBlobs, @@ -38,11 +45,10 @@ import { resetStorageCache, } from '../storage/index.js'; import { changesPath } from '../storage/paths.js'; -import { withSpaceDirHandlesReleased } from '../workspace/disk/space-dir-handles.js'; import { setWorkspacePath } from '../workspace.js'; +import type * as SpaceDirHandlesModule from '../storage/backends/disk/space-dir-handles.js'; import type * as StorageModule from '../storage/index.js'; -import type * as SpaceDirHandlesModule from '../workspace/disk/space-dir-handles.js'; import type { RecentAction } from '@huabu/shared'; let tmp: string; diff --git a/apps/server/src/modules/canvas/canvas.route.ts b/apps/server/src/modules/canvas/canvas.route.ts index 367a15558..b6ffedde6 100644 --- a/apps/server/src/modules/canvas/canvas.route.ts +++ b/apps/server/src/modules/canvas/canvas.route.ts @@ -59,11 +59,12 @@ import { deleteSpace, getCanvasStore, getStructuredStore, - updateNode, + spaceDirectory, type CanvasFile, type UpdateNodeOutcome, + updateNode, } from '../storage/index.js'; -import { canvasRoot, nodesDir, SPACE_JSON_FILENAME } from '../storage/paths.js'; +import { nodesDir, SPACE_JSON_FILENAME } from '../storage/paths.js'; import { getWorkspacePath } from '../workspace.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; @@ -1609,7 +1610,7 @@ const canvasRoutes: FastifyPluginAsync = async (fastify) => { return reply.code(404).send({ message: 'Canvas not found' }); } - const canvasDir = canvasRoot(canvasId); + const canvasDir = spaceDirectory(canvasId); if (!existsSync(canvasDir)) { return reply.code(404).send({ message: 'Canvas directory not found' }); } diff --git a/apps/server/src/modules/canvas/external-watcher.test.ts b/apps/server/src/modules/canvas/external-watcher.test.ts index d679ed043..cf3de2474 100644 --- a/apps/server/src/modules/canvas/external-watcher.test.ts +++ b/apps/server/src/modules/canvas/external-watcher.test.ts @@ -72,9 +72,18 @@ const canvasStore = vi.hoisted(() => ({ read: vi.fn(() => ({ state: { nodes: [] } })), })); -vi.mock('../storage/index.js', () => ({ - getCanvasStore: () => canvasStore, -})); +// The facade is stubbed for the store, but the handle helpers must stay the +// real ones: these cases drive `withSpaceDirHandlesReleased` and assert the +// watcher released its handles, which only works if both sides share the one +// module instance that holds the registry. +vi.mock('../storage/index.js', async () => { + const handles = await import('../storage/backends/disk/space-dir-handles.js'); + return { + getCanvasStore: () => canvasStore, + registerSpaceDirHandleOwner: handles.registerSpaceDirHandleOwner, + withSpaceDirHandlesReleased: handles.withSpaceDirHandlesReleased, + }; +}); function makeFakeNativeWatcher() { const nativeWatcher = { @@ -96,7 +105,7 @@ import { openExternalNoteSession, resetExternalNoteSessions, } from './external-watcher.js'; -import { withSpaceDirHandlesReleased } from '../workspace/disk/space-dir-handles.js'; +import { withSpaceDirHandlesReleased } from '../storage/index.js'; import type { ExternalNoteEvent } from '@huabu/shared'; diff --git a/apps/server/src/modules/canvas/external-watcher.ts b/apps/server/src/modules/canvas/external-watcher.ts index 38048184a..fc37894a7 100644 --- a/apps/server/src/modules/canvas/external-watcher.ts +++ b/apps/server/src/modules/canvas/external-watcher.ts @@ -34,8 +34,8 @@ import { getLogger } from '../../utils/logger.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; import { listAllCanvasDirEntries } from '../storage/canvas-dirs.js'; import { getCanvasStore } from '../storage/index.js'; +import { registerSpaceDirHandleOwner } from '../storage/index.js'; import { SPACE_JSON_FILENAME } from '../storage/paths.js'; -import { registerSpaceDirHandleOwner } from '../workspace/disk/space-dir-handles.js'; import { getWorkspacePath, isWorkspaceConfigured } from '../workspace.js'; import type { CanvasFile } from '../storage/index.js'; diff --git a/apps/server/src/modules/canvas/external.route.ts b/apps/server/src/modules/canvas/external.route.ts index 89e229f18..0984daa1c 100644 --- a/apps/server/src/modules/canvas/external.route.ts +++ b/apps/server/src/modules/canvas/external.route.ts @@ -17,7 +17,7 @@ import { takeExternalNote, } from './external-watcher.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; -import { canvasRoot } from '../storage/paths.js'; +import { spaceDirectory } from '../storage/index.js'; import type { FastifyPluginAsync } from 'fastify'; @@ -95,7 +95,7 @@ const externalRoutes: FastifyPluginAsync = async (fastify): Promise => { return reply.code(404).send({ message: 'External note not found' }); } - const abs = path.join(canvasRoot(canvasId), item.relativePath); + const abs = path.join(spaceDirectory(canvasId), item.relativePath); let raw: string; try { raw = await readFile(abs, 'utf8'); diff --git a/apps/server/src/modules/canvas/import-node-src.test.ts b/apps/server/src/modules/canvas/import-node-src.test.ts index d24500a5a..8e4c15fed 100644 --- a/apps/server/src/modules/canvas/import-node-src.test.ts +++ b/apps/server/src/modules/canvas/import-node-src.test.ts @@ -15,8 +15,11 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { importForeignNodeSources } from './import-node-src.js'; import { createCanvas } from '../storage/compatibility/canvas.js'; -import { canvasBlobs, getCanvasStore } from '../storage/index.js'; -import { canvasRoot } from '../storage/paths.js'; +import { + canvasBlobs, + getCanvasStore, + spaceDirectory, +} from '../storage/index.js'; import { setWorkspacePath } from '../workspace.js'; import type { CanvasCommand } from '@huabu/shared'; @@ -45,7 +48,7 @@ afterEach(() => { /** Stage a file under the canvas's hidden `.upload/` scratch dir. */ function stageUpload(canvasId: string, name: string, body: string): string { - const uploadDir = path.join(canvasRoot(canvasId), '.upload'); + const uploadDir = path.join(spaceDirectory(canvasId), '.upload'); mkdirSync(uploadDir, { recursive: true }); const abs = path.join(uploadDir, name); writeFileSync(abs, body); diff --git a/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts b/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts index a936d377d..934ca5e9c 100644 --- a/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts +++ b/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts @@ -22,7 +22,7 @@ import { getCanvasStore, resetStorageCache, } from '../storage/index.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import { setWorkspacePath } from '../workspace.js'; let workspace: string; diff --git a/apps/server/src/modules/remote_fs/skill.ts b/apps/server/src/modules/remote_fs/skill.ts index 45c8985fe..7dcbb5c97 100644 --- a/apps/server/src/modules/remote_fs/skill.ts +++ b/apps/server/src/modules/remote_fs/skill.ts @@ -15,7 +15,7 @@ import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; import { renderPromptFile } from '../../prompt/agents/loader.js'; -import { canvasRoot } from '../storage/paths.js'; +import { spaceDirectory } from '../storage/index.js'; /** PROMPT-ROOT-relative path of the bundled access guide. */ const ACCESS_GUIDE_TEMPLATE = 'external-agent/access-huabu.md'; @@ -40,7 +40,7 @@ export function resolveBundledRootSkill(): string { * markdown text (served with `Content-Type: text/markdown`). */ export function resolveCanvasSkill(canvasId: string): string { - const override = path.join(canvasRoot(canvasId), 'skill.md'); + const override = path.join(spaceDirectory(canvasId), 'skill.md'); if (existsSync(override)) { return readFileSync(override, 'utf8'); } diff --git a/apps/server/src/modules/workspace/disk/space-dir-handles.ts b/apps/server/src/modules/storage/backends/disk/space-dir-handles.ts similarity index 100% rename from apps/server/src/modules/workspace/disk/space-dir-handles.ts rename to apps/server/src/modules/storage/backends/disk/space-dir-handles.ts diff --git a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts index b3bb4604e..b688e8fbb 100644 --- a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts @@ -25,7 +25,7 @@ import { import { DiskSpaceNodes } from './space-nodes.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; -import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceNodesContract } from '../../ports/contracts/space-nodes.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.ts b/apps/server/src/modules/storage/backends/disk/space-repository.ts index 80384135d..e59b78fd2 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -29,6 +29,7 @@ import { forgetCanvasStore, getCanvasStore, } from './legacy/canvas-store-cache.js'; +import { withSpaceDirHandlesReleased } from './space-dir-handles.js'; import { readValidCanvasFile } from './space-record-validation.js'; import { readDiskSpaceRecord } from './space-record.js'; import { @@ -37,7 +38,6 @@ import { } from './space-title.js'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../../utils/fs.js'; import { normalizeForCompare } from '../../../../utils/naming.js'; -import { withSpaceDirHandlesReleased } from '../../../workspace/disk/space-dir-handles.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed, diff --git a/apps/server/src/modules/storage/backends/disk/space-write.test.ts b/apps/server/src/modules/storage/backends/disk/space-write.test.ts index 55f1e3773..2707301f0 100644 --- a/apps/server/src/modules/storage/backends/disk/space-write.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-write.test.ts @@ -22,7 +22,7 @@ import { import { DiskSpaceRepository } from './space-repository.js'; import { createDiskSpaceWrite } from './space-write.js'; import { DiskStructuredStore } from './structured-store.js'; -import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceWriteContract } from '../../ports/contracts/space-write.contract.js'; diff --git a/apps/server/src/modules/workspace/disk/world-canvas.test.ts b/apps/server/src/modules/storage/backends/disk/world-canvas.test.ts similarity index 100% rename from apps/server/src/modules/workspace/disk/world-canvas.test.ts rename to apps/server/src/modules/storage/backends/disk/world-canvas.test.ts diff --git a/apps/server/src/modules/workspace/disk/world-canvas.ts b/apps/server/src/modules/storage/backends/disk/world-canvas.ts similarity index 90% rename from apps/server/src/modules/workspace/disk/world-canvas.ts rename to apps/server/src/modules/storage/backends/disk/world-canvas.ts index c2f41e26a..d3c5395ec 100644 --- a/apps/server/src/modules/workspace/disk/world-canvas.ts +++ b/apps/server/src/modules/storage/backends/disk/world-canvas.ts @@ -6,13 +6,10 @@ import path from 'node:path'; import { createId } from '@huabu/shared'; -import { atomicWriteJson, readJson, sanitizeId } from '../../../utils/fs.js'; -import { - SPACE_JSON_FILENAME, - WORLD_CANVAS_DIR_NAME, -} from '../../storage/paths.js'; +import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; +import { atomicWriteJson, readJson, sanitizeId } from '../../../../utils/fs.js'; -import type { CanvasFile } from '../../canvas/persistence-types.js'; +import type { CanvasFile } from '../../../canvas/persistence-types.js'; function readWorldCanvas(filePath: string): CanvasFile { const canvas = readJson(filePath); diff --git a/apps/server/src/modules/storage/canvas-dirs.ts b/apps/server/src/modules/storage/canvas-dirs.ts index 5f8d94485..f504101ef 100644 --- a/apps/server/src/modules/storage/canvas-dirs.ts +++ b/apps/server/src/modules/storage/canvas-dirs.ts @@ -4,7 +4,7 @@ /** * @deprecated Forwarding shim — the Workspace layout owns these now. * - * Import from `modules/workspace/disk/canvas-dirs.js` instead. This file + * Import from `storage/backends/disk/canvas-dirs.js` instead. This file * exists only so the many existing physical-Disk capability imports keep * resolving while they migrate; it must never contain logic, and no new call * site may import it (enforced by the module-boundary test). diff --git a/apps/server/src/modules/storage/index.ts b/apps/server/src/modules/storage/index.ts index 37446c833..88d251d1b 100644 --- a/apps/server/src/modules/storage/index.ts +++ b/apps/server/src/modules/storage/index.ts @@ -29,6 +29,23 @@ export { isWorldCanvasId, requireWorldCanvasId, } from './backends/disk/canvas-dirs.js'; + +/** + * Materialization-tier capabilities, re-exported so consumers that need a + * real Space directory reach them through the facade rather than naming a + * backend (§12.5.4). + * + * Each is Disk-shaped by nature, not by accident: releasing directory handles + * exists so Windows can rename a Space folder, and the World bootstrap writes + * one. A profile that does not materialize Spaces has nothing for either to + * do, which is the gate that keeps them off the portable surface. + */ +export { + registerSpaceDirHandleOwner, + withSpaceDirHandlesReleased, +} from './backends/disk/space-dir-handles.js'; +export type { SpaceDirHandleOwner } from './backends/disk/space-dir-handles.js'; +export { ensureWorldCanvasOnDisk } from './backends/disk/world-canvas.js'; export { withCanvasMutex, updateNode } from '../canvas/write-coordinator.js'; export type { UpdateNodeOptions, diff --git a/apps/server/src/modules/storage/module-boundaries.test.ts b/apps/server/src/modules/storage/module-boundaries.test.ts index ca1e1a926..b46933a2c 100644 --- a/apps/server/src/modules/storage/module-boundaries.test.ts +++ b/apps/server/src/modules/storage/module-boundaries.test.ts @@ -156,6 +156,11 @@ describe('storage dependency direction', () => { const violations: string[] = []; for (const file of sourceFiles) { if (file.startsWith('modules/storage/')) continue; + // Same exemption, and the same reason, as the composition-root rule + // below: exercising an adapter means naming it. A production file that + // names one has bound the application to a backend, which is the thing + // being prevented; a test that names one is choosing its subject. + if (file.endsWith('.test.ts')) continue; for (const spec of specifiersOf(file)) { const target = resolveSpecifier(file, spec); if (target?.includes('modules/storage/backends')) { @@ -202,6 +207,76 @@ describe('storage dependency direction', () => { }); }); +/** + * Phase 4.5's outcome, guarded (proposal §12.5). + * + * The workspace module used to hold a `disk/` segment containing the Disk + * record layout, the `space.json`-derived directory index, and pure naming + * rules — so "where is a Space" was answered outside the storage boundary, in + * a module whose name asserted the substrate. These pin the correction: what + * remains describes the workspace as a place, and anything needing a real + * Space directory asks for it by capability. + */ +describe('workspace module names no backend', () => { + const workspaceFiles = sourceFiles.filter((f) => + f.startsWith('modules/workspace/'), + ); + + it('has no substrate segment', () => { + const substrate = workspaceFiles.filter((f) => + f.startsWith('modules/workspace/disk/'), + ); + expect(substrate).toEqual([]); + expect(workspaceFiles.length).toBeGreaterThan(0); + }); + + it('never imports a storage backend', () => { + const violations: string[] = []; + for (const file of workspaceFiles) { + for (const spec of specifiersOf(file)) { + const target = resolveSpecifier(file, spec); + if (target?.includes('modules/storage/backends')) { + violations.push(`${file} → ${spec}`); + } + } + } + // A Space's directory comes from `spaceDirectory()` on the facade, which + // is the capability; reaching a backend for it would restore exactly the + // coupling this phase removed. + expect(violations).toEqual([]); + }); + + it('names no Disk record or blob layout symbol', () => { + // These are the members that moved to `backends/disk/layout.ts`. Their + // reappearance here would mean the workspace had started describing how a + // backend stores things again, whatever the import path said. + const DISK_LAYOUT = [ + 'SPACE_JSON_FILENAME', + 'canvasJsonPath', + 'nodesDir', + 'nodeFilePath', + 'artifactsDir', + 'artifactPath', + 'tasksPath', + 'eventsPath', + 'deltaLogPath', + 'changesPath', + 'canvasRoot', + ]; + const violations: string[] = []; + for (const file of workspaceFiles) { + if (file.startsWith('modules/workspace/migrations/')) continue; + const source = read(file); + for (const symbol of DISK_LAYOUT) { + if (new RegExp(`\\b${symbol}\\b`).test(source)) { + violations.push(`${file} → ${symbol}`); + } + } + } + expect(violations).toEqual([]); + }); +}); + describe('structured write authority', () => { it('does not expose compatibility create/delete writers from the public barrel', () => { expect(read('modules/storage/index.ts')).not.toMatch( @@ -346,7 +421,6 @@ describe('root forwarding shims', () => { 'modules/agent/memory/analyzer.test.ts', 'modules/canvas/canvas-content-cas.test.ts', 'modules/canvas/canvas.route.test.ts', - 'modules/workspace/disk/world-canvas.ts', 'modules/workspace/migrations/migrate-acp-sessions.ts', ], }; diff --git a/apps/server/src/modules/storage/paths.ts b/apps/server/src/modules/storage/paths.ts index 5f829c8d6..e67803d3c 100644 --- a/apps/server/src/modules/storage/paths.ts +++ b/apps/server/src/modules/storage/paths.ts @@ -6,7 +6,7 @@ * * Import from `storage/backends/disk/layout.js` if you are inside the storage * module; everyone else wants `spaceDirectory()` from `storage/index.js` or - * the workspace-owned paths in `modules/workspace/disk/paths.js`. This file + * the workspace-owned paths in `modules/workspace/paths.js`. This file * exists only so the remaining physical-Disk capability imports keep * resolving while they migrate; it must never contain logic, and no new call * site may import it (enforced by the module-boundary test). diff --git a/apps/server/src/modules/workspace-prepare.ts b/apps/server/src/modules/workspace-prepare.ts index eb9b6f5bd..719c8b922 100644 --- a/apps/server/src/modules/workspace-prepare.ts +++ b/apps/server/src/modules/workspace-prepare.ts @@ -11,7 +11,7 @@ import { mkdirSync } from 'node:fs'; -import { ensureWorldCanvasOnDisk } from './workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './storage/index.js'; import { migrateLegacyAcpSessions } from './workspace/migrations/migrate-acp-sessions.js'; import { migrateLegacyAgenetesThreads, diff --git a/apps/server/src/modules/workspace/disk/paths.ts b/apps/server/src/modules/workspace/paths.ts similarity index 97% rename from apps/server/src/modules/workspace/disk/paths.ts rename to apps/server/src/modules/workspace/paths.ts index 4dad391c1..773816e27 100644 --- a/apps/server/src/modules/workspace/disk/paths.ts +++ b/apps/server/src/modules/workspace/paths.ts @@ -35,10 +35,12 @@ import path from 'node:path'; -import { spaceDirectory } from '../../storage/index.js'; -import { getWorkspacePath } from '../../workspace.js'; +import { sanitizeId } from '../../utils/fs.js'; +import { spaceDirectory } from '../storage/index.js'; +import { getWorkspacePath } from '../workspace.js'; import type { Namespace } from '@agenetes/protocol'; + /** * The `.history/` tier is named by the Disk backend, which owns most of what * is in it. The families below sit there only because they were written next diff --git a/apps/server/src/prompt/skills/loader.ts b/apps/server/src/prompt/skills/loader.ts index 4001a298a..07340b780 100644 --- a/apps/server/src/prompt/skills/loader.ts +++ b/apps/server/src/prompt/skills/loader.ts @@ -49,7 +49,7 @@ import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { userSkillsDir } from '../../modules/workspace/disk/paths.js'; +import { userSkillsDir } from '../../modules/workspace/paths.js'; import { getWorkspacePath } from '../../modules/workspace.js'; import { getLogger } from '../../utils/logger.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; @@ -140,7 +140,7 @@ export const SYSTEM_SKILLS_DIR = existsSync(BUNDLED_SKILLS_DIR) // `skills//SKILL.md` path) lives in the memory module — see // `modules/agent/memory/sandbox.ts` + `writers.ts`. Keeping it out of // the loader means the loader does not need to expose write paths; the -// user-side root is owned by `userSkillsDir()` in `workspace/disk/paths.ts`. +// user-side root is owned by `userSkillsDir()` in `workspace/paths.ts`. // ─── Validation ───────────────────────────────────────────────────────────── diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index 84e22ad13..b722c29ff 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1673,7 +1673,7 @@ bearing: change-review records and Tasks are not history, whatever Disk's arrives, the group comes back — and `events` is where it was before, so nothing else has to move. -### 12.5 Phase 4.5 — storage-owned layout moves inside the boundary — **planned** +### 12.5 Phase 4.5 — storage-owned layout moves inside the boundary — **implemented** Phase 5 adds a second structured backend. Before it does, the layout knowledge that belongs to the _Disk_ backend has to stop living outside `storage/`. @@ -1841,6 +1841,16 @@ scope by definition. Phase 5 rebases onto this and drops its `utils/naming.ts` extraction, its `workspace/disk/naming.ts` shim, and the corresponding roadmap edits. +**Landed.** `modules/workspace/` is flat and holds `paths.ts` plus +`migrations/`; the Disk record layout, blob layout, directory index, name +index, directory-handle coordination, and World bootstrap all sit under +`storage/backends/disk/`. Consumers needing a real Space directory call +`spaceDirectory()`; the two Disk capabilities the application still needs — +directory-handle release and World bootstrap — are re-exported from the facade +rather than reached by path. Three guards in `module-boundaries.test.ts` pin +the result: the workspace module has no substrate segment, imports no backend, +and names no Disk layout symbol. + #### 12.5.7 Findings from step 5, and one follow-up Working the six consumers of §12.5.1 individually produced four different From b8a1e1677070013f6a6d8a465c558f44fbc56233 Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Mon, 17 Aug 2026 14:05:51 +0800 Subject: [PATCH 8/9] fix(storage): address phase 4.5 review findings --- .../agent/conversation/prompt/debug-prompt.ts | 11 +-- .../src/modules/agent/memory/analyzer.test.ts | 9 -- .../src/modules/agent/memory/analyzer.ts | 23 ----- .../src/modules/agent/memory/trigger.ts | 6 +- .../src/modules/agent/memory/worker.test.ts | 7 +- .../server/src/modules/agent/memory/worker.ts | 14 +-- .../agent/tools/handlers/fs-sandbox.test.ts | 8 +- .../agent/tools/handlers/fs-sandbox.ts | 21 ++--- .../modules/canvas/canvas-executor.test.ts | 2 +- .../modules/canvas/import-node-src.test.ts | 33 +++++++ .../src/modules/canvas/import-node-src.ts | 21 +++-- .../server/src/modules/storage/canvas-dirs.ts | 11 +-- .../modules/storage/module-boundaries.test.ts | 65 +++----------- apps/server/src/modules/storage/paths.ts | 12 +-- apps/server/src/modules/workspace-prepare.ts | 4 +- .../migrations/migrate-chat-threads.test.ts | 86 +++++++++++++++++-- .../migrations/migrate-chat-threads.ts | 78 +++++++++++++++-- .../migrations/migrate-chat-turns.test.ts | 70 +++++++++++++++ .../migrations/migrate-chat-turns.ts | 34 ++++++-- docs/architecture/agent-memory.md | 12 +-- docs/architecture/canvas-storage.md | 4 +- docs/proposals/multi-backend-storage.md | 28 +++--- docs/proposals/world-canvas.md | 2 +- 23 files changed, 379 insertions(+), 182 deletions(-) diff --git a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts index 2c9f5ef55..d5f703834 100644 --- a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts +++ b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts @@ -15,9 +15,9 @@ */ import { appendFileSync } from 'node:fs'; +import path from 'node:path'; import { mkdirp } from '../../../../utils/fs.js'; -import { chatDir } from '../../../storage/paths.js'; import { chatPromptLogPath } from '../../../workspace/paths.js'; import type { Context } from '@earendil-works/pi-ai'; @@ -167,12 +167,9 @@ export function dumpAssembledPrompt(params: DumpPromptParams): void { }); out.push('', ''); - mkdirp(chatDir(canvasId)); - appendFileSync( - chatPromptLogPath(canvasId, params.threadId), - out.join('\n'), - 'utf-8', - ); + const logPath = chatPromptLogPath(canvasId, params.threadId); + mkdirp(path.dirname(logPath)); + appendFileSync(logPath, out.join('\n'), 'utf-8'); } catch (err) { params.logger.warn( { err: String(err) }, diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index aa70f8e16..caa15d40e 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -10,7 +10,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const physicalState = vi.hoisted(() => ({ root: '' })); vi.mock('../agent.service.js', () => ({ runAgent: vi.fn() })); -vi.mock('./trigger.js', () => ({ readMemoryState: vi.fn() })); vi.mock('../../../prompt/index.js', () => ({ loadAgent: vi.fn(), listSkills: vi.fn(), @@ -24,7 +23,6 @@ vi.mock('../../workspace/paths.js', () => ({ import { runAgent } from '../agent.service.js'; import { runAnalysisPass } from './analyzer.js'; -import { readMemoryState } from './trigger.js'; import { loadAgent, listSkills } from '../../../prompt/index.js'; import { getStructuredStore } from '../../storage/index.js'; @@ -98,11 +96,6 @@ beforeEach(() => { .mockReturnValue( emptyAgentStream() as unknown as ReturnType, ); - vi.mocked(readMemoryState).mockReset().mockReturnValue({ - counter: 0, - lastAnalyzedAt: null, - lastSeenThreadCursor: null, - }); vi.mocked(loadAgent) .mockReset() .mockReturnValue({ @@ -132,7 +125,6 @@ describe('runAnalysisPass repository sources', () => { expect(space).toHaveBeenCalledWith('canvas-a'); expect(recordRead).toHaveBeenCalledTimes(1); expect(eventsRead).not.toHaveBeenCalled(); - expect(readMemoryState).not.toHaveBeenCalled(); expect(loadAgent).not.toHaveBeenCalled(); expect(runAgent).not.toHaveBeenCalled(); }); @@ -162,7 +154,6 @@ describe('runAnalysisPass repository sources', () => { await expect(runAnalysisPass('canvas-a')).resolves.toEqual({ status: 'completed', results: [], - latestChatTs: null, }); expect(space).toHaveBeenCalledTimes(1); diff --git a/apps/server/src/modules/agent/memory/analyzer.ts b/apps/server/src/modules/agent/memory/analyzer.ts index 6d70595e9..fb21fc66f 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -59,17 +59,11 @@ const MAX_EVENTS_IN_DIGEST = 100; * does NOT call `markAnalyzed` (so the next trigger retries). Writer * rejections are *not* errors — they come back as `ok:false` tool * results which we surface in the returned summary. - * - * `latestChatTs` is always `null` since the chat digest was removed; see - * {@link ContextBundle}. The worker still persists it as - * `lastSeenThreadCursor`, which is the resume point a reinstated digest - * would need. */ export type AnalysisPassResult = | { status: 'completed'; results: WriteResult[]; - latestChatTs: number | null; } | { status: 'skipped'; reason: 'space-not-found' }; @@ -129,7 +123,6 @@ export async function runAnalysisPass( return { status: 'completed', results: writeResults, - latestChatTs: bundle.latestChatTs, }; } @@ -166,21 +159,6 @@ function parseWriteResult(raw: string): WriteResult | null { interface ContextBundle { messages: Message[]; summary: string; - /** - * Always `null`. The chat digest that produced it read - * `/.history/chat/*.json` for a `{ messages: [] }` shape that two - * migrations retired — turns moved to `.turns.jsonl` and then into the - * Agenetes Tier-2 store under `chat_v2/`, so the only files left matching - * that glob are change-record arrays with no `messages` key. The reader - * had therefore returned nothing for some time, in production and in a - * test that pointed it at a non-existent directory. - * - * The field and its `lastSeenThreadCursor` plumbing survive the removal - * because they are the resume point any reinstated digest needs; the - * turns themselves belong to the agent runtime and are read through - * `agenetes.history()`, not through storage. See proposal §12.5.7. - */ - latestChatTs: number | null; } /** @@ -236,7 +214,6 @@ async function assembleContext( return { messages, summary: parts.join(', ') || '(empty)', - latestChatTs: null, }; } diff --git a/apps/server/src/modules/agent/memory/trigger.ts b/apps/server/src/modules/agent/memory/trigger.ts index 10ed258a0..6367f1c58 100644 --- a/apps/server/src/modules/agent/memory/trigger.ts +++ b/apps/server/src/modules/agent/memory/trigger.ts @@ -11,9 +11,9 @@ * enqueued when this crosses {@link OP_THRESHOLD}. * lastAnalyzedAt epoch ms of the last successful analysis; * null until the first pass lands. - * lastSeenThreadCursor pi-ai context timestamp of the last - * analysed chat turn — lets `context.ts` (PR-C) - * only pull "new" turns into the analysis prompt. + * lastSeenThreadCursor retained compatibility field from the removed + * legacy chat digest. Existing state files preserve it, + * but current analysis passes do not advance it. * * Persisted at `/.memory/state.json` so the counter * survives process restarts. The file is kept tiny (<128 B) and diff --git a/apps/server/src/modules/agent/memory/worker.test.ts b/apps/server/src/modules/agent/memory/worker.test.ts index e1962ace7..4b89e395b 100644 --- a/apps/server/src/modules/agent/memory/worker.test.ts +++ b/apps/server/src/modules/agent/memory/worker.test.ts @@ -59,19 +59,16 @@ describe('memory worker outcomes', () => { ); }); - it('marks completed passes with only the non-null cursors', async () => { + it('marks completed passes after summarising writer results', async () => { vi.mocked(runAnalysisPass).mockResolvedValue({ status: 'completed', results: [{ ok: true, target: 'space', reason: 'updated' }], - latestChatTs: 25, }); const log = logger(); await runScheduled('canvas-a', log); - expect(markAnalyzed).toHaveBeenCalledWith('canvas-a', { - lastSeenThreadCursor: 25, - }); + expect(markAnalyzed).toHaveBeenCalledWith('canvas-a'); expect(log.info).toHaveBeenCalledWith( '[memory] pass for canvas canvas-a done — 1 ok, 0 rejected', ); diff --git a/apps/server/src/modules/agent/memory/worker.ts b/apps/server/src/modules/agent/memory/worker.ts index 790a9d50e..24b55a6cc 100644 --- a/apps/server/src/modules/agent/memory/worker.ts +++ b/apps/server/src/modules/agent/memory/worker.ts @@ -89,23 +89,13 @@ async function runOnce(canvasId: string, logger?: MemoryLogger): Promise { ); return; } - const { results, latestChatTs } = outcome; + const { results } = outcome; // markAnalyzed is intentionally always called when the pass finished // without throwing — even if individual writers rejected (e.g. a // create-rationale violation). The bookkeeping records "we tried", // not "we wrote". This avoids hammering the threshold with retries // when the LLM keeps producing rejected outputs. - // - // `latestChatTs` advances the chat cursor so the next pass's digest - // only includes strictly newer rows. `null` means that source saw - // nothing new past the existing cursor — in which case we leave the - // cursor untouched (handled by markAnalyzed when the field is - // omitted). - const cursorUpdate: { - lastSeenThreadCursor?: number; - } = {}; - if (latestChatTs !== null) cursorUpdate.lastSeenThreadCursor = latestChatTs; - markAnalyzed(canvasId, cursorUpdate).catch((err: unknown) => { + markAnalyzed(canvasId).catch((err: unknown) => { // markAnalyzed is now async (it shares the per-canvas state // lock with bumpOpCounter). A bookkeeping write failure does // not invalidate the pass — log and continue. diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts index 4f5617e0c..4b3122bf6 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts @@ -6,10 +6,10 @@ import { describe, expect, it } from 'vitest'; import { isArtifactsRel, toPhysicalRel } from './fs-sandbox.js'; /** - * `isArtifactsRel` decides whether a node `src` already points at an - * artifact, which is what stops the import hook copying a file that is - * already stored. It answers from the ref alone — no workspace, no canvas - * directory, no backend — so these cases need no fixture. + * `isArtifactsRel` applies the segment-aware membership rules after a node + * `src` has been safely resolved relative to its actual Space. The import-hook + * tests cover that filesystem resolution; these pure cases pin the remaining + * path classification without a fixture. */ describe('isArtifactsRel', () => { it('accepts both the virtual and physical spellings', () => { diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts index fcf5abd4f..18b80a5ea 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts @@ -72,21 +72,22 @@ const VIRTUAL_PREFIX: ReadonlyArray = [ ]; /** - * Whether a canvas-relative ref denotes something already under `.artifacts/`. + * Whether an already-resolved, Space-relative physical path denotes something + * under `.artifacts/`. * - * A question about the *ref*, not about storage: a node `src` that already - * points at an artifact needs no import, whatever backend holds the bytes. - * Resolving against a synthetic root keeps it that way — no workspace, no - * canvas directory, no filesystem — while still collapsing any `..` that - * would slip past a bare prefix test, the same normalization - * {@link safeResolve} relies on. + * Callers must first resolve the original ref with {@link safeResolve}, then + * make that absolute target relative to the actual Space root. The second + * step matters for refs such as `../Canvas/.artifacts/pic.png`, which leave + * and re-enter the same Space before resolving inside `.artifacts/`. * - * Takes the *physical* form, so pass {@link toPhysicalRel} output. + * Resolving the resulting relative path against a synthetic root keeps this + * membership check independent of storage while preserving segment-aware + * normalization and sibling-prefix protection. */ -export function isArtifactsRel(physicalRel: string): boolean { +export function isArtifactsRel(resolvedPhysicalRel: string): boolean { const [, artifactsPhysical] = VIRTUAL_PREFIX[0]; const root = path.resolve('/', artifactsPhysical); - const target = path.resolve('/', physicalRel); + const target = path.resolve('/', resolvedPhysicalRel); return target === root || target.startsWith(root + path.sep); } diff --git a/apps/server/src/modules/canvas/canvas-executor.test.ts b/apps/server/src/modules/canvas/canvas-executor.test.ts index dd7e50c8a..8bf897503 100644 --- a/apps/server/src/modules/canvas/canvas-executor.test.ts +++ b/apps/server/src/modules/canvas/canvas-executor.test.ts @@ -242,7 +242,7 @@ describe('executeOnServer — MERGE_NODE_DATA CAS', () => { expect(out.conflicts ?? []).toHaveLength(0); expect(out.toVersion).toBe(out.fromVersion + 1); - expect(getCanvasStore('c1').readNode('m1')?.src).toBe('artifacts/new.png'); + expect(getCanvasStore('c1').readNode('m1')?.src).toBe('new.png'); }); it('auto-updates image height when MERGE_NODE_DATA rewrites src', async () => { diff --git a/apps/server/src/modules/canvas/import-node-src.test.ts b/apps/server/src/modules/canvas/import-node-src.test.ts index 8e4c15fed..089ad92ac 100644 --- a/apps/server/src/modules/canvas/import-node-src.test.ts +++ b/apps/server/src/modules/canvas/import-node-src.test.ts @@ -37,6 +37,7 @@ beforeEach(() => { 'c-web-merge-local', 'c-web-merge-remote', 'c-image-local', + 'c-image-reentered', ]) { createCanvas(canvasId); } @@ -269,4 +270,36 @@ describe('importForeignNodeSources — media nodes (regression)', () => { if (src === undefined) throw new Error('Expected a rewritten image src'); expect(await canvasBlobs(canvasId).head(src)).not.toBeNull(); }); + + it('canonicalizes an artifact path that leaves and re-enters the Space', async () => { + const canvasId = 'c-image-reentered'; + const store = getCanvasStore(canvasId); + const spaceDir = spaceDirectory(canvasId); + const artifactsDir = path.join(spaceDir, '.artifacts'); + mkdirSync(artifactsDir, { recursive: true }); + writeFileSync(path.join(artifactsDir, 'pic.png'), 'existing artifact'); + const src = path.join( + '..', + path.basename(spaceDir), + '.artifacts', + 'pic.png', + ); + + const commands: CanvasCommand[] = [ + { + type: 'CREATE_NODES', + nodes: [ + { + nodeType: 'image', + data: { src }, + position: { x: 0, y: 0 }, + }, + ], + }, + ]; + + const out = await importForeignNodeSources(store, canvasId, commands); + + expect(firstSrc(out)).toBe('pic.png'); + }); }); diff --git a/apps/server/src/modules/canvas/import-node-src.ts b/apps/server/src/modules/canvas/import-node-src.ts index e201d42fc..39837f701 100644 --- a/apps/server/src/modules/canvas/import-node-src.ts +++ b/apps/server/src/modules/canvas/import-node-src.ts @@ -41,7 +41,7 @@ import { isArtifactsRel, toPhysicalRel, } from '../agent/tools/handlers/fs-sandbox.js'; -import { canvasBlobs } from '../storage/index.js'; +import { canvasBlobs, spaceDirectory } from '../storage/index.js'; import type { CanvasStore } from '../storage/index.js'; @@ -266,10 +266,21 @@ async function resolveImportedSrc( return null; } - // Already an artifact ref (or a bare artifact key, which `toPhysicalRel` - // maps into `.artifacts/`) — nothing to import. Asked of the ref rather - // than of a resolved path, so no storage layout is involved. - if (isArtifactsRel(physicalRel)) return null; + // A direct artifact child needs no copy, but it still needs the canonical + // bare-key spelling the web resolver serves. Classify the path after + // sandbox resolution so a ref that leaves and re-enters the current Space + // is judged by where it actually lands, while the helper still owns the + // virtual/physical `.artifacts` vocabulary. A nested path is not a blob key, + // so it falls through and is copied into the artifact root below. + const resolvedPhysicalRel = path.relative(spaceDirectory(canvasId), absPath); + if (isArtifactsRel(resolvedPhysicalRel)) { + const key = path.basename(absPath); + const canonicalPath = safeResolve( + canvasId, + toPhysicalRel(`artifacts/${key}`), + ); + if (absPath === canonicalPath) return key; + } // A bare key like `art_abc.png` resolves under the canvas root but has no // file on disk there — leave it so the web resolver builds the artifact URL. diff --git a/apps/server/src/modules/storage/canvas-dirs.ts b/apps/server/src/modules/storage/canvas-dirs.ts index f504101ef..cd38b2510 100644 --- a/apps/server/src/modules/storage/canvas-dirs.ts +++ b/apps/server/src/modules/storage/canvas-dirs.ts @@ -2,12 +2,13 @@ // Licensed under the MIT license. /** - * @deprecated Forwarding shim — the Workspace layout owns these now. + * @deprecated Forwarding shim — the Disk backend owns this directory index. * - * Import from `storage/backends/disk/canvas-dirs.js` instead. This file - * exists only so the many existing physical-Disk capability imports keep - * resolving while they migrate; it must never contain logic, and no new call - * site may import it (enforced by the module-boundary test). + * Inside the storage module, import from + * `storage/backends/disk/canvas-dirs.js`. This file exists only so the + * existing application-level Disk capability imports keep resolving while + * they migrate; it must never contain logic, and no new call site may import + * it (enforced by the module-boundary test). */ export * from './backends/disk/canvas-dirs.js'; diff --git a/apps/server/src/modules/storage/module-boundaries.test.ts b/apps/server/src/modules/storage/module-boundaries.test.ts index b46933a2c..80fdca34e 100644 --- a/apps/server/src/modules/storage/module-boundaries.test.ts +++ b/apps/server/src/modules/storage/module-boundaries.test.ts @@ -219,7 +219,7 @@ describe('storage dependency direction', () => { */ describe('workspace module names no backend', () => { const workspaceFiles = sourceFiles.filter((f) => - f.startsWith('modules/workspace/'), + /^modules\/workspace(?:[./-])/.test(f), ); it('has no substrate segment', () => { @@ -252,11 +252,16 @@ describe('workspace module names no backend', () => { // backend stores things again, whatever the import path said. const DISK_LAYOUT = [ 'SPACE_JSON_FILENAME', + 'WORLD_CANVAS_DIR_NAME', 'canvasJsonPath', 'nodesDir', 'nodeFilePath', + 'ARTIFACTS_DIR_NAME', 'artifactsDir', 'artifactPath', + 'HISTORY_DIR_NAME', + 'historyDir', + 'chatDir', 'tasksPath', 'eventsPath', 'deltaLogPath', @@ -351,15 +356,9 @@ describe('root forwarding shims', () => { expect(body[0]).toMatch(/^export \* from '\.[^']+\.js';$/); }); - /** - * Frozen snapshot of the call sites that already imported these paths when - * the shims were installed. The lists may shrink as consumers migrate; - * a new entry means someone added an importer of a deprecated path, which - * is what the shims exist to stop. - */ - const ALLOWED_IMPORTERS: Record = { + /** Exact snapshot of the remaining deprecated-path importers. */ + const EXPECTED_IMPORTERS: Record = { 'storage/canvas-store.js': [ - 'modules/agent/sketch.service.ts', 'modules/canvas/canvas-search.test.ts', 'modules/canvas/canvas-search.ts', 'modules/canvas/canvas-spatial.ts', @@ -368,10 +367,6 @@ describe('root forwarding shims', () => { 'modules/canvas/node-prompt.ts', 'modules/canvas/world-reference-resolver.ts', 'modules/canvas/world-target-access.ts', - 'modules/preprocessing/pipeline.test.ts', - 'modules/preprocessing/pipeline.ts', - 'modules/preprocessing/stages/cache-check.ts', - 'modules/preprocessing/stages/persist.ts', ], 'storage/canvas-dirs.js': [ 'modules/agent/tools/world-target-read.test.ts', @@ -389,44 +384,17 @@ describe('root forwarding shims', () => { 'modules/workspace.ts', ], 'storage/paths.js': [ - 'modules/agent/acp/service.ts', - 'modules/agent/acp/threads.route.ts', - 'modules/agent/agent.route.ts', - 'modules/agent/agent.service.ts', - 'modules/agent/conversation/prompt/debug-prompt.ts', - 'modules/agent/memory/analyzer.ts', - 'modules/agent/memory/read.ts', - 'modules/agent/memory/sandbox.ts', - 'modules/agent/memory/trigger.ts', - 'modules/agent/skills.route.test.ts', - 'modules/agent/tools/handlers/fs-sandbox.ts', - 'modules/agent/tools/handlers/fs-write.test.ts', - 'modules/agent/tools/handlers/fs-write.ts', - 'modules/canvas/canvas-search.test.ts', - 'modules/canvas/canvas-search.ts', + 'modules/canvas/canvas-content-cas.test.ts', + 'modules/canvas/canvas.route.test.ts', 'modules/canvas/canvas.route.ts', 'modules/canvas/external-watcher.ts', - 'modules/canvas/external.route.ts', - 'modules/canvas/import-node-src.test.ts', - 'modules/canvas/import-node-src.ts', 'modules/canvas/world-target-access.ts', - 'modules/remote_fs/rfs.route.ts', - 'modules/remote_fs/skill.ts', - 'prompt/skills/loader.ts', - // Phase 4.5 relocations, not new couplings. Each of these already read - // a Disk-owned path; it read it from `workspace/disk/paths.js`, which - // this phase emptied of Disk layout (§12.5.2). The same call site now - // names the shim instead. Fourteen entries left this list in the same - // change, because their symbols turned out to be workspace-owned. - 'modules/agent/memory/analyzer.test.ts', - 'modules/canvas/canvas-content-cas.test.ts', - 'modules/canvas/canvas.route.test.ts', 'modules/workspace/migrations/migrate-acp-sessions.ts', ], }; - it.each(Object.keys(ALLOWED_IMPORTERS))( - 'gains no new importer of %s', + it.each(Object.keys(EXPECTED_IMPORTERS))( + 'keeps the exact importer snapshot for %s', (shimPath) => { const importers = sourceFiles .filter((file) => !file.startsWith('modules/storage/')) @@ -435,14 +403,7 @@ describe('root forwarding shims', () => { ) .sort(); - const added = importers.filter( - (f) => !ALLOWED_IMPORTERS[shimPath].includes(f), - ); - expect(added).toEqual([]); - // Shrinking is the goal, so the snapshot is a ceiling, not an equality. - expect(importers.length).toBeLessThanOrEqual( - ALLOWED_IMPORTERS[shimPath].length, - ); + expect(importers).toEqual(EXPECTED_IMPORTERS[shimPath]); }, ); }); diff --git a/apps/server/src/modules/storage/paths.ts b/apps/server/src/modules/storage/paths.ts index e67803d3c..b11ab904e 100644 --- a/apps/server/src/modules/storage/paths.ts +++ b/apps/server/src/modules/storage/paths.ts @@ -4,12 +4,12 @@ /** * @deprecated Forwarding shim — the Disk backend owns its layout now. * - * Import from `storage/backends/disk/layout.js` if you are inside the storage - * module; everyone else wants `spaceDirectory()` from `storage/index.js` or - * the workspace-owned paths in `modules/workspace/paths.js`. This file - * exists only so the remaining physical-Disk capability imports keep - * resolving while they migrate; it must never contain logic, and no new call - * site may import it (enforced by the module-boundary test). + * Inside the storage module, import from + * `storage/backends/disk/layout.js`. Application code should use + * `spaceDirectory()` or the workspace-owned paths when those express the + * capability it needs. This file exists for the remaining explicit Disk + * layout reads while they migrate; it must never contain logic, and no new + * call site may import it (enforced by the module-boundary test). */ export * from './backends/disk/layout.js'; diff --git a/apps/server/src/modules/workspace-prepare.ts b/apps/server/src/modules/workspace-prepare.ts index 719c8b922..83d5503ef 100644 --- a/apps/server/src/modules/workspace-prepare.ts +++ b/apps/server/src/modules/workspace-prepare.ts @@ -40,7 +40,9 @@ export function prepareWorkspaceOnDisk(workspacePath: string): void { migrateLegacyChatThreads(workspacePath); // Second hop (M6.9 row 2): fold legacy `.history/chat/*.turns.jsonl` turns // into the Agenetes two-tier log (`chat_v2/`). MUST run AFTER the pi-ai - // `.json` -> `.turns.jsonl` hop above. + // `.json` -> `.turns.jsonl` hop above. A same-thread Context left by an + // unresolved hop-1 migration, including unreadable JSON, keeps its turn log + // out of this hop until a later activation can reconcile the pair. migrateLegacyChatTurns(workspacePath); // Convert the strict workload/state boundary before any writer opens the // namespace. Keeps the original v1 file as `.agenetes-v1.bak`. diff --git a/apps/server/src/modules/workspace/migrations/migrate-chat-threads.test.ts b/apps/server/src/modules/workspace/migrations/migrate-chat-threads.test.ts index 2d5359043..e7feb5bf0 100644 --- a/apps/server/src/modules/workspace/migrations/migrate-chat-threads.test.ts +++ b/apps/server/src/modules/workspace/migrations/migrate-chat-threads.test.ts @@ -8,7 +8,7 @@ * ✓ seeds selection ids/refs from the [Selected Nodes] block + tag * ✓ folds [SYSTEM Error/Interrupted] rows into the open turn transcript * ✓ migrateLegacyThreadFile: writes .turns.jsonl, renames .json → .bak - * ✓ idempotent — skips when a .turns.jsonl already exists + * ✓ repairs supported legacy/new-format coexistence without data loss * ✓ migrateLegacyChatThreads: sweeps canvases, ignores active sidecars */ @@ -126,12 +126,67 @@ describe('migrateLegacyThreadFile', () => { expect(readJsonLines(turnsPath)).toHaveLength(2); }); - it('is idempotent — skips when a .turns.jsonl already exists', () => { + it('atomically completes a coexisting log that is a legacy prefix', () => { const jsonPath = join(tmp, 'tr.json'); + const turnsPath = jsonPath.replace(/\.json$/, '.turns.jsonl'); + const legacyTurns = legacyContextToTurns(legacyContext()); + writeFileSync(jsonPath, JSON.stringify(legacyContext())); + writeFileSync(turnsPath, `${JSON.stringify(legacyTurns[0])}\n`); + + expect(migrateLegacyThreadFile(jsonPath)).toBe(true); + expect(readJsonLines(turnsPath)).toEqual(legacyTurns); + expect(existsSync(jsonPath)).toBe(false); + expect(existsSync(`${jsonPath}.bak`)).toBe(true); + }); + + it('preserves a newer tail when the legacy conversion is its prefix', () => { + const jsonPath = join(tmp, 'tr.json'); + const turnsPath = jsonPath.replace(/\.json$/, '.turns.jsonl'); + const legacyTurns = legacyContextToTurns(legacyContext()); + const newerTurn: ChatTurnRecord = { + ...legacyTurns[1], + envelope: { + ...legacyTurns[1].envelope, + user: { ...legacyTurns[1].envelope.user, text: 'new-format tail' }, + }, + }; writeFileSync(jsonPath, JSON.stringify(legacyContext())); - writeFileSync(jsonPath.replace(/\.json$/, '.turns.jsonl'), ''); - expect(migrateLegacyThreadFile(jsonPath)).toBe(false); - expect(existsSync(jsonPath)).toBe(true); // untouched + writeFileSync( + turnsPath, + [...legacyTurns, newerTurn] + .map((turn) => JSON.stringify(turn)) + .join('\n') + '\n', + ); + + expect(migrateLegacyThreadFile(jsonPath)).toBe(true); + expect(readJsonLines(turnsPath)).toEqual([ + ...legacyTurns, + newerTurn, + ]); + expect(existsSync(jsonPath)).toBe(false); + expect(existsSync(`${jsonPath}.bak`)).toBe(true); + }); + + it('rejects divergent coexisting logs without retiring either copy', () => { + const jsonPath = join(tmp, 'tr.json'); + const turnsPath = jsonPath.replace(/\.json$/, '.turns.jsonl'); + const [first] = legacyContextToTurns(legacyContext()); + const divergent: ChatTurnRecord = { + ...first, + envelope: { + ...first.envelope, + user: { ...first.envelope.user, text: 'different first turn' }, + }, + }; + writeFileSync(jsonPath, JSON.stringify(legacyContext())); + writeFileSync(turnsPath, `${JSON.stringify(divergent)}\n`); + + expect(() => migrateLegacyThreadFile(jsonPath)).toThrow( + 'diverges from legacy context', + ); + expect(readJsonLines(turnsPath)).toEqual([divergent]); + expect(existsSync(jsonPath)).toBe(true); + expect(existsSync(`${jsonPath}.bak`)).toBe(false); }); }); @@ -153,4 +208,25 @@ describe('migrateLegacyChatThreads', () => { expect(existsSync(join(chat, 'tr.active.json'))).toBe(true); expect(existsSync(join(chat, 'tr.active.json.bak'))).toBe(false); }); + + it('keeps divergent coexistence for a later activation retry', () => { + const chat = join(tmp, 'cv-1', '.history', 'chat'); + const jsonPath = join(chat, 'tr.json'); + const turnsPath = join(chat, 'tr.turns.jsonl'); + const [first] = legacyContextToTurns(legacyContext()); + const divergent: ChatTurnRecord = { + ...first, + envelope: { + ...first.envelope, + user: { ...first.envelope.user, text: 'different first turn' }, + }, + }; + mkdirSync(chat, { recursive: true }); + writeFileSync(jsonPath, JSON.stringify(legacyContext())); + writeFileSync(turnsPath, `${JSON.stringify(divergent)}\n`); + + expect(() => migrateLegacyChatThreads(tmp)).not.toThrow(); + expect(existsSync(jsonPath)).toBe(true); + expect(readJsonLines(turnsPath)).toEqual([divergent]); + }); }); diff --git a/apps/server/src/modules/workspace/migrations/migrate-chat-threads.ts b/apps/server/src/modules/workspace/migrations/migrate-chat-threads.ts index 5b7ebe451..1a8e622b6 100644 --- a/apps/server/src/modules/workspace/migrations/migrate-chat-threads.ts +++ b/apps/server/src/modules/workspace/migrations/migrate-chat-threads.ts @@ -19,8 +19,14 @@ import { existsSync, readdirSync, renameSync } from 'node:fs'; import path from 'node:path'; +import { isDeepStrictEqual } from 'node:util'; -import { appendJsonLine, mkdirp, readJson } from '../../../utils/fs.js'; +import { + atomicWriteText, + mkdirp, + readJsonStrict, + readJsonLinesStrict, +} from '../../../utils/fs.js'; import { buildAgentNodePreview } from '../../agent/node-ref.js'; import type { LegacyChatTurnRecord as ChatTurnRecord } from './legacy/chat-turn-record.js'; @@ -30,6 +36,27 @@ import type { CanvasNodeType } from '@huabu/shared'; type PiMessage = Context['messages'][number]; +/** Whether a parsed value has the legacy Context shape this migration uses. */ +export function isLegacyChatContext(value: unknown): value is Context { + if (typeof value !== 'object' || value === null) return false; + const messages = (value as { messages?: unknown }).messages; + if (!Array.isArray(messages)) return false; + return messages.every((message) => { + if (typeof message !== 'object' || message === null) return false; + const candidate = message as { role?: unknown; content?: unknown }; + if (candidate.role === 'user') { + return ( + typeof candidate.content === 'string' || + Array.isArray(candidate.content) + ); + } + if (candidate.role === 'assistant' || candidate.role === 'toolResult') { + return Array.isArray(candidate.content); + } + return false; + }); +} + /** Trailing tag carrying the explicitly-selected top-level ids. */ const SELECTED_IDS_RE = /\n?\[SYSTEM selectedNodeIds:(\[[^\]]*\])\]\s*$/; @@ -129,14 +156,49 @@ export function legacyContextToTurns(ctx: Context): ChatTurnRecord[] { return turns; } -/** Migrate one thread file in place. Returns true when migrated. */ +function encodeTurns(turns: readonly ChatTurnRecord[]): string { + if (turns.length === 0) return ''; + return `${turns.map((turn) => JSON.stringify(turn)).join('\n')}\n`; +} + +/** + * Migrate one thread file in place. Returns true when the legacy source was + * retired. + * + * A previous launch may have written some turns and then failed before + * renaming the source. Reconcile that coexistence only when one log is a + * prefix of the other: complete a partial converted prefix atomically, or + * preserve a newer tail that follows the complete conversion. Divergent logs + * are left untouched because their ordering cannot be inferred safely. + */ export function migrateLegacyThreadFile(jsonPath: string): boolean { const turnsPath = jsonPath.replace(/\.json$/, '.turns.jsonl'); - if (existsSync(turnsPath)) return false; // already on new format - const ctx = readJson(jsonPath); - if (!ctx || !Array.isArray(ctx.messages)) return false; + const ctx = readJsonStrict(jsonPath); + if (!isLegacyChatContext(ctx)) return false; const turns = legacyContextToTurns(ctx); - for (const t of turns) appendJsonLine(turnsPath, t); + + if (existsSync(turnsPath)) { + const existing = readJsonLinesStrict(turnsPath); + const overlap = Math.min(existing.length, turns.length); + let commonPrefix = 0; + while ( + commonPrefix < overlap && + isDeepStrictEqual(existing[commonPrefix], turns[commonPrefix]) + ) { + commonPrefix += 1; + } + if (commonPrefix < overlap) { + throw new Error( + `Existing turn log diverges from legacy context at turn ${commonPrefix + 1}: ${turnsPath}`, + ); + } + if (existing.length < turns.length) { + atomicWriteText(turnsPath, encodeTurns(turns)); + } + } else { + atomicWriteText(turnsPath, encodeTurns(turns)); + } + renameSync(jsonPath, `${jsonPath}.bak`); return true; } @@ -172,7 +234,9 @@ export function migrateLegacyChatThreads(workspace: string): void { try { migrateLegacyThreadFile(path.join(chatDir, file)); } catch { - // tolerant: one bad thread never aborts the batch + // Tolerant per-file migration: leave an unresolved pair in place for a + // later activation. Hop 2 skips it while the same-thread `.json` + // remains, so neither copy is consumed or overwritten. } } } diff --git a/apps/server/src/modules/workspace/migrations/migrate-chat-turns.test.ts b/apps/server/src/modules/workspace/migrations/migrate-chat-turns.test.ts index bbbdfd296..4ec0c07fe 100644 --- a/apps/server/src/modules/workspace/migrations/migrate-chat-turns.test.ts +++ b/apps/server/src/modules/workspace/migrations/migrate-chat-turns.test.ts @@ -9,6 +9,7 @@ * ✓ pins an empty Tier-1 range (seqStart 1 > seqEnd 0) on every turn * ✓ renames the consumed source log to `.turns.jsonl.bak` * ✓ idempotent — a second sweep neither re-writes nor throws + * ✓ skips a turn log while the same thread's legacy Context remains * ✓ tolerant — a canvas with no chat dir is skipped */ @@ -125,6 +126,75 @@ describe('migrateLegacyChatTurns', () => { expect(readJsonLines(target)).toHaveLength(before.length); }); + it("skips a turn log while the same thread's legacy Context remains", () => { + const src = seedLegacyLog('Canvas A', 'thread-1', [legacyRecord('one')]); + const legacyContext = join( + tmp, + 'Canvas A', + '.history', + 'chat', + 'thread-1.json', + ); + writeFileSync(legacyContext, JSON.stringify({ messages: [] })); + + migrateLegacyChatTurns(tmp); + + expect(existsSync(src)).toBe(true); + expect(existsSync(`${src}.bak`)).toBe(false); + expect(existsSync(legacyContext)).toBe(true); + expect( + existsSync( + join(tmp, 'Canvas A', '.history', 'chat_v2', 'thread-1.turns.jsonl'), + ), + ).toBe(false); + }); + + it('preserves a turn log while the same-thread Context is malformed', () => { + const src = seedLegacyLog('Canvas A', 'thread-1', [legacyRecord('one')]); + const malformedLegacy = join( + tmp, + 'Canvas A', + '.history', + 'chat', + 'thread-1.json', + ); + writeFileSync(malformedLegacy, '{invalid'); + + migrateLegacyChatTurns(tmp); + + expect(existsSync(src)).toBe(true); + expect(existsSync(`${src}.bak`)).toBe(false); + expect(existsSync(malformedLegacy)).toBe(true); + expect( + existsSync( + join(tmp, 'Canvas A', '.history', 'chat_v2', 'thread-1.turns.jsonl'), + ), + ).toBe(false); + }); + + it('does not let non-Context JSON block a valid turn log', () => { + const src = seedLegacyLog('Canvas A', 'thread-1', [legacyRecord('one')]); + const invalidContext = join( + tmp, + 'Canvas A', + '.history', + 'chat', + 'thread-1.json', + ); + writeFileSync(invalidContext, JSON.stringify({ messages: [null] })); + + migrateLegacyChatTurns(tmp); + + expect(existsSync(src)).toBe(false); + expect(existsSync(`${src}.bak`)).toBe(true); + expect(existsSync(invalidContext)).toBe(true); + expect( + existsSync( + join(tmp, 'Canvas A', '.history', 'chat_v2', 'thread-1.turns.jsonl'), + ), + ).toBe(true); + }); + it('skips a canvas that has no legacy chat dir', () => { mkdirSync(join(tmp, 'Empty Canvas', '.history'), { recursive: true }); expect(() => migrateLegacyChatTurns(tmp)).not.toThrow(); diff --git a/apps/server/src/modules/workspace/migrations/migrate-chat-turns.ts b/apps/server/src/modules/workspace/migrations/migrate-chat-turns.ts index 59cb0c7a6..f9b270c7f 100644 --- a/apps/server/src/modules/workspace/migrations/migrate-chat-turns.ts +++ b/apps/server/src/modules/workspace/migrations/migrate-chat-turns.ts @@ -24,11 +24,12 @@ * * ### Idempotent, launch-only * - * Skips any thread whose `chat_v2` log already exists, and renames each - * source log to `.bak` on success, so a re-run never double-writes. One bad - * thread never aborts the batch. The frozen {@link LegacyChatTurnRecord} - * descriptor (never the live chat-store types) is the only dependency on - * the old shape. + * Skips any thread whose `chat_v2` log already exists or whose oldest + * same-thread `.json` Context remains unresolved or unreadable, and renames + * each consumed source log to `.bak` on success, so a re-run never + * double-writes. One bad thread never aborts the batch. The frozen + * {@link LegacyChatTurnRecord} descriptor (never the live chat-store types) + * is the only dependency on the old shape. */ import { existsSync, readdirSync, renameSync } from 'node:fs'; @@ -38,13 +39,28 @@ import { FileTurnStore } from '@agenetes/agenetes'; import { isLegacyChatTurnRecord } from './legacy/chat-turn-record.js'; import { legacyChatTurnToAgentTurn } from './legacy/fold-legacy-turn.js'; -import { readJsonLines } from '../../../utils/fs.js'; +import { isLegacyChatContext } from './migrate-chat-threads.js'; +import { readJsonLines, readJsonStrict } from '../../../utils/fs.js'; import type { PersistedTurn } from '@agenetes/agenetes'; import type { Namespace } from '@agenetes/protocol'; const LEGACY_SUFFIX = '.turns.jsonl'; +/** + * Whether hop 2 must preserve a turn log for hop 1 to reconcile later. + * Malformed or unreadable JSON is treated as unresolved durable state; a + * parsed value only blocks when it is a Context shape hop 1 can consume. + */ +function hasUnresolvedLegacyContext(contextPath: string): boolean { + try { + const candidate = readJsonStrict(contextPath); + return candidate !== null && isLegacyChatContext(candidate); + } catch { + return true; + } +} + /** * Migrate one legacy `.turns.jsonl` into the thread's `chat_v2` * Tier-2 log. Returns true when migrated. No-op (returns false) when the @@ -122,6 +138,12 @@ export function migrateLegacyChatTurns(workspace: string): void { // sidecar, already-retired `.bak` files, and anything else. if (!file.endsWith(LEGACY_SUFFIX)) continue; const threadId = file.slice(0, -LEGACY_SUFFIX.length); + // Hop 1 leaves the oldest Context in place when coexistence cannot be + // reconciled safely. Do not consume its paired turn log: preserving both + // formats lets a later workspace activation retry the pair. + if (hasUnresolvedLegacyContext(path.join(chatDir, `${threadId}.json`))) { + continue; + } try { migrateLegacyTurnFile( turnStore, diff --git a/docs/architecture/agent-memory.md b/docs/architecture/agent-memory.md index bcaddb3b1..27b77825b 100644 --- a/docs/architecture/agent-memory.md +++ b/docs/architecture/agent-memory.md @@ -1,7 +1,7 @@ # Agent Memory > Status: Shipped -> Last updated: 2026-08-07 +> Last updated: 2026-08-17 Lets the agent remember, across sessions and canvases, "who the user is, what this canvas is about, and which approaches are reusable". The whole mechanism is @@ -40,13 +40,13 @@ Two independent write paths: - Then a **per-canvas single-flight** worker runs ([memory/worker.ts](../../apps/server/src/modules/agent/memory/worker.ts)): an already-running pass just sets a pending flag, no queue. - `setImmediate` dispatch — the route responds to the client first; the curator starts on the next tick. - Failures only `warn`, never throw; the next trigger naturally retries. -- If the Space record no longer exists, the pass is skipped before reading memory state/chat files or calling the model, and `markAnalyzed` is not advanced. +- If the Space record no longer exists, the pass is skipped before reading memory files or calling the model, and `markAnalyzed` is not advanced. - The curator uses [agents/memory/AGENT.md](../../apps/server/src/prompt/agents/memory/AGENT.md), max 5 iterations, sequential tool calls. - The curator runs with the `memory` model role, which resolves through the Utility Model and, when Utility is not configured, defaults to the cheapest eligible model in the chat provider (ultimately the Chat Model). ### 2.2 Explicit requests in chat -Normal ask / operate turns do not write memory directly. Ask is read-only, and operate reserves `fs_write` for an explicitly invoked `/create-skill` or `/update-skill`; a plain-language "remember this" request instead enters the chat digest and becomes a high-confidence candidate when the background curator next runs. This path is delayed until the canvas reaches the automatic-curation threshold, and the curator may still choose not to write. +Normal ask / operate turns do not write memory directly. Ask is read-only, and operate reserves `fs_write` for an explicitly invoked `/create-skill` or `/update-skill`. The background curator no longer scans chat files; its evidence is the current Space snapshot, recent action events, and existing memory. A plain-language "remember this" request therefore becomes a curation candidate only when it is also reflected in those Space-owned sources. User Skill creation and updates are explicit slash-command flows on the built-in operate surface. `/create-skill` checks the catalogue for near matches but does not silently switch to an update; `/update-skill` resolves and reads an existing user or merged Skill before writing it. @@ -135,7 +135,7 @@ The curator AGENT.md points at all three sub-docs. The operate Agent receives Sk | overwrite + replace_string primitives | [memory/writers.ts](../../apps/server/src/modules/agent/memory/writers.ts) | | Dual-root path check | [memory/sandbox.ts](../../apps/server/src/modules/agent/memory/sandbox.ts) | | Read entry | [memory/read.ts](../../apps/server/src/modules/agent/memory/read.ts) | -| Path helpers | [storage/paths.ts](../../apps/server/src/modules/storage/paths.ts) | +| Path helpers | [workspace/paths.ts](../../apps/server/src/modules/workspace/paths.ts) | | `fs_write` tool def | [tools/definitions.ts](../../apps/server/src/modules/agent/tools/definitions.ts) | | fs_write handler | [tools/handlers/fs-write.ts](../../apps/server/src/modules/agent/tools/handlers/fs-write.ts) | | fs_read handler | [tools/handlers/fs-read.ts](../../apps/server/src/modules/agent/tools/handlers/fs-read.ts) | @@ -148,7 +148,7 @@ The curator AGENT.md points at all three sub-docs. The operate Agent receives Sk ## 6. Relationship to existing systems -- The curator reads Space existence, the bounded `events.jsonl` action tail, and intents through structured repositories. Chat digests and memory body/state files remain physical Disk capabilities; the storage migration changes no file format. +- The curator reads Space existence and the bounded action-event tail through the structured repository. Memory body/state files remain materialized workspace capabilities. Chat history is owned by Agenetes and is not part of the curator bundle. - `/.memory/` is in `ALWAYS_SKIP` ([fs-sandbox.ts](../../apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts)) — invisible to grep / find / ls; reachable only through the controlled `read("memory/space.md")` path. - The skill loader uses mtime + 2s TTL + `invalidateUserSkill(id)` for write-then-read freshness. System skills are cached once-and-done. @@ -158,5 +158,5 @@ The curator AGENT.md points at all three sub-docs. The operate Agent receives Sk - Every write path goes through `MemorySandboxError` validation. - Within a single Node process, per-canvas single-flight keeps the curator from concurrently writing the same canvas; workspace memory is serialised across canvases by an in-module `workspaceMemoryLock`. Multi-process deployment needs separate design; single-process is assumed today. -- `markAnalyzed` advances `lastSeenThreadCursor`, so the next chat digest only sees new turns and never re-scans history. +- `markAnalyzed` records only the completion timestamp. The legacy `lastSeenThreadCursor` key is preserved when old `state.json` files are rewritten, but current passes do not advance or consume it. - A failed write (rationale too short, cap exceeded, non-unique `oldString`, …) → `WriteResult.ok=false`; the worker logs it into the summary and the next trigger retries. diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index ce181e9d2..39a9cabb9 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -55,11 +55,11 @@ Key points: - The Disk `BlobStore` maps each Space scope to `.artifacts/`, with blobs named `` and no manifest file — the filename is the URL key. Ordinary callers resolve the scope through `canvasBlobs(canvasId)`: `put()` requires an existing Space record, while reads and `deleteAll()` remain available for recovery after a record goes missing. `CanvasStore` owns no artifact methods. Only the Disk blob and structured backends are implemented and selectable today. - Remote PDF preprocessing writes the already-fetched source bytes into the Space BlobStore as `artifact-.pdf` before structured persistence and replaces the node's remote `src` with that key. As with other artifact imports, this blob write precedes the node write operation; a later structured persistence failure may therefore leave an unreferenced blob until Space deletion, while a blob-write failure degrades to retaining the remote URL. - Events are append-only JSONL (`events.jsonl`); each line is `{ ts: number, payload: RecentAction }`. -- The memory analyzer reads Space existence, at most 100 recent action events, and intent episodes through one `SpaceHandle`. A missing Space skips the pass before reading memory state/chat files or calling the model; corrupt part data still fails the pass. Chat digest and memory body/state files remain explicit Disk paths. +- The memory analyzer reads Space existence and at most 100 recent action events through one `SpaceHandle`. A missing Space skips the pass before reading memory files or calling the model; corrupt part data still fails the pass. Memory body/state files remain materialized workspace paths, while Agenetes-owned chat history is not part of the curator bundle. - **Chat history is Chat-V2, owned by Agenetes L2 — not `CanvasStore`.** The canonical per-thread conversation is a two-tier append-only log under `chat_v2/`: Tier-1 `.events.jsonl` (`AgentStreamEvent` deltas a running turn appends, written by `FileEventLogStore`) and Tier-2 `.turns.jsonl` (folded `AgentTurn`s, written by `FileTurnStore` — the only tier `history()` reads back). These files sit under the canvas `.history/` only because it is the Agenetes namespace `storage.root` (`canvasAcpNamespace(canvasId)`); `CanvasStore` never touches them. Do **not** confuse `chat_v2/.events.jsonl` (agent stream events) with the sibling `events.jsonl` (canvas action log) — same suffix, unrelated content. - Durable Agenetes workload records live in `.history/threads.json` (`agenetes-v2` schema, one record per thread; written by `FileThreadStore`). The host-local `namespace.storage.root` is never persisted: reads bind each record to the current Space namespace, so a Home synchronized across computers cannot redirect storage back to another machine's absolute path. - Canonical Task and Run records live in `.history/tasks.json`, owned by Huabu Server through the async `SpaceTasks` ledger (`read`, `create`, and `runs.create`/`runs.update`). The Disk adapter validates the versioned snapshot and referential integrity on every read, rejects duplicate identifiers and Runs whose Task is absent, serializes read-modify-write operations with an independent per-Canvas process-local mutex, and atomically replaces the file. This mutex is intentionally separate from the Canvas topology write coordinator, so Task metadata does not participate in `space.json` version CAS. -- Legacy chat files are one-way migrated into `chat_v2/` at workspace activation and retired to `.bak`: the oldest pi-ai `Context` `chat/.json` via `migrate-chat-threads.ts` (hop 1), then the M5.6 `chat/.turns.jsonl` / `.active.json` via `migrate-chat-turns.ts` (hop 2). The obsolete `CanvasStore` chat methods and `chatPath()` helper were removed in Phase 2; `chatDir()` remains because change-review and agent-owned files still use that directory. +- Legacy chat files are one-way migrated into `chat_v2/` at workspace activation and retired to `.bak`: the oldest pi-ai `Context` `chat/.json` via `migrate-chat-threads.ts` (hop 1), then the M5.6 `chat/.turns.jsonl` / `.active.json` via `migrate-chat-turns.ts` (hop 2). If hop 1 finds both formats after an interrupted launch, it completes a strict converted prefix atomically or preserves an existing tail when the full conversion is its prefix. Divergent logs are retained rather than guessed or overwritten; hop 2 skips the paired turn log while a valid same-thread legacy Context remains or its JSON cannot be read safely, so a later activation can retry both copies without blocking unrelated migrations. The obsolete `CanvasStore` chat methods and `chatPath()` helper were removed in Phase 2; `chatDir()` remains because change-review and agent-owned files still use that directory. ## 3. Storage composition and ownership diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index b722c29ff..b787dd494 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1818,8 +1818,10 @@ In: `modules/workspace/`, with no substrate segment. 4. Introduce the Space materialization capability, re-found `canvasRoot` on it, and move the ACP, watcher, memory, and World-bootstrap consumers onto it. -5. Move `agent/memory/analyzer.ts` off `chatDir`, and relocate the agent-owned - families of §12.5.3 into the agent domain. +5. Move `agent/memory/analyzer.ts` off `chatDir`. Keep the agent-owned path + families of §12.5.3 in `modules/workspace/paths.ts` as a transitional + materialization surface; moving them into their owning domains remains + follow-up work. 6. Extend the module-boundary test to fail when a non-storage file imports a storage-owned layout symbol — the guard that stops this recurring. @@ -1841,15 +1843,17 @@ scope by definition. Phase 5 rebases onto this and drops its `utils/naming.ts` extraction, its `workspace/disk/naming.ts` shim, and the corresponding roadmap edits. -**Landed.** `modules/workspace/` is flat and holds `paths.ts` plus -`migrations/`; the Disk record layout, blob layout, directory index, name -index, directory-handle coordination, and World bootstrap all sit under -`storage/backends/disk/`. Consumers needing a real Space directory call -`spaceDirectory()`; the two Disk capabilities the application still needs — -directory-handle release and World bootstrap — are re-exported from the facade -rather than reached by path. Three guards in `module-boundaries.test.ts` pin -the result: the workspace module has no substrate segment, imports no backend, -and names no Disk layout symbol. +**Landed for the Workspace-to-storage substrate move.** `modules/workspace/` +is flat and holds `paths.ts` plus `migrations/`; the Disk record layout, blob +layout, directory index, name index, directory-handle coordination, and World +bootstrap all sit under `storage/backends/disk/`. Consumers that need only a +real Space directory call `spaceDirectory()`; directory-handle release and +World bootstrap are re-exported from the facade rather than reached by path. +This does not close every application-to-Disk read: `canvas.route.ts`, +`external-watcher.ts`, and `world-target-access.ts` intentionally retain +compatibility-shim reads, alongside migration and test callers. Three guards +in `module-boundaries.test.ts` pin the Workspace move: the workspace module +has no substrate segment, imports no backend, and names no Disk layout symbol. #### 12.5.7 Findings from step 5, and one follow-up @@ -2151,7 +2155,7 @@ Before a new backend is production-ready: | [`.../storage/compatibility/canvas.ts`](../../apps/server/src/modules/storage/compatibility/canvas.ts) | Residual Disk read surface plus direct-module lifecycle test fixtures; production structured mutations enumerated in §12.4 use the portable ports. | | [`apps/server/src/modules/agent/memory/analyzer.ts`](../../apps/server/src/modules/agent/memory/analyzer.ts) | P3 repository consumer for strict Space existence, bounded action events, and intent episodes; physical chat and memory files remain Disk-specific. | | [`apps/server/src/modules/canvas/write-coordinator.ts`](../../apps/server/src/modules/canvas/write-coordinator.ts) | Canvas mutation coordinator and per-Space write lock, held across asynchronous node read, revision CAS, and put. | -| [`apps/server/src/modules/workspace/disk/`](../../apps/server/src/modules/workspace/disk/) | Cross-domain physical Workspace layout: paths, canvas dirs, naming, name index, dir handles, World bootstrap. `storage/paths.ts` forwards here. | +| [`apps/server/src/modules/workspace/`](../../apps/server/src/modules/workspace/) | Workspace-owned and transitional materialization paths plus migrations. Disk layout and directory-index code now live under `storage/backends/disk/`. | | [`apps/server/src/modules/canvas/canvas-executor.ts`](../../apps/server/src/modules/canvas/canvas-executor.ts) | Canonical command execution; submits one ordered node/record/delta batch through `SpaceHandle.write`. | | [`apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts`](../../apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts) | Current real-Disk path resolution and traversal for built-in agent file tools. | | [`apps/server/src/modules/agent/acp/capabilities/fs.ts`](../../apps/server/src/modules/agent/acp/capabilities/fs.ts) | Synthetic ACP `/space` read capability, currently not wired into the production driver. | diff --git a/docs/proposals/world-canvas.md b/docs/proposals/world-canvas.md index bdbb5cad0..946841704 100644 --- a/docs/proposals/world-canvas.md +++ b/docs/proposals/world-canvas.md @@ -97,7 +97,7 @@ This is the first-version implementation contract. It preserves the current sing Workspace preparation should run an idempotent `ensureWorldCanvasOnDisk(workspacePath)` after existing migrations. A workspace without `.world/space.json` receives an empty World with a newly generated stable `canvasId`; subsequent activations reuse the persisted identity. -The hidden directory is deliberately outside the ordinary Space index. The current scanner in [`canvas-dirs.ts`](../../apps/server/src/modules/workspace/disk/canvas-dirs.ts) already skips dot-prefixed entries; it should read `.world/space.json` into a separate World entry without returning it from `listCanvasDirEntries()`. `canvasDirName(worldCanvasId)` should resolve that entry to `.world`, allowing the existing `getCanvasStore(worldCanvasId)` and all canvas-relative path helpers to continue operating unchanged. +The hidden directory is deliberately outside the ordinary Space index. The current scanner in [`canvas-dirs.ts`](../../apps/server/src/modules/storage/backends/disk/canvas-dirs.ts) already skips dot-prefixed entries; it should read `.world/space.json` into a separate World entry without returning it from `listCanvasDirEntries()`. `canvasDirName(worldCanvasId)` should resolve that entry to `.world`, allowing the existing `getCanvasStore(worldCanvasId)` and all canvas-relative path helpers to continue operating unchanged. The World must use a generated ID rather than a fixed `canvasId = "world"`. Frontend viewport persistence is keyed only by canvas ID, and Agenetes uses canvas ID as its namespace name; a generated ID prevents different workspaces from sharing those identities. From 23f2de74a63fbe6e253fd13d72da40f115a378fd Mon Sep 17 00:00:00 2001 From: Yuge Zhang Date: Mon, 17 Aug 2026 16:37:48 +0800 Subject: [PATCH 9/9] fix(storage): stop a coexisting chat pair from stalling both migration hops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workspace can hold both formats for one thread: the legacy pi-ai `Context` and a `.turns.jsonl` the live app wrote independently. Hop 1 threw whenever the Context's conversion was not a prefix of that log, the sweep swallowed the error, and hop 2 then skipped the thread because the `.json` was still there. Neither copy advanced, `chat_v2` never received the turns, and the thread rendered empty — silently, on every launch. The comment promised a later activation would retry, but nothing changed between activations, so the stall was permanent. Give the coexistence decision one owner and make every branch terminate: - Hop 1 resolves divergence instead of throwing. The live log wins, because it is the one the app has been appending to; the Context is preserved verbatim as `.json.unresolved` and the divergence is logged. Its extra turns stay off the canvas, which is also what happened before the reconciliation existed — but the bytes remain and the turn log is free to fold. - Hop 2 drops its same-thread `.json` gate. A Context still sitting there is one hop 1 could not read, and folding the turn log anyway is what keeps the history reachable. - The admission gate no longer validates every message against today's `Message` union. These files come from older builds, so an unfamiliar row is expected; `legacyContextToTurns` already tolerates unknown roles and now skips non-object rows too. One junk row cost the whole thread its history. - The tolerant sweep logs what it could not migrate. Add an end-to-end suite that activates a legacy workspace the way a launch does and drives the production routes at the production prefixes: the pre-rename files migrate, the Space / node body / artifact / events read back over HTTP, `GET /api/agent/history/:threadId` serves all three seeded threads, and new writes (node content, an agent execute that imports an artifact, a new Space) survive a second activation without double-folding. The Space is titled so its directory name differs from its canvasId, so a layout that leaked the id into a path fails there. Reinstating either old behaviour fails that suite with an empty message list for the divergent thread, which was the user-visible symptom. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/modules/workspace-prepare.ts | 5 +- .../legacy-workspace-activation.test.ts | 540 ++++++++++++++++++ .../migrations/migrate-chat-threads.test.ts | 65 ++- .../migrations/migrate-chat-threads.ts | 96 +++- .../migrations/migrate-chat-turns.test.ts | 61 +- .../migrations/migrate-chat-turns.ts | 34 +- 6 files changed, 683 insertions(+), 118 deletions(-) create mode 100644 apps/server/src/modules/workspace/legacy-workspace-activation.test.ts diff --git a/apps/server/src/modules/workspace-prepare.ts b/apps/server/src/modules/workspace-prepare.ts index 83d5503ef..c04514d40 100644 --- a/apps/server/src/modules/workspace-prepare.ts +++ b/apps/server/src/modules/workspace-prepare.ts @@ -40,9 +40,8 @@ export function prepareWorkspaceOnDisk(workspacePath: string): void { migrateLegacyChatThreads(workspacePath); // Second hop (M6.9 row 2): fold legacy `.history/chat/*.turns.jsonl` turns // into the Agenetes two-tier log (`chat_v2/`). MUST run AFTER the pi-ai - // `.json` -> `.turns.jsonl` hop above. A same-thread Context left by an - // unresolved hop-1 migration, including unreadable JSON, keeps its turn log - // out of this hop until a later activation can reconcile the pair. + // `.json` -> `.turns.jsonl` hop above, which resolves every coexisting pair + // before this hop folds the turn logs. migrateLegacyChatTurns(workspacePath); // Convert the strict workload/state boundary before any writer opens the // namespace. Keeps the original v1 file as `.agenetes-v1.bak`. diff --git a/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts b/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts new file mode 100644 index 000000000..756569b33 --- /dev/null +++ b/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts @@ -0,0 +1,540 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * End-to-end activation of a legacy workspace, over the production routes. + * + * Phase 4.5 moved the Disk record layout inside the storage boundary and + * routed every "where is this Space" question through `spaceDirectory()`. This + * suite exists to prove that the move did not change what the app can read or + * write. It does not test a module — it activates a workspace the way a launch + * does (`setWorkspacePath` → `prepareWorkspaceOnDisk` → every migration) and + * then drives the same URLs the web client uses, mounted at the same prefixes + * as `app.ts`. + * + * How the "old" workspace is built, and why it is honest: + * + * - `space.json`, `nodes/