Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 44 additions & 7 deletions apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,15 @@
}
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-<ts>-exit<code>.log`
Expand Down Expand Up @@ -456,9 +465,8 @@

// 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)) {
Expand All @@ -469,9 +477,6 @@
);
}

// 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
Expand All @@ -480,12 +485,27 @@
// 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',
};
Expand Down Expand Up @@ -612,7 +632,7 @@
// non-zero exit we dump the captured stderr ring as a crash file.

serverExitPromise = new Promise((resolve) => {
serverProcess!.on('exit', (code) => {

Check warning on line 635 in apps/desktop/src/main.ts

View workflow job for this annotation

GitHub Actions / Lint / Format / Typecheck / Test / Build

Forbidden non-null assertion
if (code !== 0) {
console.error(`[desktop] server exited with code ${code}`);
dumpServerCrash(code ?? -1);
Expand Down Expand Up @@ -786,6 +806,19 @@
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');
Expand Down Expand Up @@ -1335,7 +1368,11 @@
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;
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ contextBridge.exposeInMainWorld('electronBridge', {
'workspace:remove-recent',
path,
) as Promise<WorkspaceStoreSnapshot>,
/**
* 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<void> =>
ipcRenderer.invoke('workspace:restart') as Promise<void>,
},

window: {
Expand Down
14 changes: 3 additions & 11 deletions apps/server/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
19 changes: 8 additions & 11 deletions apps/server/src/modules/canvas/external-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down Expand Up @@ -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()]) {
Expand All @@ -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.
Expand Down
11 changes: 5 additions & 6 deletions apps/server/src/modules/storage/backends/disk/blob-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {};
Expand Down Expand Up @@ -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 });
Expand Down
24 changes: 7 additions & 17 deletions apps/server/src/modules/storage/backends/disk/blob-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@
* Maps a canvas scope to `<canvasDir>/.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';
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
});

Expand All @@ -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',
Expand All @@ -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 () => {
Expand Down
Loading
Loading