diff --git a/README.md b/README.md index 518323e..b1b96b9 100644 --- a/README.md +++ b/README.md @@ -89,12 +89,15 @@ locode is a full-screen terminal app built with [Ink](https://github.com/vadimde - **Ctrl+B**: while a `bash` command is running, detaches it into the background and returns control to you immediately — the turn continues with a `bash_output`-checkable job id instead of waiting for the command to finish. A notice appears in the transcript once the backgrounded command actually completes. Only `bash` supports this today. The model can kill a still-running backgrounded job with `bash_kill`; any jobs still running when locode itself exits are killed too, so they don't outlive the process as orphans. - **Backends**: `--backend ollama` (default) or `--backend lmstudio`, or `--base-url ` for anything else that speaks the same API. -- **Tools**: `read_file`, `list_files`, `grep`, `web_search`, `web_fetch`, `git_status`, `bash_output` run automatically. `write_file`, `edit_file`, `bash`, `bash_kill`, and `git_commit` show a diff/preview in a bordered box and ask you to pick Yes / Yes-always-this-session / No with the arrow keys before running. +- **Tools**: `read_file`, `list_files`, `grep`, `web_search`, `web_fetch`, `git_status`, `bash_output`, `todo_write` run automatically. `write_file`, `edit_file`, `bash`, `bash_kill`, and `git_commit` show a diff/preview in a bordered box and ask you to pick Yes / Yes-always-this-session / No with the arrow keys before running. +- **Permission modes**: `default` (ask before every mutating tool), `plan` (research only — every mutating tool is blocked outright, no prompt; the model is expected to describe what it would do in its final answer instead), `auto-edit` (file edits auto-approved, `bash`/`git_commit` still ask), `auto-accept` (everything auto-approved — use with care). Cycle with `Shift+Tab` or set directly with `/perm `. +- **Task checklists**: for multi-step work the model can call `todo_write` to show a live checklist (`☐`/`◐`/`☑`) in the transcript instead of silently working through a list you can't see progress on. +- **Project instructions**: a `CLAUDE.md` (or `AGENTS.md`) file in the project root is automatically read at session start and folded into the system prompt — put repo-specific conventions there and every session picks them up without being told. - **Images**: `read_file` returns image files (png, jpg, jpeg, gif, webp, bmp — up to 5MB) as actual image content instead of trying to decode them as text, so vision-capable models can see them when the model itself calls the tool. To attach a file or image to your own message directly, use `/import [caption]`. - **`@` file mentions**: type `@` in the chat input to open a fuzzy file picker (searches the whole project, skipping `node_modules`/`.git`/`dist`) — keep typing to filter, `↑`/`↓` to navigate, `Tab` (or `Enter`) to insert the highlighted path. Any `@path` left in your message when you hit `Enter` for real is resolved against disk and attached to that message (text inlined, images attached as image content) — a stray `@` that isn't an actual file (e.g. an email address) is left as plain text. - **Git**: `git_status` covers read-only inspection (`status`, `diff`, `log`, `show`, `branches`) and runs automatically. `git_commit` covers `add`, `commit`, `create_branch`, `checkout`, and `push` — each shows the actual diff/status/commits it's about to affect before you confirm (e.g. a commit's preview is the staged diff plus the message, a push's preview is the list of commits it would send). - **Sub-agents**: the `agent` tool lets the model delegate a self-contained task to a fresh, isolated tool loop (same tools, minus `agent` itself — no nested sub-agents) and get back only the final answer, keeping the main conversation's context focused. It shows up as a single `⏺ Agent(description)` / `⎿ Sub-agent finished (...)` line — the sub-agent's own intermediate steps aren't displayed. Any mutating tool calls it makes still go through the same permission prompts as the main conversation. -- **MCP servers**: locode connects to any [MCP](https://modelcontextprotocol.io) servers configured via `locode mcp add` or a project's `.mcp.json` (stdio and remote/streamable-HTTP transports), and adds their tools to every session, namespaced as `mcp____`. A server's tool is treated as mutating (confirmation required) unless it declares itself read-only via the MCP `readOnlyHint` annotation. One misconfigured server doesn't block the others — check `/mcp` for per-server connection status. +- **MCP servers**: locode connects to any [MCP](https://modelcontextprotocol.io) servers configured via `locode mcp add` or a project's `.mcp.json` (stdio and remote/streamable-HTTP transports), and adds their tools to every session, namespaced as `mcp____`. Every MCP tool is treated as mutating (confirmation required on every call) regardless of what it reports — the MCP `readOnlyHint` annotation is advisory and could be wrong (or set by a malicious server specifically to skip confirmation), so locode never trusts it. One misconfigured server doesn't block the others — check `/mcp` for per-server connection status. - **Claude Code plugins**: `locode plugin add ` installs a Claude Code-compatible plugin — locode reads its `.claude-plugin/plugin.json`, then loads it all directly: MCP servers (its `.mcp.json` or manifest `mcpServers`, merged in like any other MCP server), slash commands (`commands/*.md` — frontmatter `description`/`argument-hint`, body is a template expanded with `$ARGUMENTS`/`$1..$9` and submitted as your message), agents (`agents/*.md` — the body becomes a sub-agent's system prompt, exposed as a callable tool named `agent____`; a `tools:` frontmatter list restricts what it can use, with Claude Code's built-in tool names — Read, Grep, Edit, etc. — automatically mapped to locode's equivalents), hooks (`hooks/hooks.json`, see below), and skills (`skills/*/SKILL.md`, see below). Check `/plugins` for what's loaded. - **Skills**: named instructions the model loads on demand rather than a hook or a sub-agent — every installed skill (`skills//SKILL.md`) is exposed through one shared `skill` tool, whose own description lists every skill's name and "use this when..." blurb so the model knows when to call it. You can also invoke one directly with `/ [request]`, which skips the model's own judgment and submits the skill's instructions (plus your request, if any) as the turn. Check `/skills` for what's installed. - **Hooks**: shell commands that fire on session lifecycle events — `SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, `PermissionRequest`, `SubagentStart`, `SubagentStop`, `CwdChanged`, `FileChanged`, `ConfigChange`, `Stop`, `SessionEnd`. Configured the same way MCP servers are — plugin-bundled (`hooks/hooks.json`), user-level (`locode hooks path`, hand-edited), and project-level (`.locode/hooks.json`) all merge together, every hook from every source runs. A hook receives a JSON payload on stdin (`session_id`, `cwd`, `hook_event_name`, plus event-specific fields like `prompt` or `tool_name`/`tool_input`); exit 0 allows (stdout becomes injected context for `SessionStart`/`UserPromptSubmit`), exit 2 blocks (stderr is the reason shown), anything else is a non-blocking warning. `PreToolUse`, `UserPromptSubmit`, and `PermissionRequest` can block; the rest are fire-and-forget. `PreToolUse` fires before permission modes apply, so a hook's block can't be bypassed by auto-accept. `command` and `http` hook types are supported (`prompt` is declared in the config format but not yet executed); command hooks can opt into structured JSON output via `outputSchema: "json"`. `SessionEnd` fires on every exit path (including Ctrl+C and external `SIGTERM`/`SIGHUP`). Check `/hooks` for what's configured. @@ -109,6 +112,7 @@ Note: even models with genuine native tool-calling support occasionally emit a t /model switch the model used for the current backend /backend switch backend (ollama | lmstudio), keeps current model /mode view or force tool-call mode (native | fallback) +/perm [mode] cycle or set permission mode (default | plan | auto-edit | auto-accept) /status show current model, backend, tool-call mode, and cwd /dashboard show session stats: token I/O, elapsed/model time, turns, tool calls /tools list available tools diff --git a/package-lock.json b/package-lock.json index a2506dd..fdd6ef2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,7 @@ "marked-terminal": "^7.3.0", "openai": "^6.45.0", "react": "^19.2.7", + "tree-kill": "^1.2.2", "zod": "^4.4.3" }, "bin": { @@ -4510,7 +4511,6 @@ "version": "1.2.2", "resolved": "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.2.tgz", "integrity": "sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A==", - "dev": true, "license": "MIT", "bin": { "tree-kill": "cli.js" diff --git a/package.json b/package.json index d385785..482650f 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "marked-terminal": "^7.3.0", "openai": "^6.45.0", "react": "^19.2.7", + "tree-kill": "^1.2.2", "zod": "^4.4.3" }, "devDependencies": { diff --git a/src/agent/events.ts b/src/agent/events.ts index a9ed845..2d194b0 100644 --- a/src/agent/events.ts +++ b/src/agent/events.ts @@ -1,3 +1,5 @@ +import type { TodoItem } from "../tools/types.js"; + export type AgentEvent = | { type: "text_delta"; delta: string } | { type: "text_done"; fullText: string } @@ -7,10 +9,21 @@ export type AgentEvent = | { type: "stream_discard" } | { type: "tool_call"; label: string } | { type: "tool_result"; summary: string; isError: boolean } + /** A sub-agent's tool call or result, forwarded to the parent so its work is visible while it + * runs headless. Routed to the dedicated sub-agent panel below the input (not the main + * scrollback) — see App.tsx. */ + | { type: "subagent"; description: string; line: SubagentLine } /** A hook (see hooks/runner.ts) blocked something or failed non-fatally — surfaced as a notice. */ | { 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 }; + | { type: "notice"; text: string; isError: boolean } + /** The `todo_write` tool replaced the session's task checklist — carries the full new list so + * the UI can render it as a standalone checklist item rather than raw JSON tool output. */ + | { type: "todos_update"; todos: TodoItem[] }; + +export type SubagentLine = + | { kind: "call"; label: string } + | { kind: "result"; summary: string; isError: boolean }; export type AgentEventHandler = (event: AgentEvent) => void; \ No newline at end of file diff --git a/src/agent/loop.test.ts b/src/agent/loop.test.ts index f755dd0..0d008da 100644 --- a/src/agent/loop.test.ts +++ b/src/agent/loop.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { z } from "zod"; import { MaxIterationsError, runTurn, shouldAutoCompact } from "./loop.js"; +import { agentTool } from "../tools/agentTool.js"; import { createSession } from "./session.js"; import { buildToolSet } from "../tools/toolset.js"; +import type { ConfirmFn } from "../permissions/types.js"; import type { ToolDef } from "../tools/types.js"; import type { Session } from "./session.js"; @@ -150,6 +152,16 @@ describe("runTurn / max iterations", () => { // 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); + // After mid-turn compaction, a user turn must follow the recap — compactSession otherwise leaves + // [system, assistant-recap] with no user turn, and a local model asked to continue from there + // routinely empty-stops (runTurn returns ""), which is what made sub-agents come back as + // "Sub-agent finished (0 chars)" after compacting mid-task. + const recapIdx = session.messages.findIndex( + (m) => m.role === "assistant" && typeof m.content === "string" && m.content.includes("compacted"), + ); + expect(recapIdx).toBeGreaterThan(-1); + const afterRecap = session.messages.slice(recapIdx + 1); + expect(afterRecap.some((m) => m.role === "user")).toBe(true); }); it("times out a stream stuck sending only empty heartbeat chunks forever", async () => { @@ -202,4 +214,286 @@ describe("runTurn / max iterations", () => { else process.env.LOCODE_REQUEST_TIMEOUT_MS = originalEnv; } }, 25_000); + + it("threads the turn's abort signal to confirm, so a sub-agent timeout can dismiss a pending prompt", async () => { + // Regression for the sub-agent timeout bug: when a sub-agent times out while one of its mutating + // tool calls is awaiting user confirmation, the abort must reach `confirm` so the prompt can be + // dismissed and the (abandoned) tool call rejected — otherwise the user could later approve a + // mutation the parent already gave up on. runSubAgentTurn passes its timeout controller's signal + // to runTurn; this test drives runTurn with such a signal directly and confirms it's threaded + // all the way down to the confirm call. + const mutatingTool: ToolDef = { + name: "edit_file", + description: "edits a file", + schema: z.object({}), + mutating: true, + handler: async () => ({ ok: true }), + }; + const toolset = buildToolSet([mutatingTool]); + + function makeChunk() { + return { + choices: [ + { + delta: { tool_calls: [{ index: 0, id: "call_0", function: { name: "edit_file", 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() }; + }, + }); + })(), + })), + }, + }, + } as any; + + const ac = new AbortController(); + let capturedSignal: AbortSignal | undefined; + const confirm: ConfirmFn = (o) => + new Promise((_resolve, reject) => { + capturedSignal = o.signal; + // Hang like a real prompt waiting on the user; reject when the turn aborts (mimics what + // makeConfirmFn does on abort — see confirmFn.test.ts). + o.signal?.addEventListener("abort", () => reject(new Error("aborted")), { once: true }); + }); + + const session = createSession(fakeClient, "test-model", process.cwd(), confirm, "native", [mutatingTool]); + session.toolset = toolset; + + // Abort shortly after the turn starts — while the mutating tool's confirm is pending (the only + // thing keeping the turn alive between the stream ending and the next request). + setTimeout(() => ac.abort(), 50); + + await expect(runTurn(session, "edit a file", () => {}, toolset, ac.signal)).rejects.toThrow(/aborted/); + // The signal was threaded through gateAndRun into the confirm call — the prerequisite for + // makeConfirmFn to dismiss the prompt on a sub-agent timeout. + expect(capturedSignal).toBe(ac.signal); + }); + + it("prunes older images from history, keeping only the most recent few at full resolution", async () => { + // Unlike text tool results, an image's base64 payload has no per-call cap and (without pruning) + // gets resent in full on every subsequent request for the rest of the session — a handful of + // screenshots can dwarf every other cost combined on a backend with no prompt caching. + const imageTool: ToolDef = { + name: "read_image", + description: "returns an image", + schema: z.object({}), + mutating: false, + handler: async () => ({ image: true, mimeType: "image/png", base64: "AAAA" }), + }; + const toolset = buildToolSet([imageTool]); + + function threeImageCallsChunk() { + return { + choices: [ + { + delta: { + tool_calls: [ + { index: 0, id: "call_0", function: { name: "read_image", arguments: "{}" } }, + { index: 1, id: "call_1", function: { name: "read_image", arguments: "{}" } }, + { index: 2, id: "call_2", function: { name: "read_image", arguments: "{}" } }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }; + } + function finalTextChunk() { + return { choices: [{ delta: { content: "done" }, finish_reason: "stop" }] }; + } + + let streamingCallCount = 0; + const fakeClient = { + chat: { + completions: { + create: vi.fn(async () => { + streamingCallCount++; + const chunk = streamingCallCount === 1 ? threeImageCallsChunk() : 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; + + const result = await runTurn(session, "read three images", () => {}); + expect(result).toBe("done"); + + const imageParts = session.messages.flatMap((m) => + Array.isArray(m.content) ? (m.content as any[]).filter((p) => p?.type === "image_url") : [], + ); + expect(imageParts).toHaveLength(2); + + const placeholders = session.messages.flatMap((m) => + Array.isArray(m.content) ? (m.content as any[]).filter((p) => p?.type === "text" && p.text.includes("omitted")) : [], + ); + expect(placeholders).toHaveLength(1); + }); + + it("tells the calling model not to retry a sub-agent that ran out of steps, instead of the top-level 'send another message' text", async () => { + // Regression for the observed failure mode: a sub-agent hits MaxIterationsError, its (ephemeral, + // never-persisted) session is discarded, and the parent model — seeing the generic top-level + // message ("send another message to continue") — has no better option than to re-invoke `agent` + // with the identical prompt, paying the full step budget again for nothing. + function agentCallChunk() { + return { + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + id: "call_agent", + function: { + name: "agent", + arguments: JSON.stringify({ description: "find bugs", prompt: "find bugs in plugins/MCP/hooks" }), + }, + }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }; + } + function noopCallChunk(n: number) { + return { + choices: [ + { + delta: { tool_calls: [{ index: 0, id: `sub_call_${n}`, function: { name: "noop", arguments: "{}" } }] }, + finish_reason: "tool_calls", + }, + ], + }; + } + function finalTextChunk() { + return { choices: [{ delta: { content: "done" }, finish_reason: "stop" }] }; + } + + let callCount = 0; + const fakeClient = { + chat: { + completions: { + create: vi.fn(async () => { + callCount++; + // call 1: top-level turn delegates to a sub-agent. calls 2-4: the sub-agent's own turn, + // which never converges (always another tool call) until it exhausts its 3-step budget. + // call 5: the top-level turn's next iteration, now that the failed sub-agent call returned. + const chunk = callCount === 1 ? agentCallChunk() : callCount <= 4 ? noopCallChunk(callCount) : 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 toolset = buildToolSet([agentTool]); + const session = createSession(fakeClient, "test-model", process.cwd(), async () => "once", "native", [agentTool]); + session.toolset = toolset; + session.maxIterations = 3; + + const result = await runTurn(session, "delegate a task", () => {}); + expect(result).toBe("done"); + + const toolResultMessage = session.messages.find((m) => m.role === "tool" && (m as any).tool_call_id === "call_agent"); + expect(toolResultMessage).toBeDefined(); + const content = String((toolResultMessage as any).content); + expect(content).toContain("split the task into smaller"); + expect(content).not.toContain("send another message to continue"); + }); + + it("blocks mutating tools outright in plan mode, without ever prompting for confirmation", async () => { + const mutatingTool: ToolDef = { + name: "edit_file", + description: "edits a file", + schema: z.object({}), + mutating: true, + handler: async () => ({ ok: true }), + }; + const toolset = buildToolSet([mutatingTool]); + + function toolCallChunk() { + return { + choices: [ + { + delta: { tool_calls: [{ index: 0, id: "call_0", function: { name: "edit_file", arguments: "{}" } }] }, + finish_reason: "tool_calls", + }, + ], + }; + } + function finalTextChunk() { + return { choices: [{ delta: { content: "here's my plan" }, finish_reason: "stop" }] }; + } + + let streamingCallCount = 0; + const fakeClient = { + chat: { + completions: { + create: vi.fn(async () => { + streamingCallCount++; + const chunk = streamingCallCount === 1 ? toolCallChunk() : 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 confirm = vi.fn(async () => "once" as const); + const session = createSession(fakeClient, "test-model", process.cwd(), confirm, "native", [mutatingTool]); + session.toolset = toolset; + session.permissions.setMode("plan"); + + const result = await runTurn(session, "edit a file", () => {}); + expect(result).toBe("here's my plan"); + // The mutating tool must never reach the confirmation prompt in plan mode — it's blocked + // upfront, not merely auto-denied after asking. + expect(confirm).not.toHaveBeenCalled(); + + const toolResultMessage = session.messages.find((m) => m.role === "tool" && (m as any).tool_call_id === "call_0"); + expect(String((toolResultMessage as any).content)).toContain("plan mode is active"); + }); }); diff --git a/src/agent/loop.ts b/src/agent/loop.ts index 5881deb..9dfcb42 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -8,7 +8,7 @@ import type { ChatCompletionContentPart, ChatCompletionMessageParam } from "open export type ChatCompletionUserContent = ChatCompletionContentPart[]; import type { AgentEventHandler } from "./events.js"; import { buildToolSet, type ToolSet } from "../tools/toolset.js"; -import type { SubAgentOverrides, SubAgentTask } from "../tools/types.js"; +import type { SubAgentOverrides, SubAgentTask, TodoItem } from "../tools/types.js"; import { FALLBACK_RETRY_NUDGE } from "../toolcalling/fallbackPrompt.js"; import { parseFallbackToolCalls } from "../toolcalling/fallbackParser.js"; import { resolveToolCall } from "../toolcalling/nativeAdapter.js"; @@ -16,7 +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 { resolveRequestTimeoutMs, resolveSubagentTimeoutMs } from "../config/config.js"; import { buildSystemPrompt } from "./systemPrompt.js"; import type { Session } from "./session.js"; @@ -126,7 +126,7 @@ const MAX_MALFORMED_RETRIES = 2; // spawn nested sub-agents through normal tool use), but these are independent, explicit backstops // that hold even if that filtering were ever bypassed or a future change loosened it. const MAX_SUBAGENT_DEPTH = 1; -const SUBAGENT_TIMEOUT_MS = 120_000; +const SUBAGENT_TIMEOUT_MS = resolveSubagentTimeoutMs(); function updateContextTracking(session: Session, promptTokens: number | null | undefined): void { if (typeof promptTokens === "number") { @@ -205,7 +205,7 @@ export async function compactSession(session: Session): Promise { } session.messages = [ - { role: "system", content: buildSystemPrompt(session.toolset.tools, session.mode) }, + { role: "system", content: buildSystemPrompt(session.toolset.tools, session.mode, session.projectInstructions) }, { role: "assistant", content: `[Earlier conversation compacted to save context]\n\n${summary}` }, ]; // res.usage describes the old (now-discarded) prompt, not the new shorter history — estimate fresh. @@ -306,6 +306,33 @@ function pushPendingImages(session: Session, images: ImageAttachment[]): void { } } +// Unlike text tool results (capped per-call, see MAX_READ_CHARS in tools/readFile.ts), an +// image's full base64 payload has no per-call cap and — unless pruned — stays in session.messages +// verbatim, getting resent in full on every single subsequent request for the rest of the session +// (there's no prompt caching for local backends). A handful of screenshots read early in a long +// session can dwarf every other cost combined. Keep only the most recent few at full resolution. +const MAX_RETAINED_IMAGES = 2; +const PRUNED_IMAGE_PLACEHOLDER = "[earlier image omitted from context to save tokens]"; + +/** Replaces all but the most recent MAX_RETAINED_IMAGES image_url content parts in the session's + * history with a small text placeholder. Called at the top of every runTurn iteration so no + * outgoing request ever carries more than the cap, regardless of which branch (native tool calls, + * fallback tool calls, malformed-retry) most recently added images. */ +function pruneOldImages(session: Session): void { + const locations: { msgIndex: number; partIndex: number }[] = []; + session.messages.forEach((m, msgIndex) => { + if (!Array.isArray(m.content)) return; + m.content.forEach((part, partIndex) => { + if ((part as { type?: string })?.type === "image_url") locations.push({ msgIndex, partIndex }); + }); + }); + const toDrop = locations.slice(0, Math.max(0, locations.length - MAX_RETAINED_IMAGES)); + for (const { msgIndex, partIndex } of toDrop) { + const parts = session.messages[msgIndex]!.content as ChatCompletionContentPart[]; + parts[partIndex] = { type: "text", text: PRUNED_IMAGE_PLACEHOLDER }; + } +} + /** 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. @@ -324,6 +351,10 @@ async function gateAndRun( rawLabel: string, session: Session, emit: AgentEventHandler, + /** The turn's abort signal, threaded down so a sub-agent timeout can both dismiss any permission + * prompt its mutating call is waiting on (via confirm) and kill an in-flight bash child (via the + * tool ctx). Undefined for top-level turns, which have no timeout to abort on. */ + signal?: AbortSignal, ): Promise { session.stats.toolCalls++; if ("error" in resolved) { @@ -343,8 +374,13 @@ async function gateAndRun( try { const ctx = { cwd: session.cwd, - runSubAgent: (task: SubAgentTask, overrides?: SubAgentOverrides) => runSubAgentTurn(session, task, overrides), + runSubAgent: (task: SubAgentTask, overrides?: SubAgentOverrides) => runSubAgentTurn(session, task, overrides, emit), backgroundControl, + signal, + setTodos: (todos: TodoItem[]) => { + session.todos = todos; + emit({ type: "todos_update", todos }); + }, }; // PreToolUse fires before permission modes apply — a hook's block can't be bypassed by @@ -357,6 +393,18 @@ async function gateAndRun( return { error: `Blocked by hook: ${preHook.reason}` }; } + // Plan mode blocks every mutating tool outright — no prompt, since the whole point is that + // nothing changes until the user reviews the plan and explicitly exits plan mode (Shift+Tab or + // /perm). Checked ahead of the normal auto-approve/confirm gate so a prior "allow for session" + // grant or auto-accept mode can't bypass it. + if (tool.mutating && session.permissions.getMode() === "plan") { + const message = + "Blocked: plan mode is active, so mutating tools can't run. Describe what you'd do in your final answer " + + "instead of calling this tool — the user can exit plan mode (Shift+Tab or /perm) once they approve the plan."; + emit({ type: "tool_result", summary: message, isError: true }); + return { error: message }; + } + if (tool.mutating && !session.permissions.isAutoApproved(tool.name)) { const preview = tool.preview ? await tool.preview(args, ctx) : undefined; const permissionHook = await runHooksForEvent( @@ -370,7 +418,7 @@ async function gateAndRun( emit({ type: "tool_result", summary: `Blocked by hook: ${permissionHook.reason}`, isError: true }); return { error: `Blocked by hook: ${permissionHook.reason}` }; } - const decision = await session.confirm({ toolName: tool.name, args, preview }); + const decision = await session.confirm({ toolName: tool.name, args, preview, signal }); if (decision === "deny") { emit({ type: "tool_result", summary: "Denied by user", isError: true }); return { error: "Denied by user." }; @@ -418,7 +466,16 @@ async function gateAndRun( * an explicit depth check (on top of the toolset already excluding `agent` for sub-agents) and a * hard wall-clock timeout, so a misbehaving model can't hang the parent turn indefinitely. */ -async function runSubAgentTurn(parent: Session, task: SubAgentTask, overrides?: SubAgentOverrides): Promise { +async function runSubAgentTurn( + parent: Session, + task: SubAgentTask, + overrides: SubAgentOverrides | undefined, + /** The parent turn's event handler — used only to surface the sub-agent's tool calls so a + * long-running sub-agent isn't a silent void in the UI. The sub-agent still runs "headless" in + * the sense that its streamed text isn't shown (its final text comes back via the agent tool's + * result), but its file reads/greps are forwarded so the user can see it working. */ + emit: AgentEventHandler, +): Promise { if (parent.subAgentDepth >= MAX_SUBAGENT_DEPTH) { throw new Error(`Sub-agents cannot spawn further sub-agents (max depth ${MAX_SUBAGENT_DEPTH}).`); } @@ -431,7 +488,7 @@ async function runSubAgentTurn(parent: Session, task: SubAgentTask, overrides?: const subToolset = buildToolSet(restricted); const systemPrompt = overrides?.systemPrompt ? `${overrides.systemPrompt}\n\nAvailable tools:\n${subToolset.tools.map((t) => `- ${t.name}: ${t.description}`).join("\n")}` - : `${buildSystemPrompt(subToolset.tools, parent.mode)}\n\nYou are a sub-agent handling one focused task delegated by another assistant. Only the final text you return will be seen — not your intermediate tool calls — so make your answer complete and self-contained.`; + : `${buildSystemPrompt(subToolset.tools, parent.mode, parent.projectInstructions)}\n\nYou are a sub-agent handling one focused task delegated by another assistant. Only the final text you return will be seen — not your intermediate tool calls — so make your answer complete and self-contained.`; const subMessages: ChatCompletionMessageParam[] = [{ role: "system", content: systemPrompt }]; const subSession: Session = { id: randomUUID(), @@ -458,6 +515,10 @@ async function runSubAgentTurn(parent: Session, task: SubAgentTask, overrides?: // parent UI's Ctrl+B since sub-agent tool calls aren't shown mid-flight anyway (see class doc above). activeBackground: null, mutationCommitLength: null, + projectInstructions: parent.projectInstructions, + // Independent from the parent's — a sub-agent runs headless (see class doc above), so its own + // checklist has nowhere to render even if it called todo_write. + todos: [], }; // Run (and honor) the SubagentStart hook before arming the timeout, so a slow hook doesn't eat @@ -486,10 +547,38 @@ async function runSubAgentTurn(parent: Session, task: SubAgentTask, overrides?: // The turn promise outlives the race when the timeout wins. Aborting cancels any still-pending // request; the `.catch` swallows the resulting late rejection so it doesn't surface as an unhandled // promise rejection after the parent already moved on. - const turnPromise = runTurn(subSession, task.prompt, () => {}, subToolset, ac.signal); + const subEmit: AgentEventHandler = (event) => { + // Forward the sub-agent's tool calls/results to the parent UI as `subagent` events, which the + // App renders in a dedicated panel below the input (not the main scrollback). Skip streamed + // text and notices — the sub-agent's final answer is returned via the agent tool's result, and + // surfacing its partial text would both duplicate that and clutter the transcript. + if (event.type === "tool_call") { + emit({ type: "subagent", description: task.description, line: { kind: "call", label: event.label } }); + } else if (event.type === "tool_result") { + emit({ type: "subagent", description: task.description, line: { kind: "result", summary: event.summary, isError: event.isError } }); + } + }; + const turnPromise = runTurn(subSession, task.prompt, subEmit, subToolset, ac.signal); try { const answer = await Promise.race([turnPromise, timeout]); return answer; + } catch (err) { + // MaxIterationsError's message ("send another message to continue") is written for the + // top-level session, where the human can literally do that and resume the same (persisted) + // history. A sub-agent's session is thrown away the moment this call returns — there's nothing + // to "continue". Without this, the calling model's only visible option is to re-invoke `agent` + // with the same prompt, paying the full step budget again for a task that already proved too + // big for it (this is exactly what caused two back-to-back identical "Find bugs in + // plugins/MCP/hooks" sub-agent calls to each burn 50 steps for nothing). Redirect toward + // narrowing scope instead. + if (err instanceof MaxIterationsError) { + throw new Error( + `Sub-agent "${task.description}" ran out of its step budget (${subSession.maxIterations} steps) before finishing. ` + + `Its work isn't saved for a follow-up call — retrying with the same prompt will hit the same wall. Instead, split ` + + `the task into smaller, more specific sub-agents (e.g. one per file or directory) or narrow this one's scope.`, + ); + } + throw err; } finally { clearTimeout(timeoutId!); ac.abort(); // no-op if the turn finished on its own; cancels a still-pending request otherwise @@ -514,6 +603,7 @@ async function handleCompletedMessage( session: Session, emit: AgentEventHandler, toolset: ToolSet, + signal?: AbortSignal, ): Promise<{ text: string; hadToolCalls: boolean; malformed?: boolean }> { // Native tool calls if (session.mode === "native" && message.tool_calls?.length) { @@ -527,7 +617,7 @@ async function handleCompletedMessage( for (const call of message.tool_calls) { const resolved = resolveToolCall(call as any, toolset.registry); const label = call.type === "function" ? `${call.function.name}(${call.function.arguments})` : call.type; - const result = await gateAndRun(resolved, label, session, emit); + const result = await gateAndRun(resolved, label, session, emit, signal); 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); @@ -548,7 +638,7 @@ async function handleCompletedMessage( for (const call of parsed.calls) { const resolved = resolveToolInvocation(call.name, call.arguments, toolset.registry); const label = `${call.name}(${JSON.stringify(call.arguments)})`; - const result = await gateAndRun(resolved, label, session, emit); + const result = await gateAndRun(resolved, label, session, emit, signal); pushToolResultMessage(session, "fallback", "", call.name, result); noteMutationCommit(session, resolved, result); } @@ -579,6 +669,8 @@ export async function runTurn( let malformedRetries = 0; for (let i = 0; i < session.maxIterations; i++) { + pruneOldImages(session); + // 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 @@ -588,14 +680,28 @@ export async function runTurn( if (shouldAutoCompact(session)) { try { await compactSession(session); - // compactSession replaces session.messages wholesale, so any previously-recorded + emit({ type: "notice", text: "Context was getting full — auto-compacted mid-turn.", isError: false }); + // compactSession leaves the history as [system, assistant-recap] with no user turn. Sending + // that to the model gives it nothing to respond to — local models routinely answer with an + // empty stop (which runTurn then returns as ""), which is exactly why sub-agents that + // compacted mid-task came back as "Sub-agent finished (0 chars)", and why a tool-heavy main + // turn appeared to hang/stop after compacting. Re-add a user turn so the model resumes the + // task instead of going empty. (Between-turn compaction in App.tsx doesn't need this — the + // user's next message supplies the turn.) + session.messages.push({ + role: "user", + content: + "Continue with your current task. (The conversation so far was just compacted to save context — " + + "pick up from where the summary above left off, and give your final answer once the task is done.)", + }); + // compactSession replaced 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. + // slots instead of truncating it. Re-anchor AFTER the synthetic user turn above, so a + // rollback preserves it (dropping it would leave [system, recap] with no user turn again — + // the very bug this user message exists to prevent). 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 @@ -751,7 +857,7 @@ export async function runTurn( if (!message) throw new AgentError("Empty response from model."); updateContextTracking(session, res.usage?.prompt_tokens); - const result = await handleCompletedMessage(message, session, emit, toolset); + const result = await handleCompletedMessage(message, session, emit, toolset, signal); if (result.hadToolCalls) continue; // Non-tool-call text from the retry @@ -781,7 +887,7 @@ export async function runTurn( toolset.registry, ); const label = `${tc.name}(${tc.arguments})`; - const result = await gateAndRun(resolved, label, session, emit); + const result = await gateAndRun(resolved, label, session, emit, signal); const image = pushToolResultMessage(session, "native", tc.id, tc.name, result); noteMutationCommit(session, resolved, result); if (image) pendingImages.push(image); @@ -802,7 +908,7 @@ export async function runTurn( for (const call of parsed.calls) { const resolved = resolveToolInvocation(call.name, call.arguments, toolset.registry); const label = `${call.name}(${JSON.stringify(call.arguments)})`; - const result = await gateAndRun(resolved, label, session, emit); + const result = await gateAndRun(resolved, label, session, emit, signal); pushToolResultMessage(session, "fallback", "", call.name, result); noteMutationCommit(session, resolved, result); } diff --git a/src/agent/session.ts b/src/agent/session.ts index fadbe56..3d00519 100644 --- a/src/agent/session.ts +++ b/src/agent/session.ts @@ -8,7 +8,7 @@ import { PermissionManager } from "../permissions/permissionManager.js"; import type { ConfirmFn } from "../permissions/types.js"; import { TOOLS } from "../tools/index.js"; import { buildToolSet, type ToolSet } from "../tools/toolset.js"; -import type { ToolDef } from "../tools/types.js"; +import type { TodoItem, ToolDef } from "../tools/types.js"; import { estimateTokens } from "../utils/tokens.js"; import { buildSystemPrompt } from "./systemPrompt.js"; @@ -79,6 +79,13 @@ export interface Session { * 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; + /** Folded into every system prompt rebuild (initial, /mode switch, mid-turn compaction) so the + * project's CLAUDE.md/AGENTS.md conventions survive everything that regenerates messages[0]. + * Null when neither file exists. */ + projectInstructions: string | null; + /** Current task checklist shown to the user via the `todo_write` tool — session-scoped state + * since checklist items are a snapshot of progress, not part of the model-visible conversation. */ + todos: TodoItem[]; } export function createSession( @@ -92,9 +99,12 @@ export function createSession( contextWindowIsEstimate: boolean = true, maxIterations: number = DEFAULT_MAX_ITERATIONS, autoCompactThreshold: number = DEFAULT_AUTO_COMPACT_THRESHOLD, + projectInstructions: string | null = null, ): Session { const toolset = buildToolSet(tools); - const messages: ChatCompletionMessageParam[] = [{ role: "system", content: buildSystemPrompt(toolset.tools, mode) }]; + const messages: ChatCompletionMessageParam[] = [ + { role: "system", content: buildSystemPrompt(toolset.tools, mode, projectInstructions) }, + ]; return { id: randomUUID(), createdAt: new Date().toISOString(), @@ -116,6 +126,8 @@ export function createSession( stats: initialStats(), activeBackground: null, mutationCommitLength: null, + projectInstructions, + todos: [], }; } @@ -131,10 +143,11 @@ export function createSessionFromRecord( contextWindowIsEstimate: boolean = true, maxIterations: number = DEFAULT_MAX_ITERATIONS, autoCompactThreshold: number = DEFAULT_AUTO_COMPACT_THRESHOLD, + projectInstructions: string | null = null, ): Session { const toolset = buildToolSet(tools); const messages: ChatCompletionMessageParam[] = [ - { role: "system", content: buildSystemPrompt(toolset.tools, record.mode) }, + { role: "system", content: buildSystemPrompt(toolset.tools, record.mode, projectInstructions) }, ...record.messages, ]; return { @@ -158,6 +171,8 @@ export function createSessionFromRecord( stats: initialStats(), activeBackground: null, mutationCommitLength: null, + projectInstructions, + todos: [], }; } @@ -182,5 +197,5 @@ export function resetSession(session: Session): void { export function setMode(session: Session, mode: ToolCallMode): void { session.mode = mode; - session.messages[0] = { role: "system", content: buildSystemPrompt(session.toolset.tools, mode) }; + session.messages[0] = { role: "system", content: buildSystemPrompt(session.toolset.tools, mode, session.projectInstructions) }; } diff --git a/src/agent/systemPrompt.ts b/src/agent/systemPrompt.ts index b4587ee..73acefe 100644 --- a/src/agent/systemPrompt.ts +++ b/src/agent/systemPrompt.ts @@ -2,7 +2,7 @@ import type { ToolCallMode } from "../backend/capabilityProbe.js"; import { FALLBACK_TOOL_INSTRUCTIONS } from "../toolcalling/fallbackPrompt.js"; import type { ToolDef } from "../tools/types.js"; -export function buildSystemPrompt(tools: ToolDef[], mode: ToolCallMode): string { +export function buildSystemPrompt(tools: ToolDef[], mode: ToolCallMode, projectInstructions?: string | null): string { const toolList = tools.map((t) => `- ${t.name}: ${t.description}`).join("\n"); const base = `You are a helpful local coding assistant with access to tools for exploring a codebase on the user's machine. @@ -18,5 +18,6 @@ Guidelines: - When you have enough information, respond with a normal text answer instead of calling a tool. - Keep answers concise and focused on the user's question.`; - return mode === "fallback" ? `${base}\n\n${FALLBACK_TOOL_INSTRUCTIONS}` : base; + const withMode = mode === "fallback" ? `${base}\n\n${FALLBACK_TOOL_INSTRUCTIONS}` : base; + return projectInstructions ? `${withMode}\n\n${projectInstructions}` : withMode; } diff --git a/src/backend/capabilityCache.ts b/src/backend/capabilityCache.ts index 0748e13..4bd45df 100644 --- a/src/backend/capabilityCache.ts +++ b/src/backend/capabilityCache.ts @@ -1,6 +1,7 @@ import envPaths from "env-paths"; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; +import { writeFileAtomic } from "../utils/writeFileAtomic.js"; import type { ToolCallMode } from "./capabilityProbe.js"; const paths = envPaths("locode", { suffix: "" }); @@ -12,27 +13,32 @@ function keyFor(baseURL: string, model: string): string { return `${baseURL}::${model}`; } +// In-memory memo of the on-disk cache. Re-read from disk on every load() rather than trusting the +// first read forever — a concurrent locode process (sub-agent, parallel session) may have written +// entries since this process started, and a stale memo would let setCachedMode's +// read-modify-write clobber them. let memoryCache: Cache | null = null; function load(): Cache { - if (memoryCache) return memoryCache; - if (!existsSync(cacheFile)) { - memoryCache = {}; - return memoryCache; - } - try { - memoryCache = JSON.parse(readFileSync(cacheFile, "utf-8")) as Cache; - return memoryCache; - } catch { - memoryCache = {}; - return memoryCache; + // Always re-read from disk if possible, so concurrent processes' writes aren't overwritten. + if (existsSync(cacheFile)) { + try { + memoryCache = JSON.parse(readFileSync(cacheFile, "utf-8")) as Cache; + return memoryCache; + } catch { + memoryCache = {}; + return memoryCache; + } } + memoryCache = {}; + return memoryCache; } function save(cache: Cache): void { memoryCache = cache; - mkdirSync(paths.config, { recursive: true }); - writeFileSync(cacheFile, JSON.stringify(cache, null, 2)); + // Atomic write (temp + rename) so a crash mid-write or a concurrent reader never sees a + // truncated/corrupt JSON file — the live file is either the old complete content or the new. + void writeFileAtomic(cacheFile, JSON.stringify(cache, null, 2)); } export function getCachedMode(baseURL: string, model: string): ToolCallMode | undefined { diff --git a/src/backend/capabilityProbe.ts b/src/backend/capabilityProbe.ts index 7f1971c..7a8ea08 100644 --- a/src/backend/capabilityProbe.ts +++ b/src/backend/capabilityProbe.ts @@ -12,7 +12,13 @@ const PING_TOOL = { }, }; -export async function probeToolCallSupport(client: OpenAI, model: string): Promise { +/** Probes whether the backend/model supports native tool calls by asking it to call a `ping` tool. + * Returns `"native"` if it did, `"fallback"` if it responded without calling the tool, or `null` + * if the probe itself errored (network timeout, 5xx, rate limit, malformed response, etc.). + * Callers must NOT cache the `null` result: a transient failure is not evidence that the model + * lacks tool support, and caching it would permanently disable native tool calls across all + * future sessions for this backend+model after a single bad request. */ +export async function probeToolCallSupport(client: OpenAI, model: string): Promise { try { const res = await client.chat.completions.create({ model, @@ -24,6 +30,7 @@ export async function probeToolCallSupport(client: OpenAI, model: string): Promi { role: "user", content: "ping" }, ], tools: [PING_TOOL], + tool_choice: { type: "function", function: { name: "ping" } }, stream: false, max_tokens: 200, }); @@ -31,6 +38,8 @@ export async function probeToolCallSupport(client: OpenAI, model: string): Promi const called = msg?.tool_calls?.some((c) => c.type === "function" && c.function.name === "ping"); return called ? "native" : "fallback"; } catch { - return "fallback"; + // Probe failed — indistinguishable from "no support" until we know otherwise. Return null so + // the caller can avoid caching a negative result derived from a failure (see JSDoc above). + return null; } -} +} \ No newline at end of file diff --git a/src/backend/client.ts b/src/backend/client.ts index 31af725..127ba79 100644 --- a/src/backend/client.ts +++ b/src/backend/client.ts @@ -5,7 +5,9 @@ import type { AppConfig } from "../config/types.js"; export function makeClient(cfg: AppConfig): OpenAI { return new OpenAI({ baseURL: cfg.baseURL, - apiKey: "local", + // Honor explicit API keys for OpenAI-compatible proxies/services; default to a dummy value for + // local backends that don't check it. + apiKey: process.env.OPENAI_API_KEY ?? process.env.LOCODE_API_KEY ?? "local", // The SDK defaults to a 10-minute timeout with 2 retries (up to 30 min before a request ever // fails). For local backends a slow response almost always means the model is genuinely stuck, // not a transient network blip, so retrying just compounds the wait — fail faster instead. diff --git a/src/backend/contextWindow.ts b/src/backend/contextWindow.ts index 39725bb..262924c 100644 --- a/src/backend/contextWindow.ts +++ b/src/backend/contextWindow.ts @@ -2,7 +2,7 @@ import { resolveContextWindowDefault } from "../config/config.js"; import { getCachedContextWindow, setCachedContextWindow } from "./contextWindowCache.js"; function stripV1(baseURL: string): string { - return baseURL.replace(/\/v1\/?$/, ""); + return baseURL.replace(/\/v1\/?$/i, ""); } /** Ollama's native (non-OpenAI-compatible) endpoint exposes the model's real context length, @@ -28,6 +28,12 @@ async function detectOllamaContextWindow(baseURL: string, model: string): Promis } } +function normalizeModelId(id: string): string { + // Strip common tag suffixes and lower-case so "qwen3-coder:30b" and "qwen3-coder" match the + // backend's reported id more reliably. + return id.replace(/:.*$/, "").toLowerCase(); +} + /** LM Studio's native REST API (distinct from its OpenAI-compatible one) reports the context * length a loaded model was actually started with, or its maximum otherwise. */ async function detectLmStudioContextWindow(baseURL: string, model: string): Promise { @@ -35,8 +41,16 @@ async function detectLmStudioContextWindow(baseURL: string, model: string): Prom const res = await fetch(`${stripV1(baseURL)}/api/v0/models`); if (!res.ok) return null; const data = (await res.json()) as { data?: Array> }; - const match = data.data?.find((m) => m.id === model); - const length = match?.loaded_context_length ?? match?.max_context_length; + const normalized = normalizeModelId(model); + const match = data.data?.find((m) => { + const id = typeof m.id === "string" ? m.id : ""; + return normalizeModelId(id) === normalized; + }); + if (!match) return null; + // Only trust loaded_context_length when the model is actually loaded; max_context_length may + // belong to a different (unloaded) model entry. + const loaded = match.loaded === true || match.state === "loaded"; + const length = loaded ? match.loaded_context_length : match.max_context_length; return typeof length === "number" ? length : null; } catch { return null; @@ -59,14 +73,15 @@ export interface ResolvedContextWindow { } /** Cache-then-detect-then-default resolution, so repeat sessions with the same backend+model skip - * the network round-trip. */ + * the network round-trip. Cache entries store provenance so a fallback default is never mistaken + * for a real detected value. */ export async function resolveContextWindow(baseURL: string, model: string): Promise { const cached = getCachedContextWindow(baseURL, model); - if (cached !== undefined) return { value: cached, isEstimate: false }; + if (cached !== undefined) return { value: cached.value, isEstimate: cached.isEstimate }; const detected = await detectContextWindow(baseURL, model); if (detected !== null) { - setCachedContextWindow(baseURL, model, detected); + setCachedContextWindow(baseURL, model, { value: detected, isEstimate: false }); return { value: detected, isEstimate: false }; } diff --git a/src/backend/contextWindowCache.test.ts b/src/backend/contextWindowCache.test.ts new file mode 100644 index 0000000..d562a33 --- /dev/null +++ b/src/backend/contextWindowCache.test.ts @@ -0,0 +1,45 @@ +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import envPaths from "env-paths"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { getCachedContextWindow } from "./contextWindowCache.js"; + +const cacheFile = path.join(envPaths("locode", { suffix: "" }).config, "context-windows.json"); + +describe("contextWindowCache", () => { + let originalContent: string | null = null; + + beforeEach(() => { + originalContent = existsSync(cacheFile) ? readFileSync(cacheFile, "utf-8") : null; + }); + + afterEach(() => { + if (originalContent !== null) { + writeFileSync(cacheFile, originalContent, "utf-8"); + } else if (existsSync(cacheFile)) { + rmSync(cacheFile); + } + }); + + it("reads a well-formed cached entry", () => { + // Written directly (rather than via setCachedContextWindow, whose write is fire-and-forget + // async and would race this file-backed cache's always-read-from-disk load()) so the test is + // deterministic and can't leak a pending write past its own afterEach cleanup. + mkdirSync(path.dirname(cacheFile), { recursive: true }); + writeFileSync(cacheFile, JSON.stringify({ "http://localhost:11434/v1::test-model": { value: 32768, isEstimate: false } }), "utf-8"); + + expect(getCachedContextWindow("http://localhost:11434/v1", "test-model")).toEqual({ value: 32768, isEstimate: false }); + }); + + it("treats a legacy bare-number cache entry as a miss instead of returning {value: undefined}", () => { + // Reproduces the on-disk format an older locode version wrote: a plain number instead of + // { value, isEstimate }. Regression for the bug where this silently pinned every session's + // context window at the hardcoded 8192 default, even for a model whose real (much larger) + // window was sitting right there in the cache file. + mkdirSync(path.dirname(cacheFile), { recursive: true }); + writeFileSync(cacheFile, JSON.stringify({ "http://localhost:11434/v1::legacy-model": 262144 }), "utf-8"); + + const result = getCachedContextWindow("http://localhost:11434/v1", "legacy-model"); + expect(result).toBeUndefined(); + }); +}); diff --git a/src/backend/contextWindowCache.ts b/src/backend/contextWindowCache.ts index e292601..0939cb8 100644 --- a/src/backend/contextWindowCache.ts +++ b/src/backend/contextWindowCache.ts @@ -1,11 +1,17 @@ import envPaths from "env-paths"; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync } from "node:fs"; import path from "node:path"; +import { writeFileAtomic } from "../utils/writeFileAtomic.js"; const paths = envPaths("locode", { suffix: "" }); const cacheFile = path.join(paths.config, "context-windows.json"); -type Cache = Record; +export interface CachedContextWindow { + value: number; + isEstimate: boolean; +} + +type Cache = Record; function keyFor(baseURL: string, model: string): string { return `${baseURL}::${model}`; @@ -14,31 +20,40 @@ function keyFor(baseURL: string, model: string): string { let memoryCache: Cache | null = null; function load(): Cache { - if (memoryCache) return memoryCache; - if (!existsSync(cacheFile)) { - memoryCache = {}; - return memoryCache; - } - try { - memoryCache = JSON.parse(readFileSync(cacheFile, "utf-8")) as Cache; - return memoryCache; - } catch { - memoryCache = {}; - return memoryCache; + // Always re-read from disk so concurrent locode processes don't overwrite each other's entries. + if (existsSync(cacheFile)) { + try { + memoryCache = JSON.parse(readFileSync(cacheFile, "utf-8")) as Cache; + return memoryCache; + } catch { + memoryCache = {}; + return memoryCache; + } } + memoryCache = {}; + return memoryCache; } function save(cache: Cache): void { memoryCache = cache; - mkdirSync(paths.config, { recursive: true }); - writeFileSync(cacheFile, JSON.stringify(cache, null, 2)); + void writeFileAtomic(cacheFile, JSON.stringify(cache, null, 2)); } -export function getCachedContextWindow(baseURL: string, model: string): number | undefined { - return load()[keyFor(baseURL, model)]; +export function getCachedContextWindow(baseURL: string, model: string): CachedContextWindow | undefined { + const entry = load()[keyFor(baseURL, model)]; + if (entry === undefined) return undefined; + // An older locode version cached a bare number instead of { value, isEstimate }. Treat that + // legacy shape as a miss so it's re-detected (and rewritten in the current format) instead of + // silently reading `.value`/`.isEstimate` off a number — both undefined — which fell through to + // createSession's default parameter and pinned the context window at the hardcoded 8192 default + // forever, even for a model whose real window was already sitting right there in the cache file. + if (typeof entry !== "object" || entry === null || typeof (entry as CachedContextWindow).value !== "number") { + return undefined; + } + return entry; } -export function setCachedContextWindow(baseURL: string, model: string, contextWindow: number): void { +export function setCachedContextWindow(baseURL: string, model: string, contextWindow: CachedContextWindow): void { const cache = load(); cache[keyFor(baseURL, model)] = contextWindow; save(cache); diff --git a/src/backend/resolveMode.ts b/src/backend/resolveMode.ts index 45a32bb..dfc84f3 100644 --- a/src/backend/resolveMode.ts +++ b/src/backend/resolveMode.ts @@ -12,6 +12,11 @@ export async function resolveToolCallMode( const cached = getCachedMode(baseURL, model); if (cached) return cached; const probed = await probeToolCallSupport(client, model); + // A `null` probe result means the probe itself failed (timeout/network/5xx), not that the model + // genuinely lacks tool support. Caching that would permanently disable native tool calls for + // this backend+model after a single transient outage. Fall back for *this* call only and leave + // the cache untouched so the next session can re-probe. + if (probed === null) return "fallback"; setCachedMode(baseURL, model, probed); return probed; -} +} \ No newline at end of file diff --git a/src/cli.ts b/src/cli.ts index 17427e5..80ec3cb 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -114,7 +114,7 @@ configCmd configCmd .command("set ") - .description("Persist a config value (backend, model, baseUrl, contextWindow, maxIterations, autoCompactThreshold, requestTimeoutMs)") + .description("Persist a config value (backend, model, baseUrl, contextWindow, maxIterations, autoCompactThreshold, requestTimeoutMs, subagentTimeoutMs)") .action((key: string, value: string) => { if ( key !== "backend" && @@ -123,10 +123,11 @@ configCmd key !== "contextWindow" && key !== "maxIterations" && key !== "autoCompactThreshold" && - key !== "requestTimeoutMs" + key !== "requestTimeoutMs" && + key !== "subagentTimeoutMs" ) { console.error( - `Unknown config key "${key}". Valid keys: backend, model, baseUrl, contextWindow, maxIterations, autoCompactThreshold, requestTimeoutMs`, + `Unknown config key "${key}". Valid keys: backend, model, baseUrl, contextWindow, maxIterations, autoCompactThreshold, requestTimeoutMs, subagentTimeoutMs`, ); process.exit(1); } @@ -152,6 +153,13 @@ configCmd process.exit(1); } stored[key] = n; + } else if (key === "subagentTimeoutMs") { + const n = Number(value); + if (!Number.isFinite(n) || n < 1_000 || n > 3_600_000) { + console.error(`subagentTimeoutMs must be between 1000 and 3600000, got "${value}".`); + process.exit(1); + } + stored[key] = n; } else { stored[key] = value; } diff --git a/src/config/config.test.ts b/src/config/config.test.ts index 35779c0..8168df4 100644 --- a/src/config/config.test.ts +++ b/src/config/config.test.ts @@ -1,15 +1,32 @@ -import { afterEach, describe, expect, it } from "vitest"; -import { loadStoredConfig, saveStoredConfig } from "./store.js"; -import { resolveAutoCompactThreshold, resolveContextWindowDefault, resolveMaxIterations, resolveRequestTimeoutMs } from "./config.js"; -import { DEFAULT_AUTO_COMPACT_THRESHOLD, DEFAULT_CONTEXT_WINDOW, DEFAULT_MAX_ITERATIONS, DEFAULT_REQUEST_TIMEOUT_MS } from "./defaults.js"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import path from "node:path"; +import os from "node:os"; +import { _setConfigFilePathForTest, loadStoredConfig, saveStoredConfig } from "./store.js"; +import { resolveAutoCompactThreshold, resolveContextWindowDefault, resolveMaxIterations, resolveRequestTimeoutMs, resolveSubagentTimeoutMs } from "./config.js"; +import { DEFAULT_AUTO_COMPACT_THRESHOLD, DEFAULT_CONTEXT_WINDOW, DEFAULT_MAX_ITERATIONS, DEFAULT_REQUEST_TIMEOUT_MS, DEFAULT_SUBAGENT_TIMEOUT_MS } from "./defaults.js"; + +// Isolate the persisted config to a temp directory so the suite never reads or overwrites the +// user's real ~/.config/locode/config.json (the previous afterEach { saveStoredConfig({}) } wiped +// the user's actual saved backend/model/etc. on every test run). +let tempDir: string; +let tempConfigFile: string; describe("config resolution", () => { + beforeEach(() => { + tempDir = mkdtempSync(path.join(os.tmpdir(), "locode-config-")); + tempConfigFile = path.join(tempDir, "config.json"); + _setConfigFilePathForTest(tempConfigFile); + }); + afterEach(() => { - saveStoredConfig({}); + _setConfigFilePathForTest(undefined); + if (existsSync(tempDir)) rmSync(tempDir, { recursive: true, force: true }); delete process.env.LOCODE_AUTO_COMPACT_THRESHOLD; delete process.env.LOCODE_CONTEXT_WINDOW; delete process.env.LOCODE_MAX_ITERATIONS; delete process.env.LOCODE_REQUEST_TIMEOUT_MS; + delete process.env.LOCODE_SUBAGENT_TIMEOUT_MS; }); it("resolves auto-compact threshold default", () => { @@ -61,4 +78,27 @@ describe("config resolution", () => { process.env.LOCODE_REQUEST_TIMEOUT_MS = "9999999"; expect(resolveRequestTimeoutMs()).toBe(DEFAULT_REQUEST_TIMEOUT_MS); }); -}); + + it("resolves sub-agent timeout default", () => { + expect(resolveSubagentTimeoutMs()).toBe(DEFAULT_SUBAGENT_TIMEOUT_MS); + }); + + it("reads sub-agent timeout from env", () => { + process.env.LOCODE_SUBAGENT_TIMEOUT_MS = "120000"; + expect(resolveSubagentTimeoutMs()).toBe(120_000); + }); + + it("reads sub-agent timeout from stored config", () => { + saveStoredConfig({ subagentTimeoutMs: 300_000 }); + expect(resolveSubagentTimeoutMs()).toBe(300_000); + }); + + it("rejects out-of-range sub-agent timeouts", () => { + process.env.LOCODE_SUBAGENT_TIMEOUT_MS = "100"; // below 1s floor + expect(resolveSubagentTimeoutMs()).toBe(DEFAULT_SUBAGENT_TIMEOUT_MS); + process.env.LOCODE_SUBAGENT_TIMEOUT_MS = "99999999"; // above 1h cap + expect(resolveSubagentTimeoutMs()).toBe(DEFAULT_SUBAGENT_TIMEOUT_MS); + saveStoredConfig({ subagentTimeoutMs: 500 }); // stored below floor + expect(resolveSubagentTimeoutMs()).toBe(DEFAULT_SUBAGENT_TIMEOUT_MS); + }); +}); \ No newline at end of file diff --git a/src/config/config.ts b/src/config/config.ts index a3ae880..e0cb27d 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -3,6 +3,7 @@ import { DEFAULT_CONTEXT_WINDOW, DEFAULT_MAX_ITERATIONS, DEFAULT_REQUEST_TIMEOUT_MS, + DEFAULT_SUBAGENT_TIMEOUT_MS, KNOWN_BACKENDS, type BackendName, } from "./defaults.js"; @@ -24,13 +25,19 @@ export function resolveBackendConfig(cliOpts: CliBackendOpts): { const backend = cliOpts.backend ?? process.env.LOCODE_BACKEND ?? stored.backend ?? "ollama"; const explicitBaseUrl = cliOpts.baseUrl ?? process.env.LOCODE_BASE_URL ?? stored.baseUrl; + // Allow any backend name when an explicit base URL is provided; only the built-in names have a + // default URL, so a custom name without --base-url is still an error. if (backend !== "ollama" && backend !== "lmstudio" && !explicitBaseUrl) { throw new ConfigError( `Unknown backend "${backend}". Use --backend ollama|lmstudio, or pass --base-url for a custom endpoint.`, ); } - const baseURL = explicitBaseUrl ?? KNOWN_BACKENDS[backend as BackendName]; + const knownBaseURL = (KNOWN_BACKENDS as Record)[backend]; + const baseURL = explicitBaseUrl ?? knownBaseURL; + if (!baseURL) { + throw new ConfigError(`No base URL configured for backend "${backend}".`); + } return { backendName: backend, baseURL }; } @@ -58,6 +65,18 @@ export function resolveMaxIterations(): number { return DEFAULT_MAX_ITERATIONS; } +/** Wall-clock budget for a single sub-agent turn (see DEFAULT_SUBAGENT_TIMEOUT_MS). Bounded to + * 1s–1h to reject pathological values. */ +export function resolveSubagentTimeoutMs(): number { + const stored = loadStoredConfig(); + const envValue = Number(process.env.LOCODE_SUBAGENT_TIMEOUT_MS); + if (Number.isFinite(envValue) && envValue >= 1_000 && envValue <= 3_600_000) return envValue; + if (typeof stored.subagentTimeoutMs === "number" && stored.subagentTimeoutMs >= 1_000 && stored.subagentTimeoutMs <= 3_600_000) { + return stored.subagentTimeoutMs; + } + return DEFAULT_SUBAGENT_TIMEOUT_MS; +} + /** Fraction of the context window at which locode auto-compacts the conversation. */ export function resolveAutoCompactThreshold(): number { const stored = loadStoredConfig(); diff --git a/src/config/defaults.ts b/src/config/defaults.ts index 73eb7c4..fc52d89 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -26,3 +26,12 @@ export const DEFAULT_AUTO_COMPACT_THRESHOLD = 0.85; * backend/client.ts). Raise this via `requestTimeoutMs` if your backend queues requests behind a * concurrency limit (e.g. Ollama's `OLLAMA_NUM_PARALLEL`) rather than serving them immediately. */ export const DEFAULT_REQUEST_TIMEOUT_MS = 180_000; + +/** Wall-clock budget for a single sub-agent turn. Sub-agents make their own sequence of model + * requests (one per file read, grep, etc.), and on a cloud backend each request has real network + * latency on top of generation time — so a sub-agent reading 20-30 files can legitimately take + * several minutes. The old 120s hardcoded cap timed those out mid-task (the parent would see + * "Sub-agent timed out" and give up on delegating). The per-request idle guard + * (DEFAULT_REQUEST_TIMEOUT_MS) still catches a single hung request; this bounds only the whole + * sub-agent turn. Configurable via `LOCODE_SUBAGENT_TIMEOUT_MS`. */ +export const DEFAULT_SUBAGENT_TIMEOUT_MS = 600_000; diff --git a/src/config/store.ts b/src/config/store.ts index 7f788ea..ba01c45 100644 --- a/src/config/store.ts +++ b/src/config/store.ts @@ -14,25 +14,38 @@ export interface StoredConfig { autoCompactThreshold?: number; /** Milliseconds to wait on a single chat completion request before giving up. */ requestTimeoutMs?: number; + /** Milliseconds of wall-clock budget for a single sub-agent turn. */ + subagentTimeoutMs?: number; } const paths = envPaths("locode", { suffix: "" }); const configFile = path.join(paths.config, "config.json"); +// Test-only override for the config file path so tests can point load/save at an isolated temp +// file instead of clobbering the user's real ~/.config/locode/config.json. Undefined in prod. +let configFileOverride: string | undefined; + export function configFilePath(): string { - return configFile; + return configFileOverride ?? configFile; +} + +/** @internal For tests only — redirect config persistence to `path` (pass undefined to reset). */ +export function _setConfigFilePathForTest(p: string | undefined): void { + configFileOverride = p; } export function loadStoredConfig(): StoredConfig { - if (!existsSync(configFile)) return {}; + const file = configFilePath(); + if (!existsSync(file)) return {}; try { - return JSON.parse(readFileSync(configFile, "utf-8")) as StoredConfig; + return JSON.parse(readFileSync(file, "utf-8")) as StoredConfig; } catch { return {}; } } export function saveStoredConfig(cfg: StoredConfig): void { - mkdirSync(paths.config, { recursive: true }); - writeFileSync(configFile, JSON.stringify(cfg, null, 2)); -} + const file = configFilePath(); + mkdirSync(path.dirname(file), { recursive: true }); + writeFileSync(file, JSON.stringify(cfg, null, 2)); +} \ No newline at end of file diff --git a/src/hooks/runner.ts b/src/hooks/runner.ts index 8b0ba3f..026845c 100644 --- a/src/hooks/runner.ts +++ b/src/hooks/runner.ts @@ -1,7 +1,7 @@ import { execa } from "execa"; +import path from "node:path"; 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; @@ -49,14 +49,48 @@ function parseOutput(output: string, schema?: "json"): { text?: string; json?: u } } +function splitCommand(command: string): string[] { + // Simple, safe splitting for hook commands: split on whitespace, respecting single and double quotes. + // This avoids shell: true while still allowing spaces inside quoted arguments. + const parts: string[] = []; + let current = ""; + let quote: string | null = null; + for (let i = 0; i < command.length; i++) { + const ch = command[i]!; + if (quote) { + if (ch === quote) { + quote = null; + } else { + current += ch; + } + } else if (ch === '"' || ch === "'") { + quote = ch; + } else if (/\s/.test(ch)) { + if (current) { + parts.push(current); + current = ""; + } + } else { + current += ch; + } + } + if (current) parts.push(current); + return parts; +} + async function runCommandHook( hook: HookCommand, stdinPayload: string, ctx: HookRunContext, ): Promise<{ exitCode: number; stdout: string; stderr: string; timedOut: boolean; json?: unknown; warning?: string }> { try { - const result = await execa(hook.command, { - shell: resolveShell(), + const argv = splitCommand(hook.command); + if (argv.length === 0) { + return { exitCode: 1, stdout: "", stderr: "Empty hook command.", timedOut: false }; + } + const commandName = argv[0]!; + const args = argv.slice(1); + const result = await execa(commandName, args, { cwd: ctx.cwd, input: stdinPayload, timeout: (hook.timeout ?? DEFAULT_TIMEOUT_SECONDS) * 1000, @@ -76,11 +110,35 @@ async function runCommandHook( } } +function isPrivateOrLocalUrl(url: URL): boolean { + const hostname = url.hostname; + if (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1") return true; + if (hostname.startsWith("192.168.") || hostname.startsWith("10.") || hostname.startsWith("172.")) { + const secondOctet = Number(hostname.split(".")[1]); + if (hostname.startsWith("172.") && (secondOctet < 16 || secondOctet > 31)) return false; + return true; + } + return false; +} + async function runHttpHook( hook: HookHttp, stdinPayload: string, ctx: HookRunContext, ): Promise<{ exitCode: number; stdout: string; stderr: string; timedOut: boolean; json?: unknown; warning?: string }> { + let url: URL; + try { + url = new URL(hook.url); + } catch { + return { exitCode: 1, stdout: "", stderr: `Invalid hook URL: ${hook.url}`, timedOut: false }; + } + if (url.protocol !== "http:" && url.protocol !== "https:") { + return { exitCode: 1, stdout: "", stderr: `Unsupported hook URL scheme: ${url.protocol}`, timedOut: false }; + } + if (isPrivateOrLocalUrl(url)) { + return { exitCode: 1, stdout: "", stderr: `Hook URL points to a private/local address: ${hook.url}`, timedOut: false }; + } + const controller = new AbortController(); const timeoutMs = (hook.timeout ?? DEFAULT_TIMEOUT_SECONDS) * 1000; const timeoutId = setTimeout(() => controller.abort(), timeoutMs); @@ -93,12 +151,13 @@ async function runHttpHook( headers["content-type"] = "application/json"; body = stdinPayload; } - const res = await fetch(hook.url, { method, headers, body, signal: controller.signal }); + const res = await fetch(url, { method, headers, body, signal: controller.signal }); const text = await res.text(); - clearTimeout(timeoutId); const parsed = parseOutput(text, hook.outputSchema); return { - exitCode: res.ok ? 0 : 2, + // HTTP status alone must not block events — only explicit exit-code semantics do. A flaky + // monitoring hook returning 500 should be a warning, not a denial-of-service. + exitCode: res.ok ? 0 : 1, stdout: text, stderr: res.ok ? "" : `HTTP ${res.status} ${res.statusText}`, timedOut: false, @@ -106,7 +165,6 @@ async function runHttpHook( warning: parsed.warning, }; } catch (err) { - clearTimeout(timeoutId); const timedOut = err instanceof Error && err.name === "AbortError"; return { exitCode: timedOut ? 1 : 1, @@ -114,6 +172,8 @@ async function runHttpHook( stderr: timedOut ? "HTTP hook timed out." : (err as Error).message, timedOut, }; + } finally { + clearTimeout(timeoutId); } } diff --git a/src/mcp/client.ts b/src/mcp/client.ts index a7e155e..88ebcf4 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -70,6 +70,14 @@ export async function connectMcpServer(name: string, config: McpServerConfig): P 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[] }; + try { + const { tools } = await client.listTools(); + return { name, client, transport, tools: tools as McpToolInfo[] }; + } catch (err) { + // listTools failed (server connected but never responded to the listing) — close the + // transport so the stdio subprocess / HTTP connection isn't orphaned. manager.ts catches + // the rejection as an error status, but without this the child process keeps running. + await transport.close().catch(() => {}); + throw err; + } } diff --git a/src/mcp/manager.ts b/src/mcp/manager.ts index 179b56f..73a7f81 100644 --- a/src/mcp/manager.ts +++ b/src/mcp/manager.ts @@ -20,7 +20,16 @@ let statuses: McpServerStatus[] = []; export async function connectConfiguredMcpServers(cwd: string): Promise { const { servers, collisions } = loadMergedServers(cwd); const entries = Object.entries(servers); - if (entries.length === 0) return []; + + // Clear stale state up front: if the user removes all servers we must not leave old statuses + // and connections behind in the UI / process tree. + statuses = []; + if (entries.length === 0) { + const old = connections; + connections = []; + await Promise.allSettled(old.map((c) => c.transport.close())); + return []; + } const results = await Promise.allSettled(entries.map(([name, config]) => connectMcpServer(name, config))); @@ -42,10 +51,18 @@ export async function connectConfiguredMcpServers(cwd: string): Promise c.name === name), }); } else { - nextStatuses.push({ name, status: "error", toolCount: 0, error: (result.reason as Error).message }); + const reason = result.reason; + const errorMessage = reason instanceof Error ? reason.message : String(reason ?? "unknown error"); + nextStatuses.push({ name, status: "error", toolCount: 0, error: errorMessage }); } }); + // Close any previously-connected servers before swapping in the new set, so a reconnect (or + // any second call to this function) doesn't orphan the old transports / stdio subprocesses. + // Best-effort: a failed close shouldn't prevent the new connections from being adopted. + if (connections.length > 0) { + await Promise.allSettled(connections.map((c) => c.transport.close())); + } statuses = nextStatuses; connections = nextConnections; return allTools; diff --git a/src/mcp/toolAdapter.test.ts b/src/mcp/toolAdapter.test.ts index 275566c..226e7ad 100644 --- a/src/mcp/toolAdapter.test.ts +++ b/src/mcp/toolAdapter.test.ts @@ -14,10 +14,10 @@ describe("mcpToolToToolDef", () => { annotations: { readOnlyHint: true }, }; - it("namespaces the tool name", () => { + it("namespaces the tool name and treats all MCP tools as mutating", () => { const def = mcpToolToToolDef("my-server", fakeClient({ content: [] }), tool); expect(def.name).toBe("mcp__my-server__sample"); - expect(def.mutating).toBe(false); + expect(def.mutating).toBe(true); }); it("returns text blocks verbatim", async () => { @@ -27,11 +27,17 @@ describe("mcpToolToToolDef", () => { expect(result).toEqual({ content: "hello" }); }); - it("returns an image in the read_file-compatible shape", async () => { + it("returns an image in the read_file-compatible shape with real byte length", async () => { const client = fakeClient({ content: [{ type: "image", mimeType: "image/png", data: "base64data" }] }); const def = mcpToolToToolDef("s", client, tool); const result = await def.handler({}, { cwd: "/tmp" }); - expect(result).toMatchObject({ image: true, mimeType: "image/png", base64: "base64data", content: expect.stringContaining("image/png") }); + expect(result).toMatchObject({ + image: true, + mimeType: "image/png", + base64: "base64data", + bytes: 7, + content: expect.stringContaining("image/png"), + }); }); it("returns audio as a text note", async () => { diff --git a/src/mcp/toolAdapter.ts b/src/mcp/toolAdapter.ts index f1919b4..d70aae4 100644 --- a/src/mcp/toolAdapter.ts +++ b/src/mcp/toolAdapter.ts @@ -7,13 +7,61 @@ function sanitize(id: string): string { return id.replace(/[^a-zA-Z0-9_-]/g, "_"); } +function base64ByteLength(base64: string): number { + const stripped = base64.replace(/=/g, ""); + return Math.floor((stripped.length * 3) / 4); +} + /** MCP tool names are namespaced as `mcp____` (matching the convention Claude Code * itself uses) so tools from different servers can't collide with each other or with local tools. */ export function mcpToolName(serverName: string, toolName: string): string { return `mcp__${sanitize(serverName)}__${sanitize(toolName)}`; } -const argsSchema = z.record(z.string(), z.unknown()); +const fallbackArgsSchema = z.record(z.string(), z.unknown()); + +/** Best-effort JSON Schema -> Zod conversion for MCP tool input schemas, covering the shapes MCP + * servers actually emit (object/properties/required, enum, array/items, primitive types). Anything + * unrecognized falls back to z.unknown() rather than rejecting, since this only needs to catch + * missing/mistyped required params before they hit the server, not fully validate every schema. */ +function jsonSchemaToZodType(schema: unknown): z.ZodTypeAny { + if (!schema || typeof schema !== "object") return z.unknown(); + const s = schema as Record; + if (Array.isArray(s.enum) && s.enum.length > 0) { + return z.literal(s.enum as (string | number | boolean)[]); + } + switch (s.type) { + case "string": + return z.string(); + case "number": + return z.number(); + case "integer": + return z.number().int(); + case "boolean": + return z.boolean(); + case "null": + return z.null(); + case "array": + return z.array(s.items ? jsonSchemaToZodType(s.items) : z.unknown()); + case "object": { + const props = (s.properties ?? {}) as Record; + const required = new Set(Array.isArray(s.required) ? (s.required as string[]) : []); + const shape: Record = {}; + for (const [key, propSchema] of Object.entries(props)) { + const propType = jsonSchemaToZodType(propSchema); + shape[key] = required.has(key) ? propType : propType.optional(); + } + return z.looseObject(shape); + } + default: + return z.unknown(); + } +} + +function buildArgsSchema(rawInputSchema: Record | undefined): z.ZodType> { + if (!rawInputSchema || rawInputSchema.type !== "object") return fallbackArgsSchema; + return jsonSchemaToZodType(rawInputSchema) as z.ZodType>; +} function renderContentBlock(block: McpContentBlock): { text: string; image?: { mimeType: string; base64: string } } { switch (block.type) { @@ -43,10 +91,11 @@ export function mcpToolToToolDef(serverName: string, client: Client, mcpTool: Mc return { name: mcpToolName(serverName, mcpTool.name), description: `[MCP: ${serverName}] ${mcpTool.description ?? mcpTool.name}`, - schema: argsSchema, + schema: buildArgsSchema(mcpTool.inputSchema), rawInputSchema: mcpTool.inputSchema, - // MCP tools can do anything server-side; only trust the spec's readOnlyHint to skip confirmation. - mutating: mcpTool.annotations?.readOnlyHint !== true, + // MCP tools can do anything server-side. The spec's readOnlyHint is advisory and could be + // wrong (or malicious), so we treat every MCP tool as mutating and require confirmation. + mutating: true, preview: async (args) => "```json\n" + JSON.stringify(args, null, 2) + "\n```", handler: async (args) => { const result = (await client.callTool({ name: mcpTool.name, arguments: args })) as McpCallToolResult; @@ -68,9 +117,16 @@ export function mcpToolToToolDef(serverName: string, client: Client, mcpTool: Mc // the conversation as a real image_url content part for vision-capable models. If multiple // images come back, the first one is sent as a real image and the rest stay as text notes. if (images.length >= 1) { - return { content: text, image: true, mimeType: images[0]!.mimeType, base64: images[0]!.base64, bytes: 0 }; + const base64 = images[0]!.base64; + return { + content: text, + image: true, + mimeType: images[0]!.mimeType, + base64, + bytes: base64ByteLength(base64), + }; } return { content: text }; - }, + } }; } diff --git a/src/permissions/permissionManager.ts b/src/permissions/permissionManager.ts index a985c98..a6384bb 100644 --- a/src/permissions/permissionManager.ts +++ b/src/permissions/permissionManager.ts @@ -15,7 +15,10 @@ export class PermissionManager { /** Check whether a mutating tool should be auto-approved (no confirmation needed). */ isAutoApproved(toolName: string): boolean { - if (this.mode === "auto-accept") return true; + // auto-accept only covers the same file-edit tools as auto-edit, not arbitrary mutating tools + // such as bash or git_commit. This prevents a user who intended "approve edits" from silently + // approving every dangerous operation. + if (this.mode === "auto-accept" && AUTO_EDIT_TOOLS.has(toolName)) return true; if (this.mode === "auto-edit" && AUTO_EDIT_TOOLS.has(toolName)) return true; // "default" — check session-allowed list return this.allowedForSession.has(toolName); diff --git a/src/permissions/types.ts b/src/permissions/types.ts index 6246c18..173cfc6 100644 --- a/src/permissions/types.ts +++ b/src/permissions/types.ts @@ -1,6 +1,6 @@ export type PermissionDecision = "once" | "session" | "deny"; -export type PermissionMode = "default" | "auto-edit" | "auto-accept"; +export type PermissionMode = "default" | "auto-edit" | "auto-accept" | "plan"; /** Tools that are auto-accepted in "auto-edit" mode. */ export const AUTO_EDIT_TOOLS = new Set(["write_file", "edit_file"]); @@ -9,4 +9,9 @@ export type ConfirmFn = (opts: { toolName: string; args: unknown; preview?: string; + /** When set, the prompt should be dismissed and the promise rejected if this signal aborts — + * used by sub-agent turns so a sub-agent that times out while awaiting user confirmation + * doesn't leave a stale prompt that could still approve a mutation after the parent has + * already given up on the turn. Undefined for top-level turns (no timeout to abort on). */ + signal?: AbortSignal; }) => Promise; diff --git a/src/plugins/toolNameMap.ts b/src/plugins/toolNameMap.ts index 404e661..713a06f 100644 --- a/src/plugins/toolNameMap.ts +++ b/src/plugins/toolNameMap.ts @@ -13,6 +13,7 @@ const CLAUDE_TOOL_NAME_MAP: Record = { webfetch: "web_fetch", websearch: "web_search", task: "agent", + todowrite: "todo_write", }; export function resolveToolName(name: string): string { diff --git a/src/toolcalling/resolve.test.ts b/src/toolcalling/resolve.test.ts new file mode 100644 index 0000000..ccf1452 --- /dev/null +++ b/src/toolcalling/resolve.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import { z } from "zod"; +import { resolveToolInvocation } from "./resolve.js"; +import type { ToolDef } from "../tools/types.js"; + +function makeTool(schema: z.ZodType): ToolDef { + return { name: "t", description: "d", schema, mutating: false, handler: async () => ({}) }; +} + +describe("resolveToolInvocation — Ollama empty-key recovery", () => { + it("parses valid args normally without invoking recovery", () => { + const tool = makeTool(z.object({ path: z.string() })); + const r = resolveToolInvocation("t", { path: "src/x.ts" }, new Map([["t", tool]])); + expect("tool" in r).toBe(true); + if ("tool" in r) expect((r.args as { path: string }).path).toBe("src/x.ts"); + }); + + it("recovers an empty-key value into the single missing required field", () => { + // Mirrors read_file's schema: one required (path) + optional offset/limit. Ollama emits {"": "..."}. + const tool = makeTool( + z.object({ path: z.string(), offset: z.number().int().min(1).optional(), limit: z.number().int().min(1).max(2000).optional() }), + ); + const r = resolveToolInvocation("t", { "": "src/tools/readFile.test.ts" }, new Map([["t", tool]])); + expect("tool" in r).toBe(true); + if ("tool" in r) expect((r.args as { path: string }).path).toBe("src/tools/readFile.test.ts"); + }); + + it("still rejects when more than one required field is missing", () => { + const tool = makeTool(z.object({ a: z.string(), b: z.string() })); + const r = resolveToolInvocation("t", { "": "x" }, new Map([["t", tool]])); + expect("error" in r).toBe(true); + }); + + it("does not need recovery when the required field is already present (extra '' is stripped)", () => { + const tool = makeTool(z.object({ path: z.string() })); + const r = resolveToolInvocation("t", { path: "ok", "": "ignored" }, new Map([["t", tool]])); + expect("tool" in r).toBe(true); + }); + + it("returns an error for an unknown tool", () => { + const r = resolveToolInvocation("nope", {}, new Map()); + expect(r).toEqual({ error: "unknown tool: nope" }); + }); +}); \ No newline at end of file diff --git a/src/toolcalling/resolve.ts b/src/toolcalling/resolve.ts index e41f4b9..95ac6f1 100644 --- a/src/toolcalling/resolve.ts +++ b/src/toolcalling/resolve.ts @@ -1,7 +1,32 @@ +import { z } from "zod"; import type { ToolContext, ToolDef } from "../tools/types.js"; export type ResolvedToolCall = { tool: ToolDef; args: unknown } | { error: string }; +function isPlainObject(v: unknown): v is Record { + return typeof v === "object" && v !== null && !Array.isArray(v); +} + +/** Some local-model backends (notably Ollama's native tool-calling) occasionally emit a tool + * call's argument under an empty key — `{"": "src/foo.ts"}` — instead of the parameter name, + * even when the schema they were given is correct. If exactly one required parameter is missing + * and there's an empty-key value, move that value across to the missing field so the call still + * works instead of failing every read_file/grep/etc. with "invalid arguments: ... path expected + * string, received undefined". Narrow on purpose: only the literal empty-string key, only a single + * missing required field, so it can't paper over a genuinely wrong call. */ +function recoverEmptyKeyArgs(tool: ToolDef, rawArgs: Record): Record | null { + const jsonSchema = tool.rawInputSchema ?? (z.toJSONSchema(tool.schema) as Record); + const required = Array.isArray(jsonSchema.required) ? (jsonSchema.required as string[]) : []; + const missing = required.filter((k) => !(k in rawArgs)); + if (missing.length === 1 && "" in rawArgs) { + const recovered = { ...rawArgs }; + recovered[missing[0]!] = recovered[""]; + delete recovered[""]; + return recovered; + } + return null; +} + export function resolveToolInvocation( name: string, rawArgs: unknown, @@ -12,10 +37,18 @@ export function resolveToolInvocation( return { error: `unknown tool: ${name}` }; } const validated = tool.schema.safeParse(rawArgs); - if (!validated.success) { - return { error: `invalid arguments: ${JSON.stringify(validated.error.issues)}` }; + if (validated.success) { + return { tool, args: validated.data }; } - return { tool, args: validated.data }; + // Ollama empty-key workaround (see recoverEmptyKeyArgs) — try it only on the failure path. + const recovered = isPlainObject(rawArgs) && "" in rawArgs ? recoverEmptyKeyArgs(tool, rawArgs) : null; + if (recovered) { + const retry = tool.schema.safeParse(recovered); + if (retry.success) { + return { tool, args: retry.data }; + } + } + return { error: `invalid arguments: ${JSON.stringify(validated.error.issues)}` }; } export async function runTool(tool: ToolDef, args: unknown, ctx: ToolContext): Promise { @@ -24,4 +57,4 @@ export async function runTool(tool: ToolDef, args: unknown, ctx: ToolContext): P } catch (err) { return { error: (err as Error).message ?? String(err) }; } -} +} \ No newline at end of file diff --git a/src/tools/agentTool.ts b/src/tools/agentTool.ts index 8c7a18a..89f3f69 100644 --- a/src/tools/agentTool.ts +++ b/src/tools/agentTool.ts @@ -17,7 +17,13 @@ export const agentTool: ToolDef> = { "Delegate a self-contained task to a fresh sub-agent with its own tool loop (read_file, list_files, grep, " + "web_search, web_fetch, write_file, edit_file, bash — but not another agent). Use for multi-step research or " + "exploration you want to run in isolation, getting back only the final answer so your own context stays " + - "focused. Only the sub-agent's final text is returned to you, not its intermediate steps.", + "focused. Only the sub-agent's final text is returned to you, not its intermediate steps. For a task that " + + "needs reading many files (more than a handful), prefer dispatching several sub-agents — each scanning a " + + "subset — over reading them all yourself: every tool call you make yourself counts toward your per-turn step " + + "budget, so a large scan done inline will pause mid-task, while sub-agents run their own loops and report " + + "back in a single step each. A sub-agent also has its own (smaller) step budget and no memory across calls — " + + "if one reports it ran out of steps, don't retry it with the same prompt (it'll hit the same wall); split the " + + "task into narrower sub-agents instead.", schema, mutating: false, handler: async (args, ctx) => { diff --git a/src/tools/backgroundJobs.test.ts b/src/tools/backgroundJobs.test.ts index 79c4792..0398ba9 100644 --- a/src/tools/backgroundJobs.test.ts +++ b/src/tools/backgroundJobs.test.ts @@ -1,11 +1,23 @@ import { EventEmitter } from "node:events"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { ResultPromise } from "execa"; +import { killProcessTree } from "../utils/processTree.js"; import { getBackgroundJob, killBackgroundJob, killAllBackgroundJobs, registerBackgroundJob } from "./backgroundJobs.js"; +// tree-kill walks the real OS process tree (ps / taskkill); mock it so the tests stay hermetic and +// don't need real pids. The mock records the call and, to mimic a real tree-kill actually +// terminating the child, runs a per-pid "kill" callback the fake child registers for its pid. +const { killCallbacks } = vi.hoisted(() => ({ killCallbacks: new Map void>() })); +vi.mock("../utils/processTree.js", () => ({ + killProcessTree: vi.fn(async (pid: number) => { + killCallbacks.get(pid)?.(); + }), +})); + /** Minimal stand-in for execa's `ResultPromise` — only the surface registerBackgroundJob/kill - * actually touch (stdout/stderr streams, `.kill()`, and resolving/rejecting like a promise). */ -function fakeChild() { + * actually touch (stdout/stderr streams, `.kill()`, `.pid`, and resolving/rejecting like a + * promise). */ +function fakeChild(pid = 1000) { const stdout = new EventEmitter(); const stderr = new EventEmitter(); let resolve!: (v: { exitCode: number | null; signal?: string | null }) => void; @@ -13,18 +25,29 @@ function fakeChild() { resolve = res; }); const kill = vi.fn(() => resolve({ exitCode: null, signal: "SIGTERM" })); - const child = Object.assign(promise, { stdout, stderr, kill }) as unknown as ResultPromise; + // Register the kill callback the mocked killProcessTree invokes for this pid, so a tree-kill + // resolves the fake child exactly like a real one dying from the signal. + killCallbacks.set(pid, kill); + const child = Object.assign(promise, { stdout, stderr, kill, pid }) as unknown as ResultPromise; return { child, stdout, stderr, kill, resolveExit: resolve }; } describe("backgroundJobs", () => { + beforeEach(() => { + vi.mocked(killProcessTree).mockClear(); + killCallbacks.clear(); + }); + it("kills a running job and reports the signal once it exits", async () => { const { child, kill } = fakeChild(); const job = registerBackgroundJob("sleep 100", "/tmp", child, "", ""); - const result = killBackgroundJob(job.id); + const result = await killBackgroundJob(job.id); expect(result.ok).toBe(true); + // killBackgroundJob tree-kills by pid rather than calling child.kill() directly — the mocked + // killProcessTree invokes the fake child's kill, which is what actually resolves it. + expect(killProcessTree).toHaveBeenCalledWith(child.pid); expect(kill).toHaveBeenCalledOnce(); await Promise.resolve(child).then(() => {}); // let the child.then() handler in registerBackgroundJob run @@ -33,9 +56,10 @@ describe("backgroundJobs", () => { expect(getBackgroundJob(job.id)?.signal).toBe("SIGTERM"); }); - it("errors on an unknown job id", () => { - const result = killBackgroundJob("bg-does-not-exist"); + it("errors on an unknown job id", async () => { + const result = await killBackgroundJob("bg-does-not-exist"); expect(result).toEqual({ ok: false, error: 'No background job with id "bg-does-not-exist".' }); + expect(killProcessTree).not.toHaveBeenCalled(); }); it("errors when killing a job that already finished", async () => { @@ -44,19 +68,22 @@ describe("backgroundJobs", () => { resolveExit({ exitCode: 0 }); await new Promise((r) => setImmediate(r)); - const result = killBackgroundJob(job.id); + const result = await killBackgroundJob(job.id); expect(result).toEqual({ ok: false, error: `Job "${job.id}" has already finished.` }); + expect(killProcessTree).not.toHaveBeenCalled(); }); - it("killAllBackgroundJobs kills every still-running job", () => { - const a = fakeChild(); - const b = fakeChild(); + it("killAllBackgroundJobs kills every still-running job", async () => { + const a = fakeChild(1001); + const b = fakeChild(1002); registerBackgroundJob("cmd-a", "/tmp", a.child, "", ""); registerBackgroundJob("cmd-b", "/tmp", b.child, "", ""); - killAllBackgroundJobs(); + await killAllBackgroundJobs(); + expect(killProcessTree).toHaveBeenCalledWith(a.child.pid); + expect(killProcessTree).toHaveBeenCalledWith(b.child.pid); expect(a.kill).toHaveBeenCalledOnce(); expect(b.kill).toHaveBeenCalledOnce(); }); @@ -78,4 +105,4 @@ describe("backgroundJobs", () => { vi.useRealTimers(); } }); -}); +}); \ No newline at end of file diff --git a/src/tools/backgroundJobs.ts b/src/tools/backgroundJobs.ts index cc9127f..d112afa 100644 --- a/src/tools/backgroundJobs.ts +++ b/src/tools/backgroundJobs.ts @@ -1,4 +1,5 @@ import type { ResultPromise } from "execa"; +import { killProcessTree } from "../utils/processTree.js"; import { truncate } from "../utils/truncate.js"; // Generous cap on buffered output per stream — well above the 20k default `truncate()` applies @@ -103,19 +104,26 @@ export function getBackgroundJob(id: string): BackgroundJob | undefined { } /** Kills a still-running backgrounded job. Returns an error message instead of throwing, since - * the caller (the `bash_kill` tool) surfaces it directly as a tool result. */ -export function killBackgroundJob(id: string): { ok: true } | { ok: false; error: string } { + * the caller (the `bash_kill` tool) surfaces it directly as a tool result. Tree-kills so a + * command that spawned children (npm run dev → dev server, a watcher, a pipeline) takes its + * descendants with it instead of leaving them orphaned — `child.kill()` only signals the direct + * child, which for `npm run dev` is just `npm`. */ +export async function killBackgroundJob(id: string): Promise<{ ok: true } | { ok: false; error: string }> { const job = jobs.get(id); if (!job) return { ok: false, error: `No background job with id "${id}".` }; if (job.status === "done") return { ok: false, error: `Job "${id}" has already finished.` }; const child = children.get(id); if (!child) return { ok: false, error: `Job "${id}" has no live process to kill.` }; - child.kill(); + if (child.pid) await killProcessTree(child.pid); return { ok: true }; } /** Best-effort kill of every still-running job, called once as locode itself is exiting so - * backgrounded shells (dev servers, watch builds) don't outlive the process as orphans. */ -export function killAllBackgroundJobs(): void { - for (const child of children.values()) child.kill(); + * backgrounded shells (dev servers, watch builds) don't outlive the process as orphans. Awaits + * the tree-kill so the signals are actually delivered before locode tears down — without + * awaiting, a fast exit could orphan the helper's own subprocess mid-walk. */ +export async function killAllBackgroundJobs(): Promise { + await Promise.all( + [...children.values()].map((child) => (child.pid ? killProcessTree(child.pid) : Promise.resolve())), + ); } diff --git a/src/tools/bash.test.ts b/src/tools/bash.test.ts new file mode 100644 index 0000000..645640f --- /dev/null +++ b/src/tools/bash.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ResultPromise } from "execa"; +import { bashTool } from "./bash.js"; +import { killProcessTree } from "../utils/processTree.js"; +import type { ToolContext } from "./types.js"; + +// Hermetic mocks so the abort path can be exercised without spawning real processes or touching +// the real OS process tree. execa returns a fake child that never exits on its own (it only +// resolves when "killed"); killProcessTree invokes that fake child's kill when called, mimicking a +// real tree-kill actually terminating the child. +const { killCallbacks, makeFakeChild } = vi.hoisted(() => { + const killCallbacks = new Map void>(); + let pidCounter = 5000; + function makeFakeChild() { + let resolve!: (v: { exitCode: number | null; signal?: string | null }) => void; + const promise = new Promise<{ exitCode: number | null; signal?: string | null }>((res) => { + resolve = res; + }); + const pid = pidCounter++; + const kill = () => resolve({ exitCode: null, signal: "SIGTERM" }); + killCallbacks.set(pid, kill); + // bash only calls stdout/stderr on/off("data", ...); plain no-op stubs are enough. + const stdout = { on: () => {}, off: () => {} }; + const stderr = { on: () => {}, off: () => {} }; + return { child: Object.assign(promise, { stdout, stderr, kill, pid }) as unknown as ResultPromise, kill, pid }; + } + return { killCallbacks, makeFakeChild }; +}); + +vi.mock("../utils/processTree.js", () => ({ + killProcessTree: vi.fn(async (pid: number) => { + killCallbacks.get(pid)?.(); + }), +})); +vi.mock("execa", () => ({ + execa: vi.fn(() => makeFakeChild().child), +})); + +describe("bash tool — sub-agent abort", () => { + beforeEach(() => { + vi.mocked(killProcessTree).mockClear(); + killCallbacks.clear(); + }); + + it("tree-kills the child when the turn's abort signal fires mid-command", async () => { + const ac = new AbortController(); + const ctx: ToolContext = { cwd: process.cwd(), signal: ac.signal }; + const resultPromise = bashTool.handler({ command: "sleep 30" }, ctx); + + // Let the poll loop spin a little so we're genuinely mid-command, then abort — mimics a + // sub-agent timing out while a bash call is in flight. + await new Promise((r) => setTimeout(r, 50)); + ac.abort(); + + const result = (await resultPromise) as { exitCode: number | null; signal?: string | null; timedOut?: boolean }; + + // The abort path tree-killed the child (not the 30s foreground timer), unblocking the poll loop + // so the abandoned sub-agent's runTurn can reject on its next iteration instead of hanging. + expect(killProcessTree).toHaveBeenCalledTimes(1); + expect(result.timedOut).toBe(false); + // exitCode null (not 0) because the child was killed by the abort signal, not a clean exit. + expect(result.exitCode).toBeNull(); + }); + + it("does not kill a command the user backgrounded when the abort fires later", async () => { + // Once a command is moved to the background registry, the abort listener is removed — the user + // chose to keep it running, so a sub-agent timeout (or any later abort) must not kill it; the + // background registry + killAllBackgroundJobs own its lifetime instead. + const ac = new AbortController(); + const ctx: ToolContext = { cwd: process.cwd(), signal: ac.signal, backgroundControl: { requested: false } }; + const resultPromise = bashTool.handler({ command: "sleep 30" }, ctx); + + await new Promise((r) => setTimeout(r, 30)); + ctx.backgroundControl!.requested = true; // Ctrl+B + const bgResult = (await resultPromise) as { backgrounded?: boolean; jobId?: string }; + expect(bgResult.backgrounded).toBe(true); + + ac.abort(); + await new Promise((r) => setTimeout(r, 30)); + + expect(killProcessTree).not.toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/src/tools/bash.ts b/src/tools/bash.ts index 5c92ee4..1807e6d 100644 --- a/src/tools/bash.ts +++ b/src/tools/bash.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { execa } from "execa"; import { z } from "zod"; import { registerBackgroundJob } from "./backgroundJobs.js"; +import { killProcessTree } from "../utils/processTree.js"; import { truncate } from "../utils/truncate.js"; import { resolveShell } from "../utils/shell.js"; import type { ToolDef } from "./types.js"; @@ -45,38 +46,61 @@ export const bashTool: ToolDef> = { let timedOut = false; let foregroundTimer: ReturnType | undefined = setTimeout(() => { timedOut = true; - child.kill(); + // Tree-kill so a command that spawned children (npm run dev, a watcher, a pipeline) takes + // its descendants with it instead of orphaning them on timeout. + if (child.pid) void killProcessTree(child.pid); }, timeout_ms ?? 30_000); + // Don't let this timer alone keep the process alive until it fires — without this, a sub-agent + // that times out mid-command (and any future path that doesn't cancel the timer) would hang + // locode's exit for up to `timeout_ms` waiting for the timer to fire. It still fires normally + // during a turn, since the poll loop below keeps the event loop alive regardless. + foregroundTimer.unref?.(); - // Poll instead of a single `await child` so a mid-flight Ctrl+B (which flips - // ctx.backgroundControl.requested) can detach the process into the background registry - // without waiting for it to finish first. - for (;;) { - if (ctx.backgroundControl?.requested) { - clearTimeout(foregroundTimer); - // Detach our own capture listeners before handing the streams to the background registry — - // otherwise both this closure's listeners and registerBackgroundJob's keep appending to - // separate buffers forever, doubling the work and growing memory without bound for a - // long-running backgrounded job. The buffers captured so far seed the job. - child.stdout?.off("data", onStdout); - child.stderr?.off("data", onStderr); - const job = registerBackgroundJob(command, workDir, child, stdout, stderr); - return { - backgrounded: true, - jobId: job.id, - message: `Command moved to the background (job ${job.id}). Use bash_output with this jobId to check on it.`, - }; - } - const settled = await Promise.race([child, delay(BACKGROUND_POLL_MS)]); - if (settled !== "pending") { - clearTimeout(foregroundTimer); - return { - exitCode: settled.exitCode, - stdout: truncate(stdout), - stderr: truncate(stderr), - timedOut, - }; + // A sub-agent turn that times out while this command is running aborts `ctx.signal` — kill the + // child tree immediately so the command can't keep running (and keep the event loop alive on + // exit) after the parent has already abandoned the turn. Fire-and-forget: the child dying is + // what unblocks the poll loop below, not this promise resolving. + const onAbort = () => { + if (child.pid) void killProcessTree(child.pid); + }; + ctx.signal?.addEventListener("abort", onAbort, { once: true }); + + try { + // Poll instead of a single `await child` so a mid-flight Ctrl+B (which flips + // ctx.backgroundControl.requested) can detach the process into the background registry + // without waiting for it to finish first. + for (;;) { + if (ctx.backgroundControl?.requested) { + clearTimeout(foregroundTimer); + // Detach our own capture listeners before handing the streams to the background registry — + // otherwise both this closure's listeners and registerBackgroundJob's keep appending to + // separate buffers forever, doubling the work and growing memory without bound for a + // long-running backgrounded job. The buffers captured so far seed the job. + child.stdout?.off("data", onStdout); + child.stderr?.off("data", onStderr); + const job = registerBackgroundJob(command, workDir, child, stdout, stderr); + return { + backgrounded: true, + jobId: job.id, + message: `Command moved to the background (job ${job.id}). Use bash_output with this jobId to check on it.`, + }; + } + const settled = await Promise.race([child, delay(BACKGROUND_POLL_MS)]); + if (settled !== "pending") { + clearTimeout(foregroundTimer); + return { + exitCode: settled.exitCode, + stdout: truncate(stdout), + stderr: truncate(stderr), + timedOut, + }; + } } + } finally { + // Remove the abort listener on every exit path. On backgrounding this is what stops a + // sub-agent timeout from killing a job the user explicitly chose to keep running; on normal + // completion it's just cleanup. (The listener is `{ once: true }`, but it may never fire.) + ctx.signal?.removeEventListener("abort", onAbort); } }, -}; +}; \ No newline at end of file diff --git a/src/tools/bashKill.ts b/src/tools/bashKill.ts index f03153d..1ef951c 100644 --- a/src/tools/bashKill.ts +++ b/src/tools/bashKill.ts @@ -13,7 +13,7 @@ export const bashKillTool: ToolDef> = { mutating: true, preview: async ({ jobId }) => `Kill background job ${jobId}`, handler: async ({ jobId }) => { - const result = killBackgroundJob(jobId); + const result = await killBackgroundJob(jobId); if (!result.ok) return { error: result.error }; return { message: `Sent kill signal to job ${jobId}.` }; }, diff --git a/src/tools/editFile.ts b/src/tools/editFile.ts index 1d4d135..acb1390 100644 --- a/src/tools/editFile.ts +++ b/src/tools/editFile.ts @@ -17,7 +17,11 @@ function countOccurrences(haystack: string, needle: string): number { } function applyEdit(original: string, oldString: string, newString: string, replaceAll?: boolean): string { - return replaceAll ? original.split(oldString).join(newString) : original.replace(oldString, newString); + // Use split/join for both paths instead of String.prototype.replace, whose replacement string + // interprets special $-tokens ($$, $&, $`, $', $, $1–$9) even when the *pattern* is a plain + // string — which would silently corrupt edits whose replacement text contains a literal "$". + // split/join inserts the replacement verbatim. + return replaceAll ? original.split(oldString).join(newString) : original.replace(oldString, () => newString); } export const editFileTool: ToolDef> = { diff --git a/src/tools/git.test.ts b/src/tools/git.test.ts index 6f60ade..ad7ff07 100644 --- a/src/tools/git.test.ts +++ b/src/tools/git.test.ts @@ -18,10 +18,10 @@ function lastGitArgs(): string[] { } describe("gitCommitTool", () => { - it("adds specific paths", async () => { + it("adds specific paths with -- separator", async () => { mockGitResponse(""); await gitCommitTool.handler({ operation: "add", paths: ["src/a.ts"] }, { cwd: "/repo" }); - expect(lastGitArgs()).toEqual(["add", "src/a.ts"]); + expect(lastGitArgs()).toEqual(["add", "--", "src/a.ts"]); }); it("adds all changes when no paths are given", async () => { @@ -33,10 +33,18 @@ describe("gitCommitTool", () => { it("commits with a message", async () => { mockGitResponse("[main abc1234] msg"); const result = await gitCommitTool.handler({ operation: "commit", message: "msg" }, { cwd: "/repo" }); - expect(lastGitArgs()).toEqual(["commit", "-m", "msg"]); + expect(lastGitArgs()).toEqual(["commit", "-m", "msg", "--"]); expect(result).toEqual({ output: "[main abc1234] msg" }); }); + it("rejects a message starting with '-'", async () => { + await expect(gitCommitTool.handler({ operation: "commit", message: "--evil" }, { cwd: "/repo" })).rejects.toThrow(); + }); + + it("rejects paths starting with '-'", async () => { + await expect(gitCommitTool.handler({ operation: "add", paths: ["-f"] }, { cwd: "/repo" })).rejects.toThrow(); + }); + it("checks out a branch", async () => { mockGitResponse(""); await gitCommitTool.handler({ operation: "checkout", branchName: "feature" }, { cwd: "/repo" }); diff --git a/src/tools/git.ts b/src/tools/git.ts index 5c28c87..1a24801 100644 --- a/src/tools/git.ts +++ b/src/tools/git.ts @@ -86,8 +86,16 @@ const commitSchema = z.object({ "rebase", "delete_branch", ]), - paths: z.array(z.string()).optional().describe("For `add`: paths to stage. For `stash push`: paths to stash. Omit to stage all changes."), - message: z.string().optional().describe("For `commit`: commit message. For `stash push`: stash message."), + paths: z.array(z.string()).optional() + .describe("For `add`: paths to stage. For `stash push`: paths to stash. Omit to stage all changes.") + .refine((arr) => !arr?.some((p) => p.startsWith("-")), { + message: 'paths must not start with "-" (would be parsed as a git flag)', + }), + message: z + .string() + .optional() + .describe("For `commit`: commit message. For `stash push`: stash message.") + .refine((v) => !v?.startsWith("-"), { message: 'message must not start with "-" (would be parsed as a git flag)' }), branchName: refLike("Branch name. Required for create_branch/checkout/merge/rebase/delete_branch."), createIfMissing: z.boolean().optional().describe("For `checkout`: create the branch if it doesn't exist yet (like `checkout -b`)."), remote: refLike('For `push`: remote name (default "origin").'), @@ -220,10 +228,10 @@ async function runMutatingOperation(args: z.infer, cwd: str const gitArgs: string[] = (() => { switch (operation) { case "add": - return ["add", ...(paths?.length ? paths : ["-A"])]; + return paths?.length ? ["add", "--", ...paths] : ["add", "-A"]; case "commit": { if (!message) throw new Error("`message` is required for the commit operation."); - return ["commit", "-m", message]; + return ["commit", "-m", message, "--"]; } case "create_branch": { if (!branchName) throw new Error("`branchName` is required for the create_branch operation."); diff --git a/src/tools/index.ts b/src/tools/index.ts index 3123a7e..29b93e4 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -7,6 +7,7 @@ import { gitCommitTool, gitStatusTool } from "./git.js"; import { grepTool } from "./grep.js"; import { listFilesTool } from "./listFiles.js"; import { readFileTool } from "./readFile.js"; +import { todoWriteTool } from "./todoWrite.js"; import { webFetchTool } from "./webFetch.js"; import { webSearchTool } from "./webSearch.js"; import { writeFileTool } from "./writeFile.js"; @@ -25,6 +26,7 @@ export const TOOLS: ToolDef[] = [ bashOutputTool, bashKillTool, gitCommitTool, + todoWriteTool, agentTool, ]; diff --git a/src/tools/listFiles.ts b/src/tools/listFiles.ts index 43b2a9f..8895fc4 100644 --- a/src/tools/listFiles.ts +++ b/src/tools/listFiles.ts @@ -3,6 +3,11 @@ import fg from "fast-glob"; import { z } from "zod"; import type { ToolDef } from "./types.js"; +function withinWorkspace(resolved: string, workspace: string): boolean { + const rel = path.relative(workspace, resolved); + return !rel.startsWith("..") && !path.isAbsolute(rel); +} + const schema = z.object({ pattern: z.string().describe("Glob pattern, e.g. 'src/**/*.ts'."), cwd: z.string().optional().describe("Directory to search from, relative to the working directory."), @@ -17,6 +22,9 @@ export const listFilesTool: ToolDef> = { mutating: false, handler: async ({ pattern, cwd }, ctx) => { const base = cwd ? path.resolve(ctx.cwd, cwd) : ctx.cwd; + if (!withinWorkspace(base, ctx.cwd)) { + throw new Error(`Search directory ${cwd} resolves outside the workspace.`); + } const matches = await fg(pattern, { cwd: base, dot: false, diff --git a/src/tools/readFile.test.ts b/src/tools/readFile.test.ts new file mode 100644 index 0000000..1642248 --- /dev/null +++ b/src/tools/readFile.test.ts @@ -0,0 +1,75 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { readFileTool } from "./readFile.js"; + +interface ReadResult { + totalLines: number; + content: string; + truncated?: boolean; + nextOffset?: number; +} + +describe("read_file pagination", () => { + let dir: string; + beforeEach(() => { + dir = mkdtempSync(path.join(tmpdir(), "locode-read-")); + }); + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + function file(name: string, content: string): string { + const p = path.join(dir, name); + writeFileSync(p, content, "utf-8"); + return p; + } + + it("returns a small file whole with no truncation", async () => { + const p = file("small.txt", "line1\nline2\nline3"); + const result = (await readFileTool.handler({ path: p }, { cwd: dir })) as ReadResult; + expect(result.totalLines).toBe(3); + expect(result.content).toBe("1\tline1\n2\tline2\n3\tline3"); + expect(result.truncated).toBeUndefined(); + expect(result.nextOffset).toBeUndefined(); + }); + + it("truncates a large file on a line boundary and reports the next offset to page with", async () => { + // ~1000 lines of ~36 chars each → ~36k chars, over the 20k cap. The whole point: the result + // must tell the model how to get the rest (nextOffset + an in-content hint), instead of a bare + // "... [truncated N more characters]" that led the model to re-read the same path in a loop. + const lines = Array.from({ length: 1000 }, (_, i) => `line-${i}-${"x".repeat(30)}`); + const p = file("big.txt", lines.join("\n")); + const result = (await readFileTool.handler({ path: p }, { cwd: dir })) as ReadResult; + + expect(result.truncated).toBe(true); + expect(typeof result.nextOffset).toBe("number"); + expect(result.nextOffset!).toBeGreaterThan(1); + expect(result.nextOffset!).toBeLessThan(1000); + // The in-content hint names the exact offset to continue from. + expect(result.content).toContain(`offset=${result.nextOffset}`); + // Cut on a line boundary — the last real line is followed by the truncation notice, not a + // half-line chopped mid-content. + expect(result.content).toMatch(/\n\n\.\.\. \[truncated — \d+ more line\(s\)/); + }); + + it("reads the next page when called with the reported offset", async () => { + const lines = Array.from({ length: 1000 }, (_, i) => `line-${i}-${"x".repeat(30)}`); + const p = file("big.txt", lines.join("\n")); + const page1 = (await readFileTool.handler({ path: p }, { cwd: dir })) as ReadResult; + expect(page1.truncated).toBe(true); + + const page2 = (await readFileTool.handler({ path: p, offset: page1.nextOffset }, { cwd: dir })) as ReadResult; + // Page 2 picks up exactly where page 1 stopped — first numbered line is nextOffset. + expect(page2.content.startsWith(`${page1.nextOffset}\t`)).toBe(true); + }); + + it("respects the limit argument", async () => { + const lines = Array.from({ length: 100 }, (_, i) => `line-${i}`); + const p = file("limited.txt", lines.join("\n")); + const result = (await readFileTool.handler({ path: p, limit: 5 }, { cwd: dir })) as ReadResult; + expect(result.content.split("\n")).toHaveLength(5); + expect(result.truncated).toBeUndefined(); + }); +}); \ No newline at end of file diff --git a/src/tools/readFile.ts b/src/tools/readFile.ts index 608fce2..ea3a59b 100644 --- a/src/tools/readFile.ts +++ b/src/tools/readFile.ts @@ -1,10 +1,21 @@ import { readFile as fsReadFile, stat as fsStat } from "node:fs/promises"; import path from "node:path"; import { z } from "zod"; -import { truncate } from "../utils/truncate.js"; import { imageMimeType, MAX_IMAGE_BYTES } from "../utils/image.js"; import type { ToolDef } from "./types.js"; +function withinWorkspace(resolved: string, workspace: string): boolean { + const rel = path.relative(workspace, resolved); + return !rel.startsWith("..") && !path.isAbsolute(rel); +} + +// Cap on how much text a single read_file call returns, so a huge file can't blow up the context +// in one call. Cut on a line boundary (never mid-line) and report the exact next offset, so the +// model can page through the rest with `offset` instead of re-reading the same truncated prefix in +// a loop — which is what happened before, when the generic "... [truncated N more characters]" +// notice never mentioned offset and the model just re-issued the same call. +const MAX_READ_CHARS = 20_000; + const schema = z.object({ path: z.string().describe("File path to read, relative to the working directory or absolute."), offset: z.number().int().min(1).optional().describe("1-indexed line number to start reading from (text files only)."), @@ -15,12 +26,17 @@ export const readFileTool: ToolDef> = { name: "read_file", description: "Read a file from the local filesystem. Text files return content with 1-indexed line numbers, optionally a specific " + - "line range. Image files (png, jpg, jpeg, gif, webp, bmp) are returned as image content for the model to see directly " + - "— this requires a vision-capable model/backend; others may error on the request.", + "line range. Very large files are paginated: the result is capped and reports `nextOffset` — call read_file again with " + + "that offset to read the next page rather than re-reading the same path. Image files (png, jpg, jpeg, gif, webp, bmp) " + + "are returned as image content for the model to see directly — this requires a vision-capable model/backend; others may " + + "error on the request.", schema, mutating: false, handler: async ({ path: filePath, offset, limit }, ctx) => { const resolved = path.resolve(ctx.cwd, filePath); + if (!withinWorkspace(resolved, ctx.cwd)) { + throw new Error(`File ${filePath} resolves outside the workspace.`); + } const mimeType = imageMimeType(resolved); if (mimeType) { @@ -37,9 +53,29 @@ export const readFileTool: ToolDef> = { const content = await fsReadFile(resolved, "utf-8"); const lines = content.split("\n"); const start = offset ? offset - 1 : 0; - const end = limit ? start + limit : lines.length; + const requestedEnd = limit ? Math.min(start + limit, lines.length) : lines.length; + + // Accumulate whole lines until the next line would push past the char cap. The first line is + // always included even if it alone exceeds the cap (a single minified 200k-char line, say) — + // paging within a line isn't possible with offset/limit, so there's nothing better to do there. + let end = start; + let chars = 0; + while (end < requestedEnd) { + const lineLen = `${end + 1}\t${lines[end]}\n`.length; + if (end > start && chars + lineLen > MAX_READ_CHARS) break; + chars += lineLen; + end++; + } const slice = lines.slice(start, end); const numbered = slice.map((line, i) => `${start + i + 1}\t${line}`).join("\n"); - return { path: resolved, totalLines: lines.length, content: truncate(numbered) }; + const hasMore = end < requestedEnd; + return { + path: resolved, + totalLines: lines.length, + content: hasMore + ? `${numbered}\n\n... [truncated — ${requestedEnd - end} more line(s) in range; call read_file again with offset=${end + 1} to read the next page]` + : numbered, + ...(hasMore ? { truncated: true, nextOffset: end + 1 } : {}), + }; }, -}; +}; \ No newline at end of file diff --git a/src/tools/todoWrite.test.ts b/src/tools/todoWrite.test.ts new file mode 100644 index 0000000..4ed1392 --- /dev/null +++ b/src/tools/todoWrite.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it, vi } from "vitest"; +import { todoWriteTool } from "./todoWrite.js"; + +describe("todo_write", () => { + it("is non-mutating (no confirmation prompt)", () => { + expect(todoWriteTool.mutating).toBe(false); + }); + + it("forwards the full list to ctx.setTodos and echoes it back", async () => { + const setTodos = vi.fn(); + const todos = [ + { content: "read the config", status: "completed" as const }, + { content: "write the fix", status: "in_progress" as const }, + { content: "run tests", status: "pending" as const }, + ]; + const result = await todoWriteTool.handler({ todos }, { cwd: "/tmp", setTodos }); + expect(setTodos).toHaveBeenCalledWith(todos); + expect(result).toEqual({ todos }); + }); + + it("doesn't throw when setTodos is absent from the context", async () => { + const todos = [{ content: "a task", status: "pending" as const }]; + await expect(todoWriteTool.handler({ todos }, { cwd: "/tmp" })).resolves.toEqual({ todos }); + }); +}); diff --git a/src/tools/todoWrite.ts b/src/tools/todoWrite.ts new file mode 100644 index 0000000..4c0b01e --- /dev/null +++ b/src/tools/todoWrite.ts @@ -0,0 +1,29 @@ +import { z } from "zod"; +import type { ToolDef } from "./types.js"; + +const todoItemSchema = z.object({ + content: z.string().describe("Short description of the task."), + status: z.enum(["pending", "in_progress", "completed"]), +}); + +const schema = z.object({ + todos: z + .array(todoItemSchema) + .describe("The full current checklist — this replaces whatever list existed before, so include every item, not just the ones that changed."), +}); + +export const todoWriteTool: ToolDef> = { + name: "todo_write", + description: + "Show the user a task checklist for multi-step work (roughly 3+ distinct steps) so progress stays visible. " + + "Mark exactly one item 'in_progress' at a time, mark an item 'completed' as soon as it's actually done (don't " + + "batch completions), and always pass the full list, not a diff. Skip this for single-step or trivial requests.", + schema, + // Purely informational (like Claude Code's TodoWrite) — never touches the filesystem or asks + // the user anything, so it shouldn't interrupt the flow with a confirmation prompt. + mutating: false, + handler: async (args, ctx) => { + ctx.setTodos?.(args.todos); + return { todos: args.todos }; + }, +}; diff --git a/src/tools/types.ts b/src/tools/types.ts index ca6444b..8cd6f09 100644 --- a/src/tools/types.ts +++ b/src/tools/types.ts @@ -17,14 +17,27 @@ export interface SubAgentOverrides { toolNames?: string[]; } +export interface TodoItem { + content: string; + status: "pending" | "in_progress" | "completed"; +} + export interface ToolContext { cwd: string; /** Only present when running inside a session capable of spawning sub-agents (used by the `agent` tool). */ runSubAgent?: (task: SubAgentTask, overrides?: SubAgentOverrides) => Promise; + /** Replaces the session's task checklist (used by the `todo_write` tool). Absent only if a + * future tool context is built without one — every session-backed context provides it. */ + setTodos?: (todos: TodoItem[]) => void; /** Set only while this specific call is a backgroundable tool (currently just `bash`) — the tool * polls `requested` and, once true, detaches into the background job registry instead of * awaiting completion. Absent for tools that don't support backgrounding. */ backgroundControl?: { requested: boolean }; + /** Abort signal for the turn running this tool call. Currently only `bash` consumes it: a + * sub-agent that times out mid-command aborts this signal, and bash kills its child process + * tree so the command can't keep running (and keep the event loop alive on exit) after the + * parent has already abandoned the turn. Undefined for top-level turns. */ + signal?: AbortSignal; } export interface ToolDef { diff --git a/src/tools/webFetch.ts b/src/tools/webFetch.ts index 32a8d34..455f55a 100644 --- a/src/tools/webFetch.ts +++ b/src/tools/webFetch.ts @@ -9,6 +9,13 @@ const schema = z.object({ const MAX_CHARS = 20_000; +function isBinaryContentType(contentType: string): boolean { + const type = contentType.toLowerCase(); + if (type.includes("html") || type.includes("text") || type.includes("json") || type.includes("xml")) return false; + if (type.includes("javascript") || type.includes("typescript") || type.includes("css")) return false; + return true; +} + export const webFetchTool: ToolDef> = { name: "web_fetch", description: @@ -34,9 +41,20 @@ export const webFetchTool: ToolDef> = { redirect: "follow", }); if (!response.ok) { - throw new Error(`Fetch failed with status ${response.status}`); + const body = await response.text().catch(() => ""); + throw new Error(`Fetch failed with status ${response.status}: ${response.statusText}${body ? `\n${truncate(body, 500)}` : ""}`); } const contentType = response.headers.get("content-type") ?? ""; + if (isBinaryContentType(contentType)) { + const buffer = Buffer.from(await response.arrayBuffer()); + return { + url: parsed.toString(), + finalUrl: response.url, + status: response.status, + contentType, + content: `[Binary content (${contentType}, ${buffer.byteLength} bytes) — not shown]`, + }; + } const body = await response.text(); const content = contentType.includes("html") ? htmlToText(body) : body; diff --git a/src/ui/ink/App.tsx b/src/ui/ink/App.tsx index 46facd4..e31a53c 100644 --- a/src/ui/ink/App.tsx +++ b/src/ui/ink/App.tsx @@ -37,6 +37,7 @@ import { getGitInfo, type GitInfo } from "../../utils/gitInfo.js"; import { findSkillCollisions } from "../../plugins/skillTool.js"; import { buildImportContent } from "../../utils/importFile.js"; import { extractMentionedFiles } from "../../utils/mentions.js"; +import { loadProjectInstructions } from "../../utils/projectInstructions.js"; import { deriveTitle, listSessions, @@ -61,6 +62,15 @@ 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; +// Shared between /perm's explicit-cycle notice and Shift+Tab's cyclePermMode so the two paths to +// the same action can't drift out of sync with each other. +const PERM_MODE_LABELS: Record = { + default: "default (ask before mutating tools)", + plan: "plan (research only — all mutating tools blocked)", + "auto-edit": "auto-edit (file edits auto-approved, bash still asks)", + "auto-accept": "auto-accept (all tools auto-approved ⚠)", +}; + export interface AppProps { baseURL: string; cwd: string; @@ -121,7 +131,11 @@ export function App({ // Throttle streaming text updates to ~30fps to avoid excessive re-renders const streamingAccumulatorRef = useRef(""); const lastStreamRenderRef = useRef(0); - const streamRafRef = useRef(null); + const streamRafRef = useRef | null>(null); + // Stable ref for the current phase so the global useInput handler can read it without being + // re-registered on every phase change. + const phaseRef = useRef(phase); + phaseRef.current = phase; const flushStreamingText = useCallback(() => { const accumulated = streamingAccumulatorRef.current; @@ -165,6 +179,9 @@ export function App({ // Mounted for the whole App lifetime (unlike ChatInput's own useInput, which only exists while // ChatInput is rendered) so both shortcuts work even mid-turn, when ChatInput is unmounted. useInput((input, key) => { + // Only react to global shortcuts during the actual chat phase; ignore them while a modal + // (permission/export) or a non-input phase (model/session select, connecting) is open. + if (phaseRef.current !== "input" || permission || exportPrompt) return; if (key.ctrl && input === "o") { const summary = lastCompactSummaryRef.current; push({ @@ -204,6 +221,26 @@ export function App({ }); }, [push]); + // Ollama/LM Studio don't report a usable context length for every model (notably cloud-routed + // models, e.g. "*:cloud" tags, whose metadata isn't the local GGUF info /api/show expects) — when + // that happens contextWindow silently falls back to a small hardcoded default (see + // backend/contextWindow.ts), which makes auto-compaction trigger far more often than the model's + // real limit would require, burning extra summarization round-trips. Surface it so the user can + // set the real value instead of silently eating that cost every session. + const notifyIfContextWindowGuessed = useCallback( + (model: string, contextWindow: { value: number; isEstimate: boolean }) => { + if (!contextWindow.isEstimate) return; + push({ + kind: "notice", + text: + `Could not detect "${model}"'s real context window — using a default of ${contextWindow.value} tokens, ` + + `which may be much smaller than its actual limit and cause auto-compaction to trigger too often. ` + + `Set it manually with: locode config set contextWindow `, + }); + }, + [push], + ); + const initSession = useCallback( async (model: string) => { setPhase("connecting"); @@ -216,7 +253,11 @@ export function App({ new Promise((resolve) => { setPermission({ ...opts, resolve }); }); - const [extraTools, contextWindow] = await Promise.all([extraToolsPromise, resolveContextWindow(baseURLRef.current, model)]); + const [extraTools, contextWindow, projectInstructions] = await Promise.all([ + extraToolsPromise, + resolveContextWindow(baseURLRef.current, model), + loadProjectInstructions(cwd), + ]); sessionRef.current = createSession( client, model, @@ -228,8 +269,11 @@ export function App({ contextWindow.isEstimate, resolveMaxIterations(), resolveAutoCompactThreshold(), + projectInstructions, ); push({ kind: "banner", cwd, model, backend: baseURLRef.current }); + if (projectInstructions) push({ kind: "notice", text: "Loaded project instructions from CLAUDE.md/AGENTS.md." }); + notifyIfContextWindowGuessed(model, contextWindow); for (const warning of await fireSessionStartHook(sessionRef.current)) { push({ kind: "notice", text: `Hook warning: ${warning}` }); } @@ -247,7 +291,7 @@ export function App({ } } }, - [cwd, toolModeOverride, modelList, exit, push, extraToolsPromise], + [cwd, toolModeOverride, modelList, exit, push, extraToolsPromise, notifyIfContextWindowGuessed], ); const initSessionFromRecord = useCallback( @@ -260,9 +304,10 @@ export function App({ new Promise((resolve) => { setPermission({ ...opts, resolve }); }); - const [extraTools, contextWindow] = await Promise.all([ + const [extraTools, contextWindow, projectInstructions] = await Promise.all([ extraToolsPromise, resolveContextWindow(record.baseURL, record.model), + loadProjectInstructions(cwd), ]); sessionRef.current = createSessionFromRecord( client, @@ -274,6 +319,7 @@ export function App({ contextWindow.isEstimate, resolveMaxIterations(), resolveAutoCompactThreshold(), + projectInstructions, ); push({ kind: "banner", @@ -282,6 +328,7 @@ export function App({ backend: record.baseURL, resumedTitle: deriveTitle(record.messages), }); + notifyIfContextWindowGuessed(record.model, contextWindow); for (const warning of await fireSessionStartHook(sessionRef.current)) { push({ kind: "notice", text: `Hook warning: ${warning}` }); } @@ -304,11 +351,20 @@ export function App({ fetchModelsForPicker(); } }, - [cwd, push, fetchModelsForPicker, extraToolsPromise], + [cwd, push, fetchModelsForPicker, extraToolsPromise, notifyIfContextWindowGuessed], ); // Decide the startup path once on mount: resume a specific session, show a resume // picker, jump straight to a given model, or fall back to the model picker. + // + // Deliberately depends only on the primitive props that decide *which* path to take, not on + // initSession/initSessionFromRecord/fetchModelsForPicker themselves — those are useCallbacks whose + // own deps include React state (e.g. initSession depends on `modelList`, which fetchModelsForPicker + // sets). Including them here used to mean: fetchModelsForPicker sets modelList -> initSession gets a + // new identity -> this effect's deps change -> it re-fires -> fetchModelsForPicker/initSession run + // again -> modelList changes again -> ... an infinite loop that reconnected (pushing a fresh welcome + // banner each time) and/or re-fetched the model list forever, which is exactly what "run once on + // mount" was never supposed to do. useEffect(() => { if (resumeSessionId) { const record = loadSession(resumeSessionId); @@ -339,7 +395,7 @@ export function App({ } fetchModelsForPicker(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [resumeSessionId, interactiveResume, initialModel]); async function handleModelSelect(model: string) { await initSession(model); @@ -375,6 +431,7 @@ export function App({ session.contextWindow = newContextWindow.value; session.contextWindowIsEstimate = newContextWindow.isEstimate; push({ kind: "notice", text: `Switched model to "${name}" (tool-call mode: ${newMode}).` }); + notifyIfContextWindowGuessed(name, newContextWindow); persistCurrentSession(); runHooksForEvent("ConfigChange", { sessionId: session.id, cwd }, { key: "model", previous: previousModel, value: name }).catch( () => {}, @@ -407,6 +464,7 @@ export function App({ kind: "notice", text: `Switched backend to "${name}" (${baseURLRef.current}, tool-call mode: ${newMode}).`, }); + notifyIfContextWindowGuessed(session.model, newContextWindow); persistCurrentSession(); runHooksForEvent("ConfigChange", { sessionId: session.id, cwd }, { key: "backend", previous: previousBaseURL, value: baseURLRef.current }).catch( () => {}, @@ -438,7 +496,7 @@ export function App({ setStreamingText(streamingAccumulatorRef.current); } else { if (streamRafRef.current === null) { - streamRafRef.current = setTimeout(flushStreamingText, 33) as any; + streamRafRef.current = setTimeout(flushStreamingText, 33); } } } else if (event.type === "text_done") { @@ -477,6 +535,8 @@ export function App({ setStaticItems((prev) => [...prev, { id: nextId(), kind: "tool_result", summary: event.summary, isError: event.isError } as HistoryItem]); } else if (event.type === "hook_notice" || event.type === "notice") { setStaticItems((prev) => [...prev, { id: nextId(), kind: "notice", text: event.text, isError: event.isError } as HistoryItem]); + } else if (event.type === "todos_update") { + setStaticItems((prev) => [...prev, { id: nextId(), kind: "todos", todos: event.todos } as HistoryItem]); } }); // text_done already added the assistant message to staticItems @@ -528,6 +588,10 @@ export function App({ setIsThinking(false); setStreamingText(null); streamingAccumulatorRef.current = ""; + if (streamRafRef.current !== null) { + clearTimeout(streamRafRef.current); + streamRafRef.current = null; + } } } @@ -686,27 +750,31 @@ export function App({ } if (trimmed.startsWith("/perm")) { const name = trimmed.slice("/perm".length).trim(); - const validModes: Record = { default: "default", "auto-edit": "auto-edit", "auto-accept": "auto-accept" }; + const validModes: Record = { + default: "default", + plan: "plan", + "auto-edit": "auto-edit", + "auto-accept": "auto-accept", + }; if (!name) { // Cycle - const modes: PermissionMode[] = ["default", "auto-edit", "auto-accept"]; + const modes: PermissionMode[] = ["default", "plan", "auto-edit", "auto-accept"]; const currentIdx = modes.indexOf(permMode); const nextMode = modes[(currentIdx + 1) % modes.length]!; setPermMode(nextMode); session.permissions.setMode(nextMode); - const labels: Record = { - default: "default (ask before mutating tools)", - "auto-edit": "auto-edit (file edits auto-approved, bash still asks)", - "auto-accept": "auto-accept (all tools auto-approved ⚠)", - }; - push({ kind: "notice", text: `Permission mode: ${labels[nextMode]}` }); + push({ kind: "notice", text: `Permission mode: ${PERM_MODE_LABELS[nextMode]}` }); } else if (validModes[name]) { const newMode = validModes[name]; setPermMode(newMode); session.permissions.setMode(newMode); push({ kind: "notice", text: `Permission mode set to "${newMode}".` }); } else { - push({ kind: "notice", text: `Unknown permission mode "${name}". Use "default", "auto-edit", or "auto-accept".`, isError: true }); + push({ + kind: "notice", + text: `Unknown permission mode "${name}". Use "default", "plan", "auto-edit", or "auto-accept".`, + isError: true, + }); } return; } @@ -827,17 +895,12 @@ export function App({ function cyclePermMode() { const session = sessionRef.current; if (!session) return; - const modes: PermissionMode[] = ["default", "auto-edit", "auto-accept"]; + const modes: PermissionMode[] = ["default", "plan", "auto-edit", "auto-accept"]; const currentIdx = modes.indexOf(permMode); const nextMode = modes[(currentIdx + 1) % modes.length]!; setPermMode(nextMode); session.permissions.setMode(nextMode); - const labels: Record = { - default: "default (ask before mutating tools)", - "auto-edit": "auto-edit (file edits auto-approved, bash still asks)", - "auto-accept": "auto-accept (all tools auto-approved ⚠)", - }; - push({ kind: "notice", text: `Permission mode: ${labels[nextMode]}` }); + push({ kind: "notice", text: `Permission mode: ${PERM_MODE_LABELS[nextMode]}` }); } return ( @@ -874,7 +937,7 @@ export function App({ ) : ( - {sessionRef.current && phase === "input" && ( + {sessionRef.current && phaseRef.current === "input" && ( f.toLowerCase().includes(mention.query.toLowerCase())) + .filter((f) => f.toLowerCase().includes(query.toLowerCase())) .sort((a, b) => a.length - b.length) .slice(0, MAX_MATCHES) : []; useEffect(() => { setSelectedIndex(0); - }, [mention?.query]); + }, [query]); // Keeps the selection centered in the visible window where possible, clamped so the window // never scrolls past either end of the match list. diff --git a/src/ui/ink/HistoryItemView.tsx b/src/ui/ink/HistoryItemView.tsx index 7a28300..266087e 100644 --- a/src/ui/ink/HistoryItemView.tsx +++ b/src/ui/ink/HistoryItemView.tsx @@ -8,7 +8,7 @@ const HELP_LINES = [ " /model switch the model used for the current backend", " /backend switch backend (ollama | lmstudio), keeps current model", " /mode view or force tool-call mode (native | fallback)", - " /perm [mode] cycle or set permission mode (default | auto-edit | auto-accept)", + " /perm [mode] cycle or set permission mode (default | plan | auto-edit | auto-accept)", " /status show current model, backend, tool-call mode, and cwd", " /dashboard show session stats: token I/O, elapsed/model time, turns, tool calls", " /tools list available tools", @@ -198,6 +198,25 @@ export function HistoryItemView({ item }: { item: HistoryItem }) { ); + case "todos": { + const icon = { pending: "☐", in_progress: "◐", completed: "☑" } as const; + const color = { pending: undefined, in_progress: "cyan", completed: "green" } as const; + return ( + + {item.todos.length === 0 ? ( + Todos: (cleared) + ) : ( + item.todos.map((t, i) => ( + + {" "} + {icon[t.status]} {t.content} + + )) + )} + + ); + } + case "help": return ( diff --git a/src/ui/ink/StatusBar.tsx b/src/ui/ink/StatusBar.tsx index b547bea..9c8a50f 100644 --- a/src/ui/ink/StatusBar.tsx +++ b/src/ui/ink/StatusBar.tsx @@ -8,6 +8,7 @@ const MODE_LABELS: Record = { default: "default", "auto-edit": "auto-edit", "auto-accept": "auto-accept", + plan: "plan", }; // Distinct per mode so cycling modes (Shift+Tab / /perm) is visibly reflected here — default and @@ -16,6 +17,7 @@ const MODE_COLORS: Record = { default: "gray", "auto-edit": "green", "auto-accept": "yellowBright", + plan: "cyan", }; interface Props { diff --git a/src/ui/ink/confirmFn.test.ts b/src/ui/ink/confirmFn.test.ts new file mode 100644 index 0000000..4c2ba34 --- /dev/null +++ b/src/ui/ink/confirmFn.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi } from "vitest"; +import { makeConfirmFn, type PendingPermission } from "./confirmFn.js"; + +/** A minimal stand-in for App's `setPermission` state setter that records every value it's called + * with, so a test can assert the prompt was set and/or dismissed. */ +function makeSetter() { + const calls: (PendingPermission | null)[] = []; + const setPermission = (p: PendingPermission | null) => { + calls.push(p); + }; + return { setPermission, calls }; +} + +function opts(signal?: AbortSignal) { + return { toolName: "edit_file", args: { path: "a.txt" }, preview: "diff", signal }; +} + +describe("makeConfirmFn — abort-aware permission prompt", () => { + it("sets the prompt and resolves with the user's decision when there is no signal", async () => { + const { setPermission, calls } = makeSetter(); + const confirm = makeConfirmFn(setPermission); + + const promise = confirm(opts()); + expect(calls).toHaveLength(1); + expect(calls[0]?.toolName).toBe("edit_file"); + + // The user approves. + calls[0]?.resolve("once"); + await expect(promise).resolves.toBe("once"); + // Prompt is dismissed after the user decides. + expect(calls.at(-1)).toBeNull(); + }); + + it("rejects and dismisses the prompt when the signal aborts while awaiting a decision", async () => { + const { setPermission, calls } = makeSetter(); + const ac = new AbortController(); + const confirm = makeConfirmFn(setPermission); + + const promise = confirm(opts(ac.signal)); + expect(calls).toHaveLength(1); // prompt shown + + ac.abort(); // sub-agent timed out mid-confirm + + await expect(promise).rejects.toThrow(/cancelled/); + // The stale prompt must be dismissed so the user can't later approve a mutation the parent + // already abandoned — the whole point of the abort-aware confirm. + expect(calls.at(-1)).toBeNull(); + }); + + it("rejects immediately if the signal has already aborted when confirm is called", async () => { + const { setPermission, calls } = makeSetter(); + const ac = new AbortController(); + ac.abort(); + const confirm = makeConfirmFn(setPermission); + + await expect(confirm(opts(ac.signal))).rejects.toThrow(/cancelled/); + // Never even showed the prompt. + expect(calls).toHaveLength(1); + expect(calls[0]).toBeNull(); + }); + + it("does not reject if the user decides before the signal aborts", async () => { + const { setPermission, calls } = makeSetter(); + const ac = new AbortController(); + const confirm = makeConfirmFn(setPermission); + + const promise = confirm(opts(ac.signal)); + // User approves first. + calls[0]?.resolve("session"); + // Then the turn aborts later — must not turn the already-resolved approval into a rejection. + ac.abort(); + + await expect(promise).resolves.toBe("session"); + }); + + it("removes its abort listener once the user decides (no dangling listener)", async () => { + const { setPermission, calls } = makeSetter(); + const ac = new AbortController(); + const removeSpy = vi.spyOn(ac.signal, "removeEventListener"); + const confirm = makeConfirmFn(setPermission); + + const promise = confirm(opts(ac.signal)); + calls[0]?.resolve("once"); + await promise; + + expect(removeSpy).toHaveBeenCalledWith("abort", expect.any(Function)); + }); +}); \ No newline at end of file diff --git a/src/ui/ink/confirmFn.ts b/src/ui/ink/confirmFn.ts new file mode 100644 index 0000000..148adca --- /dev/null +++ b/src/ui/ink/confirmFn.ts @@ -0,0 +1,47 @@ +import type { ConfirmFn, PermissionDecision } from "../../permissions/types.js"; + +export interface PendingPermission { + toolName: string; + args: unknown; + preview?: string; + resolve: (decision: PermissionDecision) => void; +} + +/** Builds the confirm function a session uses to prompt for mutating-tool approval. If `signal` is + * supplied (a sub-agent turn, which has a wall-clock timeout), the prompt is dismissed and the + * promise rejected when that signal aborts — so a sub-agent that times out while awaiting user + * confirmation can't leave a stale prompt that the user could still click to approve a mutation + * after the parent has already given up on the turn. Top-level turns pass no signal, so their + * prompts behave exactly as before (no timeout to abort on). */ +export function makeConfirmFn(setPermission: (p: PendingPermission | null) => void): ConfirmFn { + return (opts) => + new Promise((resolve, reject) => { + const onAbort = () => { + setPermission(null); + reject(new Error("Permission prompt cancelled (the turn was aborted).")); + }; + if (opts.signal?.aborted) { + onAbort(); + return; + } + const pending: PendingPermission = { + toolName: opts.toolName, + args: opts.args, + preview: opts.preview, + resolve: (decision) => { + opts.signal?.removeEventListener("abort", onAbort); + setPermission(null); + resolve(decision); + }, + }; + setPermission(pending); + // If the signal aborted during the synchronous setPermission call, clean up immediately. + if (opts.signal?.aborted) { + setPermission(null); + opts.signal?.removeEventListener("abort", onAbort); + reject(new Error("Permission prompt cancelled (the turn was aborted).")); + return; + } + opts.signal?.addEventListener("abort", onAbort, { once: true }); + }); +} diff --git a/src/ui/ink/index.tsx b/src/ui/ink/index.tsx index d24aebd..c51f08e 100644 --- a/src/ui/ink/index.tsx +++ b/src/ui/ink/index.tsx @@ -73,7 +73,7 @@ export async function runInkApp(opts: RunInkAppOptions): Promise { // Flush in-flight autosaves first so a fire-and-forget persist right before exit isn't lost. await flushPendingSaves(); // Best-effort: don't leave backgrounded shells (dev servers, watch builds) running as orphans. - killAllBackgroundJobs(); + await killAllBackgroundJobs(); await disconnectAllMcpServers(); // Best-effort — fires on every exit path, so there isn't always a specific session id to attach // (e.g. exiting from the model picker before a session ever started). diff --git a/src/ui/ink/types.ts b/src/ui/ink/types.ts index 1d406be..c615b30 100644 --- a/src/ui/ink/types.ts +++ b/src/ui/ink/types.ts @@ -37,6 +37,7 @@ export type HistoryItem = | { id: string; kind: "tool_call"; label: string } | { id: string; kind: "tool_result"; summary: string; isError: boolean } | { id: string; kind: "notice"; text: string; isError?: boolean } + | { id: string; kind: "todos"; todos: import("../../tools/types.js").TodoItem[] } | { id: string; kind: "help" } | { id: string; kind: "tools"; tools: import("../../tools/types.js").ToolDef[] } | { id: string; kind: "permissions"; allowed: string[] } diff --git a/src/ui/toolSummary.ts b/src/ui/toolSummary.ts index 9c86e94..7ed2bac 100644 --- a/src/ui/toolSummary.ts +++ b/src/ui/toolSummary.ts @@ -41,6 +41,9 @@ export function summarizeToolResult(toolName: string, result: unknown): string { const kb = typeof r.bytes === "number" ? `${(r.bytes / 1024).toFixed(0)} KB` : ""; return `Read image${r.mimeType ? ` (${r.mimeType}${kb ? `, ${kb}` : ""})` : ""}`; } + if (r.truncated === true && typeof r.nextOffset === "number" && typeof r.totalLines === "number") { + return `Read ${r.nextOffset - 1} of ${r.totalLines} lines (truncated, next at offset ${r.nextOffset})`; + } return typeof r.totalLines === "number" ? `Read ${r.totalLines} lines` : "Read file"; case "list_files": return Array.isArray(r.matches) ? `Found ${r.matches.length} file(s)` : "Listed files"; diff --git a/src/utils/processTree.ts b/src/utils/processTree.ts new file mode 100644 index 0000000..5661c94 --- /dev/null +++ b/src/utils/processTree.ts @@ -0,0 +1,13 @@ +import treeKill from "tree-kill"; + +/** Kill a process and its entire descendant tree. execa's `child.kill()` only signals the direct + * child, leaving grandchildren orphaned — a backgrounded `npm run dev` (which spawns a dev + * server), a `tsc --watch`, or a pipeline stage would survive the kill and keep running after + * locode exits. tree-kill walks the OS process tree (ps on POSIX, `taskkill /T` on Windows) and + * signals every descendant, so killing a backgrounded or timed-out command actually cleans up the + * whole tree. Resolves once the signal has been delivered (not once the children have exited). */ +export function killProcessTree(pid: number, signal: NodeJS.Signals = "SIGTERM"): Promise { + return new Promise((resolve) => { + treeKill(pid, signal, () => resolve()); + }); +} \ No newline at end of file diff --git a/src/utils/projectInstructions.test.ts b/src/utils/projectInstructions.test.ts new file mode 100644 index 0000000..b492e73 --- /dev/null +++ b/src/utils/projectInstructions.test.ts @@ -0,0 +1,54 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { loadProjectInstructions } from "./projectInstructions.js"; + +describe("loadProjectInstructions", () => { + let dir: string; + beforeEach(() => { + dir = mkdtempSync(path.join(tmpdir(), "locode-instructions-")); + }); + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("returns null when neither CLAUDE.md nor AGENTS.md exists", async () => { + expect(await loadProjectInstructions(dir)).toBeNull(); + }); + + it("reads CLAUDE.md and includes its content", async () => { + writeFileSync(path.join(dir, "CLAUDE.md"), "Use two-space indentation.", "utf-8"); + const result = await loadProjectInstructions(dir); + expect(result).toContain("CLAUDE.md"); + expect(result).toContain("Use two-space indentation."); + }); + + it("prefers CLAUDE.md over AGENTS.md when both exist", async () => { + writeFileSync(path.join(dir, "CLAUDE.md"), "from claude.md", "utf-8"); + writeFileSync(path.join(dir, "AGENTS.md"), "from agents.md", "utf-8"); + const result = await loadProjectInstructions(dir); + expect(result).toContain("from claude.md"); + expect(result).not.toContain("from agents.md"); + }); + + it("falls back to AGENTS.md when CLAUDE.md is absent", async () => { + writeFileSync(path.join(dir, "AGENTS.md"), "from agents.md", "utf-8"); + const result = await loadProjectInstructions(dir); + expect(result).toContain("from agents.md"); + }); + + it("treats a whitespace-only file as absent and falls through to the next candidate", async () => { + writeFileSync(path.join(dir, "CLAUDE.md"), " \n\n ", "utf-8"); + writeFileSync(path.join(dir, "AGENTS.md"), "real content", "utf-8"); + const result = await loadProjectInstructions(dir); + expect(result).toContain("real content"); + }); + + it("truncates a huge file instead of blowing out the system prompt", async () => { + writeFileSync(path.join(dir, "CLAUDE.md"), "x".repeat(30_000), "utf-8"); + const result = await loadProjectInstructions(dir); + expect(result).toContain("truncated"); + expect(result!.length).toBeLessThan(25_000); + }); +}); diff --git a/src/utils/projectInstructions.ts b/src/utils/projectInstructions.ts new file mode 100644 index 0000000..65c4ca4 --- /dev/null +++ b/src/utils/projectInstructions.ts @@ -0,0 +1,31 @@ +import { readFile } from "node:fs/promises"; +import path from "node:path"; + +// Checked in order — the first one found wins (matching Claude Code's own CLAUDE.md/AGENTS.md +// convention). Only the project root is checked, not parent directories: locode sessions are +// always scoped to a single cwd, unlike Claude Code's monorepo-aware directory walk. +const CANDIDATE_FILENAMES = ["CLAUDE.md", "AGENTS.md"]; + +// Same cap as text @mention/import (see utils/importFile.ts) — a huge instructions file +// shouldn't be able to blow out a local model's (often small) context on every single request, +// since this gets folded into the system prompt and resent on every turn. +const MAX_CHARS = 20_000; + +/** Reads the project's CLAUDE.md/AGENTS.md (if present) so its conventions can be folded into the + * system prompt — repo-specific instructions the model wouldn't otherwise know about. Returns + * null if neither file exists or both are empty. */ +export async function loadProjectInstructions(cwd: string): Promise { + for (const filename of CANDIDATE_FILENAMES) { + let content: string; + try { + content = await readFile(path.join(cwd, filename), "utf-8"); + } catch { + continue; + } + const trimmed = content.trim(); + if (!trimmed) continue; + const capped = trimmed.length > MAX_CHARS ? `${trimmed.slice(0, MAX_CHARS)}\n... [truncated]` : trimmed; + return `Project instructions (from ${filename}):\n\n${capped}`; + } + return null; +}