From f01898a9c3ba5fd21f1c19c5b4a8944b486833af Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 8 Jul 2026 18:06:52 +0900 Subject: [PATCH] Bump to 0.3.1: fix backend hangs, UI layout, and dead-code type error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A day of dogfooding locode on itself surfaced several real bugs, most centered on the backend request lifecycle silently hanging with no error surfaced to the user: - bash/hooks: execa's shell:true defaulted to cmd.exe on Windows, which lacks Unix tools (cat, sed, grep, ...) and fails a whole pipe with an unhelpful exit 255 if any stage isn't found. Added src/utils/shell.ts to resolve real Git Bash when available. - agent/loop: the backend client's own `timeout` only bounds time-to-first-byte — once a streaming response starts, a backend that goes silent mid-stream (or one that sends periodic content-less "heartbeat" chunks to keep the connection alive through a proxy) hung the request forever with zero output. Added an idle-abort guard that pokes only on chunks carrying real progress (usage/text/tool-calls/ finish_reason), covering the main stream, the non-streaming malformed-JSON retry, and compactSession. - agent/loop: a single tool-call-heavy turn (e.g. reading dozens of files) had no auto-compaction safety net at all — shouldAutoCompact was only ever checked between turns, never during one, so a long turn could blow past the context window with no mid-turn correction. Now checked every iteration; re-anchors mutationCommitLength afterward since compaction replaces session.messages wholesale. - agent/loop: hitting the iteration cap surfaced as an opaque "Request failed" even when every prior tool call (including file edits) had actually succeeded. Added MaxIterationsError, rendered as a calm "paused, send another message to continue" notice instead, and raised the default cap 25 -> 50. - agent/loop: removed a dead-code branch (fallback-malformed retry handling placed inside a native-mode-only code path, so session.mode === "fallback" could never be true there) that TypeScript correctly flagged as a type error. - ui/App: text_done fires with an empty fullText for pure tool-call turns (no preceding prose); the assistant-message push had no guard, rendering a blank line every time. Now skipped when fullText is empty. - ui/App+index: alternate-screen-buffer mode and StatusBar-above-ChatInput layout, fixing the reported "prompt starts below the dashboard, jumps up on space" glitch. Added input history (up/down recall). - utils/writeFileAtomic: random temp-file suffix instead of a fixed `.tmp`, so concurrent writes to the same directory can't collide. Also fixes two hardcoded version strings (cli.ts --version, mcp/client.ts's self-reported MCP identity) that were left stale at 0.2.0 through the 0.3.0 bump, and a stale maxIterations default noted in the README. Co-Authored-By: Claude Sonnet 5 --- README.md | 2 +- package-lock.json | 4 +- package.json | 2 +- src/agent/events.ts | 5 +- src/agent/idleAbort.test.ts | 63 ++++++++++++ src/agent/loop.test.ts | 190 ++++++++++++++++++++++++++++++++++- src/agent/loop.ts | 180 +++++++++++++++++++++++++++++---- src/agent/session.ts | 11 ++ src/cli.ts | 2 +- src/config/defaults.ts | 8 +- src/hooks/runner.ts | 3 +- src/mcp/client.ts | 2 +- src/tools/bash.ts | 3 +- src/tools/listFiles.ts | 12 ++- src/ui/ink/App.tsx | 82 ++++++++++----- src/ui/ink/ChatInput.tsx | 46 +++++++-- src/ui/ink/index.tsx | 6 ++ src/utils/shell.test.ts | 66 ++++++++++++ src/utils/shell.ts | 48 +++++++++ src/utils/writeFileAtomic.ts | 22 +++- 20 files changed, 689 insertions(+), 68 deletions(-) create mode 100644 src/agent/idleAbort.test.ts create mode 100644 src/utils/shell.test.ts create mode 100644 src/utils/shell.ts diff --git a/README.md b/README.md index f8c7515..518323e 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ Config precedence: CLI flags > env vars (`LOCODE_BACKEND`, `LOCODE_MODEL`, `LOCO locode config set backend ollama locode config set model qwen3-coder:30b locode config set contextWindow 32768 # fallback size when auto-detection fails -locode config set maxIterations 40 # max tool calls per turn before locode gives up (default 25) +locode config set maxIterations 40 # max tool calls per turn before locode gives up (default 50) locode config set autoCompactThreshold 0.85 # fraction of context window at which auto-compact triggers locode config set requestTimeoutMs 300000 # per-request timeout in ms (default 180000); raise this if # your backend queues requests behind a concurrency limit diff --git a/package-lock.json b/package-lock.json index f2de5bd..a2506dd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "locode", - "version": "0.3.0", + "version": "0.3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "locode", - "version": "0.3.0", + "version": "0.3.1", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", "@vscode/ripgrep": "^1.18.0", diff --git a/package.json b/package.json index 5a50d70..d385785 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "locode", - "version": "0.3.0", + "version": "0.3.1", "description": "Agentic coding CLI for local models served via Ollama and LM Studio", "type": "module", "bin": { diff --git a/src/agent/events.ts b/src/agent/events.ts index e492e9e..a9ed845 100644 --- a/src/agent/events.ts +++ b/src/agent/events.ts @@ -8,6 +8,9 @@ export type AgentEvent = | { type: "tool_call"; label: string } | { type: "tool_result"; summary: string; isError: boolean } /** A hook (see hooks/runner.ts) blocked something or failed non-fatally — surfaced as a notice. */ - | { type: "hook_notice"; text: string; isError: boolean }; + | { type: "hook_notice"; text: string; isError: boolean } + /** A general informational notice from the loop itself (not tied to a hook) — e.g. a mid-turn + * auto-compaction. Surfaced the same way as hook_notice. */ + | { type: "notice"; text: string; isError: boolean }; export type AgentEventHandler = (event: AgentEvent) => void; \ No newline at end of file diff --git a/src/agent/idleAbort.test.ts b/src/agent/idleAbort.test.ts new file mode 100644 index 0000000..be73e5b --- /dev/null +++ b/src/agent/idleAbort.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createIdleAbort } from "./loop.js"; + +describe("createIdleAbort", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("aborts after idleMs with no poke()", () => { + const guard = createIdleAbort(1000); + expect(guard.signal.aborted).toBe(false); + vi.advanceTimersByTime(999); + expect(guard.signal.aborted).toBe(false); + vi.advanceTimersByTime(2); + expect(guard.signal.aborted).toBe(true); + expect(guard.didTimeOut()).toBe(true); + guard.dispose(); + }); + + it("stays alive as long as poke() keeps resetting the timer", () => { + const guard = createIdleAbort(1000); + for (let i = 0; i < 5; i++) { + vi.advanceTimersByTime(700); + guard.poke(); + } + expect(guard.signal.aborted).toBe(false); + expect(guard.didTimeOut()).toBe(false); + guard.dispose(); + }); + + it("aborts if poking stops (simulating a mid-stream stall)", () => { + const guard = createIdleAbort(1000); + guard.poke(); + vi.advanceTimersByTime(700); + guard.poke(); + // No more pokes after this — simulates the backend going silent mid-stream. + vi.advanceTimersByTime(1001); + expect(guard.signal.aborted).toBe(true); + expect(guard.didTimeOut()).toBe(true); + guard.dispose(); + }); + + it("propagates an external signal's abort without marking it as a timeout", () => { + const external = new AbortController(); + const guard = createIdleAbort(1000, external.signal); + external.abort(); + expect(guard.signal.aborted).toBe(true); + expect(guard.didTimeOut()).toBe(false); + guard.dispose(); + }); + + it("dispose() prevents a pending timer from firing later", () => { + const guard = createIdleAbort(1000); + guard.dispose(); + vi.advanceTimersByTime(2000); + expect(guard.signal.aborted).toBe(false); + expect(guard.didTimeOut()).toBe(false); + }); +}); diff --git a/src/agent/loop.test.ts b/src/agent/loop.test.ts index fe4482f..f755dd0 100644 --- a/src/agent/loop.test.ts +++ b/src/agent/loop.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { shouldAutoCompact } from "./loop.js"; +import { z } from "zod"; +import { MaxIterationsError, runTurn, shouldAutoCompact } from "./loop.js"; +import { createSession } from "./session.js"; +import { buildToolSet } from "../tools/toolset.js"; +import type { ToolDef } from "../tools/types.js"; import type { Session } from "./session.js"; describe("shouldAutoCompact", () => { @@ -15,3 +19,187 @@ describe("shouldAutoCompact", () => { expect(shouldAutoCompact(session)).toBe(false); }); }); + +describe("runTurn / max iterations", () => { + it("throws MaxIterationsError (not a generic error) when a model keeps calling tools forever", async () => { + // A no-op tool the fake model calls on every single turn, forever — simulates a model that + // never converges on a final text answer within the turn's iteration budget. + const noopTool: ToolDef = { + name: "noop", + description: "does nothing", + schema: z.object({}), + mutating: false, + handler: async () => ({ ok: true }), + }; + const toolset = buildToolSet([noopTool]); + + function makeChunk(index: number) { + return { + choices: [ + { + delta: { tool_calls: [{ index: 0, id: `call_${index}`, function: { name: "noop", arguments: "{}" } }] }, + finish_reason: "tool_calls", + }, + ], + }; + } + + const fakeClient = { + chat: { + completions: { + create: vi.fn(async () => ({ + [Symbol.asyncIterator]: (() => { + let yielded = false; + return () => ({ + next: async () => { + if (yielded) return { done: true, value: undefined }; + yielded = true; + return { done: false, value: makeChunk(0) }; + }, + }); + })(), + })), + }, + }, + } as any; + + const session = createSession(fakeClient, "test-model", process.cwd(), async () => "once", "native", []); + session.toolset = toolset; + session.maxIterations = 5; + + await expect(runTurn(session, "keep going forever", () => {})).rejects.toThrow(MaxIterationsError); + await expect(runTurn(session, "keep going forever", () => {})).rejects.toThrow(/Paused after 5 steps/); + }); + + it("auto-compacts mid-turn instead of only checking between turns", async () => { + // A tool-heavy turn (read a bunch of files, say) that never gives runTurn a chance to return to + // the caller — App.tsx's shouldAutoCompact check only runs *between* turns, so without a + // mid-turn check this scenario previously had no compaction safety net at all. + const readTool: ToolDef = { + name: "read", + description: "reads a file", + schema: z.object({}), + mutating: false, + handler: async () => ({ content: "x".repeat(1000) }), + }; + const toolset = buildToolSet([readTool]); + + function toolCallChunk(index: number) { + return { + choices: [ + { + delta: { tool_calls: [{ index: 0, id: `call_${index}`, function: { name: "read", arguments: "{}" } }] }, + finish_reason: "tool_calls", + }, + ], + }; + } + function finalTextChunk() { + return { choices: [{ delta: { content: "done" }, finish_reason: "stop" }] }; + } + + let streamingCallCount = 0; + let compactionCalls = 0; + + const fakeClient = { + chat: { + completions: { + create: vi.fn(async (params: any) => { + if (params.stream === false) { + // The compactSession() request. + compactionCalls++; + return { choices: [{ message: { content: "a short summary" } }], usage: { prompt_tokens: 10, completion_tokens: 5 } }; + } + streamingCallCount++; + // First streaming call makes a tool call (to run up the fake context usage); second + // (after compaction should have kicked in) returns final text. + const chunk = streamingCallCount === 1 ? toolCallChunk(0) : finalTextChunk(); + let yielded = false; + return { + [Symbol.asyncIterator]: () => ({ + next: async () => { + if (yielded) return { done: true, value: undefined }; + yielded = true; + return { done: false, value: chunk }; + }, + }), + }; + }), + }, + }, + } as any; + + const session = createSession(fakeClient, "test-model", process.cwd(), async () => "once", "native", []); + session.toolset = toolset; + session.maxIterations = 10; + // Force shouldAutoCompact to be true on every check so the mid-turn path is exercised + // deterministically, without depending on real token estimation. + session.contextWindow = 100; + session.lastContextTokens = 1000; + session.autoCompactThreshold = 0.5; + + const notices: string[] = []; + const result = await runTurn(session, "read a bunch of files", (event) => { + if (event.type === "notice") notices.push(event.text); + }); + + expect(result).toBe("done"); + expect(compactionCalls).toBeGreaterThan(0); + expect(notices.some((n) => n.includes("auto-compacted mid-turn"))).toBe(true); + // mutationCommitLength must be re-anchored to the post-compaction array, never left pointing + // past the end of it (which would corrupt a later rollback via `messages.length = commitLength`). + expect(session.mutationCommitLength).not.toBeNull(); + expect(session.mutationCommitLength!).toBeLessThanOrEqual(session.messages.length); + }); + + it("times out a stream stuck sending only empty heartbeat chunks forever", async () => { + // Some backends/proxies keep a stalled generation's connection alive with periodic empty chunks + // (no content, no tool_calls, no finish_reason). If the idle guard poked on every yielded chunk + // regardless of content, these would reset the timer forever and the request would hang + // indefinitely with zero output — exactly what was reported. Use a short real timeout so the + // test both runs fast and proves real wall-clock behavior (not fake-timer bookkeeping). + const originalEnv = process.env.LOCODE_REQUEST_TIMEOUT_MS; + process.env.LOCODE_REQUEST_TIMEOUT_MS = "10000"; // the resolver's own 10s floor + + try { + const fakeClient = { + chat: { + completions: { + create: vi.fn(async (_params: any, options: any) => { + const signal: AbortSignal | undefined = options?.signal; + return { + [Symbol.asyncIterator]: () => ({ + next: async () => { + // A real aborted fetch stream ends/errors once its controller aborts — mimic that + // instead of yielding forever, so this test actually depends on the guard firing. + if (signal?.aborted) { + const err = new Error("The operation was aborted."); + err.name = "AbortError"; + throw err; + } + await new Promise((resolve) => setTimeout(resolve, 300)); + return { done: false, value: { choices: [{ delta: {}, index: 0 }] } }; // heartbeat, no content + }, + }), + }; + }), + }, + }, + } as any; + + const session = createSession(fakeClient, "test-model", process.cwd(), async () => "once", "native", []); + + const start = Date.now(); + await expect(runTurn(session, "hang forever behind heartbeats", () => {})).rejects.toThrow( + /Backend stopped responding mid-stream/, + ); + const elapsed = Date.now() - start; + // Should trip at ~10s despite the stream having "yielded" the whole time — well under a minute, + // which is the failure mode this guards against (the report was a 30-minute silent hang). + expect(elapsed).toBeLessThan(20_000); + } finally { + if (originalEnv === undefined) delete process.env.LOCODE_REQUEST_TIMEOUT_MS; + else process.env.LOCODE_REQUEST_TIMEOUT_MS = originalEnv; + } + }, 25_000); +}); diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 17849b9..5881deb 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -16,6 +16,7 @@ import { resolveToolInvocation, runTool, type ResolvedToolCall } from "../toolca import { formatCallLabel, summarizeToolResult } from "../ui/toolSummary.js"; import { estimateTokens } from "../utils/tokens.js"; import { runHooksForEvent } from "../hooks/runner.js"; +import { resolveRequestTimeoutMs } from "../config/config.js"; import { buildSystemPrompt } from "./systemPrompt.js"; import type { Session } from "./session.js"; @@ -77,6 +78,48 @@ export async function fireUserPromptSubmitHook(session: Session, prompt: string) export class AgentError extends Error {} +/** Thrown when a turn hits session.maxIterations without producing a final answer. Distinct from + * AgentError so the UI can render it as a soft pause rather than a failure — unlike a genuine + * backend/tool error, nothing actually went wrong: every tool call up to this point (including any + * file edits) already succeeded and is preserved (see noteMutationCommit/mutationCommitLength), the + * model just ran out of budget mid-task. Sending another message resumes from where it left off. */ +export class MaxIterationsError extends AgentError {} + +/** The backend client's own `timeout` option only bounds time-to-first-byte: once a streaming + * response's headers arrive, `fetchWithTimeout` clears its abort timer, so a backend that accepts + * the connection and then goes silent mid-stream (no more chunks, no close) hangs the request + * forever with no error surfaced anywhere. This builds a signal that also aborts if `poke()` isn't + * called again within `idleMs` — call it on every received chunk (or once for a non-streaming + * response) to keep the request alive. Composes with an optional `externalSignal` (e.g. a + * sub-agent's hard wall-clock timeout) so aborting either one aborts the request. */ +export function createIdleAbort( + idleMs: number, + externalSignal?: AbortSignal, +): { signal: AbortSignal; poke: () => void; dispose: () => void; didTimeOut: () => boolean } { + const controller = new AbortController(); + let timedOut = false; + let timer: ReturnType; + const arm = () => { + clearTimeout(timer); + timer = setTimeout(() => { + timedOut = true; + controller.abort(); + }, idleMs); + }; + arm(); + const onExternalAbort = () => controller.abort(); + externalSignal?.addEventListener("abort", onExternalAbort, { once: true }); + return { + signal: controller.signal, + poke: arm, + dispose: () => { + clearTimeout(timer); + externalSignal?.removeEventListener("abort", onExternalAbort); + }, + didTimeOut: () => timedOut, + }; +} + const MAX_MALFORMED_RETRIES = 2; // Sub-agent safety limits. The toolset already excludes `agent` for sub-agents (so a model can't @@ -132,12 +175,28 @@ export async function compactSession(session: Session): Promise { ]; const requestStart = Date.now(); - const res = await session.client.chat.completions.create({ - model: session.model, - messages: requestMessages, - stream: false, - max_tokens: 1024, - }); + const compactGuard = createIdleAbort(resolveRequestTimeoutMs()); + let res; + try { + res = await session.client.chat.completions.create( + { + model: session.model, + messages: requestMessages, + stream: false, + max_tokens: 1024, + }, + { signal: compactGuard.signal }, + ); + } catch (err) { + if (compactGuard.didTimeOut()) { + throw new AgentError( + `Compaction failed: backend stopped responding (no data for ${Math.round(resolveRequestTimeoutMs() / 1000)}s).`, + ); + } + throw err; + } finally { + compactGuard.dispose(); + } session.stats.modelTimeMs += Date.now() - requestStart; recordUsage(session, res.usage); const summary = res.choices[0]?.message?.content; @@ -247,6 +306,19 @@ function pushPendingImages(session: Session, images: ImageAttachment[]): void { } } +/** After a mutating tool has run successfully and its result message has been pushed onto + * session.messages, record the current history length as a "commit point" so the turn's error + * rollback (App.tsx submitTurn) doesn't erase the record of a mutation that already hit disk. + * Without this, a later backend failure in the same turn would truncate history back to before the + * turn, leaving the model blind to its own filesystem change and causing it to blindly retry the + * edit (which then fails with "old_string not found", etc.). */ +function noteMutationCommit(session: Session, resolved: ResolvedToolCall, result: unknown): void { + if ("error" in resolved) return; + if (!resolved.tool.mutating) return; + if (result && typeof result === "object" && "error" in (result as object)) return; + session.mutationCommitLength = session.messages.length; +} + async function gateAndRun( resolved: ResolvedToolCall, rawLabel: string, @@ -385,6 +457,7 @@ async function runSubAgentTurn(parent: Session, task: SubAgentTask, overrides?: // Independent from the parent's — a sub-agent's own bash calls aren't backgroundable via the // parent UI's Ctrl+B since sub-agent tool calls aren't shown mid-flight anyway (see class doc above). activeBackground: null, + mutationCommitLength: null, }; // Run (and honor) the SubagentStart hook before arming the timeout, so a slow hook doesn't eat @@ -456,6 +529,7 @@ async function handleCompletedMessage( const label = call.type === "function" ? `${call.function.name}(${call.function.arguments})` : call.type; const result = await gateAndRun(resolved, label, session, emit); const image = pushToolResultMessage(session, "native", call.id, call.type === "function" ? call.function.name : call.type, result); + noteMutationCommit(session, resolved, result); if (image) pendingImages.push(image); } pushPendingImages(session, pendingImages); @@ -476,6 +550,7 @@ async function handleCompletedMessage( const label = `${call.name}(${JSON.stringify(call.arguments)})`; const result = await gateAndRun(resolved, label, session, emit); pushToolResultMessage(session, "fallback", "", call.name, result); + noteMutationCommit(session, resolved, result); } return { text: "", hadToolCalls: true }; } @@ -500,9 +575,34 @@ export async function runTurn( ): Promise { session.messages.push({ role: "user", content: userInput } as ChatCompletionMessageParam); if (session.subAgentDepth === 0) session.stats.turns++; + session.mutationCommitLength = null; let malformedRetries = 0; for (let i = 0; i < session.maxIterations; i++) { + // A single turn that makes many tool calls in a row (e.g. reading dozens of files) can blow + // past the context window entirely within one runTurn call — the caller (App.tsx) only checks + // shouldAutoCompact *between* turns, so without this a long tool-heavy turn had no compaction + // safety net at all, and would keep sending an ever-growing prompt until the backend choked on + // it or hung trying to process it. Check on every iteration, including the first, since a prior + // turn's post-turn compaction may not have run (e.g. if it errored). + if (shouldAutoCompact(session)) { + try { + await compactSession(session); + // compactSession replaces session.messages wholesale, so any previously-recorded + // mutationCommitLength now indexes into an array that no longer exists — a later error in + // this same turn would roll back to a stale, out-of-bounds length (App.tsx's + // `session.messages.length = commitLength ?? rollbackLength`), padding the array with empty + // slots instead of truncating it. The compacted history is itself always a safe rollback + // floor (nothing after it has happened yet), so re-anchor to its current length. + session.mutationCommitLength = session.messages.length; + emit({ type: "notice", text: "Context was getting full — auto-compacted mid-turn.", isError: false }); + } catch { + // Best-effort: if compaction itself fails, proceed with the oversized context rather than + // aborting the whole turn — the idle-abort guard on the next request still protects against + // an outright hang, it'll just be a slower/costlier request. + } + } + // --- Streaming path --- let fullText = ""; let finishReason: string | null = null; @@ -511,6 +611,7 @@ export async function runTurn( let usage: CompletionUsage | undefined; const requestStart = Date.now(); + const idleGuard = createIdleAbort(resolveRequestTimeoutMs(), signal); try { const stream = await session.client.chat.completions.create( { @@ -521,7 +622,7 @@ export async function runTurn( stream_options: { include_usage: true }, max_tokens: 4096, }, - signal ? { signal } : undefined, + { signal: idleGuard.signal }, ); for await (const chunk of stream) { @@ -530,6 +631,16 @@ export async function runTurn( if (chunk.usage) usage = chunk.usage; const choice = chunk.choices[0]; + // Some backends/proxies send periodic empty "heartbeat" chunks (no delta content, no + // tool_calls, no finish_reason) to keep a long-lived connection alive through intermediaries + // during slow generation. Poking the idle guard unconditionally on every yielded chunk would + // let those reset the timer forever, defeating it entirely — a generation truly stuck for + // 30+ minutes would never trip if the transport keeps trickling empty chunks the whole time. + // Only chunks carrying real progress (usage, text, tool-call deltas, or a finish reason) + // count as the backend being alive and working. + const isMeaningfulChunk = !!chunk.usage || !!choice?.delta?.content || !!choice?.delta?.tool_calls || !!choice?.finish_reason; + if (isMeaningfulChunk) idleGuard.poke(); + if (!choice) continue; const delta = choice.delta; @@ -567,7 +678,23 @@ export async function runTurn( if (fullText) { emit({ type: "text_done", fullText }); } + if (idleGuard.didTimeOut()) { + throw new AgentError( + `Backend stopped responding mid-stream (no data for ${Math.round(resolveRequestTimeoutMs() / 1000)}s) — connection aborted. The backend may have crashed or hung; try again.`, + ); + } throw annotateIfImageRelated(streamErr, session.messages); + } finally { + idleGuard.dispose(); + } + + // An idle-triggered abort can also surface as a clean (chunk-less) end of the async iterator + // instead of a thrown error, depending on how far into the stream it landed — check unconditionally + // rather than only in the catch above, or it'd silently fall through to "Empty response from model." + if (idleGuard.didTimeOut()) { + throw new AgentError( + `Backend stopped responding mid-stream (no data for ${Math.round(resolveRequestTimeoutMs() / 1000)}s) — connection aborted. The backend may have crashed or hung; try again.`, + ); } session.stats.modelTimeMs += Date.now() - requestStart; @@ -595,16 +722,29 @@ export async function runTurn( emit({ type: "stream_discard" }); // Retry non-streaming for this turn const retryStart = Date.now(); - const res = await session.client.chat.completions.create( - { - model: session.model, - messages: session.messages, - tools: toolset.openaiTools, - stream: false, - max_tokens: 4096, - }, - signal ? { signal } : undefined, - ); + const retryGuard = createIdleAbort(resolveRequestTimeoutMs(), signal); + let res; + try { + res = await session.client.chat.completions.create( + { + model: session.model, + messages: session.messages, + tools: toolset.openaiTools, + stream: false, + max_tokens: 4096, + }, + { signal: retryGuard.signal }, + ); + } catch (retryErr) { + if (retryGuard.didTimeOut()) { + throw new AgentError( + `Backend stopped responding (no data for ${Math.round(resolveRequestTimeoutMs() / 1000)}s) — connection aborted. The backend may have crashed or hung; try again.`, + ); + } + throw retryErr; + } finally { + retryGuard.dispose(); + } session.stats.modelTimeMs += Date.now() - retryStart; recordUsage(session, res.usage); const message = res.choices[0]?.message; @@ -643,6 +783,7 @@ export async function runTurn( const label = `${tc.name}(${tc.arguments})`; const result = await gateAndRun(resolved, label, session, emit); const image = pushToolResultMessage(session, "native", tc.id, tc.name, result); + noteMutationCommit(session, resolved, result); if (image) pendingImages.push(image); } pushPendingImages(session, pendingImages); @@ -663,6 +804,7 @@ export async function runTurn( const label = `${call.name}(${JSON.stringify(call.arguments)})`; const result = await gateAndRun(resolved, label, session, emit); pushToolResultMessage(session, "fallback", "", call.name, result); + noteMutationCommit(session, resolved, result); } continue; } @@ -687,5 +829,7 @@ export async function runTurn( throw new AgentError("Empty response from model."); } - throw new AgentError("Max tool-call iterations reached without a final answer."); + throw new MaxIterationsError( + `Paused after ${session.maxIterations} steps in this turn. Everything done so far (including any file edits) is saved — send another message to continue.`, + ); } \ No newline at end of file diff --git a/src/agent/session.ts b/src/agent/session.ts index d56d9fa..fadbe56 100644 --- a/src/agent/session.ts +++ b/src/agent/session.ts @@ -70,6 +70,15 @@ export interface Session { * `bash`) is in flight; Ctrl+B in the UI flips `.requested` to detach it. Null the rest of the * time, including while non-backgroundable tools run. */ activeBackground: { requested: boolean } | null; + /** Message-array length recorded after the most recent successfully-applied mutating tool result + * (or mid-turn auto-compaction) in the current turn, or null until one has happened. Used by the + * turn's error-rollback path (App.tsx submitTurn) so a later backend failure doesn't erase the + * history of a mutating tool that already changed the filesystem — erasing it would leave the + * model blind to its own change and cause it to blindly retry the edit (which then fails with + * "old_string not found"). Also re-anchored after a mid-turn compactSession call (runTurn), since + * that replaces session.messages wholesale and any earlier index would otherwise point past the + * end of the new, shorter array. Reset to null at the start of each turn in runTurn. */ + mutationCommitLength: number | null; } export function createSession( @@ -106,6 +115,7 @@ export function createSession( autoCompactThreshold, stats: initialStats(), activeBackground: null, + mutationCommitLength: null, }; } @@ -147,6 +157,7 @@ export function createSessionFromRecord( autoCompactThreshold, stats: initialStats(), activeBackground: null, + mutationCommitLength: null, }; } diff --git a/src/cli.ts b/src/cli.ts index 60c7789..17427e5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -24,7 +24,7 @@ function addBackendOptions(cmd: Command): Command { program .name("locode") .description("Agentic coding CLI for local models via Ollama and LM Studio") - .version("0.2.0"); + .version("0.3.1"); addBackendOptions(program) .option("-m, --model ", "model name as known to the backend") diff --git a/src/config/defaults.ts b/src/config/defaults.ts index a23802d..73eb7c4 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -12,9 +12,11 @@ export type BackendName = keyof typeof KNOWN_BACKENDS; * and the user hasn't configured one — a conservative size common among smaller local models. */ export const DEFAULT_CONTEXT_WINDOW = 8192; -/** Max tool calls per turn before locode gives up rather than looping forever. 25 gives real - * multi-file tasks room to breathe; still bounded so a genuinely stuck model fails fast. */ -export const DEFAULT_MAX_ITERATIONS = 25; +/** Max tool calls per turn before locode gives up rather than looping forever. 50 gives real + * multi-file tasks room to breathe (local models often issue one tool call per turn, so a + * multi-file edit + verify sequence can easily run past 25); still bounded so a genuinely stuck + * model fails fast, and hitting the cap is a soft pause, not a failure (see MaxIterationsError). */ +export const DEFAULT_MAX_ITERATIONS = 50; /** Fraction of the context window at which locode automatically summarizes the conversation. * User-configurable via `locode config set autoCompactThreshold`. */ diff --git a/src/hooks/runner.ts b/src/hooks/runner.ts index 116f465..8b0ba3f 100644 --- a/src/hooks/runner.ts +++ b/src/hooks/runner.ts @@ -1,6 +1,7 @@ import { execa } from "execa"; import { loadMergedHooks } from "./config.js"; import { matcherMatches } from "./matcher.js"; +import { resolveShell } from "../utils/shell.js"; import type { Hook, HookCommand, HookEventName, HookHttp } from "./types.js"; const DEFAULT_TIMEOUT_SECONDS = 30; @@ -55,7 +56,7 @@ async function runCommandHook( ): Promise<{ exitCode: number; stdout: string; stderr: string; timedOut: boolean; json?: unknown; warning?: string }> { try { const result = await execa(hook.command, { - shell: true, + shell: resolveShell(), cwd: ctx.cwd, input: stdinPayload, timeout: (hook.timeout ?? DEFAULT_TIMEOUT_SECONDS) * 1000, diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 97d11b3..a7e155e 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -68,7 +68,7 @@ export async function connectMcpServer(name: string, config: McpServerConfig): P stderr: "pipe", }); - const client = new Client({ name: "locode", version: "0.2.0" }); + const client = new Client({ name: "locode", version: "0.3.1" }); await client.connect(transport); const { tools } = await client.listTools(); return { name, client, transport, tools: tools as McpToolInfo[] }; diff --git a/src/tools/bash.ts b/src/tools/bash.ts index 468de0c..5c92ee4 100644 --- a/src/tools/bash.ts +++ b/src/tools/bash.ts @@ -3,6 +3,7 @@ import { execa } from "execa"; import { z } from "zod"; import { registerBackgroundJob } from "./backgroundJobs.js"; import { truncate } from "../utils/truncate.js"; +import { resolveShell } from "../utils/shell.js"; import type { ToolDef } from "./types.js"; const schema = z.object({ @@ -29,7 +30,7 @@ export const bashTool: ToolDef> = { // backgrounding via Ctrl+B can cancel it below — execa's own timeout kills the process on a // fixed schedule regardless of what happens to it afterward, which would silently kill a // long-running command right after the user chose to keep it running in the background. - const child = execa(command, { shell: true, cwd: workDir, reject: false }); + const child = execa(command, { shell: resolveShell(), cwd: workDir, reject: false }); let stdout = ""; let stderr = ""; const onStdout = (d: Buffer) => { diff --git a/src/tools/listFiles.ts b/src/tools/listFiles.ts index 22936f7..43b2a9f 100644 --- a/src/tools/listFiles.ts +++ b/src/tools/listFiles.ts @@ -17,7 +17,17 @@ export const listFilesTool: ToolDef> = { mutating: false, handler: async ({ pattern, cwd }, ctx) => { const base = cwd ? path.resolve(ctx.cwd, cwd) : ctx.cwd; - const matches = await fg(pattern, { cwd: base, dot: false, onlyFiles: true, absolute: false }); + const matches = await fg(pattern, { + cwd: base, + dot: false, + onlyFiles: true, + absolute: false, + // node_modules and .git are pure noise for an agent exploring a project and are + // always excluded. Build output dirs (dist/, build/, out/) are NOT excluded + // here — the agent may legitimately need to inspect compiled output (e.g. to + // verify a build), and hiding them would silently make those files invisible. + ignore: ["**/node_modules/**", "**/.git/**"], + }); return { matches: matches.slice(0, MAX_MATCHES), truncated: matches.length > MAX_MATCHES }; }, }; diff --git a/src/ui/ink/App.tsx b/src/ui/ink/App.tsx index dd33b2b..46facd4 100644 --- a/src/ui/ink/App.tsx +++ b/src/ui/ink/App.tsx @@ -6,6 +6,7 @@ import { contextUsageRatio, fireSessionStartHook, fireUserPromptSubmitHook, + MaxIterationsError, runTurn, shouldAutoCompact, type ChatCompletionUserContent, @@ -57,6 +58,9 @@ import { StatusBar } from "./StatusBar.js"; import { ThinkingIndicator } from "./ThinkingIndicator.js"; import { nextId, type HistoryItem, type NewHistoryItem } from "./types.js"; +// Cap for the input-history ring buffer used for ↑/↓ recall in the chat input. +const MAX_HISTORY = 100; + export interface AppProps { baseURL: string; cwd: string; @@ -101,6 +105,7 @@ export function App({ const [modelList, setModelList] = useState([]); const [sessionList, setSessionList] = useState([]); const [gitInfo, setGitInfo] = useState(null); + const [history, setHistory] = useState([]); const baseURLRef = useRef(initialBaseURL); const sessionRef = useRef(null); @@ -443,7 +448,11 @@ export function App({ } setStreamingText(null); streamingAccumulatorRef.current = ""; - setStaticItems((prev) => [...prev, { id: nextId(), kind: "assistant", text: event.fullText } as HistoryItem]); + // A native tool-call-only turn (no preceding prose) still emits text_done with an empty + // fullText purely to flush streaming state above — skip adding a blank assistant line for it. + if (event.fullText.trim().length > 0) { + setStaticItems((prev) => [...prev, { id: nextId(), kind: "assistant", text: event.fullText } as HistoryItem]); + } } else if (event.type === "stream_discard") { // The turn is being retried non-streaming after a partially-streamed malformed tool call — // drop the partial text without committing it as an assistant message (it was never added to @@ -455,6 +464,10 @@ export function App({ setStreamingText(null); streamingAccumulatorRef.current = ""; } else if (event.type === "tool_call") { + if (streamRafRef.current !== null) { + clearTimeout(streamRafRef.current); + streamRafRef.current = null; + } setStreamingText(null); setIsThinking(true); setRunningToolIsBash(event.label.startsWith("Bash(")); @@ -462,7 +475,7 @@ export function App({ } else if (event.type === "tool_result") { setRunningToolIsBash(false); setStaticItems((prev) => [...prev, { id: nextId(), kind: "tool_result", summary: event.summary, isError: event.isError } as HistoryItem]); - } else if (event.type === "hook_notice") { + } else if (event.type === "hook_notice" || event.type === "notice") { setStaticItems((prev) => [...prev, { id: nextId(), kind: "notice", text: event.text, isError: event.isError } as HistoryItem]); } }); @@ -496,9 +509,21 @@ export function App({ } setStreamingText(null); streamingAccumulatorRef.current = ""; - session.messages.length = rollbackLength; - const reason = err instanceof AgentError ? err.message : (err as Error).message; - push({ kind: "notice", text: `Request failed: ${reason}`, isError: true }); + // Roll back the uncommitted tail of the turn — but preserve history through the last + // successfully-applied mutating tool result (session.mutationCommitLength), so a backend + // failure that comes *after* a mutation already hit disk doesn't erase the record of that + // mutation. Erasing it would leave the model blind to its own change and cause it to blindly + // retry the edit on the next turn (which then fails with "old_string not found", etc.). + const commitLength = session.mutationCommitLength; + session.messages.length = commitLength ?? rollbackLength; + if (err instanceof MaxIterationsError) { + // Not a failure — the model just ran out of per-turn budget. No "Request failed" framing, + // no isError styling, since nothing actually broke and the work done so far is intact. + push({ kind: "notice", text: err.message, isError: false }); + } else { + const reason = err instanceof AgentError ? err.message : (err as Error).message; + push({ kind: "notice", text: `Request failed: ${reason}`, isError: true }); + } } finally { setIsThinking(false); setStreamingText(null); @@ -514,6 +539,15 @@ export function App({ const session = sessionRef.current; if (!session) return; + // Record into the input history (most-recent-last) for ↑/↓ recall. Skip exact + // consecutive duplicates and cap the length so a long session doesn't grow + // the array unbounded — same UX convention as a typical shell. + setHistory((h) => { + if (h.length > 0 && h[h.length - 1] === trimmed) return h; + const next = [...h, trimmed]; + return next.length > MAX_HISTORY ? next.slice(next.length - MAX_HISTORY) : next; + }); + push({ kind: "user", text: trimmed }); if (trimmed === "/exit" || trimmed === "/quit") { @@ -807,7 +841,7 @@ export function App({ } return ( - + {(item) => } @@ -839,23 +873,25 @@ export function App({ ) : phase === "model-select" ? ( ) : ( - - )} - {sessionRef.current && phase === "input" && ( - + + {sessionRef.current && phase === "input" && ( + + )} + + )} ); diff --git a/src/ui/ink/ChatInput.tsx b/src/ui/ink/ChatInput.tsx index 63c096f..a0033ad 100644 --- a/src/ui/ink/ChatInput.tsx +++ b/src/ui/ink/ChatInput.tsx @@ -11,14 +11,17 @@ interface Props { onSubmit: (value: string) => void; onCyclePermMode?: () => void; cwd: string; + history?: string[]; } const MAX_MATCHES = 50; const VISIBLE_SUGGESTIONS = 8; -export function ChatInput({ value, onChange, onSubmit, onCyclePermMode, cwd }: Props) { +export function ChatInput({ value, onChange, onSubmit, onCyclePermMode, cwd, history = [] }: Props) { const [allFiles, setAllFiles] = useState(null); const [selectedIndex, setSelectedIndex] = useState(0); + const [historyIndex, setHistoryIndex] = useState(-1); + const [tempValue, setTempValue] = useState(""); // ink-text-input tracks its cursor position internally and has no way to be told the value // changed externally — without this, accepting a suggestion leaves the cursor at its old // (now-wrong) offset, so further typing lands mid-string instead of at the end. Bumping this @@ -96,13 +99,38 @@ export function ChatInput({ value, onChange, onSubmit, onCyclePermMode, cwd }: P cancelMention(); return; } - if (matches.length === 0) return; - if (key.downArrow) { - setSelectedIndex((i) => Math.min(i + 1, matches.length - 1)); - } else if (key.upArrow) { - setSelectedIndex((i) => Math.max(i - 1, 0)); - } else if (key.tab) { - acceptSuggestion(matches[selectedIndex] ?? matches[0]!); + if (matches.length > 0) { + if (key.downArrow) { + setSelectedIndex((i) => Math.min(i + 1, matches.length - 1)); + return; + } else if (key.upArrow) { + setSelectedIndex((i) => Math.max(i - 1, 0)); + return; + } else if (key.tab) { + acceptSuggestion(matches[selectedIndex] ?? matches[0]!); + return; + } + } else { + // History navigation + if (key.upArrow) { + if (historyIndex < history.length - 1) { + const newIndex = historyIndex + 1; + if (historyIndex === -1) setTempValue(value); + setHistoryIndex(newIndex); + onChange(history[history.length - 1 - newIndex]!); + setInputKey((k) => k + 1); + } + return; + } else if (key.downArrow) { + if (historyIndex > -1) { + const newIndex = historyIndex - 1; + setHistoryIndex(newIndex); + const newValue = newIndex === -1 ? tempValue : history[history.length - 1 - newIndex]!; + onChange(newValue); + setInputKey((k) => k + 1); + } + return; + } } }, ); @@ -113,6 +141,8 @@ export function ChatInput({ value, onChange, onSubmit, onCyclePermMode, cwd }: P acceptSuggestion(matches[selectedIndex] ?? matches[0]!); return; } + setHistoryIndex(-1); + setTempValue(""); onSubmit(raw); } diff --git a/src/ui/ink/index.tsx b/src/ui/ink/index.tsx index f831afd..d24aebd 100644 --- a/src/ui/ink/index.tsx +++ b/src/ui/ink/index.tsx @@ -43,6 +43,12 @@ export async function runInkApp(opts: RunInkAppOptions): Promise { // Start the UI immediately — no blocking network calls before rendering. // Model listing happens inside the App component so the user sees the UI right away. + // + // NOTE: we deliberately do NOT enter the alternate screen buffer (\x1b[?1049h) here. + // Alt-screen has no scrollback, so the user couldn't scroll up through history with + // the mouse wheel / Shift+PgUp. The dashboard-above-input layout in App.tsx already + // fixes the original "prompt starts below the status bar" glitch without sacrificing + // the terminal's native scrollback — keep the main screen. const instance = render( ({ existsSync: vi.fn() })); + +import { existsSync } from "node:fs"; +import { _resetShellCache, resolveShell } from "./shell.js"; + +const mockedExistsSync = vi.mocked(existsSync); + +describe("resolveShell", () => { + const originalPlatform = process.platform; + const originalPath = process.env.PATH; + + beforeEach(() => { + _resetShellCache(); + mockedExistsSync.mockReset(); + }); + + afterEach(() => { + Object.defineProperty(process, "platform", { value: originalPlatform }); + process.env.PATH = originalPath; + }); + + it("returns true (default shell) on non-Windows platforms", () => { + Object.defineProperty(process, "platform", { value: "linux" }); + expect(resolveShell()).toBe(true); + expect(mockedExistsSync).not.toHaveBeenCalled(); + }); + + it("finds Git Bash via a git.exe directory on PATH", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env.PATH = ["C:\\Program Files\\Git\\cmd", "C:\\Windows\\System32"].join(path.delimiter); + mockedExistsSync.mockImplementation((p) => p === "C:\\Program Files\\Git\\bin\\bash.exe"); + + expect(resolveShell()).toBe("C:\\Program Files\\Git\\bin\\bash.exe"); + }); + + it("falls back to well-known install locations when PATH has no git dir", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env.PATH = "C:\\Windows\\System32"; + mockedExistsSync.mockImplementation((p) => p === "C:\\Program Files\\Git\\bin\\bash.exe"); + + expect(resolveShell()).toBe("C:\\Program Files\\Git\\bin\\bash.exe"); + }); + + it("falls back to true (cmd.exe) when Git Bash can't be found anywhere", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env.PATH = "C:\\Windows\\System32"; + mockedExistsSync.mockReturnValue(false); + + expect(resolveShell()).toBe(true); + }); + + it("memoizes the result across calls", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env.PATH = "C:\\Windows\\System32"; + mockedExistsSync.mockReturnValue(false); + + resolveShell(); + resolveShell(); + expect(mockedExistsSync).toHaveBeenCalledTimes(WINDOWS_GIT_BASH_CANDIDATE_COUNT); + }); +}); + +const WINDOWS_GIT_BASH_CANDIDATE_COUNT = 2; diff --git a/src/utils/shell.ts b/src/utils/shell.ts new file mode 100644 index 0000000..f768f48 --- /dev/null +++ b/src/utils/shell.ts @@ -0,0 +1,48 @@ +import { existsSync } from "node:fs"; +import path from "node:path"; + +/** Candidate Git-for-Windows install locations, checked in order. Git Bash ships `bash.exe` plus a + * real POSIX `usr/bin` (cat, sed, grep, rm, ...) — without it, execa's `shell: true` falls back to + * cmd.exe, which has none of those and returns exit 255 on the first unresolved command in a pipe. */ +const WINDOWS_GIT_BASH_CANDIDATES = [ + "C:\\Program Files\\Git\\bin\\bash.exe", + "C:\\Program Files (x86)\\Git\\bin\\bash.exe", +]; + +let cachedShell: string | true | undefined; + +/** Resolves the shell execa should use for `shell: true`-style commands. On Windows, prefers Git + * Bash (via PATH's `git.exe` location, then well-known install dirs) so LLM-generated Unix + * commands (cat, sed, grep, rm -rf, &&, ...) behave as the model expects instead of silently + * failing under cmd.exe. Falls back to `true` (the platform default shell) everywhere else. */ +export function resolveShell(): string | true { + if (cachedShell !== undefined) return cachedShell; + cachedShell = findShell(); + return cachedShell; +} + +function findShell(): string | true { + if (process.platform !== "win32") return true; + + const pathVar = process.env.PATH ?? process.env.Path ?? ""; + for (const dir of pathVar.split(path.delimiter)) { + if (!dir) continue; + // git.exe on PATH lives in \cmd or \bin; Git Bash's own bash.exe is at \bin. + const root = path.basename(dir).toLowerCase() === "cmd" || path.basename(dir).toLowerCase() === "bin" ? path.dirname(dir) : null; + if (root) { + const candidate = path.join(root, "bin", "bash.exe"); + if (existsSync(candidate)) return candidate; + } + } + + for (const candidate of WINDOWS_GIT_BASH_CANDIDATES) { + if (existsSync(candidate)) return candidate; + } + + return true; +} + +/** Test-only: clears the memoized shell resolution. */ +export function _resetShellCache(): void { + cachedShell = undefined; +} diff --git a/src/utils/writeFileAtomic.ts b/src/utils/writeFileAtomic.ts index b8bfbb6..77a66cf 100644 --- a/src/utils/writeFileAtomic.ts +++ b/src/utils/writeFileAtomic.ts @@ -1,13 +1,25 @@ -import { mkdirSync } from "node:fs"; +import { randomBytes } from "node:crypto"; +import { mkdirSync, unlinkSync } from "node:fs"; import { rename as fsRename, writeFile as fsWriteFile } from "node:fs/promises"; import path from "node:path"; /** Writes `content` to `file` atomically: write to a sibling temp file, then rename into place, so a * crash mid-write can never leave a truncated/corrupt file at `file` — a reader always sees either - * the old complete content or the new complete content, never a partial write. */ + * the old complete content or the new complete content, never a partial write. The temp file uses a + * random suffix (not a fixed `.tmp`) so concurrent writes to the same directory can't collide + * and overwrite each other's temp file mid-write. */ export async function writeFileAtomic(file: string, content: string): Promise { mkdirSync(path.dirname(file), { recursive: true }); - const tmp = `${file}.tmp`; - await fsWriteFile(tmp, content, "utf-8"); - await fsRename(tmp, file); + const tmp = `${file}.${randomBytes(6).toString("hex")}.tmp`; + try { + await fsWriteFile(tmp, content, "utf-8"); + await fsRename(tmp, file); + } catch (err) { + try { + unlinkSync(tmp); + } catch { + // The temp file may not exist if fsWriteFile failed; ignore. + } + throw err; + } }