Improve daemon RPC resiliency and diff handling

This commit is contained in:
Mohamed Boudra
2026-02-04 10:21:36 +07:00
parent 10a2ef34a3
commit 79aec0069c
26 changed files with 467 additions and 109 deletions

View File

@@ -1,5 +1,5 @@
import { afterEach, describe, expect, test, vi } from "vitest";
import { DaemonClientV2, type DaemonTransport } from "./daemon-client-v2";
import { DaemonClient, type DaemonTransport } from "./daemon-client";
function createMockLogger() {
return {
@@ -49,8 +49,8 @@ function createMockTransport() {
};
}
describe("DaemonClientV2", () => {
const clients: DaemonClientV2[] = [];
describe("DaemonClient", () => {
const clients: DaemonClient[] = [];
afterEach(async () => {
for (const client of clients) {
@@ -63,7 +63,7 @@ describe("DaemonClientV2", () => {
const logger = createMockLogger();
const mock = createMockTransport();
const client = new DaemonClientV2({
const client = new DaemonClient({
url: "ws://test",
logger,
reconnect: { enabled: false },
@@ -150,7 +150,7 @@ describe("DaemonClientV2", () => {
},
});
const client = new DaemonClientV2({
const client = new DaemonClient({
url: "ws://test",
logger,
reconnect: { enabled: false },

View File

@@ -139,7 +139,7 @@ export type DaemonEvent =
export type DaemonEventHandler = (event: DaemonEvent) => void;
export type DaemonClientV2Config = {
export type DaemonClientConfig = {
url: string;
authHeader?: string;
suppressSendErrors?: boolean;
@@ -256,7 +256,7 @@ interface PendingSend {
timeoutHandle: ReturnType<typeof setTimeout>;
}
export class DaemonClientV2 {
export class DaemonClient {
private transport: DaemonTransport | null = null;
private transportCleanup: Array<() => void> = [];
private rawMessageListeners: Set<(message: SessionOutboundMessage) => void> = new Set();
@@ -285,7 +285,7 @@ export class DaemonClientV2 {
private logger: Logger;
private pendingSendQueue: PendingSend[] = [];
constructor(private config: DaemonClientV2Config) {
constructor(private config: DaemonClientConfig) {
this.logger = config.logger ?? consoleLogger;
}

View File

@@ -326,7 +326,7 @@ describe("Codex app-server provider (integration)", () => {
"*** Add File: patch.txt",
"+patched",
"*** End Patch",
].join("\\n");
].join("\n");
const patchEvents = session.stream(
[
"Use the apply_patch tool and nothing else.",
@@ -335,7 +335,7 @@ describe("Codex app-server provider (integration)", () => {
"Apply the following patch exactly:",
patch,
"After it completes, reply PATCH_DONE.",
].join("\\n")
].join("\n")
);
for await (const event of patchEvents) {
@@ -612,7 +612,7 @@ describe("Codex app-server provider (integration)", () => {
"*** Add File: approval-test.txt",
"+ok",
"*** End Patch",
].join("\\n");
].join("\n");
const events = session.stream(
[
"Use the apply_patch tool and nothing else.",
@@ -621,7 +621,7 @@ describe("Codex app-server provider (integration)", () => {
"Apply the following patch exactly:",
patch,
"After approval, reply FILE_DONE.",
].join("\\n")
].join("\n")
);
let failure: string | null = null;

View File

@@ -18,7 +18,7 @@ import {
} from "./test-utils/dictation-e2e.js";
function tmpCwd(): string {
return mkdtempSync(path.join(tmpdir(), "daemon-client-v2-"));
return mkdtempSync(path.join(tmpdir(), "daemon-client-"));
}
function waitForSignal<T>(
@@ -52,7 +52,7 @@ function waitForSignal<T>(
});
}
describe("daemon client v2 E2E", () => {
describe("daemon client E2E", () => {
let ctx: DaemonTestContext;
beforeAll(async () => {

View File

@@ -10,7 +10,7 @@
*/
import { WebSocket } from "ws";
import { DaemonClientV2 } from "../../client/daemon-client-v2.js";
import { DaemonClient } from "../../client/daemon-client.js";
// Patch WebSocket to log all messages
const OriginalWebSocket = WebSocket;
@@ -36,7 +36,7 @@ async function testMultiAgentSequence() {
console.log("\n=== Testing multi-agent checkout sequence ===");
console.log(`Daemon URL: ${DAEMON_URL}`);
const client = new DaemonClientV2({
const client = new DaemonClient({
url: DAEMON_URL,
webSocketFactory: (url) => new LoggingWebSocket(url) as any,
reconnect: { enabled: false },

View File

@@ -4,7 +4,7 @@ export { loadConfig } from "./config.js";
export { resolvePaseoHome } from "./paseo-home.js";
export { createRootLogger, type LogLevel, type LogFormat } from "./logger.js";
export { loadPersistedConfig, type PersistedConfig } from "./persisted-config.js";
export { DaemonClientV2, type DaemonClientV2Config, type ConnectionState, type DaemonEvent } from "../client/daemon-client-v2.js";
export { DaemonClient, type DaemonClientConfig, type ConnectionState, type DaemonEvent } from "../client/daemon-client.js";
// Agent SDK types for CLI commands
export type {

View File

@@ -1987,7 +1987,11 @@ export class Session {
}
private async generateCommitMessage(cwd: string): Promise<string> {
const diff = await getCheckoutDiff(cwd, { mode: "uncommitted" }, { paseoHome: this.paseoHome });
const diff = await getCheckoutDiff(
cwd,
{ mode: "uncommitted", includeStructured: true },
{ paseoHome: this.paseoHome }
);
const schema = z.object({
message: z
.string()
@@ -1995,11 +1999,29 @@ export class Session {
.max(72)
.describe("Concise git commit message, imperative mood, no trailing period."),
});
const fileList =
diff.structured && diff.structured.length > 0
? [
"Files changed:",
...diff.structured.map((file) => {
const changeType = file.isNew ? "A" : file.isDeleted ? "D" : "M";
const status = file.status && file.status !== "ok" ? ` [${file.status}]` : "";
return `${changeType}\t${file.path}\t(+${file.additions} -${file.deletions})${status}`;
}),
].join("\n")
: "Files changed: (unknown)";
const maxPatchChars = 120_000;
const patch =
diff.diff.length > maxPatchChars
? `${diff.diff.slice(0, maxPatchChars)}\n\n... (diff truncated to ${maxPatchChars} chars)\n`
: diff.diff;
const prompt = [
"Write a concise git commit message for the changes below.",
"Return JSON only with a single field 'message'.",
"",
diff.diff.length > 0 ? diff.diff : "(No diff available)",
fileList,
"",
patch.length > 0 ? patch : "(No diff available)",
].join("\n");
try {
const result = await generateStructuredAgentResponse({
@@ -2034,6 +2056,7 @@ export class Session {
{
mode: "base",
baseRef,
includeStructured: true,
},
{ paseoHome: this.paseoHome }
);
@@ -2041,11 +2064,29 @@ export class Session {
title: z.string().min(1).max(72),
body: z.string().min(1),
});
const fileList =
diff.structured && diff.structured.length > 0
? [
"Files changed:",
...diff.structured.map((file) => {
const changeType = file.isNew ? "A" : file.isDeleted ? "D" : "M";
const status = file.status && file.status !== "ok" ? ` [${file.status}]` : "";
return `${changeType}\t${file.path}\t(+${file.additions} -${file.deletions})${status}`;
}),
].join("\n")
: "Files changed: (unknown)";
const maxPatchChars = 200_000;
const patch =
diff.diff.length > maxPatchChars
? `${diff.diff.slice(0, maxPatchChars)}\n\n... (diff truncated to ${maxPatchChars} chars)\n`
: diff.diff;
const prompt = [
"Write a pull request title and body for the changes below.",
"Return JSON only with fields 'title' and 'body'.",
"",
diff.diff.length > 0 ? diff.diff : "(No diff available)",
fileList,
"",
patch.length > 0 ? patch : "(No diff available)",
].join("\n");
try {
return await generateStructuredAgentResponse({

View File

@@ -1,13 +1,13 @@
import WebSocket from "ws";
import {
DaemonClientV2 as SharedDaemonClient,
type DaemonClientV2Config as SharedDaemonClientConfig,
DaemonClient as SharedDaemonClient,
type DaemonClientConfig as SharedDaemonClientConfig,
type CreateAgentRequestOptions,
type DaemonEvent,
type DaemonEventHandler,
type SendMessageOptions,
type WebSocketLike,
} from "../../client/daemon-client-v2.js";
} from "../../client/daemon-client.js";
export type DaemonClientConfig = Omit<
SharedDaemonClientConfig,

View File

@@ -1,4 +1,4 @@
import type { DaemonClientV2 } from "../../client/daemon-client-v2.js";
import type { DaemonClient } from "../../client/daemon-client.js";
import type { SessionOutboundMessage } from "../../shared/messages.js";
export interface MessageCollector {
@@ -7,7 +7,7 @@ export interface MessageCollector {
unsubscribe: () => void;
}
export function createMessageCollector(client: DaemonClientV2): MessageCollector {
export function createMessageCollector(client: DaemonClient): MessageCollector {
const messages: SessionOutboundMessage[] = [];
const unsubscribe = client.subscribeRawMessages((message) => {
messages.push(message);
@@ -20,4 +20,3 @@ export function createMessageCollector(client: DaemonClientV2): MessageCollector
unsubscribe,
};
}

View File

@@ -102,6 +102,36 @@ describe("checkout git utilities", () => {
expect(logMessage).toBe(message);
});
it("diffs base mode against merge-base (no base-only deletions)", async () => {
execSync("git checkout -b feature", { cwd: repoDir });
// Advance base branch after feature splits off.
execSync("git checkout main", { cwd: repoDir });
writeFileSync(join(repoDir, "base-only.txt"), "base\n");
execSync("git add base-only.txt", { cwd: repoDir });
execSync("git -c commit.gpgsign=false commit -m 'base only'", { cwd: repoDir });
// Make a feature change.
execSync("git checkout feature", { cwd: repoDir });
writeFileSync(join(repoDir, "feature.txt"), "feature\n");
execSync("git add feature.txt", { cwd: repoDir });
execSync("git -c commit.gpgsign=false commit -m 'feature commit'", { cwd: repoDir });
const diff = await getCheckoutDiff(repoDir, { mode: "base", baseRef: "main" });
expect(diff.diff).toContain("feature.txt");
expect(diff.diff).not.toContain("base-only.txt");
});
it("does not throw on large diffs (marks file as too_large)", async () => {
const large = Array.from({ length: 200_000 }, (_, i) => `line ${i}`).join("\n") + "\n";
writeFileSync(join(repoDir, "file.txt"), large);
const diff = await getCheckoutDiff(repoDir, { mode: "uncommitted", includeStructured: true });
expect(diff.structured?.some((f) => f.path === "file.txt" && f.status === "too_large")).toBe(
true
);
});
it("handles status/diff/commit in a .paseo worktree", async () => {
const result = await createWorktree({
branchName: "main",

View File

@@ -1,4 +1,4 @@
import { exec, execFile } from "child_process";
import { exec, execFile, spawn } from "child_process";
import { promisify } from "util";
import { resolve, dirname, basename } from "path";
import { realpathSync } from "fs";
@@ -14,6 +14,203 @@ const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = {
GIT_OPTIONAL_LOCKS: "0",
};
const SMALL_OUTPUT_MAX_BUFFER = 20 * 1024 * 1024; // 20MB
async function execGit(command: string, options: { cwd: string; env?: NodeJS.ProcessEnv }): Promise<{ stdout: string; stderr: string }> {
return execAsync(command, { ...options, maxBuffer: SMALL_OUTPUT_MAX_BUFFER });
}
async function execGitFile(
args: string[],
options: { cwd: string; env?: NodeJS.ProcessEnv }
): Promise<{ stdout: string; stderr: string }> {
return execFileAsync("git", args, { ...options, maxBuffer: SMALL_OUTPUT_MAX_BUFFER });
}
type LimitedTextResult = {
text: string;
truncated: boolean;
exitCode: number | null;
signal: NodeJS.Signals | null;
};
async function spawnLimitedText(params: {
cmd: string;
args: string[];
cwd: string;
env?: NodeJS.ProcessEnv;
maxBytes: number;
acceptExitCodes?: number[];
}): Promise<LimitedTextResult> {
const accept = new Set(params.acceptExitCodes ?? [0]);
return new Promise((resolvePromise, rejectPromise) => {
const child = spawn(params.cmd, params.args, {
cwd: params.cwd,
env: params.env,
stdio: ["ignore", "pipe", "pipe"],
});
const stdoutChunks: Buffer[] = [];
let stdoutBytes = 0;
let truncated = false;
const stop = () => {
if (child.killed) return;
try {
child.kill("SIGKILL");
} catch {
// ignore
}
};
child.stdout.on("data", (chunk: Buffer) => {
if (truncated) return;
stdoutBytes += chunk.length;
if (stdoutBytes > params.maxBytes) {
truncated = true;
stop();
return;
}
stdoutChunks.push(chunk);
});
// We don't buffer stderr (it can be large too). Keep it minimal for debugging.
let stderrPreview = "";
child.stderr.on("data", (chunk: Buffer) => {
if (stderrPreview.length > 2048) return;
stderrPreview += chunk.toString("utf8");
});
child.on("error", (error) => {
rejectPromise(error);
});
child.on("close", (code, signal) => {
if (code !== null && !accept.has(code) && !truncated) {
rejectPromise(new Error(`Command failed: ${params.cmd} ${params.args.join(" ")} (code ${code})\n${stderrPreview}`));
return;
}
resolvePromise({
text: Buffer.concat(stdoutChunks).toString("utf8"),
truncated,
exitCode: code,
signal,
});
});
});
}
type CheckoutFileChange = {
path: string;
oldPath?: string;
status: string;
isNew: boolean;
isDeleted: boolean;
isUntracked?: boolean;
};
async function listCheckoutFileChanges(cwd: string, ref: string): Promise<CheckoutFileChange[]> {
const changes: CheckoutFileChange[] = [];
const { stdout: nameStatusOut } = await execGit(`git diff --name-status ${ref}`, {
cwd,
env: READ_ONLY_GIT_ENV,
});
for (const line of nameStatusOut.split("\n").map((l) => l.trim()).filter(Boolean)) {
// `--name-status` uses TAB separators, which preserves filenames with spaces.
const tabParts = line.split("\t");
const rawStatus = (tabParts[0] ?? "").trim();
if (!rawStatus) continue;
if (rawStatus.startsWith("R") || rawStatus.startsWith("C")) {
const oldPath = tabParts[1];
const newPath = tabParts[2];
if (newPath) {
changes.push({
path: newPath,
...(oldPath ? { oldPath } : {}),
status: rawStatus,
isNew: false,
isDeleted: false,
});
}
continue;
}
const path = tabParts[1];
if (!path) continue;
const code = rawStatus[0];
changes.push({
path,
status: rawStatus,
isNew: code === "A",
isDeleted: code === "D",
});
}
const { stdout: untrackedOut } = await execGit("git ls-files --others --exclude-standard", {
cwd,
env: READ_ONLY_GIT_ENV,
});
for (const file of untrackedOut.split("\n").map((l) => l.trim()).filter(Boolean)) {
changes.push({
path: file,
status: "U",
isNew: true,
isDeleted: false,
isUntracked: true,
});
}
// Deduplicate by path (prefer tracked status over untracked marker if both appear).
const byPath = new Map<string, CheckoutFileChange>();
for (const change of changes) {
const existing = byPath.get(change.path);
if (!existing) {
byPath.set(change.path, change);
continue;
}
if (existing.isUntracked && !change.isUntracked) {
byPath.set(change.path, change);
}
}
return Array.from(byPath.values());
}
async function tryResolveMergeBase(cwd: string, baseRef: string): Promise<string | null> {
try {
const { stdout } = await execGit(`git merge-base ${baseRef} HEAD`, { cwd, env: READ_ONLY_GIT_ENV });
const sha = stdout.trim();
return sha.length > 0 ? sha : null;
} catch {
return null;
}
}
type FileStat = { additions: number; deletions: number; isBinary: boolean } | null;
async function tryGetNumstat(cwd: string, args: string[]): Promise<FileStat> {
try {
const { stdout } = await execGitFile(args, { cwd, env: READ_ONLY_GIT_ENV });
const line = stdout.trim().split("\n").map((l) => l.trim()).filter(Boolean)[0] ?? "";
if (!line) return null;
const [aRaw, dRaw] = line.split(/\s+/);
if (!aRaw || !dRaw) return null;
if (aRaw === "-" || dRaw === "-") {
return { additions: 0, deletions: 0, isBinary: true };
}
const additions = Number.parseInt(aRaw, 10);
const deletions = Number.parseInt(dRaw, 10);
if (Number.isNaN(additions) || Number.isNaN(deletions)) {
return null;
}
return { additions, deletions, isBinary: false };
} catch {
return null;
}
}
export class NotGitRepoError extends Error {
readonly cwd: string;
readonly code = "NOT_GIT_REPO";
@@ -435,32 +632,58 @@ async function getAheadOfOrigin(cwd: string, currentBranch: string): Promise<num
}
}
async function getUntrackedDiff(cwd: string): Promise<string> {
let untrackedDiff = "";
try {
const { stdout: untrackedFiles } = await execAsync(
"git ls-files --others --exclude-standard",
{ cwd, env: READ_ONLY_GIT_ENV }
);
const newFiles = untrackedFiles.trim().split("\n").filter(Boolean);
const PER_FILE_DIFF_MAX_BYTES = 1024 * 1024; // 1MB
const TOTAL_DIFF_MAX_BYTES = 2 * 1024 * 1024; // 2MB
for (const file of newFiles) {
try {
const { stdout: fileDiff } = await execAsync(
`git diff --no-index /dev/null "${file}" || true`,
{ cwd, env: READ_ONLY_GIT_ENV }
);
if (fileDiff) {
untrackedDiff += fileDiff;
}
} catch {
// Ignore errors for individual files
}
}
} catch {
// Ignore errors getting untracked files
function buildPlaceholderParsedDiffFile(
change: CheckoutFileChange,
options: { status: "too_large" | "binary"; stat?: FileStat }
): ParsedDiffFile {
return {
path: change.path,
isNew: change.isNew,
isDeleted: change.isDeleted,
additions: options.stat?.additions ?? 0,
deletions: options.stat?.deletions ?? 0,
hunks: [],
status: options.status,
};
}
async function getPerFileDiffText(
cwd: string,
ref: string,
change: CheckoutFileChange
): Promise<{ text: string; truncated: boolean; stat: FileStat }> {
const stat: FileStat =
change.isUntracked
? null
: await tryGetNumstat(cwd, ["diff", "--numstat", ref, "--", change.path]);
if (stat?.isBinary) {
return { text: "", truncated: false, stat };
}
return untrackedDiff;
if (change.isUntracked) {
const result = await spawnLimitedText({
cmd: "git",
args: ["diff", "--no-index", "/dev/null", "--", change.path],
cwd,
env: READ_ONLY_GIT_ENV,
maxBytes: PER_FILE_DIFF_MAX_BYTES,
acceptExitCodes: [0, 1],
});
return { text: result.text, truncated: result.truncated, stat };
}
const result = await spawnLimitedText({
cmd: "git",
args: ["diff", ref, "--", change.path],
cwd,
env: READ_ONLY_GIT_ENV,
maxBytes: PER_FILE_DIFF_MAX_BYTES,
});
return { text: result.text, truncated: result.truncated, stat };
}
export async function getCheckoutStatus(
@@ -530,37 +753,102 @@ export async function getCheckoutDiff(
): Promise<CheckoutDiffResult> {
await requireGitRepo(cwd);
let diff = "";
let refForDiff: string;
if (compare.mode === "uncommitted") {
const { stdout: trackedDiff } = await execAsync("git diff HEAD", {
cwd,
env: READ_ONLY_GIT_ENV,
});
const untrackedDiff = await getUntrackedDiff(cwd);
diff = trackedDiff + untrackedDiff;
refForDiff = "HEAD";
} else {
const configured = await getConfiguredBaseRefForCwd(cwd, context);
const baseRef = configured.baseRef ?? compare.baseRef ?? (await resolveBaseRef(cwd));
if (!baseRef) {
diff = "";
} else if (configured.isPaseoOwnedWorktree && compare.baseRef && compare.baseRef !== baseRef) {
throw new Error(`Base ref mismatch: expected ${baseRef}, got ${compare.baseRef}`);
} else {
const normalizedBaseRef = normalizeLocalBranchRefName(baseRef);
// Diff base ref against working tree (includes uncommitted changes)
const { stdout: trackedDiff } = await execAsync(`git diff ${normalizedBaseRef}`, {
cwd,
env: READ_ONLY_GIT_ENV,
});
const untrackedDiff = await getUntrackedDiff(cwd);
diff = trackedDiff + untrackedDiff;
return { diff: "" };
}
if (configured.isPaseoOwnedWorktree && compare.baseRef && compare.baseRef !== baseRef) {
throw new Error(`Base ref mismatch: expected ${baseRef}, got ${compare.baseRef}`);
}
const normalizedBaseRef = normalizeLocalBranchRefName(baseRef);
const bestBaseRef = await resolveBestBaseRefForMerge(cwd, normalizedBaseRef);
refForDiff = (await tryResolveMergeBase(cwd, bestBaseRef)) ?? bestBaseRef;
}
const changes = await listCheckoutFileChanges(cwd, refForDiff);
changes.sort((a, b) => a.path.localeCompare(b.path));
const structured: ParsedDiffFile[] = [];
let diffText = "";
let diffBytes = 0;
const appendDiff = (text: string) => {
if (!text) return;
if (diffBytes >= TOTAL_DIFF_MAX_BYTES) return;
const buf = Buffer.from(text, "utf8");
if (diffBytes + buf.length <= TOTAL_DIFF_MAX_BYTES) {
diffText += text;
diffBytes += buf.length;
return;
}
const remaining = TOTAL_DIFF_MAX_BYTES - diffBytes;
if (remaining > 0) {
diffText += buf.subarray(0, remaining).toString("utf8");
diffBytes = TOTAL_DIFF_MAX_BYTES;
}
};
for (const change of changes) {
const { text, truncated, stat } = await getPerFileDiffText(cwd, refForDiff, change);
if (!compare.includeStructured) {
if (stat?.isBinary) {
appendDiff(`# ${change.path}: binary diff omitted\n`);
} else if (truncated) {
appendDiff(`# ${change.path}: diff too large omitted\n`);
} else {
appendDiff(text);
}
if (diffBytes >= TOTAL_DIFF_MAX_BYTES) {
break;
}
continue;
}
if (stat?.isBinary) {
structured.push(buildPlaceholderParsedDiffFile(change, { status: "binary", stat }));
appendDiff(`# ${change.path}: binary diff omitted\n`);
continue;
}
if (truncated) {
structured.push(buildPlaceholderParsedDiffFile(change, { status: "too_large", stat }));
appendDiff(`# ${change.path}: diff too large omitted\n`);
continue;
}
appendDiff(text);
const parsed = await parseAndHighlightDiff(text, cwd);
const parsedFile =
parsed[0] ??
({
path: change.path,
isNew: change.isNew,
isDeleted: change.isDeleted,
additions: stat?.additions ?? 0,
deletions: stat?.deletions ?? 0,
hunks: [],
} satisfies ParsedDiffFile);
structured.push({
...parsedFile,
path: change.path,
isNew: change.isNew,
isDeleted: change.isDeleted,
status: "ok",
});
}
if (compare.includeStructured) {
return { diff, structured: await parseAndHighlightDiff(diff, cwd) };
return { diff: diffText, structured };
}
return { diff };
return { diff: diffText };
}
export async function commitChanges(