diff --git a/README.md b/README.md index 3e84a91..246448d 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,47 @@ Only user and assistant messages are persisted, capped at the most recent messag History written by a version before sessions existed is imported once into a `legacy` bucket, reachable from the `/resume` picker with Ctrl+A. Resume it and take a turn and it becomes that project's session; open it only to read and it stays put. +## How it is measured + +Woopcode is benchmarked on [Harbor](https://github.com/laude-institute/harbor)'s +`terminal-bench-2` as an installed agent: the harness installs the published CLI +into the task container and runs `woopcode -p` once per task, so what is measured +is the thing users get rather than a bespoke harness build. `harbor_woopcode/` +holds the integration. + +Context changes are measured before they ship, against ten recorded benchmark +trajectories rather than against intuition: + +```bash +bun run replay:baseline +``` + +The harness replays each trajectory's prompt assembly and reports peak size and +what a given budget would have done — 932 iterations, no API calls, nothing spent. + +**The measurements decide the defaults, including against the obvious answer.** +Tool history is the only part of the prompt that grows: across the corpus, peak +prompt size ran from 22,639 to 219,179 characters while the system prompt, +repository context and conversation stayed flat. Compacting it works by the +character count — 36–43% off peak prompts at matched depth, confirmed live — and +it is **off by default**, because the same benchmark run cost a task that had +been passing. Reading the provider's own token counts back out of both runs +explained why: implicit caching stopped entirely, 18.1M cached tokens of 23.2M +becoming 1.1M of 11.6M, because rewriting the older messages moves the cache +prefix on every request. At iteration 200, uncompacted, only 16k of a 96k prompt +was billed at full rate; compacted, all 29k was. Peak characters fell by two +thirds for roughly no saving. + +The code, its tests and the measurements all stay — `WOOPCODE_TOOL_HISTORY_BUDGET` +enables it — but the default follows the billing, not the character count. +`runtime/compaction.ts` carries the full numbers and the two variants worth +trying next. + +What the harness cannot tell you is stated where it runs: it reports cache rates +observed for the original recordings, and a modified prompt assembly cannot +inherit them. The fixtures reconstruct prompt *sizes* faithfully; they are not a +conversation that can be replayed against a live provider. + ## Built-in tools Woopcode ships with a fixed set of tools, grouped by what they touch. diff --git a/commands/agent.tsx b/commands/agent.tsx index c723c06..84876b7 100644 --- a/commands/agent.tsx +++ b/commands/agent.tsx @@ -293,10 +293,10 @@ async function runInteractive( modelOverride?: string, session: InitializeOptions = { continueLatest: true }, ) { - // Register slash commands registerCommands(); - // Ensure provider is configured (launches onboarding if needed) + // Launches onboarding when nothing is configured, so this may not return + // immediately on a first run. const { provider, apiKey } = await ensureProviderConfigured(); const config = await getConfig(); @@ -375,7 +375,6 @@ async function runInteractive( }, onDone() { - //console.log("onDone received"); store.finishAssistantMessage(); store.setStatus("Ready"); }, diff --git a/commands/slash/commands.ts b/commands/slash/commands.ts index 4a7ce42..2847ab3 100644 --- a/commands/slash/commands.ts +++ b/commands/slash/commands.ts @@ -346,7 +346,6 @@ const loginCommand: SlashCommand = { return unsupportedProviderMessage(provider); } - // Validate API key const { loginProvider } = await import("../../config/authProvider"); const isValid = await loginProvider(provider, apiKey); @@ -358,7 +357,6 @@ const loginCommand: SlashCommand = { return `Cannot change provider while the agent is running. Press Esc to cancel first.`; } - // Save the API key config.providers[provider].apiKey = apiKey; config.defaultProvider = provider; @@ -408,12 +406,11 @@ const logoutCommand: SlashCommand = { return `Cannot log out of the active provider while the agent is running. Press Esc to cancel first.`; } - // Remove API key delete config.providers[provider].apiKey; - // If logging out from default provider, clear default + // Leaving a logged-out provider as the default would send the next turn at + // a provider with no key, so hand the default to one that still has one. if (config.defaultProvider === provider) { - // Find another logged-in provider const otherProvider = Object.entries(config.providers).find( ([name, details]: [string, any]) => name !== provider && details.apiKey && isProviderEnabled(name) diff --git a/commands/slash/parser.ts b/commands/slash/parser.ts index 4187c2b..fa9b64d 100644 --- a/commands/slash/parser.ts +++ b/commands/slash/parser.ts @@ -3,7 +3,8 @@ import type { ParsedCommand } from "./types"; export function parseInput(input: string): ParsedCommand { const trimmed = input.trim(); - //discovery mode + // A bare slash lists what is available rather than failing as an unknown + // command, which is what makes the commands discoverable at all. if (trimmed === "/") { return { type: "discovery", originalInput: input }; } diff --git a/commands/slash/registry.ts b/commands/slash/registry.ts index b9ac183..84ce8fb 100644 --- a/commands/slash/registry.ts +++ b/commands/slash/registry.ts @@ -27,7 +27,6 @@ export class SlashCommandRegistry { return this.getAll().filter((cmd) => cmd.category === category); } - // Auto-generated help generateHelp(): string { const categories = { session: "Session", @@ -55,7 +54,6 @@ export class SlashCommandRegistry { return output.trim(); } - // Discovery list generateDiscoveryList(): string { return this.getAll() .map((cmd) => `/${cmd.name}`) diff --git a/config/paths.ts b/config/paths.ts index 9766297..4c49ea6 100644 --- a/config/paths.ts +++ b/config/paths.ts @@ -36,7 +36,6 @@ export function getConfigDir(): string { configDir = join(xdgConfigHome, "woopcode"); } - // Ensure directory exists if (!existsSync(configDir)) { mkdirSync(configDir, { recursive: true }); } diff --git a/config/sessions.ts b/config/sessions.ts index 5d42296..1a29db0 100644 --- a/config/sessions.ts +++ b/config/sessions.ts @@ -361,6 +361,44 @@ async function writeIndex(slug: string, index: SessionIndex): Promise { await writeJsonAtomic(getSessionIndexPath(slug), index); } +/** + * What a caller means when the index it wanted to change is not there. + * + * Not a detail to default: saving a record rebuilds from the files on disk so + * the new row joins the existing ones rather than replacing them; pruning + * starts from empty because it is about to write the rows it kept; and removing + * a session does nothing at all, since there is no row to take out and writing + * an index here would create the directory lazy creation exists to avoid. + */ +type MissingIndex = "rebuild" | "empty" | "skip"; + +/** + * Reads a project's index, applies `mutate`, writes the result back. + * + * Every caller wants those three steps and no caller wants two of them, but + * each spelled the sequence out itself — five copies of a read-modify-write + * that has already lost a row once. This does not close the window between the + * read and the write; two processes still interleave, which is why + * `summariesFor` compares the row count against the files on disk and rebuilds + * when they disagree. It puts the pattern in one place, so the next change to + * it is one edit rather than five. + */ +async function updateIndex( + slug: string, + onMissing: MissingIndex, + mutate: (index: SessionIndex) => SessionIndex, +): Promise { + const existing = await readIndex(slug); + if (!existing && onMissing === "skip") return; + + const index = existing ?? { + version: SESSION_VERSION, + sessions: onMissing === "rebuild" ? await rebuildIndex(slug) : [], + }; + + await writeIndex(slug, mutate(index)); +} + /** Session files on disk for a project, ignoring the index entirely. */ function sessionFileCount(slug: string): number { const directory = getProjectSessionsDir(slug); @@ -481,16 +519,15 @@ async function writeSessionRecord(record: SessionRecord): Promise mkdirSync(getProjectSessionsDir(slug), { recursive: true }); await writeJsonAtomic(getSessionPath(slug, trimmed.id), trimmed); - const index = (await readIndex(slug)) ?? { - version: SESSION_VERSION, - sessions: await rebuildIndex(slug), - }; const summary = summarize(trimmed, slug); - const sessions = [ - summary, - ...index.sessions.filter((session) => session.id !== trimmed.id), - ].sort(byRecency); - await writeIndex(slug, { ...index, version: SESSION_VERSION, sessions }); + await updateIndex(slug, "rebuild", (index) => ({ + ...index, + version: SESSION_VERSION, + sessions: [ + summary, + ...index.sessions.filter((session) => session.id !== trimmed.id), + ].sort(byRecency), + })); return trimmed; } @@ -540,13 +577,10 @@ async function removeFromProject(slug: string, id: string): Promise { const path = getSessionPath(slug, id); if (existsSync(path)) rmSync(path, { force: true }); - const index = await readIndex(slug); - if (!index) return; - - await writeIndex(slug, { + await updateIndex(slug, "skip", (index) => ({ ...index, sessions: index.sessions.filter((session) => session.id !== id), - }); + })); } catch { // See above: leaving it listed is the safer failure. } @@ -666,13 +700,12 @@ export async function pruneSessions( } if (removedHere > 0) { - const index = (await readIndex(slug)) ?? { version: SESSION_VERSION, sessions: [] }; - await writeIndex(slug, { + await updateIndex(slug, "empty", (index) => ({ ...index, version: SESSION_VERSION, lastPrunedAt: now, sessions: kept, - }); + })); } } @@ -709,8 +742,7 @@ export async function pruneIfDue( // Stamped even when nothing was removed, or a store with no expired sessions // would rescan every launch. - const current = (await readIndex(slug)) ?? { version: SESSION_VERSION, sessions: [] }; - await writeIndex(slug, { ...current, lastPrunedAt: now }); + await updateIndex(slug, "empty", (index) => ({ ...index, lastPrunedAt: now })); return removed; } diff --git a/onboarding/index.ts b/onboarding/index.ts index 6a88b4e..ceac304 100644 --- a/onboarding/index.ts +++ b/onboarding/index.ts @@ -100,7 +100,6 @@ function runOnboarding(): Promise { }), ); - // Handle Ctrl+C gracefully const handleExit = () => { if (!hasCompleted) { unmount(); diff --git a/onboarding/setupWizard.tsx b/onboarding/setupWizard.tsx index 818171b..09bf348 100644 --- a/onboarding/setupWizard.tsx +++ b/onboarding/setupWizard.tsx @@ -71,7 +71,6 @@ export function SetupWizard({ onComplete, onError }: SetupWizardProps) { return; } - // Save configuration const config = await getConfig(); config.defaultProvider = selectedProvider.id; // A provider chosen in the wizard may have no entry yet. diff --git a/packages/tests/config/sessions.test.ts b/packages/tests/config/sessions.test.ts index 0c01d75..658e018 100644 --- a/packages/tests/config/sessions.test.ts +++ b/packages/tests/config/sessions.test.ts @@ -420,6 +420,58 @@ describe("an index that has fallen behind the files", () => { }); }); +/** + * A missing index means something different to each caller that writes one, and + * the three answers are not interchangeable. Asserted against the file on disk + * rather than through listSessions, because listSessions heals a wrong index by + * comparing its row count to the files and rebuilding — which would hide every + * one of these. + */ +describe("writing an index that is not there", () => { + test("saving a session rebuilds the other rows instead of replacing them", async () => { + const first = await seed(); + rmSync(join(projectDir(), "index.json"), { force: true }); + + const second = await seed(); + + const index = JSON.parse(await Bun.file(join(projectDir(), "index.json")).text()); + const ids = index.sessions.map((entry: any) => entry.id); + expect(ids).toContain(second.id); + expect(ids).toContain(first.id); + }); + + test("moving a session out of a project does not create an index there", async () => { + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, "conversation.json"), + JSON.stringify([{ role: "user", content: "legacy work" }]), + ); + resetSessionStoreForTests(); + const imported = (await migrateLegacyConversation())!; + + const legacyIndex = join(sessionsDir, LEGACY_SLUG, "index.json"); + rmSync(legacyIndex, { force: true }); + + await adoptSession(imported); + + // Nothing is left in that bucket to list, so an index recording zero rows + // is state the feature promises not to keep. + expect(existsSync(legacyIndex)).toBe(false); + }); + + test("stamping a prune keeps the sessions it did not remove", async () => { + const saved = await seed(); + const indexPath = join(projectDir(), "index.json"); + rmSync(indexPath, { force: true }); + + await pruneIfDue(30); + + const index = JSON.parse(await Bun.file(indexPath).text()); + expect(index.lastPrunedAt).toBeGreaterThan(0); + expect(index.sessions.map((entry: any) => entry.id)).toEqual([saved.id]); + }); +}); + describe("retention", () => { test("removes sessions past the cutoff and keeps the rest", async () => { const day = 24 * 60 * 60 * 1000; diff --git a/tools/editFile.ts b/tools/editFile.ts index fe61406..6819bce 100644 --- a/tools/editFile.ts +++ b/tools/editFile.ts @@ -102,12 +102,10 @@ export const editFileTool: Tool = { return `No changes needed for ${path}`; } - // Generate unified diff const diff = createTwoFilesPatch(path, path, content, updated, "", "", { context: 3, }); - // Create pending edit const pendingEdit: PendingEdit = { id: crypto.randomUUID(), filePath: path, @@ -117,7 +115,6 @@ export const editFileTool: Tool = { toolCallId: crypto.randomUUID(), }; - // Request approval from UI let approved: boolean; try { approved = await store.setPendingEdit(pendingEdit); @@ -133,7 +130,8 @@ export const editFileTool: Tool = { return message; } - // Write file after approval + // The only write in this tool, and it sits below both exits above. Nothing + // may move above them: the diff review is the product's whole guarantee. await Bun.write(path, updated); return outcome.replacements > 1 diff --git a/tools/glob.ts b/tools/glob.ts index c5ee215..830a8b3 100644 --- a/tools/glob.ts +++ b/tools/glob.ts @@ -1,5 +1,6 @@ import type { Tool } from "../config/types"; import path from "path"; +import { statSync } from "fs"; import { resolveWorkspacePath } from "./workspace"; export const globTool: Tool = { @@ -35,32 +36,27 @@ Returns up to 100 matching file paths.`, throw new Error("Pattern is required"); } - // Resolve search path const resolvedPath = await resolveWorkspacePath(searchPath, { mustExist: true }); - // Check if path is a file (not a directory) try { - const { statSync } = await import("fs"); - const stats = statSync(resolvedPath); - - if (stats.isFile()) { + if (statSync(resolvedPath).isFile()) { throw new Error(`glob path must be a directory: ${resolvedPath}`); } } catch (err: any) { - // If error is "glob path must be a directory", re-throw it + // Only the directory check is worth failing on. Anything else stat can + // raise means the path is not readable yet, which Glob reports itself as + // an empty scan. if (err.message?.includes("glob path must be a directory")) { throw err; } - // Otherwise, path might not exist yet, which is okay - Glob will handle it } - // Perform glob search with limit const LIMIT = 100; const files: string[] = []; - + try { const glob = new Bun.Glob(pattern); - + for await (const file of glob.scan({ cwd: resolvedPath, onlyFiles: true, @@ -68,23 +64,21 @@ Returns up to 100 matching file paths.`, if (files.length >= LIMIT) { break; } - - // Resolve to absolute path - const absolutePath = path.resolve(resolvedPath, file); - files.push(absolutePath); + + files.push(path.resolve(resolvedPath, file)); } } catch (error) { throw new Error(`Glob search failed: ${error instanceof Error ? error.message : String(error)}`); } - // Format output const output: string[] = []; - + if (files.length === 0) { output.push("No files found"); } else { output.push(...files); - + + if (files.length === LIMIT) { output.push(""); output.push( diff --git a/tools/question.ts b/tools/question.ts index 8b72fcd..2b14769 100644 --- a/tools/question.ts +++ b/tools/question.ts @@ -35,7 +35,6 @@ Examples: throw new Error("At least one question is required"); } - // Validate questions const validQuestions = questions.filter( (q) => typeof q === "string" && q.trim().length > 0 ); diff --git a/tools/readFile.ts b/tools/readFile.ts index 53e6f09..bbfafda 100644 --- a/tools/readFile.ts +++ b/tools/readFile.ts @@ -96,7 +96,6 @@ export const readFileTool: Tool = { throw Error(`File ${path} does not exist`); } - // Check if path is a directory try { const stats = statSync(path); if (stats.isDirectory()) { diff --git a/tools/runTests.ts b/tools/runTests.ts index 7ae49c8..040dc5d 100644 --- a/tools/runTests.ts +++ b/tools/runTests.ts @@ -24,7 +24,8 @@ export const runTestsTool: Tool = { return "Command rejected by user. It was not run."; } - // Warn about server commands + // A server never exits, so it would hold the tool open until the timeout + // rather than failing. Refused here instead, with the reason. if (command.includes("run src/index") || command.includes("run index") || command.includes("start")) { return "Error: This command appears to start a server. Use run_tests only for test suites, not for starting servers. Servers run indefinitely and will cause timeouts."; } diff --git a/tools/webFetch.ts b/tools/webFetch.ts index b629a18..9e423cd 100644 --- a/tools/webFetch.ts +++ b/tools/webFetch.ts @@ -43,7 +43,6 @@ Default timeout: 30 seconds`, const format = (args.format as string) || "markdown"; const timeoutSeconds = Math.min((args.timeout as number) || 30, 120); - // Validation if (!url || (!url.startsWith("http://") && !url.startsWith("https://"))) { throw new Error("URL must start with http:// or https://"); } @@ -81,27 +80,23 @@ Default timeout: 30 seconds`, throw new Error(`HTTP ${response.status}: ${response.statusText}`); } - // Check content length const contentLength = response.headers.get("content-length"); if (contentLength && parseInt(contentLength) > MAX_SIZE) { throw new Error("Response too large (exceeds 5MB limit)"); } const contentType = response.headers.get("content-type") || ""; - - // Handle images + if (contentType.includes("image/")) { return `Image fetched from ${url}\nContent-Type: ${contentType}\n\nNote: Image content cannot be displayed in text format.`; } - // Get content const text = await response.text(); if (text.length > MAX_SIZE) { throw new Error("Response too large (exceeds 5MB limit)"); } - // Process based on format let output = text; if (format === "text" && contentType.includes("text/html")) { @@ -152,13 +147,9 @@ function extractTextFromHTML(html: string): string { .replace(/<\/li>/gi, "\n") .replace(/<\/tr>/gi, "\n"); - // Remove all remaining HTML tags text = text.replace(/<[^>]+>/g, ""); - - // Decode HTML entities text = decodeHTMLEntities(text); - // Clean up whitespace text = text .replace(/\n\s*\n\s*\n/g, "\n\n") // Remove excessive newlines .replace(/[ \t]+/g, " ") // Normalize spaces @@ -167,10 +158,12 @@ function extractTextFromHTML(html: string): string { return text; } +/** + * A deliberately small subset of HTML, not a conformant converter: the output is + * read by a model, so an unhandled tag costs a little fidelity rather than + * breaking anything. `turndown` is the upgrade if that stops being true. + */ function convertHTMLToMarkdown(html: string): string { - // Basic HTML to Markdown conversion - // For production, consider using a library like 'turndown' - let md = html; // Remove script, style tags @@ -211,13 +204,9 @@ function convertHTMLToMarkdown(html: string): string { md = md.replace(/<\/p>/gi, "\n\n"); md = md.replace(//gi, "\n"); - // Remove remaining HTML tags md = md.replace(/<[^>]+>/g, ""); - - // Decode HTML entities md = decodeHTMLEntities(md); - // Clean up whitespace md = md .replace(/\n\s*\n\s*\n/g, "\n\n") .replace(/[ \t]+/g, " ") diff --git a/tools/writeFile.ts b/tools/writeFile.ts index 219f4f3..8d372d9 100644 --- a/tools/writeFile.ts +++ b/tools/writeFile.ts @@ -51,7 +51,6 @@ export const writeFileTool: Tool = { throw new Error(`File not found: ${path}`); } - // Read current content const oldContent = await file.text(); // If content is identical, skip diff preview @@ -59,12 +58,10 @@ export const writeFileTool: Tool = { return `No changes needed for ${path}`; } - // Generate unified diff const diff = createTwoFilesPatch(path, path, oldContent, content, "", "", { context: 3, }); - // Create pending edit const pendingEdit: PendingEdit = { id: crypto.randomUUID(), filePath: path, @@ -74,7 +71,6 @@ export const writeFileTool: Tool = { toolCallId: crypto.randomUUID(), }; - // Request approval from UI let approved: boolean; try { approved = await store.setPendingEdit(pendingEdit); @@ -90,7 +86,8 @@ export const writeFileTool: Tool = { return outcome; } - // Write file after approval + // The only write in this tool, and it sits below both exits above. Nothing + // may move above them: the diff review is the product's whole guarantee. await Bun.write(path, content); return `Updated ${path}`; diff --git a/tui/src/components/CodeBlock.tsx b/tui/src/components/CodeBlock.tsx index 25b8915..86dfa51 100644 --- a/tui/src/components/CodeBlock.tsx +++ b/tui/src/components/CodeBlock.tsx @@ -29,7 +29,6 @@ export function CodeBlock({ code, language }: CodeBlockProps) { } const lines = highlighted.split("\n"); - // Use theme colors for borders const borderColor = chalk.hex(colors.borderBase); const termWidth = Math.max((process.stdout.columns || 80) - 6, 30); @@ -49,7 +48,6 @@ export function CodeBlock({ code, language }: CodeBlockProps) { // ╰────────────────────╯ const bottom = borderColor(`╰${"─".repeat(boxWidth - 2)}╯`); - // empty padding row const empty = borderColor("│") + " ".repeat(boxWidth - 2) + borderColor("│"); // code rows — truncate long lines at inner width so the box never breaks diff --git a/tui/src/components/Markdown.tsx b/tui/src/components/Markdown.tsx index 71b51fb..088b7b9 100644 --- a/tui/src/components/Markdown.tsx +++ b/tui/src/components/Markdown.tsx @@ -275,7 +275,6 @@ function renderInline(tokens: Token[], md: MarkdownPalette): ReactNode[] { case "link": { const t = token as Tokens.Link; - // Render link text with accent color return ( {renderInline(t.tokens, md)} diff --git a/tui/src/prompt.tsx b/tui/src/prompt.tsx index ebc07ae..47f0ab5 100644 --- a/tui/src/prompt.tsx +++ b/tui/src/prompt.tsx @@ -198,7 +198,6 @@ export function Prompt({ // neither is a prompt the user would want ↑ to bring back. history.current.push(prompt); - // 🔥 Slash command interception const context = { controller, onExit, @@ -207,6 +206,8 @@ export function Prompt({ }, }; + // Every slash command, `/exit` included, is resolved by the registry. What + // reaches the agent below is what the registry did not claim. const result = await handleSlashCommand(prompt, context); if (result.handled) { @@ -214,20 +215,13 @@ export function Prompt({ return; } - // Original flow - if (prompt === "/exit") { - await onExit(); - return; - } - onValueChange(""); - - // Run the agent with error handling to keep app alive + try { await controller.run(prompt); } catch (error) { - // Error already displayed via callbacks.onError - // Just catch it here to prevent app crash + // The failure has already been rendered by callbacks.onError. Swallowed + // here only so a provider error cannot unmount the app mid-session. if (process.env.DEBUG) { console.error("Prompt handler caught error:", error); } diff --git a/tui/src/store/ui-store.test.ts b/tui/src/store/ui-store.test.ts index b31f594..87b7c1a 100644 --- a/tui/src/store/ui-store.test.ts +++ b/tui/src/store/ui-store.test.ts @@ -138,6 +138,51 @@ describe("UIStore edit approvals", () => { expect(store.getState().pendingEdit).toBeNull(); }); + /** + * Cancelling and declining are two different answers, and only the rejection + * distinguishes them. `editFile` catches it to report "Edit cancelled"; a + * resolved false takes the other branch and tells the model "Edit rejected + * ... Do not claim this edit was completed". Collapsing the two would report + * an interrupted turn as a refusal the user never made. + * + * The dismissal test below covers `dismissTopModal`, which declines. Nothing + * covered this path: the overlay tests call `clearPendingEdit` only as + * teardown and swallow the rejection, so resolving false here stayed green. + */ + test("cancelling a turn rejects the waiting edit rather than declining it", async () => { + const store = new UIStore(); + const decision = store.setPendingEdit({ + id: "edit-cancel", + filePath: "src/example.ts", + oldContent: "old", + newContent: "new", + diff: "@@ -1 +1 @@\n-old\n+new", + toolCallId: "tool-1", + }); + + store.clearPendingEdit(); + + await expect(decision).rejects.toThrow("Edit cancelled"); + expect(store.getState().pendingEdit).toBeNull(); + }); + + test("clearing the timeline cancels an edit left waiting on the screen", async () => { + const store = new UIStore(); + const decision = store.setPendingEdit({ + id: "edit-clear", + filePath: "src/example.ts", + oldContent: "old", + newContent: "new", + diff: "@@ -1 +1 @@\n-old\n+new", + toolCallId: "tool-1", + }); + + store.clearTimeline(); + + await expect(decision).rejects.toThrow("Edit cancelled"); + expect(store.getState().pendingEdit).toBeNull(); + }); + test("marks a failed tool instead of leaving a spinner running", () => { const store = new UIStore(); store.startTool({ id: "tool-1", name: "edit_file", arguments: {} }); diff --git a/tui/src/store/ui-store.ts b/tui/src/store/ui-store.ts index 4fc0fa5..c687a88 100644 --- a/tui/src/store/ui-store.ts +++ b/tui/src/store/ui-store.ts @@ -503,7 +503,6 @@ export class UIStore { this.emit(); } - // Pending Edit Management setPendingEdit(edit: PendingEdit): Promise { if (this.nonInteractive) { return Promise.resolve(this.nonInteractiveApproval); @@ -523,48 +522,37 @@ export class UIStore { } approvePendingEdit() { - const edit = this.state.pendingEdit; - if (!edit) return; - - const resolver = this.editResolvers.get(edit.id); - if (resolver) { - resolver.resolve(true); - this.editResolvers.delete(edit.id); - } - - this.state = { - ...this.state, - pendingEdit: null, - pendingEditScrollOffset: 0, - }; - this.emit(); + this.resolvePendingEdit("approved"); } rejectPendingEdit() { - const edit = this.state.pendingEdit; - if (!edit) return; - - const resolver = this.editResolvers.get(edit.id); - if (resolver) { - resolver.resolve(false); - this.editResolvers.delete(edit.id); - } - - this.state = { - ...this.state, - pendingEdit: null, - pendingEditScrollOffset: 0, - }; - this.emit(); + this.resolvePendingEdit("rejected"); } clearPendingEdit() { + this.resolvePendingEdit("cancelled"); + } + + /** + * Settles the pending edit and takes it off the screen. + * + * Cancelling rejects where rejecting resolves false, and the two are not + * interchangeable: `editFile` catches the rejection to report that the edit + * was cancelled, and returns the rather firmer "Edit rejected ... Do not + * claim this edit was completed" on a false. Collapsing them would tell the + * model a dismissed dialog was a refusal. + */ + private resolvePendingEdit(outcome: "approved" | "rejected" | "cancelled") { const edit = this.state.pendingEdit; if (!edit) return; const resolver = this.editResolvers.get(edit.id); if (resolver) { - resolver.reject(new Error("Edit cancelled")); + if (outcome === "cancelled") { + resolver.reject(new Error("Edit cancelled")); + } else { + resolver.resolve(outcome === "approved"); + } this.editResolvers.delete(edit.id); }