diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 20178346..23b5e1dc 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -56,6 +56,7 @@ import { originGuardPlugin, resolveAllowedHostnames, } from './modules/security/index.js'; +import { closeStorage } from './modules/storage/index.js'; import webRoutes from './modules/web/web.route.js'; import { initWorkspaceFromEnv, @@ -366,6 +367,11 @@ if (bundledAgentTeamsPath) { // after the process is gone. Closing them here lets `app.close()` (driven // by the SIGTERM/SIGINT handlers in server.ts) tear them down gracefully. app.addHook('onClose', async () => resetExternalNoteSessions()); +// Close the storage connections on graceful shutdown. Disk holds nothing a +// process exit would not release, so this earns its place by being the seat +// a connection-holding backend will need — a pool nobody closes leaks on +// every restart, and the lifecycle is where that is visible. +app.addHook('onClose', async () => closeStorage()); // Capture the bound TCP port for L1-owned reachback (RFS): the // canvas-scoped `HUABU_RFS_URL` base is built from this. RFS is // canvas-coupled and therefore a pure L1 concern, so the port lives in diff --git a/apps/server/src/modules/agent/conversation/prompt/attachments.ts b/apps/server/src/modules/agent/conversation/prompt/attachments.ts index 3beedf85..27396046 100644 --- a/apps/server/src/modules/agent/conversation/prompt/attachments.ts +++ b/apps/server/src/modules/agent/conversation/prompt/attachments.ts @@ -227,7 +227,7 @@ export async function buildAttachmentParts( if (resolvedCanvasId && resolvedFilename) { try { const bytes = - await space(resolvedCanvasId).blobs.read(resolvedFilename); + await space(resolvedCanvasId).artifacts.read(resolvedFilename); // Attachments are inlined as text; binary bytes simply // decode to mojibake and the URL-only branch is used instead. if (bytes) fileContent = bytes.toString('utf-8'); 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 d5f70383..2eaf8a69 100644 --- a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts +++ b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts @@ -17,9 +17,10 @@ import { appendFileSync } from 'node:fs'; import path from 'node:path'; -import { mkdirp } from '../../../../utils/fs.js'; -import { chatPromptLogPath } from '../../../workspace/paths.js'; +import { sanitizeId } from '../../../../utils/fs.js'; +import { space } from '../../../storage/index.js'; +import type { SpaceSubstrate } from '../../../storage/index.js'; import type { Context } from '@earendil-works/pi-ai'; import type { FastifyBaseLogger } from 'fastify'; @@ -133,10 +134,31 @@ export interface DumpPromptParams { logger: FastifyBaseLogger; } +/** This module's namespace on the storage extension substrate. */ +const PROMPT_LOG_NAMESPACE = 'huabu.prompt.log'; + +/** + * Appends chained so turns land in the log in the order they were dumped. + * + * Resolving a substrate is asynchronous while the call site is a synchronous + * driver callback, so the write cannot happen inline any more. Each turn's + * text is still rendered synchronously — a snapshot of the messages as they + * were — and only the append is deferred. + */ +let appendTail: Promise = Promise.resolve(); + +/** Where this module keeps one log per thread on a Disk substrate. */ +function diskLogPath(substrate: SpaceSubstrate, threadId: string): string { + return path.join( + substrate.directory, + `${sanitizeId(threadId, 'threadId')}.prompt.log`, + ); +} + /** * Append a readable dump of the assembled prompt for one turn. No-op - * unless `HUABU_DEBUG_PROMPT` is set or `canvasId` is missing (the log - * lives under the canvas chat dir). Never throws. + * unless `HUABU_DEBUG_PROMPT` is set, or `canvasId` is missing (the log is + * per-Space state). Never throws, and never blocks the turn. */ export function dumpAssembledPrompt(params: DumpPromptParams): void { if (!isPromptDebugEnabled()) return; @@ -167,13 +189,25 @@ export function dumpAssembledPrompt(params: DumpPromptParams): void { }); out.push('', ''); - const logPath = chatPromptLogPath(canvasId, params.threadId); - mkdirp(path.dirname(logPath)); - appendFileSync(logPath, out.join('\n'), 'utf-8'); + const block = out.join('\n'); + appendTail = appendTail + .then(async () => { + // A Space deleted mid-turn has no substrate, and this is the last + // thing that should recreate one for a debug file. + const substrate = await space(canvasId).extension(PROMPT_LOG_NAMESPACE); + if (!substrate) return; + appendFileSync(diskLogPath(substrate, params.threadId), block, 'utf-8'); + }) + .catch((err: unknown) => { + params.logger.warn( + { err: String(err) }, + 'dumpAssembledPrompt: failed to write prompt debug log', + ); + }); } catch (err) { params.logger.warn( { err: String(err) }, - 'dumpAssembledPrompt: failed to write prompt debug log', + 'dumpAssembledPrompt: failed to render prompt debug log', ); } } diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index caa15d40..6aaa95b7 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -7,17 +7,32 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const physicalState = vi.hoisted(() => ({ root: '' })); +const physicalState = vi.hoisted(() => ({ + root: '', + canvasMemory: null as string | null, +})); vi.mock('../agent.service.js', () => ({ runAgent: vi.fn() })); vi.mock('../../../prompt/index.js', () => ({ loadAgent: vi.fn(), listSkills: vi.fn(), })); -vi.mock('../../storage/index.js', () => ({ getStructuredStore: vi.fn() })); +vi.mock('../../storage/index.js', () => ({ + getStructuredStore: vi.fn(), + // The memory body is a blob under the Space's own scope now, so the + // snapshot reads it through here rather than off a path. + space: (canvasId: string) => ({ + memory: { + read: async () => + physicalState.canvasMemory === null + ? null + : Buffer.from(physicalState.canvasMemory, 'utf8'), + canvasId, + }, + }), + SPACE_MEMORY_BLOB_NAME: 'space.md', +})); 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 fb21fc66..e5da5d82 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -33,11 +33,9 @@ import { type CanvasFile, type SpaceHandle, } from '../../storage/index.js'; -import { - canvasMemoryPath, - workspaceMemoryPath, -} from '../../workspace/paths.js'; +import { workspaceMemoryPath } from '../../workspace/paths.js'; import { runAgent } from '../agent.service.js'; +import { readCanvasMemory } from './read.js'; import type { MemoryLogger } from './index.js'; import type { WriteResult } from './writers.js'; @@ -197,7 +195,7 @@ async function assembleContext( parts.push(`${events.count} ops`); } - const memorySnapshot = readMemorySnapshot(canvasId); + const memorySnapshot = await readMemorySnapshot(canvasId); messages.push({ role: 'user', content: `[SYSTEM Current memory]\n${memorySnapshot}`, @@ -279,14 +277,14 @@ function readEventsDigest(events: readonly CanvasEvent[]): EventsDigest | null { return { text: summaries.join('\n'), count: events.length }; } -function readMemorySnapshot(canvasId: string): string { +async function readMemorySnapshot(canvasId: string): Promise { const parts: string[] = []; const longTerm = readFileSafe(workspaceMemoryPath()); parts.push('## Long-term memory'); parts.push(longTerm.trim().length > 0 ? longTerm.trim() : '(empty)'); - const canvas = readFileSafe(canvasMemoryPath(canvasId)); + const canvas = (await readCanvasMemory(canvasId)) ?? ''; parts.push(''); parts.push('## Canvas memory'); parts.push(canvas.trim().length > 0 ? canvas.trim() : '(empty)'); diff --git a/apps/server/src/modules/agent/memory/read.ts b/apps/server/src/modules/agent/memory/read.ts index c8776b05..a9f6fa8d 100644 --- a/apps/server/src/modules/agent/memory/read.ts +++ b/apps/server/src/modules/agent/memory/read.ts @@ -15,10 +15,8 @@ import { existsSync, readFileSync } from 'node:fs'; -import { - workspaceMemoryPath, - canvasMemoryPath, -} from '../../workspace/paths.js'; +import { space, SPACE_MEMORY_BLOB_NAME } from '../../storage/index.js'; +import { workspaceMemoryPath } from '../../workspace/paths.js'; /** * Read the user memory body. @@ -34,12 +32,19 @@ export function readWorkspaceMemory(): string | null { } /** - * Read the per-canvas canvas memory body. + * Read the per-Space memory body. * - * Same null-on-empty contract as {@link readWorkspaceMemory}. + * Same null-on-empty contract as {@link readWorkspaceMemory}. A blob under the + * Space's own memory scope (proposal §6.4.3, disposition D) — unlike the + * Workspace memory above, which is not scoped to a Space and stays a file. */ -export function readCanvasMemory(canvasId: string): string | null { - return readNonEmpty(canvasMemoryPath(canvasId)); +export async function readCanvasMemory( + canvasId: string, +): Promise { + const bytes = await space(canvasId).memory.read(SPACE_MEMORY_BLOB_NAME); + if (bytes === null) return null; + const raw = bytes.toString('utf8'); + return raw.trim().length === 0 ? null : raw; } function readNonEmpty(file: string): string | null { diff --git a/apps/server/src/modules/agent/memory/sandbox.ts b/apps/server/src/modules/agent/memory/sandbox.ts index 0085b4e4..24d34ae1 100644 --- a/apps/server/src/modules/agent/memory/sandbox.ts +++ b/apps/server/src/modules/agent/memory/sandbox.ts @@ -13,7 +13,7 @@ * * Everything else is rejected. Each writer goes through * {@link resolveLongTermPath} / {@link resolveUserSkillPath} / - * {@link resolveWorkingMemoryPath} so the security model lives in one + * one resolver per tier so the security model lives in one * place — same posture as the chat sandbox at * `modules/agent/tools/handlers/fs-sandbox.ts`. * @@ -29,8 +29,6 @@ import { workspaceMemoryPath, settingDir, userSkillsDir, - canvasMemoryDir, - canvasMemoryPath, } from '../../workspace/paths.js'; /** Thrown by every resolver below on out-of-sandbox attempts. */ @@ -97,16 +95,3 @@ export function resolveUserSkillPath(id: string): string { ensureUnderRoot(root, target, 'user skills'); return target; } - -/** - * 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/paths.ts` already constrains the result, - * but going through `ensureUnderRoot` keeps the invariant explicit). - */ -export function resolveWorkingMemoryPath(canvasId: string): string { - const root = canvasMemoryDir(canvasId); - const target = canvasMemoryPath(canvasId); - ensureUnderRoot(root, target, 'canvas memory'); - return target; -} diff --git a/apps/server/src/modules/agent/memory/trigger.test.ts b/apps/server/src/modules/agent/memory/trigger.test.ts new file mode 100644 index 00000000..f661111e --- /dev/null +++ b/apps/server/src/modules/agent/memory/trigger.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Memory-worker bookkeeping over the storage extension substrate. + * + * This is the first owner to build a store on a substrate, so it doubles as + * evidence that the substrate is usable without the port growing a data API: + * everything here is this module's own format, in a place storage handed it + * and never reads (proposal §6.4.4). + */ + +import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { + bumpOpCounter, + markAnalyzed, + OP_THRESHOLD, + readMemoryState, +} from './trigger.js'; +import { createSpace, deleteSpace, space } from '../../storage/index.js'; +import { setWorkspacePath } from '../../workspace.js'; + +const CANVAS = 'canvas-memory-trigger'; +let tmp: string; + +beforeEach(async () => { + tmp = mkdtempSync(path.join(tmpdir(), 'huabu-memory-trigger-')); + setWorkspacePath(tmp); + const created = await createSpace(CANVAS, 'Trigger'); + if (!created.ok) throw new Error('Expected to create the Space'); +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); +}); + +describe('memory trigger state', () => { + it('starts from zero and persists what it counts', async () => { + await expect(readMemoryState(CANVAS)).resolves.toEqual({ + counter: 0, + lastAnalyzedAt: null, + lastSeenThreadCursor: null, + }); + + await expect(bumpOpCounter(CANVAS, 3)).resolves.toBe(false); + await expect(bumpOpCounter(CANVAS, 4)).resolves.toBe(false); + + await expect(readMemoryState(CANVAS)).resolves.toMatchObject({ + counter: 7, + }); + }); + + it('signals once at the threshold and resets in the same write', async () => { + await expect(bumpOpCounter(CANVAS, OP_THRESHOLD)).resolves.toBe(true); + + // Reset in the same write, so the very next op batch cannot double-fire. + await expect(readMemoryState(CANVAS)).resolves.toMatchObject({ + counter: 0, + }); + await expect(bumpOpCounter(CANVAS, 1)).resolves.toBe(false); + }); + + it('keeps concurrent bumps from losing increments', async () => { + await Promise.all( + Array.from({ length: 10 }, () => bumpOpCounter(CANVAS, 1)), + ); + + await expect(readMemoryState(CANVAS)).resolves.toMatchObject({ + counter: 10, + }); + }); + + it('records an analysis pass without touching the counter', async () => { + await bumpOpCounter(CANVAS, 5); + await markAnalyzed(CANVAS, { lastSeenThreadCursor: 42 }); + + const state = await readMemoryState(CANVAS); + expect(state.counter).toBe(5); + expect(state.lastSeenThreadCursor).toBe(42); + expect(state.lastAnalyzedAt).toEqual(expect.any(Number)); + }); + + it('keeps each Space to its own bookkeeping', async () => { + const other = 'canvas-memory-trigger-other'; + const created = await createSpace(other, 'Other'); + if (!created.ok) throw new Error('Expected to create the second Space'); + + await bumpOpCounter(CANVAS, 4); + await bumpOpCounter(other, 9); + + await expect(readMemoryState(CANVAS)).resolves.toMatchObject({ + counter: 4, + }); + await expect(readMemoryState(other)).resolves.toMatchObject({ counter: 9 }); + }); + + it('drops a write for a Space that was deleted mid-flight', async () => { + await bumpOpCounter(CANVAS, 1); + const spaceDir = space(CANVAS).diskTree?.directory(); + if (!spaceDir) throw new Error('Expected the Disk backend in this test'); + + await deleteSpace(CANVAS); + + // The op-counter hook fires after the delete has already removed the + // Space. This used to need a guard here — a bare write would recreate the + // directory as a stub holding nothing but bookkeeping. The port refuses a + // substrate for a Space that is gone, so the write is a silent no-op and + // nothing is resurrected. + await expect(bumpOpCounter(CANVAS, 1)).resolves.toBe(false); + expect(existsSync(spaceDir)).toBe(false); + }); +}); diff --git a/apps/server/src/modules/agent/memory/trigger.ts b/apps/server/src/modules/agent/memory/trigger.ts index 1b619b7c..f2d4b1ce 100644 --- a/apps/server/src/modules/agent/memory/trigger.ts +++ b/apps/server/src/modules/agent/memory/trigger.ts @@ -15,19 +15,38 @@ * 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 - * atomic-written; read / write failures are reported but never thrown - * — losing this state at worst means we miss or duplicate one - * analysis pass, which is harmless. + * Persisted on the storage extension substrate under the `huabu.memory` + * namespace, so the counter survives process restarts. Storage hands this + * module a place and nothing else — it never sees these three fields, and this + * module owns the format, one small store per backend kind (proposal §6.4.4). + * The state is kept tiny (<128 B) and atomic-written; read / write failures are + * reported but never thrown — losing it at worst means we miss or duplicate + * one analysis pass, which is harmless. */ -import { existsSync } from 'node:fs'; +import path from 'node:path'; -import { atomicWriteJson, mkdirp, readJson } from '../../../utils/fs.js'; +import { atomicWriteJson, readJson } from '../../../utils/fs.js'; import { createKeyedMutex } from '../../../utils/keyed-mutex.js'; import { space } from '../../storage/index.js'; -import { memoryStatePath, canvasMemoryDir } from '../../workspace/paths.js'; + +import type { SpaceSubstrate } from '../../storage/index.js'; + +/** This module's namespace on the substrate. */ +const MEMORY_NAMESPACE = 'huabu.memory'; + +/** + * Where this module keeps its state on a Disk substrate. + * + * The whole store, because the state is one whole-value rewrite. A key/value + * member on the port would have covered exactly this and nothing else, for + * every owner forever — which is why the port hands over a place instead + * (§6.4.4). An owner that later wants the same shape extracts a helper *over* + * the substrate, never a port member. + */ +function diskStatePath(substrate: SpaceSubstrate): string { + return path.join(substrate.directory, 'state.json'); +} /** Op-count threshold that triggers a memory analysis pass. */ export const OP_THRESHOLD = 50; @@ -59,9 +78,10 @@ const EMPTY_STATE: MemoryState = { * we'd rather miscount a few ops than crash the request pipeline on * a corrupted bookkeeping file. */ -export function readMemoryState(canvasId: string): MemoryState { - if (!existsSync(memoryStatePath(canvasId))) return { ...EMPTY_STATE }; - const raw = readJson>(memoryStatePath(canvasId)); +export async function readMemoryState(canvasId: string): Promise { + const substrate = await space(canvasId).extension(MEMORY_NAMESPACE); + if (!substrate) return { ...EMPTY_STATE }; + const raw = readJson>(diskStatePath(substrate)); if (!raw || typeof raw !== 'object') return { ...EMPTY_STATE }; return { counter: typeof raw.counter === 'number' ? raw.counter : 0, @@ -74,23 +94,24 @@ export function readMemoryState(canvasId: string): MemoryState { }; } -/** Atomic write of the memory state, creating `.memory/` on demand. */ -export function writeMemoryState(canvasId: string, state: MemoryState): void { - // Resurrection guard: the op-counter `onResponse` hook fires - // *after* DELETE /api/canvas/:id has rm -rf'd the canvas dir, and - // would otherwise mkdirp `.memory/` + drop a fresh `state.json` - // here \u2014 leaving behind a stub canvas dir containing only that - // 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. - // Disk-only by construction: the hazard is an ad-hoc file write - // recreating a directory the delete removed, and a backend with no - // directory has no such hazard. Phase 4.6 retires the guard entirely when - // this state moves onto the extension substrate (proposal §12.6.3). - const tree = space(canvasId).diskTree; - if (tree && !existsSync(tree.directory())) return; - mkdirp(canvasMemoryDir(canvasId)); - atomicWriteJson(memoryStatePath(canvasId), state); +/** + * Atomic write of the memory state. + * + * No resurrection guard of its own. The op-counter `onResponse` hook fires + * *after* DELETE /api/canvas/:id has removed the Space, and this module used + * to check the Space directory itself before writing — otherwise it would drop + * a fresh `state.json` into a directory the delete had just removed, leaving a + * stub Space behind. `extension()` refuses a substrate for a Space that is + * gone, so the guard now lives in the one place that can state it for every + * owner rather than being re-derived by each (proposal §12.6.3). + */ +export async function writeMemoryState( + canvasId: string, + state: MemoryState, +): Promise { + const substrate = await space(canvasId).extension(MEMORY_NAMESPACE); + if (!substrate) return; + atomicWriteJson(diskStatePath(substrate), state); } /** @@ -112,16 +133,16 @@ export async function bumpOpCounter( delta: number, ): Promise { if (!Number.isFinite(delta) || delta <= 0) return false; - return stateLock(canvasId, () => { - const state = readMemoryState(canvasId); + return stateLock(canvasId, async () => { + const state = await readMemoryState(canvasId); state.counter += delta; if (state.counter < OP_THRESHOLD) { - writeMemoryState(canvasId, state); + await writeMemoryState(canvasId, state); return false; } // Threshold crossed: reset and signal. state.counter = 0; - writeMemoryState(canvasId, state); + await writeMemoryState(canvasId, state); return true; }); } @@ -144,12 +165,12 @@ export async function markAnalyzed( lastSeenThreadCursor?: number; } = {}, ): Promise { - await stateLock(canvasId, () => { - const state = readMemoryState(canvasId); + await stateLock(canvasId, async () => { + const state = await readMemoryState(canvasId); state.lastAnalyzedAt = Date.now(); if (opts.lastSeenThreadCursor !== undefined) { state.lastSeenThreadCursor = opts.lastSeenThreadCursor; } - writeMemoryState(canvasId, state); + await writeMemoryState(canvasId, state); }); } diff --git a/apps/server/src/modules/agent/memory/writers.ts b/apps/server/src/modules/agent/memory/writers.ts index 9462eb87..fa50c83f 100644 --- a/apps/server/src/modules/agent/memory/writers.ts +++ b/apps/server/src/modules/agent/memory/writers.ts @@ -12,10 +12,14 @@ * unique substring (Claude * Code style edit). * - * Both take an already-sandbox-resolved absolute path; callers - * (currently `tools/handlers/fs-write.ts`) own the path → tier - * mapping. The `tier` knob here is purely for behaviour that varies - * by destination: + * Both take a {@link MemoryDocument} — where the bytes live, resolved by the + * caller (currently `tools/handlers/fs-write.ts`), which owns the tier + * mapping. The indirection exists because the tiers no longer share a + * substrate: the Workspace-scoped ones are files, while a Space's memory body + * is a blob under its own scope (proposal §6.4.3, disposition D). Everything + * below is rules about the *content*, so none of it should know which. + * + * The `tier` knob is purely for behaviour that varies by destination: * * - cap enforcement (workspace + canvas only — skill bodies are * allowed to grow larger) @@ -38,6 +42,58 @@ import { atomicWriteText, mkdirp } from '../../../utils/fs.js'; import { createKeyedMutex } from '../../../utils/keyed-mutex.js'; import type { MemoryLogger } from './index.js'; +import type { BlobScope } from '../../storage/index.js'; + +// ─── Where a document lives ──────────────────────────────────────────────── + +/** + * One memory document, addressed however its tier stores it. + * + * `target` is what a {@link WriteResult} names, so it is the caller's own + * vocabulary rather than a physical location — an agent that reads + * `memory/space.md` should see that spelling back, not wherever the bytes + * happen to sit. + */ +export interface MemoryDocument { + readonly target: string; + read(): Promise; + write(body: string): Promise; +} + +/** A document that is a real file — the Workspace-scoped tiers. */ +export function fileDocument( + absPath: string, + parentDir: string, +): MemoryDocument { + return { + target: absPath, + async read(): Promise { + return existsSync(absPath) ? readFileSync(absPath, 'utf8') : null; + }, + async write(body: string): Promise { + mkdirp(parentDir); + atomicWriteText(absPath, body); + }, + }; +} + +/** A document that is a blob — a Space's memory body. */ +export function blobDocument( + scope: BlobScope, + name: string, + target: string, +): MemoryDocument { + return { + target, + async read(): Promise { + const bytes = await scope.read(name); + return bytes === null ? null : bytes.toString('utf8'); + }, + async write(body: string): Promise { + await scope.put(name, Buffer.from(body, 'utf8')); + }, + }; +} // ─── Concurrency guards ──────────────────────────────────────────────────── // @@ -71,10 +127,8 @@ export const SKILL_CREATE_RATIONALE_MIN = 20; interface CommonArgs { tier: MemoryTier; - /** Absolute path, already sandbox-validated by the caller. */ - absPath: string; - /** Directory to `mkdirp` before writing. */ - parentDir: string; + /** Where the bytes live, already sandbox-validated by the caller. */ + document: MemoryDocument; /** * Required when `tier === 'skill'` — used to invalidate the user * skill loader cache after a successful write. Ignored otherwise. @@ -102,24 +156,24 @@ export async function overwriteMemoryFile( return runForTier(args.tier, () => doOverwrite(args)); } -function doOverwrite(args: OverwriteArgs): WriteResult { +async function doOverwrite(args: OverwriteArgs): Promise { + const { target } = args.document; try { const body = ensureTrailingNewline(args.body); if (args.tier !== 'skill') { const capCheck = checkCap(body); - if (!capCheck.ok) return reject(args.absPath, capCheck.reason); + if (!capCheck.ok) return reject(target, capCheck.reason); } - mkdirp(args.parentDir); - atomicWriteText(args.absPath, body); + await args.document.write(body); if (args.tier === 'skill' && args.skillId) { invalidateUserSkill(args.skillId); } args.logger?.info( - `[memory] ${args.tier} overwritten at ${args.absPath} (${body.length} bytes)`, + `[memory] ${args.tier} overwritten at ${target} (${body.length} bytes)`, ); - return { ok: true, target: args.absPath, reason: 'overwritten' }; + return { ok: true, target, reason: 'overwritten' }; } catch (err) { - return rejectFromError(err, args.absPath); + return rejectFromError(err, target); } } @@ -147,34 +201,32 @@ export async function replaceStringInMemoryFile( return runForTier(args.tier, () => doReplaceString(args)); } -function doReplaceString(args: ReplaceStringArgs): WriteResult { +async function doReplaceString(args: ReplaceStringArgs): Promise { + const { target } = args.document; try { if (typeof args.oldString !== 'string' || args.oldString.length === 0) { - return reject(args.absPath, 'oldString is required and non-empty'); + return reject(target, 'oldString is required and non-empty'); } if (typeof args.newString !== 'string') { - return reject(args.absPath, 'newString is required (use "" to delete)'); + return reject(target, 'newString is required (use "" to delete)'); } if (args.oldString === args.newString) { - return reject(args.absPath, 'oldString and newString are identical'); + return reject(target, 'oldString and newString are identical'); } - if (!existsSync(args.absPath)) { + const before = await args.document.read(); + if (before === null) { return reject( - args.absPath, + target, `file does not exist — use mode="overwrite" to create it`, ); } - const before = readFileSync(args.absPath, 'utf8'); const idx = before.indexOf(args.oldString); if (idx === -1) { - return reject( - args.absPath, - 'oldString not found in file — no edit applied', - ); + return reject(target, 'oldString not found in file — no edit applied'); } if (before.indexOf(args.oldString, idx + 1) !== -1) { return reject( - args.absPath, + target, 'oldString matches multiple times — add more surrounding context to make it unique', ); } @@ -185,19 +237,18 @@ function doReplaceString(args: ReplaceStringArgs): WriteResult { ); if (args.tier !== 'skill') { const capCheck = checkCap(after); - if (!capCheck.ok) return reject(args.absPath, capCheck.reason); + if (!capCheck.ok) return reject(target, capCheck.reason); } - mkdirp(args.parentDir); - atomicWriteText(args.absPath, after); + await args.document.write(after); if (args.tier === 'skill' && args.skillId) { invalidateUserSkill(args.skillId); } args.logger?.info( - `[memory] ${args.tier} edited at ${args.absPath} (${after.length} bytes)`, + `[memory] ${args.tier} edited at ${target} (${after.length} bytes)`, ); - return { ok: true, target: args.absPath, reason: 'edited' }; + return { ok: true, target, reason: 'edited' }; } catch (err) { - return rejectFromError(err, args.absPath); + return rejectFromError(err, target); } } diff --git a/apps/server/src/modules/agent/tools/handlers/fs-read.ts b/apps/server/src/modules/agent/tools/handlers/fs-read.ts index c3abcc46..2931d985 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-read.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-read.ts @@ -177,7 +177,7 @@ export async function handleRead( 'memory/space.md is Space-scoped but no canvasId is bound to this request', ); } - content = readCanvasMemory(args.canvasId); + content = await readCanvasMemory(args.canvasId); } else { throw new Error( `Unknown memory path "${rel}". Valid: memory/user.md, memory/space.md`, 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 90de3f1b..07673982 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts @@ -30,7 +30,7 @@ import { readFileSync, readdirSync, statSync, type Dirent } from 'node:fs'; import path from 'node:path'; import { parseFrontmatter } from '../../../../utils/markdown-frontmatter.js'; -import { space } from '../../../storage/index.js'; +import { space, unavailableCapabilityMessage } from '../../../storage/index.js'; // ─── Always-skipped directory names ───────────────────────────────────────── @@ -143,17 +143,13 @@ export function safeResolve(canvasId: string, rel: string): string { ) { throw new Error(`Invalid canvasId: ${canvasId}`); } - // The built-in file tools are Disk-only and stated as such (proposal - // §6.4.3, disposition A): off Disk the first-party agent reaches a Space - // over RFS/HTTP, which is what external agents already use. Refusing here - // is the backstop behind the capability matrix, not the primary check. + // Disk-only, and declared as such: `builtin-file-tools` is a + // capability-matrix entry, so an operator learns this when they select a + // profile rather than when an agent calls a tool. Refusing here is the + // backstop behind that declaration, phrased the same way. const tree = space(canvasId).diskTree; - if (!tree) { - throw new Error( - 'Built-in file tools need a Space directory, which the active ' + - 'structured backend does not provide.', - ); - } + if (!tree) + throw new Error(unavailableCapabilityMessage('builtin-file-tools')); const root = tree.directory(); // Accept the clean virtual prefixes (`upload/`, `artifacts/`) as aliases // for their hidden on-disk dirs so agents can reference either form. 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 5f17d9ae..ce4d010a 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 @@ -13,26 +13,27 @@ * ✓ result envelope shape (`{ ok, target, reason }`) for both ok and reject */ -import { - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - rmSync, -} from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { handleFsWrite } from './fs-write.js'; +import { createSpace, space } from '../../../storage/index.js'; import { - canvasMemoryPath, userSkillsDir, workspaceMemoryPath, } from '../../../workspace/paths.js'; import { setWorkspacePath } from '../../../workspace.js'; +/** Where Disk places the Space memory blob. */ +function memoryBlobFile(canvasId: string): string { + const tree = space(canvasId).diskTree; + if (!tree) throw new Error('Expected the Disk backend in this test'); + return join(tree.directory(), '.memory', 'space.md'); +} + interface ParsedResult { ok: boolean; target: string; @@ -46,15 +47,14 @@ function parse(raw: string): ParsedResult { let tmp: string; const canvasId = 'cv-fs-write-test'; -beforeEach(() => { +beforeEach(async () => { tmp = mkdtempSync(join(tmpdir(), 'huabu-fs-write-')); setWorkspacePath(tmp); - // `canvasRoot(canvasId)` falls back to `/` when the - // canvas-dir index has no entry for the id (see `canvasDirName` in - // `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 }); + // A real Space, not just a directory: the memory body is a blob under the + // Space's own scope now, and blobs may only be written for a Space whose + // record exists. + const created = await createSpace(canvasId, canvasId); + if (!created.ok) throw new Error('Expected to create the Space'); }); afterEach(() => { @@ -172,8 +172,9 @@ describe('handleFsWrite — overwrite', () => { } as never), ); expect(r.ok).toBe(true); - expect(r.target).toBe(canvasMemoryPath(canvasId)); - expect(readFileSync(canvasMemoryPath(canvasId), 'utf8')).toBe( + // The agent sees back the path it asked for, not wherever the bytes sit. + expect(r.target).toBe('memory/space.md'); + expect(readFileSync(memoryBlobFile(canvasId), 'utf8')).toBe( 'canvas briefing\n', ); }); 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 501d8527..da6c039f 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.ts @@ -32,20 +32,19 @@ import { existsSync } from 'node:fs'; import path from 'node:path'; import { normalizeRel } from './fs-sandbox.js'; -import { - canvasMemoryDir, - settingDir, - userSkillsDir, -} from '../../../workspace/paths.js'; +import { space, SPACE_MEMORY_BLOB_NAME } from '../../../storage/index.js'; +import { settingDir, userSkillsDir } from '../../../workspace/paths.js'; import { resolveLongTermPath, resolveUserSkillPath, - resolveWorkingMemoryPath, } from '../../memory/sandbox.js'; import { + blobDocument, + fileDocument, overwriteMemoryFile, replaceStringInMemoryFile, SKILL_CREATE_RATIONALE_MIN, + type MemoryDocument, type MemoryTier, type WriteResult, } from '../../memory/writers.js'; @@ -79,7 +78,7 @@ export async function handleFsWrite(args: FsWriteArgs): Promise { // future literal added to the schema produces a clear runtime error // until the dispatcher is updated. return reject( - target.absPath, + target.document.target, `unknown mode: ${(args as { mode?: string }).mode ?? '(missing)'}`, ); } @@ -88,9 +87,16 @@ export async function handleFsWrite(args: FsWriteArgs): Promise { interface ResolvedTarget { tier: MemoryTier; - absPath: string; - parentDir: string; + /** Where the bytes live — a file for the Workspace tiers, a blob for a + * Space's memory body. */ + document: MemoryDocument; skillId?: string; + /** + * Skill tier only. The create-rationale rule asks whether the file exists + * yet and where its parent is, which are questions about a real path — and + * user skills are Workspace-scoped, so they have one. + */ + skillPaths?: { absPath: string; parentDir: string }; /** The normalised relative path, for error messages. */ path: string; } @@ -106,8 +112,7 @@ function resolveTarget( if (rel === 'memory/user.md') { return { tier: 'workspace', - absPath: resolveLongTermPath(), - parentDir: settingDir(), + document: fileDocument(resolveLongTermPath(), settingDir()), path: rel, }; } @@ -122,8 +127,15 @@ function resolveTarget( } return { tier: 'canvas', - absPath: resolveWorkingMemoryPath(args.canvasId), - parentDir: canvasMemoryDir(args.canvasId), + // A blob under the Space's own memory scope (proposal §6.4.3, + // disposition D). The agent still sees the path it asked for; the + // sandbox resolver this used to need is gone, and so is the ad-hoc + // write against a directory it assembled. + document: blobDocument( + space(args.canvasId).memory, + SPACE_MEMORY_BLOB_NAME, + rel, + ), path: rel, }; } @@ -144,10 +156,11 @@ function resolveTarget( const skillId = segs[1]; try { const absPath = resolveUserSkillPath(skillId); + const parentDir = path.dirname(absPath); return { tier: 'skill', - absPath, - parentDir: path.dirname(absPath), + document: fileDocument(absPath, parentDir), + skillPaths: { absPath, parentDir }, skillId, path: rel, }; @@ -169,7 +182,7 @@ async function handleOverwrite( target: ResolvedTarget, ): Promise { if (typeof args.body !== 'string') { - return reject(target.absPath, 'mode="overwrite" requires "body"'); + return reject(target.document.target, 'mode="overwrite" requires "body"'); } // Skill create rule: when the target file does not yet exist on a @@ -178,11 +191,15 @@ async function handleOverwrite( // hard-require a rationale (≥ N chars) here so the LLM cannot // sneak a new skill in without justifying why an existing one // could not be edited. - if (target.tier === 'skill' && !existsSync(target.absPath)) { + if ( + target.tier === 'skill' && + target.skillPaths && + !existsSync(target.skillPaths.absPath) + ) { const r = (args.rationale ?? '').trim(); if (r.length < SKILL_CREATE_RATIONALE_MIN) { return reject( - target.absPath, + target.document.target, `skill create rejected: provide a "rationale" (>= ${SKILL_CREATE_RATIONALE_MIN} chars) explaining why no existing skill can be updated`, ); } @@ -190,9 +207,9 @@ async function handleOverwrite( // exists even before the writer's mkdirp runs, so a malformed // resolveUserSkillPath result surfaces as a clear error here // rather than mid-write. - if (!target.parentDir.startsWith(userSkillsDir())) { + if (!target.skillPaths.parentDir.startsWith(userSkillsDir())) { return reject( - target.absPath, + target.document.target, 'skill target escapes the user skills sandbox', ); } @@ -200,8 +217,7 @@ async function handleOverwrite( const result = await overwriteMemoryFile({ tier: target.tier, - absPath: target.absPath, - parentDir: target.parentDir, + document: target.document, skillId: target.skillId, body: args.body, }); @@ -213,15 +229,20 @@ async function handleReplaceString( target: ResolvedTarget, ): Promise { if (typeof args.oldString !== 'string') { - return reject(target.absPath, 'mode="replace_string" requires "oldString"'); + return reject( + target.document.target, + 'mode="replace_string" requires "oldString"', + ); } if (typeof args.newString !== 'string') { - return reject(target.absPath, 'mode="replace_string" requires "newString"'); + return reject( + target.document.target, + 'mode="replace_string" requires "newString"', + ); } const result = await replaceStringInMemoryFile({ tier: target.tier, - absPath: target.absPath, - parentDir: target.parentDir, + document: target.document, skillId: target.skillId, oldString: args.oldString, newString: args.newString, diff --git a/apps/server/src/modules/agent/tools/handlers/image-generation.ts b/apps/server/src/modules/agent/tools/handlers/image-generation.ts index 96691375..4655bee5 100644 --- a/apps/server/src/modules/agent/tools/handlers/image-generation.ts +++ b/apps/server/src/modules/agent/tools/handlers/image-generation.ts @@ -131,7 +131,7 @@ export async function handleGenerateImage( // ── Load reference artifacts upfront ────────────────────────────────── // Any missing/invalid ref is an early hard error — better than sending // a partial set to Azure and getting cryptic results. - const blobs = space(args.canvasId).blobs; + const artifacts = space(args.canvasId).artifacts; const refImages: Array<{ key: string; bytes: Buffer }> = []; for (const key of refs) { if (typeof key !== 'string' || !key.trim()) { @@ -139,7 +139,7 @@ export async function handleGenerateImage( `Invalid reference artifact key: ${JSON.stringify(key)}. Use the bare \`src\` string returned by snapshot_nodes.`, ); } - const bytes = await blobs.read(key); + const bytes = await artifacts.read(key); if (!bytes) { throw new Error( `Reference artifact "${key}" not found on canvas ${args.canvasId}. It may have been deleted.`, @@ -272,7 +272,7 @@ export async function handleGenerateImage( // `canvas_commands` insert or embeds them in a note body — from // user-uploaded artifacts that should never be auto-collected. const name = `${createId('gen')}.png`; - await blobs.put(name, png); + await artifacts.put(name, png); // The requested size string ("auto" included) drives what we // report back; gpt-image-* generally honours the request size, and diff --git a/apps/server/src/modules/artifact/artifact.route.test.ts b/apps/server/src/modules/artifact/artifact.route.test.ts index 1bd48e18..fee9b55b 100644 --- a/apps/server/src/modules/artifact/artifact.route.test.ts +++ b/apps/server/src/modules/artifact/artifact.route.test.ts @@ -86,9 +86,8 @@ function installDeleteBlock(canvasId: string): { init: () => current.blobs.init(), health: () => current.blobs.health(), close: () => current.blobs.close(), - scope(ref) { - const delegate = current.blobs.scope(ref); - return { + space(id) { + const observe = (delegate: BlobScope): BlobScope => ({ put(name, body) { putCalls += 1; return delegate.put(name, body); @@ -100,12 +99,19 @@ function installDeleteBlock(canvasId: string): { list: () => delegate.list(), materialize: (name) => delegate.materialize(name), async deleteAll() { - if (ref.canvasId === canvasId) { + if (id === canvasId) { started.resolve(); await release.promise; } await delegate.deleteAll(); }, + }); + const areas = current.blobs.space(id); + return { + artifacts: observe(areas.artifacts), + guide: observe(areas.guide), + memory: observe(areas.memory), + uploads: observe(areas.uploads), }; }, }; @@ -194,7 +200,7 @@ describe('artifact route', () => { }); expect(upload.statusCode).toBe(500); - expect(await space('missing').blobs.list()).toEqual([]); + expect(await space('missing').artifacts.list()).toEqual([]); await app.close(); }); @@ -234,7 +240,7 @@ describe('artifact route', () => { const upload = await uploading; expect(upload.statusCode).toBe(500); expect(blocker.putCalls()).toBe(0); - expect(await space('c1').blobs.list()).toEqual([]); + expect(await space('c1').artifacts.list()).toEqual([]); } finally { blocker.releaseDelete(); blocker.restore(); @@ -244,7 +250,7 @@ describe('artifact route', () => { it('serves a byte range so media nodes can seek', async () => { const app = await buildApp(); - await space('c1').blobs.put('a.png', png); + await space('c1').artifacts.put('a.png', png); const res = await app.inject({ method: 'GET', @@ -260,7 +266,7 @@ describe('artifact route', () => { it('answers 304 for an unchanged artifact', async () => { const app = await buildApp(); - await space('c1').blobs.put('a.png', png); + await space('c1').artifacts.put('a.png', png); const first = await app.inject({ method: 'GET', @@ -295,12 +301,18 @@ describe('artifact route', () => { return { ok: false, kind: 'disk' }; }, async close() {}, - scope() { - return { + space() { + const unavailable = { async head() { throw new Error('blob backend unavailable'); }, } as unknown as BlobScope; + return { + artifacts: unavailable, + guide: unavailable, + memory: unavailable, + uploads: unavailable, + }; }, }; const restore = setStorageForTesting({ ...current, blobs: failingBlobs }); @@ -332,7 +344,7 @@ describe('artifact route', () => { it('clones an artifact into another canvas under a fresh key', async () => { const app = await buildApp(); - await space('src-canvas').blobs.put('a.png', png); + await space('src-canvas').artifacts.put('a.png', png); const res = await app.inject({ method: 'POST', @@ -346,8 +358,8 @@ describe('artifact route', () => { expect(uri).toMatch(/\.png$/); // Destination owns its own copy; the source is untouched. - expect(await space('dst-canvas').blobs.read(uri)).toEqual(png); - expect(await space('src-canvas').blobs.read('a.png')).toEqual(png); + expect(await space('dst-canvas').artifacts.read(uri)).toEqual(png); + expect(await space('src-canvas').artifacts.read('a.png')).toEqual(png); await app.close(); }); diff --git a/apps/server/src/modules/artifact/artifact.route.ts b/apps/server/src/modules/artifact/artifact.route.ts index 1d9a8f70..c544480e 100644 --- a/apps/server/src/modules/artifact/artifact.route.ts +++ b/apps/server/src/modules/artifact/artifact.route.ts @@ -61,7 +61,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { const name = `${id}${ext}`; try { - await space(canvasId).blobs.put(name, data.file); + await space(canvasId).artifacts.put(name, data.file); } catch (error) { request.log.error({ err: error }, 'Failed to stream artifact to storage'); return reply.code(500).send({ message: 'Failed to save file' }); @@ -82,7 +82,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { '/:canvasId/artifact/:filename', async (request, reply) => { const { canvasId, filename } = request.params; - const blobs = space(canvasId).blobs; + const artifacts = space(canvasId).artifacts; const safeName = path.basename(filename); // `.mhtml` snapshots are stored as proper multipart/related MHTML @@ -91,7 +91,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { // we strip the wrapper on the fly and serve the inner HTML as // `text/html` so no browser-side MHTML handler is required. if (safeName.toLowerCase().endsWith('.mhtml')) { - const buffer = await blobs.read(safeName); + const buffer = await artifacts.read(safeName); if (!buffer) { return reply.code(404).send({ message: 'Artifact not found' }); } @@ -115,7 +115,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { ); } - const served = await sendBlob(request, reply, blobs, safeName); + const served = await sendBlob(request, reply, artifacts, safeName); if (!served) { return reply.code(404).send({ message: 'Artifact not found' }); } @@ -151,7 +151,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { let buffer: Buffer | null; try { - buffer = await space(srcCanvasId).blobs.read(srcKey); + buffer = await space(srcCanvasId).artifacts.read(srcKey); } catch (err) { request.log.error({ err }, 'Failed to read source artifact for clone'); return reply @@ -167,7 +167,7 @@ const artifactRoute: FastifyPluginAsync = async (fastify) => { const name = `${id}${ext}`; try { - await space(dstCanvasId).blobs.put(name, buffer); + await space(dstCanvasId).artifacts.put(name, buffer); } catch (err) { request.log.error({ err }, 'Failed to clone artifact'); return reply diff --git a/apps/server/src/modules/artifact/utils.ts b/apps/server/src/modules/artifact/utils.ts index e27b69aa..d5962c38 100644 --- a/apps/server/src/modules/artifact/utils.ts +++ b/apps/server/src/modules/artifact/utils.ts @@ -56,7 +56,7 @@ export async function resolveArtifactImageUrl( if (!canvasId || !filename) return url; try { - const buffer = await space(canvasId).blobs.read(filename); + const buffer = await space(canvasId).artifacts.read(filename); if (!buffer) return url; const ext = path.extname(filename).toLowerCase(); // Never guess `image/png` for an unknown extension: callers forward this 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 bb0b55b5..80ab3923 100644 --- a/apps/server/src/modules/canvas/canvas-content-cas.test.ts +++ b/apps/server/src/modules/canvas/canvas-content-cas.test.ts @@ -531,7 +531,12 @@ describe('artifact presence hydration', () => { return { ok: true, kind: 'disk' }; }, async close() {}, - scope: () => scope, + space: () => ({ + artifacts: scope, + guide: scope, + memory: scope, + uploads: scope, + }), }; } diff --git a/apps/server/src/modules/canvas/canvas-executor.test.ts b/apps/server/src/modules/canvas/canvas-executor.test.ts index 67bd529f..cf54e726 100644 --- a/apps/server/src/modules/canvas/canvas-executor.test.ts +++ b/apps/server/src/modules/canvas/canvas-executor.test.ts @@ -273,7 +273,7 @@ describe('executeOnServer — MERGE_NODE_DATA CAS', () => { src: 'old.svg', content: '', }); - await space('c1').blobs.put( + await space('c1').artifacts.put( 'new.svg', Buffer.from( '', @@ -329,7 +329,7 @@ describe('executeOnServer — MERGE_NODE_DATA CAS', () => { src: 'pic.svg', content: '', }); - await space('c1').blobs.put( + await space('c1').artifacts.put( 'pic.svg', Buffer.from( '', diff --git a/apps/server/src/modules/canvas/canvas-executor.ts b/apps/server/src/modules/canvas/canvas-executor.ts index 97b5eadc..a8a9b47b 100644 --- a/apps/server/src/modules/canvas/canvas-executor.ts +++ b/apps/server/src/modules/canvas/canvas-executor.ts @@ -368,7 +368,7 @@ async function aspectHeightForWidth( width: number, ): Promise { try { - const dim = await readImageDimensions(space(canvasId).blobs, src); + const dim = await readImageDimensions(space(canvasId).artifacts, src); if (!dim?.width || !dim?.height || dim.width <= 0 || dim.height <= 0) { return null; } diff --git a/apps/server/src/modules/canvas/canvas.route.test.ts b/apps/server/src/modules/canvas/canvas.route.test.ts index c34a7a81..6e5e6855 100644 --- a/apps/server/src/modules/canvas/canvas.route.test.ts +++ b/apps/server/src/modules/canvas/canvas.route.test.ts @@ -738,7 +738,7 @@ describe('Space export/import persistence', () => { change, ]); const blob = Buffer.from([0, 1, 2, 3, 255]); - await space('c1').blobs.put('asset.bin', blob); + await space('c1').artifacts.put('asset.bin', blob); const app = await buildApp(); try { @@ -790,7 +790,7 @@ describe('Space export/import persistence', () => { expect(await importedSpace.changes.read('thread-export')).toEqual( storedChanges, ); - expect(await space(importedId).blobs.read('asset.bin')).toEqual(blob); + expect(await space(importedId).artifacts.read('asset.bin')).toEqual(blob); } finally { await app.close(); } diff --git a/apps/server/src/modules/canvas/canvas.route.ts b/apps/server/src/modules/canvas/canvas.route.ts index 4dd1b132..822f810f 100644 --- a/apps/server/src/modules/canvas/canvas.route.ts +++ b/apps/server/src/modules/canvas/canvas.route.ts @@ -56,6 +56,7 @@ import { createSpace, deleteSpace, stageSpaceImport, + unavailableCapabilityMessage, getStructuredStore, type CanvasFile, type NodeContent, @@ -334,7 +335,7 @@ async function singleArtifactProbe( ): Promise<(key: string) => boolean> { const key = extractArtifactKey(src); if (!key) return () => false; - const exists = (await space(canvasId).blobs.hasMany([key])).has(key); + const exists = (await space(canvasId).artifacts.hasMany([key])).has(key); return (candidate) => candidate === key && exists; } @@ -517,7 +518,7 @@ async function hydrateNodeContent( const present = referenced.size === 0 ? new Set() - : await handle.blobs.hasMany([...referenced]); + : await handle.artifacts.hasMany([...referenced]); const artifactExists = (key: string): boolean => present.has(key); return nodes.map((node) => { @@ -1613,9 +1614,9 @@ const canvasRoutes: FastifyPluginAsync = async (fastify) => { return reply.code(404).send({ message: 'Canvas not found' }); } - // The Space bundle is a Disk projection (proposal §6.4.3, disposition - // A); a portable export generated from records plus reachable blob - // references is a separate later design. + // Disk-only, declared as `space-bundle-export` in the capability matrix; + // a portable export generated from records plus reachable blob references + // is a separate later design. const tree = handle.diskTree; const canvasDir = tree?.directory(); if (canvasDir === undefined || !existsSync(canvasDir)) { @@ -1684,11 +1685,8 @@ const canvasRoutes: FastifyPluginAsync = async (fastify) => { // format and nothing else (proposal §12.6.2). const staged = stageSpaceImport(targetCanvasId); if (!staged) { - // Phrased here only until the capability matrix owns the wording - // (§12.8), so a Disk-only refusal reads the same everywhere. return reply.code(400).send({ - message: - 'Space bundle import is not available on this storage backend.', + message: unavailableCapabilityMessage('space-bundle-import'), }); } const stagingDir = staged.stagingDirectory; diff --git a/apps/server/src/modules/canvas/external.route.ts b/apps/server/src/modules/canvas/external.route.ts index 6460a9ad..e5fc9e18 100644 --- a/apps/server/src/modules/canvas/external.route.ts +++ b/apps/server/src/modules/canvas/external.route.ts @@ -95,8 +95,8 @@ const externalRoutes: FastifyPluginAsync = async (fastify): Promise => { return reply.code(404).send({ message: 'External note not found' }); } - // External-note claim is Disk-only (proposal §6.4.3, disposition A): it - // exists to adopt documents that arrived without going through the + // Disk-only, declared as `external-note-discovery` in the capability + // matrix: it adopts documents that arrived without going through the // application, and no database backend has such an arrival path. const tree = space(canvasId).diskTree; if (!tree) { 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 5dc22e47..0c0fcd1b 100644 --- a/apps/server/src/modules/canvas/import-node-src.test.ts +++ b/apps/server/src/modules/canvas/import-node-src.test.ts @@ -139,7 +139,7 @@ describe('importForeignNodeSources — web nodes', () => { // …whose file exists in the artifact store… expect(src).toBeDefined(); if (src === undefined) throw new Error('Expected a rewritten web src'); - expect(await space(canvasId).blobs.head(src)).not.toBeNull(); + expect(await space(canvasId).artifacts.head(src)).not.toBeNull(); // …and the staging upload was reclaimed (move semantics). expect(existsSync(uploadAbs)).toBe(false); }); @@ -231,7 +231,7 @@ describe('importForeignNodeSources — web nodes', () => { expect(src).toMatch(/^artifact-[^/]+\.html$/); expect(src).toBeDefined(); if (src === undefined) throw new Error('Expected a rewritten web src'); - expect(await space(canvasId).blobs.head(src)).not.toBeNull(); + expect(await space(canvasId).artifacts.head(src)).not.toBeNull(); expect(existsSync(uploadAbs)).toBe(false); }); @@ -309,7 +309,7 @@ describe('importForeignNodeSources — media nodes (regression)', () => { expect(src).toMatch(/^artifact-[^/]+\.png$/); expect(src).toBeDefined(); if (src === undefined) throw new Error('Expected a rewritten image src'); - expect(await space(canvasId).blobs.head(src)).not.toBeNull(); + expect(await space(canvasId).artifacts.head(src)).not.toBeNull(); }); it('canonicalizes an artifact path that leaves and re-enters the Space', async () => { diff --git a/apps/server/src/modules/canvas/import-node-src.ts b/apps/server/src/modules/canvas/import-node-src.ts index b51faeba..8668ee86 100644 --- a/apps/server/src/modules/canvas/import-node-src.ts +++ b/apps/server/src/modules/canvas/import-node-src.ts @@ -307,7 +307,7 @@ async function copyToArtifact( const id = createId('artifact'); const key = `${id}${ext}`; const buffer = await readFile(absPath); - await space(canvasId).blobs.put(key, buffer); + await space(canvasId).artifacts.put(key, buffer); // Move semantics: reclaim RFS scratch uploads once they are safely // stored. Never delete user node files or other canvas content — @@ -363,7 +363,7 @@ async function downloadToArtifact( } const ext = pickDownloadExt(pathname, contentType); const key = `${createId('artifact')}${ext}`; - await space(canvasId).blobs.put(key, buffer); + await space(canvasId).artifacts.put(key, buffer); return key; } catch (err) { log.warn({ err, url }, 'Failed to download online node src into artifacts'); diff --git a/apps/server/src/modules/canvas/snapshot-nodes.ts b/apps/server/src/modules/canvas/snapshot-nodes.ts index faf621b6..92b7b333 100644 --- a/apps/server/src/modules/canvas/snapshot-nodes.ts +++ b/apps/server/src/modules/canvas/snapshot-nodes.ts @@ -451,7 +451,7 @@ async function loadContextImage( if (!mimeType) return null; const { width, height } = nodeBoxSize(node); if (width <= 0 || height <= 0) return null; - const bytes = await handle.blobs.read(src); + const bytes = await handle.artifacts.read(src); if (!bytes) return null; return { node, resolvedSrc: src, bytes, mimeType, width, height }; } @@ -800,11 +800,11 @@ async function maybeResizeImageArtifact( src: string, maxEdge: number, ): Promise<{ src: string; width: number; height: number } | null> { - const blobs = handle.blobs; + const artifacts = handle.artifacts; const ext = path.extname(src).toLowerCase(); const mimeType = IMAGE_EXT_MIME[ext]; if (!mimeType) return null; - const bytes = await blobs.read(src); + const bytes = await artifacts.read(src); if (!bytes) return null; const dims = readImageDimensions(bytes, mimeType); if (!dims) return null; @@ -816,7 +816,7 @@ async function maybeResizeImageArtifact( const originalStem = path.basename(src, path.extname(src)); const id = `${originalStem}-resized-${maxEdge}`; const filename = `${id}.png`; - const cachedBytes = await blobs.read(filename); + const cachedBytes = await artifacts.read(filename); if (cachedBytes) { // Re-derive dimensions from the cached blob so the result is // accurate without paying for another resvg pass. @@ -837,7 +837,7 @@ async function maybeResizeImageArtifact( dims.height, maxEdge, ); - await blobs.put(filename, resized.png); + await artifacts.put(filename, resized.png); return { src: filename, width: resized.width, height: resized.height }; } @@ -1061,10 +1061,10 @@ export async function snapshotNodesToArtifacts( ? `sketch-raster-${fingerprint}` : `sketch-raster-${fingerprint}-${maxEdge}`; const filename = `${id}.png`; - const existing = await handle.blobs.head(filename); + const existing = await handle.artifacts.head(filename); if (!existing) { const png = await renderClusterPng(built.svg, built.width); - await handle.blobs.put(filename, png); + await handle.artifacts.put(filename, png); } results.push({ src: filename, diff --git a/apps/server/src/modules/interactive-view/interactive-view.route.ts b/apps/server/src/modules/interactive-view/interactive-view.route.ts index 470daaf7..143b8975 100644 --- a/apps/server/src/modules/interactive-view/interactive-view.route.ts +++ b/apps/server/src/modules/interactive-view/interactive-view.route.ts @@ -58,7 +58,7 @@ const interactiveViewRoutes: FastifyPluginAsync = async (app) => { }); } try { - const blobs = space(params.data.canvasId).blobs; + const artifacts = space(params.data.canvasId).artifacts; const resource = await interactiveViewService.get( params.data.canvasId, params.data.nodeId, @@ -85,7 +85,7 @@ const interactiveViewRoutes: FastifyPluginAsync = async (app) => { const sent = await sendBlob( request, reply, - blobs, + artifacts, resource.rendererArtifact, ); if (!sent) { diff --git a/apps/server/src/modules/interactive-view/interactive-view.service.ts b/apps/server/src/modules/interactive-view/interactive-view.service.ts index cadedc5e..917f64e8 100644 --- a/apps/server/src/modules/interactive-view/interactive-view.service.ts +++ b/apps/server/src/modules/interactive-view/interactive-view.service.ts @@ -363,7 +363,7 @@ export class InteractiveViewService { : null; const rendererExists = request.rendererArtifact.startsWith('upload/') ? stagedPath !== null && existsSync(stagedPath) - : Boolean(await space(canvasId).blobs.head(request.rendererArtifact)); + : Boolean(await space(canvasId).artifacts.head(request.rendererArtifact)); if (!rendererExists) { throw new InteractiveViewServiceError( 'renderer_not_found', diff --git a/apps/server/src/modules/preprocessing/dispatcher.ts b/apps/server/src/modules/preprocessing/dispatcher.ts index 1f1652bb..6e00db2e 100644 --- a/apps/server/src/modules/preprocessing/dispatcher.ts +++ b/apps/server/src/modules/preprocessing/dispatcher.ts @@ -166,7 +166,7 @@ export class PreprocessDispatcher { const deps: PipelineDeps = { nodes: getStructuredStore().space(request.canvasId).nodes, - blobs: space(request.canvasId).blobs, + artifacts: space(request.canvasId).artifacts, provider: this.provider, }; diff --git a/apps/server/src/modules/preprocessing/pipeline.test.ts b/apps/server/src/modules/preprocessing/pipeline.test.ts index 77f2d7a6..e81ead72 100644 --- a/apps/server/src/modules/preprocessing/pipeline.test.ts +++ b/apps/server/src/modules/preprocessing/pipeline.test.ts @@ -35,7 +35,7 @@ function deps(release: () => Promise) { canvasId: request.canvasId, read: async () => null, } as unknown as SpaceNodes, - blobs: { materialize, put } as unknown as BlobScope, + artifacts: { materialize, put } as unknown as BlobScope, provider: {} as ProviderManager, }, }; diff --git a/apps/server/src/modules/preprocessing/pipeline.ts b/apps/server/src/modules/preprocessing/pipeline.ts index e312dcc2..ff9c09fd 100644 --- a/apps/server/src/modules/preprocessing/pipeline.ts +++ b/apps/server/src/modules/preprocessing/pipeline.ts @@ -39,7 +39,7 @@ const log = getLogger('preprocessing.pipeline'); /** Dependencies injected into the pipeline runner. */ export interface PipelineDeps { nodes: SpaceNodes; - blobs: BlobScope; + artifacts: BlobScope; provider: ProviderManager; } @@ -106,7 +106,7 @@ async function runPipelineStages( // other blob reader takes bytes. const artifactName = ctx.resolved.artifactName; if (artifactName) { - const lease = await deps.blobs.materialize(artifactName); + const lease = await deps.artifacts.materialize(artifactName); if (lease) { leases.push(lease); ctx.resolved.filePath = lease.path; @@ -176,7 +176,10 @@ async function runPipelineStages( ) { try { const artifactName = `${createId('artifact')}.pdf`; - const info = await deps.blobs.put(artifactName, ctx.extracted.rawPdf); + const info = await deps.artifacts.put( + artifactName, + ctx.extracted.rawPdf, + ); ctx.resolved.artifactUri = info.name; ctx.resolved.artifactName = info.name; } catch (snapshotError) { @@ -213,7 +216,7 @@ async function runPipelineStages( ? ctx.extracted.title : ctx.resolved.normalizedUri, ); - await deps.blobs.put(artifactName, buffer); + await deps.artifacts.put(artifactName, buffer); // Inject the artifact key into metadata so the Normalize → // Persist chain writes it as a top-level YAML field on the // node sidecar. The web route reads `mhtmlArtifact` directly 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 3f41c9b0..55c26b3d 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 @@ -216,7 +216,7 @@ describe('Interactive View RFS resources', () => { code: 'renderer_not_found', }); - await space('c1').blobs.put('view.html', Buffer.from('

view

')); + await space('c1').artifacts.put('view.html', Buffer.from('

view

')); const invalidState = await app.inject({ method: 'POST', url: '/rfs/c1/interactive-views', diff --git a/apps/server/src/modules/remote_fs/rfs.route.ts b/apps/server/src/modules/remote_fs/rfs.route.ts index 8e229389..d72d3846 100644 --- a/apps/server/src/modules/remote_fs/rfs.route.ts +++ b/apps/server/src/modules/remote_fs/rfs.route.ts @@ -270,7 +270,7 @@ const rfsRoutes: FastifyPluginAsync = async (app) => { const { canvasId } = request.params; try { const guide = request.headers.authorization - ? resolveCanvasSkill(canvasId) + ? await resolveCanvasSkill(canvasId) : resolveBundledRootSkill(); return reply .header('Content-Type', 'text/markdown; charset=utf-8') diff --git a/apps/server/src/modules/remote_fs/skill.ts b/apps/server/src/modules/remote_fs/skill.ts index 213dd690..7de40554 100644 --- a/apps/server/src/modules/remote_fs/skill.ts +++ b/apps/server/src/modules/remote_fs/skill.ts @@ -11,11 +11,8 @@ * there is no pushed copy to drift from (see the proposal §6c). */ -import { existsSync, readFileSync } from 'node:fs'; -import path from 'node:path'; - import { renderPromptFile } from '../../prompt/agents/loader.js'; -import { space } from '../storage/index.js'; +import { space, SPACE_GUIDE_SKILL_NAME } from '../storage/index.js'; /** PROMPT-ROOT-relative path of the bundled access guide. */ const ACCESS_GUIDE_TEMPLATE = 'external-agent/access-huabu.md'; @@ -39,19 +36,15 @@ export function resolveBundledRootSkill(): string { * override when it exists, otherwise the bundled default. Returned as raw * markdown text (served with `Content-Type: text/markdown`). */ -export function resolveCanvasSkill(canvasId: string): string { - // A user-authored override read from the Space root. Disposition D - // (proposal §6.4.3): it becomes a blob under its own scope kind, at which - // point this reads through the port and the branch goes away. Until then a - // backend without a directory simply has no override to find. - const tree = space(canvasId).diskTree; - if (tree) { - const override = path.join(tree.directory(), 'skill.md'); - if (existsSync(override)) { - return readFileSync(override, 'utf8'); - } - } - return resolveBundledRootSkill(); +export async function resolveCanvasSkill(canvasId: string): Promise { + // A user-authored override, read as a blob under the Space's guide scope + // (proposal §6.4.3, disposition D). The scope is the Space root bounded to + // the guide names, so the file a user authors is exactly where they left it + // and this no longer assembles a path. + const override = await space(canvasId).guide.read(SPACE_GUIDE_SKILL_NAME); + return override === null + ? resolveBundledRootSkill() + : override.toString('utf8'); } /** Resolve one fixed, authenticated advanced guide. */ diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.rename-retry.test.ts b/apps/server/src/modules/storage/backends/disk/blob-store.rename-retry.test.ts index 1d5fbefa..142e0806 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.rename-retry.test.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.rename-retry.test.ts @@ -38,10 +38,7 @@ describe('DiskBlobStore retry cleanup', () => { testState.renameAsync.mockRejectedValue(error); try { - const scope = new DiskBlobStore().scope({ - kind: 'canvas', - canvasId: 'canvas-under-test', - }); + const scope = new DiskBlobStore().space('canvas-under-test').artifacts; await expect(scope.put('blocked.bin', Buffer.from('bytes'))).rejects.toBe( error, diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts index 7d287192..ba731770 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts @@ -28,7 +28,7 @@ describeBlobStoreContract('DiskBlobStore', () => { workspaceState.path = root; return { store: new DiskBlobStore(), - ref: { kind: 'canvas', canvasId: 'canvas-under-test' }, + canvasId: 'canvas-under-test', cleanup: () => rmSync(root, { recursive: true, force: true }), }; }); @@ -54,7 +54,7 @@ describe('DiskBlobStore temp file hygiene', () => { }); it('cleans up after both successful and failed writes', async () => { - const scope = new DiskBlobStore().scope({ kind: 'canvas', canvasId }); + const scope = new DiskBlobStore().space(canvasId).artifacts; await scope.put('kept.bin', Buffer.from('fine')); await scope.put('streamed.bin', Readable.from([Buffer.from('also fine')])); @@ -77,10 +77,7 @@ describe('DiskBlobStore temp file hygiene', () => { }); it('cleans up siblings from concurrent writers to one key', async () => { - const scope = new DiskBlobStore().scope({ - kind: 'canvas', - canvasId: 'concurrent-canvas', - }); + const scope = new DiskBlobStore().space('concurrent-canvas').artifacts; await Promise.all( Array.from({ length: 8 }, (_, i) => @@ -95,7 +92,7 @@ describe('DiskBlobStore temp file hygiene', () => { it('binds in-flight paths to their original workspace and rejects a held scope after activation', async () => { const otherRoot = mkdtempSync(path.join(tmpdir(), 'huabu-blob-switched-')); - const scope = new DiskBlobStore().scope({ kind: 'canvas', canvasId }); + const scope = new DiskBlobStore().space(canvasId).artifacts; let signalStarted = (): void => {}; const started = new Promise((resolve) => { signalStarted = resolve; 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 88dd8eb6..9404f22a 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -4,9 +4,9 @@ /** * Disk implementation of the blob port. * - * Maps a canvas scope to `/.artifacts/`, preserving the layout - * the workspace format has always used: one file per blob, named by the - * URL key, no manifest indirection. + * Maps each area of a Space to a directory under its Space folder, preserving + * the layout the workspace format has always used: one file per blob, named by + * the URL key, no manifest indirection. * * Each scope is bound to the workspace active when it is created. A fresh * scope follows a free-mode workspace switch; a retained scope rejects the @@ -26,10 +26,20 @@ import { import path from 'node:path'; import { pipeline } from 'node:stream/promises'; -import { artifactsDir } from './layout.js'; +import { + artifactsDir, + canvasRoot, + spaceMemoryDir, + spaceUploadDir, +} from './layout.js'; import { renameOverWithRetry } from '../../../../utils/fs.js'; import { getWorkspacePath } from '../../../workspace.js'; -import { createBlobLease, normalizeBlobName } from '../../ports/blob.js'; +import { + BlobNameError, + createBlobLease, + normalizeBlobName, + SPACE_GUIDE_BLOB_NAMES, +} from '../../ports/blob.js'; import type { BlobInfo, @@ -37,8 +47,8 @@ import type { BlobRange, BlobRead, BlobScope, - BlobScopeRef, BlobStore, + SpaceBlobs, } from '../../ports/blob.js'; import type { StorageHealth } from '../../ports/common.js'; import type { Readable } from 'node:stream'; @@ -56,9 +66,37 @@ function isTempEntry(entry: string): boolean { return entry.startsWith(TEMP_PREFIX); } -/** Resolve a scope to its backing directory. */ -function scopeDir(ref: BlobScopeRef): string { - return artifactsDir(ref.canvasId); +/** The areas of a Space, as this adapter places them. */ +type SpaceBlobArea = keyof SpaceBlobs; + +/** + * Where one area's bytes sit, and which names it owns there. + * + * `members: null` means the directory *is* the area — everything in it + * belongs. A name list means the area is bounded by its members instead, for + * a directory shared with files that are not blobs at all. + */ +interface ScopePlacement { + readonly directory: string; + readonly members: readonly string[] | null; +} + +function scopePlacement(area: SpaceBlobArea, canvasId: string): ScopePlacement { + switch (area) { + case 'artifacts': + return { directory: artifactsDir(canvasId), members: null }; + case 'memory': + return { directory: spaceMemoryDir(canvasId), members: null }; + case 'uploads': + return { directory: spaceUploadDir(canvasId), members: null }; + case 'guide': + // The Space root, which also holds `space.json` and every node + // directory — so this area is the guide names, not the folder. + return { + directory: canvasRoot(canvasId), + members: SPACE_GUIDE_BLOB_NAMES, + }; + } } /** Resolve one blob beneath an already-bound scope directory. */ @@ -72,26 +110,62 @@ function isMissing(err: unknown): boolean { } class DiskBlobScope implements BlobScope { - readonly #ref: BlobScopeRef; + readonly #area: SpaceBlobArea; + readonly #canvasId: string; readonly #workspacePath: string; - constructor(ref: BlobScopeRef) { - this.#ref = ref; + constructor(area: SpaceBlobArea, canvasId: string) { + this.#area = area; + this.#canvasId = canvasId; this.#workspacePath = path.resolve(getWorkspacePath()); } - #resolveDir(): string { + #placement(): ScopePlacement { const active = path.resolve(getWorkspacePath()); if (active !== this.#workspacePath) { throw new Error( - `DiskBlobScope(${this.#ref.canvasId}) belongs to an inactive workspace. ` + + `DiskBlobScope(${this.#canvasId}) belongs to an inactive workspace. ` + `Resolve a fresh scope after workspace activation.`, ); } // Resolve once per operation, before its first await. Every later path in // that operation is derived from this absolute directory, so a workspace // switch cannot combine a temp in A with a destination in B. - return scopeDir(this.#ref); + return scopePlacement(this.#area, this.#canvasId); + } + + /** Names this scope owns in `dir`, given what is actually there. */ + async #entries(placement: ScopePlacement): Promise { + if (placement.members) { + // A member-bounded scope never reads the directory: everything else in + // it belongs to someone else, and listing it would claim otherwise. + const present = await Promise.all( + placement.members.map(async (name) => + (await this.#headAt(placement.directory, name)) ? name : null, + ), + ); + return present.filter((name): name is string => name !== null); + } + try { + return (await readdir(placement.directory)).filter( + (entry) => !isTempEntry(entry), + ); + } catch (err) { + if (isMissing(err)) return []; + throw err; + } + } + + /** Refuse a name this scope does not own, before it reaches the filesystem. */ + #assertMember(placement: ScopePlacement, name: string): string { + const safe = normalizeBlobName(name); + if (placement.members && !placement.members.includes(safe)) { + throw new BlobNameError( + `"${safe}" is not a member of the ${this.#area} area. ` + + `It holds: ${placement.members.join(', ')}.`, + ); + } + return safe; } async #headAt(dir: string, name: string): Promise { @@ -122,8 +196,9 @@ class DiskBlobScope implements BlobScope { * no per-key delete to clean up. */ async put(name: string, body: Readable | Buffer): Promise { - const safe = normalizeBlobName(name); - const dir = this.#resolveDir(); + const placement = this.#placement(); + const safe = this.#assertMember(placement, name); + const dir = placement.directory; await mkdir(dir, { recursive: true }); const full = blobPath(dir, safe); @@ -152,12 +227,17 @@ class DiskBlobScope implements BlobScope { } async head(name: string): Promise { - return this.#headAt(this.#resolveDir(), name); + const placement = this.#placement(); + return this.#headAt( + placement.directory, + this.#assertMember(placement, name), + ); } async open(name: string, range?: BlobRange): Promise { - const dir = this.#resolveDir(); - const info = await this.#headAt(dir, name); + const placement = this.#placement(); + const dir = placement.directory; + const info = await this.#headAt(dir, this.#assertMember(placement, name)); if (!info) return null; // `info.size` stays the full blob size; the range only bounds the body. const body = createReadStream(blobPath(dir, info.name), { @@ -168,9 +248,10 @@ class DiskBlobScope implements BlobScope { } async read(name: string): Promise { - const dir = this.#resolveDir(); + const placement = this.#placement(); + const safe = this.#assertMember(placement, name); try { - return await readFile(blobPath(dir, name)); + return await readFile(blobPath(placement.directory, safe)); } catch (err) { if (isMissing(err)) return null; throw err; @@ -178,48 +259,32 @@ class DiskBlobScope implements BlobScope { } async hasMany(names: readonly string[]): Promise> { - const dir = this.#resolveDir(); + const placement = this.#placement(); const requested = new Set(names.map(normalizeBlobName)); if (requested.size === 0) return new Set(); - let entries: string[]; - try { - entries = await readdir(dir); - } catch (err) { - if (isMissing(err)) return new Set(); - throw err; - } - - const candidates = entries.filter( - (entry) => !isTempEntry(entry) && requested.has(entry), + const entries = (await this.#entries(placement)).filter((entry) => + requested.has(entry), ); const infos = await Promise.all( - candidates.map((entry) => this.#headAt(dir, entry)), + entries.map((entry) => this.#headAt(placement.directory, entry)), ); return new Set(infos.flatMap((info) => (info === null ? [] : [info.name]))); } async list(): Promise { - const dir = this.#resolveDir(); - let entries: string[]; - try { - entries = await readdir(dir); - } catch (err) { - if (isMissing(err)) return []; - throw err; - } - + const placement = this.#placement(); + const entries = await this.#entries(placement); const infos = await Promise.all( - entries - .filter((entry) => !isTempEntry(entry)) - .map((entry) => this.#headAt(dir, entry)), + entries.map((entry) => this.#headAt(placement.directory, entry)), ); return infos.filter((info): info is BlobInfo => info !== null); } async materialize(name: string): Promise { - const dir = this.#resolveDir(); - const info = await this.#headAt(dir, name); + const placement = this.#placement(); + const dir = placement.directory; + const info = await this.#headAt(dir, this.#assertMember(placement, name)); if (!info) return null; // Disk already *is* a filesystem: hand back the real path and make // release a no-op. No copy, so this costs nothing today. The lease @@ -229,7 +294,18 @@ class DiskBlobScope implements BlobScope { } async deleteAll(): Promise { - await rm(this.#resolveDir(), { recursive: true, force: true }); + const placement = this.#placement(); + if (!placement.members) { + await rm(placement.directory, { recursive: true, force: true }); + return; + } + // Removing the directory would take the Space with it. Only the members + // are this area's to delete. + await Promise.all( + placement.members.map((name) => + rm(blobPath(placement.directory, name), { force: true }), + ), + ); } } @@ -237,7 +313,7 @@ export class DiskBlobStore implements BlobStore { readonly kind = 'disk' as const; async init(): Promise { - // Scope directories are created on first write; nothing to prepare. + // Area directories are created on first write; nothing to prepare. } async health(): Promise { @@ -246,7 +322,12 @@ export class DiskBlobStore implements BlobStore { async close(): Promise {} - scope(ref: BlobScopeRef): BlobScope { - return new DiskBlobScope(ref); + space(canvasId: string): SpaceBlobs { + return { + artifacts: new DiskBlobScope('artifacts', canvasId), + guide: new DiskBlobScope('guide', canvasId), + memory: new DiskBlobScope('memory', canvasId), + uploads: new DiskBlobScope('uploads', canvasId), + }; } } diff --git a/apps/server/src/modules/storage/backends/disk/layout.ts b/apps/server/src/modules/storage/backends/disk/layout.ts index 35afbdbd..9201c450 100644 --- a/apps/server/src/modules/storage/backends/disk/layout.ts +++ b/apps/server/src/modules/storage/backends/disk/layout.ts @@ -80,6 +80,26 @@ export function artifactsDir(canvasId: string): string { return path.join(canvasRoot(canvasId), ARTIFACTS_DIR_NAME); } +/** + * Hidden directory holding the agent's private memory document. + * + * Named here rather than in the workspace module because it is now a blob + * scope's placement — where Disk puts the bytes of one user-visible area — + * and every other such placement already lives beside this one. + */ +export const MEMORY_DIR_NAME = '.memory'; + +export function spaceMemoryDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), MEMORY_DIR_NAME); +} + +/** Hidden scratch an upload lands in before anything claims it. */ +export const UPLOAD_DIR_NAME = '.upload'; + +export function spaceUploadDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), UPLOAD_DIR_NAME); +} + export function artifactPath(canvasId: string, filename: string): string { const base = path.basename(filename); if (!base || base === '.' || base === '..') { diff --git a/apps/server/src/modules/storage/backends/disk/space-extension.ts b/apps/server/src/modules/storage/backends/disk/space-extension.ts new file mode 100644 index 00000000..588419a1 --- /dev/null +++ b/apps/server/src/modules/storage/backends/disk/space-extension.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Disk implementation of the extension substrate. + * + * One reserved directory per namespace, under a hidden `.ext/` tier inside the + * Space — the same tier as `.artifacts/` and `.history/`, because an + * extension's store is machine state rather than something a user authors. + * + * Destruction is free here, and that is worth stating rather than relying on: + * the namespace lives *inside* the Space directory, so the existing + * `store.destroy()` removes it with everything else. A backend that keys + * extensions by table prefix or schema has no such placement and must drop + * them explicitly — which is why the contract asserts the outcome instead of + * trusting the mechanism. + */ + +import path from 'node:path'; + +import { canvasRoot } from './layout.js'; +import { mkdirp } from '../../../../utils/fs.js'; +import { assertValidNamespace } from '../../ports/namespace.js'; + +import type { CanvasStore } from './legacy/canvas-store.js'; +import type { SpaceHandle, SpaceSubstrate } from '../../ports/structured.js'; + +/** Hidden tier holding one directory per extension namespace. */ +export const EXTENSIONS_DIR_NAME = '.ext'; + +export function createDiskSpaceExtension( + store: CanvasStore, + readRecord: SpaceHandle['read'], +): SpaceHandle['extension'] { + return async function extension( + namespace: string, + ): Promise { + assertValidNamespace(namespace); + // The existence check is the point, not a courtesy. Owners write through + // ordinary filesystem calls, so handing back a path for a Space that was + // just deleted would let the first write recreate its directory as a stub + // holding nothing but bookkeeping. Every owner used to carry its own guard + // against that; this is the one place that can state it. + if ((await readRecord()) === null) return null; + + const directory = path.join( + canvasRoot(store.canvasId), + EXTENSIONS_DIR_NAME, + namespace, + ); + // Created on demand, so an owner receives somewhere it can write rather + // than a path it has to prepare. The window between the check above and + // an owner's own write stays open — a Space deleted inside it is the same + // race the per-owner guards had — but nothing widens it. + mkdirp(directory); + return { kind: 'disk', directory }; + }; +} 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 9f19aff1..250a6102 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 @@ -2,6 +2,7 @@ // Licensed under the MIT license. import { + existsSync, mkdirSync, mkdtempSync, readFileSync, @@ -27,6 +28,7 @@ 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 { describeSpaceExtensionContract } from '../../ports/contracts/space-extension.contract.js'; import { describeSpaceRepositoryContract } from '../../ports/contracts/space-repository.contract.js'; import type { CanvasFile } from '../../../canvas/persistence-types.js'; @@ -129,6 +131,25 @@ describeSpaceRepositoryContract('Disk', () => { }; }); +describeSpaceExtensionContract('Disk', () => { + const root = makeWorkspace('huabu-space-extension-contract-'); + seedWorld(root); + const store = new DiskStructuredStore(); + return { + repository: store.spaces(), + space: (canvasId: string) => store.space(canvasId), + // An owner of a Disk namespace writes files into its directory; nothing + // about the shape is storage's business, so the suite borrows the + // simplest one an owner could pick. + write: (substrate, value) => + writeFileSync(path.join(substrate.directory, 'value'), value, 'utf8'), + read: (substrate) => { + const file = path.join(substrate.directory, 'value'); + return existsSync(file) ? readFileSync(file, 'utf8') : null; + }, + }; +}); + describe('DiskSpaceRepository membership', () => { it('refreshes external additions, deletions, and directory renames', async () => { const root = makeWorkspace('huabu-space-membership-refresh-'); 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 3f02d662..47858cbd 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 @@ -464,7 +464,7 @@ describe('Space lifecycle guards and reopen', () => { handle.changes.append('thread-1', [change('n1')]), ).rejects.toThrow(/missing Space/); await expect( - space('missing-space').blobs.put('x.bin', rejectedBuffer), + space('missing-space').artifacts.put('x.bin', rejectedBuffer), ).rejects.toThrow(/missing Space/); expect(rejectedBuffer.toString()).toBe('x'); @@ -476,7 +476,7 @@ describe('Space lifecycle guards and reopen', () => { const ended = once(body, 'end'); await expect( - space('missing-stream-space').blobs.put('x.bin', body), + space('missing-stream-space').artifacts.put('x.bin', body), ).rejects.toThrow(/missing Space/); await ended; @@ -501,7 +501,10 @@ describe('Space lifecycle guards and reopen', () => { const storedChanges = await first.changes.append('thread-1', [ change('n1'), ]); - await space('reopen').blobs.put('payload.bin', Buffer.from('persisted')); + await space('reopen').artifacts.put( + 'payload.bin', + Buffer.from('persisted'), + ); resetStorageCache(); const reopened = new DiskStructuredStore().space('reopen'); @@ -512,8 +515,8 @@ describe('Space lifecycle guards and reopen', () => { 7, ]); expect(await reopened.changes.read('thread-1')).toEqual(storedChanges); - expect((await space('reopen').blobs.read('payload.bin'))?.toString()).toBe( - 'persisted', - ); + expect( + (await space('reopen').artifacts.read('payload.bin'))?.toString(), + ).toBe('persisted'); }); }); diff --git a/apps/server/src/modules/storage/backends/disk/structured-store.ts b/apps/server/src/modules/storage/backends/disk/structured-store.ts index e02f25d4..af4f133c 100644 --- a/apps/server/src/modules/storage/backends/disk/structured-store.ts +++ b/apps/server/src/modules/storage/backends/disk/structured-store.ts @@ -22,6 +22,7 @@ */ import { getCanvasStore } from './legacy/canvas-store-cache.js'; +import { createDiskSpaceExtension } from './space-extension.js'; import { createDiskSpaceLogs } from './space-logs.js'; import { DiskSpaceNodes } from './space-nodes.js'; import { createDiskSpaceRecordReader } from './space-record.js'; @@ -54,14 +55,18 @@ export class DiskStructuredStore implements StructuredStore { // `getCanvasStore` validates the id and owns the instance cache. const store = getCanvasStore(canvasId); const { events, changes } = createDiskSpaceLogs(store); + const read = createDiskSpaceRecordReader(store); return { canvasId: store.canvasId, - read: createDiskSpaceRecordReader(store), + read, write: createDiskSpaceWrite(store), nodes: new DiskSpaceNodes(store), changes, tasks: new DiskSpaceTasks(store), events, + // Shares the handle's own record read, so the existence check that + // guards a substrate is the same one every other member answers to. + extension: createDiskSpaceExtension(store, read), }; } } diff --git a/apps/server/src/modules/storage/capabilities.test.ts b/apps/server/src/modules/storage/capabilities.test.ts new file mode 100644 index 00000000..527dd8b2 --- /dev/null +++ b/apps/server/src/modules/storage/capabilities.test.ts @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * The capability matrix (proposal §6.4.2, disposition A). + * + * What is worth asserting is not the contents — those change as features do — + * but that the matrix stays a *declaration an operator can act on*: every + * entry names a backend that exists, the Disk profile loses nothing, and an + * unavailable feature is reported rather than raised. + */ + +import { describe, expect, it } from 'vitest'; + +import { + describeUnavailableCapabilities, + hasStorageCapability, + STORAGE_CAPABILITIES, + unavailableCapabilities, +} from './capabilities.js'; +import { validateStorageProfile } from './profile.js'; + +import type { StorageProfile } from './profile.js'; + +const DISK: StorageProfile = { + structured: { kind: 'disk' }, + blobs: { kind: 'disk' }, +}; + +/** + * A profile naming a structured backend that has no adapter. + * + * The matrix has to answer for one before it exists — that is the point of + * declaring rather than discovering — so this stands in for the first backend + * that keeps Spaces in tables. + */ +const TABLES: StorageProfile = { + structured: { kind: 'sqlite' }, + blobs: { kind: 'disk' }, +}; + +describe('storage capability matrix', () => { + it('lists only real, identifiable capabilities', () => { + const ids = STORAGE_CAPABILITIES.map((capability) => capability.id); + expect(new Set(ids).size).toBe(ids.length); + + for (const capability of STORAGE_CAPABILITIES) { + expect(capability.backends.length).toBeGreaterThan(0); + // A capability nothing can serve is not a limitation, it is a removed + // feature; a capability every backend serves does not belong here. + expect(capability.summary).not.toHaveLength(0); + expect(capability.rationale).not.toHaveLength(0); + } + }); + + it('offers every capability on the Disk profile', () => { + expect(unavailableCapabilities(DISK)).toEqual([]); + expect(describeUnavailableCapabilities(DISK)).toEqual([]); + }); + + it('answers for a backend that has no adapter yet', () => { + const missing = unavailableCapabilities(TABLES); + + // Every entry is Disk-only today, so a structured backend that is not + // Disk loses all of them. The assertion is the shape, not the count. + expect(missing).toEqual(STORAGE_CAPABILITIES); + expect(hasStorageCapability(TABLES, 'reveal-space-folder')).toBe(false); + expect(hasStorageCapability(DISK, 'reveal-space-folder')).toBe(true); + }); + + it('treats an unknown id as available rather than guessing', () => { + // The matrix is an exception list. A feature nobody wrote down is + // portable by construction, and inventing a refusal for it would make + // adding a portable feature a matrix edit. + expect(hasStorageCapability(TABLES, 'something-portable')).toBe(true); + }); + + it('states a limitation without making it a misconfiguration', () => { + // A profile that merely offers fewer features must not fail validation — + // that is reserved for a backend that cannot serve at all. `sqlite` has + // no adapter yet, so it does fail; the distinction is which check + // rejects it. + expect(describeUnavailableCapabilities(TABLES).length).toBeGreaterThan(0); + expect(() => validateStorageProfile(TABLES)).toThrow(/not implemented/); + expect(() => validateStorageProfile(DISK)).not.toThrow(); + }); + + it('describes each loss in operator terms', () => { + const lines = describeUnavailableCapabilities(TABLES); + + for (const capability of STORAGE_CAPABILITIES) { + const line = lines.find((entry) => entry.startsWith(`${capability.id}:`)); + expect(line).toBeDefined(); + // The id to search for, what is lost, and why it cannot be emulated. + expect(line).toContain(capability.summary); + expect(line).toContain('sqlite'); + } + }); +}); diff --git a/apps/server/src/modules/storage/capabilities.ts b/apps/server/src/modules/storage/capabilities.ts new file mode 100644 index 00000000..68c41268 --- /dev/null +++ b/apps/server/src/modules/storage/capabilities.ts @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Which product features a storage profile can actually serve. + * + * Some features are *about* a filesystem — showing a folder in Finder, + * adopting a document a user dropped in from outside, a bundle that is a + * directory zipped up. A backend that keeps Spaces in tables has no honest + * answer for them, and the honest outcome is that the feature is + * **unavailable, not emulated** (proposal §6.4.2, disposition A). A + * workaround that makes such a feature *nearly* work is worse than its + * absence: it has to be built, tested, and explained, and it hides the + * limitation instead of stating it. + * + * "Stated" is what this file is for. An outcome of A is an acceptable product + * limitation only if an operator can learn it when they select a profile + * rather than when a user clicks the button — so the matrix sits beside + * profile validation, which is the one place a profile is inspected before + * anything opens. + * + * This is a *declaration*, not an enforcement point. Each listed feature also + * refuses at its own call site, because a matrix nobody consults at runtime is + * documentation. What the matrix adds is the up-front answer. + */ + +import type { StructuredBackendKind } from './ports/structured.js'; +import type { StorageProfile } from './profile.js'; + +/** + * A product feature whose availability depends on the structured backend. + * + * Keyed by structured kind alone: every entry here needs a Space to be a real + * directory, which is a structured-backend property. A feature that turned on + * the blob backend instead would be a second matrix, and there are none. + */ +export interface StorageCapability { + /** Stable id, for a diagnostic an operator can search for. */ + readonly id: string; + /** What a user loses, in their vocabulary rather than the port's. */ + readonly summary: string; + /** Structured backends that serve it. */ + readonly backends: readonly StructuredBackendKind[]; + /** Why it cannot be served elsewhere, and what remains instead. */ + readonly rationale: string; +} + +/** + * Every feature that is not available on every backend. + * + * Deliberately not "every feature" — a matrix that listed the portable ones + * too would need updating whenever anything was built, and would go stale + * silently. What must stay accurate is the exception list. + */ +export const STORAGE_CAPABILITIES: readonly StorageCapability[] = [ + { + id: 'space-bundle-export', + summary: 'Export a Space as a .huabu.zip bundle', + backends: ['disk'], + rationale: + 'The bundle is a Disk projection — the Space directory, archived. A ' + + 'portable export generated from records plus reachable blob references ' + + 'is a separate design.', + }, + { + id: 'space-bundle-import', + summary: 'Import a Space from a .huabu.zip bundle', + backends: ['disk'], + rationale: 'Pairs with export; unzips into place.', + }, + { + id: 'reveal-space-folder', + summary: 'Reveal a Space in the OS file manager', + backends: ['disk'], + rationale: + 'The feature is "show me this in Finder". Without a folder there is ' + + 'nothing to show.', + }, + { + id: 'builtin-file-tools', + summary: 'Built-in agent file tools (read, write, glob, grep)', + backends: ['disk'], + rationale: + 'They sandbox on the Space directory. Off Disk the first-party agent ' + + 'reaches a Space over RFS/HTTP, which is what external agents already ' + + 'use.', + }, + { + id: 'external-note-discovery', + summary: 'Adopt Markdown files dropped into a Space from outside the app', + backends: ['disk'], + rationale: + 'It watches for documents that arrived without going through the ' + + 'application. A database backend has no such arrival path unless ' + + 'someone writes to the store out of band, and inventing one would buy ' + + 'nothing.', + }, + { + id: 'space-directory-handle-coordination', + summary: 'Windows: rename or delete a Space while a watcher holds it open', + backends: ['disk'], + rationale: + 'Exists so a directory rename can succeed against a live `fs.watch` ' + + 'handle. No directory, no problem.', + }, +]; + +/** Capabilities this profile cannot serve. */ +export function unavailableCapabilities( + profile: StorageProfile, +): readonly StorageCapability[] { + return STORAGE_CAPABILITIES.filter( + (capability) => + !(capability.backends as readonly string[]).includes( + profile.structured.kind, + ), + ); +} + +/** Whether this profile serves `id`. Unknown ids are available by omission. */ +export function hasStorageCapability( + profile: StorageProfile, + id: string, +): boolean { + const capability = STORAGE_CAPABILITIES.find((entry) => entry.id === id); + if (!capability) return true; + return (capability.backends as readonly string[]).includes( + profile.structured.kind, + ); +} + +/** + * The refusal a Disk-only feature raises when it finds no Space directory. + * + * Shares its wording with the startup declaration, so the sentence an + * operator read when they chose the profile is the sentence they see in the + * failure. A feature that phrased its own refusal would drift from the + * matrix, and the drift would only show up in a support thread. + */ +export function unavailableCapabilityMessage(id: string): string { + const capability = STORAGE_CAPABILITIES.find((entry) => entry.id === id); + if (!capability) { + return `Storage capability "${id}" is not available on this backend.`; + } + return ( + `${capability.summary} is not available on this storage backend ` + + `(capability "${capability.id}"). ${capability.rationale}` + ); +} + +/** + * One operator-facing line per feature this profile does not offer. + * + * Rendered at startup rather than raised: an unavailable feature is a stated + * limitation, not a misconfiguration, so it must not stop the Server. The + * distinction matters — a profile naming an unimplemented backend *is* a + * misconfiguration and still fails fast in `validateStorageProfile`. + */ +export function describeUnavailableCapabilities( + profile: StorageProfile, +): readonly string[] { + return unavailableCapabilities(profile).map( + (capability) => + `${capability.id}: ${capability.summary} — unavailable on the ` + + `"${profile.structured.kind}" structured backend. ${capability.rationale}`, + ); +} 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 74118247..8467909f 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -20,6 +20,7 @@ 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'; +import { spaceBlobAreas } from '../ports/blob.js'; import { composeStorage, space, @@ -32,8 +33,8 @@ import type { BlobRange, BlobRead, BlobScope, - BlobScopeRef, BlobStore, + SpaceBlobs, } from '../ports/blob.js'; import type { Readable } from 'node:stream'; @@ -56,6 +57,24 @@ vi.mock('../../workspace.js', () => ({ }, })); +/** Every area of one Space, each wrapped the same way. */ +function wrapAreas( + blobs: SpaceBlobs, + wrap: (scope: BlobScope) => BlobScope, +): SpaceBlobs { + return { + artifacts: wrap(blobs.artifacts), + guide: wrap(blobs.guide), + memory: wrap(blobs.memory), + uploads: wrap(blobs.uploads), + }; +} + +/** How many sweeps one Space deletion must perform. */ +const SPACE_AREA_COUNT = spaceBlobAreas( + new DiskBlobStore().space('probe'), +).length; + function writeCanvas(directory: string, canvasId: string, title: string): void { const root = path.join(workspaceState.path, directory); mkdirSync(root, { recursive: true }); @@ -100,10 +119,9 @@ class OrderRecordingBlobStore implements BlobStore { return this.inner.close(); } - scope(ref: BlobScopeRef): BlobScope { - const scope = this.inner.scope(ref); + space(canvasId: string): SpaceBlobs { const seen = this.recordPresentAtSweep; - return { + return wrapAreas(this.inner.space(canvasId), (scope) => ({ put: (name: string, body: Readable | Buffer): Promise => scope.put(name, body), head: (name: string): Promise => scope.head(name), @@ -115,10 +133,10 @@ class OrderRecordingBlobStore implements BlobStore { list: (): Promise => scope.list(), materialize: (name: string) => scope.materialize(name), deleteAll: async (): Promise => { - seen.push(existsSync(canvasJsonPath(ref.canvasId))); + seen.push(existsSync(canvasJsonPath(canvasId))); await scope.deleteAll(); }, - }; + })); } } @@ -163,9 +181,8 @@ class ControllableBlobStore implements BlobStore { return this.#inner.close(); } - scope(ref: BlobScopeRef): BlobScope { - const scope = this.#inner.scope(ref); - return { + space(canvasId: string): SpaceBlobs { + return wrapAreas(this.#inner.space(canvasId), (scope) => ({ put: async (name: string, body: Readable | Buffer): Promise => { this.putCalls += 1; this.putStarted.resolve(); @@ -186,7 +203,7 @@ class ControllableBlobStore implements BlobStore { if (this.blockDeletes) await this.#deletesReleased.promise; await scope.deleteAll(); }, - }; + })); } } @@ -243,7 +260,12 @@ describe('deleteSpace composition', () => { // Blobs first: after the structured record is gone nothing names them, // so a failed sweep on a remote backend would strand them permanently. - expect(blobs.recordPresentAtSweep).toEqual([true]); + // One sweep per user-visible area — a Space's bytes are spread across an + // area each, and an unswept one is an orphan on a backend where dropping + // the record does not remove the place they sit in. + expect(blobs.recordPresentAtSweep).toEqual( + Array.from({ length: SPACE_AREA_COUNT }, () => true), + ); expect(existsSync(artifactPath('canvas-a', 'art_1.png'))).toBe(false); expect(existsSync(canvasJsonPath('canvas-a'))).toBe(false); }); @@ -313,7 +335,7 @@ describe('deleteSpace composition', () => { controlled.blockPuts = true; installBlobStore(controlled); - const putting = space('canvas-a').blobs.put( + const putting = space('canvas-a').artifacts.put( 'in-flight.bin', Buffer.from('bytes'), ); @@ -340,7 +362,7 @@ describe('deleteSpace composition', () => { await putting; await expect(deleting).resolves.toEqual({ ok: true, reason: 'deleted' }); - expect(controlled.deleteCalls).toBe(1); + expect(controlled.deleteCalls).toBe(SPACE_AREA_COUNT); expect(existsSync(canvasJsonPath('canvas-a'))).toBe(false); expect(existsSync(artifactPath('canvas-a', 'in-flight.bin'))).toBe(false); }); @@ -353,7 +375,7 @@ describe('deleteSpace composition', () => { const deleting = deleteSpace('canvas-a'); await controlled.deleteStarted.promise; expect(workspaceState.leaseCount).toBe(1); - const putting = space('canvas-a').blobs.put( + const putting = space('canvas-a').artifacts.put( 'too-late.bin', Buffer.from('orphan'), ); @@ -466,8 +488,14 @@ describe('deleteSpace composition', () => { controlled.blockPuts = true; installBlobStore(controlled); - const first = space('canvas-a').blobs.put('first.bin', Buffer.from('1')); - const second = space('canvas-a').blobs.put('second.bin', Buffer.from('2')); + const first = space('canvas-a').artifacts.put( + 'first.bin', + Buffer.from('1'), + ); + const second = space('canvas-a').artifacts.put( + 'second.bin', + Buffer.from('2'), + ); await vi.waitFor(() => expect(controlled.putCalls).toBe(2)); controlled.releasePuts(); @@ -488,7 +516,7 @@ describe('deleteSpace composition', () => { await controlled.deleteStarted.promise; await expect( - space('canvas-b').blobs.put('independent.bin', Buffer.from('free')), + space('canvas-b').artifacts.put('independent.bin', Buffer.from('free')), ).resolves.toMatchObject({ name: 'independent.bin' }); expect(existsSync(artifactPath('canvas-b', 'independent.bin'))).toBe(true); diff --git a/apps/server/src/modules/storage/index.ts b/apps/server/src/modules/storage/index.ts index a43b0cb2..0a12f64c 100644 --- a/apps/server/src/modules/storage/index.ts +++ b/apps/server/src/modules/storage/index.ts @@ -72,6 +72,7 @@ export type { export { adoptWorkspaceDirectory, + closeStorage, composeStorage, createSpace, createStorage, @@ -97,8 +98,27 @@ export { StorageProfileError, validateStorageProfile, } from './profile.js'; +export { + describeUnavailableCapabilities, + hasStorageCapability, + STORAGE_CAPABILITIES, + unavailableCapabilities, + unavailableCapabilityMessage, +} from './capabilities.js'; +export type { StorageCapability } from './capabilities.js'; export type { StorageProfile } from './profile.js'; -export { BlobNameError, normalizeBlobName } from './ports/blob.js'; +export { + assertValidNamespace, + SpaceNamespaceError, +} from './ports/namespace.js'; +export { + BlobNameError, + normalizeBlobName, + SPACE_GUIDE_BLOB_NAMES, + SPACE_GUIDE_SKILL_NAME, + SPACE_MEMORY_BLOB_NAME, + spaceBlobAreas, +} from './ports/blob.js'; export type { BlobBackendKind, BlobInfo, @@ -106,8 +126,8 @@ export type { BlobRange, BlobRead, BlobScope, - BlobScopeRef, BlobStore, + SpaceBlobs, } from './ports/blob.js'; export type { StorageHealth } from './ports/common.js'; export type { @@ -135,6 +155,7 @@ export type { SpaceRenameInput, SpaceRenameResult, SpaceRepository, + SpaceSubstrate, SpaceTaskRuns, SpaceTasks, SpaceWriteInput, diff --git a/apps/server/src/modules/storage/module-boundaries.test.ts b/apps/server/src/modules/storage/module-boundaries.test.ts index 5f7b90d9..aa9c3ead 100644 --- a/apps/server/src/modules/storage/module-boundaries.test.ts +++ b/apps/server/src/modules/storage/module-boundaries.test.ts @@ -85,13 +85,17 @@ describe('storage module tree', () => { expect(rootFiles.sort()).toEqual([ 'canvas-dirs.ts', + 'capabilities.test.ts', + 'capabilities.ts', 'index.ts', 'module-boundaries.test.ts', 'paths.ts', + 'product-boundary.test.ts', 'profile.test.ts', 'profile.ts', 'space-lifecycle-admission.ts', 'storage.ts', + 'testing.ts', ]); }); @@ -302,14 +306,13 @@ describe('Disk Space tree capability', () => { 'modules/canvas/canvas.route.ts', // A — external-note claim. 'modules/canvas/external.route.ts', - // C — the resurrection guard, which disappears with the substrate. - 'modules/agent/memory/trigger.ts', // B, deferred — RFS's sidecar-to-record mapping. Portable in principle, // Disk's in practice until a second backend has a file plane at all. 'modules/remote_fs/node-meta.ts', - // D — the per-Space RFS access guide, headed for a blob. - 'modules/remote_fs/skill.ts', - // C and D — memory files, the debug prompt log, ACP session state. + // C — ACP session state, which leaves with phase 6's `Namespace` change. + // Everything else this module addressed has already left: the memory + // bookkeeping and debug prompt log onto the extension substrate, the + // memory body and the RFS access guide into blob scopes. 'modules/workspace/paths.ts', ].sort(); @@ -472,6 +475,62 @@ describe('no production module outside storage names a Disk layout', () => { }); }); +/** + * The product suite proves the exit criterion only while it stays ignorant of + * the backend (proposal §12.8). + * + * A case that reaches for a directory or a filename has stopped being + * evidence that anything is portable — it would keep passing for Disk and + * fail for the first backend that has neither, which is exactly backwards + * from what the suite is for. Enforced by reading the source, because the + * failure mode is a helpful-looking assertion someone adds later. + */ +describe('product boundary suite stays backend-blind', () => { + const SUITE = 'modules/storage/product-boundary.test.ts'; + + it('names no Disk record, blob, or directory vocabulary', () => { + // Quoted forms for the hidden tiers, so a scope *member* named `memory` + // — which is portable vocabulary — is not confused for the directory + // `.memory/`, which is not. + const DISK_VOCABULARY = [ + "'space.json'", + "'.artifacts", + "'.history", + "'.memory", + "'.upload", + "'.world", + 'diskTree', + 'canvasRoot', + 'nodesDir', + 'readFileSync', + 'existsSync', + 'mkdirSync', + ]; + const source = read(SUITE); + const found = DISK_VOCABULARY.filter((token) => source.includes(token)); + + expect(found).toEqual([]); + }); + + it('reaches storage only through the portable surface and the harness', () => { + const allowed = new Set([ + 'modules/storage/storage', + 'modules/storage/testing', + 'modules/storage/profile', + 'modules/storage/ports/blob', + 'modules/canvas/persistence-types', + ]); + const violations = specifiersOf(SUITE) + .map((spec) => resolveSpecifier(SUITE, spec)) + .filter((target): target is string => target !== null) + .filter((target) => !allowed.has(target)); + + // A backend import would let a case assert against an adapter directly, + // which is what the per-adapter suites are for. + 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( diff --git a/apps/server/src/modules/storage/ports/blob.ts b/apps/server/src/modules/storage/ports/blob.ts index c710d0f0..3f324dd7 100644 --- a/apps/server/src/modules/storage/ports/blob.ts +++ b/apps/server/src/modules/storage/ports/blob.ts @@ -4,10 +4,11 @@ /** * Blob storage port — opaque bytes, not application records. * - * The connection ({@link BlobStore}) is the primary object; a - * {@link BlobScope} is a bounded namespace derived from it. Blob storage is - * not canvas-specific — canvas scoping is one derived view, and new scope - * kinds extend {@link BlobScopeRef} without changing the connection. + * The connection ({@link BlobStore}) is the primary object; it vends one + * {@link SpaceBlobs} handle per Space, mirroring how `StructuredStore` vends + * one {@link SpaceHandle} per Space. Both ports are reached the same way for + * the same reason: a Space is the unit the application addresses, and a port + * that made the caller assemble a descriptor first would be the odd one out. * * Artifact identity, ownership, MIME representation, and lifecycle are * application concerns; this port only moves bytes. Today the HTTP boundary @@ -24,12 +25,66 @@ import type { Readable } from 'node:stream'; export type BlobBackendKind = 'disk' | 'azure'; /** - * Identifies a bounded namespace of blobs within a connection. + * Every area of one Space that holds bytes. * - * A one-member union today. Adding a scope kind (workspace assets, agent - * scratch) extends this type; the connection interface is unaffected. + * One member per **user-visible area**, not one bag per Space. That + * distinction is what keeps blob names flat: a Space's bytes live in several + * places a user can tell apart — the artifacts the app writes, the guide they + * author, the memory the agent keeps, the scratch an upload lands in — and the + * alternative to naming those areas is hierarchical blob names, which §7.1 + * excluded for every backend. An area is a member here and one placement rule + * per adapter; a path separator inside a name is a contract change. + * + * It also lets retention diverge later — scratch is not an artifact — without + * moving any bytes a second time. + */ +export interface SpaceBlobs { + /** Artifacts the application writes and the client fetches by key. */ + readonly artifacts: BlobScope; + /** + * User-authored guide documents at the Space root. + * + * Bounded by {@link SPACE_GUIDE_BLOB_NAMES} rather than by a directory, + * because the area a user authors in *is* the Space root — which also holds + * storage's own record. An area that claimed the whole directory would list + * `space.json` and delete the Space on `deleteAll()`. Naming the members is + * what makes the boundary real, and a fixed set is a tighter namespace than + * a directory, not a looser one. + */ + readonly guide: BlobScope; + /** The agent's private memory document for a Space. */ + readonly memory: BlobScope; + /** Scratch an upload lands in before anything claims it. */ + readonly uploads: BlobScope; +} + +/** + * Every blob name the `guide` area may hold. + * + * Product vocabulary rather than adapter detail: these are the documents a + * user is invited to author at the Space root, so the set belongs beside the + * area it defines. + */ +export const SPACE_GUIDE_SKILL_NAME = 'skill.md'; + +export const SPACE_GUIDE_BLOB_NAMES: readonly string[] = [ + SPACE_GUIDE_SKILL_NAME, +]; + +/** The one blob in the `memory` area: the agent's memory document. */ +export const SPACE_MEMORY_BLOB_NAME = 'space.md'; + +/** + * Every area of a Space, as a list. + * + * Deletion has to sweep all of them, and an area added without being swept + * would orphan bytes on a backend where dropping the Space does not remove the + * place they sit in. Written once here, beside the interface it enumerates, + * rather than restated by the saga. */ -export type BlobScopeRef = { kind: 'canvas'; canvasId: string }; +export function spaceBlobAreas(blobs: SpaceBlobs): readonly BlobScope[] { + return [blobs.artifacts, blobs.guide, blobs.memory, blobs.uploads]; +} export interface BlobInfo { /** Scope-relative name, e.g. `artifact_abc123.png`. */ @@ -125,13 +180,14 @@ export interface BlobScope { deleteAll(): Promise; } -/** A connection to a blob backend. Process-wide; scopes are derived. */ +/** A connection to a blob backend. Process-wide; handles are derived. */ export interface BlobStore { readonly kind: BlobBackendKind; init(): Promise; health(): Promise; close(): Promise; - scope(ref: BlobScopeRef): BlobScope; + /** The areas holding this Space's bytes. */ + space(canvasId: string): SpaceBlobs; } /** Thrown when a blob name is not a usable single path segment. */ 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 9623fcd8..61539556 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 @@ -24,13 +24,18 @@ import { Readable } from 'node:stream'; import { afterEach, describe, expect, it } from 'vitest'; -import { BlobLeaseError, BlobNameError } from '../blob.js'; +import { + BlobLeaseError, + BlobNameError, + SPACE_GUIDE_SKILL_NAME, +} from '../blob.js'; -import type { BlobScope, BlobScopeRef, BlobStore } from '../blob.js'; +import type { BlobScope, BlobStore } from '../blob.js'; export interface BlobContractHarness { store: BlobStore; - ref: BlobScopeRef; + /** The Space every case addresses. */ + canvasId: string; /** Release any resources the harness allocated. */ cleanup?: () => Promise | void; } @@ -59,7 +64,7 @@ export function describeBlobStoreContract( async function scope(): Promise { harness = await createHarness(); await harness.store.init(); - return harness.store.scope(harness.ref); + return harness.store.space(harness.canvasId).artifacts; } afterEach(async () => { @@ -67,6 +72,52 @@ export function describeBlobStoreContract( harness = null; }); + /** + * A member-bounded area is a real scope, not a filtered view of a + * directory. Disk maps `guide` onto the Space root — shared with + * `space.json` and every node directory — so a scope that answered for + * the folder would list storage's own records and take the Space with it + * on `deleteAll()`. These pin the boundary that makes the mapping safe. + */ + it('answers only for the names a bounded scope owns', async () => { + const h = await createHarness(); + harness = h; + await h.store.init(); + const guide = h.store.space(h.canvasId).guide; + + await guide.put(SPACE_GUIDE_SKILL_NAME, Buffer.from('# guide')); + expect(await guide.read(SPACE_GUIDE_SKILL_NAME)).toEqual( + Buffer.from('# guide'), + ); + expect((await guide.list()).map((info) => info.name)).toEqual([ + SPACE_GUIDE_SKILL_NAME, + ]); + + // A name outside the scope is refused before it reaches the backend, + // rather than resolving to whatever sits next to the members. + await expect(guide.read('space.json')).rejects.toThrow(); + await expect(guide.head('space.json')).rejects.toThrow(); + await expect( + guide.put('space.json', Buffer.from('{}')), + ).rejects.toThrow(); + }); + + it('deletes only its members when a scope is bounded by name', async () => { + const h = await createHarness(); + harness = h; + await h.store.init(); + const { artifacts, guide } = h.store.space(h.canvasId); + await artifacts.put('kept.bin', Buffer.from('artifact')); + await guide.put(SPACE_GUIDE_SKILL_NAME, Buffer.from('# guide')); + + await guide.deleteAll(); + + expect(await guide.read(SPACE_GUIDE_SKILL_NAME)).toBeNull(); + // The neighbours are not this scope's to remove — on Disk they are the + // Space itself. + expect(await artifacts.read('kept.bin')).toEqual(Buffer.from('artifact')); + }); + it('reports its backend kind and health', async () => { const s = await createHarness(); harness = s; @@ -326,13 +377,13 @@ export function describeBlobStoreContract( } }); - it('isolates scopes from one another', async () => { + it('isolates Spaces and areas from one another', async () => { const s = await createHarness(); harness = s; await s.store.init(); - const a = s.store.scope(s.ref); - const b = s.store.scope({ kind: 'canvas', canvasId: 'other-canvas-id' }); + const a = s.store.space(s.canvasId).artifacts; + const b = s.store.space('other-canvas-id').artifacts; await a.put('shared-name.txt', Buffer.from('from a')); expect(await b.head('shared-name.txt')).toBeNull(); diff --git a/apps/server/src/modules/storage/ports/contracts/space-extension.contract.ts b/apps/server/src/modules/storage/ports/contracts/space-extension.contract.ts new file mode 100644 index 00000000..4f2f7cbb --- /dev/null +++ b/apps/server/src/modules/storage/ports/contracts/space-extension.contract.ts @@ -0,0 +1,165 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Reusable contract for the extension substrate. + * + * Isolation and lifecycle, and nothing else — there is no data behaviour to + * assert, because the port never sees what an owner puts in a namespace. That + * is also why the harness supplies the read and write: the suite states what + * must be true of *whatever* an owner stored, and each backend says how one + * stores it. + */ + +import { afterEach, describe, expect, it } from 'vitest'; + +import type { + SpaceHandle, + SpaceRepository, + SpaceSubstrate, +} from '../structured.js'; + +export interface SpaceExtensionContractHarness { + /** Collection, so a case can create and destroy the Space it works on. */ + readonly repository: SpaceRepository; + readonly space: (canvasId: string) => SpaceHandle; + /** Store `value` in a substrate the way an owner of it would. */ + readonly write: ( + substrate: SpaceSubstrate, + value: string, + ) => Promise | void; + /** Read back what {@link write} stored, or null when nothing is there. */ + readonly read: ( + substrate: SpaceSubstrate, + ) => Promise | string | null; + readonly cleanup?: () => Promise | void; +} + +const OWNED = 'contract.owner'; +const OTHER = 'contract.other'; + +export function describeSpaceExtensionContract( + name: string, + createHarness: () => + | Promise + | SpaceExtensionContractHarness, +): void { + describe(`Space extension contract: ${name}`, () => { + let harness: SpaceExtensionContractHarness | null = null; + + async function open(): Promise { + harness = await createHarness(); + return harness; + } + + afterEach(async () => { + await harness?.cleanup?.(); + harness = null; + }); + + /** Create a Space and return a substrate for it. */ + async function substrateFor( + h: SpaceExtensionContractHarness, + canvasId: string, + namespace = OWNED, + ): Promise { + const created = await h.repository.create({ canvasId, title: canvasId }); + if (!created.ok && created.reason !== 'already-exists') { + throw new Error(`Expected to create ${canvasId}`); + } + const substrate = await h.space(canvasId).extension(namespace); + if (!substrate) { + throw new Error(`Expected a substrate for ${canvasId}/${namespace}`); + } + return substrate; + } + + it('refuses a namespace without an owner prefix', async () => { + const { repository, space } = await open(); + await repository.create({ + canvasId: 'contract-ext-grammar', + title: 'Grammar', + }); + + // The owner prefix is what makes a collision the owner's problem rather + // than storage's, so a bare name is refused rather than accommodated. + await expect( + space('contract-ext-grammar').extension('memory'), + ).rejects.toThrow(); + await expect( + space('contract-ext-grammar').extension('Huabu.Memory'), + ).rejects.toThrow(); + await expect( + space('contract-ext-grammar').extension('huabu.mem_ory'), + ).rejects.toThrow(); + }); + + it('has no substrate for a Space that does not exist', async () => { + const { space } = await open(); + + // Load-bearing: an owner writing through a substrate it was handed for a + // deleted Space could recreate that Space as a stub. + await expect( + space('contract-ext-absent').extension(OWNED), + ).resolves.toBeNull(); + }); + + it('keeps two namespaces on one Space apart', async () => { + const h = await open(); + const owned = await substrateFor(h, 'contract-ext-two-ns', OWNED); + const other = await substrateFor(h, 'contract-ext-two-ns', OTHER); + + await h.write(owned, 'owned value'); + await h.write(other, 'other value'); + + await expect(await h.read(owned)).toBe('owned value'); + await expect(await h.read(other)).toBe('other value'); + }); + + it('keeps one namespace on two Spaces apart', async () => { + const h = await open(); + const first = await substrateFor(h, 'contract-ext-space-a'); + const second = await substrateFor(h, 'contract-ext-space-b'); + + await h.write(first, 'first value'); + await h.write(second, 'second value'); + + await expect(await h.read(first)).toBe('first value'); + await expect(await h.read(second)).toBe('second value'); + }); + + it('resolves the same namespace to the same place every time', async () => { + const h = await open(); + const first = await substrateFor(h, 'contract-ext-stable'); + await h.write(first, 'written once'); + + const again = await h.space('contract-ext-stable').extension(OWNED); + if (!again) throw new Error('Expected a substrate'); + + await expect(await h.read(again)).toBe('written once'); + }); + + it('destroys a namespace with the Space', async () => { + const h = await open(); + const canvasId = 'contract-ext-lifecycle'; + const substrate = await substrateFor(h, canvasId); + await h.write(substrate, 'should not survive'); + await expect(await h.read(substrate)).toBe('should not survive'); + + const started = await h.repository.beginDelete({ canvasId }); + if (!started.ok) throw new Error('Ordinary Space must be deletable'); + await expect(started.session.finish()).resolves.toEqual({ + ok: true, + reason: 'deleted', + }); + + // Recreated under the same id, so this asserts the namespace was + // destroyed rather than merely unreachable. Storage owns this because + // only it can: no owner can clean up a layout that is not its own, and + // requiring one to register a hook would make deletion depend on every + // extension being loaded. + const recreated = await substrateFor(h, canvasId); + await expect(await h.read(recreated)).toBeNull(); + }); + }); +} diff --git a/apps/server/src/modules/storage/ports/namespace.test.ts b/apps/server/src/modules/storage/ports/namespace.test.ts new file mode 100644 index 00000000..f44522c9 --- /dev/null +++ b/apps/server/src/modules/storage/ports/namespace.test.ts @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { describe, expect, it } from 'vitest'; + +import { assertValidNamespace, SpaceNamespaceError } from './namespace.js'; + +describe('extension namespace grammar', () => { + it.each(['huabu.memory', 'agenetes.acp', 'huabu.prompt.log', 'owner2.name3'])( + 'accepts %s', + (namespace) => { + expect(assertValidNamespace(namespace)).toBe(namespace); + }, + ); + + it.each([ + // No owner prefix — the collision this token exists to prevent. + ['memory'], + ['huabu'], + // Reserved because a backend keyed on identifiers folds the dots into + // `_`; allowing it here would make that fold ambiguous. + ['huabu.mem_ory'], + ['huabu_memory'], + ['huabu.prompt-log'], + // Case-insensitive filesystems and case-folding identifiers would let + // these share one place with their lowercase spelling. + ['Huabu.Memory'], + // Empty or digit-led segments have no legal spelling as an identifier. + ['huabu.'], + ['.memory'], + ['huabu..memory'], + ['1huabu.memory'], + ['huabu.2memory'], + // Path traversal is not special-cased; it simply is not the grammar. + ['../escape'], + ['huabu/memory'], + [''], + ])('rejects %j', (namespace) => { + expect(() => assertValidNamespace(namespace)).toThrow(SpaceNamespaceError); + }); + + it('rejects a namespace too long to leave room for an owner’s own suffixes', () => { + const long = `huabu.${'a'.repeat(64)}`; + expect(() => assertValidNamespace(long)).toThrow(/longer than/); + }); +}); diff --git a/apps/server/src/modules/storage/ports/namespace.ts b/apps/server/src/modules/storage/ports/namespace.ts new file mode 100644 index 00000000..f41fb744 --- /dev/null +++ b/apps/server/src/modules/storage/ports/namespace.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Extension-namespace validation. + * + * The namespace is the isolation token for {@link SpaceHandle.extension}, and + * the only thing storage validates about an extension. It has to be safe as a + * directory name on every host, and as part of an identifier in every backend + * an adapter may later target — so the grammar is the intersection, decided + * once here rather than per adapter. + */ + +/** + * `.`, lowercase, dot-separated. + * + * Each segment is a letter followed by letters and digits. Deliberately narrow: + * + * - **An owner prefix is required.** A bare `memory` invites the collision the + * namespace exists to prevent; `huabu.memory` and `agenetes.acp` say whose + * it is. Storage cannot arbitrate a conflict it never sees the data behind. + * - **No `_` or `-`.** A backend keyed on identifiers rather than directories + * has to fold the dots into something legal, and the obvious fold is `_`. + * Allowing `_` in a namespace would make that fold ambiguous — `a.b` and + * `a_b` would land in the same place — so the character is reserved. + * - **Lowercase only.** Case-insensitive filesystems and case-folding + * identifier rules would otherwise let two namespaces that look distinct + * share one directory or one table. + */ +const NAMESPACE_RE = /^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9]*)+$/; + +/** Bounded so a namespace stays inside identifier-length limits with room for + * an owner's own suffixes. */ +const MAX_NAMESPACE_LENGTH = 64; + +export class SpaceNamespaceError extends Error { + override name = 'SpaceNamespaceError'; +} + +/** Validate an extension namespace, returning it unchanged. */ +export function assertValidNamespace(namespace: string): string { + if (namespace.length > MAX_NAMESPACE_LENGTH) { + throw new SpaceNamespaceError( + `Extension namespace is longer than ${MAX_NAMESPACE_LENGTH} characters: ` + + `${JSON.stringify(namespace)}`, + ); + } + if (!NAMESPACE_RE.test(namespace)) { + throw new SpaceNamespaceError( + `Extension namespace ${JSON.stringify(namespace)} is not of the form ` + + '"." in lowercase letters and digits (e.g. "huabu.memory").', + ); + } + return namespace; +} diff --git a/apps/server/src/modules/storage/ports/structured.ts b/apps/server/src/modules/storage/ports/structured.ts index 90571547..60332eeb 100644 --- a/apps/server/src/modules/storage/ports/structured.ts +++ b/apps/server/src/modules/storage/ports/structured.ts @@ -250,8 +250,64 @@ export interface SpaceHandle { * learns nothing from. A second kind of past record can reintroduce it. */ readonly events: SpaceEvents; + /** + * An isolated place for one namespace to build its own store on. + * + * The port hands over a *connection point* and nothing else. It never sees + * what the owner puts there, has no opinion about the shape, and offers no + * read or write of its own — because the alternative is one port member per + * feature (`memory`, `acpSessions`, `promptLogs`), obliging every future + * backend to model data it has no stake in. §12.5.7 rejected exactly that + * when it declined a `SpaceChats.list()` port. + * + * A namespaced opaque key/value member would be the obvious repair and is + * also wrong: it fixes one access shape — whole-value rewrite, no queries, + * no indexes — for every owner forever, and an owner with real query needs + * then encodes its own index inside an opaque value. So the owner brings its + * own store implementation and its own queries, per backend kind, and + * storage supplies only the place (§6.4.4). + * + * Storage keeps *lifecycle*, because only it can: a namespace is created on + * demand here and destroyed with the Space, which keeps + * {@link SpaceRepository.beginDelete} whole without any owner registering a + * cleanup hook — the one operation an owner could not perform without + * knowing a layout that is not its own. + * + * Returns `null` when the Space does not exist. That is deliberate and + * load-bearing: an owner writing through an ad-hoc path could recreate a + * Space that was just deleted, leaving a stub behind, and every such owner + * grew its own existence guard. Refusing to hand out a substrate for a Space + * that is gone puts that guard in the one place that can state it. + * + * Two consequences to accept openly rather than design against. The backend + * kind is part of the extension API, so an owner written against one kind + * does not load under another — inherent in letting owners write their own + * queries, and the alternative is the lowest common denominator this member + * exists to avoid. And the substrate is *unfenced*: the namespace bounds an + * owner by convention, not by enforcement. Extensions are in-process code, + * trusted at the same level as the Server. + * + * @param namespace Isolation token, `.` (see + * {@link assertValidNamespace}). Storage guarantees it is unique and + * unshared, and guarantees nothing about what is inside. + */ + extension(namespace: string): Promise; } +/** + * What a namespace gets to build on, discriminated by the live backend. + * + * One member per backend that exists, like {@link StructuredBackendKind} and + * for the same reason: a union that named `sqlite` today would advertise a + * substrate no adapter can supply. It grows with each adapter — a table prefix + * for SQLite, a schema for Postgres — and an owner switches on `kind`. + */ +export type SpaceSubstrate = { + readonly kind: 'disk'; + /** A directory reserved for this namespace, created and ready to write. */ + readonly directory: string; +}; + // ─── The ordered Space write ───────────────────────────────────────────────── export type SpaceWriteResult = diff --git a/apps/server/src/modules/storage/product-boundary.test.ts b/apps/server/src/modules/storage/product-boundary.test.ts new file mode 100644 index 00000000..e034f0dd --- /dev/null +++ b/apps/server/src/modules/storage/product-boundary.test.ts @@ -0,0 +1,356 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Phase 4.6's exit criterion, run against a real backend. + * + * The criterion: adding another `StructuredStore` changes adapter, + * composition, and migration code, but does not require Canvas, agent, web, + * RFS, interactive-view, Task, or Workspace feature modules to learn that + * backend's record layout (proposal §12.6). + * + * A suite cannot assert that about code it does not run, so this asserts the + * observable half: every durable thing the product does with a Space, driven + * through the portable surface, against a profile mounted the way a Server + * mounts one. **It names no directory, no filename, and no `space.json`** — + * `module-boundaries.test.ts` enforces that mechanically, because the moment a + * case reaches for a path it stops being evidence of anything portable. + * + * Phase 5 adds SQLite to `PRODUCT_STORAGE_PROFILES` and every case below runs + * against it unchanged. A case that then fails is a real gap in the adapter; + * a case that needs editing to pass is a leak in this suite. + */ + +import { afterEach, describe, expect, it } from 'vitest'; + +import { extractCanvasChanges } from '@huabu/shared/canvas-engine'; + +import { + SPACE_GUIDE_SKILL_NAME, + SPACE_MEMORY_BLOB_NAME, +} from './ports/blob.js'; +import { deleteSpace } from './storage.js'; +import { + forEachProductProfile, + mountTestWorkspace, + type MountedTestStorage, +} from './testing.js'; + +import type { StorageProfile } from './profile.js'; +import type { CanvasFile } from '../canvas/persistence-types.js'; +import type { CanvasNode } from '@huabu/shared/canvas-engine'; + +const NODE_A = 'node-product-a'; +const NODE_B = 'node-product-b'; + +function record(canvasId: string, version: number): CanvasFile { + return { + canvasId, + title: 'Product Space', + version, + state: { + nodes: [ + { id: NODE_A, type: 'note', position: { x: 0, y: 0 }, data: {} }, + { id: NODE_B, type: 'note', position: { x: 10, y: 0 }, data: {} }, + ], + edges: [], + }, + createdAt: 1, + updatedAt: 2, + } as CanvasFile; +} + +function note(nodeId: string, content: string) { + return { nodeId, type: 'note', label: nodeId, content }; +} + +/** + * A real change record, built through the engine rather than hand-rolled. + * + * `coalesceChanges` groups by the forward delta it reconstructs from + * `revertDeltas`, so a fabricated record with none is silently dropped and + * the case would assert nothing. + */ +function change(nodeId: string) { + const [built] = extractCanvasChanges([ + { + type: 'INSERT_NODE', + node: { + id: nodeId, + type: 'note', + position: { x: 0, y: 0 }, + data: { label: nodeId, content: 'body' }, + } as CanvasNode, + }, + ]); + return built; +} + +forEachProductProfile((profile: StorageProfile, label: string) => { + describe(`product storage boundary (${label})`, () => { + let mounted: MountedTestStorage | null = null; + + async function open(): Promise { + mounted = await mountTestWorkspace(profile, `huabu-product-${label}-`); + return mounted; + } + + /** A Space with two nodes, created the way the product creates one. */ + async function seedSpace(canvasId: string): Promise { + const m = await open(); + const created = await m.storage.structured + .spaces() + .create({ canvasId, title: 'Product Space' }); + if (!created.ok) throw new Error('Expected to create the Space'); + + const write = await m.storage.space(canvasId).write({ + expectedVersion: 0, + nextRecord: { + ...record(canvasId, 1), + title: created.record.title, + createdAt: created.record.createdAt, + }, + nodeMutations: [ + { kind: 'put', nodeId: NODE_A, record: note(NODE_A, 'alpha') }, + { kind: 'put', nodeId: NODE_B, record: note(NODE_B, 'beta') }, + ], + }); + expect(write).toEqual({ ok: true }); + return m; + } + + afterEach(async () => { + await mounted?.close(); + mounted = null; + }); + + it('bootstraps a World that ordinary listings exclude', async () => { + const m = await open(); + const spaces = m.storage.structured.spaces(); + + // The mount already ensured it — a Workspace with no World has no + // Portal target, so this is not something the product does later. + const worldId = await spaces.worldId(); + expect(worldId).toEqual(expect.any(String)); + await expect(spaces.list()).resolves.toEqual([]); + await expect(spaces.ensureWorld()).resolves.toBe(worldId); + }); + + it('creates a Space and reads back the record it was promised', async () => { + const m = await open(); + const spaces = m.storage.structured.spaces(); + const created = await spaces.create({ + canvasId: 'space-product-create', + title: 'Product Space', + }); + if (!created.ok) throw new Error('Expected to create the Space'); + + expect(created.record).toMatchObject({ + canvasId: 'space-product-create', + version: 0, + state: { nodes: [], edges: [] }, + }); + await expect( + m.storage.space('space-product-create').read(), + ).resolves.toEqual(created.record); + expect((await spaces.list()).map((row) => row.canvasId)).toEqual([ + 'space-product-create', + ]); + }); + + it('serves one ordered write through every node read shape', async () => { + const canvasId = 'space-product-nodes'; + const m = await seedSpace(canvasId); + const nodes = m.storage.space(canvasId).nodes; + + await expect(m.storage.space(canvasId).read()).resolves.toMatchObject({ + version: 1, + }); + + const single = await nodes.read(NODE_A); + expect(single?.record.content).toBe('alpha'); + + const listed = await nodes.list(); + expect([...listed.keys()].sort()).toEqual([NODE_A, NODE_B].sort()); + expect(listed.get(NODE_A)).toEqual(single); + + const selection = await nodes.readMany([NODE_B]); + expect([...selection.keys()]).toEqual([NODE_B]); + + const delivered: string[] = []; + const streamed = await nodes.stream((snapshot) => + delivered.push(snapshot.record.nodeId), + ); + expect(streamed).toEqual(listed); + expect(delivered.sort()).toEqual([NODE_A, NODE_B].sort()); + }); + + it('refuses a write from a version the caller no longer holds', async () => { + const canvasId = 'space-product-cas'; + const m = await seedSpace(canvasId); + + await expect( + m.storage.space(canvasId).write({ + expectedVersion: 0, + nextRecord: record(canvasId, 1), + nodeMutations: [], + }), + ).resolves.toEqual({ + ok: false, + reason: 'version-conflict', + actualVersion: 1, + }); + }); + + it('keeps bytes for every area of a Space and tells them apart', async () => { + const canvasId = 'space-product-blobs'; + const m = await seedSpace(canvasId); + const handle = m.storage.space(canvasId); + + await handle.artifacts.put('artifact.bin', Buffer.from('artifact bytes')); + await handle.memory.put(SPACE_MEMORY_BLOB_NAME, Buffer.from('# memory')); + await handle.guide.put(SPACE_GUIDE_SKILL_NAME, Buffer.from('# guide')); + await handle.uploads.put('staged.bin', Buffer.from('staged bytes')); + + expect(await handle.artifacts.read('artifact.bin')).toEqual( + Buffer.from('artifact bytes'), + ); + expect(await handle.memory.read(SPACE_MEMORY_BLOB_NAME)).toEqual( + Buffer.from('# memory'), + ); + // Areas are separate namespaces, so one area's name is not another's. + expect(await handle.artifacts.read(SPACE_MEMORY_BLOB_NAME)).toBeNull(); + expect((await handle.uploads.list()).map((info) => info.name)).toEqual([ + 'staged.bin', + ]); + }); + + it('refuses bytes for a Space that does not exist', async () => { + const m = await open(); + + // The one cross-store rule: bytes only for a Space whose record exists. + await expect( + m.storage + .space('space-product-absent') + .artifacts.put('orphan.bin', Buffer.from('x')), + ).rejects.toThrow(); + }); + + it('gives a namespace an isolated place and destroys it with the Space', async () => { + const canvasId = 'space-product-extension'; + const m = await seedSpace(canvasId); + + const substrate = await m.storage + .space(canvasId) + .extension('product.owner'); + expect(substrate?.kind).toBe(profile.structured.kind); + + // A Space that is gone has no substrate, which is what keeps an owner + // from resurrecting one by writing its own bookkeeping. + await deleteSpace(canvasId); + await expect( + m.storage.space(canvasId).extension('product.owner'), + ).resolves.toBeNull(); + }); + + it('appends and reads the Space log families', async () => { + const canvasId = 'space-product-logs'; + const m = await seedSpace(canvasId); + const handle = m.storage.space(canvasId); + + await handle.events.append([ + { + payload: { + action: 'node_created', + nodes: [{ id: NODE_A, type: 'note' }], + }, + ts: 10, + }, + ]); + const events = await handle.events.read(); + expect(events).toHaveLength(1); + + const appended = await handle.changes.append('thread-product', [ + change(NODE_A), + ]); + expect(appended).toHaveLength(1); + const changeId = appended[0].id; + await expect(handle.changes.read('thread-product')).resolves.toHaveLength( + 1, + ); + await expect( + handle.changes.delete('thread-product', changeId), + ).resolves.toMatchObject({ id: changeId }); + await expect(handle.changes.read('thread-product')).resolves.toEqual([]); + }); + + it('keeps a Task and its Runs in one ledger', async () => { + const canvasId = 'space-product-tasks'; + const m = await seedSpace(canvasId); + const tasks = m.storage.space(canvasId).tasks; + + await tasks.create({ + taskId: 'task-1', + canvasId, + goal: 'Do the thing', + defaultRootProfileId: 'profile-1', + anchorNodeId: NODE_A, + createdAt: 1, + }); + await tasks.runs.create({ + runId: 'run-1', + taskId: 'task-1', + canvasIdSnapshot: canvasId, + goalSnapshot: 'Do the thing', + rootProfileIdSnapshot: 'profile-1', + status: 'pending', + createdAt: 2, + }); + + const snapshot = await tasks.read(); + expect(snapshot.tasks.map((task) => task.taskId)).toEqual(['task-1']); + expect(snapshot.runs.map((run) => run.runId)).toEqual(['run-1']); + + await tasks.runs.update('run-1', { status: 'running', startedAt: 3 }); + const completed = await tasks.runs.complete('task-1', 'run-1', { + completedAt: 4, + message: 'done', + }); + expect(completed.outcome).toBe('completed'); + }); + + it('removes a Space and everything it held', async () => { + const canvasId = 'space-product-delete'; + const m = await seedSpace(canvasId); + const handle = m.storage.space(canvasId); + await handle.artifacts.put('artifact.bin', Buffer.from('bytes')); + await handle.memory.put(SPACE_MEMORY_BLOB_NAME, Buffer.from('# memory')); + + await expect(deleteSpace(canvasId)).resolves.toEqual({ + ok: true, + reason: 'deleted', + }); + + const after = m.storage.space(canvasId); + await expect(after.read()).resolves.toBeNull(); + await expect(after.nodes.list()).resolves.toEqual(new Map()); + // Every area, not only artifacts: an unswept one is an orphan on a + // backend where dropping the record does not remove the area. + expect(await after.artifacts.read('artifact.bin')).toBeNull(); + expect(await after.memory.read(SPACE_MEMORY_BLOB_NAME)).toBeNull(); + await expect(m.storage.structured.spaces().list()).resolves.toEqual([]); + }); + + it('refuses to delete the World', async () => { + const m = await open(); + const spaces = m.storage.structured.spaces(); + const worldId = await spaces.worldId(); + + await expect(deleteSpace(worldId)).resolves.toEqual({ + ok: false, + reason: 'world-forbidden', + }); + await expect(spaces.worldId()).resolves.toBe(worldId); + }); + }); +}); diff --git a/apps/server/src/modules/storage/profile.ts b/apps/server/src/modules/storage/profile.ts index f548af17..f962e871 100644 --- a/apps/server/src/modules/storage/profile.ts +++ b/apps/server/src/modules/storage/profile.ts @@ -89,6 +89,13 @@ export function parseStorageProfile( * cross-axis rules belong as backends land — for example, Postgres paired * with a node-local disk blob root is unsafe across replicas unless the * path is a deliberately shared filesystem. + * + * A profile that merely offers *fewer features* is not rejected here. Those + * are stated limitations rather than misconfigurations, and they are declared + * in `capabilities.ts` and reported at startup — see + * {@link describeUnavailableCapabilities}. Conflating the two would either + * refuse a legitimate deployment or let a real misconfiguration through as a + * warning. */ export function validateStorageProfile(profile: StorageProfile): void { if (!IMPLEMENTED_STRUCTURED.includes(profile.structured.kind)) { diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index 7363c44e..dc8ede82 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -38,6 +38,7 @@ import { DiskWorkspaceRepository, workspaceRegistryPath, } from './backends/disk/workspace-repository.js'; +import { spaceBlobAreas } from './ports/blob.js'; import { parseStorageProfile, requiresExplicitInit, @@ -56,6 +57,7 @@ import type { BlobRead, BlobScope, BlobStore, + SpaceBlobs, } from './ports/blob.js'; import type { StorageHealth } from './ports/common.js'; import type { @@ -129,27 +131,21 @@ export interface Storage { * One Space across every axis that holds part of it. * * A **composition-layer facade, not a port type**. `StructuredStore.space()` - * keeps returning the structured-only {@link SpaceHandle}, `BlobStore.scope()` - * keeps returning a {@link BlobScope}, and neither port imports the other. - * They are joined here because this is the only object in the process that - * holds both, and because this layer already owns every cross-store rule: the - * blob-put precondition and the blob-first delete saga. + * returns the structured {@link SpaceHandle}, `BlobStore.space()` returns the + * {@link SpaceBlobs} areas, and neither port imports the other. They are + * joined here because this is the only object in the process that holds both, + * and because this layer already owns the one cross-store rule the join needs: + * bytes may only be added to a Space whose record exists. * * The join cannot move down into a port. The two axes are configured * independently, so a `SpaceHandle` that vended blobs would oblige the Disk - * structured adapter to construct an Azure blob scope; deletion ordering - * deliberately keeps remote blob I/O outside any database transaction; and - * `BlobScopeRef` covers scopes that have no Space at all, which a blob store - * reachable only through a Space handle could not serve. + * structured adapter to construct an Azure blob handle, and deletion ordering + * deliberately keeps remote blob I/O outside any database transaction. + * + * Every member is a durable part of one Space, flat: which axis stores a part + * is this module's business, not its callers' (§6.4.1). */ -export interface Space extends SpaceHandle { - /** - * This Space's blobs, with the cross-store precondition applied. - * - * Bytes may only be added to a Space whose record exists. Reads and - * `deleteAll()` stay available for cleanup when a record has already gone. - */ - readonly blobs: BlobScope; +export interface Space extends SpaceHandle, SpaceBlobs { /** * Disk's directory for this Space. `null` on every other backend. * @@ -164,6 +160,9 @@ export interface Space extends SpaceHandle { function composeSpace(storage: Storage, canvasId: string): Space { const handle = storage.structured.space(canvasId); + const blobs = storage.blobs.space(canvasId); + const guarded = (scope: BlobScope): BlobScope => + guardedBlobScope(storage, canvasId, scope); return { canvasId: handle.canvasId, read: () => handle.read(), @@ -172,7 +171,11 @@ function composeSpace(storage: Storage, canvasId: string): Space { changes: handle.changes, tasks: handle.tasks, events: handle.events, - blobs: guardedBlobScope(storage, canvasId), + extension: (namespace) => handle.extension(namespace), + artifacts: guarded(blobs.artifacts), + guide: guarded(blobs.guide), + memory: guarded(blobs.memory), + uploads: guarded(blobs.uploads), diskTree: storage.profile.structured.kind === 'disk' ? diskSpaceTree(canvasId) @@ -379,6 +382,26 @@ export function getStorage(): Storage { return ensure(); } +/** + * Close the process's storage connections and forget them. + * + * Registered on graceful Server shutdown. Disk holds nothing a process exit + * would not release, so today this is close to a no-op — which is exactly why + * it has to exist before a connection-holding backend does: a pool that is + * never closed leaks on every restart, and the place to notice that is the + * lifecycle, not the adapter. + * + * Idempotent and safe before {@link initStorage}: shutdown must not depend on + * whether anything ever reached for storage. + */ +export async function closeStorage(): Promise { + const storage = current; + current = null; + workspaces = null; + if (!storage) return; + await Promise.all([storage.structured.close(), storage.blobs.close()]); +} + export function getBlobStore(): BlobStore { return ensure().blobs; } @@ -436,7 +459,15 @@ export async function deleteSpace( try { // Preserve the old retryable cleanup behavior: sweep even when the // structured record is already absent, so orphan blobs can be removed. - await storage.blobs.scope({ kind: 'canvas', canvasId }).deleteAll(); + // Every area, not just artifacts: a Space's bytes are spread across one + // scope per user-visible area, and on a backend where dropping the + // structured record does not remove the area they sit in, an unswept + // kind is an orphan. + await Promise.all( + spaceBlobAreas(storage.blobs.space(canvasId)).map((area) => + area.deleteAll(), + ), + ); return await started.session.finish(); } catch (error) { await started.session.abort(); @@ -448,7 +479,7 @@ export async function deleteSpace( } /** - * Blob scope for one Space, with the cross-store precondition applied. + * One blob area, with the cross-store precondition applied. * * The raw BlobStore intentionally knows nothing about structured lifecycle, * so composition owns the one cross-store invariant: bytes may only be added @@ -459,9 +490,12 @@ export async function deleteSpace( * one Space facade is composed entirely from the connections it was built * against — a scope that re-resolved the holder could outlive them. */ -function guardedBlobScope(storage: Storage, canvasId: string): BlobScope { +function guardedBlobScope( + storage: Storage, + canvasId: string, + delegate: BlobScope, +): BlobScope { const workspacePath = activeWorkspacePath(); - const delegate = storage.blobs.scope({ kind: 'canvas', canvasId }); async function requireSpace(): Promise { const record = await storage.structured.space(canvasId).read(); diff --git a/apps/server/src/modules/storage/testing.ts b/apps/server/src/modules/storage/testing.ts new file mode 100644 index 00000000..a5218314 --- /dev/null +++ b/apps/server/src/modules/storage/testing.ts @@ -0,0 +1,104 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Product-level storage harness. + * + * Opens a **real** profile against a temporary Workspace through the + * production lifecycle — prepared Workspace, opened connections, + * `ensureWorld()` — rather than swapping in a stub. That distinction is the + * whole point: a suite written against a stub proves that the application + * talks to an interface, while this one proves that a *backend* serves the + * product (proposal §12.8). + * + * It exists so a product test is written once and run against every profile. + * Phase 5 adds one entry to {@link PRODUCT_STORAGE_PROFILES} and the same + * behaviours are covered for SQLite, without a line of the suite changing — + * which is also the check that the suite never learned a backend's layout. + */ + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { closeStorage, initStorage } from './storage.js'; +import { setWorkspacePath } from '../workspace.js'; + +import type { StorageProfile } from './profile.js'; +import type { Storage } from './storage.js'; + +/** + * Every profile the product suite must pass against. + * + * A backend joins this list when it claims to serve the product, not when its + * adapter first compiles — an adapter may exist for isolated testing before + * its profile is selectable. + */ +export const PRODUCT_STORAGE_PROFILES: readonly StorageProfile[] = [ + { structured: { kind: 'disk' }, blobs: { kind: 'disk' } }, +]; + +/** Readable name for a profile, for test titles. */ +export function describeProfile(profile: StorageProfile): string { + return `${profile.structured.kind}/${profile.blobs.kind}`; +} + +export interface MountedTestStorage { + readonly profile: StorageProfile; + readonly storage: Storage; + /** The temporary Workspace. Only the harness itself should name paths. */ + readonly workspacePath: string; + close(): Promise; +} + +/** + * Open `profile` against a fresh temporary Workspace. + * + * Goes through `setWorkspacePath` and {@link initStorage} rather than + * reaching for the adapters, so a test exercises the same preparation, + * connection, and World bootstrap a running Server does. A backend whose + * startup is broken fails here, in the harness, instead of surfacing as a + * confusing product failure later. + */ +export async function mountTestWorkspace( + profile: StorageProfile, + prefix = 'huabu-product-', +): Promise { + // A profile label reads as `disk/disk`, which is not a directory name. + const safePrefix = prefix.replace(/[^a-zA-Z0-9._-]/g, '-'); + const workspacePath = mkdtempSync(path.join(tmpdir(), safePrefix)); + // Prepares and commits the Workspace, exactly as a synchronous activation + // does. Workspace selection precedes storage here for the same reason it + // does at boot: the backend is process-wide and the Workspace is the + // namespace selected inside it. + setWorkspacePath(workspacePath); + + const storage = await initStorage(profile); + // A namespace nobody has opened before has no World, and a Workspace + // without one has no home view. Every backend meets that state once. + await storage.structured.spaces().ensureWorld(); + + return { + profile, + storage, + workspacePath, + async close(): Promise { + await closeStorage(); + rmSync(workspacePath, { recursive: true, force: true }); + }, + }; +} + +/** + * Run `body` once per product profile. + * + * The suite names the profile only in its title. Anything a case needs to + * know about the backend it is running against would be a leak. + */ +export function forEachProductProfile( + body: (profile: StorageProfile, label: string) => void, +): void { + for (const profile of PRODUCT_STORAGE_PROFILES) { + body(profile, describeProfile(profile)); + } +} diff --git a/apps/server/src/modules/workspace/paths.ts b/apps/server/src/modules/workspace/paths.ts index de4407ae..5071f68f 100644 --- a/apps/server/src/modules/workspace/paths.ts +++ b/apps/server/src/modules/workspace/paths.ts @@ -9,10 +9,15 @@ * * - 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 the Space's Disk tree from the storage - * facade, so they no longer consult the Disk name index (§12.5.4). + * - Per-Space state owned by *other* domains — the memory body and ACP + * sessions — which need a materialized directory but not the Disk record + * layout. They anchor on the Space's Disk tree from the storage facade, + * so they no longer consult the Disk name index (§12.5.4). + * + * Two families have already left: memory-worker bookkeeping and the debug + * prompt log now build their own stores on the storage extension substrate + * (§6.4.4), which is a place rather than a path. The memory body follows as a + * blob, and ACP sessions with phase 6. * * The Disk record and blob layout moved to `storage/backends/disk/layout.ts`. * @@ -24,10 +29,8 @@ * / * .memory/ Space-scoped memory (AI-private) * space.md Space memory body - * state.json memory worker bookkeeping * .history/ * 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; * anything without the prefix is user-visible. @@ -35,7 +38,6 @@ import path from 'node:path'; -import { sanitizeId } from '../../utils/fs.js'; import { space } from '../storage/index.js'; import { getWorkspacePath } from '../workspace.js'; @@ -87,28 +89,6 @@ export function workspaceMemoryPath(): string { return path.join(settingDir(), 'user.md'); } -/** Hidden directory holding canvas-scoped canvas memory + bookkeeping. */ -export const WORKING_MEMORY_DIR_NAME = '.memory'; - -export function canvasMemoryDir(canvasId: string): string { - return path.join(spaceRoot(canvasId), WORKING_MEMORY_DIR_NAME); -} - -/** Working memory body for a canvas. */ -export function canvasMemoryPath(canvasId: string): string { - return path.join(canvasMemoryDir(canvasId), 'space.md'); -} - -/** - * Bookkeeping JSON for the memory worker, per canvas: - * `{ counter, lastAnalyzedAt, lastSeenThreadCursor }` - * - * Read/written by `modules/agent/memory/trigger.ts` (PR-B/C). - */ -export function memoryStatePath(canvasId: string): string { - return path.join(canvasMemoryDir(canvasId), 'state.json'); -} - // ─── Workspace-level setting / user skills ───────────────────────────────── /** @@ -129,20 +109,6 @@ export function userSkillsDir(): string { return path.join(settingDir(), 'skills'); } -/** - * Human-readable debug dump of the assembled prompt sent to the agent, - * one block per turn with strong turn separators. Append-only, written - * only when the `HUABU_DEBUG_PROMPT` env flag is set. Never read by the - * app — purely a developer post-mortem aid. See `conversation/prompt/debug-prompt.ts`. - */ -export function chatPromptLogPath(canvasId: string, threadId: string): string { - return path.join( - legacyHistoryDir(canvasId), - 'chat', - `${sanitizeId(threadId, 'threadId')}.prompt.log`, - ); -} - /** * ACP session persistence — maps each Huabu thread on this canvas * to the live ACP `sessionId` returned by `session/new`, so we can diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 4b22089c..2112d7c5 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -7,7 +7,10 @@ import { app } from './app.js'; import { resolveBindHost } from './bind-host.js'; import { prewarmOAuthCredentials } from './modules/agent/oauth.js'; import { resolveDeploymentConfig } from './modules/security/deployment-config.js'; -import { initStorage } from './modules/storage/index.js'; +import { + describeUnavailableCapabilities, + initStorage, +} from './modules/storage/index.js'; import { initializeSecretStore } from './security/secret-store.js'; import { getLogger } from './utils/logger.js'; @@ -34,14 +37,20 @@ async function start(): Promise { // Before anything serves: an unknown or unimplemented backend must // fail here with an actionable message, not on the first upload. - const storage = await initStorage(); + const { profile } = await initStorage(); log.info( { - structured: storage.profile.structured.kind, - blobs: storage.profile.blobs.kind, + structured: profile.structured.kind, + blobs: profile.blobs.kind, }, 'Storage backends ready', ); + // Not a warning about a misconfiguration — a stated product limitation of + // the selected profile, said up front rather than when a user clicks the + // button (proposal §6.4.2). Startup continues either way. + for (const line of describeUnavailableCapabilities(profile)) { + log.info({ capability: line }, 'Storage capability unavailable'); + } await initializeSecretStore(); await app.listen({ port: PORT, host: HOST }); diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index 3c1162d7..9b30ae22 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -4,7 +4,7 @@ ## 1. Overview -Every Space remains fully self-contained on Disk by default, but storage no longer presents one all-purpose `CanvasStore` as its backend contract. `apps/server/src/modules/storage/` separates backend-neutral blob and structured ports, Disk adapters, process-wide composition, and a shrinking Disk compatibility facade. Opaque artifact bytes flow through `BlobStore`; `StructuredStore` exposes one `spaces()` repository for the Space collection — membership, World identity, and create/delete/rename — while `SpaceHandle` exposes the Space's own async record read and ordered write plus the parts it holds: `nodes`, `changes`, `tasks`, and `events`. `SpaceNodes` reads one node by id, a named selection through `readMany()`, and the whole Space through `list()` / `stream()`; all four return the same records and the same opaque revisions. The application reaches all of it through one `space(canvasId)` handle on the composition root, which joins the structured handle with the Space's blob scope and, on Disk, its `diskTree`. Space creation and deletion, standalone node writes, executor and revert batches, preprocessing persistence, and event/change mutations enter these ports. Every production structured read now enters these ports too; `CanvasStore` lives inside the Disk adapter and its own suites, and no production module outside `storage/` imports it or a Disk layout symbol. What is left outside the ports are Disk-shaped _capabilities_ rather than reads: bundle export and import, reveal-in-file-manager, the built-in file tools, external-note observation and claim, RFS upload/delete, and boot migrations. Each mutates or names physical files, so they keep non-Disk profiles unselectable until they are declared as unavailable there rather than merely undesigned. +Every Space remains fully self-contained on Disk by default, but storage no longer presents one all-purpose `CanvasStore` as its backend contract. `apps/server/src/modules/storage/` separates backend-neutral blob and structured ports, Disk adapters, process-wide composition, and a shrinking Disk compatibility facade. Opaque artifact bytes flow through `BlobStore`; `StructuredStore` exposes one `spaces()` repository for the Space collection — membership, World identity, and create/delete/rename — while `SpaceHandle` exposes the Space's own async record read and ordered write plus the parts it holds: `nodes`, `changes`, `tasks`, and `events`. `SpaceNodes` reads one node by id, a named selection through `readMany()`, and the whole Space through `list()` / `stream()`; all four return the same records and the same opaque revisions. The application reaches all of it through one `space(canvasId)` handle on the composition root, which joins the structured handle with the Space's blob areas and, on Disk, its `diskTree`. Space creation and deletion, standalone node writes, executor and revert batches, preprocessing persistence, and event/change mutations enter these ports. Every production structured read now enters these ports too; `CanvasStore` lives inside the Disk adapter and its own suites, and no production module outside `storage/` imports it or a Disk layout symbol. What is left outside the ports are Disk-shaped _capabilities_ rather than reads: bundle export and import, reveal-in-file-manager, the built-in file tools, external-note observation and claim, RFS upload/delete, and boot migrations. Each mutates or names physical files, so they keep non-Disk profiles unselectable until they are declared as unavailable there rather than merely undesigned. Runtime Home-folder activation reserves the namespace switch before preparing and migrating the selected directory in a disposable child process. An in-flight Workspace operation therefore refuses the switch before the target is touched, and no new operation can enter the old Workspace while preparation is pending. This isolation is required because synchronous filesystem calls against cloud, network, or virtual drives can block indefinitely; a stuck preparation is terminated after 70 seconds with `WORKSPACE_ACTIVATION_TIMEOUT`, while the Server event loop and previously active workspace remain available. Concurrent activation attempts return `WORKSPACE_ACTIVATION_IN_PROGRESS`. Managed-mode startup still prepares synchronously before the Server accepts requests. @@ -24,13 +24,16 @@ Runtime Home-folder activation reserves the namespace switch before preparing an skills//SKILL.md # user / memory-agent authored skills / # dir name = safe(title) space.json # { canvasId, title, version, state:{nodes,edges,...}, createdAt, updatedAt } + skill.md # optional per-Space RFS access guide (blob area `guide`) nodes/ .md # frontmatter: id/type/label/src/... + content(markdown body) .artifacts/ # Disk BlobStore mapping for this Space # raw uploads (PDF / image / video / cover) .memory/ # hidden, AI-private canvas memory - space.md # canvas memory body - state.json # memory worker bookkeeping + space.md # canvas memory body (blob area `memory`) + .ext/ # hidden; one reserved directory per extension namespace + huabu.memory/state.json # memory worker bookkeeping + huabu.prompt.log/ # .prompt.log dumps, opt-in .history/ # hidden dir; also the Agenetes namespace storage.root chat_v2/ # canonical chat log — owned by Agenetes L2, NOT CanvasStore .events.jsonl # Tier-1: append-only AgentStreamEvent delta log (live turn) @@ -55,6 +58,10 @@ Key points: - Managed deployments expose exactly one Workspace — the active one. Other registrations in the same data directory are unaddressable there, so listing them would leak host folder names through the API that redacts host paths. - `space(canvasId)` is the one entry point to a Space. It is a composition-layer facade, not a port type: `StructuredStore` and `BlobStore` never import each other, and they are joined only where the cross-store rules already live — the blob-put precondition and the blob-first delete saga. It composes from its receiver, so substituting one axis on a `Storage` object yields Spaces built on the substitute. - A capability only one backend has hangs off that same handle, named for the backend and typed by its absence rather than stubbed to throw: `diskTree` is the Disk Space directory and is `null` on every other backend. It is not a port and does not live in `ports/`. `module-boundaries.test.ts` holds its exact production consumer census — a list that may shrink and must not grow — and asserts the barrel exposes nothing that reads as a portable path API. +- A Space's bytes are reached the same way as its records: `BlobStore.space(id)` returns one member per user-visible area — `artifacts`, `guide`, `memory`, `uploads` — so the Disk paths a user sees are unchanged and retention can diverge later without moving bytes. The `guide` area is bounded by its member names rather than by a directory, because its area is the Space root: a directory scope there would let `list()` claim `space.json` and `deleteAll()` remove the Space. Rename and per-key delete remain unsupported. +- `space(canvasId).extension(namespace)` hands an owner an isolated place to keep its own per-Space state — a reserved directory on Disk — and nothing else. Storage validates the namespace, creates it on demand, and destroys it with the Space, which is the one operation an owner cannot perform itself; it guarantees nothing about the contents, and cannot, because it never sees them. `extension()` returns `null` for a Space that is gone, which is where the per-owner `existsSync` resurrection guards went. Memory-worker bookkeeping and the debug prompt log are its first two owners; ACP session state is assigned here but moves with the Agenetes `Namespace` change. +- Features that are _about_ a filesystem are declared, not emulated. `capabilities.ts` lists bundle export and import, reveal-in-file-manager, the built-in file tools, external-note discovery, and Windows directory-handle coordination as Disk-only; startup logs the ones the selected profile does not offer and each refusal reuses that same wording. An unavailable feature is a stated limitation and startup continues, while a profile naming an unimplemented backend stays a misconfiguration that fails fast. +- `closeStorage()` closes both connections on graceful Server shutdown and forgets the holder. On Disk it releases nothing a process exit would not, and it exists for the backend that will hold a pool. - `SpaceRepository.ensureWorld()` is the backend-neutral World bootstrap: it returns the established World or mints exactly one version-0 World when the namespace holds none. An _established_ World that is missing or malformed stays the integrity error `worldId()` reports, because regenerating identity there would orphan every reference to it. Disk delegates to the same idempotent primitive Workspace preparation calls, so one file keeps one writer. - An ordinary Space **directory name** is derived from its title via `toSafeFilename(title)`, not from `canvasId`. The stable `canvasId` only lives inside `space.json`; the World is the reserved `.world` exception. - `SpaceRepository.list()` rescans on every call, returns ordinary Spaces only, skips ordinary directories without `space.json`, rejects malformed records (including a corrupt established World), and leaves ordering to the caller. `worldId()` resolves the hidden World from the same rescan and rejects missing or malformed state; it is the single World resolution point the collection's own create/delete/rename refusals also go through. @@ -67,7 +74,7 @@ Key points: - Canonical World preview identity is server-owned: non-system commands cannot create, repoint, or delete managed previews. Users may move and resize them. Ordinary Spaces may create and delete their own `spacePreview` nodes through normal UI commands. - Legacy `canvasRef`, `frameRef`, `nodeRef`, `SET_PORTAL_NODE_PINS`, and `GET /api/canvas/:worldCanvasId/references` remain compatibility surfaces for stored World data but are no longer created or exposed by the redesigned World UI. The current model is specified in [space-preview.md](./space-preview.md). - Node filenames are `safe(label).md`; the node's stable id lives in the `id:` frontmatter field. -- 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 `space(canvasId).blobs`: `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. +- The Disk `BlobStore` maps each Space scope to `.artifacts/`, with blobs named `` and no manifest file — the filename is the URL key. Artifacts are one of four blob areas a Space has, resolved as `space(canvasId).artifacts`: `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 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. @@ -85,7 +92,9 @@ Key points: | `ports/blob.ts` | Backend-neutral `BlobStore` connection/scope contract for opaque bytes and bounded materialization leases. | | `ports/workspace.ts` | Backend-neutral Workspace identity, membership, and locator repository. | | `ports/structured.ts` | Backend-neutral `StructuredStore`, the `SpaceRepository` collection — including the `ensureWorld()` bootstrap hook — and the `SpaceHandle` composite: record read/ordered write, nodes, changes, Tasks, and events. | -| `ports/contracts/` | Reusable Space-collection, node, Space-write, log, blob, and store suites; guarantees are the minimum every adapter implements. | +| `ports/contracts/` | Reusable Space-collection, node, Space-write, log, blob, extension-substrate, and store suites; guarantees are the minimum every adapter implements. | +| `capabilities.ts` | The Disk-only capability matrix `validateStorageProfile()` consults, plus the shared wording for a startup declaration and a runtime refusal. | +| `testing.ts` | Product harness: opens a real profile on a temporary Workspace through the production lifecycle, once per entry in `PRODUCT_STORAGE_PROFILES`. | | `backends/disk/` | Disk implementations plus before-image restoration for rejected in-process ordered batches; no journal or startup recovery. | | `backends/disk/legacy/` | The legacy `CanvasStore` and its synchronous adapter primitives, bounded Workspace-qualified cache, and process-local node tombstones. | | `compatibility/canvas.ts` | Residual Disk reads plus direct-module create/delete test fixtures; lifecycle writers are not exported from the public storage barrel. | @@ -98,7 +107,7 @@ The Disk structured adapter and compatibility facade resolve the same cached leg Canvas persistence DTOs and the write coordinator live under `modules/canvas/`; Workspace identity, durable membership, and Disk locators live under `modules/storage/`, while active-Workspace lifecycle and boot migrations remain under `modules/workspace/`; generic filesystem and Markdown codecs live under `utils/`. `canvasRoot()` validates the identifier and then verifies that the resolved Space directory remains a strict descendant of the active Workspace before any downstream Disk operation receives it. `module-boundaries.test.ts` enforces the storage dependency direction, prevents new consumers of the forwarding shims, and holds the neutrality guard: no production file outside `storage/` may import a Disk layout symbol or a legacy `CanvasStore` symbol. The check is import-level and symbol-level — a local variable that happens to be called `artifactPath` is not a violation, while importing `canvasRoot`, or reaching `getCanvasStore` through the barrel, is. Migrations are exempt because they rewrite frozen historical on-disk shapes; tests are exempt for the same reason they may name an adapter. -Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. +Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence; each is a declared Disk-only capability rather than an undesigned gap, so a non-Disk profile refuses the feature instead of being unselectable because of it. Retained Disk Space repository and handle instances, blob scopes, and legacy `CanvasStore` instances reject use after the active Workspace changes. Each `spaces()` call returns a fresh Workspace-bound handle and each read rescans current Disk state. The Workspace-qualified LRU is cleared and rebuilt on the next lookup after a switch. The delete-session contract covers overlapping operations through one configured backend instance. Disk realizes it with the shared process-local coordinator; it is not a multi-process transaction or distributed lock, and a SQL adapter must supply an equivalent backend-instance fence using its own mechanisms. diff --git a/docs/proposals/multi-backend-storage.md b/docs/proposals/multi-backend-storage.md index 20906ee1..aaf4c090 100644 --- a/docs/proposals/multi-backend-storage.md +++ b/docs/proposals/multi-backend-storage.md @@ -1,6 +1,6 @@ # Multi-Backend Storage -Status: Phases 1–4.5 implemented; §§12.6–12.8 in progress +Status: Phases 1–4.5 and §§12.6–12.8 implemented Last updated: 2026-08-24 > **Scope and decision confidence.** This proposal records the two-port @@ -57,7 +57,7 @@ Last updated: 2026-08-24 > phase: §12.6 (one Space handle and the portable read surface, > **implemented**), §12.7 (backend-agnostic application reads, > **implemented**), and §12.8 (the dispositions and the product-level -> harness). §12 is the authoritative plan; +> harness, **implemented**). §12 is the authoritative plan; > the decision table in §2 marks what each step has actually settled. --- @@ -98,7 +98,7 @@ built above these ports, but its form is intentionally unresolved here. | Blob key, staging, deletion, and GC semantics | Proposed / open | Names are the existing `` keys; `deleteAll()` covers Space destruction. Staging, reference counting, and GC remain undesigned. Per-key deletion stays out of the public port, but the absence of any cleanup path is what makes atomic replace mandatory (§6.2). | | Space-handle identity and caching | **Corrected** (P1) | `space(id)` returning a stable handle is bounded by the LRU behind it, not guaranteed. In-memory tombstones and the filename index are therefore adapter-local caches, never durable state (§12.1.1, §12.2.4). | | Reaching one Space | **Accepted** (§12.6) | One `space(canvasId)` facade on the composition root joins both ports; the two ports keep their independence and are joined only where the cross-store rules already live. A capability only one backend has hangs off the same handle, named for that backend and typed by its absence — `diskTree`, `null` elsewhere (§6.4.1). | -| Residual per-Space files | **Settled direction** | Four dispositions, not one: Disk-only and declared, portable and re-implemented, structured record, or blob (§6.4.2). Every current consumer is assigned in §6.4.3; scheduling is what stays open, and nothing is built before a backend needs it. | +| Residual per-Space files | **Accepted** (§12.8) | Four dispositions, not one: Disk-only and declared, portable and re-implemented, structured record, or blob (§6.4.2). Every current consumer is assigned in §6.4.3 and the ones that pay for themselves on Disk are built; what a second backend must pay for is named, not deferred silently. | | Backend selection scope | **Accepted** | Backend selection and its connection/pool are process-global. Workspaces are namespaces inside the configured backend; activating another Workspace re-scopes repository/handle operations without dropping or reconnecting the backend. A SQL profile serves every Workspace through one live connection/pool. | | Logical filesystem view | Open | A possible `SpaceFileView` above both stores; name and contract are not accepted yet. | | Real agent workspace | Open | Materialized directory, OS mount, protocol-only access, or a combination remain under evaluation. | @@ -370,8 +370,8 @@ method, not a second design. the structured `SpaceHandle`; `BlobStore.space(id)` returns `SpaceBlobs`, one member per user-visible area. A Space is the unit the application addresses on either axis, so a port that made the caller assemble a descriptor first would -be the odd one out — and the asymmetry is visible in the facade, which builds -scope descriptors by hand beside one structured handle. §12.8 closes it. +be the odd one out — and the asymmetry was visible in the facade, which built +scope descriptors by hand beside one structured handle until §12.8 closed it. `Space` is a **composition-layer facade, not a port type**. The two ports keep their interfaces and their independence — neither imports the other (§6.3) — @@ -2388,39 +2388,121 @@ alternative was a `strict` flag on `list()`, which the port had just finished arguing against: one caller's all-or-nothing preference is not a second read semantics every backend has to carry. -### 12.8 The dispositions, and a harness that proves them — **planned** - -The last change set applies §6.4.3 to the residual per-Space files and proves -the result against a mounted profile: - -- **D.** `BlobStore.space(id)` returns `SpaceBlobs`, symmetric with the - structured port, with one member per user-visible area — artifacts, the - `skill.md` guide, the memory body, upload scratch — so the Disk paths a user - sees are unchanged and retention can diverge later without moving bytes - again. -- **C.** The extension substrate of §6.4.4 lands as one Space-handle member - with a Disk case, namespace validation, and destruction with the Space. - Memory-worker state and the debug chat prompt log become its first two - namespaces, retiring two ad-hoc file formats. The `existsSync(spaceDir)` - resurrection guard disappears rather than moving. ACP session state keeps - disposition C but moves with the Agenetes `Namespace` change, not here. -- **A.** The Disk-only families become entries in a capability matrix - `validateStorageProfile()` consults, so an operator selecting a profile - learns up front which product features it does not offer. -- **Proof.** `storage/testing.ts` mounts a real profile onto a temporary - Workspace through the production lifecycle rather than swapping in a stub, - and a product-level suite runs the exit criterion against every profile in - `PRODUCT_STORAGE_PROFILES`, naming no directory, filename, or `space.json`. - -The exit criterion for the three change sets together has two halves. -**Neutrality:** adding another `StructuredStore` changes adapter, composition, -and migration code, but does not require Canvas, agent, web, RFS, -interactive-view, Task, or Workspace feature modules to learn that backend's -record layout. **One handle:** every storage capability for one Space is -reached through one `space(canvasId)` handle, and every family that is still a -bare file is one the capability matrix declares Disk-only. - -Out of scope throughout: a SQLite adapter or schema, Disk→SQLite data +### 12.8 The dispositions, and a harness that proves them — **implemented** + +The last of the three change sets. It applies §6.4.3 to the residual +per-Space files, declares what the resulting profile cannot do, and proves the +result against a real backend rather than a stub. It contains no SQLite +schema, driver, migration, or profile-selectability branch. + +#### 12.8.1 The extension substrate, and its first two namespaces + +§6.4.4 lands as one Space-handle member with a Disk case, namespace +validation, and destruction with the Space. The port exposes the connection +point and nothing else — a reserved directory on Disk — and the owner brings +its own store implementation and its own queries. Its contract is isolation +and lifecycle only; there is no data behaviour to assert, because the port +never sees the data. + +Storage keeps lifecycle because only it can: a namespace is created on demand +and destroyed with the Space, which keeps `beginDelete()` / `finish()` whole +without any owner registering a cleanup hook. + +Memory-worker state and the debug chat prompt log are its first two owners, +retiring two ad-hoc file formats. They are the proof that the substrate is +usable without a shared key/value helper; if a second owner wants the same +access shape, that helper goes in `utils/` **over** the substrate, never as a +port member. + +**The resurrection guard belonged in the port.** Several owners each carried +their own `existsSync(spaceDir)` check before writing bookkeeping into a Space +that might already be gone. `extension()` returning `null` for an absent Space +states it once, and the per-owner guards were deleted rather than moved. + +ACP session state keeps disposition **C** and does not move here: +`Namespace.storage.root` is a filesystem path in the shared +`@agenetes/protocol` contract, and replacing it with a composition-injected +substrate is an Agenetes port change, not a storage one. + +#### 12.8.2 One blob area per user-visible family + +`BlobStore.space(id)` returns `SpaceBlobs`, symmetric with +`StructuredStore.space(id)`, so both ports are reached the same way and the +facade stops assembling scope descriptors by hand. Each user-visible area is +its own member — `artifacts`, `guide`, `memory`, `uploads` — so the Disk paths +a user sees are unchanged and retention can diverge later without moving bytes +again. `skill.md` and the memory body qualify under §6.4.2's "simplifies Disk +on its own merits" exception: each was a bare `readFileSync` / `writeFile` +against a path the caller assembled, and the move retires those plus one +resolver in the memory sandbox. + +**A blob scope for the Space root cannot be a directory scope.** The `guide` +area shares its directory with `space.json` and every node directory, so +`list()` would claim storage's own records and `deleteAll()` would remove the +Space. It is bounded by its member names instead — a fixed set is a tighter +namespace than a directory, not a looser one. + +Upload scratch was **narrowed deliberately**. It received its own area, so it +is named, swept on delete, and free to diverge in retention — but its writers +still reach it through the `fs-sandbox` path, because RFS upload is a +streaming HTTP handler plus path classification, not the bare +`readFileSync` / `writeFile` the exception describes. Routing it through the +port would add machinery rather than retire any: the sandbox still needs the +physical path for the Disk-only file tools. It moves with RFS's path +vocabulary (**B**), not before. + +Rename and per-key delete stay unsupported, as they were. + +#### 12.8.3 What the profile cannot do, declared + +The **A** families — bundle export and import, reveal-in-file-manager, the +built-in file tools, external-note discovery, and Windows directory-handle +coordination — become entries in a capability matrix `validateStorageProfile()` +can consult. An operator selecting a profile learns up front which product +features it does not offer, alongside the existing rule that an unimplemented +kind fails at startup. The two are deliberately different outcomes: an +unavailable feature is a stated limitation and startup continues, while a +profile naming an unimplemented backend is a misconfiguration and still fails +fast. + +A feature's refusal shares its wording with the startup declaration, so the +sentence an operator read when they chose the profile is the sentence they see +in the failure. A feature that phrased its own refusal would drift from the +matrix, and the drift would only show up in a support thread. + +#### 12.8.4 Proof, and what the criterion now rests on + +The product harness is `storage/testing.ts`: it opens a real profile against a +temporary Workspace through the production lifecycle — prepared Workspace, +opened connections, `ensureWorld()` — rather than swapping in a stub. A stub +proves the application talks to an interface; only a real backend proves one +serves the product, which is the half that decides whether a second adapter +works. `product-boundary.test.ts` runs the criterion against every profile in +`PRODUCT_STORAGE_PROFILES`, naming no directory, filename, or `space.json`; +Phase 5 adds one entry to that list and the same behaviours are covered for +SQLite. A guard reads the suite's own source and rejects a directory, a +filename, or a `readFileSync` appearing in it, because the failure mode here +is a helpful-looking assertion someone adds later. The records the suite reads +back are built through the write engine, because a fixture that skips the +engine asserts nothing about what the product actually stores. + +`closeStorage()` arrives with it, registered on graceful Server shutdown and +used by the harness between profiles. On Disk it is close to a no-op — which +is why it has to exist before a connection-holding backend does: a pool nobody +closes leaks on every restart, and the lifecycle is where that is visible +rather than the adapter. + +With this the exit criterion holds on both halves. **Neutrality:** no +production module outside `storage/` imports a Disk layout symbol or a legacy +`CanvasStore` symbol, enforced import-level and symbol-level, migrations and +tests exempt. **One handle:** every storage capability for one Space is +reached through `space(canvasId)`, and the five consumers still holding +`diskTree` are the two the matrix declares Disk-only (the file-tool sandbox, +bundle export, external-note claim), RFS's sidecar-to-record mapping (**B**, +deferred until a second backend has a file plane at all), and the ACP session +path that leaves with the Agenetes `Namespace` change. + +Out of scope, unchanged: a SQLite adapter or schema, Disk→SQLite data migration, SQLite profile registration, Postgres/Azure, the portable change-notification capability, RFS's backend-neutral path vocabulary, ACP session relocation, the rest of the Agenetes persistence migration, the