diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 13e5a43d7..1108bc1c5 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -193,6 +193,15 @@ function resolveIconPath(filename: string): string | undefined { } const PREFERRED_PORT = 3001; +/** + * The backend may spend up to 70 seconds validating or recovering a saved + * workspace before it starts listening. Keep the Electron-owned readiness + * budget above that bound so the shell does not kill a healthy recovery and + * retry it indefinitely. External development servers retain waitForPort's + * shorter default because Electron does not own their startup lifecycle. + */ +const OWNED_SERVER_READY_TIMEOUT_MS = 90_000; + /** * How much of the server's stderr to keep in memory at any given time. * On non-zero exit we dump this ring buffer to a `crash--exit.log` @@ -456,9 +465,8 @@ function buildServerEnv(port: number): NodeJS.ProcessEnv { // Ensure the data directory exists so the server doesn't have to // race-condition on first-use creation. The workspace directory is - // intentionally NOT pre-created: in free mode the user picks it via - // the in-app UI (folder picker / path input), and the web client - // persists the selection across launches via localStorage. + // intentionally NOT pre-created: on first launch the user picks it via the + // in-app UI (folder picker / path input), and we remember it from then on. mkdirSync(dataDir, { recursive: true }); if (IS_DEV && webDistPath && !existsSync(webDistPath)) { @@ -469,9 +477,6 @@ function buildServerEnv(port: number): NodeJS.ProcessEnv { ); } - // Notably absent: HUABU_WORKSPACE. Omitting it puts the server in - // free mode, so the web UI shows its workspace picker on first launch. - // // External-agent (ACP) integration: the server embeds an `agentlet` // daemon supervisor (`DaemonSupervisor`) which fork()s the daemon // entry point itself. In packaged builds the entry resolves to @@ -480,12 +485,27 @@ function buildServerEnv(port: number): NodeJS.ProcessEnv { // in dev it falls back to `external/agentlet/packages/local/dist/index.js`. // No env var injection is needed here \u2014 the resolver in // `daemon-supervisor.ts` covers both layouts. + + // The workspace the user last chose, handed to the server at fork. + // + // A server process serves one workspace for its lifetime, so this is where + // the choice takes effect — the shell owns both `workspace.json` and the + // child process, which makes it the only thing that can apply a new one. + // Deliberately *not* `HUABU_WORKSPACE`: that is the operator's lock, and it + // hides the path, removes the picker, and fails the boot when the folder + // cannot be opened. This is the user's own choice, so it stays free mode — + // the picker remains available and a folder that has gone missing lands the + // user back on it instead of killing the app. Absent on first launch, which + // is what shows the picker then. + const savedWorkspace = readWorkspaceStore().path; + return { ...process.env, SERVER_PORT: String(port), HUABU_BIND_HOST: '127.0.0.1', HUABU_DATA_DIR: dataDir, HUABU_SECRET_BRIDGE: '1', + ...(savedWorkspace ? { HUABU_WORKSPACE_STARTUP: savedWorkspace } : {}), ...(webDistPath ? { WEB_DIST_PATH: webDistPath } : {}), NODE_ENV: IS_DEV ? 'development' : 'production', }; @@ -786,6 +806,19 @@ function registerWorkspaceIpc(): void { return next; }); + /** + * Restart the app so the server comes up on the saved workspace. + * + * The renderer calls this after `workspace:set` when a workspace is already + * active. Relaunching the whole app rather than re-forking the server keeps + * one rule about what a running process is looking at: the window, its + * caches, and the server all start again on the same choice. + */ + ipcMain.handle('workspace:restart', () => { + app.relaunch(); + app.quit(); + }); + ipcMain.handle('workspace:remove-recent', (_event, rawPath: unknown) => { if (typeof rawPath !== 'string') { throw new Error('workspace:remove-recent requires a string path'); @@ -1335,7 +1368,11 @@ app.whenReady().then(async () => { tried.add(candidate); try { await startServer(candidate); - await waitForPort(candidate, 20_000, serverExitPromise ?? undefined); + await waitForPort( + candidate, + OWNED_SERVER_READY_TIMEOUT_MS, + serverExitPromise ?? undefined, + ); serverPort = candidate; lastErr = null; break; diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index c3efef445..5b5c866fb 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -87,6 +87,14 @@ contextBridge.exposeInMainWorld('electronBridge', { 'workspace:remove-recent', path, ) as Promise, + /** + * Restart the app onto the saved workspace. + * + * A server process serves one workspace for its lifetime, so this is how a + * new choice takes effect. Never resolves — the app is on its way down. + */ + restart: (): Promise => + ipcRenderer.invoke('workspace:restart') as Promise, }, window: { diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index a2d0fd579..da809c1aa 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -57,20 +57,13 @@ import { resolveAllowedHostnames, } from './modules/security/index.js'; import webRoutes from './modules/web/web.route.js'; -import { - initWorkspaceFromEnv, - isWorkspaceConfigured, -} from './modules/workspace.js'; +import { isWorkspaceConfigured } from './modules/workspace.js'; import workspaceRoutes from './modules/workspace.route.js'; import { preloadSkills } from './prompt/index.js'; import { getPersistedSecret, setSecrets } from './security/secret-store.js'; import { MAX_UPLOAD_BYTES } from './upload-limits.js'; import { logger } from './utils/logger.js'; -// Lock the workspace at startup if HUABU_WORKSPACE is set (managed mode). -// In free mode this is a no-op and the client will activate at runtime. -initWorkspaceFromEnv(); - // Eagerly scan + validate every SKILL.md frontmatter at boot so a malformed // skill (missing `appliesTo`, mismatched `id`, etc.) crashes the process at // startup instead of surfacing as a 500 on the first agent request. The @@ -357,9 +350,8 @@ if (bundledAgentTeamsPath) { } else { app.log.warn('[agent-team] bundled collection not found'); } -// Release every active external-note session on shutdown. Their `fs.watch` -// handles are otherwise only closed on a workspace switch, so a -// force-terminated process leaves them open — and on virtual/network +// Release every active external-note session on shutdown. A force-terminated +// process otherwise leaves its `fs.watch` handles open — and on virtual/network // filesystems (Google Drive) an abandoned watch request can stay wedged // after the process is gone. Closing them here lets `app.close()` (driven // by the SIGTERM/SIGINT handlers in server.ts) tear them down gracefully. diff --git a/apps/server/src/modules/canvas/external-watcher.ts b/apps/server/src/modules/canvas/external-watcher.ts index fc37894a7..7519e72a2 100644 --- a/apps/server/src/modules/canvas/external-watcher.ts +++ b/apps/server/src/modules/canvas/external-watcher.ts @@ -96,10 +96,10 @@ let workspaceGeneration = 0; let nextSessionGeneration = 1; /** - * Stamp identifying the workspace and session a piece of async work started - * under. A slow cloud-drive read may resolve long after a workspace switch or - * after the Space was closed and reopened; comparing stamps stops it from - * repopulating unrelated state. + * Stamp identifying the workspace generation and session a piece of async work + * started under. A slow cloud-drive read may resolve after process teardown, a + * test-only workspace reset, or a Space close/reopen; comparing stamps stops + * it from repopulating unrelated state. */ function stampOf(session: ActiveSpaceWatch): string { return `${workspaceGeneration}:${session.sessionGeneration}`; @@ -614,10 +614,9 @@ function resyncSession(session: ActiveSpaceWatch): void { /** * Tear every active session down and tell its subscribers the Space is now - * empty. Called on workspace switch and shutdown: the previous workspace's - * canvasIds are meaningless afterwards, and the client reconnects its stream - * when it navigates into the new workspace. Bumping the workspace generation - * rejects any scan or event still in flight from the previous workspace. + * empty. Called on shutdown, failed startup cleanup, and test-only workspace + * resets. Bumping the workspace generation rejects any scan or event still in + * flight from the previous namespace. */ function destroyAllSessions(): void { for (const session of [...sessions.values()]) { @@ -630,9 +629,7 @@ function destroyAllSessions(): void { /** * Drop every active external-note session and release its handles. * - * Called on workspace switch (the previous workspace's canvasIds are - * meaningless afterwards, and the client reconnects its stream when it - * navigates into the new workspace) and on server shutdown, so live + * Called on server shutdown and process-local workspace cleanup, so live * `fs.watch` handles are released cleanly instead of being force-killed — * on virtual/network filesystems (Google Drive) a force-terminated process * can leave in-flight watch requests wedged. diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts index 7d287192f..d1561db5c 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts @@ -93,7 +93,7 @@ describe('DiskBlobStore temp file hygiene', () => { ).toEqual(['hot.bin']); }); - it('binds in-flight paths to their original workspace and rejects a held scope after activation', async () => { + it("resolves an operation's paths once, before its first await", async () => { const otherRoot = mkdtempSync(path.join(tmpdir(), 'huabu-blob-switched-')); const scope = new DiskBlobStore().scope({ kind: 'canvas', canvasId }); let signalStarted = (): void => {}; @@ -122,12 +122,11 @@ describe('DiskBlobStore temp file hygiene', () => { expect( readFileSync(path.join(root, canvasId, '.artifacts', 'bound.bin')), ).toEqual(Buffer.from('bound bytes')); + // Every path in one operation derives from the directory it resolved + // before its first await, so a Space directory that moves underneath a + // streaming write cannot land the temp file in one place and the + // destination in another. expect(existsSync(path.join(otherRoot, canvasId))).toBe(false); - - await expect(scope.read('bound.bin')).rejects.toThrow( - /inactive workspace/, - ); - await expect(scope.deleteAll()).rejects.toThrow(/inactive workspace/); } finally { workspaceState.path = root; rmSync(otherRoot, { recursive: true, force: true }); diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 88dd8eb6d..66c69228e 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -7,10 +7,6 @@ * Maps a canvas scope to `/.artifacts/`, preserving the layout * the workspace format has always used: one file per blob, named by the * URL key, no manifest indirection. - * - * Each scope is bound to the workspace active when it is created. A fresh - * scope follows a free-mode workspace switch; a retained scope rejects the - * next operation instead of silently redirecting it into the new workspace. */ import { randomUUID } from 'node:crypto'; @@ -28,7 +24,6 @@ import { pipeline } from 'node:stream/promises'; import { artifactsDir } from './layout.js'; import { renameOverWithRetry } from '../../../../utils/fs.js'; -import { getWorkspacePath } from '../../../workspace.js'; import { createBlobLease, normalizeBlobName } from '../../ports/blob.js'; import type { @@ -73,24 +68,19 @@ function isMissing(err: unknown): boolean { class DiskBlobScope implements BlobScope { readonly #ref: BlobScopeRef; - readonly #workspacePath: string; constructor(ref: BlobScopeRef) { this.#ref = ref; - this.#workspacePath = path.resolve(getWorkspacePath()); } + /** + * Resolve once per operation, before its first await. + * + * Every later path in that operation derives from this absolute directory, + * so an externally renamed Space directory cannot combine a temp file under + * the old name with a destination under the new one. + */ #resolveDir(): string { - const active = path.resolve(getWorkspacePath()); - if (active !== this.#workspacePath) { - throw new Error( - `DiskBlobScope(${this.#ref.canvasId}) belongs to an inactive workspace. ` + - `Resolve a fresh scope after workspace activation.`, - ); - } - // Resolve once per operation, before its first await. Every later path in - // that operation is derived from this absolute directory, so a workspace - // switch cannot combine a temp in A with a destination in B. return scopeDir(this.#ref); } 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 41903fb0a..e0335e771 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 @@ -120,7 +120,13 @@ describe('CanvasStore cache boundaries', () => { expect(afterEviction.isNodeWriteSuppressed('n1')).toBe(false); }); - it('invalidates cache state on a direct workspace switch and rejects a held handle', () => { + /** + * A process serves one Workspace, so nothing here carries an answer to + * "which Workspace am I?" — committing one drops the cached instances and + * their warm filename indexes instead. That is the whole invariant: after a + * commit, every handle is new. + */ + it('drops cached instances when a Workspace is committed', () => { const firstRoot = activateWorkspace('huabu-cache-workspace-a-'); createSpace('shared-id', 'First'); const held = getCanvasStore('shared-id'); @@ -133,20 +139,15 @@ describe('CanvasStore cache boundaries', () => { expect(active).not.toBe(held); expect(active.writeNode('node-b', note('node-b', 'From B')).ok).toBe(true); expect(active.nodeIdForFilename('From B.md')).toBe('node-b'); + // A's warm index is not consulted for B's Space. + expect(active.nodeIdForFilename('From A.md')).toBeNull(); - // The old instance has a warm filename index from workspace A. It must not - // be allowed to consult that index — or the new workspace's disk — while B - // is active. - expect(() => held.nodeIdForFilename('From B.md')).toThrow( - /inactive workspace.*Resolve a fresh Space handle/s, - ); - expect(() => held.readNode('node-a')).toThrow(/inactive workspace/); - - // Switching back also invalidates B's cache rather than reviving A's old - // instance and its potentially stale in-memory index. + // Committing back to A also drops B's instance rather than reviving one + // whose in-memory index describes the other Workspace. setWorkspacePath(firstRoot); const reopened = getCanvasStore('shared-id'); expect(reopened).not.toBe(held); + expect(reopened).not.toBe(active); expect(reopened.nodeIdForFilename('From A.md')).toBe('node-a'); }); @@ -166,11 +167,10 @@ describe('CanvasStore cache boundaries', () => { expect(getCanvasStore('canvas-b').read()?.canvasId).toBe('canvas-b'); }); - it('rejects a held event repository after a workspace switch', async () => { + it('reads the committed Workspace through a freshly resolved handle', async () => { activateWorkspace('huabu-log-workspace-a-'); createSpace('shared-id', 'First'); - const held = new DiskStructuredStore().space('shared-id'); - await held.events.append([ + await new DiskStructuredStore().space('shared-id').events.append([ { payload: { action: 'node_selected', @@ -193,47 +193,34 @@ describe('CanvasStore cache boundaries', () => { }, ]); - // This read uses strict JSONL helpers directly. Without its own - // workspace-lifetime guard, the retained A facade would silently read - // B's same-id file instead of rejecting the stale handle. - await expect(held.events.read()).rejects.toThrow(/inactive workspace/); + // The same Space id in two Workspaces is two different logs, and the + // handle resolved after the commit reads the one that is active. expect((await active.events.read()).map((event) => event.ts)).toEqual([2]); }); - it('guards a held record repository before probing the active workspace', async () => { + it("reports a corrupt record rather than the previous Workspace's copy", async () => { activateWorkspace('huabu-record-workspace-a-'); - const first = createSpace('shared-id', 'First'); - const held = new DiskStructuredStore().space('shared-id'); - await expect(held.read()).resolves.toMatchObject({ title: 'First' }); + createSpace('shared-id', 'First'); + await expect( + new DiskStructuredStore().space('shared-id').read(), + ).resolves.toMatchObject({ title: 'First' }); activateWorkspace('huabu-record-workspace-b-'); createSpace('shared-id', 'Second'); - const active = new DiskStructuredStore().space('shared-id'); - await expect(active.read()).resolves.toMatchObject({ - title: 'Second', - }); - - await expect(held.read()).rejects.toThrow(/inactive workspace/); await expect( - held.write({ - expectedVersion: first.version, - nextRecord: { - ...first, - version: first.version + 1, - updatedAt: first.updatedAt + 1, - }, - nodeMutations: [], - }), - ).rejects.toThrow(/inactive workspace/); + new DiskStructuredStore().space('shared-id').read(), + ).resolves.toMatchObject({ title: 'Second' }); - // Even a corrupt same-id record in B must not leak through the strict - // probe as a SyntaxError before the retained A handle is rejected. + // A same-id record the user broke by hand surfaces as the integrity error + // it is. Nothing falls back to the copy another Workspace happens to hold. writeFileSync( path.join(canvasRoot('shared-id'), SPACE_JSON_FILENAME), '{broken', 'utf8', ); - await expect(held.read()).rejects.toThrow(/inactive workspace/); + await expect( + new DiskStructuredStore().space('shared-id').read(), + ).rejects.toThrow(); }); it('does not create a Space directory for a node write to a missing Space', async () => { 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 5a4ee1d61..09aaa3147 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 @@ -18,27 +18,22 @@ * promised: an entry can be evicted and rebuilt. Anything that must survive * eviction is either durable state in a repository or explicitly-scoped, * expiring coordination state such as node tombstones. + * + * Keyed by Space id alone. A process serves one Workspace for its lifetime, so + * there is no second Workspace an id could mean something else in; committing + * a Workspace calls {@link resetStorageCache}, which is what keeps that true + * for a test moving through several temporary ones. */ -import path from 'node:path'; - import { CanvasStore } from './canvas-store.js'; import { sanitizeId } from '../../../../../utils/fs.js'; -import { getWorkspacePath } from '../../../../workspace.js'; -import { refreshCanvasDirIndex } from '../canvas-dirs.js'; const MAX_CACHE = 16; const cache = new Map(); -let cacheWorkspacePath: string | null = null; -function cacheKey(workspacePath: string, canvasId: string): string { - // NUL is not legal in either an OS path or a validated canvas id. - return `${workspacePath}\0${canvasId}`; -} - -function rememberInstance(key: string, store: CanvasStore): CanvasStore { - cache.delete(key); - cache.set(key, store); +function rememberInstance(canvasId: string, store: CanvasStore): CanvasStore { + cache.delete(canvasId); + cache.set(canvasId, store); if (cache.size > MAX_CACHE) { const firstKey = cache.keys().next().value; if (firstKey !== undefined) cache.delete(firstKey); @@ -46,58 +41,27 @@ function rememberInstance(key: string, store: CanvasStore): CanvasStore { return store; } -/** - * `setWorkspacePath()` intentionally knows nothing about storage backends. - * Detect its effect here so even a direct activation invalidates all cached - * per-workspace indexes before the next handle is handed out. - */ -function activateCacheWorkspace(workspacePath: string): void { - if (cacheWorkspacePath === workspacePath) return; - cache.clear(); - cacheWorkspacePath = workspacePath; - // The directory index is process-global too. Refresh it before constructing - // a handle so an id from the previous workspace cannot resolve through a - // same-named directory in the newly-active one. - refreshCanvasDirIndex(); -} - /** * Get (or create) the `CanvasStore` for the given canvas id. Instances * are cheap; the cache only avoids re-validating ids on hot paths. */ export function getCanvasStore(canvasId: string): CanvasStore { const safeId = sanitizeId(canvasId, 'canvasId'); - const workspacePath = path.resolve(getWorkspacePath()); - activateCacheWorkspace(workspacePath); - const key = cacheKey(workspacePath, safeId); - const cached = cache.get(key); + const cached = cache.get(safeId); if (cached) { - cache.delete(key); - cache.set(key, cached); + cache.delete(safeId); + cache.set(safeId, cached); return cached; } - return rememberInstance(key, new CanvasStore(safeId, workspacePath)); + return rememberInstance(safeId, new CanvasStore(safeId)); } /** Drop a single cached instance. */ export function forgetCanvasStore(canvasId: string): void { - const safeId = sanitizeId(canvasId, 'canvasId'); - // Before first activation there cannot be an instance to forget, and - // `getWorkspacePath()` deliberately throws. Keep cleanup harmless during - // boot without weakening get/create operations. - if (cacheWorkspacePath === null) return; - const workspacePath = path.resolve(getWorkspacePath()); - activateCacheWorkspace(workspacePath); - cache.delete(cacheKey(workspacePath, safeId)); + cache.delete(sanitizeId(canvasId, 'canvasId')); } -/** Clear the instance cache explicitly (workspace changes are auto-detected). */ +/** Drop every cached instance. */ export function resetStorageCache(): void { cache.clear(); - // Initial null is also the pre-Workspace state. `commitWorkspacePath()` - // refreshes the directory index when the first Workspace is activated, so - // there is nothing further to invalidate here. - if (cacheWorkspacePath === null) return; - cacheWorkspacePath = path.resolve(getWorkspacePath()); - refreshCanvasDirIndex(); } 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 d020fd9bd..72d297f57 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts @@ -44,7 +44,6 @@ import { toFrontmatter, } from '../../../../../utils/markdown-frontmatter.js'; import { toSafeFilename } from '../../../../../utils/naming.js'; -import { getWorkspacePath } from '../../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../../space-lifecycle-admission.js'; import { patchCanvasDirTitle, @@ -297,8 +296,6 @@ function readNodeSidecar( export class CanvasStore { readonly canvasId: string; - /** Workspace this handle was created for; handles never follow activation. */ - readonly #workspacePath: string; private nodes: NameIndex | null = null; /** Whether the cached index was built without swallowing sidecar failures. */ private nodeIndexIsConclusive = false; @@ -323,24 +320,8 @@ export class CanvasStore { /** Live ids from the latest structural write, reconciled after log commit. */ private deferredTombstoneReconciliationNodeIds: Set | null = null; - constructor(canvasId: string, workspacePath = getWorkspacePath()) { + constructor(canvasId: string) { this.canvasId = sanitizeId(canvasId, 'canvasId'); - this.#workspacePath = path.resolve(workspacePath); - } - - /** - * A cached handle is scoped to the workspace that created it. Without this - * guard, a caller retaining a handle across `setWorkspacePath()` would send - * its cached node filename/index state into the newly-active workspace. - */ - private assertActiveWorkspace(): void { - const active = path.resolve(getWorkspacePath()); - if (active !== this.#workspacePath) { - throw new Error( - `CanvasStore(${this.canvasId}) belongs to an inactive workspace. ` + - `Resolve a fresh Space handle after workspace activation.`, - ); - } } // ── Canvas structure ───────────────────────────────────────────────────── @@ -355,7 +336,6 @@ export class CanvasStore { * would silently strip the user's typed characters from the title. */ read(): CanvasFile | null { - this.assertActiveWorkspace(); let file = readJson(canvasJsonPath(this.canvasId)); if (!file) { refreshCanvasDirIndex(); @@ -368,7 +348,6 @@ export class CanvasStore { /** @internal Apply legacy Finder-title semantics without rereading disk. */ reconcileValidatedRecord(file: CanvasFile): CanvasFile { - this.assertActiveWorkspace(); if (file.canvasId !== this.canvasId) { throw new Error( `CanvasStore(${this.canvasId}) cannot reconcile record "${file.canvasId}"`, @@ -387,7 +366,7 @@ export class CanvasStore { updatedAt: Date.now(), }; try { - assertSpaceMutationAllowed(this.#workspacePath, this.canvasId); + assertSpaceMutationAllowed(this.canvasId); atomicWriteJson(canvasJsonPath(this.canvasId), next); patchCanvasDirTitle(this.canvasId, visibleTitle); return next; @@ -409,8 +388,7 @@ export class CanvasStore { } private writeRecord(canvas: CanvasFile, reconcileTombstones: boolean): void { - this.assertActiveWorkspace(); - assertSpaceMutationAllowed(this.#workspacePath, this.canvasId); + assertSpaceMutationAllowed(this.canvasId); if (canvas.canvasId !== this.canvasId) { throw new Error( `CanvasStore(${this.canvasId}) refusing to write canvas with id "${canvas.canvasId}"`, @@ -425,7 +403,7 @@ export class CanvasStore { const reconcileNodeIds = new Set(); if (reconcileTombstones) { const tombstonedLiveIds = [...liveNodeIds].filter((id) => - isNodeTombstoned(this.#workspacePath, this.canvasId, id), + isNodeTombstoned(this.canvasId, id), ); if (tombstonedLiveIds.length > 0) { const previous = readJson(canvasJsonPath(this.canvasId)); @@ -456,7 +434,7 @@ export class CanvasStore { return; } for (const id of reconcileNodeIds) { - clearNodeTombstone(this.#workspacePath, this.canvasId, id); + clearNodeTombstone(this.canvasId, id); } } @@ -467,7 +445,7 @@ export class CanvasStore { * the legacy reader can self-heal and rewrite them. */ private readValidSpaceForMutation(operation: string): CanvasFile | null { - assertSpaceMutationAllowed(this.#workspacePath, this.canvasId); + assertSpaceMutationAllowed(this.canvasId); try { let record = readValidCanvasFile( canvasJsonPath(this.canvasId), @@ -492,7 +470,7 @@ export class CanvasStore { private requireExistingSpaceForMutation(operation: string): void { if (this.nodeMutationTransactionDepth > 0) { - assertSpaceMutationAllowed(this.#workspacePath, this.canvasId); + assertSpaceMutationAllowed(this.canvasId); return; } const record = this.readValidSpaceForMutation(operation); @@ -519,7 +497,6 @@ export class CanvasStore { }, callback: () => T, ): T { - this.assertActiveWorkspace(); if (this.nodeMutationTransactionDepth > 0) { throw new Error('CanvasStore node mutation transactions cannot nest'); } @@ -536,7 +513,6 @@ export class CanvasStore { // mutex, on the hottest write path there is. const tombstoneSnapshot = captureNodeTombstones( - this.#workspacePath, this.canvasId, options.affectedNodeIds, ); @@ -548,15 +524,11 @@ export class CanvasStore { // `callback` includes the structural write and delta-log append. Only // after both return successfully may a listed id clear its tombstone. for (const id of this.deferredTombstoneReconciliationNodeIds) { - clearNodeTombstone(this.#workspacePath, this.canvasId, id); + clearNodeTombstone(this.canvasId, id); } return result; } catch (error) { - restoreNodeTombstones( - this.#workspacePath, - this.canvasId, - tombstoneSnapshot, - ); + restoreNodeTombstones(this.canvasId, tombstoneSnapshot); throw error; } finally { this.deferredTombstoneReconciliationNodeIds = null; @@ -570,8 +542,7 @@ export class CanvasStore { * throwing so the route layer can map it to a 409. */ renameSelf(newTitle: string | null): RenameSelfResult { - this.assertActiveWorkspace(); - assertSpaceMutationAllowed(this.#workspacePath, this.canvasId); + assertSpaceMutationAllowed(this.canvasId); if (isWorldCanvasId(this.canvasId)) { return { ok: false, reason: 'forbidden' }; } @@ -668,7 +639,6 @@ export class CanvasStore { * `readdir`; per-file contents are only re-read when a rescan fires. */ revalidateNodeForRead(nodeId: string, strict = false): void { - this.assertActiveWorkspace(); const idx = this.nodeIndex(strict); if (this.nodeDuplicateIds.has(nodeId) || this.nodeIndexCountStale(idx)) { this.invalidateNodeIndex(); @@ -683,7 +653,6 @@ export class CanvasStore { * populated the set, so no extra scan happens there. */ isDuplicateNode(nodeId: string): boolean { - this.assertActiveWorkspace(); this.nodeIndex(); return this.nodeDuplicateIds.has(nodeId); } @@ -696,7 +665,6 @@ export class CanvasStore { * O(directory size) — only called on the rare duplicate path. */ duplicateNodeFiles(nodeId: string): string[] { - this.assertActiveWorkspace(); return this.duplicateNodeFilenames(nodeId); } @@ -781,12 +749,10 @@ export class CanvasStore { * not match `toSafeFilename(label)` (dedupe suffixes, external renames). */ nodeIdForFilename(filename: string): string | null { - this.assertActiveWorkspace(); return this.nodeIndex().findByName(filename)?.id ?? null; } readNode(nodeId: string): NodeContent | null { - this.assertActiveWorkspace(); const filename = this.nodeFilenameOf(nodeId); const fullPath = nodeFilePath(this.canvasId, filename); let raw = readText(fullPath); @@ -821,7 +787,6 @@ export class CanvasStore { * outcome instead of overwriting either file. */ readNodeStrict(nodeId: string): NodeContent | null { - this.assertActiveWorkspace(); this.revalidateNodeForRead(nodeId, true); const read = (filename: string): string | null => @@ -883,7 +848,6 @@ export class CanvasStore { async readAllNodes(options?: { strict?: boolean; }): Promise> { - this.assertActiveWorkspace(); const generation = this.nodeIndexGeneration; const contents = new Map(); const idx = new NameIndex(); @@ -953,7 +917,6 @@ export class CanvasStore { onNode: (id: string, content: NodeContent) => void, signal?: { readonly aborted: boolean }, ): Promise> { - this.assertActiveWorkspace(); const generation = this.nodeIndexGeneration; const contents = new Map(); const idx = new NameIndex(); @@ -1005,7 +968,6 @@ export class CanvasStore { content: NodeContent, opts: { strictRename?: boolean } = {}, ): RenameResult { - this.assertActiveWorkspace(); if (content.nodeId !== nodeId) { throw new Error( `nodeId mismatch: argument="${nodeId}" payload="${content.nodeId}"`, @@ -1193,7 +1155,6 @@ export class CanvasStore { * `.md` stays on disk as a permanent orphan. */ deleteNode(nodeId: string): 'deleted' | 'absent' { - this.assertActiveWorkspace(); // Keep idempotent delete semantics, but do not retain a tombstone for an // id whose Space itself does not exist. if ( @@ -1211,7 +1172,7 @@ export class CanvasStore { // in-flight write cannot resurrect the sidecar regardless of which delete // branch we take. The process registry outlives an evicted LRU instance // and expires the entry on its own timer. - markNodeDeleted(this.#workspacePath, this.canvasId, nodeId); + markNodeDeleted(this.canvasId, nodeId); const idx = this.nodeIndex(); const filename = idx.get(nodeId)?.filename ?? this.nodeFilenameOf(nodeId); @@ -1259,8 +1220,7 @@ export class CanvasStore { * recently-deleted id (the common case short-circuits on an empty map). */ isNodeWriteSuppressed(nodeId: string): boolean { - this.assertActiveWorkspace(); - if (!isNodeTombstoned(this.#workspacePath, this.canvasId, nodeId)) { + if (!isNodeTombstoned(this.canvasId, nodeId)) { return false; } // An authoritative INSERT (undo/revert or explicit id reuse) may recreate @@ -1309,7 +1269,6 @@ export class CanvasStore { * un-coalesced sidecar. */ readChanges(threadId: string): CanvasChangeRecord[] { - this.assertActiveWorkspace(); return coalesceChanges( readJson(changesPath(this.canvasId, threadId)) ?? [], @@ -1332,7 +1291,6 @@ export class CanvasStore { threadId: string, records: CanvasChangeRecord[], ): CanvasChangeRecord[] { - this.assertActiveWorkspace(); this.requireExistingSpaceForMutation('append change records'); const existing = this.readChanges(threadId); const merged = coalesceChanges([...existing, ...records]); @@ -1345,7 +1303,6 @@ export class CanvasStore { * record, or null when the id was not present. */ removeChange(threadId: string, changeId: string): CanvasChangeRecord | null { - this.assertActiveWorkspace(); this.requireExistingSpaceForMutation('remove a change record'); const existing = this.readChanges(threadId); const idx = existing.findIndex((r) => r.id === changeId); @@ -1367,7 +1324,6 @@ export class CanvasStore { appendEvents( events: ReadonlyArray<{ payload: RecentAction; ts?: number }>, ): void { - this.assertActiveWorkspace(); this.requireExistingSpaceForMutation('append events'); if (events.length === 0) return; const now = Date.now(); @@ -1383,7 +1339,6 @@ export class CanvasStore { * most recent `limit` records are returned (tail read). */ readEvents(limit?: number): CanvasEvent[] { - this.assertActiveWorkspace(); return readJsonLines(eventsPath(this.canvasId), limit); } @@ -1400,7 +1355,6 @@ export class CanvasStore { // crash mid-write drops the trailing partial line on read. appendDeltaLogEntry(entry: DeltaLogEntry): void { - this.assertActiveWorkspace(); this.requireExistingSpaceForMutation('append a delta'); appendJsonLine(deltaLogPath(this.canvasId), entry); } @@ -1412,7 +1366,6 @@ export class CanvasStore { * guarantees monotonic appends). */ readDeltaLogSince(fromVersion: number): DeltaLogEntry[] { - this.assertActiveWorkspace(); const all = readJsonLines(deltaLogPath(this.canvasId)); if (fromVersion <= 0) return all; return all.filter((row) => row.version > fromVersion); @@ -1427,7 +1380,6 @@ export class CanvasStore { * out-of-order append without paying O(log size) on every write. */ lastDeltaLogEntry(): DeltaLogEntry | null { - this.assertActiveWorkspace(); const tail = readJsonLines(deltaLogPath(this.canvasId), 1); return tail[tail.length - 1] ?? null; } @@ -1442,7 +1394,6 @@ export class CanvasStore { /** Recursively delete the entire canvas directory. */ destroy(): boolean { - this.assertActiveWorkspace(); if (isWorldCanvasId(this.canvasId)) { throw new Error('World canvas cannot be deleted'); } @@ -1450,7 +1401,7 @@ export class CanvasStore { if (!existsSync(root)) { unregisterCanvasDir(this.canvasId); this.invalidateNodeIndex(); - clearSpaceNodeTombstones(this.#workspacePath, this.canvasId); + clearSpaceNodeTombstones(this.canvasId); return false; } rmSync(root, { @@ -1461,7 +1412,7 @@ export class CanvasStore { }); unregisterCanvasDir(this.canvasId); this.invalidateNodeIndex(); - clearSpaceNodeTombstones(this.#workspacePath, this.canvasId); + clearSpaceNodeTombstones(this.canvasId); return true; } } diff --git a/apps/server/src/modules/storage/backends/disk/legacy/node-tombstones.ts b/apps/server/src/modules/storage/backends/disk/legacy/node-tombstones.ts index 940d757a6..060a50416 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/node-tombstones.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/node-tombstones.ts @@ -7,11 +7,11 @@ * Tombstones cannot live on a {@link CanvasStore} instance: those instances * sit in a bounded LRU and may be evicted while an already-started writer is * still running. This registry is therefore shared by every instance, scoped - * by both workspace and Space id, and expires entries after a short TTL. + * by Space id, and expires entries after a short TTL. * * A single unref'd timer removes expired entries even when no later storage - * call happens, so old workspaces and node ids do not become a permanent, - * ever-growing process map. + * call happens, so old node ids do not become a permanent, ever-growing + * process map. */ export const NODE_TOMBSTONE_TTL_MS = 5 * 60_000; @@ -24,11 +24,6 @@ export interface NodeTombstoneSnapshot { readonly expiresAtByNodeId: ReadonlyMap; } -function scopeKey(workspacePath: string, canvasId: string): string { - // NUL cannot occur in a filesystem path or id, so the pair is unambiguous. - return `${workspacePath}\0${canvasId}`; -} - function sweepExpired(now = Date.now()): void { for (const [scope, entries] of tombstones) { for (const [nodeId, expiresAt] of entries) { @@ -62,69 +57,63 @@ function scheduleCleanup(): void { (cleanupTimer as { unref?: () => void }).unref?.(); } -export function markNodeDeleted( - workspacePath: string, - canvasId: string, - nodeId: string, -): void { +export function markNodeDeleted(canvasId: string, nodeId: string): void { sweepExpired(); - const scope = scopeKey(workspacePath, canvasId); - let entries = tombstones.get(scope); + let entries = tombstones.get(canvasId); if (!entries) { entries = new Map(); - tombstones.set(scope, entries); + tombstones.set(canvasId, entries); } entries.set(nodeId, Date.now() + NODE_TOMBSTONE_TTL_MS); scheduleCleanup(); } -export function clearNodeTombstone( - workspacePath: string, - canvasId: string, - nodeId: string, -): void { - const scope = scopeKey(workspacePath, canvasId); - const entries = tombstones.get(scope); +export function clearNodeTombstone(canvasId: string, nodeId: string): void { + const entries = tombstones.get(canvasId); if (!entries) return; if (!entries.delete(nodeId)) return; - if (entries.size === 0) tombstones.delete(scope); + if (entries.size === 0) tombstones.delete(canvasId); scheduleCleanup(); } -export function clearSpaceNodeTombstones( - workspacePath: string, - canvasId: string, -): void { - tombstones.delete(scopeKey(workspacePath, canvasId)); +export function clearSpaceNodeTombstones(canvasId: string): void { + tombstones.delete(canvasId); scheduleCleanup(); } -export function isNodeTombstoned( - workspacePath: string, - canvasId: string, - nodeId: string, -): boolean { - const scope = scopeKey(workspacePath, canvasId); - const entries = tombstones.get(scope); +/** + * Drop every tombstone, for a process that is changing Workspace. + * + * A tombstone fences a sidecar path, so it means nothing once a different + * Workspace is active. Production commits one Workspace and never calls this; + * a test moving through several temporary ones would otherwise carry a fence + * from one into the next. + */ +export function clearAllNodeTombstones(): void { + tombstones.clear(); + scheduleCleanup(); +} + +export function isNodeTombstoned(canvasId: string, nodeId: string): boolean { + const entries = tombstones.get(canvasId); const expiresAt = entries?.get(nodeId); if (expiresAt === undefined) return false; if (Date.now() < expiresAt) return true; entries?.delete(nodeId); - if (entries?.size === 0) tombstones.delete(scope); + if (entries?.size === 0) tombstones.delete(canvasId); scheduleCleanup(); return false; } /** Capture exact process-local tombstone state for transaction rollback. */ export function captureNodeTombstones( - workspacePath: string, canvasId: string, nodeIds: ReadonlySet, ): NodeTombstoneSnapshot { sweepExpired(); const ids = [...nodeIds]; - const entries = tombstones.get(scopeKey(workspacePath, canvasId)); + const entries = tombstones.get(canvasId); const expiresAtByNodeId = new Map(); for (const nodeId of ids) { const expiresAt = entries?.get(nodeId); @@ -135,23 +124,21 @@ export function captureNodeTombstones( /** Restore only the ids captured by {@link captureNodeTombstones}. */ export function restoreNodeTombstones( - workspacePath: string, canvasId: string, snapshot: NodeTombstoneSnapshot, ): void { - const scope = scopeKey(workspacePath, canvasId); - let entries = tombstones.get(scope); + let entries = tombstones.get(canvasId); for (const nodeId of snapshot.nodeIds) entries?.delete(nodeId); if (snapshot.expiresAtByNodeId.size > 0) { if (!entries) { entries = new Map(); - tombstones.set(scope, entries); + tombstones.set(canvasId, entries); } for (const [nodeId, expiresAt] of snapshot.expiresAtByNodeId) { entries.set(nodeId, expiresAt); } } - if (entries?.size === 0) tombstones.delete(scope); + if (entries?.size === 0) tombstones.delete(canvasId); scheduleCleanup(); } 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 322966b84..4239a3286 100644 --- a/apps/server/src/modules/storage/backends/disk/space-logs.ts +++ b/apps/server/src/modules/storage/backends/disk/space-logs.ts @@ -18,8 +18,6 @@ * by its own tests. */ -import path from 'node:path'; - import { canvasEventInputSchema, canvasEventRecordSchema } from '@huabu/shared'; import { coalesceChanges, @@ -33,7 +31,6 @@ import { readJsonLinesStrict, readJsonStrict, } from '../../../../utils/fs.js'; -import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; import type { CanvasStore } from './legacy/canvas-store.js'; @@ -99,24 +96,13 @@ export interface DiskSpaceLogs { class DiskSpaceLogCoordinator { readonly #store: CanvasStore; - readonly #workspacePath: string; constructor(store: CanvasStore) { this.#store = store; - this.#workspacePath = path.resolve(getWorkspacePath()); - } - - private assertActiveWorkspace(): void { - if (path.resolve(getWorkspacePath()) !== this.#workspacePath) { - throw new Error( - `Space logs(${this.#store.canvasId}) belong to an inactive workspace. ` + - 'Resolve a fresh Space handle after workspace activation.', - ); - } } private requireSpace(): void { - assertSpaceMutationAllowed(this.#workspacePath, this.#store.canvasId); + assertSpaceMutationAllowed(this.#store.canvasId); if (!readDiskSpaceRecord(this.#store)) { throw new Error( `Space logs(${this.#store.canvasId}) cannot write logs for a missing Space`, @@ -127,7 +113,6 @@ class DiskSpaceLogCoordinator { // ── Events ──────────────────────────────────────────────────────────────── async appendEvents(events: readonly NewCanvasEvent[]): Promise { - this.assertActiveWorkspace(); if (events.length === 0) return; events.forEach(validateEventInput); this.requireSpace(); @@ -137,14 +122,12 @@ class DiskSpaceLogCoordinator { } async readEvents(limit?: number): Promise { - this.assertActiveWorkspace(); return readValidatedEvents(eventsPath(this.#store.canvasId), limit); } // ── Change-review records ───────────────────────────────────────────────── async readChanges(threadId: string): Promise { - this.assertActiveWorkspace(); return coalesceChanges( readJsonArray( changesPath(this.#store.canvasId, threadId), @@ -157,7 +140,6 @@ class DiskSpaceLogCoordinator { threadId: string, records: readonly CanvasChangeRecord[], ): Promise { - this.assertActiveWorkspace(); this.requireSpace(); const filePath = changesPath(this.#store.canvasId, threadId); const existing = coalesceChanges( @@ -172,7 +154,6 @@ class DiskSpaceLogCoordinator { threadId: string, changeId: string, ): Promise { - this.assertActiveWorkspace(); this.requireSpace(); const filePath = changesPath(this.#store.canvasId, threadId); const existing = coalesceChanges( diff --git a/apps/server/src/modules/storage/backends/disk/space-nodes.ts b/apps/server/src/modules/storage/backends/disk/space-nodes.ts index 759d07a1c..833679709 100644 --- a/apps/server/src/modules/storage/backends/disk/space-nodes.ts +++ b/apps/server/src/modules/storage/backends/disk/space-nodes.ts @@ -4,9 +4,6 @@ /** Disk implementation of the asynchronous node-record port. */ import { createHash } from 'node:crypto'; -import path from 'node:path'; - -import { getWorkspacePath } from '../../../workspace.js'; import type { CanvasStore } from './legacy/canvas-store.js'; import type { NodeContent } from '../../../canvas/persistence-types.js'; @@ -34,22 +31,18 @@ export class DiskSpaceNodes implements SpaceNodes { readonly canvasId: string; readonly #store: CanvasStore; - readonly #workspacePath: string; constructor(store: CanvasStore) { this.#store = store; this.canvasId = store.canvasId; - this.#workspacePath = path.resolve(getWorkspacePath()); } async read(nodeId: string): Promise { - this.#assertActiveWorkspace(); const record = this.#store.readNodeStrict(nodeId); return record === null ? null : snapshotOf(record); } async put(input: NodePutInput): Promise { - this.#assertActiveWorkspace(); if (input.record.nodeId !== input.nodeId) { throw new Error( `SpaceNodes(${this.canvasId}) nodeId mismatch: ` + @@ -122,16 +115,6 @@ export class DiskSpaceNodes implements SpaceNodes { } async delete(nodeId: string): Promise { - this.#assertActiveWorkspace(); return this.#store.deleteNode(nodeId); } - - #assertActiveWorkspace(): void { - if (path.resolve(getWorkspacePath()) !== this.#workspacePath) { - throw new Error( - `SpaceNodes(${this.canvasId}) belongs to an inactive workspace. ` + - 'Resolve a fresh Space handle after workspace activation.', - ); - } - } } 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 3a1737129..afd9a285e 100644 --- a/apps/server/src/modules/storage/backends/disk/space-record.ts +++ b/apps/server/src/modules/storage/backends/disk/space-record.ts @@ -9,12 +9,9 @@ * which owns the version check and the node/delta batch around it. */ -import path from 'node:path'; - import { refreshCanvasDirIndex } from './canvas-dirs.js'; import { canvasJsonPath } from './layout.js'; import { readValidCanvasFile } from './space-record-validation.js'; -import { getWorkspacePath } from '../../../workspace.js'; import type { CanvasStore } from './legacy/canvas-store.js'; import type { CanvasFile } from '../../../canvas/persistence-types.js'; @@ -23,21 +20,14 @@ import type { SpaceHandle } from '../../ports/structured.js'; /** * Bind the record read for one Space handle. * - * The workspace active at bind time is captured here, so a handle retained - * across a workspace switch rejects rather than reading the newly active - * workspace — the same guard the other Disk parts carry. + * The port's read is asynchronous while the Disk one is not, so this is the + * whole adapter: one shared reader, so every member of a handle answers to + * the same view of the record. */ export function createDiskSpaceRecordReader( store: CanvasStore, ): SpaceHandle['read'] { - const workspacePath = path.resolve(getWorkspacePath()); return async function readSpaceRecord(): Promise { - if (path.resolve(getWorkspacePath()) !== workspacePath) { - throw new Error( - `Space record(${store.canvasId}) belongs to an inactive workspace. ` + - 'Resolve a fresh Space handle after workspace activation.', - ); - } return readDiskSpaceRecord(store); }; } 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 dd7ab747d..829d6674a 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 @@ -234,16 +234,13 @@ describe('DiskSpaceRepository membership', () => { await expect(spaces.worldId()).rejects.toBeInstanceOf(SyntaxError); }); - it('rejects a retained handle after the active Workspace changes', async () => { - const firstRoot = makeWorkspace('huabu-space-membership-stale-a-'); + it('reads the Workspace that is active when it is resolved', async () => { + const firstRoot = makeWorkspace('huabu-space-membership-a-'); seedWorld(firstRoot, 'world-a'); - const held = new DiskSpaceRepository(); - await expect(held.worldId()).resolves.toBe('world-a'); + await expect(new DiskSpaceRepository().worldId()).resolves.toBe('world-a'); - const secondRoot = makeWorkspace('huabu-space-membership-stale-b-'); + const secondRoot = makeWorkspace('huabu-space-membership-b-'); seedWorld(secondRoot, 'world-b'); - await expect(held.list()).rejects.toThrow(/inactive workspace/i); - await expect(held.worldId()).rejects.toThrow(/inactive workspace/i); await expect(new DiskSpaceRepository().worldId()).resolves.toBe('world-b'); }); }); 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 e59b78fd2..7e80d8a5a 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -72,7 +72,6 @@ export class DiskSpaceRepository implements SpaceRepository { } async list(): Promise { - this.#assertActiveWorkspace(); refreshCanvasDirIndex(); return listCanvasDirEntries().map((entry) => { @@ -87,14 +86,12 @@ export class DiskSpaceRepository implements SpaceRepository { } async worldId(): Promise { - this.#assertActiveWorkspace(); return this.#requireWorld(); } async create(input: SpaceCreateInput): Promise { - this.#assertActiveWorkspace(); const canvasId = sanitizeId(input.canvasId, 'canvasId'); - assertSpaceMutationAllowed(this.#workspacePath, canvasId); + assertSpaceMutationAllowed(canvasId); // Creation owns membership allocation. Refresh here rather than relying // on a caller having listed first: an externally imported Space must // participate in both stable-id and directory-name collision checks. @@ -130,7 +127,6 @@ export class DiskSpaceRepository implements SpaceRepository { } async beginDelete(input: SpaceDeleteInput): Promise { - this.#assertActiveWorkspace(); const canvasId = sanitizeId(input.canvasId, 'canvasId'); // Revalidate the protected World identity before a destructive session, // as the old composition path did before touching blobs. @@ -139,10 +135,7 @@ export class DiskSpaceRepository implements SpaceRepository { } const store = getCanvasStore(canvasId); - const release = await beginSpaceDeleteAdmission( - this.#workspacePath, - canvasId, - ); + const release = await beginSpaceDeleteAdmission(canvasId); let state: 'open' | 'finishing' | 'closed' = 'open'; const close = (): void => { if (state === 'closed') return; @@ -180,9 +173,8 @@ export class DiskSpaceRepository implements SpaceRepository { } async rename(input: SpaceRenameInput): Promise { - this.#assertActiveWorkspace(); const canvasId = sanitizeId(input.canvasId, 'canvasId'); - assertSpaceMutationAllowed(this.#workspacePath, canvasId); + assertSpaceMutationAllowed(canvasId); if (this.#isWorld(canvasId)) { return { ok: false, reason: 'world-forbidden' }; } @@ -250,15 +242,6 @@ export class DiskSpaceRepository implements SpaceRepository { return requireWorldCanvasId(); } - #assertActiveWorkspace(): void { - if (path.resolve(getWorkspacePath()) !== this.#workspacePath) { - throw new Error( - 'Space repository belongs to an inactive workspace. ' + - 'Resolve a fresh Space repository after workspace activation.', - ); - } - } - #conflictingTitle(directoryName: string): string | null { const entry = listAllCanvasDirEntries().find( (candidate) => 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 35e91e34e..3ae779d1f 100644 --- a/apps/server/src/modules/storage/backends/disk/space-tasks.ts +++ b/apps/server/src/modules/storage/backends/disk/space-tasks.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import path from 'node:path'; - import { taskRecordSchema, taskRunCompletionSchema, @@ -17,7 +15,6 @@ import { import { tasksPath } from './layout.js'; import { readDiskSpaceRecord } from './space-record.js'; import { atomicWriteJson, readJsonStrict } from '../../../../utils/fs.js'; -import { getWorkspacePath } from '../../../workspace.js'; import { assertSpaceMutationAllowed } from '../../space-lifecycle-admission.js'; import type { CanvasStore } from './legacy/canvas-store.js'; @@ -28,6 +25,7 @@ import type { TaskRunUpdate, } from '../../ports/structured.js'; +/** In-flight Task mutations, one chain per Space. */ const taskMutationChains = new Map>(); async function withTaskMutationMutex( @@ -103,11 +101,9 @@ export class DiskSpaceTasks implements SpaceTasks { readonly runs: SpaceTaskRuns; readonly #store: CanvasStore; - readonly #workspacePath: string; constructor(store: CanvasStore) { this.#store = store; - this.#workspacePath = path.resolve(getWorkspacePath()); this.runs = Object.freeze({ create: (run: TaskRunRecord) => this.#createRun(run), update: (runId: string, update: TaskRunUpdate) => @@ -120,16 +116,8 @@ export class DiskSpaceTasks implements SpaceTasks { }); } - #assertActiveWorkspace(): void { - if (path.resolve(getWorkspacePath()) !== this.#workspacePath) { - throw new Error( - `Space Tasks(${this.#store.canvasId}) belong to an inactive workspace`, - ); - } - } - #requireSpace(): void { - assertSpaceMutationAllowed(this.#workspacePath, this.#store.canvasId); + assertSpaceMutationAllowed(this.#store.canvasId); if (!readDiskSpaceRecord(this.#store)) { throw new Error( `Space Tasks(${this.#store.canvasId}) cannot write a missing Space`, @@ -138,7 +126,6 @@ export class DiskSpaceTasks implements SpaceTasks { } async read(): Promise { - this.#assertActiveWorkspace(); return readTaskStore(this.#store.canvasId); } @@ -248,10 +235,7 @@ export class DiskSpaceTasks implements SpaceTasks { } async #mutate(apply: (snapshot: TaskStoreSnapshot) => T): Promise { - this.#assertActiveWorkspace(); - const key = `${this.#workspacePath}\0${this.#store.canvasId}`; - return withTaskMutationMutex(key, () => { - this.#assertActiveWorkspace(); + return withTaskMutationMutex(this.#store.canvasId, () => { this.#requireSpace(); const current = readTaskStore(this.#store.canvasId); const next: TaskStoreSnapshot = { diff --git a/apps/server/src/modules/storage/backends/disk/space-write.ts b/apps/server/src/modules/storage/backends/disk/space-write.ts index 577eed916..913071946 100644 --- a/apps/server/src/modules/storage/backends/disk/space-write.ts +++ b/apps/server/src/modules/storage/backends/disk/space-write.ts @@ -24,12 +24,9 @@ * lock. A comment is not a mechanism. */ -import path from 'node:path'; - import { runCanvasPersistenceTransaction } from './canvas-persistence-transaction.js'; import { canvasFileShapeError } from './space-record-validation.js'; import { readDiskSpaceRecord } from './space-record.js'; -import { getWorkspacePath } from '../../../workspace.js'; import type { CanvasStore } from './legacy/canvas-store.js'; import type { @@ -53,21 +50,9 @@ function nodeMutationError(mutation: SpaceNodeMutation, detail: string): Error { * Bind the ordered write to one Space. * * A closure rather than a class: this is the Space's write action, so the only - * thing it needs to own is the store it writes through and the Workspace it - * was resolved in. + * thing it needs to own is the store it writes through. */ export function createDiskSpaceWrite(store: CanvasStore): SpaceHandle['write'] { - const boundWorkspacePath = path.resolve(getWorkspacePath()); - - function assertActiveWorkspace(): void { - if (path.resolve(getWorkspacePath()) !== boundWorkspacePath) { - throw new Error( - `SpaceWrite(${store.canvasId}) belongs to an inactive workspace. ` + - 'Resolve a fresh Space handle after workspace activation.', - ); - } - } - function validateInput(input: SpaceWriteInput): void { if (!Number.isFinite(input.expectedVersion)) { throw new TypeError('expectedVersion must be a finite number'); @@ -146,7 +131,6 @@ export function createDiskSpaceWrite(store: CanvasStore): SpaceHandle['write'] { return async function write( input: SpaceWriteInput, ): Promise { - assertActiveWorkspace(); validateInput(input); const current = readDiskSpaceRecord(store); 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 72278d483..a86d7e152 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 @@ -288,17 +288,6 @@ describe('Disk Space Tasks', () => { /references missing Task/, ); }); - - it('rejects a retained handle after the active Workspace changes', async () => { - const retained = store.space('canvas-empty').tasks; - const replacement = freshWorkspace('huabu-task-repo-next-'); - - await expect(retained.read()).rejects.toThrow(/inactive workspace/); - - workspaceState.path = root; - resetStorageCache(); - rmSync(replacement, { recursive: true, force: true }); - }); }); /** diff --git a/apps/server/src/modules/storage/backends/disk/structured-store.ts b/apps/server/src/modules/storage/backends/disk/structured-store.ts index e02f25d49..4b0756148 100644 --- a/apps/server/src/modules/storage/backends/disk/structured-store.ts +++ b/apps/server/src/modules/storage/backends/disk/structured-store.ts @@ -7,11 +7,8 @@ * Builds a composite {@link SpaceHandle} on demand over the legacy per-Space * object that `getCanvasStore` already caches. It adds **no cache of its * own**: a second cache would have to be invalidated in lockstep with the - * first, and `resetStorageCache()` — called on workspace switch — clears only - * the legacy map, so a separately cached composite would survive a workspace - * change still wrapping the previous workspace's object. The handle is a few - * field assignments over an object the existing cache returns, so there is - * nothing to gain by caching it twice. + * first, and the handle is a few field assignments over an object the existing + * cache returns, so there is nothing to gain by caching it twice. * * Because the record, log-backed, and node adapters all wrap the *same* legacy * object the compatibility facade resolves, a write through either view is diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index cbcda58cc..4e56a040e 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -37,23 +37,10 @@ import type { } from '../ports/blob.js'; import type { Readable } from 'node:stream'; -const workspaceState = vi.hoisted(() => ({ path: '', leaseCount: 0 })); +const workspaceState = vi.hoisted(() => ({ path: '' })); vi.mock('../../workspace.js', () => ({ getWorkspacePath: () => workspaceState.path, - acquireWorkspaceOperationLease: () => { - const workspacePath = workspaceState.path; - workspaceState.leaseCount += 1; - let released = false; - return Object.freeze({ - workspacePath, - release: () => { - if (released) return; - released = true; - workspaceState.leaseCount -= 1; - }, - }); - }, })); function writeCanvas(directory: string, canvasId: string, title: string): void { @@ -204,7 +191,6 @@ function installBlobStore(next: BlobStore): void { beforeEach(() => { workspaceState.path = mkdtempSync(path.join(tmpdir(), 'huabu-delete-')); - workspaceState.leaseCount = 0; writeCanvas('.world', 'canvas-world', 'World'); writeCanvas('Project A', 'canvas-a', 'Project A'); refreshCanvasDirIndex(); @@ -219,7 +205,6 @@ beforeEach(() => { }); afterEach(() => { - expect(workspaceState.leaseCount).toBe(0); restoreStorage(); resetStorageCache(); rmSync(workspaceState.path, { recursive: true, force: true }); @@ -348,7 +333,6 @@ describe('deleteSpace composition', () => { const deleting = deleteSpace('canvas-a'); await controlled.deleteStarted.promise; - expect(workspaceState.leaseCount).toBe(1); const putting = canvasBlobs('canvas-a').put( 'too-late.bin', Buffer.from('orphan'), @@ -359,7 +343,6 @@ describe('deleteSpace composition', () => { controlled.releaseDeletes(); await expect(deleting).resolves.toEqual({ ok: true, reason: 'deleted' }); - expect(workspaceState.leaseCount).toBe(0); await expect(putting).rejects.toThrow(/missing Space/); expect(controlled.putCalls).toBe(0); diff --git a/apps/server/src/modules/storage/index.ts b/apps/server/src/modules/storage/index.ts index 88d251d1b..3cd102fd0 100644 --- a/apps/server/src/modules/storage/index.ts +++ b/apps/server/src/modules/storage/index.ts @@ -69,6 +69,7 @@ export { getStorage, getStructuredStore, initStorage, + resetStorage, setStorageForTesting, spaceDirectory, storageHealth, diff --git a/apps/server/src/modules/storage/ports/structured.ts b/apps/server/src/modules/storage/ports/structured.ts index 274074304..e46c573e5 100644 --- a/apps/server/src/modules/storage/ports/structured.ts +++ b/apps/server/src/modules/storage/ports/structured.ts @@ -72,10 +72,9 @@ export interface StructuredStore { /** * Return a repository for the currently-bound Space collection. * - * Handles are scoped to the backend namespace that was active when they - * were created. A caller that changes Workspace must resolve a fresh - * handle; retained Disk handles reject instead of reading the newly active - * Workspace. + * A production process serves one Workspace for its lifetime, so every + * derived handle belongs to that fixed namespace. Tests that reset the + * process-local Workspace must resolve fresh handles afterwards. */ spaces(): SpaceRepository; /** diff --git a/apps/server/src/modules/storage/space-lifecycle-admission.ts b/apps/server/src/modules/storage/space-lifecycle-admission.ts index 260e7f8ab..650c69de9 100644 --- a/apps/server/src/modules/storage/space-lifecycle-admission.ts +++ b/apps/server/src/modules/storage/space-lifecycle-admission.ts @@ -96,64 +96,51 @@ class SpaceLifecycleGate { const gates = new Map(); -function key(workspacePath: string, canvasId: string): string { - return `${workspacePath}\0${canvasId}`; -} - -function gateFor(workspacePath: string, canvasId: string): SpaceLifecycleGate { - const gateKey = key(workspacePath, canvasId); - let gate = gates.get(gateKey); +/** + * Keyed by Space alone. + * + * A process serves one Workspace for its lifetime, so a Space id already + * denotes one Space; a Workspace component in the key would distinguish + * nothing. + */ +function gateFor(canvasId: string): SpaceLifecycleGate { + let gate = gates.get(canvasId); if (!gate) { gate = new SpaceLifecycleGate(); - gates.set(gateKey, gate); + gates.set(canvasId, gate); } return gate; } -async function withPutAdmission( - workspacePath: string, +export async function withSpacePutAdmission( canvasId: string, operation: () => Promise, ): Promise { - const gateKey = key(workspacePath, canvasId); - const gate = gateFor(workspacePath, canvasId); + const gate = gateFor(canvasId); try { return await gate.withPut(operation); } finally { - if (gate.idle && gates.get(gateKey) === gate) gates.delete(gateKey); + if (gate.idle && gates.get(canvasId) === gate) gates.delete(canvasId); } } -export function withSpacePutAdmission( - workspacePath: string, - canvasId: string, - operation: () => Promise, -): Promise { - return withPutAdmission(workspacePath, canvasId, operation); -} - export async function beginSpaceDeleteAdmission( - workspacePath: string, canvasId: string, ): Promise<() => void> { - const gateKey = key(workspacePath, canvasId); - const gate = gateFor(workspacePath, canvasId); + const gate = gateFor(canvasId); const releaseGate = await gate.acquireDelete(); let released = false; return () => { if (released) return; released = true; releaseGate(); - if (gate.idle && gates.get(gateKey) === gate) gates.delete(gateKey); + if (gate.idle && gates.get(canvasId) === gate) gates.delete(canvasId); }; } /** Reject a structured mutation once deletion is active or queued. */ -export function assertSpaceMutationAllowed( - workspacePath: string, - canvasId: string, -): void { - if (gates.get(key(workspacePath, canvasId))?.deletionPending) { +export function assertSpaceMutationAllowed(canvasId: string): void { + if (gates.get(canvasId)?.deletionPending) { throw new Error( `Cannot mutate Space "${canvasId}" while deletion is pending`, ); diff --git a/apps/server/src/modules/storage/storage.ts b/apps/server/src/modules/storage/storage.ts index c11a93e0b..06c83b5cf 100644 --- a/apps/server/src/modules/storage/storage.ts +++ b/apps/server/src/modules/storage/storage.ts @@ -21,14 +21,10 @@ * through it. */ -import path from 'node:path'; - -import { - acquireWorkspaceOperationLease, - getWorkspacePath, -} from '../workspace.js'; import { DiskBlobStore } from './backends/disk/blob-store.js'; import { canvasRoot } from './backends/disk/layout.js'; +import { resetStorageCache } from './backends/disk/legacy/canvas-store-cache.js'; +import { clearAllNodeTombstones } from './backends/disk/legacy/node-tombstones.js'; import { DiskStructuredStore } from './backends/disk/structured-store.js'; import { parseStorageProfile, @@ -66,19 +62,6 @@ export type SpaceDeleteOutcome = | SpaceDeleteFinishResult | { readonly ok: false; readonly reason: 'world-forbidden' }; -function activeWorkspacePath(): string { - return path.resolve(getWorkspacePath()); -} - -function assertActiveWorkspace(workspacePath: string, canvasId: string): void { - if (activeWorkspacePath() !== workspacePath) { - throw new Error( - `Blob scope for Space "${canvasId}" belongs to an inactive workspace. ` + - `Resolve a fresh scope after workspace activation.`, - ); - } -} - /** * Release a rejected streaming body that storage never fully consumed. * @@ -208,28 +191,19 @@ export function getStructuredStore(): StructuredStore { * Create one ordinary Space through the selected structured backend. * * Default-title allocation and lifecycle creation share one process-local - * serialization point. The Workspace lease is acquired before queueing, so - * an async catalogue read cannot strand the request in a newly activated - * Workspace and concurrent defaults remain Untitled, Untitled (1), ... . + * serialization point, so concurrent defaults remain Untitled, Untitled (1), + * ... rather than racing for the same name. */ export function createSpace( canvasId: string, title?: string | null, ): Promise { const structured = ensure().structured; - const workspaceLease = acquireWorkspaceOperationLease(); return serializeSpaceCreate(async () => { - try { - // One repository instance spans the read and the create, so a Workspace - // switch between them is rejected by the handle rather than silently - // creating the Space in the newly activated Workspace. - const spaces = structured.spaces(); - const effectiveTitle = - title === undefined ? defaultSpaceTitle(await spaces.list()) : title; - return await spaces.create({ canvasId, title: effectiveTitle }); - } finally { - workspaceLease.release(); - } + const spaces = structured.spaces(); + const effectiveTitle = + title === undefined ? defaultSpaceTitle(await spaces.list()) : title; + return spaces.create({ canvasId, title: effectiveTitle }); }); } @@ -245,22 +219,17 @@ export function createSpace( export async function deleteSpace( canvasId: string, ): Promise { - const workspaceLease = acquireWorkspaceOperationLease(); + const storage = ensure(); + const started = await storage.structured.spaces().beginDelete({ canvasId }); + if (!started.ok) return started; try { - const storage = ensure(); - const started = await storage.structured.spaces().beginDelete({ canvasId }); - if (!started.ok) return started; - try { - // Preserve the old retryable cleanup behavior: sweep even when the - // structured record is already absent, so orphan blobs can be removed. - await storage.blobs.scope({ kind: 'canvas', canvasId }).deleteAll(); - return await started.session.finish(); - } catch (error) { - await started.session.abort(); - throw error; - } - } finally { - workspaceLease.release(); + // Preserve the old retryable cleanup behavior: sweep even when the + // structured record is already absent, so orphan blobs can be removed. + await storage.blobs.scope({ kind: 'canvas', canvasId }).deleteAll(); + return await started.session.finish(); + } catch (error) { + await started.session.abort(); + throw error; } } @@ -274,7 +243,6 @@ export async function deleteSpace( */ export function canvasBlobs(canvasId: string): BlobScope { const storage = ensure(); - const workspacePath = activeWorkspacePath(); const delegate = storage.blobs.scope({ kind: 'canvas', canvasId }); async function requireSpace(): Promise { @@ -287,16 +255,10 @@ export function canvasBlobs(canvasId: string): BlobScope { return { async put(name: string, body: Readable | Buffer): Promise { try { - return await withSpacePutAdmission( - workspacePath, - canvasId, - async () => { - assertActiveWorkspace(workspacePath, canvasId); - await requireSpace(); - assertActiveWorkspace(workspacePath, canvasId); - return delegate.put(name, body); - }, - ); + return await withSpacePutAdmission(canvasId, async () => { + await requireSpace(); + return delegate.put(name, body); + }); } catch (error) { drainRejectedBody(body); throw error; @@ -350,6 +312,20 @@ export function spaceDirectory(canvasId: string): string { return canvasRoot(canvasId); } +/** + * Drop everything the process built against whichever workspace was active. + * + * The Disk adapter's instance cache and its node fences go together: both + * describe the workspace being served, and neither carries an answer to + * *which* one, because a process only ever has the one. Called by + * `commitWorkspacePath`, which is what keeps that true for a test moving + * through several temporary workspaces. + */ +export function resetStorage(): void { + resetStorageCache(); + clearAllNodeTombstones(); +} + /** * Swap the active storage, returning a restore function. * diff --git a/apps/server/src/modules/workspace-activation.test.ts b/apps/server/src/modules/workspace-activation.test.ts index ac05eadca..c5fe4111c 100644 --- a/apps/server/src/modules/workspace-activation.test.ts +++ b/apps/server/src/modules/workspace-activation.test.ts @@ -10,8 +10,14 @@ import { runWorkspacePreparation, WorkspaceActivationInProgressError, WorkspaceActivationTimeoutError, + WorkspaceRestartRequiredError, } from './workspace-activation.js'; -import { getWorkspacePath, setWorkspacePath } from './workspace.js'; +import { + clearWorkspacePath, + getWorkspacePath, + isWorkspaceConfigured, + setWorkspacePath, +} from './workspace.js'; describe('workspace activation isolation', () => { const roots: string[] = []; @@ -29,7 +35,14 @@ describe('workspace activation isolation', () => { return file; } + beforeEach(() => { + // Every case decides for itself whether a workspace is already active, + // because that is the branch under test. + clearWorkspacePath(); + }); + afterAll(() => { + clearWorkspacePath(); for (const root of roots) { rmSync(root, { recursive: true, force: true }); } @@ -55,16 +68,74 @@ describe('workspace activation isolation', () => { ).rejects.toBeInstanceOf(WorkspaceActivationTimeoutError); }); - it('keeps the previous workspace active after preparation times out', async () => { - const previous = tempDir('huabu-workspace-previous-'); + it('leaves the process unconfigured when preparation times out', async () => { const next = tempDir('huabu-workspace-next-'); const workerPath = worker(`setInterval(() => {}, 1_000);`); - setWorkspacePath(previous); await expect( activateWorkspacePath(next, { workerPath, timeoutMs: 30 }), ).rejects.toBeInstanceOf(WorkspaceActivationTimeoutError); - expect(getWorkspacePath()).toBe(path.resolve(previous)); + // Nothing half-activated: the client sees the same state it started from + // and can pick again. + expect(isWorkspaceConfigured()).toBe(false); + }); + + /** + * The restart rule (issue #126). A process serves one workspace, so the + * second choice is validated without changing the active process, then the + * client persists it for the next launch. + */ + it('validates a different workspace before requiring a restart', async () => { + const active = tempDir('huabu-workspace-active-'); + const other = tempDir('huabu-workspace-other-'); + setWorkspacePath(active); + const workerPath = worker(`process.send({ ok: true });`); + + const refusal = activateWorkspacePath(other, { workerPath }); + await expect(refusal).rejects.toBeInstanceOf(WorkspaceRestartRequiredError); + await expect(refusal).rejects.toMatchObject({ + requestedPath: path.resolve(other), + }); + expect(getWorkspacePath()).toBe(path.resolve(active)); + }); + + it('does not request a restart when validating the next workspace fails', async () => { + const active = tempDir('huabu-workspace-active-'); + const other = tempDir('huabu-workspace-other-'); + setWorkspacePath(active); + const workerPath = worker( + `process.send({ ok: false, message: 'target cannot be prepared' });`, + ); + + await expect(activateWorkspacePath(other, { workerPath })).rejects.toThrow( + 'target cannot be prepared', + ); + expect(getWorkspacePath()).toBe(path.resolve(active)); + }); + + it('bounds validation of the next workspace without changing the active one', async () => { + const active = tempDir('huabu-workspace-active-'); + const other = tempDir('huabu-workspace-other-'); + setWorkspacePath(active); + const workerPath = worker(`setInterval(() => {}, 1_000);`); + + await expect( + activateWorkspacePath(other, { workerPath, timeoutMs: 30 }), + ).rejects.toBeInstanceOf(WorkspaceActivationTimeoutError); + expect(getWorkspacePath()).toBe(path.resolve(active)); + }); + + it('accepts the active workspace again without reactivating it', async () => { + const active = tempDir('huabu-workspace-idempotent-'); + setWorkspacePath(active); + const workerPath = worker(`process.send({ ok: false, message: 'ran' });`); + + // The client re-sends its remembered path on every boot, and a second tab + // must not be told to restart. + await expect( + activateWorkspacePath(active, { workerPath }), + ).resolves.toBeUndefined(); + expect(getWorkspacePath()).toBe(path.resolve(active)); }); it('rejects a concurrent activation while preparation is running', async () => { diff --git a/apps/server/src/modules/workspace-activation.ts b/apps/server/src/modules/workspace-activation.ts index ca2144d05..24db8cb43 100644 --- a/apps/server/src/modules/workspace-activation.ts +++ b/apps/server/src/modules/workspace-activation.ts @@ -10,50 +10,22 @@ * preparation is committed to the Server's in-process workspace state. */ -import { fork, type ChildProcess } from 'node:child_process'; -import { existsSync } from 'node:fs'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; - +import { + runWorkspacePreparation, + type WorkspacePreparationOptions, +} from './workspace-preparation-process.js'; import { commitWorkspacePath, + getWorkspacePath, isManagedMode, + isWorkspaceConfigured, resolveWorkspacePath, } from './workspace.js'; -import { getLogger } from '../utils/logger.js'; - -const DEFAULT_ACTIVATION_TIMEOUT_MS = 70_000; - -/** - * Grace period between the polite `SIGTERM` and a forced `SIGKILL` when a - * preparation child overruns its timeout. `SIGTERM` alone cannot dislodge a - * process blocked in an uninterruptible syscall (the exact hung cloud/network - * mount this isolation defends against), so we escalate. - */ -const FORCE_KILL_GRACE_MS = 2_000; - -/** Cap on retained child stderr so a chatty failure cannot grow unbounded. */ -const MAX_STDERR_CHARS = 8_192; - -const log = getLogger('workspace-activation'); - -type PreparationResult = { ok: true } | { ok: false; message: string }; - -export class WorkspaceActivationTimeoutError extends Error { - /** Configured timeout in whole seconds, surfaced to the UI copy. */ - readonly timeoutSeconds: number; - constructor(timeoutMs: number) { - // Round up and clamp to >= 1 so sub-second timeouts never render as an - // awkward "0 seconds" and the copy never understates the real budget. - const timeoutSeconds = Math.max(1, Math.ceil(timeoutMs / 1000)); - super( - `Workspace activation timed out after ${timeoutSeconds} seconds. The folder may be on an unavailable or slow cloud/network drive.`, - ); - this.name = 'WorkspaceActivationTimeoutError'; - this.timeoutSeconds = timeoutSeconds; - } -} +export { + runWorkspacePreparation, + WorkspaceActivationTimeoutError, +} from './workspace-preparation-process.js'; export class WorkspaceActivationInProgressError extends Error { constructor() { @@ -62,138 +34,67 @@ export class WorkspaceActivationInProgressError extends Error { } } -interface PreparationOptions { - timeoutMs?: number; - workerPath?: string; -} - -let activationInProgress = false; +/** + * Raised when the caller asks for a workspace other than the active one. + * + * Not a failure of the request — the choice is valid and the client should + * persist it. What cannot happen is this process serving it: caches, directory + * indexes, and open handles are built against one workspace and the machinery + * to move a live process between two of them costs more than the feature is + * worth (issue #126). The client saves the path and restarts. + */ +export class WorkspaceRestartRequiredError extends Error { + /** The workspace the caller asked for, to persist for the next launch. */ + readonly requestedPath: string; -function defaultWorkerPath(): string { - const currentFile = fileURLToPath(import.meta.url); - const bundledWorker = path.join( - path.dirname(currentFile), - 'workspace-prepare.worker.js', - ); - if (existsSync(bundledWorker)) return bundledWorker; - return fileURLToPath( - new URL('./workspace-prepare.worker.ts', import.meta.url), - ); + constructor(requestedPath: string) { + super( + 'Changing the workspace takes effect after a restart. The selection has ' + + 'been validated; save it and start the server again to open it.', + ); + this.name = 'WorkspaceRestartRequiredError'; + this.requestedPath = requestedPath; + } } -/** Run all potentially blocking filesystem preparation outside the Server. */ -export function runWorkspacePreparation( - workspacePath: string, - options: PreparationOptions = {}, -): Promise { - const timeoutMs = options.timeoutMs ?? DEFAULT_ACTIVATION_TIMEOUT_MS; - const workerPath = options.workerPath ?? defaultWorkerPath(); - - return new Promise((resolve, reject) => { - let child: ChildProcess; - let settled = false; - let forceKillTimer: NodeJS.Timeout | undefined; - let stderr = ''; - - const settle = (error?: Error): void => { - if (settled) return; - settled = true; - clearTimeout(timer); - if (error) reject(error); - else resolve(); - }; - - try { - // Pipe stderr so an import-time crash in the worker (before its own - // try/catch runs) is captured for diagnosis instead of vanishing. - child = fork(workerPath, [workspacePath], { - stdio: ['ignore', 'ignore', 'pipe', 'ipc'], - }); - } catch (error) { - reject(error); - return; - } - - child.stderr?.on('data', (chunk: Buffer) => { - stderr = (stderr + chunk.toString('utf8')).slice(-MAX_STDERR_CHARS); - }); - - const timer = setTimeout(() => { - // Escalate SIGTERM -> SIGKILL: a child wedged in an uninterruptible - // filesystem syscall ignores SIGTERM, so force-kill after a grace - // period to guarantee the orphaned process is reaped. - child.kill(); - forceKillTimer = setTimeout( - () => child.kill('SIGKILL'), - FORCE_KILL_GRACE_MS, - ); - forceKillTimer.unref(); - settle(new WorkspaceActivationTimeoutError(timeoutMs)); - }, timeoutMs); - timer.unref(); - - child.once('message', (raw: unknown) => { - const result = raw as PreparationResult; - // Proactively close the IPC channel so a worker that reports its result - // but forgets to `process.disconnect()` / `process.exit()` (e.g. a - // minimal test helper) still loses the handle keeping it alive and can - // exit, instead of lingering as an orphan. - try { - child.disconnect(); - } catch { - // Already disconnected by a well-behaved worker; nothing to do. - } - if (result?.ok === true) { - settle(); - } else { - settle( - new Error( - result && typeof result.message === 'string' - ? result.message - : 'Workspace preparation failed', - ), - ); - } - }); - child.once('error', (error) => settle(error)); - child.once('exit', (code, signal) => { - // The child has been reaped; cancel any pending force-kill. - if (forceKillTimer) clearTimeout(forceKillTimer); - if (settled) return; - const detail = stderr.trim(); - if (detail) { - log.error( - { workspacePath, code, signal, detail }, - 'Workspace preparation crashed', - ); - } - settle( - new Error( - `Workspace preparation process exited before completion (${signal ?? code ?? 'unknown'})${detail ? `: ${detail}` : ''}`, - ), - ); - }); - }); -} +let activationInProgress = false; -/** Prepare a free-mode workspace and commit it only after full success. */ +/** + * Prepare and adopt the one workspace this process will serve. + * + * Free mode's first — and only — activation: the disposable child prepares and + * migrates the directory, and the path is committed once that has succeeded. + * A failure commits nothing, leaving the process unconfigured, which is the + * state the client already knows how to recover from. + * + * Asking for a *different* workspace once one is active prepares it in the + * disposable child to prove it is usable, then raises + * {@link WorkspaceRestartRequiredError} without changing process-local state. + * Asking for the active one again is a no-op, because the client re-sends its + * remembered path on every boot and a second tab must not be told to restart. + */ export async function activateWorkspacePath( newPath: string, - options: PreparationOptions = {}, + options: WorkspacePreparationOptions = {}, ): Promise { if (isManagedMode()) { throw new Error( 'Server is in managed mode; the workspace is fixed at startup', ); } + const resolvedPath = resolveWorkspacePath(newPath); + const activePath = isWorkspaceConfigured() ? getWorkspacePath() : null; + if (activePath === resolvedPath) return; if (activationInProgress) { throw new WorkspaceActivationInProgressError(); } - const resolvedPath = resolveWorkspacePath(newPath); activationInProgress = true; try { await runWorkspacePreparation(resolvedPath, options); + if (activePath !== null) { + throw new WorkspaceRestartRequiredError(resolvedPath); + } commitWorkspacePath(resolvedPath); } finally { activationInProgress = false; diff --git a/apps/server/src/modules/workspace-preparation-process.ts b/apps/server/src/modules/workspace-preparation-process.ts new file mode 100644 index 000000000..cd26586a9 --- /dev/null +++ b/apps/server/src/modules/workspace-preparation-process.ts @@ -0,0 +1,141 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Isolated workspace preparation. + * + * Workspace preparation includes synchronous filesystem calls and migrations. + * Cloud drives and network filesystems can block those calls indefinitely, so + * every production path runs them in a disposable child with a hard timeout. + */ + +import { fork, type ChildProcess } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { getLogger } from '../utils/logger.js'; + +const DEFAULT_PREPARATION_TIMEOUT_MS = 70_000; +const FORCE_KILL_GRACE_MS = 2_000; +const MAX_STDERR_CHARS = 8_192; + +const log = getLogger('workspace-preparation'); + +type PreparationResult = { ok: true } | { ok: false; message: string }; + +export interface WorkspacePreparationOptions { + timeoutMs?: number; + workerPath?: string; +} + +export class WorkspaceActivationTimeoutError extends Error { + /** Configured timeout in whole seconds, surfaced to the UI copy. */ + readonly timeoutSeconds: number; + + constructor(timeoutMs: number) { + const timeoutSeconds = Math.max(1, Math.ceil(timeoutMs / 1000)); + super( + `Workspace activation timed out after ${timeoutSeconds} seconds. The folder may be on an unavailable or slow cloud/network drive.`, + ); + this.name = 'WorkspaceActivationTimeoutError'; + this.timeoutSeconds = timeoutSeconds; + } +} + +function defaultWorkerPath(): string { + const currentFile = fileURLToPath(import.meta.url); + const bundledWorker = path.join( + path.dirname(currentFile), + 'workspace-prepare.worker.js', + ); + if (existsSync(bundledWorker)) return bundledWorker; + return fileURLToPath( + new URL('./workspace-prepare.worker.ts', import.meta.url), + ); +} + +/** Run all potentially blocking filesystem preparation outside the server. */ +export function runWorkspacePreparation( + workspacePath: string, + options: WorkspacePreparationOptions = {}, +): Promise { + const timeoutMs = options.timeoutMs ?? DEFAULT_PREPARATION_TIMEOUT_MS; + const workerPath = options.workerPath ?? defaultWorkerPath(); + + return new Promise((resolve, reject) => { + let child: ChildProcess; + let settled = false; + let forceKillTimer: NodeJS.Timeout | undefined; + let stderr = ''; + + const settle = (error?: Error): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + if (error) reject(error); + else resolve(); + }; + + try { + child = fork(workerPath, [workspacePath], { + stdio: ['ignore', 'ignore', 'pipe', 'ipc'], + }); + } catch (error) { + reject(error); + return; + } + + child.stderr?.on('data', (chunk: Buffer) => { + stderr = (stderr + chunk.toString('utf8')).slice(-MAX_STDERR_CHARS); + }); + + const timer = setTimeout(() => { + child.kill(); + forceKillTimer = setTimeout( + () => child.kill('SIGKILL'), + FORCE_KILL_GRACE_MS, + ); + forceKillTimer.unref(); + settle(new WorkspaceActivationTimeoutError(timeoutMs)); + }, timeoutMs); + timer.unref(); + + child.once('message', (raw: unknown) => { + const result = raw as PreparationResult; + try { + child.disconnect(); + } catch { + // Already disconnected by a well-behaved worker. + } + if (result?.ok === true) { + settle(); + } else { + settle( + new Error( + result && typeof result.message === 'string' + ? result.message + : 'Workspace preparation failed', + ), + ); + } + }); + child.once('error', (error) => settle(error)); + child.once('exit', (code, signal) => { + if (forceKillTimer) clearTimeout(forceKillTimer); + if (settled) return; + const detail = stderr.trim(); + if (detail) { + log.error( + { workspacePath, code, signal, detail }, + 'Workspace preparation crashed', + ); + } + settle( + new Error( + `Workspace preparation process exited before completion (${signal ?? code ?? 'unknown'})${detail ? `: ${detail}` : ''}`, + ), + ); + }); + }); +} diff --git a/apps/server/src/modules/workspace-prepare.ts b/apps/server/src/modules/workspace-prepare.ts index c04514d40..2ebbad6ce 100644 --- a/apps/server/src/modules/workspace-prepare.ts +++ b/apps/server/src/modules/workspace-prepare.ts @@ -4,9 +4,9 @@ /** * Synchronous, on-disk workspace preparation. * - * Runtime workspace switches execute this function in a disposable child - * process so a slow virtual filesystem cannot block the Server event loop. - * Startup and tests may still call it in-process through `setWorkspacePath`. + * Production startup and free-mode activation execute this function in a + * disposable child so a slow virtual filesystem cannot block the Server event + * loop. Tests may still call it in-process through `setWorkspacePath`. */ import { mkdirSync } from 'node:fs'; diff --git a/apps/server/src/modules/workspace.route.ts b/apps/server/src/modules/workspace.route.ts index 1a0f98353..8495ed6e7 100644 --- a/apps/server/src/modules/workspace.route.ts +++ b/apps/server/src/modules/workspace.route.ts @@ -8,16 +8,17 @@ import path from 'node:path'; import { validatePathSchema, workspacePathSchema } from '@huabu/shared'; -import { resetPreprocessDispatcher } from './preprocessing/index.js'; -import { getStructuredStore, resetStorageCache } from './storage/index.js'; +import { getStructuredStore } from './storage/index.js'; import { activateWorkspacePath, WorkspaceActivationInProgressError, WorkspaceActivationTimeoutError, + WorkspaceRestartRequiredError, } from './workspace-activation.js'; import { getWorkspaceName, getWorkspacePath, + getWorkspaceStartupError, isManagedMode, isWorkspaceConfigured, } from './workspace.js'; @@ -166,6 +167,7 @@ async function buildWorkspaceState(): Promise { worldCanvasId: configured ? await getStructuredStore().spaces().worldId() : null, + startupError: getWorkspaceStartupError(), capabilities: { canChangeWorkspace: !managed, nativePicker: !managed && canShowNativePicker(), @@ -183,8 +185,10 @@ const workspaceRoutes: FastifyPluginAsync = async (app) => { ); // ──────────────────────────────────────────────────────────── - // The endpoints below mutate the active workspace and only exist - // in free mode. Managed mode rejects them with 403. + // The endpoints below administer free-mode workspace selection. PUT + // activates the first path in this process or validates a later choice for + // restart; it never switches an already-active process. Managed mode rejects + // all of them with 403. // ──────────────────────────────────────────────────────────── app.post<{ Reply: ApiResult }>( @@ -262,9 +266,6 @@ const workspaceRoutes: FastifyPluginAsync = async (app) => { } try { await activateWorkspacePath(parsed.data.path); - // Reset singletons that cache filesystem handles for the old workspace. - resetStorageCache(); - resetPreprocessDispatcher(); return await buildWorkspaceState(); } catch (e) { if (e instanceof WorkspaceActivationTimeoutError) { @@ -286,6 +287,13 @@ const workspaceRoutes: FastifyPluginAsync = async (app) => { 'WORKSPACE_ACTIVATION_IN_PROGRESS', ); } + // Not a rejection of the choice — the path is valid and the client + // should save it. This process just cannot be the one to open it. + if (e instanceof WorkspaceRestartRequiredError) { + return sendError(reply, 409, e.message, 'WORKSPACE_RESTART_REQUIRED', { + path: e.requestedPath, + }); + } return sendError(reply, 400, (e as Error).message); } }); diff --git a/apps/server/src/modules/workspace.test.ts b/apps/server/src/modules/workspace.test.ts index 882a36fde..a6da3c3e0 100644 --- a/apps/server/src/modules/workspace.test.ts +++ b/apps/server/src/modules/workspace.test.ts @@ -1,19 +1,28 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +/** + * Adopting the workspace a process was started on. + * + * The two env forms exist to differ in one place — what a failure means — so + * every case here is about that difference, or about the state a client is + * left in when a remembered workspace cannot be opened (issue #126). + */ + +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { - acquireWorkspaceOperationLease, - commitWorkspacePath, + clearWorkspacePath, getWorkspacePath, - setWorkspacePath, - WorkspaceOperationInProgressError, + getWorkspaceStartupError, + initWorkspaceFromEnv, + isManagedMode, + isWorkspaceConfigured, } from './workspace.js'; -describe('workspace operation leases', () => { +describe('startup workspace adoption', () => { const roots: string[] = []; function tempDir(prefix: string): string { @@ -22,57 +31,136 @@ describe('workspace operation leases', () => { return dir; } - afterAll(() => { + /** A path *inside a file*, so creating the directory fails with ENOTDIR. */ + function unopenable(): string { + const blocker = path.join(tempDir('huabu-workspace-blocked-'), 'not-a-dir'); + writeFileSync(blocker, 'occupied', 'utf8'); + return path.join(blocker, 'workspace'); + } + + function worker(source: string): string { + const file = path.join(tempDir('huabu-workspace-worker-'), 'worker.mjs'); + writeFileSync(file, source, 'utf8'); + return file; + } + + function successfulWorker(): string { + return worker(`process.send({ ok: true });`); + } + + function failingWorker(message: string): string { + return worker( + `process.send({ ok: false, message: ${JSON.stringify(message)} });`, + ); + } + + beforeEach(() => { + delete process.env.HUABU_WORKSPACE; + delete process.env.HUABU_WORKSPACE_STARTUP; + clearWorkspacePath(); + }); + + afterAll(async () => { + delete process.env.HUABU_WORKSPACE; + delete process.env.HUABU_WORKSPACE_STARTUP; + await initWorkspaceFromEnv({ workerPath: successfulWorker() }); + clearWorkspacePath(); for (const root of roots) { rmSync(root, { recursive: true, force: true }); } }); - it('blocks a commit to another workspace until every lease is released', () => { - const current = tempDir('huabu-workspace-current-'); - const next = tempDir('huabu-workspace-next-'); - expect(() => setWorkspacePath(current)).not.toThrow(); + it('starts unconfigured when neither variable is set', async () => { + await initWorkspaceFromEnv({ workerPath: successfulWorker() }); - const first = acquireWorkspaceOperationLease(); - const second = acquireWorkspaceOperationLease(); - expect(first.workspacePath).toBe(path.resolve(current)); - expect(second.workspacePath).toBe(path.resolve(current)); + expect(isManagedMode()).toBe(false); + expect(isWorkspaceConfigured()).toBe(false); + expect(getWorkspaceStartupError()).toBeNull(); + }); - expect(() => commitWorkspacePath(path.resolve(next))).toThrow( - WorkspaceOperationInProgressError, - ); - expect(getWorkspacePath()).toBe(path.resolve(current)); + it('locks the operator-named workspace and prepares it', async () => { + const root = tempDir('huabu-workspace-managed-'); + process.env.HUABU_WORKSPACE = root; - first.release(); - first.release(); - expect(() => commitWorkspacePath(path.resolve(next))).toThrow( - WorkspaceOperationInProgressError, - ); + await initWorkspaceFromEnv({ workerPath: successfulWorker() }); + + expect(isManagedMode()).toBe(true); + expect(getWorkspacePath()).toBe(path.resolve(root)); + }); + + it('adopts a shell-chosen workspace without locking it', async () => { + const root = tempDir('huabu-workspace-startup-'); + process.env.HUABU_WORKSPACE_STARTUP = root; + + await initWorkspaceFromEnv({ workerPath: successfulWorker() }); + + // Free mode: the path is the user's own choice, so the picker stays + // available and the path stays visible. + expect(isManagedMode()).toBe(false); + expect(getWorkspacePath()).toBe(path.resolve(root)); + expect(getWorkspaceStartupError()).toBeNull(); + }); + + it('prefers the operator variable when both are set', async () => { + const managed = tempDir('huabu-workspace-both-managed-'); + const shell = tempDir('huabu-workspace-both-shell-'); + process.env.HUABU_WORKSPACE = managed; + process.env.HUABU_WORKSPACE_STARTUP = shell; + + await initWorkspaceFromEnv({ workerPath: successfulWorker() }); + + expect(isManagedMode()).toBe(true); + expect(getWorkspacePath()).toBe(path.resolve(managed)); + }); + + it('fails startup when the operator names a workspace it cannot open', async () => { + const blocked = unopenable(); + process.env.HUABU_WORKSPACE = blocked; - second.release(); - expect(() => commitWorkspacePath(path.resolve(next))).not.toThrow(); - expect(getWorkspacePath()).toBe(path.resolve(next)); + // A deployment misconfiguration. Coming up unconfigured instead would + // offer a remote user a folder picker for the host filesystem. + await expect( + initWorkspaceFromEnv({ + workerPath: failingWorker('workspace cannot be prepared'), + }), + ).rejects.toThrow('workspace cannot be prepared'); + expect(existsSync(blocked)).toBe(false); }); - it('allows same-path activation but rejects setWorkspacePath before preparing another path', () => { - const current = tempDir('huabu-workspace-current-'); - const parent = tempDir('huabu-workspace-parent-'); - const next = path.join(parent, 'not-created'); - setWorkspacePath(current); + it('recovers to the picker when a shell-chosen workspace cannot be opened', async () => { + process.env.HUABU_WORKSPACE_STARTUP = unopenable(); - const lease = acquireWorkspaceOperationLease(); - expect(() => setWorkspacePath(path.join(current, '.'))).not.toThrow(); - expect(getWorkspacePath()).toBe(path.resolve(current)); + await initWorkspaceFromEnv({ + workerPath: failingWorker('workspace cannot be prepared'), + }); + + // The user's folder moved, was renamed, or lives on a drive that is not + // mounted today. The recovery for that is picking another one, so the + // process serves nothing and says why rather than dying. + expect(isWorkspaceConfigured()).toBe(false); + expect(getWorkspaceStartupError()).toBeTruthy(); + }); - expect(() => setWorkspacePath(next)).toThrow( - WorkspaceOperationInProgressError, + it('bounds a blocked shell-chosen workspace and recovers to the picker', async () => { + process.env.HUABU_WORKSPACE_STARTUP = tempDir( + 'huabu-workspace-blocked-startup-', ); - expect(existsSync(next)).toBe(false); - expect(getWorkspacePath()).toBe(path.resolve(current)); - lease.release(); - expect(() => setWorkspacePath(next)).not.toThrow(); - expect(existsSync(next)).toBe(true); - expect(getWorkspacePath()).toBe(path.resolve(next)); + await initWorkspaceFromEnv({ + workerPath: worker(`setInterval(() => {}, 1_000);`), + timeoutMs: 30, + }); + + expect(isWorkspaceConfigured()).toBe(false); + expect(getWorkspaceStartupError()).toMatch(/timed out/i); + }); + + it('rejects a relative path in either variable', async () => { + process.env.HUABU_WORKSPACE_STARTUP = 'relative/path'; + await expect(initWorkspaceFromEnv()).rejects.toThrow(/absolute/); + + delete process.env.HUABU_WORKSPACE_STARTUP; + process.env.HUABU_WORKSPACE = 'relative/path'; + await expect(initWorkspaceFromEnv()).rejects.toThrow(/absolute/); }); }); diff --git a/apps/server/src/modules/workspace.ts b/apps/server/src/modules/workspace.ts index 235042a92..de56e6561 100644 --- a/apps/server/src/modules/workspace.ts +++ b/apps/server/src/modules/workspace.ts @@ -4,26 +4,38 @@ /** * Centralised workspace path management. * - * Two operating modes, decided at process startup by the presence of the - * `HUABU_WORKSPACE` environment variable: + * **A process serves one workspace.** Once a workspace is active it does not + * change for the lifetime of the process; selecting a different one persists + * the choice and takes effect on restart (issue #126). That is what lets every + * cache, directory index, and open handle be built once and trusted + * afterwards, instead of each one carrying its own answer to "which workspace + * am I looking at?". * - * ── Free mode (default for local dev) ── - * `HUABU_WORKSPACE` is *unset*. - * The user picks any absolute directory at runtime via the client - * (folder picker / path input). `setWorkspacePath(absPath)` is the - * entry point. The active path can change during the process lifetime. + * Three ways the one workspace is chosen, in precedence order: * - * ── Managed mode (recommended for remote / single-tenant deployments) ── - * `HUABU_WORKSPACE=/abs/path` is set at process start. - * The path is locked at boot via {@link initWorkspaceFromEnv} and - * CANNOT be changed at runtime. The client gets a read-only label - * (the basename of the path) and no folder-picker UI. To run a - * different workspace, restart the process with a different env - * value — typically one process per user / per workspace, with the - * network access controlled by your reverse proxy. + * ── Managed: `HUABU_WORKSPACE=/abs/path` ── + * The operator's choice, for remote / single-tenant deployments. Locked at + * boot by {@link initWorkspaceFromEnv}; the client gets a read-only label + * (the basename) and no folder-picker UI, and a workspace that cannot be + * prepared fails startup rather than degrading to a picker a remote user + * should not see. To run a different workspace, restart the process with a + * different env value. * - * Either way, every storage layer resolves its directory relative to a - * single workspace root. + * ── Shell-chosen: `HUABU_WORKSPACE_STARTUP=/abs/path` ── + * The user's own choice, remembered by a shell that owns both the saved + * path and the server process — today the Electron main process and its + * `workspace.json`. Free mode otherwise: the path is shown, the picker is + * offered, and picking another one is a restart. A path that cannot be + * prepared leaves the process unconfigured with + * {@link getWorkspaceStartupError} set, so the shell can show the picker + * instead of dying. + * + * ── Runtime activation (free mode, first time only) ── + * Neither variable is set, so the process starts unconfigured and the + * client activates a path through `PUT /api/workspace`. That is the + * browser deployment's only route, since `localStorage` cannot configure a + * server before it starts. It happens at most once per process; see + * `workspace-activation.ts`. * * Directory layout inside the active workspace (canvas-centric): * @@ -40,37 +52,20 @@ import path from 'node:path'; import { resetExternalNoteSessions } from './canvas/external-watcher.js'; import { refreshCanvasDirIndex } from './storage/canvas-dirs.js'; +import { resetStorage } from './storage/index.js'; +import { + runWorkspacePreparation, + type WorkspacePreparationOptions, +} from './workspace-preparation-process.js'; import { prepareWorkspaceOnDisk } from './workspace-prepare.js'; import { invalidateUserSkill } from '../prompt/index.js'; const ENV_KEY = 'HUABU_WORKSPACE'; +const STARTUP_ENV_KEY = 'HUABU_WORKSPACE_STARTUP'; let _workspacePath: string | null = null; let _managed = false; -let _leasedWorkspacePath: string | null = null; -let _workspaceOperationLeaseCount = 0; - -/** - * A short-lived claim that keeps an async operation on one workspace. - * - * The release callback is deliberately synchronous and idempotent so callers - * can always put it in a `finally` block without masking the operation's - * original result. - */ -export interface WorkspaceOperationLease { - readonly workspacePath: string; - release(): void; -} - -/** Raised when a workspace switch would strand an in-flight operation. */ -export class WorkspaceOperationInProgressError extends Error { - constructor() { - super( - 'Cannot change workspace while an operation is still using the active workspace', - ); - this.name = 'WorkspaceOperationInProgressError'; - } -} +let _startupError: string | null = null; // ────────────────────────────────────────────────────────────────────── // Mode + lifecycle @@ -89,26 +84,71 @@ export function isWorkspaceConfigured(): boolean { } /** - * If `HUABU_WORKSPACE` is set, lock the server to that path. - * Must be called once at startup, before any request handlers run. - * Throws if the env value is invalid (non-absolute) so misconfiguration - * is surfaced loudly. + * Adopt the workspace this process was started on, if it was given one. + * + * Must be called once at startup, before any request handlers run. Handles + * both env forms, and they differ in exactly one place — what a failure means. + * An operator naming a workspace that cannot be prepared has misconfigured the + * deployment, and a server that quietly came up unconfigured would offer a + * remote user a folder picker for the host filesystem. A *shell* naming one + * has a user whose folder moved, was renamed, or lives on a drive that is not + * mounted today, and the recovery for that is the picker. */ -export function initWorkspaceFromEnv(): void { - const fromEnv = process.env[ENV_KEY]; - if (!fromEnv) { - _managed = false; - return; - } - if (!path.isAbsolute(fromEnv)) { - throw new Error( - `${ENV_KEY} must be an absolute path, got: ${JSON.stringify(fromEnv)}`, - ); +export async function initWorkspaceFromEnv( + options: WorkspacePreparationOptions = {}, +): Promise { + const managedPath = readEnvPath(ENV_KEY); + const startupPath = managedPath ? null : readEnvPath(STARTUP_ENV_KEY); + const resolvedPath = managedPath ?? startupPath; + _managed = managedPath !== null; + _startupError = null; + if (!resolvedPath) return; + + try { + await runWorkspacePreparation(resolvedPath, options); + commitWorkspacePath(resolvedPath); + } catch (error) { + if (_managed) throw error; + reportWorkspaceStartupFailure(error); } - const resolvedPath = path.resolve(fromEnv); - _managed = true; - prepareWorkspaceOnDisk(resolvedPath); - commitWorkspacePath(resolvedPath); +} + +/** + * Give up on the workspace this process was started on. + * + * Leaves the process unconfigured with the reason recorded, which is a state + * the client already knows how to recover from: it shows the picker. Only ever + * right for a *shell-chosen* workspace — an operator's `HUABU_WORKSPACE` that + * cannot be opened is a misconfiguration, and offering a remote user a folder + * picker for the host filesystem instead is not a recovery. + */ +export function reportWorkspaceStartupFailure(error: unknown): void { + clearWorkspacePath(); + _startupError = + error instanceof Error ? error.message : 'Workspace could not be opened'; +} + +/** + * Return the process to its unconfigured state. + * + * Not a way to change workspaces — nothing is put in the old one's place. It + * exists so an activation that fails partway leaves no half-open workspace + * behind, and so tests can start from a clean process. + */ +export function clearWorkspacePath(): void { + _workspacePath = null; + dropWorkspaceScopedState(); +} + +/** + * Why the workspace this process was started on could not be opened. + * + * `null` when there was nothing to open or it opened fine. Reported to the + * client so a shell-chosen workspace that has gone missing explains itself in + * the picker rather than looking like a first launch. + */ +export function getWorkspaceStartupError(): string | null { + return _startupError; } // ────────────────────────────────────────────────────────────────────── @@ -130,40 +170,6 @@ export function getWorkspacePath(): string { return _workspacePath; } -/** - * Keep the currently-active workspace stable for an async operation. - * - * Multiple operations may hold leases concurrently. Switching to another - * workspace is rejected until every lease has been released; recommitting the - * same path remains allowed. - */ -export function acquireWorkspaceOperationLease(): WorkspaceOperationLease { - const workspacePath = getWorkspacePath(); - - if ( - _workspaceOperationLeaseCount > 0 && - _leasedWorkspacePath !== workspacePath - ) { - throw new Error('Workspace operation lease invariant violated'); - } - - _leasedWorkspacePath = workspacePath; - _workspaceOperationLeaseCount += 1; - - let released = false; - return Object.freeze({ - workspacePath, - release(): void { - if (released) return; - released = true; - _workspaceOperationLeaseCount -= 1; - if (_workspaceOperationLeaseCount === 0) { - _leasedWorkspacePath = null; - } - }, - }); -} - /** * Display label for the currently-active workspace. In managed mode this * is the basename of the locked path; in free mode it's also the basename @@ -178,12 +184,17 @@ export function getWorkspaceName(): string | null { } /** - * (Free mode) Activate any absolute path as the current workspace and - * create the workspace folder. Rejected in managed mode — the workspace - * is locked at boot. + * Prepare an absolute path and make it the active workspace, synchronously. * - * Also converts any legacy pi-ai `Context` chat threads on the new - * workspace to structured turns (idempotent). + * The primitive underneath both startup paths. Production reaches a workspace + * through {@link initWorkspaceFromEnv} or `activateWorkspacePath`, which is + * where the "one workspace per process" rule is enforced and where preparation + * runs in a disposable child; this function does the preparation inline and + * enforces nothing, so it stays available to tests that drive many temporary + * workspaces through one process. + * + * Also converts any legacy pi-ai `Context` chat threads on the new workspace + * to structured turns (idempotent). */ export function setWorkspacePath(newPath: string): void { if (_managed) { @@ -192,7 +203,6 @@ export function setWorkspacePath(newPath: string): void { ); } const resolvedPath = resolveWorkspacePath(newPath); - assertWorkspacePathChangeAllowed(resolvedPath); prepareWorkspaceOnDisk(resolvedPath); commitWorkspacePath(resolvedPath); } @@ -208,21 +218,33 @@ export function resolveWorkspacePath(newPath: string): string { * * This function intentionally performs no disk I/O. Runtime activation calls * it only after the isolated preparation process has completed successfully. + * + * Production commits once, before anything has been opened against a + * workspace. It still drops every workspace-scoped cache below, because tests + * drive it repeatedly to move between temporary workspaces and nothing should + * keep serving the previous one. */ export function commitWorkspacePath(resolvedPath: string): void { - assertWorkspacePathChangeAllowed(resolvedPath); _workspacePath = resolvedPath; - // Drop the cached canvas-dir index so subsequent lookups (used by - // migrations and route handlers) reflect the new workspace. + _startupError = null; + dropWorkspaceScopedState(); +} + +/** + * Drop everything built against whichever workspace was active. + * + * Storage's caches and fences, the directory index, the skill cache, and the + * external-note watchers are all workspace-scoped, and none of them names a + * workspace any more — dropping them here is what lets them stop. The import + * cycles with storage and the prompt loader (both depend on + * `getWorkspacePath` from this module) are safe because Node ESM allows + * cycles as long as no top-level code on either side dereferences the + * late-bound import — each of these is only called from inside a function + * body, after both modules have finished evaluating. + */ +function dropWorkspaceScopedState(): void { + resetStorage(); refreshCanvasDirIndex(); - // Drop any user-skill cache built against the previous workspace so - // the next `listSkills` / `read("skills/...")` call rescans the new - // `/setting/skills/` from scratch. The import-cycle with - // the prompt loader (which depends on `getWorkspacePath` from this - // module) is safe because Node ESM allows cycles as long as no - // top-level code on either side dereferences the late-bound import - // — here `invalidateUserSkill` is only ever called from within - // function bodies, after both modules have finished evaluating. invalidateUserSkill(); resetExternalNoteSessions(); } @@ -247,12 +269,14 @@ function validateAbsolutePath(p: string): void { } } -function assertWorkspacePathChangeAllowed(resolvedPath: string): void { - if ( - _workspaceOperationLeaseCount > 0 && - _leasedWorkspacePath !== null && - _leasedWorkspacePath !== resolvedPath - ) { - throw new WorkspaceOperationInProgressError(); +/** Read one env var as an absolute path, or `null` when unset. */ +function readEnvPath(key: string): string | null { + const raw = process.env[key]; + if (!raw) return null; + if (!path.isAbsolute(raw)) { + throw new Error( + `${key} must be an absolute path, got: ${JSON.stringify(raw)}`, + ); } + return path.resolve(raw); } diff --git a/apps/server/src/prompt/skills/loader.ts b/apps/server/src/prompt/skills/loader.ts index 07340b780..0f4468e2f 100644 --- a/apps/server/src/prompt/skills/loader.ts +++ b/apps/server/src/prompt/skills/loader.ts @@ -366,7 +366,7 @@ function rescanUserSkills( /** * Ensure the user cache reflects the current workspace + on-disk state. * - * - Re-keys on workspace switch (full rebuild). + * - Re-keys if tests reset the process-local workspace (full rebuild). * - Throttles full re-scans via {@link USER_SCAN_TTL_MS}; pass * `forceFresh=true` to bypass. */ @@ -505,8 +505,8 @@ export function invalidateSkillCache(): void { * - `fs_write` on a `skills//SKILL.md` path after a successful * write: pass the id so the very next `read("skills//SKILL.md")` * sees fresh content without waiting for the TTL. - * - `setWorkspacePath()` after activation: invalidates everything - * so the new workspace's user skills replace the old ones. + * - Initial workspace commit (and test-only workspace resets): invalidates + * everything so no cache survives a namespace boundary. */ export function invalidateUserSkill(id?: string): void { if (id === undefined) { diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 4b22089c4..ce60e9da2 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -8,6 +8,7 @@ import { resolveBindHost } from './bind-host.js'; import { prewarmOAuthCredentials } from './modules/agent/oauth.js'; import { resolveDeploymentConfig } from './modules/security/deployment-config.js'; import { initStorage } from './modules/storage/index.js'; +import { initWorkspaceFromEnv } from './modules/workspace.js'; import { initializeSecretStore } from './security/secret-store.js'; import { getLogger } from './utils/logger.js'; @@ -25,6 +26,10 @@ const HOST = resolveBindHost(); async function start(): Promise { try { + // Prepare the one startup workspace in an isolated, bounded child before + // storage or request handlers can open workspace-scoped state. + await initWorkspaceFromEnv(); + const deployment = resolveDeploymentConfig(); if (deployment.bindScope === 'network') { log.warn( diff --git a/apps/web/src/hooks/useElectron.ts b/apps/web/src/hooks/useElectron.ts index 8b2605044..b505dd107 100644 --- a/apps/web/src/hooks/useElectron.ts +++ b/apps/web/src/hooks/useElectron.ts @@ -30,6 +30,13 @@ interface ElectronWorkspaceApi { get: () => Promise; set: (path: string) => Promise; removeRecent: (path: string) => Promise; + /** + * Restart the app onto the saved workspace. + * + * Present only on builds whose shell can apply a new workspace, which is + * what makes switching offerable at all once one is active. Never resolves. + */ + restart?: () => Promise; } interface ElectronWindowApi { diff --git a/apps/web/src/i18n/resources/en/common.json b/apps/web/src/i18n/resources/en/common.json index f110f829d..2ecef0479 100644 --- a/apps/web/src/i18n/resources/en/common.json +++ b/apps/web/src/i18n/resources/en/common.json @@ -311,7 +311,8 @@ "welcome": "Welcome to {{appName}}", "intro": "Choose a Home folder to store your Spaces, notes, and artifacts.", "folder": "Home folder", - "pathPlaceholder": "Type an absolute path, then press Enter", + "pathPlaceholder": "Type an absolute path", + "useFolder": "Use this Home folder", "recent": "Recent Home folders", "removeRecent": "Remove from recent", "path": "Path: {{path}}", @@ -321,7 +322,16 @@ "clickToSwitch": "Click to switch", "openPathFailed": "Failed to open path", "activationTimeout": "Opening Home folder “{{path}}” took longer than {{seconds}} seconds. Check that the cloud or network drive is available, then try again.", - "activationInProgress": "Another Home folder is still opening. Please wait for it to finish." + "activationInProgress": "Another Home folder is still opening. Please wait for it to finish.", + "restartRequired": "“{{path}}” has been saved as your Home folder. Restart the server to open it.", + "restartTitle": "Restart required", + "restartDescription": "Your next Home folder is saved and validated. Restart Huabu, or the server hosting it, to open that folder.", + "activeHome": "Active now", + "pendingHome": "After restart", + "continueCurrent": "Continue with the current Home folder", + "restarting": "Restarting to open “{{path}}”…", + "startupFailed": "Could not open your saved Home folder: {{reason}} Choose a folder to continue.", + "persistenceFailed": "Could not save “{{path}}” for the next launch: {{reason}}" }, "canvasList": { "title": "Spaces", diff --git a/apps/web/src/i18n/resources/zh-CN/common.json b/apps/web/src/i18n/resources/zh-CN/common.json index 129a00aff..038346b3b 100644 --- a/apps/web/src/i18n/resources/zh-CN/common.json +++ b/apps/web/src/i18n/resources/zh-CN/common.json @@ -311,7 +311,8 @@ "welcome": "欢迎使用 {{appName}}", "intro": "选择一个 Home 文件夹来保存你的 Space、笔记和产物。", "folder": "Home 文件夹", - "pathPlaceholder": "输入绝对路径,然后按 Enter", + "pathPlaceholder": "输入绝对路径", + "useFolder": "使用此 Home 文件夹", "recent": "最近的 Home 文件夹", "removeRecent": "从最近列表中移除", "path": "路径:{{path}}", @@ -321,7 +322,16 @@ "clickToSwitch": "点击切换", "openPathFailed": "打开路径失败", "activationTimeout": "打开“{{path}}”已超过 {{seconds}} 秒。请检查云盘或网络磁盘是否可用,然后重试。", - "activationInProgress": "另一个主目录仍在打开,请等待其完成。" + "activationInProgress": "另一个主目录仍在打开,请等待其完成。", + "restartRequired": "已将“{{path}}”保存为你的 Home 文件夹。重启服务器后生效。", + "restartTitle": "需要重启", + "restartDescription": "新的 Home 文件夹已经保存并验证。请重启 Huabu 或托管它的服务器以打开该文件夹。", + "activeHome": "当前使用", + "pendingHome": "重启后使用", + "continueCurrent": "继续使用当前 Home 文件夹", + "restarting": "正在重启以打开“{{path}}”…", + "startupFailed": "无法打开已保存的 Home 文件夹:{{reason}} 请选择一个文件夹以继续。", + "persistenceFailed": "无法保存“{{path}}”供下次启动使用:{{reason}}" }, "canvasList": { "title": "Space", diff --git a/apps/web/src/pages/WorkspaceSetupPage.tsx b/apps/web/src/pages/WorkspaceSetupPage.tsx index 8162bd769..c068b3148 100644 --- a/apps/web/src/pages/WorkspaceSetupPage.tsx +++ b/apps/web/src/pages/WorkspaceSetupPage.tsx @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { FolderOpen, X } from 'lucide-react'; +import { FolderOpen, RefreshCw, X } from 'lucide-react'; import { useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Navigate, useNavigate } from 'react-router-dom'; @@ -30,6 +30,8 @@ export default function WorkspaceSetupPage() { const mode = useWorkspaceStore((s) => s.mode); const isSyncing = useWorkspaceStore((s) => s.isSyncing); const recentWorkspaces = useWorkspaceStore((s) => s.recentWorkspaces); + const activeWorkspacePath = useWorkspaceStore((s) => s.workspacePath); + const pendingWorkspacePath = useWorkspaceStore((s) => s.pendingWorkspacePath); const removeRecentWorkspace = useWorkspaceStore( (s) => s.removeRecentWorkspace, ); @@ -64,6 +66,8 @@ export default function WorkspaceSetupPage() { void; selectWorkspace: (path: string) => Promise; @@ -90,6 +96,8 @@ interface FreeSetupProps { function FreeSetup({ isSyncing, storeError, + activeWorkspacePath, + pendingWorkspacePath, recentWorkspaces, removeRecentWorkspace, selectWorkspace, @@ -100,6 +108,7 @@ function FreeSetup({ const [pathInput, setPathInput] = useState(''); const isLoading = isSyncing; + const hasPathInput = pathInput.trim().length > 0; /** Activate a path (typed, picked or recent) and navigate on success. */ const activate = async (path: string) => { @@ -124,6 +133,57 @@ function FreeSetup({ return ( <> + {pendingWorkspacePath && ( +
+
+ +
+

+ {t('workspace.restartTitle')} +

+

+ {t('workspace.restartDescription')} +

+
+ {activeWorkspacePath && ( +
+
+ {t('workspace.activeHome')} +
+
+ {activeWorkspacePath} +
+
+ )} +
+
+ {t('workspace.pendingHome')} +
+
+ {pendingWorkspacePath} +
+
+
+
+
+ {activeWorkspacePath && ( + + )} +
+ )} + {/* Path input + optional native folder picker */}