From fb75ba481d6a82893b28a48c10b4b47b37b6a46f Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 15 Jun 2026 02:34:06 +0000 Subject: [PATCH] fix(editor): default to notepad on Windows when $VISUAL/$EDITOR are unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The external editor flow (Ctrl+G, plan editor, /todo edit) warned 'No editor configured' on Windows because getEditorCommand() returned undefined whenever neither $VISUAL nor $EDITOR was set — the default state for most Windows shells. Fall back to 'notepad' on win32 after consulting $VISUAL/$EDITOR (always present in %SystemRoot%\\System32) and trim env values so accidentally padded strings still resolve. POSIX still returns undefined so the warning continues to nudge users to configure an editor. Fixes #2604 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/utils/external-editor.ts | 17 ++++- .../coding-agent/test/external-editor.test.ts | 63 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/external-editor.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 06d11d2e8..5f74fe196 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -165,6 +165,7 @@ ### Fixed +- 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(); + }); +});