diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cc0941be..549e9660 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -57,6 +57,7 @@ - Fixed llama.cpp router download progress updates and removed redundant wording from model action confirmations. - Moved automatic model catalog network refresh out of startup initialization and into the running interactive and RPC modes. - Fixed persisted sessions being read and parsed twice when opened, reducing startup latency for large sessions ([#6793](https://github.com/earendil-works/pi/issues/6793)). +- Fixed slow Ctrl+G external-editor startup when the system temporary directory contains many entries ([#6774](https://github.com/earendil-works/pi/issues/6774)). - Fixed prompt-template defaults for all arguments (`${@:-default}` and `${ARGUMENTS:-default}`) ([#6695](https://github.com/earendil-works/pi/issues/6695)). - Fixed obsolete custom UI, custom tool, and custom editor examples in the extension documentation ([#6735](https://github.com/earendil-works/pi/issues/6735)). - Fixed Kimi Coding sessions to show API-equivalent implied costs with the subscription indicator. diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index e6cae1af..3306d71b 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -851,7 +851,7 @@ export class SettingsManager { return this.settings.showCacheMissNotices ?? false; } - getExternalEditorCommand(): string | undefined { + getExternalEditorCommand(): string { const configuredEditor = this.settings.externalEditor; if (typeof configuredEditor === "string" && configuredEditor.trim() !== "") { return configuredEditor; diff --git a/packages/coding-agent/src/modes/interactive/components/extension-editor.ts b/packages/coding-agent/src/modes/interactive/components/extension-editor.ts index a5ed3fba..9af7d8c3 100644 --- a/packages/coding-agent/src/modes/interactive/components/extension-editor.ts +++ b/packages/coding-agent/src/modes/interactive/components/extension-editor.ts @@ -3,10 +3,6 @@ * Supports Ctrl+G for external editor. */ -import { spawn } from "node:child_process"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { Container, Editor, @@ -18,6 +14,7 @@ import { type TUI, } from "@earendil-works/pi-tui"; import type { KeybindingsManager } from "../../../core/keybindings.ts"; +import { editInExternalEditor } from "../external-editor.ts"; import { getEditorTheme, theme } from "../theme/theme.ts"; import { DynamicBorder } from "./dynamic-border.ts"; import { keyHint } from "./keybinding-hints.ts"; @@ -28,7 +25,7 @@ export class ExtensionEditorComponent extends Container implements Focusable { private onCancelCallback: () => void; private tui: TUI; private keybindings: KeybindingsManager; - private externalEditorCommand: string | undefined; + private externalEditorCommand: string; private _focused = false; get focused(): boolean { @@ -53,7 +50,11 @@ export class ExtensionEditorComponent extends Container implements Focusable { this.tui = tui; this.keybindings = keybindings; - this.externalEditorCommand = externalEditorCommand; + this.externalEditorCommand = + externalEditorCommand || + process.env.VISUAL || + process.env.EDITOR || + (process.platform === "win32" ? "notepad" : "nano"); this.onSubmitCallback = onSubmit; this.onCancelCallback = onCancel; @@ -79,14 +80,13 @@ export class ExtensionEditorComponent extends Container implements Focusable { this.addChild(new Spacer(1)); // Add hint - const hasExternalEditor = !!this.getExternalEditorCommand(); const hint = keyHint("tui.select.confirm", "submit") + " " + keyHint("tui.input.newLine", "newline") + " " + keyHint("tui.select.cancel", "cancel") + - (hasExternalEditor ? ` ${keyHint("app.editor.external", "external editor")}` : ""); + ` ${keyHint("app.editor.external", "external editor")}`; this.addChild(new Text(hint, 1, 0)); this.addChild(new Spacer(1)); @@ -105,7 +105,7 @@ export class ExtensionEditorComponent extends Container implements Focusable { // External editor (app keybinding) if (this.keybindings.matches(keyData, "app.editor.external")) { - this.openExternalEditor(); + void this.handleOpenExternalEditor(); return; } @@ -113,54 +113,19 @@ export class ExtensionEditorComponent extends Container implements Focusable { this.editor.handleInput(keyData); } - private getExternalEditorCommand(): string | undefined { - const editorCmd = this.externalEditorCommand || process.env.VISUAL || process.env.EDITOR; - if (editorCmd) { - return editorCmd; - } - return process.platform === "win32" ? "notepad" : "nano"; - } - - private async openExternalEditor(): Promise { - const editorCmd = this.getExternalEditorCommand(); - if (!editorCmd) { - return; - } - - const currentText = this.editor.getText(); - const tmpFile = path.join(os.tmpdir(), `pi-extension-editor-${Date.now()}.md`); - + private async handleOpenExternalEditor(): Promise { + const content = this.editor.getText(); + this.tui.stop(); try { - fs.writeFileSync(tmpFile, currentText, "utf-8"); - this.tui.stop(); - - const [editor, ...editorArgs] = editorCmd.split(" "); - process.stdout.write(`Launching external editor: ${editorCmd}\nPi will resume when the editor exits.\n`); - - // Do not use spawnSync here. On Windows, synchronous child_process calls can keep - // Node/libuv's console input read active after tui.stop() pauses stdin, racing - // vim/nvim for the console input buffer until Ctrl+C cancels the pending read. - const status = await new Promise((resolve) => { - const child = spawn(editor, [...editorArgs, tmpFile], { - stdio: "inherit", - shell: process.platform === "win32", - }); - child.on("error", () => resolve(null)); - child.on("close", (code) => resolve(code)); + const result = await editInExternalEditor({ + command: this.externalEditorCommand, + content, }); - - if (status === 0) { - const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, ""); - this.editor.setText(newContent); + if (result.status === "complete") { + this.editor.setText(result.content); } } finally { - try { - fs.unlinkSync(tmpFile); - } catch { - // Ignore cleanup errors - } this.tui.start(); - // Force full re-render since external editor uses alternate screen this.tui.requestRender(true); } } diff --git a/packages/coding-agent/src/modes/interactive/external-editor.ts b/packages/coding-agent/src/modes/interactive/external-editor.ts new file mode 100644 index 00000000..45fb572f --- /dev/null +++ b/packages/coding-agent/src/modes/interactive/external-editor.ts @@ -0,0 +1,45 @@ +import { spawn } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export interface ExternalEditorOptions { + command: string; + content: string; +} + +export type ExternalEditorResult = { status: "complete"; content: string } | { status: "failed" }; + +export async function editInExternalEditor(options: ExternalEditorOptions): Promise { + const directory = mkdtempSync(join(tmpdir(), "pi-editor-")); + const filePath = join(directory, "prompt.md"); + try { + writeFileSync(filePath, options.content, "utf-8"); + const [editor, ...editorArgs] = options.command.split(" "); + process.stdout.write(`Launching external editor: ${options.command}\nPi will resume when the editor exits.\n`); + + // Do not use spawnSync here. On Windows, synchronous child_process calls can keep + // Node/libuv's console input read active after the parent pauses stdin, racing + // vim/nvim for the console input buffer until Ctrl+C cancels the pending read. + const exitCode = await new Promise((resolve) => { + const child = spawn(editor, [...editorArgs, filePath], { + stdio: "inherit", + shell: process.platform === "win32", + }); + child.on("error", () => resolve(null)); + child.on("close", (code) => resolve(code)); + }); + + if (exitCode !== 0) { + return { status: "failed" }; + } + + return { status: "complete", content: readFileSync(filePath, "utf-8").replace(/\n$/, "") }; + } finally { + try { + rmSync(directory, { recursive: true, force: true }); + } catch { + // Cleanup is best effort. + } + } +} diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 6a20b677..09bfd86d 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -137,6 +137,7 @@ import { TreeSelectorComponent } from "./components/tree-selector.ts"; import { TrustSelectorComponent } from "./components/trust-selector.ts"; import { UserMessageComponent } from "./components/user-message.ts"; import { UserMessageSelectorComponent } from "./components/user-message-selector.ts"; +import { editInExternalEditor } from "./external-editor.ts"; import { getModelSearchText } from "./model-search.ts"; import { getAvailableThemes, @@ -2577,7 +2578,7 @@ export class InteractiveMode { this.defaultEditor.onAction("app.model.select", () => this.showModelSelector()); this.defaultEditor.onAction("app.tools.expand", () => this.toggleToolOutputExpansion()); this.defaultEditor.onAction("app.thinking.toggle", () => this.toggleThinkingBlockVisibility()); - this.defaultEditor.onAction("app.editor.external", () => this.openExternalEditor()); + this.defaultEditor.onAction("app.editor.external", () => void this.handleOpenExternalEditor()); this.defaultEditor.onAction("app.message.copy", () => void this.handleCopyCommand()); this.defaultEditor.onAction("app.message.followUp", () => this.handleFollowUp()); this.defaultEditor.onAction("app.message.dequeue", () => this.handleDequeue()); @@ -3809,57 +3810,20 @@ export class InteractiveMode { this.showStatus(`Thinking blocks: ${this.hideThinkingBlock ? "hidden" : "visible"}`); } - private async openExternalEditor(): Promise { + private async handleOpenExternalEditor(): Promise { const editorCmd = this.settingsManager.getExternalEditorCommand(); - if (!editorCmd) { - this.showWarning("No editor configured. Set externalEditor in settings.json or $VISUAL/$EDITOR."); - return; - } - - const currentText = this.editor.getExpandedText?.() ?? this.editor.getText(); - const tmpFile = path.join(os.tmpdir(), `pi-editor-${Date.now()}.pi.md`); - + const content = this.editor.getExpandedText?.() ?? this.editor.getText(); + this.ui.stop(); try { - // Write current content to temp file - fs.writeFileSync(tmpFile, currentText, "utf-8"); - - // Stop TUI to release terminal - this.ui.stop(); - - // Split by space to support editor arguments (e.g., "code --wait") - const [editor, ...editorArgs] = editorCmd.split(" "); - - process.stdout.write(`Launching external editor: ${editorCmd}\nPi will resume when the editor exits.\n`); - - // Do not use spawnSync here. On Windows, synchronous child_process calls can keep - // Node/libuv's console input read active after ui.stop() pauses stdin, racing - // vim/nvim for the console input buffer until Ctrl+C cancels the pending read. - const status = await new Promise((resolve) => { - const child = spawn(editor, [...editorArgs, tmpFile], { - stdio: "inherit", - shell: process.platform === "win32", - }); - child.on("error", () => resolve(null)); - child.on("close", (code) => resolve(code)); + const result = await editInExternalEditor({ + command: editorCmd, + content, }); - - // On successful exit (status 0), replace editor content - if (status === 0) { - const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, ""); - this.editor.setText(newContent); + if (result.status === "complete") { + this.editor.setText(result.content); } - // On non-zero exit, keep original text (no action needed) } finally { - // Clean up temp file - try { - fs.unlinkSync(tmpFile); - } catch { - // Ignore cleanup errors - } - - // Restart TUI this.ui.start(); - // Force full re-render since external editor uses alternate screen this.ui.requestRender(true); } } diff --git a/packages/coding-agent/test/external-editor.test.ts b/packages/coding-agent/test/external-editor.test.ts new file mode 100644 index 00000000..5cbbc93b --- /dev/null +++ b/packages/coding-agent/test/external-editor.test.ts @@ -0,0 +1,63 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { type ExternalEditorResult, editInExternalEditor } from "../src/modes/interactive/external-editor.ts"; + +const editorFixturePath = fileURLToPath(new URL("./fixtures/fake-external-editor.mjs", import.meta.url)); + +interface EditorCapture { + filePath: string; + content: string; + entries: string[]; + directoryMode: number; +} + +async function runExternalEditor(fixtureFlag?: "--fail" | "--empty"): Promise<{ + result: ExternalEditorResult; + capture: EditorCapture; +}> { + const testDirectory = mkdtempSync(join(tmpdir(), "pi-external-editor-test-")); + const capturePath = join(testDirectory, "capture.json"); + try { + const result = await editInExternalEditor({ + command: `${process.execPath} ${editorFixturePath} ${capturePath}${fixtureFlag ? ` ${fixtureFlag}` : ""}`, + content: "original", + }); + const capture = JSON.parse(readFileSync(capturePath, "utf-8")) as EditorCapture; + return { result, capture }; + } finally { + rmSync(testDirectory, { recursive: true, force: true }); + } +} + +describe("editInExternalEditor", () => { + it("edits a prompt inside a private temporary directory", async () => { + const { result, capture } = await runExternalEditor(); + const directory = dirname(capture.filePath); + + expect(result).toEqual({ status: "complete", content: "edited" }); + expect(dirname(directory)).toBe(tmpdir()); + expect(basename(directory)).toMatch(/^pi-editor-.+$/); + expect(basename(capture.filePath)).toBe("prompt.md"); + expect(capture.entries).toEqual(["prompt.md"]); + expect(capture.content).toBe("original"); + if (process.platform !== "win32") { + expect(capture.directoryMode & 0o077).toBe(0); + } + expect(existsSync(directory)).toBe(false); + }); + + it("keeps the original content when the editor exits unsuccessfully", async () => { + const { result, capture } = await runExternalEditor("--fail"); + + expect(result).toEqual({ status: "failed" }); + expect(existsSync(dirname(capture.filePath))).toBe(false); + }); + it("returns empty content when the editor clears the prompt", async () => { + const { result } = await runExternalEditor("--empty"); + + expect(result).toEqual({ status: "complete", content: "" }); + }); +}); diff --git a/packages/coding-agent/test/fixtures/fake-external-editor.mjs b/packages/coding-agent/test/fixtures/fake-external-editor.mjs new file mode 100644 index 00000000..043dfcf0 --- /dev/null +++ b/packages/coding-agent/test/fixtures/fake-external-editor.mjs @@ -0,0 +1,25 @@ +import { readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { dirname } from "node:path"; + +const capturePath = process.argv[2]; +const filePath = process.argv.at(-1); +if (!capturePath || !filePath) { + process.exit(1); +} + +const directory = dirname(filePath); +writeFileSync( + capturePath, + JSON.stringify({ + filePath, + content: readFileSync(filePath, "utf-8"), + entries: readdirSync(directory), + directoryMode: statSync(directory).mode & 0o777, + }), + "utf-8", +); + +if (process.argv.includes("--fail")) { + process.exit(1); +} +writeFileSync(filePath, process.argv.includes("--empty") ? "" : "edited\n", "utf-8");