diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 28b869cb2..6c0997319 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,8 +8,12 @@ ### Changed +- `--help` no longer boots the engine to print a help screen. The usage text and the flags extensions register are answered from a cache of the last launch's flag set (`/cache/help-flags.json`, validated against the engine version and the mtime/size of every extension, settings and trust input, so an upgrade or an edited extension refreshes it); a cache miss loads extensions for their flags only and skips the model runtime, the session and every other resource class. Measured warm on an Apple M4 Pro: 790ms → 28ms on bun and 959ms → 59ms on node for `--help`; a help screen never prompts for project trust and never runs project-local extension code that is not already trusted. ([oh-my-openagent#8371](https://github.com/code-yeongyu/oh-my-openagent/issues/8371)) + ### Fixed +- The startup spinner is drawn the moment interactive startup begins instead of after a 120ms grace timer. That timer could not fire while the synchronous extension imports it was meant to cover were running, so on a real terminal the first frame appeared only once the whole load was done (measured: first byte at 2.27s, one frame before the TUI took over) and the load ran on a blank screen. ([oh-my-openagent#8371](https://github.com/code-yeongyu/oh-my-openagent/issues/8371)) + ### Removed ## [2026.9.16-2] - 2026-09-16 diff --git a/packages/coding-agent/src/changes.md b/packages/coding-agent/src/changes.md index e31d8110e..5b5354c4a 100644 --- a/packages/coding-agent/src/changes.md +++ b/packages/coding-agent/src/changes.md @@ -1,5 +1,25 @@ # changes +## 2026-09-16 - Answer `--help` without booting the engine (oh-my-openagent#8371) + +### What changed + +- `packages/coding-agent/src/cli.ts`: a plain root `--help`/`-h` is answered before `cli-main` is imported when `cli/help-fast-path.ts` finds a valid flags cache for this cwd, agent dir, `--extension` set and project-trust decision; the import stays dynamic for the same reason the `cli-main` import is. `--no-extensions` is answered without any cache. +- `packages/coding-agent/src/main.ts`: a plain `--help` stops right after CLI paths are resolved. It resolves extension flags through `cli/help-extension-flags.ts` (a `DefaultResourceLoader` with skills, prompt templates, themes and context files disabled; no `ModelRuntime`, no `SessionManager`, no `AgentSession`), prints help, writes `/cache/help-flags.json` through `cli/help-flags-cache.ts` and exits. Every full launch also refreshes that cache from the runtime's loaded extensions right after the late `parsed.help` branch, which now only serves `--help --mode json` / `-p --help`. +- Project trust for the help path is `--yolo`/override → recorded `trust.json` decision → trusted when the project carries no trust-requiring resources; it never prompts and never loads untrusted project extension code. + +### Why + +- oh-my-openagent#8371: `omo --help` measured 47.8s on Windows and 790ms warm / 8.8-13.6s cold on bun here, all spent building a runtime the help screen never uses. Cached help now costs 28ms (bun) / 59ms (node); a cache miss costs the extension load only. + +### Why an extension could not handle it + +- The help screen is printed by the host before any extension is bound, and the cost being removed is the host's own runtime construction. + +### Expected merge conflict zones + +- MEDIUM: `main.ts` around the `resolveCliPaths` block and the late `if (parsed.help)` branch; LOW: `cli.ts` next to the `--version` fast path. + ## 2026-09-16 - Print mode explains provider stalls (senpi#1740) ### What changed diff --git a/packages/coding-agent/src/cli.ts b/packages/coding-agent/src/cli.ts index 57dfc9631..5003119b8 100644 --- a/packages/coding-agent/src/cli.ts +++ b/packages/coding-agent/src/cli.ts @@ -132,6 +132,16 @@ if (isRootCommand(args) && (args.includes("--version") || args.includes("-v"))) process.exit(); } +// Help is static text plus the flags extensions registered, so a launch that already knows those +// flags must not import the engine graph to print them. The import stays dynamic for the same +// reason `cli-main` is: a static one would evaluate that graph before this answer. +if (isRootCommand(args) && args.some((arg) => arg === "--help" || arg === "-h")) { + const { tryPrintHelpWithoutEngine } = await import("./cli/help-fast-path.ts"); + if (tryPrintHelpWithoutEngine(args)) { + process.exit(); + } +} + if (isMissingBundledWorkspaceDependencies(getPackageDir())) { if (await handleBootstrapSelfUpdate(args)) { process.exit(); diff --git a/packages/coding-agent/src/cli/changes.md b/packages/coding-agent/src/cli/changes.md index 24f176966..06bf563d0 100644 --- a/packages/coding-agent/src/cli/changes.md +++ b/packages/coding-agent/src/cli/changes.md @@ -1,5 +1,23 @@ # changes +## 2026-09-16 - Startup spinner draws its first frame synchronously (oh-my-openagent#8371) + +### What changed + +- `packages/coding-agent/src/cli/startup-loading-indicator.ts`: `start()` writes the first frame itself (hidden cursor + label + phase) and the 120ms grace delay now gates only the animation interval; `resume()` redraws the same way before its grace timer. `setPhase()` therefore renders before any timer fires. + +### Why + +- The work the indicator covers is synchronous module loading (extension imports through jiti), which starves every timer until it finishes. Measured on a real pty during oh-my-openagent#8371: first spinner byte at 2.27s, a single frame before the TUI replaced it, the whole extension load on a blank terminal. A timer-driven first frame announces work that already ended. + +### Why an extension could not handle it + +- The indicator runs in the host before any extension is loaded; it is the thing extensions' own load time hides. + +### Expected merge conflict zones + +- LOW: `start()`, `resume()` and `beginAnimation()` bodies plus the class docstring; `test/startup-loading-indicator.test.ts` grace-delay cases. + ## 2026-09-10 - VENICE_API_KEY in the help output ### What changed diff --git a/packages/coding-agent/src/cli/help-extension-flags.ts b/packages/coding-agent/src/cli/help-extension-flags.ts new file mode 100644 index 000000000..5f9768985 --- /dev/null +++ b/packages/coding-agent/src/cli/help-extension-flags.ts @@ -0,0 +1,44 @@ +import type { ExtensionFlag, InlineExtension } from "../core/extensions/types.ts"; +import { DefaultResourceLoader } from "../core/resource-loader.ts"; +import type { SettingsManager } from "../core/settings-manager.ts"; + +export interface HelpExtensionFlagsResult { + readonly flags: ExtensionFlag[]; + readonly extensionPaths: string[]; +} + +/** + * Load extensions for their CLI flags and nothing else. + * + * Help renders flag descriptors, so this deliberately skips every other resource class and the + * whole model/session stack that `createAgentSessionServices` would build: skills, prompt + * templates, themes and context files cannot register a flag. + */ +export async function resolveHelpExtensionFlags(options: { + readonly cwd: string; + readonly agentDir: string; + readonly settingsManager: SettingsManager; + readonly additionalExtensionPaths: readonly string[]; + readonly noExtensions: boolean; + readonly extensionFactories?: readonly InlineExtension[]; +}): Promise { + const resourceLoader = new DefaultResourceLoader({ + cwd: options.cwd, + agentDir: options.agentDir, + settingsManager: options.settingsManager, + sharedHostEnabled: false, + additionalExtensionPaths: [...options.additionalExtensionPaths], + noExtensions: options.noExtensions, + noSkills: true, + noPromptTemplates: true, + noThemes: true, + noContextFiles: true, + ...(options.extensionFactories ? { extensionFactories: [...options.extensionFactories] } : {}), + }); + await resourceLoader.reload(); + const { extensions } = resourceLoader.getExtensions(); + return { + flags: extensions.flatMap((extension) => [...extension.flags.values()]), + extensionPaths: extensions.map((extension) => extension.resolvedPath), + }; +} diff --git a/packages/coding-agent/src/cli/help-fast-path.ts b/packages/coding-agent/src/cli/help-fast-path.ts new file mode 100644 index 000000000..9d1f01687 --- /dev/null +++ b/packages/coding-agent/src/cli/help-fast-path.ts @@ -0,0 +1,62 @@ +import { getAgentDir } from "../config.ts"; +import type { ExtensionFlag } from "../core/extensions/types.ts"; +import { hasTrustRequiringProjectResources, ProjectTrustStore } from "../core/trust-manager.ts"; +import { type Args, parseArgs, printHelp } from "./args.ts"; +import { type HelpFlagsScope, readHelpFlagsCache } from "./help-flags-cache.ts"; + +export function isPlainHelpRequest(parsed: Args): boolean { + return parsed.help === true && parsed.print !== true && parsed.mode === undefined; +} + +/** + * A help screen never prompts for project trust and never runs project-local extension code + * the user has not already trusted: only a recorded decision, or an explicit override, lets + * project resources in. + */ +export function resolveHelpProjectTrust(parsed: Args, cwd: string, agentDir: string): boolean { + if (parsed.projectTrustOverride !== undefined) return parsed.projectTrustOverride; + if (!hasTrustRequiringProjectResources(cwd)) return true; + return new ProjectTrustStore(agentDir).get(cwd) === true; +} + +export function helpFlagsScope(parsed: Args, cwd: string, agentDir: string, projectTrusted: boolean): HelpFlagsScope { + return { + cwd, + agentDir, + cliExtensionPaths: [...(parsed.extensions ?? [])], + noExtensions: parsed.noExtensions === true, + projectTrusted, + }; +} + +/** + * Answer `--help` before the engine module graph is imported. + * + * Returns false for every launch it cannot answer from what is already known - a non-plain help + * request, or a scope whose cached flags are missing or stale - and the normal startup path then + * resolves the flags and refills the cache. + */ +export function tryPrintHelpWithoutEngine(argv: readonly string[]): boolean { + let parsed: Args; + try { + parsed = parseArgs([...argv]); + } catch { + return false; + } + if (!isPlainHelpRequest(parsed) || parsed.diagnostics.length > 0) return false; + if (parsed.noExtensions === true) { + printHelp([]); + return true; + } + let flags: ExtensionFlag[] | undefined; + try { + const cwd = process.cwd(); + const agentDir = getAgentDir(); + flags = readHelpFlagsCache(helpFlagsScope(parsed, cwd, agentDir, resolveHelpProjectTrust(parsed, cwd, agentDir))); + } catch { + return false; + } + if (!flags) return false; + printHelp(flags); + return true; +} diff --git a/packages/coding-agent/src/cli/help-flags-cache.ts b/packages/coding-agent/src/cli/help-flags-cache.ts new file mode 100644 index 000000000..4c2f08bf1 --- /dev/null +++ b/packages/coding-agent/src/cli/help-flags-cache.ts @@ -0,0 +1,145 @@ +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { dirname, isAbsolute, join } from "node:path"; +import { CONFIG_DIR_NAME, DISPLAY_VERSION } from "../config.ts"; +import type { ExtensionFlag } from "../core/extensions/types.ts"; + +const CACHE_FILE_VERSION = 1; +const MAX_CACHED_SCOPES = 8; + +export interface HelpFlagsScope { + readonly cwd: string; + readonly agentDir: string; + readonly cliExtensionPaths: readonly string[]; + readonly noExtensions: boolean; + readonly projectTrusted: boolean; +} + +interface InputStamp { + readonly path: string; + readonly state: string; +} + +interface CachedScope { + readonly writtenAt: number; + readonly appVersion: string; + readonly inputs: readonly InputStamp[]; + readonly flags: readonly ExtensionFlag[]; +} + +interface CacheFile { + readonly version: number; + readonly scopes: Record; +} + +function cachePath(agentDir: string): string { + return join(agentDir, "cache", "help-flags.json"); +} + +function scopeKey(scope: HelpFlagsScope): string { + const material = [ + scope.cwd, + scope.noExtensions ? "no-extensions" : "extensions", + scope.projectTrusted ? "trusted" : "untrusted", + ...scope.cliExtensionPaths, + ].join("\u0000"); + return createHash("sha256").update(material).digest("hex").slice(0, 32); +} + +/** + * A path's identity for cache validation. Directories carry only their mtime, which is what + * changes when an extension file is added or removed inside them; files carry mtime and size, + * so an in-place upgrade of a bundled plugin invalidates the entry it produced. + */ +function stamp(path: string): string { + try { + const stats = statSync(path); + if (stats.isDirectory()) return `d:${stats.mtimeMs}`; + return `f:${stats.mtimeMs}:${stats.size}`; + } catch { + return "absent"; + } +} + +function discoveryInputs(scope: HelpFlagsScope, extensionPaths: readonly string[]): string[] { + const paths = new Set([ + join(scope.agentDir, "settings.json"), + join(scope.agentDir, "extensions"), + join(scope.agentDir, "trust.json"), + join(scope.cwd, CONFIG_DIR_NAME, "settings.json"), + join(scope.cwd, CONFIG_DIR_NAME, "extensions"), + ]); + for (const path of extensionPaths) { + if (!isAbsolute(path)) continue; + paths.add(path); + paths.add(dirname(path)); + } + for (const path of scope.cliExtensionPaths) { + if (!isAbsolute(path)) continue; + paths.add(path); + } + return [...paths].sort(); +} + +function readCacheFile(agentDir: string): CacheFile | undefined { + try { + const parsed = JSON.parse(readFileSync(cachePath(agentDir), "utf8")) as CacheFile; + if (parsed.version !== CACHE_FILE_VERSION || typeof parsed.scopes !== "object" || parsed.scopes === null) { + return undefined; + } + return parsed; + } catch { + return undefined; + } +} + +/** + * Flags a previous run resolved for this exact scope, or `undefined` when anything that feeds + * extension discovery changed. Never throws: a help screen must not depend on its own cache. + */ +export function readHelpFlagsCache(scope: HelpFlagsScope): ExtensionFlag[] | undefined { + const cached = readCacheFile(scope.agentDir)?.scopes[scopeKey(scope)]; + if (!cached || cached.appVersion !== DISPLAY_VERSION || !Array.isArray(cached.inputs)) return undefined; + for (const input of cached.inputs) { + if (stamp(input.path) !== input.state) return undefined; + } + return [...cached.flags]; +} + +/** + * Record the flags a full extension load produced. Failure is silent by contract: this runs on + * the startup path, where a cache write must never be the reason a launch fails. + */ +export function writeHelpFlagsCache(options: { + readonly scope: HelpFlagsScope; + readonly flags: readonly ExtensionFlag[]; + readonly extensionPaths: readonly string[]; +}): void { + const { scope, flags, extensionPaths } = options; + try { + const existing = readCacheFile(scope.agentDir); + const scopes: Record = { ...(existing?.scopes ?? {}) }; + scopes[scopeKey(scope)] = { + writtenAt: Date.now(), + appVersion: DISPLAY_VERSION, + inputs: discoveryInputs(scope, extensionPaths).map((path) => ({ path, state: stamp(path) })), + flags: [...flags], + }; + const keptEntries = Object.entries(scopes) + .sort(([, left], [, right]) => right.writtenAt - left.writtenAt) + .slice(0, MAX_CACHED_SCOPES); + const file: CacheFile = { version: CACHE_FILE_VERSION, scopes: Object.fromEntries(keptEntries) }; + const target = cachePath(scope.agentDir); + mkdirSync(dirname(target), { recursive: true }); + const temporary = `${target}.${process.pid}.tmp`; + writeFileSync(temporary, JSON.stringify(file), { mode: 0o600 }); + try { + renameSync(temporary, target); + } catch (error) { + if (existsSync(temporary)) rmSync(temporary, { force: true }); + throw error; + } + } catch { + return; + } +} diff --git a/packages/coding-agent/src/cli/startup-loading-indicator.ts b/packages/coding-agent/src/cli/startup-loading-indicator.ts index 3c14380c2..d8714b75e 100644 --- a/packages/coding-agent/src/cli/startup-loading-indicator.ts +++ b/packages/coding-agent/src/cli/startup-loading-indicator.ts @@ -33,9 +33,12 @@ export interface StartupLoadingIndicator { /** * Single-line ANSI loading indicator for the pre-TUI startup window, borrowed * from codex's UI-first startup design (codex-rs/tui keeps a dim placeholder - * header until the session is configured). The grace delay keeps fast startups - * flash-free; stop() must run before any other stdout writer (TUI, prompts, - * help) takes over the terminal. + * header until the session is configured). The first frame is written in + * start() itself: the work it covers is synchronous module loading, which + * starves every timer until it is done, so a timer-driven first frame lands + * only after that work (measured: 2.27s to the first byte on a real pty). The + * grace delay now gates the animation only; stop() must run before any other + * stdout writer (TUI, prompts, help) takes over the terminal. */ class AnsiStartupLoadingIndicator implements StartupLoadingIndicator { private readonly writer: (chunk: string) => void; @@ -75,6 +78,7 @@ class AnsiStartupLoadingIndicator implements StartupLoadingIndicator { if (this.drawn) this.writer(CLEAR_LINE + SHOW_CURSOR); }; process.on("exit", this.exitListener); + this.draw(true); this.startGraceTimer(); } @@ -98,6 +102,7 @@ class AnsiStartupLoadingIndicator implements StartupLoadingIndicator { if (this.graceElapsed) { this.beginAnimation(); } else { + this.draw(true); this.startGraceTimer(); } } @@ -121,7 +126,7 @@ class AnsiStartupLoadingIndicator implements StartupLoadingIndicator { } private beginAnimation(): void { - this.draw(true); + if (!this.drawn) this.draw(true); this.frameTimer = setInterval(() => { this.frameIndex = (this.frameIndex + 1) % this.frames.length; this.draw(false); diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 0ac3c66d5..caf7ec764 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -31,6 +31,9 @@ import { } from "./cli/auth-command.ts"; import { resolveCredentialForPrint } from "./cli/credential-print.ts"; import { processFileArguments } from "./cli/file-processor.ts"; +import { resolveHelpExtensionFlags } from "./cli/help-extension-flags.ts"; +import { helpFlagsScope, isPlainHelpRequest, resolveHelpProjectTrust } from "./cli/help-fast-path.ts"; +import { writeHelpFlagsCache } from "./cli/help-flags-cache.ts"; import { buildInitialMessage } from "./cli/initial-message.ts"; import { listModels } from "./cli/list-models.ts"; import { listTips } from "./cli/list-tips.ts"; @@ -1014,6 +1017,26 @@ export async function main(args: string[], options?: MainOptions) { const resolvedPromptTemplatePaths = resolveCliPaths(cwd, parsed.promptTemplates); const resolvedThemePaths = resolveCliPaths(cwd, parsed.themes); + // Help is answered from the flags alone, so it stops here instead of continuing into the model + // runtime, the session manager and the rest of the resource load. The flags are cached for the + // next run, which `cli.ts` then answers before this module is even imported. + if (isPlainHelpRequest(parsed)) { + const projectTrusted = resolveHelpProjectTrust(parsed, cwd, agentDir); + const scope = helpFlagsScope(parsed, cwd, agentDir, projectTrusted); + const { flags, extensionPaths } = await resolveHelpExtensionFlags({ + cwd, + agentDir, + settingsManager: SettingsManager.create(cwd, agentDir, { projectTrusted }), + additionalExtensionPaths: resolvedExtensionPaths ?? [], + noExtensions: parsed.noExtensions === true, + ...(extensionFactories ? { extensionFactories } : {}), + }); + printHelp(flags); + writeHelpFlagsCache({ scope, flags, extensionPaths }); + printTimings(); + process.exit(0); + } + if (parsed.listTips) { listTips(); process.exit(0); @@ -1181,13 +1204,19 @@ export async function main(args: string[], options?: MainOptions) { applyHttpProxySettings(settingsManager.getGlobalSettings().httpProxy); configureHttpDispatcher(settingsManager.getHttpIdleTimeoutMs()); + const loadedExtensions = resourceLoader.getExtensions().extensions; + const extensionFlags = loadedExtensions.flatMap((extension) => Array.from(extension.flags.values())); if (parsed.help) { - const extensionFlags = resourceLoader - .getExtensions() - .extensions.flatMap((extension) => Array.from(extension.flags.values())); printHelp(extensionFlags); process.exit(0); } + // Every full launch refreshes what `--help` reads, so the fast path stays warm without a help + // run of its own. + writeHelpFlagsCache({ + scope: helpFlagsScope(parsed, cwd, agentDir, settingsManager.isProjectTrusted()), + flags: extensionFlags, + extensionPaths: loadedExtensions.map((extension) => extension.resolvedPath), + }); // Read piped stdin content (if any) - skip for RPC mode which uses stdin for JSON-RPC let stdinContent: string | undefined; diff --git a/packages/coding-agent/test/cli-inprocess-fast-path.test.ts b/packages/coding-agent/test/cli-inprocess-fast-path.test.ts index df3d0e1db..4a21d3779 100644 --- a/packages/coding-agent/test/cli-inprocess-fast-path.test.ts +++ b/packages/coding-agent/test/cli-inprocess-fast-path.test.ts @@ -98,11 +98,12 @@ describe("CLI in-process fast path", () => { describe("#given a launch that inherits an Inspector option", () => { test("#when NODE_OPTIONS carries --inspect #then the agent still runs in a spawned child", () => { - // Port 0 lets the OS pick a free port, so concurrent runs cannot collide. - const result = runCli(["--help"], "--inspect=127.0.0.1:0"); + // Port 0 lets the OS pick a free port, so concurrent runs cannot collide. The launch must + // reach the agent: a cached `--help` is answered before the isolation decision, because no + // agent runs for a help screen and there is no socket to hand over. + const result = runCli(["--model", "definitely-not-a-real-model-id", "--print", "hi"], "--inspect=127.0.0.1:0"); - expect(result.status, result.stderr).toBe(0); - expect(result.stdout).toContain("Usage:"); + expect(result.status).toBe(1); expect(result.records).toHaveLength(2); const entries = result.records.map((record) => record.entry); expect(entries).toContain(CLI_PATH); diff --git a/packages/coding-agent/test/startup-loading-indicator.test.ts b/packages/coding-agent/test/startup-loading-indicator.test.ts index 85c8c2486..50df7ccfd 100644 --- a/packages/coding-agent/test/startup-loading-indicator.test.ts +++ b/packages/coding-agent/test/startup-loading-indicator.test.ts @@ -36,20 +36,13 @@ describe("createStartupLoadingIndicator", () => { vi.useRealTimers(); }); - it("writes nothing before the grace delay and nothing when stopped within it", () => { + // The startup work this indicator covers is synchronous module loading (extension imports), + // which starves every timer until it is done. A first frame that waits on a timer is drawn + // only after the work it was meant to announce - measured on a real pty: first byte at 2.27s, + // one frame before the TUI took over. The first frame therefore draws in start() itself. + it("draws the first frame synchronously in start() with a hidden cursor", () => { const { indicator, writes } = makeIndicator(); indicator.start(); - vi.advanceTimersByTime(119); - expect(writes).toEqual([]); - indicator.stop(); - vi.advanceTimersByTime(1000); - expect(writes).toEqual([]); - }); - - it("draws the first frame after the grace delay with a hidden cursor", () => { - const { indicator, writes } = makeIndicator(); - indicator.start(); - vi.advanceTimersByTime(120); expect(writes).toHaveLength(1); expect(writes[0]).toContain(HIDE_CURSOR); expect(writes[0]).toContain(CLEAR_LINE); @@ -57,10 +50,24 @@ describe("createStartupLoadingIndicator", () => { expect(writes[0]).toContain("Loading senpi"); }); - it("animates frames on the interval, rewriting a single line", () => { + it("erases the line and restores the cursor when stopped within the grace delay", () => { const { indicator, writes } = makeIndicator(); indicator.start(); - vi.advanceTimersByTime(120); + vi.advanceTimersByTime(119); + expect(writes).toHaveLength(1); + indicator.stop(); + expect(writes).toHaveLength(2); + expect(writes[1]).toBe(CLEAR_LINE + SHOW_CURSOR); + vi.advanceTimersByTime(1000); + expect(writes).toHaveLength(2); + }); + + it("animates frames on the interval after the grace delay, rewriting a single line", () => { + const { indicator, writes } = makeIndicator(); + indicator.start(); + vi.advanceTimersByTime(119); + expect(writes).toHaveLength(1); + vi.advanceTimersByTime(1); vi.advanceTimersByTime(80); vi.advanceTimersByTime(80); expect(writes).toHaveLength(3); @@ -70,9 +77,12 @@ describe("createStartupLoadingIndicator", () => { expect(writes[2]).toContain("C"); }); - it("setPhase updates the rendered line immediately once drawing", () => { + it("setPhase updates the rendered line immediately, before any timer fires", () => { const { indicator, writes } = makeIndicator(); indicator.start(); + indicator.setPhase("extensions & models"); + expect(writes).toHaveLength(2); + expect(writes.at(-1)).toContain("extensions & models"); vi.advanceTimersByTime(120); indicator.setPhase("opening session"); expect(writes.at(-1)).toContain("opening session"); diff --git a/packages/coding-agent/test/suite/regressions/help-fast-path.test.ts b/packages/coding-agent/test/suite/regressions/help-fast-path.test.ts new file mode 100644 index 000000000..113ae9d66 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/help-fast-path.test.ts @@ -0,0 +1,195 @@ +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +/** + * `--help` used to boot the whole engine before it could print one line: migrations, the + * settings manager, `ModelRuntime.create()` (models.json + models-store.json + availability), + * the full resource load (extensions AND skills, prompt templates, themes, context files) and + * an `AgentSession`. Measured cost of that boot for a help screen: 790ms warm on bun, 959ms on + * node, 8.8-13.6s cold — and 47.8s for the reporter of oh-my-openagent#8371 on Windows. + * + * Help needs exactly two things: the static usage text and the flags extensions registered. + * These tests pin that contract through observable side effects, never timings: + * - a probe extension appends one line per factory invocation, so the line count IS the number + * of times extensions were loaded; + * - `models-store.json` exists only if the model runtime was constructed; + * - the project directory must stay untouched by a help screen. + */ + +const CLI_PATH = fileURLToPath(new URL("../../../src/cli.ts", import.meta.url)); +// The child runs in a temp project directory, where a bare `tsx` specifier cannot resolve. +const TSX_LOADER_URL = pathToFileURL(createRequire(import.meta.url).resolve("tsx")).href; + +function probeExtensionSource(flagName: string): string { + return `import { appendFileSync } from "node:fs"; + +export default function helpProbeExtension(pi) { + appendFileSync(process.env.SENPI_HELP_PROBE_REPORT, "loaded\\n"); + pi.registerFlag("${flagName}", { + type: "boolean", + default: false, + description: "help fast path probe flag", + }); +} +`; +} + +interface HelpRun { + readonly status: number | null; + readonly stdout: string; + readonly stderr: string; + readonly loads: number; +} + +let hostDir: string; +let agentDir: string; +let projectDir: string; +let reportPath: string; + +function runHelp(args: readonly string[] = ["--help"]): HelpRun { + // A child spawned from inside an agent session inherits OMO_/SENPI_ agent-dir lanes and + // SENPI_BRAND, and the OMO lane wins brand resolution; both must be dropped so the child + // reads this test's temp agent directory (test/AGENTS.md quarantine contract). + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value === undefined) continue; + if (key === "SENPI_BRAND" || key.endsWith("_CODING_AGENT_DIR")) continue; + env[key] = value; + } + env.NODE_OPTIONS = `--import ${TSX_LOADER_URL}`; + env.SENPI_CODING_AGENT_DIR = agentDir; + env.SENPI_HELP_PROBE_REPORT = reportPath; + env.PI_OFFLINE = "1"; + + const result = spawnSync(process.execPath, [CLI_PATH, ...args], { + encoding: "utf8", + cwd: projectDir, + env, + }); + const loads = readFileSync(reportPath, "utf8") + .split("\n") + .filter((line) => line.length > 0).length; + return { status: result.status, stdout: result.stdout, stderr: result.stderr, loads }; +} + +beforeEach(() => { + hostDir = mkdtempSync(join(tmpdir(), "senpi-help-fast-path-")); + agentDir = join(hostDir, "agent"); + projectDir = join(hostDir, "project"); + reportPath = join(hostDir, "probe-report.txt"); + mkdirSync(join(agentDir, "extensions"), { recursive: true }); + mkdirSync(projectDir, { recursive: true }); + writeFileSync(reportPath, ""); + writeFileSync(join(agentDir, "extensions", "help-probe.js"), probeExtensionSource("help-probe-alpha")); +}); + +afterEach(() => { + rmSync(hostDir, { recursive: true, force: true }); +}); + +describe("help fast path (oh-my-openagent#8371)", () => { + describe("#given a first help run with no cached flags", () => { + test("#when --help runs #then it prints usage plus extension flags without building the model runtime", () => { + const run = runHelp(); + + expect(run.status, run.stderr).toBe(0); + expect(run.stdout).toContain("Usage:"); + expect(run.stdout).toContain("--help-probe-alpha"); + expect(run.loads).toBe(1); + // The model runtime is what writes this file; a help screen must never construct it. + expect(existsSync(join(agentDir, "models-store.json"))).toBe(false); + expect(readdirSync(projectDir)).toEqual([]); + }); + }); + + describe("#given help has already resolved the flags once", () => { + test("#when --help runs again #then the same output is printed without loading extensions again", () => { + const first = runHelp(); + expect(first.status, first.stderr).toBe(0); + expect(first.loads).toBe(1); + + const second = runHelp(); + + expect(second.status, second.stderr).toBe(0); + expect(second.stdout).toContain("--help-probe-alpha"); + expect(second.stdout).toBe(first.stdout); + // The cached answer must not re-enter extension loading. + expect(second.loads).toBe(1); + }); + }); + + describe("#given an extension changed after the flags were cached", () => { + test("#when --help runs #then the cache is rejected and the new flag is listed", () => { + expect(runHelp().loads).toBe(1); + writeFileSync(join(agentDir, "extensions", "help-probe.js"), probeExtensionSource("help-probe-beta")); + + const run = runHelp(); + + expect(run.status, run.stderr).toBe(0); + expect(run.stdout).toContain("--help-probe-beta"); + expect(run.stdout).not.toContain("--help-probe-alpha"); + expect(run.loads).toBe(2); + }); + }); + + describe("#given a new extension appeared after the flags were cached", () => { + test("#when --help runs #then the cache is rejected and both flags are listed", () => { + expect(runHelp().loads).toBe(1); + writeFileSync(join(agentDir, "extensions", "help-probe-2.js"), probeExtensionSource("help-probe-gamma")); + + const run = runHelp(); + + expect(run.status, run.stderr).toBe(0); + expect(run.stdout).toContain("--help-probe-alpha"); + expect(run.stdout).toContain("--help-probe-gamma"); + expect(run.loads).toBe(3); + }); + }); + + describe("#given settings changed after the flags were cached", () => { + test("#when --help runs #then the cache is rejected and extensions are loaded again", () => { + expect(runHelp().loads).toBe(1); + writeFileSync(join(agentDir, "settings.json"), JSON.stringify({ theme: "dark" })); + + const run = runHelp(); + + expect(run.status, run.stderr).toBe(0); + expect(run.stdout).toContain("--help-probe-alpha"); + expect(run.loads).toBe(2); + }); + }); + + describe("#given the project directory carries an extension the user never trusted", () => { + test("#when --help runs #then the project extension is neither loaded nor listed", () => { + const projectExtensionsDir = join(projectDir, ".senpi", "extensions"); + mkdirSync(projectExtensionsDir, { recursive: true }); + writeFileSync(join(projectExtensionsDir, "project-probe.js"), probeExtensionSource("project-probe-delta")); + + const first = runHelp(); + const second = runHelp(); + + expect(first.status, first.stderr).toBe(0); + expect(first.stdout).toContain("--help-probe-alpha"); + expect(first.stdout).not.toContain("--project-probe-delta"); + expect(first.loads).toBe(1); + expect(second.stdout).toBe(first.stdout); + expect(second.loads).toBe(1); + }); + }); + + describe("#given extensions are disabled for this run", () => { + test("#when --help --no-extensions runs #then usage prints with no extension flags and no extension load", () => { + const run = runHelp(["--help", "--no-extensions"]); + + expect(run.status, run.stderr).toBe(0); + expect(run.stdout).toContain("Usage:"); + expect(run.stdout).not.toContain("--help-probe-alpha"); + expect(run.loads).toBe(0); + }); + }); +});