diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index da7f522ee..5e6aebf62 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -20,6 +20,7 @@ - Fixed a bug where goal mode was incorrectly deactivated/set to 'none' on every wall-clock-only update (when tokenDelta <= 0) during tool execution flushes, preventing OMP from writing a mode change to 'none' in the session history database while keeping in-memory/UI state fresh. - Fixed a bug where Gemini MALFORMED_FUNCTION_CALL tool-generation errors (which are transient) surfaced as terminal error blocks. Added "malformed function call" to the transient transport error classifier so the session automatically retries the turn. - Fixed Ctrl+C teardown waiting up to 30s when an extension's `session_shutdown` handler hung — observed on Windows with `omp-discord-presence` 0.1.2 stuck on a stale Discord IPC pipe. `ExtensionRunner.emit` previously shared the generic 30s `EXTENSION_HANDLER_TIMEOUT_MS` budget for every event, including the fire-and-forget teardown event extensions cannot observe. `session_shutdown` now uses a dedicated 2s `SESSION_SHUTDOWN_HANDLER_TIMEOUT_MS` cap routed through a per-event `handlerTimeoutForEvent()` lookup, so a hung third-party handler can no longer hold dispose hostage. As a defence-in-depth ladder, a Ctrl+C arriving while interactive shutdown is already running now hard-exits with code 130 (the session JSONL has already been sync-flushed by the first press), surfaced via a new read-only `InteractiveModeContext.isShuttingDown` ([#2600](https://github.com/can1357/oh-my-pi/issues/2600)). +- Fixed the external editor flow (Ctrl+G, plan editor, `/todo edit`) warning `No editor configured` on Windows even when the user expected the system's native editor. `getEditorCommand()` now falls back to `notepad` on `win32` after consulting `$VISUAL`/`$EDITOR` (and trims those values so accidental whitespace is ignored), so Windows users get a working editor out of the box while POSIX still warns to nudge configuration ([#2604](https://github.com/can1357/oh-my-pi/issues/2604)). - Fixed Kokoro TTS setup loading the workspace/global `@huggingface/transformers` runtime before the side-installed Kokoro runtime, which could leave `onnxruntime-node@1.26.0` bound to an older `libonnxruntime.so.1` and fail with `VERS_1.26.0` missing ([#2591](https://github.com/can1357/oh-my-pi/issues/2591)). - Fixed `scripts/ci-release-notes.ts` stranding curated changelog entries from intervening *silent* tags (a `vX.Y.Z` tag pushed without a GitHub Release, e.g. the `v15.12.5`/`v15.12.6` casualties of the pre-#2564 release-cancellation bug). The generator now walks `(latest-published-release, target]` — resolved via `gh release list` from the `release_github` CI job — and merges every in-range `## [X.Y.Z]` section per package, grouped by `### ` with bullet-level dedupe so post-release changelog flattening cannot duplicate entries. Falls back to the legacy single-version extraction when no prior published release resolves, and `OMP_RELEASE_NOTES_FLOOR=v15.12.4` overrides the lookup for manual rebuilds ([#2596](https://github.com/can1357/oh-my-pi/issues/2596)). - Fixed `Test & smoke (TS)` CI timeouts caused by parallel test files racing on the process-global Settings singleton. `CustomEditor` now accepts a `magicKeywordsEnabledOverride` injection point so the shimmer-gate test can assert behaviour without calling `resetSettingsForTest()` / `Settings.init()`; the "streaming tool call preview height" describe drops its gratuitous Settings reset+init. Production wiring is unchanged ([#2582](https://github.com/can1357/oh-my-pi/issues/2582)) diff --git a/packages/coding-agent/src/utils/external-editor.ts b/packages/coding-agent/src/utils/external-editor.ts index 4050905cc..60bcc238c 100644 --- a/packages/coding-agent/src/utils/external-editor.ts +++ b/packages/coding-agent/src/utils/external-editor.ts @@ -7,9 +7,22 @@ import * as os from "node:os"; import * as path from "node:path"; import { $env, Snowflake } from "@oh-my-pi/pi-utils"; -/** Returns the user's preferred editor command, or undefined if not configured. */ +/** + * Returns the user's preferred editor command, or a platform default. + * + * Resolution order: + * 1. `$VISUAL` + * 2. `$EDITOR` + * 3. `notepad` on Windows (always present in `%SystemRoot%\System32`) + * + * POSIX returns `undefined` when neither variable is set so the caller can + * surface a warning that nudges the user to configure one. + */ export function getEditorCommand(): string | undefined { - return $env.VISUAL || $env.EDITOR || undefined; + const configured = $env.VISUAL?.trim() || $env.EDITOR?.trim(); + if (configured) return configured; + if (process.platform === "win32") return "notepad"; + return undefined; } export interface OpenInEditorOptions { 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 000000000..0e1b4eacd --- /dev/null +++ b/packages/coding-agent/test/external-editor.test.ts @@ -0,0 +1,63 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { getEditorCommand } from "../src/utils/external-editor"; + +interface MutableProcess { + platform: NodeJS.Platform; +} + +function setPlatform(value: NodeJS.Platform): void { + (process as unknown as MutableProcess).platform = value; +} + +describe("getEditorCommand", () => { + const originalPlatform = process.platform; + const originalVisual = Bun.env.VISUAL; + const originalEditor = Bun.env.EDITOR; + + afterEach(() => { + setPlatform(originalPlatform); + if (originalVisual === undefined) delete Bun.env.VISUAL; + else Bun.env.VISUAL = originalVisual; + if (originalEditor === undefined) delete Bun.env.EDITOR; + else Bun.env.EDITOR = originalEditor; + }); + + it("prefers $VISUAL over $EDITOR and the platform default", () => { + Bun.env.VISUAL = "nvim"; + Bun.env.EDITOR = "nano"; + setPlatform("win32"); + expect(getEditorCommand()).toBe("nvim"); + }); + + it("falls back to $EDITOR when $VISUAL is unset", () => { + delete Bun.env.VISUAL; + Bun.env.EDITOR = "nano"; + expect(getEditorCommand()).toBe("nano"); + }); + + it("trims whitespace so an accidentally padded value still works", () => { + Bun.env.VISUAL = " code --wait "; + delete Bun.env.EDITOR; + expect(getEditorCommand()).toBe("code --wait"); + }); + + it("treats a whitespace-only $VISUAL as unset and consults $EDITOR", () => { + Bun.env.VISUAL = " "; + Bun.env.EDITOR = "vim"; + expect(getEditorCommand()).toBe("vim"); + }); + + it("defaults to notepad on Windows when neither variable is set", () => { + delete Bun.env.VISUAL; + delete Bun.env.EDITOR; + setPlatform("win32"); + expect(getEditorCommand()).toBe("notepad"); + }); + + it("returns undefined on POSIX when neither variable is set", () => { + delete Bun.env.VISUAL; + delete Bun.env.EDITOR; + setPlatform("linux"); + expect(getEditorCommand()).toBeUndefined(); + }); +});