Skip to content
Draft
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
15 changes: 14 additions & 1 deletion apps/host-daemon/src/command-handlers/file-read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,19 @@ async function resolveReadablePath(
return realResolvedPath;
}

async function resolveRootRelativeReadablePath(
args: ReadFileForTransportArgs,
): Promise<string> {
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
Expand Down Expand Up @@ -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));
Expand Down
31 changes: 31 additions & 0 deletions apps/host-daemon/src/command-handlers/host-files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import {
readHostFile,
readHostFileMetadata,
readHostRelativeFile,
readHostStatusVersion,
writeHostRelativeFile,
deleteHostRelativeFile,
Expand Down Expand Up @@ -73,6 +74,18 @@ async function captureReadHostFileError(
throw new Error("Expected readHostFile to fail");
}

async function captureReadHostRelativeFileError(
command: CommandOf<"host.read_file_relative">,
): Promise<unknown> {
try {
await readHostRelativeFile(command);
} catch (error) {
return error;
}

throw new Error("Expected readHostRelativeFile to fail");
}

function makeStatusVersionCommand(
storageRootPath: string,
): CommandOf<"host.status_version"> {
Expand Down Expand Up @@ -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();

Expand Down
27 changes: 26 additions & 1 deletion apps/host-daemon/src/command-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ interface CommandLifecycleTiming {
totalMs: number;
}

interface CommandExecutionWarningInput {
command: HostDaemonCommand;
error: unknown;
}

type ReadCommandFetchedAt = (
envelope: HostDaemonCommandEnvelope,
) => number | undefined;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
107 changes: 100 additions & 7 deletions apps/host-daemon/test/command/command-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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';
Expand Down
78 changes: 65 additions & 13 deletions apps/server/src/services/threads/status-state-watcher.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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<void>;
}

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) {
Expand All @@ -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 {
Expand All @@ -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;
}

Expand Down Expand Up @@ -325,12 +372,11 @@ async function handleStatusDataFileUnlink(
async function handleStatusDataDirectoryAdd(
args: HandleStatusDataDirectoryAddArgs,
): Promise<void> {
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;
}

Expand Down Expand Up @@ -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) => {
Expand Down
Loading