Skip to content
Merged
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
55 changes: 22 additions & 33 deletions scripts/claude-companion.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import {
createJobProgressUpdater,
createJobRecord,
createProgressReporter,
createWorkerLogStdio,
nowIso,
runTrackedJob,
SESSION_ID_ENV
Expand Down Expand Up @@ -1080,21 +1081,26 @@ function buildReviewRequest({
return { cwd, base, scope, model, effort, focusText, reviewName, markViewedOnSuccess };
}

function spawnDetachedReviewWorker(cwd, jobId) {
function spawnDetachedWorker(cwd, command, jobId, logFile) {
const scriptPath = path.join(ROOT_DIR, "scripts", "claude-companion.mjs");
const child = spawn(
process.execPath,
[scriptPath, "review-worker", "--cwd", cwd, "--job-id", jobId],
{
cwd,
env: process.env,
detached: true,
stdio: "ignore",
windowsHide: true
}
);
child.unref();
return child;
const workerLog = createWorkerLogStdio(logFile);
try {
const child = spawn(
process.execPath,
[scriptPath, command, "--cwd", cwd, "--job-id", jobId],
{
cwd,
env: process.env,
detached: true,
stdio: workerLog.stdio,
windowsHide: true
}
);
child.unref();
return child;
} finally {
workerLog.close();
}
}

function enqueueBackgroundReview(cwd, job, request) {
Expand All @@ -1111,7 +1117,7 @@ function enqueueBackgroundReview(cwd, job, request) {
};
writeJobFile(job.workspaceRoot, job.id, queuedRecord);

const child = spawnDetachedReviewWorker(cwd, job.id);
const child = spawnDetachedWorker(cwd, "review-worker", job.id, logFile);
if (child.pid != null) {
let pidIdentity = null;
try {
Expand Down Expand Up @@ -1289,23 +1295,6 @@ async function runForegroundCommand(job, runner, options = {}) {
// Background task spawning
// ---------------------------------------------------------------------------

function spawnDetachedTaskWorker(cwd, jobId) {
const scriptPath = path.join(ROOT_DIR, "scripts", "claude-companion.mjs");
const child = spawn(
process.execPath,
[scriptPath, "task-worker", "--cwd", cwd, "--job-id", jobId],
{
cwd,
env: process.env,
detached: true,
stdio: "ignore",
windowsHide: true
}
);
child.unref();
return child;
}

function enqueueBackgroundTask(cwd, job, request) {
const { logFile } = createTrackedProgress(job);
appendLogLine(logFile, "Queued for background execution.");
Expand All @@ -1320,7 +1309,7 @@ function enqueueBackgroundTask(cwd, job, request) {
};
writeJobFile(job.workspaceRoot, job.id, queuedRecord);

const child = spawnDetachedTaskWorker(cwd, job.id);
const child = spawnDetachedWorker(cwd, "task-worker", job.id, logFile);
if (child.pid != null) {
let pidIdentity = null;
try {
Expand Down
29 changes: 28 additions & 1 deletion scripts/lib/git.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { isProbablyText } from "./fs.mjs";
import { formatCommandFailure, runCommand, runCommandChecked } from "./process.mjs";

const MAX_UNTRACKED_BYTES = 24 * 1024;
const MAX_UNTRACKED_TOTAL_BYTES = 32 * 1024;
const MAX_INLINE_REVIEW_DIFF_BYTES = 64 * 1024;
const REVIEW_DIFF_READ_MAX_BUFFER = MAX_INLINE_REVIEW_DIFF_BYTES + 8 * 1024;
const HASH_OBJECT_BATCH_SIZE = 128;
Expand Down Expand Up @@ -317,6 +318,32 @@ function formatUntrackedFile(cwd, relativePath) {
}
}

function formatUntrackedFiles(cwd, relativePaths) {
const sections = [];
let totalBytes = 0;
let omitted = 0;

for (const relativePath of relativePaths) {
const section = formatUntrackedFile(cwd, relativePath);
const separator = sections.length > 0 ? "\n\n" : "";
const sectionBytes = Buffer.byteLength(separator + section, "utf8");
if (totalBytes + sectionBytes > MAX_UNTRACKED_TOTAL_BYTES) {
omitted += 1;
continue;
}
sections.push(section);
totalBytes += sectionBytes;
}

if (omitted > 0) {
sections.push(
`### Omitted untracked files\n(skipped: ${omitted} untracked file(s) exceed the aggregate context limit)\nInspect them with \`git ls-files --others --exclude-standard\`.`
);
}

return sections.join("\n\n");
}

function shouldInlineReviewDiff(...sections) {
let totalBytes = 0;
for (const section of sections) {
Expand Down Expand Up @@ -346,7 +373,7 @@ function readBoundedGitDiff(cwd, args) {

function collectWorkingTreeContext(cwd, state) {
const status = gitChecked(cwd, ["status", "--short"]).stdout.trim();
const untrackedBody = state.untracked.map((file) => formatUntrackedFile(cwd, file)).join("\n\n");
const untrackedBody = formatUntrackedFiles(cwd, state.untracked);
const stagedDiff = readBoundedGitDiff(cwd, ["diff", "--cached", "--no-ext-diff", "--submodule=diff"]);
const unstagedDiff = readBoundedGitDiff(cwd, ["diff", "--no-ext-diff", "--submodule=diff"]);
const inlineDiffs =
Expand Down
9 changes: 9 additions & 0 deletions scripts/lib/tracked-jobs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ export function createJobLogFile(workspaceRoot, jobId, title) {
return logFile;
}

/** @returns {{ stdio: import("node:child_process").StdioOptions, close: () => void }} */
export function createWorkerLogStdio(logFile) {
const fd = fs.openSync(logFile, "a");
return {
stdio: ["ignore", fd, fd],
close: () => fs.closeSync(fd),
};
}

export function createJobRecord(base, options = {}) {
const env = options.env ?? process.env;
const sessionId =
Expand Down
27 changes: 27 additions & 0 deletions tests/git.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,33 @@ describe("collectReviewContext", () => {
assert.match(context.content, /tracked-link[\s\S]*skipped: symlink/);
});

it("bounds aggregate untracked file context", () => {
const repo = createRepo();
fs.writeFileSync(path.join(repo, "tracked.txt"), "tracked\n", "utf8");
runGit(repo, ["add", "tracked.txt"]);
runGit(repo, ["commit", "-m", "tracked"]);

for (let index = 0; index < 10; index += 1) {
fs.writeFileSync(
path.join(repo, `untracked-${index}.txt`),
`file-${index}\n${"x".repeat(6 * 1024)}`,
"utf8"
);
}

const context = collectReviewContext(repo, {
mode: "working-tree",
label: "working tree diff",
explicit: true,
});

assert.match(context.content, /file-0/);
assert.doesNotMatch(context.content, /file-9\nx/);
assert.match(context.content, /Omitted untracked files/);
assert.match(context.content, /git ls-files --others --exclude-standard/);
assert.ok(Buffer.byteLength(context.content, "utf8") < 64 * 1024);
});

it("omits very large working-tree diffs and tells the reviewer to inspect git directly", () => {
const repo = createRepo();
const largeText = `${"x".repeat(200)}\n`.repeat(500);
Expand Down
26 changes: 26 additions & 0 deletions tests/tracked-jobs.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
appendLogLine,
appendLogBlock,
createJobLogFile,
createWorkerLogStdio,
createJobRecord,
runTrackedJob,
} from "../scripts/lib/tracked-jobs.mjs";
Expand Down Expand Up @@ -194,6 +195,31 @@ describe("createJobLogFile", () => {
});
});

describe("createWorkerLogStdio", () => {
it("redirects worker stdout and stderr to the job log", () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "worker-log-"));
const logFile = path.join(dir, "worker.log");
fs.writeFileSync(logFile, "", "utf8");

const workerLog = createWorkerLogStdio(logFile);
try {
const result = spawnSync(
process.execPath,
["-e", "console.log('worker out'); console.error('worker err')"],
{ stdio: workerLog.stdio }
);
assert.equal(result.status, 0);
} finally {
workerLog.close();
}

const content = fs.readFileSync(logFile, "utf8");
assert.match(content, /worker out/);
assert.match(content, /worker err/);
fs.rmSync(dir, { recursive: true, force: true });
});
});

// ---------------------------------------------------------------------------
// createJobRecord
// ---------------------------------------------------------------------------
Expand Down