diff --git a/apps/host-daemon/src/command-handlers/file-read.ts b/apps/host-daemon/src/command-handlers/file-read.ts index d7252dcb..85cc19e3 100644 --- a/apps/host-daemon/src/command-handlers/file-read.ts +++ b/apps/host-daemon/src/command-handlers/file-read.ts @@ -222,6 +222,19 @@ async function resolveReadablePath( return realResolvedPath; } +async function resolveRootRelativeReadablePath( + args: ReadFileForTransportArgs, +): Promise { + try { + return await resolveReadablePath(args); + } catch (error) { + if (error instanceof CommandDispatchError && error.code === "ENOENT") { + throw createMissingTargetError(args.resultPath); + } + throw error; + } +} + /** * Read a file's contents at a specific git ref via `git cat-file`. Mirrors * `readFileForTransport`'s result shape (same caps, same utf-8/base64 @@ -353,7 +366,7 @@ export async function readRootRelativeFileForTransport( resultPath: relativePath.resultPath, rootPath: args.rootPath, }; - const readablePath = await resolveReadablePath(readArgs); + const readablePath = await resolveRootRelativeReadablePath(readArgs); const stat = await fs .stat(readablePath) .catch((error: unknown) => throwMissingTargetOrRethrow(readArgs, error)); diff --git a/apps/host-daemon/src/command-handlers/host-files.test.ts b/apps/host-daemon/src/command-handlers/host-files.test.ts index afed92fa..7d82d25d 100644 --- a/apps/host-daemon/src/command-handlers/host-files.test.ts +++ b/apps/host-daemon/src/command-handlers/host-files.test.ts @@ -13,6 +13,7 @@ import { import { readHostFile, readHostFileMetadata, + readHostRelativeFile, readHostStatusVersion, writeHostRelativeFile, deleteHostRelativeFile, @@ -73,6 +74,18 @@ async function captureReadHostFileError( throw new Error("Expected readHostFile to fail"); } +async function captureReadHostRelativeFileError( + command: CommandOf<"host.read_file_relative">, +): Promise { + try { + await readHostRelativeFile(command); + } catch (error) { + return error; + } + + throw new Error("Expected readHostRelativeFile to fail"); +} + function makeStatusVersionCommand( storageRootPath: string, ): CommandOf<"host.status_version"> { @@ -227,6 +240,24 @@ describe("readHostFile (no ref — disk read)", () => { expect(isExpectedCommandDispatchError(thrown)).toBe(false); }); + it("marks missing relative read roots as expected", async () => { + const parentPath = await makeTempDir("bb-host-files-relative-root-"); + const rootPath = path.join(parentPath, "STATUS"); + const thrown = await captureReadHostRelativeFileError({ + type: "host.read_file_relative", + rootPath, + path: "index.html", + dotfiles: "deny", + }); + + expect(thrown).toMatchObject({ + code: "ENOENT", + message: "Path does not exist: index.html", + name: "ExpectedCommandDispatchError", + }); + expect(isExpectedCommandDispatchError(thrown)).toBe(true); + }); + it("rejects rootless directory paths", async () => { const repoPath = await initRepo(); diff --git a/apps/host-daemon/src/command-router.ts b/apps/host-daemon/src/command-router.ts index 9a9f628b..0460047e 100644 --- a/apps/host-daemon/src/command-router.ts +++ b/apps/host-daemon/src/command-router.ts @@ -67,6 +67,11 @@ interface CommandLifecycleTiming { totalMs: number; } +interface CommandExecutionWarningInput { + command: HostDaemonCommand; + error: unknown; +} + type ReadCommandFetchedAt = ( envelope: HostDaemonCommandEnvelope, ) => number | undefined; @@ -108,6 +113,21 @@ function readCommandEnvironmentId( return undefined; } +function shouldWarnCommandExecutionFailure( + input: CommandExecutionWarningInput, +): boolean { + if (isExpectedCommandDispatchError(input.error)) { + return false; + } + if ( + input.command.type === "workspace.status" && + getErrorCode(input.error) === "path_not_found" + ) { + return false; + } + return true; +} + export class CommandRouter { private readonly reportResult; private readonly logger; @@ -310,7 +330,12 @@ export class CommandRouter { }; } catch (error) { const errorCode = getErrorCode(error); - if (!isExpectedCommandDispatchError(error)) { + if ( + shouldWarnCommandExecutionFailure({ + command: envelope.command, + error, + }) + ) { this.logger.warn( { commandId: envelope.id, diff --git a/apps/host-daemon/test/command/command-router.test.ts b/apps/host-daemon/test/command/command-router.test.ts index 9a507fb1..5cc861e5 100644 --- a/apps/host-daemon/test/command/command-router.test.ts +++ b/apps/host-daemon/test/command/command-router.test.ts @@ -7,13 +7,14 @@ import { type ClientTurnRequestId, } from "@bb/domain"; import type { HostDaemonCommandResultReportWithoutSession } from "@bb/host-daemon-contract"; -import type { - CommitOptions, - CommitResult, - HostWorkspace, - ProvisionWorkspaceArgs, - SquashMergeOptions, - SquashMergeResult, +import { + WorkspaceError, + type CommitOptions, + type CommitResult, + type HostWorkspace, + type ProvisionWorkspaceArgs, + type SquashMergeOptions, + type SquashMergeResult, } from "@bb/host-workspace"; import { afterEach, describe, expect, it, vi } from "vitest"; import { CommandRouter } from "../../src/command-router.js"; @@ -371,6 +372,98 @@ describe("CommandRouter", () => { expect(logger.warn).not.toHaveBeenCalled(); }); + it("reports missing host relative file reads without warning", async () => { + const parentPath = await makeTempDir("bb-command-router-relative-file-"); + const rootPath = path.join(parentPath, "STATUS"); + const reportResult = vi.fn(async () => undefined); + const logger = createLogger(); + const router = new CommandRouter({ + dataDir: "/tmp/bb-test-data", + fetchProjectAttachment: unexpectedProjectAttachmentFetch, + reportResult, + runtimeManager: new RuntimeManager({ + provisionWorkspace: async () => createFakeWorkspace("/tmp/env-1"), + }), + eventSink: noopEventSink, + threadStorageRootPath: "/tmp/bb-test-thread-storage", + logger, + }); + + await router.handleCommands([ + { + id: "read-missing-relative-index", + cursor: 1, + command: { + type: "host.read_file_relative", + rootPath, + path: "index.html", + dotfiles: "deny", + }, + }, + ]); + + expect(reportResult).toHaveBeenCalledWith( + expect.objectContaining({ + commandId: "read-missing-relative-index", + errorCode: "ENOENT", + errorMessage: "Path does not exist: index.html", + ok: false, + type: "host.read_file_relative", + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("reports missing workspace status paths without warning", async () => { + const parentPath = await makeTempDir("bb-command-router-workspace-"); + const missingPath = path.join(parentPath, "missing-worktree"); + const reportResult = vi.fn(async () => undefined); + const logger = createLogger(); + const router = new CommandRouter({ + dataDir: "/tmp/bb-test-data", + fetchProjectAttachment: unexpectedProjectAttachmentFetch, + reportResult, + runtimeManager: new RuntimeManager({ + provisionWorkspace: async () => { + throw new WorkspaceError( + "path_not_found", + `Managed workspace path does not exist: ${missingPath}`, + ); + }, + }), + eventSink: noopEventSink, + threadStorageRootPath: "/tmp/bb-test-thread-storage", + logger, + }); + + await router.handleCommands([ + { + id: "status-missing-workspace", + cursor: 1, + command: { + type: "workspace.status", + environmentId: "env-missing", + workspaceContext: { + workspacePath: missingPath, + workspaceProvisionType: "managed-worktree", + }, + mergeBaseBranch: "main", + }, + }, + ]); + + expect(reportResult).toHaveBeenCalledWith( + expect.objectContaining({ + commandId: "status-missing-workspace", + errorCode: "path_not_found", + errorMessage: `Managed workspace path does not exist: ${missingPath}`, + ok: false, + type: "workspace.status", + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + it("reports missing provider executables with a specific error code", async () => { const errorMessage = 'Provider "codex" exited unexpectedly\nstderr: Error: spawn /missing/codex ENOENT'; diff --git a/apps/server/src/services/threads/status-state-watcher.ts b/apps/server/src/services/threads/status-state-watcher.ts index 34c1af49..8b02e680 100644 --- a/apps/server/src/services/threads/status-state-watcher.ts +++ b/apps/server/src/services/threads/status-state-watcher.ts @@ -1,3 +1,4 @@ +import type { Stats } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { watch, type FSWatcher } from "chokidar"; @@ -87,12 +88,22 @@ interface StartStatusStateFileWatcherArgs { rootPath: string; } +interface RelativeStatusDataPathArgs { + filePath: string; + rootPath: string; +} + +interface IsIgnoredStatusDataWatchPathArgs extends RelativeStatusDataPathArgs { + stats?: Stats; +} + export interface StatusStateFileWatcher { close(): Promise; } const STATUS_DATA_AWAIT_WRITE_FINISH_MS = 25; const STATUS_DATA_AWAIT_WRITE_POLL_MS = 10; +const STATUS_DATA_WATCH_DEPTH = 2; function getErrorMessage(error: unknown): string { if (error instanceof Error && error.message.trim().length > 0) { @@ -117,6 +128,46 @@ function isHiddenStatusDataFile(filePath: string): boolean { return path.basename(filePath).startsWith("."); } +function relativeStatusDataWatchPath( + args: RelativeStatusDataPathArgs, +): string | null { + const relativePath = path.relative(args.rootPath, args.filePath); + if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) { + return null; + } + return relativePath; +} + +function isIgnoredStatusDataWatchPath( + args: IsIgnoredStatusDataWatchPathArgs, +): boolean { + const relativePath = relativeStatusDataWatchPath(args); + if (relativePath === null) { + return true; + } + if (relativePath.length === 0) { + return false; + } + if (isHiddenStatusDataFile(args.filePath)) { + return true; + } + + const parts = relativePath.split(path.sep); + if (parts.length === 1) { + return args.stats?.isFile() === true; + } + if (parts[1] !== STATUS_DATA_DIRECTORY_NAME) { + return true; + } + if (parts.length === 2) { + return false; + } + if (parts.length !== 3 || args.stats?.isDirectory() === true) { + return true; + } + return !args.filePath.endsWith(STATUS_DATA_FILE_EXTENSION); +} + function parseStatusDataFilePath( args: ParseStatusDataFilePathArgs, ): StatusDataFilePathParts | null { @@ -127,12 +178,8 @@ function parseStatusDataFilePath( return null; } - const relativePath = path.relative(args.rootPath, args.filePath); - if ( - relativePath.length === 0 || - relativePath.startsWith("..") || - path.isAbsolute(relativePath) - ) { + const relativePath = relativeStatusDataWatchPath(args); + if (relativePath === null || relativePath.length === 0) { return null; } @@ -325,12 +372,11 @@ async function handleStatusDataFileUnlink( async function handleStatusDataDirectoryAdd( args: HandleStatusDataDirectoryAddArgs, ): Promise { - const relativePath = path.relative(args.rootPath, args.dirPath); - if ( - relativePath.length === 0 || - relativePath.startsWith("..") || - path.isAbsolute(relativePath) - ) { + const relativePath = relativeStatusDataWatchPath({ + filePath: args.dirPath, + rootPath: args.rootPath, + }); + if (relativePath === null || relativePath.length === 0) { return; } @@ -454,8 +500,14 @@ export async function startStatusStateFileWatcher( stabilityThreshold: STATUS_DATA_AWAIT_WRITE_FINISH_MS, pollInterval: STATUS_DATA_AWAIT_WRITE_POLL_MS, }, + depth: STATUS_DATA_WATCH_DEPTH, ignoreInitial: true, - ignored: isHiddenStatusDataFile, + ignored: (filePath, stats) => + isIgnoredStatusDataWatchPath({ + filePath, + rootPath: args.rootPath, + stats, + }), }); watcher.on("addDir", (dirPath) => { diff --git a/apps/server/test/services/threads/status-state-watcher.test.ts b/apps/server/test/services/threads/status-state-watcher.test.ts index af2102b6..7fd20d9b 100644 --- a/apps/server/test/services/threads/status-state-watcher.test.ts +++ b/apps/server/test/services/threads/status-state-watcher.test.ts @@ -27,6 +27,10 @@ interface TestWatcherContext { watcher: Awaited>; } +interface CreateContextArgs { + precreateStatusDataDir: boolean; +} + interface AtomicWriteArgs { content: string; key: string; @@ -47,6 +51,9 @@ interface WaitForBroadcastArgs { } const contexts: TestWatcherContext[] = []; +const DEFAULT_CREATE_CONTEXT_ARGS: CreateContextArgs = { + precreateStatusDataDir: true, +}; function createMockSocket(): MockSocket { const messages: string[] = []; @@ -78,11 +85,15 @@ function sha256Text(content: string): string { return createHash("sha256").update(content).digest("hex"); } -async function createContext(): Promise { +async function createContext( + args: CreateContextArgs = DEFAULT_CREATE_CONTEXT_ARGS, +): Promise { const rootPath = await fs.mkdtemp(path.join(tmpdir(), "bb-status-watch-")); - await fs.mkdir(path.join(rootPath, "thr_watch", "STATUS-data"), { - recursive: true, - }); + if (args.precreateStatusDataDir) { + await fs.mkdir(path.join(rootPath, "thr_watch", "STATUS-data"), { + recursive: true, + }); + } const hub = new NotificationHub(); const socket = createMockSocket(); const logger = createLogger(); @@ -182,6 +193,32 @@ describe("status state file watcher", () => { ); }); + it("broadcasts writes when the STATUS-data directory is created after startup", async () => { + const context = await createContext({ precreateStatusDataDir: false }); + const content = canonicalizeStatusJson({ createdLate: true }); + await writeStatusFile({ + content, + key: "late", + rootPath: context.rootPath, + threadId: "thr_watch", + }); + + await expect(waitForBroadcast({ socket: context.socket })).resolves.toEqual( + { + type: "status-data.changed", + threadId: "thr_watch", + key: "late", + value: { createdLate: true }, + deleted: false, + previousValue: null, + previousValuePresent: false, + version: sha256Text(content), + writerClientId: null, + operationId: null, + }, + ); + }); + it("logs malformed JSON and does not broadcast it", async () => { const context = await createContext(); await writeStatusFile({