fix(coding-agent): speed up external editor launch (#6903)
* fix(coding-agent): speed up external editor launch Fixes #6774 * refactor(coding-agent): clarify external editor handling * refactor(coding-agent): simplify external editor result * refactor(coding-agent): model external editor lifecycle * fix(coding-agent): rename external editor test fake * fix(coding-agent): secure external editor temp files * fix(coding-agent): simplify external editor temp path * fix(coding-agent): restore system temp editor files
This commit is contained in:
@@ -57,6 +57,7 @@
|
|||||||
- Fixed llama.cpp router download progress updates and removed redundant wording from model action confirmations.
|
- 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.
|
- 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 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 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 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.
|
- Fixed Kimi Coding sessions to show API-equivalent implied costs with the subscription indicator.
|
||||||
|
|||||||
@@ -851,7 +851,7 @@ export class SettingsManager {
|
|||||||
return this.settings.showCacheMissNotices ?? false;
|
return this.settings.showCacheMissNotices ?? false;
|
||||||
}
|
}
|
||||||
|
|
||||||
getExternalEditorCommand(): string | undefined {
|
getExternalEditorCommand(): string {
|
||||||
const configuredEditor = this.settings.externalEditor;
|
const configuredEditor = this.settings.externalEditor;
|
||||||
if (typeof configuredEditor === "string" && configuredEditor.trim() !== "") {
|
if (typeof configuredEditor === "string" && configuredEditor.trim() !== "") {
|
||||||
return configuredEditor;
|
return configuredEditor;
|
||||||
|
|||||||
@@ -3,10 +3,6 @@
|
|||||||
* Supports Ctrl+G for external editor.
|
* 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 {
|
import {
|
||||||
Container,
|
Container,
|
||||||
Editor,
|
Editor,
|
||||||
@@ -18,6 +14,7 @@ import {
|
|||||||
type TUI,
|
type TUI,
|
||||||
} from "@earendil-works/pi-tui";
|
} from "@earendil-works/pi-tui";
|
||||||
import type { KeybindingsManager } from "../../../core/keybindings.ts";
|
import type { KeybindingsManager } from "../../../core/keybindings.ts";
|
||||||
|
import { editInExternalEditor } from "../external-editor.ts";
|
||||||
import { getEditorTheme, theme } from "../theme/theme.ts";
|
import { getEditorTheme, theme } from "../theme/theme.ts";
|
||||||
import { DynamicBorder } from "./dynamic-border.ts";
|
import { DynamicBorder } from "./dynamic-border.ts";
|
||||||
import { keyHint } from "./keybinding-hints.ts";
|
import { keyHint } from "./keybinding-hints.ts";
|
||||||
@@ -28,7 +25,7 @@ export class ExtensionEditorComponent extends Container implements Focusable {
|
|||||||
private onCancelCallback: () => void;
|
private onCancelCallback: () => void;
|
||||||
private tui: TUI;
|
private tui: TUI;
|
||||||
private keybindings: KeybindingsManager;
|
private keybindings: KeybindingsManager;
|
||||||
private externalEditorCommand: string | undefined;
|
private externalEditorCommand: string;
|
||||||
|
|
||||||
private _focused = false;
|
private _focused = false;
|
||||||
get focused(): boolean {
|
get focused(): boolean {
|
||||||
@@ -53,7 +50,11 @@ export class ExtensionEditorComponent extends Container implements Focusable {
|
|||||||
|
|
||||||
this.tui = tui;
|
this.tui = tui;
|
||||||
this.keybindings = keybindings;
|
this.keybindings = keybindings;
|
||||||
this.externalEditorCommand = externalEditorCommand;
|
this.externalEditorCommand =
|
||||||
|
externalEditorCommand ||
|
||||||
|
process.env.VISUAL ||
|
||||||
|
process.env.EDITOR ||
|
||||||
|
(process.platform === "win32" ? "notepad" : "nano");
|
||||||
this.onSubmitCallback = onSubmit;
|
this.onSubmitCallback = onSubmit;
|
||||||
this.onCancelCallback = onCancel;
|
this.onCancelCallback = onCancel;
|
||||||
|
|
||||||
@@ -79,14 +80,13 @@ export class ExtensionEditorComponent extends Container implements Focusable {
|
|||||||
this.addChild(new Spacer(1));
|
this.addChild(new Spacer(1));
|
||||||
|
|
||||||
// Add hint
|
// Add hint
|
||||||
const hasExternalEditor = !!this.getExternalEditorCommand();
|
|
||||||
const hint =
|
const hint =
|
||||||
keyHint("tui.select.confirm", "submit") +
|
keyHint("tui.select.confirm", "submit") +
|
||||||
" " +
|
" " +
|
||||||
keyHint("tui.input.newLine", "newline") +
|
keyHint("tui.input.newLine", "newline") +
|
||||||
" " +
|
" " +
|
||||||
keyHint("tui.select.cancel", "cancel") +
|
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 Text(hint, 1, 0));
|
||||||
|
|
||||||
this.addChild(new Spacer(1));
|
this.addChild(new Spacer(1));
|
||||||
@@ -105,7 +105,7 @@ export class ExtensionEditorComponent extends Container implements Focusable {
|
|||||||
|
|
||||||
// External editor (app keybinding)
|
// External editor (app keybinding)
|
||||||
if (this.keybindings.matches(keyData, "app.editor.external")) {
|
if (this.keybindings.matches(keyData, "app.editor.external")) {
|
||||||
this.openExternalEditor();
|
void this.handleOpenExternalEditor();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -113,54 +113,19 @@ export class ExtensionEditorComponent extends Container implements Focusable {
|
|||||||
this.editor.handleInput(keyData);
|
this.editor.handleInput(keyData);
|
||||||
}
|
}
|
||||||
|
|
||||||
private getExternalEditorCommand(): string | undefined {
|
private async handleOpenExternalEditor(): Promise<void> {
|
||||||
const editorCmd = this.externalEditorCommand || process.env.VISUAL || process.env.EDITOR;
|
const content = this.editor.getText();
|
||||||
if (editorCmd) {
|
this.tui.stop();
|
||||||
return editorCmd;
|
|
||||||
}
|
|
||||||
return process.platform === "win32" ? "notepad" : "nano";
|
|
||||||
}
|
|
||||||
|
|
||||||
private async openExternalEditor(): Promise<void> {
|
|
||||||
const editorCmd = this.getExternalEditorCommand();
|
|
||||||
if (!editorCmd) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const currentText = this.editor.getText();
|
|
||||||
const tmpFile = path.join(os.tmpdir(), `pi-extension-editor-${Date.now()}.md`);
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
fs.writeFileSync(tmpFile, currentText, "utf-8");
|
const result = await editInExternalEditor({
|
||||||
this.tui.stop();
|
command: this.externalEditorCommand,
|
||||||
|
content,
|
||||||
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<number | null>((resolve) => {
|
|
||||||
const child = spawn(editor, [...editorArgs, tmpFile], {
|
|
||||||
stdio: "inherit",
|
|
||||||
shell: process.platform === "win32",
|
|
||||||
});
|
|
||||||
child.on("error", () => resolve(null));
|
|
||||||
child.on("close", (code) => resolve(code));
|
|
||||||
});
|
});
|
||||||
|
if (result.status === "complete") {
|
||||||
if (status === 0) {
|
this.editor.setText(result.content);
|
||||||
const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, "");
|
|
||||||
this.editor.setText(newContent);
|
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
try {
|
|
||||||
fs.unlinkSync(tmpFile);
|
|
||||||
} catch {
|
|
||||||
// Ignore cleanup errors
|
|
||||||
}
|
|
||||||
this.tui.start();
|
this.tui.start();
|
||||||
// Force full re-render since external editor uses alternate screen
|
|
||||||
this.tui.requestRender(true);
|
this.tui.requestRender(true);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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<ExternalEditorResult> {
|
||||||
|
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<number | null>((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.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -137,6 +137,7 @@ import { TreeSelectorComponent } from "./components/tree-selector.ts";
|
|||||||
import { TrustSelectorComponent } from "./components/trust-selector.ts";
|
import { TrustSelectorComponent } from "./components/trust-selector.ts";
|
||||||
import { UserMessageComponent } from "./components/user-message.ts";
|
import { UserMessageComponent } from "./components/user-message.ts";
|
||||||
import { UserMessageSelectorComponent } from "./components/user-message-selector.ts";
|
import { UserMessageSelectorComponent } from "./components/user-message-selector.ts";
|
||||||
|
import { editInExternalEditor } from "./external-editor.ts";
|
||||||
import { getModelSearchText } from "./model-search.ts";
|
import { getModelSearchText } from "./model-search.ts";
|
||||||
import {
|
import {
|
||||||
getAvailableThemes,
|
getAvailableThemes,
|
||||||
@@ -2577,7 +2578,7 @@ export class InteractiveMode {
|
|||||||
this.defaultEditor.onAction("app.model.select", () => this.showModelSelector());
|
this.defaultEditor.onAction("app.model.select", () => this.showModelSelector());
|
||||||
this.defaultEditor.onAction("app.tools.expand", () => this.toggleToolOutputExpansion());
|
this.defaultEditor.onAction("app.tools.expand", () => this.toggleToolOutputExpansion());
|
||||||
this.defaultEditor.onAction("app.thinking.toggle", () => this.toggleThinkingBlockVisibility());
|
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.copy", () => void this.handleCopyCommand());
|
||||||
this.defaultEditor.onAction("app.message.followUp", () => this.handleFollowUp());
|
this.defaultEditor.onAction("app.message.followUp", () => this.handleFollowUp());
|
||||||
this.defaultEditor.onAction("app.message.dequeue", () => this.handleDequeue());
|
this.defaultEditor.onAction("app.message.dequeue", () => this.handleDequeue());
|
||||||
@@ -3809,57 +3810,20 @@ export class InteractiveMode {
|
|||||||
this.showStatus(`Thinking blocks: ${this.hideThinkingBlock ? "hidden" : "visible"}`);
|
this.showStatus(`Thinking blocks: ${this.hideThinkingBlock ? "hidden" : "visible"}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
private async openExternalEditor(): Promise<void> {
|
private async handleOpenExternalEditor(): Promise<void> {
|
||||||
const editorCmd = this.settingsManager.getExternalEditorCommand();
|
const editorCmd = this.settingsManager.getExternalEditorCommand();
|
||||||
if (!editorCmd) {
|
const content = this.editor.getExpandedText?.() ?? this.editor.getText();
|
||||||
this.showWarning("No editor configured. Set externalEditor in settings.json or $VISUAL/$EDITOR.");
|
this.ui.stop();
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const currentText = this.editor.getExpandedText?.() ?? this.editor.getText();
|
|
||||||
const tmpFile = path.join(os.tmpdir(), `pi-editor-${Date.now()}.pi.md`);
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Write current content to temp file
|
const result = await editInExternalEditor({
|
||||||
fs.writeFileSync(tmpFile, currentText, "utf-8");
|
command: editorCmd,
|
||||||
|
content,
|
||||||
// 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<number | null>((resolve) => {
|
|
||||||
const child = spawn(editor, [...editorArgs, tmpFile], {
|
|
||||||
stdio: "inherit",
|
|
||||||
shell: process.platform === "win32",
|
|
||||||
});
|
|
||||||
child.on("error", () => resolve(null));
|
|
||||||
child.on("close", (code) => resolve(code));
|
|
||||||
});
|
});
|
||||||
|
if (result.status === "complete") {
|
||||||
// On successful exit (status 0), replace editor content
|
this.editor.setText(result.content);
|
||||||
if (status === 0) {
|
|
||||||
const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, "");
|
|
||||||
this.editor.setText(newContent);
|
|
||||||
}
|
}
|
||||||
// On non-zero exit, keep original text (no action needed)
|
|
||||||
} finally {
|
} finally {
|
||||||
// Clean up temp file
|
|
||||||
try {
|
|
||||||
fs.unlinkSync(tmpFile);
|
|
||||||
} catch {
|
|
||||||
// Ignore cleanup errors
|
|
||||||
}
|
|
||||||
|
|
||||||
// Restart TUI
|
|
||||||
this.ui.start();
|
this.ui.start();
|
||||||
// Force full re-render since external editor uses alternate screen
|
|
||||||
this.ui.requestRender(true);
|
this.ui.requestRender(true);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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: "" });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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");
|
||||||
Reference in New Issue
Block a user