diff --git a/apps/server/src/modules/agent/acp/service.ts b/apps/server/src/modules/agent/acp/service.ts index df217379b..2437b4659 100644 --- a/apps/server/src/modules/agent/acp/service.ts +++ b/apps/server/src/modules/agent/acp/service.ts @@ -31,7 +31,7 @@ import { getProfileSessionPreferences } from './profile-session-preferences.js'; import { getProfile as getLegacyProfile } from './profile-store.js'; import { buildReachbackEnv } from './reachback-env.js'; import { renderExternalAgentSystemPreamble } from '../../../prompt/external-agent/system-preamble.js'; -import { canvasAcpNamespace } from '../../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../../workspace/paths.js'; import { agenetes, EXTERNAL_DRIVER_KIND, diff --git a/apps/server/src/modules/agent/acp/service.workload-spec.test.ts b/apps/server/src/modules/agent/acp/service.workload-spec.test.ts index 799ca15d9..f9aede8df 100644 --- a/apps/server/src/modules/agent/acp/service.workload-spec.test.ts +++ b/apps/server/src/modules/agent/acp/service.workload-spec.test.ts @@ -41,7 +41,7 @@ vi.mock('./reachback-env.js', () => ({ buildReachbackEnv: () => ({ REACHBACK: '1' }), })); -vi.mock('../../workspace/disk/paths.js', () => ({ +vi.mock('../../workspace/paths.js', () => ({ canvasAcpNamespace: (canvasId: string) => `/canvases/${canvasId}/acp`, })); diff --git a/apps/server/src/modules/agent/acp/threads.route.ts b/apps/server/src/modules/agent/acp/threads.route.ts index 3c62ed126..ea773135a 100644 --- a/apps/server/src/modules/agent/acp/threads.route.ts +++ b/apps/server/src/modules/agent/acp/threads.route.ts @@ -50,7 +50,7 @@ import { buildReachbackEnv } from './reachback-env.js'; import { getExternalAgentRuntimeConfig } from './runtime-config.js'; import { resolveBindingRecipe } from './service.js'; import { renderExternalAgentSystemPreamble } from '../../../prompt/external-agent/system-preamble.js'; -import { canvasAcpNamespace } from '../../storage/paths.js'; +import { canvasAcpNamespace } from '../../workspace/paths.js'; import { agenetes, EXTERNAL_DRIVER_KIND, diff --git a/apps/server/src/modules/agent/agent-thread.service.ts b/apps/server/src/modules/agent/agent-thread.service.ts index 3bdcaf825..1ab52dab2 100644 --- a/apps/server/src/modules/agent/agent-thread.service.ts +++ b/apps/server/src/modules/agent/agent-thread.service.ts @@ -18,7 +18,7 @@ import { readWorkspaceMemory } from './memory/index.js'; import { planSkillDispatch } from './skill-model-routing.js'; import { acquireAgentTurn, waitForAgentTurnRelease } from './turn-lease.js'; import { loadAgent } from '../../prompt/index.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { HuabuSubmission } from './agenetes/handle.js'; import type { ChatEnvelope } from './conversation/envelope.js'; diff --git a/apps/server/src/modules/agent/agent.route.ts b/apps/server/src/modules/agent/agent.route.ts index 3cc1005fc..597a2fb53 100644 --- a/apps/server/src/modules/agent/agent.route.ts +++ b/apps/server/src/modules/agent/agent.route.ts @@ -31,7 +31,7 @@ import { import { buildChatEnvelope } from '../agent/conversation/envelope.js'; import { buildHistoryFromTurns } from '../agent/conversation/transcript/history.js'; import { getLLMModel } from '../agent/llm.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { ControlMsg, Namespace } from '@agenetes/protocol'; import type { diff --git a/apps/server/src/modules/agent/agent.service.ts b/apps/server/src/modules/agent/agent.service.ts index bfb49c89f..c5e9b3845 100644 --- a/apps/server/src/modules/agent/agent.service.ts +++ b/apps/server/src/modules/agent/agent.service.ts @@ -18,8 +18,6 @@ * themselves and pull the relevant `tool_result` payload. */ -import { loadAgent, type AgentId } from '../../prompt/index.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; import { agenetes, INTERNAL_DRIVER_KIND, @@ -28,6 +26,8 @@ import { } from './agenetes/drivers.js'; import { createChatSubmission } from './agenetes/handle.js'; import { buildHuabuPiWorkloadSpec } from './agenetes/pi-driver.js'; +import { loadAgent, type AgentId } from '../../prompt/index.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import { renderInternalAgentInputs } from './conversation/prompt/build-prompt.js'; import { dumpAssembledPrompt } from './conversation/prompt/debug-prompt.js'; import { type ToolScope } from './tools/index.js'; diff --git a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts index a84292af4..d5f703834 100644 --- a/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts +++ b/apps/server/src/modules/agent/conversation/prompt/debug-prompt.ts @@ -15,9 +15,10 @@ */ import { appendFileSync } from 'node:fs'; +import path from 'node:path'; import { mkdirp } from '../../../../utils/fs.js'; -import { chatDir, chatPromptLogPath } from '../../../storage/paths.js'; +import { chatPromptLogPath } from '../../../workspace/paths.js'; import type { Context } from '@earendil-works/pi-ai'; import type { FastifyBaseLogger } from 'fastify'; @@ -166,12 +167,9 @@ export function dumpAssembledPrompt(params: DumpPromptParams): void { }); out.push('', ''); - mkdirp(chatDir(canvasId)); - appendFileSync( - chatPromptLogPath(canvasId, params.threadId), - out.join('\n'), - 'utf-8', - ); + const logPath = chatPromptLogPath(canvasId, params.threadId); + mkdirp(path.dirname(logPath)); + appendFileSync(logPath, out.join('\n'), 'utf-8'); } catch (err) { params.logger.warn( { err: String(err) }, diff --git a/apps/server/src/modules/agent/memory/analyzer.test.ts b/apps/server/src/modules/agent/memory/analyzer.test.ts index 5cd69230d..caa15d40e 100644 --- a/apps/server/src/modules/agent/memory/analyzer.test.ts +++ b/apps/server/src/modules/agent/memory/analyzer.test.ts @@ -10,23 +10,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const physicalState = vi.hoisted(() => ({ root: '' })); vi.mock('../agent.service.js', () => ({ runAgent: vi.fn() })); -vi.mock('./trigger.js', () => ({ readMemoryState: vi.fn() })); vi.mock('../../../prompt/index.js', () => ({ loadAgent: vi.fn(), listSkills: vi.fn(), })); vi.mock('../../storage/index.js', () => ({ getStructuredStore: vi.fn() })); -vi.mock('../../workspace/disk/paths.js', () => ({ +vi.mock('../../workspace/paths.js', () => ({ canvasMemoryPath: (canvasId: string) => `${physicalState.root}/${canvasId}/.memory/space.md`, - chatDir: (canvasId: string) => - `${physicalState.root}/${canvasId}/.history/chat`, workspaceMemoryPath: () => `${physicalState.root}/setting/user.md`, })); import { runAgent } from '../agent.service.js'; import { runAnalysisPass } from './analyzer.js'; -import { readMemoryState } from './trigger.js'; import { loadAgent, listSkills } from '../../../prompt/index.js'; import { getStructuredStore } from '../../storage/index.js'; @@ -100,11 +96,6 @@ beforeEach(() => { .mockReturnValue( emptyAgentStream() as unknown as ReturnType, ); - vi.mocked(readMemoryState).mockReset().mockReturnValue({ - counter: 0, - lastAnalyzedAt: null, - lastSeenThreadCursor: null, - }); vi.mocked(loadAgent) .mockReset() .mockReturnValue({ @@ -134,7 +125,6 @@ describe('runAnalysisPass repository sources', () => { expect(space).toHaveBeenCalledWith('canvas-a'); expect(recordRead).toHaveBeenCalledTimes(1); expect(eventsRead).not.toHaveBeenCalled(); - expect(readMemoryState).not.toHaveBeenCalled(); expect(loadAgent).not.toHaveBeenCalled(); expect(runAgent).not.toHaveBeenCalled(); }); @@ -164,7 +154,6 @@ describe('runAnalysisPass repository sources', () => { await expect(runAnalysisPass('canvas-a')).resolves.toEqual({ status: 'completed', results: [], - latestChatTs: null, }); expect(space).toHaveBeenCalledTimes(1); diff --git a/apps/server/src/modules/agent/memory/analyzer.ts b/apps/server/src/modules/agent/memory/analyzer.ts index 46cb5e40a..fb21fc66f 100644 --- a/apps/server/src/modules/agent/memory/analyzer.ts +++ b/apps/server/src/modules/agent/memory/analyzer.ts @@ -8,8 +8,8 @@ * The worker calls {@link runAnalysisPass}. We: * * 1. Build the system prompt from `prompt/agents/memory/AGENT.md`. - * 2. Assemble a compact context bundle from backend-owned Space records and - * logs plus the remaining Disk-owned chat and memory surfaces. + * 2. Assemble a compact context bundle from backend-owned Space records + * and logs, plus the memory surfaces. * 3. Run the sub-agent against that context. The agent's only way * to affect the world is via the `fs_write` tool, whose handler * routes by virtual path into the writers in `./writers.ts`. @@ -24,11 +24,8 @@ * mutations on the same disk targets apply in declared order. */ -import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; -import path from 'node:path'; +import { existsSync, readFileSync } from 'node:fs'; -import { runAgent } from '../agent.service.js'; -import { readMemoryState } from './trigger.js'; import { loadAgent, listSkills } from '../../../prompt/index.js'; import { getStructuredStore, @@ -38,9 +35,9 @@ import { } from '../../storage/index.js'; import { canvasMemoryPath, - chatDir, workspaceMemoryPath, -} from '../../workspace/disk/paths.js'; +} from '../../workspace/paths.js'; +import { runAgent } from '../agent.service.js'; import type { MemoryLogger } from './index.js'; import type { WriteResult } from './writers.js'; @@ -54,8 +51,6 @@ import type { Context, Message } from '@earendil-works/pi-ai'; */ const MAX_NODES_IN_SNAPSHOT = 60; const MAX_EVENTS_IN_DIGEST = 100; -const MAX_CHAT_TURNS_IN_DIGEST = 12; -const MAX_THREAD_SCAN = 6; /** * Run one memory analysis pass. @@ -64,19 +59,11 @@ const MAX_THREAD_SCAN = 6; * does NOT call `markAnalyzed` (so the next trigger retries). Writer * rejections are *not* errors — they come back as `ok:false` tool * results which we surface in the returned summary. - * - * The returned `latestChatTs` is the maximum message timestamp the - * pass scanned (independent of which were summarised into the - * prompt). The worker persists it as `lastSeenThreadCursor` via - * {@link markAnalyzed} so subsequent passes only look at strictly - * newer turns — without it the chat digest would re-include the - * same messages every threshold crossing. */ export type AnalysisPassResult = | { status: 'completed'; results: WriteResult[]; - latestChatTs: number | null; } | { status: 'skipped'; reason: 'space-not-found' }; @@ -136,7 +123,6 @@ export async function runAnalysisPass( return { status: 'completed', results: writeResults, - latestChatTs: bundle.latestChatTs, }; } @@ -173,13 +159,6 @@ function parseWriteResult(raw: string): WriteResult | null { interface ContextBundle { messages: Message[]; summary: string; - /** - * Max message timestamp scanned by the chat digest, or `null` when - * no new turns were seen. Carries to the worker so it can persist - * `lastSeenThreadCursor` and the next pass only looks at strictly - * newer turns. - */ - latestChatTs: number | null; } /** @@ -207,18 +186,6 @@ async function assembleContext( }); parts.push(`${snapshot.nodeCount} nodes`); - const state = readMemoryState(canvasId); - const chat = readChatDigest(canvasId, state.lastSeenThreadCursor); - if (chat) { - messages.push({ - role: 'user', - content: `[SYSTEM Chat digest since ${ - state.lastSeenThreadCursor ?? 'start' - }]\n${chat.text}`, - timestamp: Date.now(), - }); - parts.push(`${chat.turns} chat turns`); - } const eventRows = await handle.events.read(MAX_EVENTS_IN_DIGEST); const events = readEventsDigest(eventRows); if (events) { @@ -247,7 +214,6 @@ async function assembleContext( return { messages, summary: parts.join(', ') || '(empty)', - latestChatTs: chat?.latestTs ?? null, }; } @@ -293,116 +259,6 @@ function summariseNode(node: unknown): string { return `- [${type}] ${id} "${label.slice(0, 60)}"${pos}`; } -interface ChatDigest { - text: string; - turns: number; - /** - * Max `timestamp` seen across every message that passed the `since` - * filter, regardless of whether it landed in the digest body. The - * worker persists this as the next pass's `lastSeenThreadCursor` - * so the chat digest monotonically advances. - */ - latestTs: number | null; -} - -/** - * Pull a digest of recent chat turns from `/.history/chat/`. - * - * Strategy: - * - List every thread file, sorted by `mtime` descending. - * - Walk up to {@link MAX_THREAD_SCAN} threads, scanning each - * message in turn. For each message: - * - drop turns older than `since` (the bookkeeping's - * `lastSeenThreadCursor`); - * - track `latestTs` = max(`timestamp`) of every survivor, - * so the caller can advance the cursor even when the - * digest body itself was capped; - * - skip system / non-user / non-assistant rows; - * - emit up to {@link MAX_CHAT_TURNS_IN_DIGEST} into the body. - * - For each emitted turn, render the role + the first ~200 chars - * of the content (or `[tool: name]` for assistant turns that - * only carried tool calls). - */ -function readChatDigest( - canvasId: string, - since: number | null, -): ChatDigest | null { - const dir = chatDir(canvasId); - if (!existsSync(dir)) return null; - let files: string[]; - try { - files = readdirSync(dir); - } catch { - return null; - } - const threads = files - .filter((f) => f.endsWith('.json')) - .map((f) => path.join(dir, f)) - .map((p) => ({ path: p, mtime: safeMtime(p) })) - .filter((t) => t.mtime !== null) - .sort((a, b) => (b.mtime ?? 0) - (a.mtime ?? 0)) - .slice(0, MAX_THREAD_SCAN); - - const lines: string[] = []; - let turns = 0; - let latestTs: number | null = null; - for (const thread of threads) { - let ctx: { messages?: unknown[] } | null; - try { - ctx = JSON.parse(readFileSync(thread.path, 'utf8')) as { - messages?: unknown[]; - }; - } catch { - continue; - } - if (!ctx?.messages || !Array.isArray(ctx.messages)) continue; - for (const m of ctx.messages) { - if (!m || typeof m !== 'object') continue; - const msg = m as { - role?: string; - content?: unknown; - timestamp?: number; - }; - const ts = typeof msg.timestamp === 'number' ? msg.timestamp : null; - if (since !== null && ts !== null && ts <= since) continue; - - // Advance latestTs for every message that survived the `since` - // filter — not just the ones we end up emitting. That way the - // cursor still advances when MAX_CHAT_TURNS_IN_DIGEST has been - // reached, and we don't re-scan the same prefix next pass. - if (ts !== null && (latestTs === null || ts > latestTs)) { - latestTs = ts; - } - - const role = msg.role; - if (role !== 'user' && role !== 'assistant') continue; - if (turns >= MAX_CHAT_TURNS_IN_DIGEST) continue; - const text = digestMessageContent(msg.content); - if (text.startsWith('[SYSTEM')) continue; - lines.push(`${role}: ${text}`); - turns++; - } - } - if (turns === 0 && latestTs === null) return null; - return { text: lines.join('\n'), turns, latestTs }; -} - -function digestMessageContent(content: unknown): string { - if (typeof content === 'string') return content.slice(0, 200); - if (!Array.isArray(content)) return ''; - const parts: string[] = []; - for (const block of content) { - if (!block || typeof block !== 'object') continue; - const b = block as { type?: string; text?: string; name?: string }; - if (b.type === 'text' && typeof b.text === 'string') { - parts.push(b.text); - } else if (b.type === 'toolCall' && typeof b.name === 'string') { - parts.push(`[tool: ${b.name}]`); - } - } - return parts.join(' ').slice(0, 200); -} - interface EventsDigest { text: string; count: number; @@ -462,11 +318,3 @@ function readFileSafe(file: string): string { return ''; } } - -function safeMtime(file: string): number | null { - try { - return statSync(file).mtimeMs; - } catch { - return null; - } -} diff --git a/apps/server/src/modules/agent/memory/read.ts b/apps/server/src/modules/agent/memory/read.ts index a719e8255..c8776b05f 100644 --- a/apps/server/src/modules/agent/memory/read.ts +++ b/apps/server/src/modules/agent/memory/read.ts @@ -15,7 +15,10 @@ import { existsSync, readFileSync } from 'node:fs'; -import { workspaceMemoryPath, canvasMemoryPath } from '../../storage/paths.js'; +import { + workspaceMemoryPath, + canvasMemoryPath, +} from '../../workspace/paths.js'; /** * Read the user memory body. diff --git a/apps/server/src/modules/agent/memory/sandbox.ts b/apps/server/src/modules/agent/memory/sandbox.ts index dfbf1ebc6..0085b4e43 100644 --- a/apps/server/src/modules/agent/memory/sandbox.ts +++ b/apps/server/src/modules/agent/memory/sandbox.ts @@ -31,7 +31,7 @@ import { userSkillsDir, canvasMemoryDir, canvasMemoryPath, -} from '../../storage/paths.js'; +} from '../../workspace/paths.js'; /** Thrown by every resolver below on out-of-sandbox attempts. */ export class MemorySandboxError extends Error { @@ -101,7 +101,7 @@ export function resolveUserSkillPath(id: string): string { /** * Resolve the absolute Space memory file path. Throws if the resolved path * escapes the canvas's `.memory/` root (a defensive check — the path - * computation in `workspace/disk/paths.ts` already constrains the result, + * computation in `workspace/paths.ts` already constrains the result, * but going through `ensureUnderRoot` keeps the invariant explicit). */ export function resolveWorkingMemoryPath(canvasId: string): string { diff --git a/apps/server/src/modules/agent/memory/trigger.ts b/apps/server/src/modules/agent/memory/trigger.ts index 843e855aa..6367f1c58 100644 --- a/apps/server/src/modules/agent/memory/trigger.ts +++ b/apps/server/src/modules/agent/memory/trigger.ts @@ -11,9 +11,9 @@ * enqueued when this crosses {@link OP_THRESHOLD}. * lastAnalyzedAt epoch ms of the last successful analysis; * null until the first pass lands. - * lastSeenThreadCursor pi-ai context timestamp of the last - * analysed chat turn — lets `context.ts` (PR-C) - * only pull "new" turns into the analysis prompt. + * lastSeenThreadCursor retained compatibility field from the removed + * legacy chat digest. Existing state files preserve it, + * but current analysis passes do not advance it. * * Persisted at `/.memory/state.json` so the counter * survives process restarts. The file is kept tiny (<128 B) and @@ -26,11 +26,8 @@ import { existsSync } from 'node:fs'; import { atomicWriteJson, mkdirp, readJson } from '../../../utils/fs.js'; import { createKeyedMutex } from '../../../utils/keyed-mutex.js'; -import { - memoryStatePath, - canvasMemoryDir, - canvasRoot, -} from '../../storage/paths.js'; +import { spaceDirectory } from '../../storage/index.js'; +import { memoryStatePath, canvasMemoryDir } from '../../workspace/paths.js'; /** Op-count threshold that triggers a memory analysis pass. */ export const OP_THRESHOLD = 50; @@ -86,7 +83,7 @@ export function writeMemoryState(canvasId: string, state: MemoryState): void { // file. Same hazard for any in-flight memory worker that calls // `markAnalyzed` post-delete. Skip the write when the canvas root // is gone; losing one bookkeeping write is harmless. - if (!existsSync(canvasRoot(canvasId))) return; + if (!existsSync(spaceDirectory(canvasId))) return; mkdirp(canvasMemoryDir(canvasId)); atomicWriteJson(memoryStatePath(canvasId), state); } diff --git a/apps/server/src/modules/agent/memory/worker.test.ts b/apps/server/src/modules/agent/memory/worker.test.ts index e1962ace7..4b89e395b 100644 --- a/apps/server/src/modules/agent/memory/worker.test.ts +++ b/apps/server/src/modules/agent/memory/worker.test.ts @@ -59,19 +59,16 @@ describe('memory worker outcomes', () => { ); }); - it('marks completed passes with only the non-null cursors', async () => { + it('marks completed passes after summarising writer results', async () => { vi.mocked(runAnalysisPass).mockResolvedValue({ status: 'completed', results: [{ ok: true, target: 'space', reason: 'updated' }], - latestChatTs: 25, }); const log = logger(); await runScheduled('canvas-a', log); - expect(markAnalyzed).toHaveBeenCalledWith('canvas-a', { - lastSeenThreadCursor: 25, - }); + expect(markAnalyzed).toHaveBeenCalledWith('canvas-a'); expect(log.info).toHaveBeenCalledWith( '[memory] pass for canvas canvas-a done — 1 ok, 0 rejected', ); diff --git a/apps/server/src/modules/agent/memory/worker.ts b/apps/server/src/modules/agent/memory/worker.ts index 790a9d50e..24b55a6cc 100644 --- a/apps/server/src/modules/agent/memory/worker.ts +++ b/apps/server/src/modules/agent/memory/worker.ts @@ -89,23 +89,13 @@ async function runOnce(canvasId: string, logger?: MemoryLogger): Promise { ); return; } - const { results, latestChatTs } = outcome; + const { results } = outcome; // markAnalyzed is intentionally always called when the pass finished // without throwing — even if individual writers rejected (e.g. a // create-rationale violation). The bookkeeping records "we tried", // not "we wrote". This avoids hammering the threshold with retries // when the LLM keeps producing rejected outputs. - // - // `latestChatTs` advances the chat cursor so the next pass's digest - // only includes strictly newer rows. `null` means that source saw - // nothing new past the existing cursor — in which case we leave the - // cursor untouched (handled by markAnalyzed when the field is - // omitted). - const cursorUpdate: { - lastSeenThreadCursor?: number; - } = {}; - if (latestChatTs !== null) cursorUpdate.lastSeenThreadCursor = latestChatTs; - markAnalyzed(canvasId, cursorUpdate).catch((err: unknown) => { + markAnalyzed(canvasId).catch((err: unknown) => { // markAnalyzed is now async (it shares the per-canvas state // lock with bumpOpCounter). A bookkeeping write failure does // not invalidate the pass — log and continue. diff --git a/apps/server/src/modules/agent/node-ref.ts b/apps/server/src/modules/agent/node-ref.ts index 10398c700..9521fb140 100644 --- a/apps/server/src/modules/agent/node-ref.ts +++ b/apps/server/src/modules/agent/node-ref.ts @@ -32,7 +32,7 @@ import { nodeRevisionOf } from '@huabu/shared/canvas-engine'; -import { toSafeFilename } from '../workspace/disk/naming.js'; +import { toSafeFilename } from '../../utils/naming.js'; import type { CanvasNodeType, WireNodeRef } from '@huabu/shared'; diff --git a/apps/server/src/modules/agent/skills.route.test.ts b/apps/server/src/modules/agent/skills.route.test.ts index 897e0d4bd..6c9bdd32c 100644 --- a/apps/server/src/modules/agent/skills.route.test.ts +++ b/apps/server/src/modules/agent/skills.route.test.ts @@ -30,7 +30,7 @@ import { invalidateSkillCache, type LoadedSkill, } from '../../prompt/skills/loader.js'; -import { userSkillsDir } from '../storage/paths.js'; +import { userSkillsDir } from '../workspace/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { SkillCatalogueEntry } from '@huabu/shared'; diff --git a/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts index 39b5e0bfe..bfd76b60f 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-read.test.ts @@ -8,8 +8,8 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { handleRead } from './fs-read.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { getCanvasStore } from '../../../storage/index.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; import { setWorkspacePath } from '../../../workspace.js'; interface ReadResult { diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts new file mode 100644 index 000000000..4b3122bf6 --- /dev/null +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { describe, expect, it } from 'vitest'; + +import { isArtifactsRel, toPhysicalRel } from './fs-sandbox.js'; + +/** + * `isArtifactsRel` applies the segment-aware membership rules after a node + * `src` has been safely resolved relative to its actual Space. The import-hook + * tests cover that filesystem resolution; these pure cases pin the remaining + * path classification without a fixture. + */ +describe('isArtifactsRel', () => { + it('accepts both the virtual and physical spellings', () => { + expect(isArtifactsRel(toPhysicalRel('artifacts/pic.png'))).toBe(true); + expect(isArtifactsRel(toPhysicalRel('.artifacts/pic.png'))).toBe(true); + // A bare key resolves into the artifacts dir via the same map. + expect(isArtifactsRel(toPhysicalRel('artifacts'))).toBe(true); + }); + + it('rejects refs outside the artifacts directory', () => { + expect(isArtifactsRel(toPhysicalRel('nodes/foo.md'))).toBe(false); + expect(isArtifactsRel(toPhysicalRel('upload/pic.png'))).toBe(false); + expect(isArtifactsRel(toPhysicalRel('space.json'))).toBe(false); + // A sibling whose name merely starts with the directory name is not + // inside it — the reason this is a segment-wise test, not a prefix one. + expect(isArtifactsRel(toPhysicalRel('.artifacts-evil/pic.png'))).toBe( + false, + ); + }); + + it('collapses traversal rather than matching on the literal prefix', () => { + // Reaches the hidden dir the long way round; a bare prefix test would + // miss it and the hook would import a file that is already stored. + expect(isArtifactsRel(toPhysicalRel('nodes/../.artifacts/pic.png'))).toBe( + true, + ); + // Leaves it again, so it is an ordinary local file. + expect(isArtifactsRel(toPhysicalRel('.artifacts/../nodes/foo.md'))).toBe( + false, + ); + }); + + it('aliases the virtual prefix only at the start of the ref', () => { + // `toPhysicalRel` rewrites `artifacts/` as a prefix, so a mid-path + // occurrence stays literal and resolves to `/artifacts/…`, which is not + // the hidden directory. Pinned because it is a limitation, not a + // decision: the pre-existing check behaved the same way, and widening it + // would change which files the import hook copies. + expect(isArtifactsRel(toPhysicalRel('nodes/../artifacts/pic.png'))).toBe( + false, + ); + }); +}); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts index bad4173aa..18b80a5ea 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-sandbox.ts @@ -30,8 +30,7 @@ import { readFileSync, readdirSync, statSync, type Dirent } from 'node:fs'; import path from 'node:path'; import { parseFrontmatter } from '../../../../utils/markdown-frontmatter.js'; -import { getCanvasStore } from '../../../storage/index.js'; -import { canvasRoot } from '../../../storage/paths.js'; +import { getCanvasStore, spaceDirectory } from '../../../storage/index.js'; // ─── Always-skipped directory names ───────────────────────────────────────── @@ -72,6 +71,26 @@ const VIRTUAL_PREFIX: ReadonlyArray = [ ['upload/', '.upload/'], ]; +/** + * Whether an already-resolved, Space-relative physical path denotes something + * under `.artifacts/`. + * + * Callers must first resolve the original ref with {@link safeResolve}, then + * make that absolute target relative to the actual Space root. The second + * step matters for refs such as `../Canvas/.artifacts/pic.png`, which leave + * and re-enter the same Space before resolving inside `.artifacts/`. + * + * Resolving the resulting relative path against a synthetic root keeps this + * membership check independent of storage while preserving segment-aware + * normalization and sibling-prefix protection. + */ +export function isArtifactsRel(resolvedPhysicalRel: string): boolean { + const [, artifactsPhysical] = VIRTUAL_PREFIX[0]; + const root = path.resolve('/', artifactsPhysical); + const target = path.resolve('/', resolvedPhysicalRel); + return target === root || target.startsWith(root + path.sep); +} + /** * Rewrite a request path's virtual prefix (`artifacts/`, `upload/`) to its * hidden on-disk counterpart. Idempotent: an already-physical `.upload/…` @@ -124,7 +143,7 @@ export function safeResolve(canvasId: string, rel: string): string { ) { throw new Error(`Invalid canvasId: ${canvasId}`); } - const root = canvasRoot(canvasId); + const root = spaceDirectory(canvasId); // Accept the clean virtual prefixes (`upload/`, `artifacts/`) as aliases // for their hidden on-disk dirs so agents can reference either form. const target = path.resolve(root, toPhysicalRel(rel)); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts index 644fb0e64..5f17d9ae1 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.test.ts @@ -30,7 +30,7 @@ import { canvasMemoryPath, userSkillsDir, workspaceMemoryPath, -} from '../../../storage/paths.js'; +} from '../../../workspace/paths.js'; import { setWorkspacePath } from '../../../workspace.js'; interface ParsedResult { @@ -51,7 +51,8 @@ beforeEach(() => { setWorkspacePath(tmp); // `canvasRoot(canvasId)` falls back to `/` when the // canvas-dir index has no entry for the id (see `canvasDirName` in - // `workspace/disk/canvas-dirs.ts`). We just need the directory to exist so + // `storage/backends/disk/canvas-dirs.ts`). We just need the directory to + // exist so // writes can land in it. mkdirSync(join(tmp, canvasId), { recursive: true }); }); diff --git a/apps/server/src/modules/agent/tools/handlers/fs-write.ts b/apps/server/src/modules/agent/tools/handlers/fs-write.ts index f9413c225..501d8527a 100644 --- a/apps/server/src/modules/agent/tools/handlers/fs-write.ts +++ b/apps/server/src/modules/agent/tools/handlers/fs-write.ts @@ -36,7 +36,7 @@ import { canvasMemoryDir, settingDir, userSkillsDir, -} from '../../../storage/paths.js'; +} from '../../../workspace/paths.js'; import { resolveLongTermPath, resolveUserSkillPath, diff --git a/apps/server/src/modules/canvas/canvas-content-cas.test.ts b/apps/server/src/modules/canvas/canvas-content-cas.test.ts index eab47243b..bb0b55b5f 100644 --- a/apps/server/src/modules/canvas/canvas-content-cas.test.ts +++ b/apps/server/src/modules/canvas/canvas-content-cas.test.ts @@ -30,7 +30,7 @@ import { getStorage, setStorageForTesting, } from '../storage/index.js'; -import { nodesDir } from '../workspace/disk/paths.js'; +import { nodesDir } from '../storage/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { BlobScope, BlobStore } from '../storage/index.js'; diff --git a/apps/server/src/modules/canvas/canvas-executor.test.ts b/apps/server/src/modules/canvas/canvas-executor.test.ts index dd7e50c8a..8bf897503 100644 --- a/apps/server/src/modules/canvas/canvas-executor.test.ts +++ b/apps/server/src/modules/canvas/canvas-executor.test.ts @@ -242,7 +242,7 @@ describe('executeOnServer — MERGE_NODE_DATA CAS', () => { expect(out.conflicts ?? []).toHaveLength(0); expect(out.toVersion).toBe(out.fromVersion + 1); - expect(getCanvasStore('c1').readNode('m1')?.src).toBe('artifacts/new.png'); + expect(getCanvasStore('c1').readNode('m1')?.src).toBe('new.png'); }); it('auto-updates image height when MERGE_NODE_DATA rewrites src', async () => { diff --git a/apps/server/src/modules/canvas/canvas-search.test.ts b/apps/server/src/modules/canvas/canvas-search.test.ts index cf878d9cd..9677f057b 100644 --- a/apps/server/src/modules/canvas/canvas-search.test.ts +++ b/apps/server/src/modules/canvas/canvas-search.test.ts @@ -43,7 +43,7 @@ vi.mock('../agent/agenetes/drivers.js', () => ({ }, })); -vi.mock('../storage/paths.js', async (importActual) => ({ +vi.mock('../workspace/paths.js', async (importActual) => ({ ...((await importActual()) as Record), canvasAcpNamespace: (canvasId: string) => ({ name: canvasId, root: '' }), })); diff --git a/apps/server/src/modules/canvas/canvas-search.ts b/apps/server/src/modules/canvas/canvas-search.ts index a3eab3ab0..17e0754d6 100644 --- a/apps/server/src/modules/canvas/canvas-search.ts +++ b/apps/server/src/modules/canvas/canvas-search.ts @@ -44,7 +44,7 @@ import { import { agenetes } from '../agent/agenetes/drivers.js'; import { chatEnvelopeFromSubmission } from '../agent/agenetes/handle.js'; -import { canvasAcpNamespace } from '../storage/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; import type { AgentTurn } from '@agenetes/protocol'; diff --git a/apps/server/src/modules/canvas/canvas.route.test.ts b/apps/server/src/modules/canvas/canvas.route.test.ts index 1620b1d90..bbfeac7a8 100644 --- a/apps/server/src/modules/canvas/canvas.route.test.ts +++ b/apps/server/src/modules/canvas/canvas.route.test.ts @@ -21,15 +21,22 @@ vi.mock('../storage/index.js', async (importOriginal) => { }; }); -vi.mock('../workspace/disk/space-dir-handles.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - withSpaceDirHandlesReleased: vi.fn(actual.withSpaceDirHandlesReleased), - }; -}); +// Mocked at the module the Disk repository imports, not at the facade: these +// cases force a Space-directory rename to fail, which is Disk behavior, and a +// facade mock would not intercept the adapter's own import. +vi.mock( + '../storage/backends/disk/space-dir-handles.js', + async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + withSpaceDirHandlesReleased: vi.fn(actual.withSpaceDirHandlesReleased), + }; + }, +); import canvasRoutes from './canvas.route.js'; +import { withSpaceDirHandlesReleased } from '../storage/backends/disk/space-dir-handles.js'; import { createCanvas, deleteCanvas } from '../storage/compatibility/canvas.js'; import { canvasBlobs, @@ -37,12 +44,11 @@ import { getStructuredStore, resetStorageCache, } from '../storage/index.js'; -import { changesPath } from '../workspace/disk/paths.js'; -import { withSpaceDirHandlesReleased } from '../workspace/disk/space-dir-handles.js'; +import { changesPath } from '../storage/paths.js'; import { setWorkspacePath } from '../workspace.js'; +import type * as SpaceDirHandlesModule from '../storage/backends/disk/space-dir-handles.js'; import type * as StorageModule from '../storage/index.js'; -import type * as SpaceDirHandlesModule from '../workspace/disk/space-dir-handles.js'; import type { RecentAction } from '@huabu/shared'; let tmp: string; diff --git a/apps/server/src/modules/canvas/canvas.route.ts b/apps/server/src/modules/canvas/canvas.route.ts index 09948f8aa..b6ffedde6 100644 --- a/apps/server/src/modules/canvas/canvas.route.ts +++ b/apps/server/src/modules/canvas/canvas.route.ts @@ -43,6 +43,7 @@ import { WorldReferenceResolutionError, } from './world-reference-resolver.js'; import { MAX_UPLOAD_BYTES } from '../../upload-limits.js'; +import { toSafeFilename } from '../../utils/naming.js'; import { ARTIFACT_URL_REGEX } from '../artifact/utils.js'; import { getPreprocessDispatcher, getProfile } from '../preprocessing/index.js'; import { stripOfficeparserPreamble } from '../preprocessing/loaders/office-strip.js'; @@ -58,12 +59,12 @@ import { deleteSpace, getCanvasStore, getStructuredStore, - updateNode, + spaceDirectory, type CanvasFile, type UpdateNodeOutcome, + updateNode, } from '../storage/index.js'; -import { canvasRoot, nodesDir, SPACE_JSON_FILENAME } from '../storage/paths.js'; -import { toSafeFilename } from '../workspace/disk/naming.js'; +import { nodesDir, SPACE_JSON_FILENAME } from '../storage/paths.js'; import { getWorkspacePath } from '../workspace.js'; import type { CanvasStore, NodeContent } from '../storage/canvas-store.js'; @@ -1609,7 +1610,7 @@ const canvasRoutes: FastifyPluginAsync = async (fastify) => { return reply.code(404).send({ message: 'Canvas not found' }); } - const canvasDir = canvasRoot(canvasId); + const canvasDir = spaceDirectory(canvasId); if (!existsSync(canvasDir)) { return reply.code(404).send({ message: 'Canvas directory not found' }); } diff --git a/apps/server/src/modules/canvas/external-watcher.test.ts b/apps/server/src/modules/canvas/external-watcher.test.ts index d679ed043..cf3de2474 100644 --- a/apps/server/src/modules/canvas/external-watcher.test.ts +++ b/apps/server/src/modules/canvas/external-watcher.test.ts @@ -72,9 +72,18 @@ const canvasStore = vi.hoisted(() => ({ read: vi.fn(() => ({ state: { nodes: [] } })), })); -vi.mock('../storage/index.js', () => ({ - getCanvasStore: () => canvasStore, -})); +// The facade is stubbed for the store, but the handle helpers must stay the +// real ones: these cases drive `withSpaceDirHandlesReleased` and assert the +// watcher released its handles, which only works if both sides share the one +// module instance that holds the registry. +vi.mock('../storage/index.js', async () => { + const handles = await import('../storage/backends/disk/space-dir-handles.js'); + return { + getCanvasStore: () => canvasStore, + registerSpaceDirHandleOwner: handles.registerSpaceDirHandleOwner, + withSpaceDirHandlesReleased: handles.withSpaceDirHandlesReleased, + }; +}); function makeFakeNativeWatcher() { const nativeWatcher = { @@ -96,7 +105,7 @@ import { openExternalNoteSession, resetExternalNoteSessions, } from './external-watcher.js'; -import { withSpaceDirHandlesReleased } from '../workspace/disk/space-dir-handles.js'; +import { withSpaceDirHandlesReleased } from '../storage/index.js'; import type { ExternalNoteEvent } from '@huabu/shared'; diff --git a/apps/server/src/modules/canvas/external-watcher.ts b/apps/server/src/modules/canvas/external-watcher.ts index 38048184a..fc37894a7 100644 --- a/apps/server/src/modules/canvas/external-watcher.ts +++ b/apps/server/src/modules/canvas/external-watcher.ts @@ -34,8 +34,8 @@ import { getLogger } from '../../utils/logger.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; import { listAllCanvasDirEntries } from '../storage/canvas-dirs.js'; import { getCanvasStore } from '../storage/index.js'; +import { registerSpaceDirHandleOwner } from '../storage/index.js'; import { SPACE_JSON_FILENAME } from '../storage/paths.js'; -import { registerSpaceDirHandleOwner } from '../workspace/disk/space-dir-handles.js'; import { getWorkspacePath, isWorkspaceConfigured } from '../workspace.js'; import type { CanvasFile } from '../storage/index.js'; diff --git a/apps/server/src/modules/canvas/external.route.ts b/apps/server/src/modules/canvas/external.route.ts index 89e229f18..0984daa1c 100644 --- a/apps/server/src/modules/canvas/external.route.ts +++ b/apps/server/src/modules/canvas/external.route.ts @@ -17,7 +17,7 @@ import { takeExternalNote, } from './external-watcher.js'; import { parseFrontmatter } from '../../utils/markdown-frontmatter.js'; -import { canvasRoot } from '../storage/paths.js'; +import { spaceDirectory } from '../storage/index.js'; import type { FastifyPluginAsync } from 'fastify'; @@ -95,7 +95,7 @@ const externalRoutes: FastifyPluginAsync = async (fastify): Promise => { return reply.code(404).send({ message: 'External note not found' }); } - const abs = path.join(canvasRoot(canvasId), item.relativePath); + const abs = path.join(spaceDirectory(canvasId), item.relativePath); let raw: string; try { raw = await readFile(abs, 'utf8'); diff --git a/apps/server/src/modules/canvas/import-node-src.test.ts b/apps/server/src/modules/canvas/import-node-src.test.ts index d24500a5a..089ad92ac 100644 --- a/apps/server/src/modules/canvas/import-node-src.test.ts +++ b/apps/server/src/modules/canvas/import-node-src.test.ts @@ -15,8 +15,11 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { importForeignNodeSources } from './import-node-src.js'; import { createCanvas } from '../storage/compatibility/canvas.js'; -import { canvasBlobs, getCanvasStore } from '../storage/index.js'; -import { canvasRoot } from '../storage/paths.js'; +import { + canvasBlobs, + getCanvasStore, + spaceDirectory, +} from '../storage/index.js'; import { setWorkspacePath } from '../workspace.js'; import type { CanvasCommand } from '@huabu/shared'; @@ -34,6 +37,7 @@ beforeEach(() => { 'c-web-merge-local', 'c-web-merge-remote', 'c-image-local', + 'c-image-reentered', ]) { createCanvas(canvasId); } @@ -45,7 +49,7 @@ afterEach(() => { /** Stage a file under the canvas's hidden `.upload/` scratch dir. */ function stageUpload(canvasId: string, name: string, body: string): string { - const uploadDir = path.join(canvasRoot(canvasId), '.upload'); + const uploadDir = path.join(spaceDirectory(canvasId), '.upload'); mkdirSync(uploadDir, { recursive: true }); const abs = path.join(uploadDir, name); writeFileSync(abs, body); @@ -266,4 +270,36 @@ describe('importForeignNodeSources — media nodes (regression)', () => { if (src === undefined) throw new Error('Expected a rewritten image src'); expect(await canvasBlobs(canvasId).head(src)).not.toBeNull(); }); + + it('canonicalizes an artifact path that leaves and re-enters the Space', async () => { + const canvasId = 'c-image-reentered'; + const store = getCanvasStore(canvasId); + const spaceDir = spaceDirectory(canvasId); + const artifactsDir = path.join(spaceDir, '.artifacts'); + mkdirSync(artifactsDir, { recursive: true }); + writeFileSync(path.join(artifactsDir, 'pic.png'), 'existing artifact'); + const src = path.join( + '..', + path.basename(spaceDir), + '.artifacts', + 'pic.png', + ); + + const commands: CanvasCommand[] = [ + { + type: 'CREATE_NODES', + nodes: [ + { + nodeType: 'image', + data: { src }, + position: { x: 0, y: 0 }, + }, + ], + }, + ]; + + const out = await importForeignNodeSources(store, canvasId, commands); + + expect(firstSrc(out)).toBe('pic.png'); + }); }); diff --git a/apps/server/src/modules/canvas/import-node-src.ts b/apps/server/src/modules/canvas/import-node-src.ts index e960dafaf..39837f701 100644 --- a/apps/server/src/modules/canvas/import-node-src.ts +++ b/apps/server/src/modules/canvas/import-node-src.ts @@ -38,10 +38,10 @@ import { import { getLogger } from '../../utils/logger.js'; import { safeResolve, + isArtifactsRel, toPhysicalRel, } from '../agent/tools/handlers/fs-sandbox.js'; -import { canvasBlobs } from '../storage/index.js'; -import { artifactsDir } from '../storage/paths.js'; +import { canvasBlobs, spaceDirectory } from '../storage/index.js'; import type { CanvasStore } from '../storage/index.js'; @@ -266,16 +266,20 @@ async function resolveImportedSrc( return null; } - // Already inside `.artifacts/` (or a bare artifact key that resolves there) - // — nothing to import. This inspects the real local filesystem, as the - // whole local-import branch does; only the write below goes through the - // blob port. - const artifactsRoot = artifactsDir(canvasId); - if ( - absPath === artifactsRoot || - absPath.startsWith(artifactsRoot + path.sep) - ) { - return null; + // A direct artifact child needs no copy, but it still needs the canonical + // bare-key spelling the web resolver serves. Classify the path after + // sandbox resolution so a ref that leaves and re-enters the current Space + // is judged by where it actually lands, while the helper still owns the + // virtual/physical `.artifacts` vocabulary. A nested path is not a blob key, + // so it falls through and is copied into the artifact root below. + const resolvedPhysicalRel = path.relative(spaceDirectory(canvasId), absPath); + if (isArtifactsRel(resolvedPhysicalRel)) { + const key = path.basename(absPath); + const canonicalPath = safeResolve( + canvasId, + toPhysicalRel(`artifacts/${key}`), + ); + if (absPath === canonicalPath) return key; } // A bare key like `art_abc.png` resolves under the canvas root but has no diff --git a/apps/server/src/modules/preprocessing/stages/project.ts b/apps/server/src/modules/preprocessing/stages/project.ts index c40a58321..9c96f311e 100644 --- a/apps/server/src/modules/preprocessing/stages/project.ts +++ b/apps/server/src/modules/preprocessing/stages/project.ts @@ -8,7 +8,7 @@ * from the outputs of all previous stages. */ -import { normalizeForCompare } from '../../workspace/disk/naming.js'; +import { normalizeForCompare } from '../../../utils/naming.js'; import { isLabelProtected } from '../label-policy.js'; import type { diff --git a/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts b/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts index a936d377d..934ca5e9c 100644 --- a/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts +++ b/apps/server/src/modules/remote_fs/interactive-view.rfs.test.ts @@ -22,7 +22,7 @@ import { getCanvasStore, resetStorageCache, } from '../storage/index.js'; -import { canvasAcpNamespace } from '../workspace/disk/paths.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; import { setWorkspacePath } from '../workspace.js'; let workspace: string; diff --git a/apps/server/src/modules/remote_fs/rfs.route.test.ts b/apps/server/src/modules/remote_fs/rfs.route.test.ts index f85b5c789..7fcd331b3 100644 --- a/apps/server/src/modules/remote_fs/rfs.route.test.ts +++ b/apps/server/src/modules/remote_fs/rfs.route.test.ts @@ -48,6 +48,7 @@ vi.mock('../agent/agenetes/drivers.js', () => ({ })); import rfsRoutes from './rfs.route.js'; +import { toSafeFilename } from '../../utils/naming.js'; import { agentNodeService } from '../agent/agent-node.service.js'; import { agentThreadResolver } from '../agent/agent-thread-resolver.js'; import { @@ -55,15 +56,17 @@ import { agentThreadService, } from '../agent/agent-thread.service.js'; import * as selectableProfiles from '../agent/selectable-agent-profile.js'; -import { getCanvasStore, resetStorageCache } from '../storage/index.js'; +import { + getCanvasStore, + resetStorageCache, + spaceDirectory, +} from '../storage/index.js'; import { RunCompletionError, runCompletionService, } from '../task/run-completion.service.js'; import { RunLaunchError, runLauncher } from '../task/run-launcher.js'; import { taskService } from '../task/task.service.js'; -import { toSafeFilename } from '../workspace/disk/naming.js'; -import { canvasRoot } from '../workspace/disk/paths.js'; import { setWorkspacePath } from '../workspace.js'; import type { FixedAgentNodeTarget } from '../agent/agent-thread-resolver.js'; @@ -156,7 +159,7 @@ describe('GET /api/rfs/:canvasId/skill', () => { it('returns only the bundled root guide without authorization', async () => { seedNote('c1', 'node-1', 'Anchor', 'content'); writeFileSync( - join(canvasRoot('c1'), 'skill.md'), + join(spaceDirectory('c1'), 'skill.md'), '# Private Space Override', 'utf8', ); diff --git a/apps/server/src/modules/remote_fs/skill.ts b/apps/server/src/modules/remote_fs/skill.ts index 45c8985fe..7dcbb5c97 100644 --- a/apps/server/src/modules/remote_fs/skill.ts +++ b/apps/server/src/modules/remote_fs/skill.ts @@ -15,7 +15,7 @@ import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; import { renderPromptFile } from '../../prompt/agents/loader.js'; -import { canvasRoot } from '../storage/paths.js'; +import { spaceDirectory } from '../storage/index.js'; /** PROMPT-ROOT-relative path of the bundled access guide. */ const ACCESS_GUIDE_TEMPLATE = 'external-agent/access-huabu.md'; @@ -40,7 +40,7 @@ export function resolveBundledRootSkill(): string { * markdown text (served with `Content-Type: text/markdown`). */ export function resolveCanvasSkill(canvasId: string): string { - const override = path.join(canvasRoot(canvasId), 'skill.md'); + const override = path.join(spaceDirectory(canvasId), 'skill.md'); if (existsSync(override)) { return readFileSync(override, 'utf8'); } diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 50919956b..88dd8eb6d 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -26,8 +26,8 @@ import { import path from 'node:path'; import { pipeline } from 'node:stream/promises'; +import { artifactsDir } from './layout.js'; import { renameOverWithRetry } from '../../../../utils/fs.js'; -import { artifactsDir } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { createBlobLease, normalizeBlobName } from '../../ports/blob.js'; diff --git a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts index e683c67b0..41903fb0a 100644 --- a/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts +++ b/apps/server/src/modules/storage/backends/disk/cache-boundaries.test.ts @@ -14,6 +14,8 @@ import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex, registerCanvasDir } from './canvas-dirs.js'; +import { canvasRoot, SPACE_JSON_FILENAME } from './layout.js'; import { forgetCanvasStore, getCanvasStore, @@ -21,15 +23,7 @@ import { } from './legacy/canvas-store-cache.js'; import { NODE_TOMBSTONE_TTL_MS } from './legacy/node-tombstones.js'; import { DiskStructuredStore } from './structured-store.js'; -import { - refreshCanvasDirIndex, - registerCanvasDir, -} from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; -import { - canvasRoot, - SPACE_JSON_FILENAME, -} from '../../../workspace/disk/paths.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { setWorkspacePath } from '../../../workspace.js'; import type { diff --git a/apps/server/src/modules/workspace/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts similarity index 96% rename from apps/server/src/modules/workspace/disk/canvas-dirs.ts rename to apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index 88aa94775..1780e1065 100644 --- a/apps/server/src/modules/workspace/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -10,11 +10,15 @@ import { existsSync, readdirSync, renameSync, statSync } from 'node:fs'; import path from 'node:path'; +import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; import { NameIndex, type NameIndexResult } from './name-index.js'; -import { dedupeName, normalizeForCompare, toSafeFilename } from './naming.js'; -import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './paths.js'; -import { readJsonStrict, sanitizeId } from '../../../utils/fs.js'; -import { getWorkspacePath } from '../../workspace.js'; +import { readJsonStrict, sanitizeId } from '../../../../utils/fs.js'; +import { + dedupeName, + normalizeForCompare, + toSafeFilename, +} from '../../../../utils/naming.js'; +import { getWorkspacePath } from '../../../workspace.js'; export interface CanvasDirEntry { id: string; diff --git a/apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts similarity index 97% rename from apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts rename to apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts index 1f54b9666..e71c32fd4 100644 --- a/apps/server/src/modules/workspace/disk/canvas-dirs.world.test.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts @@ -15,7 +15,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const workspaceState = vi.hoisted(() => ({ path: '' })); -vi.mock('../../workspace.js', () => ({ +vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); @@ -29,7 +29,7 @@ import { renameCanvasDirOnDisk, suggestCanvasDir, } from './canvas-dirs.js'; -import { CanvasStore } from '../../storage/index.js'; +import { CanvasStore } from '../../index.js'; function writeCanvas( root: string, diff --git a/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts b/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts index 098377144..064af388d 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-persistence-transaction.ts @@ -27,13 +27,9 @@ import { } from 'node:fs'; import path from 'node:path'; +import { canvasJsonPath, deltaLogPath, nodesDir } from './layout.js'; import { repairJsonLinesTail } from '../../../../utils/fs.js'; import { parseFrontmatter } from '../../../../utils/markdown-frontmatter.js'; -import { - canvasJsonPath, - deltaLogPath, - nodesDir, -} from '../../../workspace/disk/paths.js'; interface FileSnapshot { path: string; diff --git a/apps/server/src/modules/workspace/disk/paths.test.ts b/apps/server/src/modules/storage/backends/disk/layout.test.ts similarity index 70% rename from apps/server/src/modules/workspace/disk/paths.test.ts rename to apps/server/src/modules/storage/backends/disk/layout.test.ts index 9f39762f5..711784c48 100644 --- a/apps/server/src/modules/workspace/disk/paths.test.ts +++ b/apps/server/src/modules/storage/backends/disk/layout.test.ts @@ -7,11 +7,11 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { refreshCanvasDirIndex } from './canvas-dirs.js'; -import { canvasRoot } from './paths.js'; -import { setWorkspacePath } from '../../workspace.js'; +import { refreshCanvasDirIndex, registerCanvasDir } from './canvas-dirs.js'; +import { canvasRoot } from './layout.js'; +import { setWorkspacePath } from '../../../workspace.js'; -describe('Disk Workspace paths', () => { +describe('Disk layout', () => { let workspacePath: string; beforeEach(() => { @@ -38,4 +38,12 @@ describe('Disk Workspace paths', () => { expect(() => canvasRoot(canvasId)).toThrow(/Invalid canvasId/); }, ); + + it('rejects an indexed directory that escapes the active Workspace', () => { + registerCanvasDir('canvas-a', '../escape', null); + + expect(() => canvasRoot('canvas-a')).toThrow( + /escapes the active Workspace/, + ); + }); }); diff --git a/apps/server/src/modules/storage/backends/disk/layout.ts b/apps/server/src/modules/storage/backends/disk/layout.ts new file mode 100644 index 000000000..fe2a400ac --- /dev/null +++ b/apps/server/src/modules/storage/backends/disk/layout.ts @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Where the Disk backend puts a Space. + * + * Every path here answers "how does *this* backend store that", so none of it + * survives a switch to a structured backend that keeps the same state in + * tables — which is the test that moved it inside the storage boundary + * (proposal §12.5.2). Nothing outside `storage/` may depend on these names. + * + * Layout under `//`: + * + * space.json topology; carries the stable canvasId + * nodes/.md per-node markdown (id in frontmatter) + * .artifacts/ raw uploads (hidden dir) + * .history/ + * chat/.changes.json pending change-review records + * events.jsonl + * tasks.json + * delta-log.jsonl + * + * `.history/` also hosts state this backend does not own — ACP sessions and + * the debug prompt log — which the agent domain addresses through the + * materialization capability instead (§12.5.3). + */ + +import path from 'node:path'; + +import { canvasDirName } from './canvas-dirs.js'; +import { sanitizeId } from '../../../../utils/fs.js'; +import { getWorkspacePath } from '../../../workspace.js'; + +/** + * The directory backing a Space. + * + * Resolved through {@link canvasDirName} rather than the canvasId, because + * Disk files a Space under its title and that name moves on rename. This is + * also the materialization anchor the rest of the app reaches by way of + * `storage`'s `spaceDirectory()`. + */ +export function canvasRoot(canvasId: string): string { + const safeId = sanitizeId(canvasId, 'canvasId'); + const workspaceRoot = path.resolve(getWorkspacePath()); + const resolved = path.resolve(workspaceRoot, canvasDirName(safeId)); + if (!resolved.startsWith(`${workspaceRoot}${path.sep}`)) { + throw new Error(`Canvas path escapes the active Workspace: "${canvasId}"`); + } + return resolved; +} + +/** + * On-disk topology filename. Agent- and user-visible (L1), so it uses the + * Space vocabulary; the TypeScript type of its contents stays `CanvasFile` + * (L2 internal). See migrate-canvas-to-space.ts for the legacy rename. + */ +export const SPACE_JSON_FILENAME = 'space.json'; +export const WORLD_CANVAS_DIR_NAME = '.world'; + +export function canvasJsonPath(canvasId: string): string { + return path.join(canvasRoot(canvasId), SPACE_JSON_FILENAME); +} + +export function nodesDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), 'nodes'); +} + +export function nodeFilePath(canvasId: string, filename: string): string { + const base = path.basename(filename); + if (!base || base === '.' || base === '..') { + throw new Error(`Invalid node filename: "${filename}"`); + } + return path.join(nodesDir(canvasId), base); +} + +/** Hidden directory holding raw uploaded files keyed by artifactId. */ +export const ARTIFACTS_DIR_NAME = '.artifacts'; + +export function artifactsDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), ARTIFACTS_DIR_NAME); +} + +export function artifactPath(canvasId: string, filename: string): string { + const base = path.basename(filename); + if (!base || base === '.' || base === '..') { + throw new Error(`Invalid artifact filename: "${filename}"`); + } + return path.join(artifactsDir(canvasId), base); +} + +/** + * The hidden per-Space tier. Shared with non-storage owners today; see the + * module note above and §12.5.3. + */ +export const HISTORY_DIR_NAME = '.history'; + +export function historyDir(canvasId: string): string { + return path.join(canvasRoot(canvasId), HISTORY_DIR_NAME); +} + +export function chatDir(canvasId: string): string { + return path.join(historyDir(canvasId), 'chat'); +} + +/** + * Pending change-review records for an ACP thread (the "what the agent + * changed" card). A mutable sidecar — entries are removed on accept / + * revert — so it lives apart from the append-only `.turns.jsonl` log. + */ +export function changesPath(canvasId: string, threadId: string): string { + return path.join( + chatDir(canvasId), + `${sanitizeId(threadId, 'threadId')}.changes.json`, + ); +} + +export function tasksPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'tasks.json'); +} + +export function eventsPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'events.jsonl'); +} + +/** + * Append-only delta log for headless executor batches (M2). + * + * One JSONL line per `POST /api/canvas/:canvasId/execute` call that + * actually mutated state. Lines carry the canvas version, run id, + * originator, applied commands, and the resulting structural deltas + * (see `shared/canvas-engine/delta.ts`). Used by M3 broadcast / replay + * and as the persistence anchor for `space.json`'s monotonic version + * counter. + * + * Lives next to `events.jsonl` so the entire `.history/` tier travels + * together in canvas export bundles. + */ +export function deltaLogPath(canvasId: string): string { + return path.join(historyDir(canvasId), 'delta-log.jsonl'); +} diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts index 96bcb0582..5a4ee1d61 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store-cache.ts @@ -24,8 +24,8 @@ import path from 'node:path'; import { CanvasStore } from './canvas-store.js'; import { sanitizeId } from '../../../../../utils/fs.js'; -import { refreshCanvasDirIndex } from '../../../../workspace/disk/canvas-dirs.js'; import { getWorkspacePath } from '../../../../workspace.js'; +import { refreshCanvasDirIndex } from '../canvas-dirs.js'; const MAX_CACHE = 16; const cache = new Map(); diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts index 483a31f19..d020fd9bd 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts @@ -43,6 +43,9 @@ import { parseFrontmatter, toFrontmatter, } from '../../../../../utils/markdown-frontmatter.js'; +import { toSafeFilename } from '../../../../../utils/naming.js'; +import { getWorkspacePath } from '../../../../workspace.js'; +import { assertSpaceMutationAllowed } from '../../../space-lifecycle-admission.js'; import { patchCanvasDirTitle, refreshCanvasDirIndex, @@ -50,9 +53,7 @@ import { renameCanvasDirOnDisk, isWorldCanvasId, unregisterCanvasDir, -} from '../../../../workspace/disk/canvas-dirs.js'; -import { NameIndex } from '../../../../workspace/disk/name-index.js'; -import { toSafeFilename } from '../../../../workspace/disk/naming.js'; +} from '../canvas-dirs.js'; import { canvasJsonPath, canvasRoot, @@ -62,9 +63,8 @@ import { eventsPath, nodeFilePath, nodesDir, -} from '../../../../workspace/disk/paths.js'; -import { getWorkspacePath } from '../../../../workspace.js'; -import { assertSpaceMutationAllowed } from '../../../space-lifecycle-admission.js'; +} from '../layout.js'; +import { NameIndex } from '../name-index.js'; import { readValidCanvasFile } from '../space-record-validation.js'; import { titleVisibleAtDirectory } from '../space-title.js'; diff --git a/apps/server/src/modules/workspace/disk/name-index.ts b/apps/server/src/modules/storage/backends/disk/name-index.ts similarity index 99% rename from apps/server/src/modules/workspace/disk/name-index.ts rename to apps/server/src/modules/storage/backends/disk/name-index.ts index 099e76a02..672bc51da 100644 --- a/apps/server/src/modules/workspace/disk/name-index.ts +++ b/apps/server/src/modules/storage/backends/disk/name-index.ts @@ -19,7 +19,7 @@ import { dedupeArtifactFilename, dedupeName, normalizeForCompare, -} from './naming.js'; +} from '../../../../utils/naming.js'; export interface NameIndexEntry { /** Stable identifier — never written to disk as a filename. */ diff --git a/apps/server/src/modules/workspace/disk/space-dir-handles.ts b/apps/server/src/modules/storage/backends/disk/space-dir-handles.ts similarity index 100% rename from apps/server/src/modules/workspace/disk/space-dir-handles.ts rename to apps/server/src/modules/storage/backends/disk/space-dir-handles.ts diff --git a/apps/server/src/modules/storage/backends/disk/space-logs.ts b/apps/server/src/modules/storage/backends/disk/space-logs.ts index f6a93262f..322966b84 100644 --- a/apps/server/src/modules/storage/backends/disk/space-logs.ts +++ b/apps/server/src/modules/storage/backends/disk/space-logs.ts @@ -26,13 +26,13 @@ import { type CanvasChangeRecord, } from '@huabu/shared/canvas-engine'; +import { changesPath, eventsPath } from './layout.js'; import { readDiskSpaceRecord } from './space-record.js'; import { atomicWriteJson, readJsonLinesStrict, readJsonStrict, } from '../../../../utils/fs.js'; -import { changesPath, eventsPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts index 80af0b56d..b688e8fbb 100644 --- a/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-nodes.test.ts @@ -16,6 +16,8 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { nodesDir } from './layout.js'; import { getCanvasStore, resetStorageCache, @@ -23,9 +25,7 @@ import { import { DiskSpaceNodes } from './space-nodes.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { nodesDir } from '../../../workspace/disk/paths.js'; -import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceNodesContract } from '../../ports/contracts/space-nodes.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-record.ts b/apps/server/src/modules/storage/backends/disk/space-record.ts index 5fe26384c..3a1737129 100644 --- a/apps/server/src/modules/storage/backends/disk/space-record.ts +++ b/apps/server/src/modules/storage/backends/disk/space-record.ts @@ -11,9 +11,9 @@ import path from 'node:path'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { canvasJsonPath } from './layout.js'; import { readValidCanvasFile } from './space-record-validation.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { canvasJsonPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import type { CanvasStore } from './legacy/canvas-store.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts index d8dd7b3ae..dd7ab747d 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts @@ -21,12 +21,12 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { WORLD_CANVAS_DIR_NAME } from './layout.js'; import { resetStorageCache } from './legacy/canvas-store-cache.js'; import { DiskSpaceRepository } from './space-repository.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; -import { WORLD_CANVAS_DIR_NAME } from '../../../workspace/disk/paths.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { describeSpaceRepositoryContract } from '../../ports/contracts/space-repository.contract.js'; import type { CanvasFile } from '../../../canvas/persistence-types.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.ts b/apps/server/src/modules/storage/backends/disk/space-repository.ts index 29e242313..e59b78fd2 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -15,10 +15,21 @@ import path from 'node:path'; +import { + isWorldCanvasId, + listAllCanvasDirEntries, + listCanvasDirEntries, + refreshCanvasDirIndex, + registerCanvasDir, + requireWorldCanvasId, + suggestCanvasDir, +} from './canvas-dirs.js'; +import { canvasJsonPath, SPACE_JSON_FILENAME } from './layout.js'; import { forgetCanvasStore, getCanvasStore, } from './legacy/canvas-store-cache.js'; +import { withSpaceDirHandlesReleased } from './space-dir-handles.js'; import { readValidCanvasFile } from './space-record-validation.js'; import { readDiskSpaceRecord } from './space-record.js'; import { @@ -26,21 +37,7 @@ import { titleVisibleAtDirectory, } from './space-title.js'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../../utils/fs.js'; -import { - isWorldCanvasId, - listAllCanvasDirEntries, - listCanvasDirEntries, - refreshCanvasDirIndex, - registerCanvasDir, - requireWorldCanvasId, - suggestCanvasDir, -} from '../../../workspace/disk/canvas-dirs.js'; -import { normalizeForCompare } from '../../../workspace/disk/naming.js'; -import { - canvasJsonPath, - SPACE_JSON_FILENAME, -} from '../../../workspace/disk/paths.js'; -import { withSpaceDirHandlesReleased } from '../../../workspace/disk/space-dir-handles.js'; +import { normalizeForCompare } from '../../../../utils/naming.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed, diff --git a/apps/server/src/modules/storage/backends/disk/space-tasks.ts b/apps/server/src/modules/storage/backends/disk/space-tasks.ts index 2aa7bded4..35e91e34e 100644 --- a/apps/server/src/modules/storage/backends/disk/space-tasks.ts +++ b/apps/server/src/modules/storage/backends/disk/space-tasks.ts @@ -14,9 +14,9 @@ import { type TaskStoreSnapshot, } from '@huabu/shared'; +import { tasksPath } from './layout.js'; import { readDiskSpaceRecord } from './space-record.js'; import { atomicWriteJson, readJsonStrict } from '../../../../utils/fs.js'; -import { tasksPath } from '../../../workspace/disk/paths.js'; import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; diff --git a/apps/server/src/modules/storage/backends/disk/space-title.ts b/apps/server/src/modules/storage/backends/disk/space-title.ts index fba4a023e..1cfffe995 100644 --- a/apps/server/src/modules/storage/backends/disk/space-title.ts +++ b/apps/server/src/modules/storage/backends/disk/space-title.ts @@ -6,7 +6,7 @@ import { normalizeForCompare, toSafeFilename, -} from '../../../workspace/disk/naming.js'; +} from '../../../../utils/naming.js'; /** * Whether `filename` is `base` carrying an allocation suffix (` (2)`, ` (3)`). diff --git a/apps/server/src/modules/storage/backends/disk/space-write.test.ts b/apps/server/src/modules/storage/backends/disk/space-write.test.ts index 7c2c7155f..2707301f0 100644 --- a/apps/server/src/modules/storage/backends/disk/space-write.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-write.test.ts @@ -13,6 +13,8 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { nodesDir } from './layout.js'; import { getCanvasStore, resetStorageCache, @@ -20,9 +22,7 @@ import { import { DiskSpaceRepository } from './space-repository.js'; import { createDiskSpaceWrite } from './space-write.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { nodesDir } from '../../../workspace/disk/paths.js'; -import { ensureWorldCanvasOnDisk } from '../../../workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './world-canvas.js'; import { setWorkspacePath } from '../../../workspace.js'; import { describeSpaceWriteContract } from '../../ports/contracts/space-write.contract.js'; diff --git a/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts b/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts index 893c2fbec..26bac81f9 100644 --- a/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts +++ b/apps/server/src/modules/storage/backends/disk/storage-recovery.test.ts @@ -25,13 +25,13 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { changesPath, eventsPath } from './layout.js'; import { getCanvasStore, resetStorageCache, } from './legacy/canvas-store-cache.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { changesPath, eventsPath } from '../../../workspace/disk/paths.js'; import { canvasBlobs, createStorage, diff --git a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts index 8bc6343ef..72278d483 100644 --- a/apps/server/src/modules/storage/backends/disk/structured-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/structured-store.test.ts @@ -13,14 +13,14 @@ vi.mock('../../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); +import { refreshCanvasDirIndex } from './canvas-dirs.js'; +import { tasksPath } from './layout.js'; import { getCanvasStore, resetStorageCache, } from './legacy/canvas-store-cache.js'; import { DiskStructuredStore } from './structured-store.js'; -import { refreshCanvasDirIndex } from '../../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../../workspace/disk/naming.js'; -import { tasksPath } from '../../../workspace/disk/paths.js'; +import { toSafeFilename } from '../../../../utils/naming.js'; import { describeSpaceLogsContract } from '../../ports/contracts/space-logs.contract.js'; import { describeStructuredStoreContract } from '../../ports/contracts/structured-store.contract.js'; diff --git a/apps/server/src/modules/workspace/disk/world-canvas.test.ts b/apps/server/src/modules/storage/backends/disk/world-canvas.test.ts similarity index 100% rename from apps/server/src/modules/workspace/disk/world-canvas.test.ts rename to apps/server/src/modules/storage/backends/disk/world-canvas.test.ts diff --git a/apps/server/src/modules/workspace/disk/world-canvas.ts b/apps/server/src/modules/storage/backends/disk/world-canvas.ts similarity index 91% rename from apps/server/src/modules/workspace/disk/world-canvas.ts rename to apps/server/src/modules/storage/backends/disk/world-canvas.ts index 5982cb842..d3c5395ec 100644 --- a/apps/server/src/modules/workspace/disk/world-canvas.ts +++ b/apps/server/src/modules/storage/backends/disk/world-canvas.ts @@ -6,10 +6,10 @@ import path from 'node:path'; import { createId } from '@huabu/shared'; -import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './paths.js'; -import { atomicWriteJson, readJson, sanitizeId } from '../../../utils/fs.js'; +import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; +import { atomicWriteJson, readJson, sanitizeId } from '../../../../utils/fs.js'; -import type { CanvasFile } from '../../canvas/persistence-types.js'; +import type { CanvasFile } from '../../../canvas/persistence-types.js'; function readWorldCanvas(filePath: string): CanvasFile { const canvas = readJson(filePath); diff --git a/apps/server/src/modules/storage/canvas-dirs.ts b/apps/server/src/modules/storage/canvas-dirs.ts index 3ce194125..cd38b2510 100644 --- a/apps/server/src/modules/storage/canvas-dirs.ts +++ b/apps/server/src/modules/storage/canvas-dirs.ts @@ -2,12 +2,13 @@ // Licensed under the MIT license. /** - * @deprecated Forwarding shim — the Workspace layout owns these now. + * @deprecated Forwarding shim — the Disk backend owns this directory index. * - * Import from `modules/workspace/disk/canvas-dirs.js` instead. This file - * exists only so the many existing physical-Disk capability imports keep - * resolving while they migrate; it must never contain logic, and no new call - * site may import it (enforced by the module-boundary test). + * Inside the storage module, import from + * `storage/backends/disk/canvas-dirs.js`. This file exists only so the + * existing application-level Disk capability imports keep resolving while + * they migrate; it must never contain logic, and no new call site may import + * it (enforced by the module-boundary test). */ -export * from '../workspace/disk/canvas-dirs.js'; +export * from './backends/disk/canvas-dirs.js'; diff --git a/apps/server/src/modules/storage/compatibility/canvas.ts b/apps/server/src/modules/storage/compatibility/canvas.ts index c24a12a0c..87b830510 100644 --- a/apps/server/src/modules/storage/compatibility/canvas.ts +++ b/apps/server/src/modules/storage/compatibility/canvas.ts @@ -21,18 +21,18 @@ import { existsSync } from 'node:fs'; import path from 'node:path'; import { atomicWriteJson, mkdirp, sanitizeId } from '../../../utils/fs.js'; +import { toSafeFilename } from '../../../utils/naming.js'; +import { getWorkspacePath } from '../../workspace.js'; import { listCanvasDirEntries, refreshCanvasDirIndex, registerCanvasDir, suggestCanvasDir, -} from '../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../workspace/disk/naming.js'; +} from '../backends/disk/canvas-dirs.js'; import { canvasJsonPath, SPACE_JSON_FILENAME, -} from '../../workspace/disk/paths.js'; -import { getWorkspacePath } from '../../workspace.js'; +} from '../backends/disk/layout.js'; import { getCanvasStore } from '../backends/disk/legacy/canvas-store-cache.js'; import { deleteSpace } from '../storage.js'; diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index 06db5ea7c..cbcda58cc 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -14,9 +14,9 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { executeOnServer } from '../../canvas/canvas-executor.js'; -import { refreshCanvasDirIndex } from '../../workspace/disk/canvas-dirs.js'; -import { artifactPath, canvasJsonPath } from '../../workspace/disk/paths.js'; import { DiskBlobStore } from '../backends/disk/blob-store.js'; +import { refreshCanvasDirIndex } from '../backends/disk/canvas-dirs.js'; +import { artifactPath, canvasJsonPath } from '../backends/disk/layout.js'; import { resetStorageCache } from '../backends/disk/legacy/canvas-store-cache.js'; import { DiskStructuredStore } from '../backends/disk/structured-store.js'; import { getCanvasStore } from '../index.js'; diff --git a/apps/server/src/modules/storage/compatibility/parity.test.ts b/apps/server/src/modules/storage/compatibility/parity.test.ts index 175bf5919..e0f177db7 100644 --- a/apps/server/src/modules/storage/compatibility/parity.test.ts +++ b/apps/server/src/modules/storage/compatibility/parity.test.ts @@ -25,8 +25,8 @@ vi.mock('../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, })); -import { refreshCanvasDirIndex } from '../../workspace/disk/canvas-dirs.js'; -import { toSafeFilename } from '../../workspace/disk/naming.js'; +import { toSafeFilename } from '../../../utils/naming.js'; +import { refreshCanvasDirIndex } from '../backends/disk/canvas-dirs.js'; import { getCanvasStore, resetStorageCache, diff --git a/apps/server/src/modules/storage/index.ts b/apps/server/src/modules/storage/index.ts index 6663dd732..88d251d1b 100644 --- a/apps/server/src/modules/storage/index.ts +++ b/apps/server/src/modules/storage/index.ts @@ -28,7 +28,24 @@ export { getWorldCanvasId, isWorldCanvasId, requireWorldCanvasId, -} from '../workspace/disk/canvas-dirs.js'; +} from './backends/disk/canvas-dirs.js'; + +/** + * Materialization-tier capabilities, re-exported so consumers that need a + * real Space directory reach them through the facade rather than naming a + * backend (§12.5.4). + * + * Each is Disk-shaped by nature, not by accident: releasing directory handles + * exists so Windows can rename a Space folder, and the World bootstrap writes + * one. A profile that does not materialize Spaces has nothing for either to + * do, which is the gate that keeps them off the portable surface. + */ +export { + registerSpaceDirHandleOwner, + withSpaceDirHandlesReleased, +} from './backends/disk/space-dir-handles.js'; +export type { SpaceDirHandleOwner } from './backends/disk/space-dir-handles.js'; +export { ensureWorldCanvasOnDisk } from './backends/disk/world-canvas.js'; export { withCanvasMutex, updateNode } from '../canvas/write-coordinator.js'; export type { UpdateNodeOptions, @@ -53,6 +70,7 @@ export { getStructuredStore, initStorage, setStorageForTesting, + spaceDirectory, storageHealth, } from './storage.js'; export type { SpaceDeleteOutcome, Storage } from './storage.js'; diff --git a/apps/server/src/modules/storage/module-boundaries.test.ts b/apps/server/src/modules/storage/module-boundaries.test.ts index 560abae88..80fdca34e 100644 --- a/apps/server/src/modules/storage/module-boundaries.test.ts +++ b/apps/server/src/modules/storage/module-boundaries.test.ts @@ -156,6 +156,11 @@ describe('storage dependency direction', () => { const violations: string[] = []; for (const file of sourceFiles) { if (file.startsWith('modules/storage/')) continue; + // Same exemption, and the same reason, as the composition-root rule + // below: exercising an adapter means naming it. A production file that + // names one has bound the application to a backend, which is the thing + // being prevented; a test that names one is choosing its subject. + if (file.endsWith('.test.ts')) continue; for (const spec of specifiersOf(file)) { const target = resolveSpecifier(file, spec); if (target?.includes('modules/storage/backends')) { @@ -185,7 +190,95 @@ describe('storage dependency direction', () => { ); const nonAdapter = importers.filter((f) => !inLayer(f, 'backends')); - expect(nonAdapter).toEqual(['modules/storage/storage.ts']); + // `storage.ts` selects the backend. The rest reach a *named* Disk module + // because the Disk layout and its directory index moved inside the + // boundary in Phase 4.5 (§12.5.2): the barrel re-exports the Disk World + // helpers, the two shims forward Disk-capability imports, and the + // compatibility facade is Disk-coupled by construction. Each entry + // disappears as its consumers move onto ports and the materialization + // capability (§12.5.5 step 5). + expect(nonAdapter).toEqual([ + 'modules/storage/canvas-dirs.ts', + 'modules/storage/compatibility/canvas.ts', + 'modules/storage/index.ts', + 'modules/storage/paths.ts', + 'modules/storage/storage.ts', + ]); + }); +}); + +/** + * Phase 4.5's outcome, guarded (proposal §12.5). + * + * The workspace module used to hold a `disk/` segment containing the Disk + * record layout, the `space.json`-derived directory index, and pure naming + * rules — so "where is a Space" was answered outside the storage boundary, in + * a module whose name asserted the substrate. These pin the correction: what + * remains describes the workspace as a place, and anything needing a real + * Space directory asks for it by capability. + */ +describe('workspace module names no backend', () => { + const workspaceFiles = sourceFiles.filter((f) => + /^modules\/workspace(?:[./-])/.test(f), + ); + + it('has no substrate segment', () => { + const substrate = workspaceFiles.filter((f) => + f.startsWith('modules/workspace/disk/'), + ); + expect(substrate).toEqual([]); + expect(workspaceFiles.length).toBeGreaterThan(0); + }); + + it('never imports a storage backend', () => { + const violations: string[] = []; + for (const file of workspaceFiles) { + for (const spec of specifiersOf(file)) { + const target = resolveSpecifier(file, spec); + if (target?.includes('modules/storage/backends')) { + violations.push(`${file} → ${spec}`); + } + } + } + // A Space's directory comes from `spaceDirectory()` on the facade, which + // is the capability; reaching a backend for it would restore exactly the + // coupling this phase removed. + expect(violations).toEqual([]); + }); + + it('names no Disk record or blob layout symbol', () => { + // These are the members that moved to `backends/disk/layout.ts`. Their + // reappearance here would mean the workspace had started describing how a + // backend stores things again, whatever the import path said. + const DISK_LAYOUT = [ + 'SPACE_JSON_FILENAME', + 'WORLD_CANVAS_DIR_NAME', + 'canvasJsonPath', + 'nodesDir', + 'nodeFilePath', + 'ARTIFACTS_DIR_NAME', + 'artifactsDir', + 'artifactPath', + 'HISTORY_DIR_NAME', + 'historyDir', + 'chatDir', + 'tasksPath', + 'eventsPath', + 'deltaLogPath', + 'changesPath', + 'canvasRoot', + ]; + const violations: string[] = []; + for (const file of workspaceFiles) { + if (file.startsWith('modules/workspace/migrations/')) continue; + const source = read(file); + for (const symbol of DISK_LAYOUT) { + if (new RegExp(`\\b${symbol}\\b`).test(source)) { + violations.push(`${file} → ${symbol}`); + } + } + } + expect(violations).toEqual([]); }); }); @@ -263,15 +356,9 @@ describe('root forwarding shims', () => { expect(body[0]).toMatch(/^export \* from '\.[^']+\.js';$/); }); - /** - * Frozen snapshot of the call sites that already imported these paths when - * the shims were installed. The lists may shrink as consumers migrate; - * a new entry means someone added an importer of a deprecated path, which - * is what the shims exist to stop. - */ - const ALLOWED_IMPORTERS: Record = { + /** Exact snapshot of the remaining deprecated-path importers. */ + const EXPECTED_IMPORTERS: Record = { 'storage/canvas-store.js': [ - 'modules/agent/sketch.service.ts', 'modules/canvas/canvas-search.test.ts', 'modules/canvas/canvas-search.ts', 'modules/canvas/canvas-spatial.ts', @@ -280,10 +367,6 @@ describe('root forwarding shims', () => { 'modules/canvas/node-prompt.ts', 'modules/canvas/world-reference-resolver.ts', 'modules/canvas/world-target-access.ts', - 'modules/preprocessing/pipeline.test.ts', - 'modules/preprocessing/pipeline.ts', - 'modules/preprocessing/stages/cache-check.ts', - 'modules/preprocessing/stages/persist.ts', ], 'storage/canvas-dirs.js': [ 'modules/agent/tools/world-target-read.test.ts', @@ -301,35 +384,17 @@ describe('root forwarding shims', () => { 'modules/workspace.ts', ], 'storage/paths.js': [ - 'modules/agent/acp/service.ts', - 'modules/agent/acp/threads.route.ts', - 'modules/agent/agent.route.ts', - 'modules/agent/agent.service.ts', - 'modules/agent/conversation/prompt/debug-prompt.ts', - 'modules/agent/memory/analyzer.ts', - 'modules/agent/memory/read.ts', - 'modules/agent/memory/sandbox.ts', - 'modules/agent/memory/trigger.ts', - 'modules/agent/skills.route.test.ts', - 'modules/agent/tools/handlers/fs-sandbox.ts', - 'modules/agent/tools/handlers/fs-write.test.ts', - 'modules/agent/tools/handlers/fs-write.ts', - 'modules/canvas/canvas-search.test.ts', - 'modules/canvas/canvas-search.ts', + 'modules/canvas/canvas-content-cas.test.ts', + 'modules/canvas/canvas.route.test.ts', 'modules/canvas/canvas.route.ts', 'modules/canvas/external-watcher.ts', - 'modules/canvas/external.route.ts', - 'modules/canvas/import-node-src.test.ts', - 'modules/canvas/import-node-src.ts', 'modules/canvas/world-target-access.ts', - 'modules/remote_fs/rfs.route.ts', - 'modules/remote_fs/skill.ts', - 'prompt/skills/loader.ts', + 'modules/workspace/migrations/migrate-acp-sessions.ts', ], }; - it.each(Object.keys(ALLOWED_IMPORTERS))( - 'gains no new importer of %s', + it.each(Object.keys(EXPECTED_IMPORTERS))( + 'keeps the exact importer snapshot for %s', (shimPath) => { const importers = sourceFiles .filter((file) => !file.startsWith('modules/storage/')) @@ -338,14 +403,7 @@ describe('root forwarding shims', () => { ) .sort(); - const added = importers.filter( - (f) => !ALLOWED_IMPORTERS[shimPath].includes(f), - ); - expect(added).toEqual([]); - // Shrinking is the goal, so the snapshot is a ceiling, not an equality. - expect(importers.length).toBeLessThanOrEqual( - ALLOWED_IMPORTERS[shimPath].length, - ); + expect(importers).toEqual(EXPECTED_IMPORTERS[shimPath]); }, ); }); diff --git a/apps/server/src/modules/storage/paths.ts b/apps/server/src/modules/storage/paths.ts index 76dd0395b..b11ab904e 100644 --- a/apps/server/src/modules/storage/paths.ts +++ b/apps/server/src/modules/storage/paths.ts @@ -2,12 +2,14 @@ // Licensed under the MIT license. /** - * @deprecated Forwarding shim — the Workspace layout owns these now. + * @deprecated Forwarding shim — the Disk backend owns its layout now. * - * Import from `modules/workspace/disk/paths.js` instead. This file exists - * only so the many existing physical-Disk capability imports keep resolving - * while they migrate; it must never contain logic, and no new call site may - * import it (enforced by the module-boundary test). + * Inside the storage module, import from + * `storage/backends/disk/layout.js`. Application code should use + * `spaceDirectory()` or the workspace-owned paths when those express the + * capability it needs. This file exists for the remaining explicit Disk + * layout reads while they migrate; it must never contain logic, and no new + * call site may import it (enforced by the module-boundary test). */ -export * from '../workspace/disk/paths.js'; +export * from './backends/disk/layout.js'; diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index 24ad60b9d..c11a93e0b 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -28,6 +28,7 @@ import { getWorkspacePath, } from '../workspace.js'; import { DiskBlobStore } from './backends/disk/blob-store.js'; +import { canvasRoot } from './backends/disk/layout.js'; import { DiskStructuredStore } from './backends/disk/structured-store.js'; import { parseStorageProfile, @@ -330,6 +331,25 @@ export async function storageHealth(): Promise { return Promise.all([storage.structured.health(), storage.blobs.health()]); } +/** + * The real directory backing a Space — the materialization capability. + * + * Some consumers genuinely need a filesystem path rather than a record: an + * ACP agent needs a working directory, the external watcher needs something + * to watch, RFS exposes a tree. That is a product requirement, not a leak + * (proposal §12.5.4), and it is the Space-level counterpart to + * `BlobScope.materialize()`. + * + * It lives in the composition root because only this module may ask a named + * backend where anything is. Every profile selectable today materializes, so + * this resolves unconditionally; a backend that stores Spaces without a + * directory would refuse here rather than hand back a path that does not + * exist. + */ +export function spaceDirectory(canvasId: string): string { + return canvasRoot(canvasId); +} + /** * Swap the active storage, returning a restore function. * diff --git a/apps/server/src/modules/workspace-prepare.ts b/apps/server/src/modules/workspace-prepare.ts index eb9b6f5bd..c04514d40 100644 --- a/apps/server/src/modules/workspace-prepare.ts +++ b/apps/server/src/modules/workspace-prepare.ts @@ -11,7 +11,7 @@ import { mkdirSync } from 'node:fs'; -import { ensureWorldCanvasOnDisk } from './workspace/disk/world-canvas.js'; +import { ensureWorldCanvasOnDisk } from './storage/index.js'; import { migrateLegacyAcpSessions } from './workspace/migrations/migrate-acp-sessions.js'; import { migrateLegacyAgenetesThreads, @@ -40,7 +40,8 @@ export function prepareWorkspaceOnDisk(workspacePath: string): void { migrateLegacyChatThreads(workspacePath); // Second hop (M6.9 row 2): fold legacy `.history/chat/*.turns.jsonl` turns // into the Agenetes two-tier log (`chat_v2/`). MUST run AFTER the pi-ai - // `.json` -> `.turns.jsonl` hop above. + // `.json` -> `.turns.jsonl` hop above, which resolves every coexisting pair + // before this hop folds the turn logs. migrateLegacyChatTurns(workspacePath); // Convert the strict workload/state boundary before any writer opens the // namespace. Keeps the original v1 file as `.agenetes-v1.bak`. diff --git a/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts b/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts new file mode 100644 index 000000000..756569b33 --- /dev/null +++ b/apps/server/src/modules/workspace/legacy-workspace-activation.test.ts @@ -0,0 +1,540 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * End-to-end activation of a legacy workspace, over the production routes. + * + * Phase 4.5 moved the Disk record layout inside the storage boundary and + * routed every "where is this Space" question through `spaceDirectory()`. This + * suite exists to prove that the move did not change what the app can read or + * write. It does not test a module — it activates a workspace the way a launch + * does (`setWorkspacePath` → `prepareWorkspaceOnDisk` → every migration) and + * then drives the same URLs the web client uses, mounted at the same prefixes + * as `app.ts`. + * + * How the "old" workspace is built, and why it is honest: + * + * - `space.json`, `nodes/