diff --git a/apps/server/src/codexTextGenerator.ts b/apps/server/src/codexTextGenerator.ts index fac58a2f8e468a8b48adee3db898a3990ad80559..4ea6f35839eaeeff9da25ead11723b4784787388 100644 --- a/apps/server/src/codexTextGenerator.ts +++ b/apps/server/src/codexTextGenerator.ts @@ -117,9 +117,11 @@ async function runCodexJson({ "codex-schema", JSON.stringify(outputSchemaJson), ); - const outputPath = await writeTempFile("codex-output", ""); + let outputPath: string | null = null; try { + outputPath = await writeTempFile("codex-output", ""); + await runProcess( "codex", [ @@ -154,7 +156,10 @@ async function runCodexJson({ return parse(parsedJson); } finally { - await Promise.all([safeUnlink(schemaPath), safeUnlink(outputPath)]); + await Promise.all([ + safeUnlink(schemaPath), + ...(outputPath ? [safeUnlink(outputPath)] : []), + ]); } } diff --git a/apps/server/src/git.test.ts b/apps/server/src/git.test.ts index 3a2a57220d0c657d887a0d9f66ae00ce6627f9ba..7b691ed8fb9f7a30bffcfae4df6f3fd7459b937f 100644 --- a/apps/server/src/git.test.ts +++ b/apps/server/src/git.test.ts @@ -462,7 +462,7 @@ describe("git integration", () => { await using remote = await makeTmpDir(); await initRepoWithCommit(tmp.path); await git(remote.path, "init --bare"); - await git(tmp.path, `remote add origin '${remote.path}'`); + await git(tmp.path, `remote add origin ${JSON.stringify(remote.path)}`); await createGitBranch({ cwd: tmp.path, branch: "feature/core-push" }); await checkoutGitBranch({ cwd: tmp.path, branch: "feature/core-push" }); diff --git a/apps/server/src/git.ts b/apps/server/src/git.ts index f0518faa32eeb9ef0d60020d9448de8ce8e3de11..6498a71bb030800955533d6d1714b0ed28938ca8 100644 --- a/apps/server/src/git.ts +++ b/apps/server/src/git.ts @@ -102,12 +102,15 @@ function commandLabel(args: readonly string[]): string { return `git ${args.join(" ")}`; } -function normalizeGitSpawnError(error: unknown): Error { +function normalizeGitSpawnError(cwd: string, error: unknown): Error { if (!(error instanceof Error)) { return new Error("Failed to run git command."); } const code = (error as NodeJS.ErrnoException).code; if (code === "ENOENT") { + if (!fs.existsSync(cwd)) { + return new Error(`Working directory does not exist: ${cwd}`); + } return new Error("Git is required but not available on PATH."); } return new Error(`Failed to run git command: ${error.message}`); @@ -135,10 +138,13 @@ async function runGitOrThrow( try { result = await runGit(args, cwd, options.timeoutMs); } catch (error) { - throw normalizeGitSpawnError(error); + throw normalizeGitSpawnError(cwd, error); } - if (!options.allowNonZeroExit && (result.code !== 0 || result.timedOut)) { + if (result.timedOut) { + throw normalizeGitExecutionError(args, result); + } + if (!options.allowNonZeroExit && result.code !== 0) { throw normalizeGitExecutionError(args, result); } return result; @@ -242,7 +248,7 @@ export class GitCoreService { throw new Error("Cannot push from detached HEAD."); } - if (details.hasUpstream && details.aheadCount === 0) { + if (details.hasUpstream && details.aheadCount === 0 && details.behindCount === 0) { return { status: "skipped_up_to_date", branch, diff --git a/apps/server/src/gitManager.ts b/apps/server/src/gitManager.ts index 775587c21214b2d9c67b3761f13226e589d2b20f..9c16f1761cbdee771521a3da2b17d89e84bc5775 100644 --- a/apps/server/src/gitManager.ts +++ b/apps/server/src/gitManager.ts @@ -21,6 +21,8 @@ import { CodexTextGenerator } from "./codexTextGenerator"; import { GitCoreService } from "./git"; import { type ProcessRunOptions, runProcess } from "./processRunner"; +const GH_CLI_TIMEOUT_MS = 30_000; + interface GitManagerDeps { runProcess?: ( command: string, @@ -107,10 +109,21 @@ function sanitizeCommitMessage( function extractBranchFromRef(ref: string): string { const normalized = ref.trim(); - const afterSlash = normalized.includes("/") - ? normalized.slice(normalized.lastIndexOf("/") + 1) - : normalized; - return afterSlash.trim(); + + if (normalized.startsWith("refs/remotes/")) { + const withoutPrefix = normalized.slice("refs/remotes/".length); + const firstSlash = withoutPrefix.indexOf("/"); + if (firstSlash === -1) { + return withoutPrefix.trim(); + } + return withoutPrefix.slice(firstSlash + 1).trim(); + } + + const firstSlash = normalized.indexOf("/"); + if (firstSlash === -1) { + return normalized; + } + return normalized.slice(firstSlash + 1).trim(); } function asCommandNotFound( @@ -401,7 +414,7 @@ export class GitManager { private async runGh(cwd: string, args: readonly string[]): Promise { try { - await this.run("gh", args, { cwd }); + await this.run("gh", args, { cwd, timeoutMs: GH_CLI_TIMEOUT_MS }); } catch (error) { throw ( asCommandNotFound("gh", error) ?? @@ -413,7 +426,10 @@ export class GitManager { private async runGhStdout(cwd: string, args: readonly string[]): Promise { try { - const result = await this.run("gh", args, { cwd }); + const result = await this.run("gh", args, { + cwd, + timeoutMs: GH_CLI_TIMEOUT_MS, + }); return result.stdout; } catch (error) { throw ( diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index c1d7102c1b9298712b2f804dc6e79dbea77fe3b6..3d690ed6b88bfdbe6f3e1e9c0c02d61a3afc4c38 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -6,6 +6,7 @@ export interface ProcessRunOptions { env?: NodeJS.ProcessEnv | undefined; stdin?: string | undefined; allowNonZeroExit?: boolean | undefined; + maxBufferBytes?: number | undefined; } export interface ProcessRunResult { @@ -52,12 +53,39 @@ function normalizeExitError( return new Error(`${commandLabel(command, args)} ${reason}.${detail}`); } +function normalizeStdinError( + command: string, + args: readonly string[], + error: unknown, +): Error { + if (!(error instanceof Error)) { + return new Error(`Failed to write stdin for ${commandLabel(command, args)}.`); + } + return new Error( + `Failed to write stdin for ${commandLabel(command, args)}: ${error.message}`, + ); +} + +function normalizeBufferError( + command: string, + args: readonly string[], + stream: "stdout" | "stderr", + maxBufferBytes: number, +): Error { + return new Error( + `${commandLabel(command, args)} exceeded ${stream} buffer limit (${maxBufferBytes} bytes).`, + ); +} + +const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024; + export async function runProcess( command: string, args: readonly string[], options: ProcessRunOptions = {}, ): Promise { const timeoutMs = options.timeoutMs ?? 60_000; + const maxBufferBytes = options.maxBufferBytes ?? DEFAULT_MAX_BUFFER_BYTES; return new Promise((resolve, reject) => { const child = spawn(command, args, { @@ -68,6 +96,8 @@ export async function runProcess( let stdout = ""; let stderr = ""; + let stdoutBytes = 0; + let stderrBytes = 0; let timedOut = false; let settled = false; let forceKillTimer: ReturnType | null = null; @@ -90,12 +120,47 @@ export async function runProcess( callback(); }; + const fail = (error: Error): void => { + child.kill("SIGTERM"); + finalize(() => { + reject(error); + }); + }; + + const appendOutput = ( + stream: "stdout" | "stderr", + chunk: Buffer | string, + ): Error | null => { + const text = chunk.toString(); + const byteLength = Buffer.byteLength(text); + if (stream === "stdout") { + stdout += text; + stdoutBytes += byteLength; + if (stdoutBytes > maxBufferBytes) { + return normalizeBufferError(command, args, "stdout", maxBufferBytes); + } + } else { + stderr += text; + stderrBytes += byteLength; + if (stderrBytes > maxBufferBytes) { + return normalizeBufferError(command, args, "stderr", maxBufferBytes); + } + } + return null; + }; + child.stdout.on("data", (chunk: Buffer | string) => { - stdout += chunk.toString(); + const error = appendOutput("stdout", chunk); + if (error) { + fail(error); + } }); child.stderr.on("data", (chunk: Buffer | string) => { - stderr += chunk.toString(); + const error = appendOutput("stderr", chunk); + if (error) { + fail(error); + } }); child.once("error", (error) => { @@ -125,10 +190,20 @@ export async function runProcess( }); }); + child.stdin.once("error", (error) => { + fail(normalizeStdinError(command, args, error)); + }); + if (options.stdin !== undefined) { - child.stdin.write(options.stdin); + child.stdin.write(options.stdin, (error) => { + if (error) { + fail(normalizeStdinError(command, args, error)); + return; + } + child.stdin.end(); + }); + return; } child.stdin.end(); }); } - diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index df5b221e328c07eef3f6b14788b4c36defaf9ff7..1082135510a8bc5d96612c5c0056a29a3f474660 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -196,6 +196,7 @@ export default function ChatView() { const modelMenuRef = useRef(null); const editorMenuRef = useRef(null); const gitMenuRef = useRef(null); + const latestGitCwdRef = useRef(null); const activeThread = state.threads.find((t) => t.id === state.activeThreadId); const activeProject = state.projects.find((p) => p.id === activeThread?.projectId); @@ -302,19 +303,27 @@ export default function ChatView() { !api || !gitCwd || !gitStatus || isGitActionRunning; const pushActionDisabled = gitActionsDisabled || gitStatus.branch === null; + useEffect(() => { + latestGitCwdRef.current = gitCwd; + }, [gitCwd]); + const refreshGitStatus = useCallback(async () => { - if (!api || !gitCwd) { + const requestCwd = gitCwd; + if (!api || !requestCwd) { setGitStatus(null); return; } - const nextStatus = await api.git.status({ cwd: gitCwd }); + const nextStatus = await api.git.status({ cwd: requestCwd }); + if (latestGitCwdRef.current !== requestCwd) return; setGitStatus(nextStatus); + setGitActionError(null); }, [api, gitCwd]); const runGitAction = useCallback( async (action: GitStackedAction) => { if (!api || !gitCwd) return; + const actionCwd = gitCwd; setIsGitMenuOpen(false); setIsGitActionRunning(true); @@ -322,7 +331,7 @@ export default function ChatView() { setGitActionNotice(null); try { const result = await api.git.runStackedAction({ - cwd: gitCwd, + cwd: actionCwd, action, }); setGitActionNotice(summarizeGitActionResult(result)); @@ -333,7 +342,9 @@ export default function ChatView() { } finally { setIsGitActionRunning(false); try { - await refreshGitStatus(); + if (latestGitCwdRef.current === actionCwd) { + await refreshGitStatus(); + } } catch { setGitStatus(null); } @@ -404,6 +415,7 @@ export default function ChatView() { const nextStatus = await api.git.status({ cwd: gitCwd }); if (!cancelled) { setGitStatus(nextStatus); + setGitActionError(null); } } catch (error) { if (!cancelled) {