From 435763c30f8cc925819f6d52f42ee70ef62089f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 06:34:15 +0000 Subject: [PATCH 1/2] Wire the startup sequence in cli (Task 1.15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements Req 12.1/12.2 (design.md §3, §15): packages/cli/src/main.ts now runs the real two-phase startup — a sync phase (argv resolution, config + keybindings load, terminal-capability stub, shell render = "first frame") followed by a deferred phase scheduled via queueMicrotask (extension discovery/registration, onStartup activation, initial file open) — closing out Phase 1: `bun run cli ` opens the real UI shell with a working command registry, keymap, documents, and config. Key wiring decisions: - The extension host is forward-referenced through a mutable `hostRef` box so commands/documents/slotRegistry (built in the sync phase) can reach activation once the host exists (built in the deferred phase, after discovery) — the same pattern host/activation.ts's own TSDoc documents. - One SlotRegistry instance is built up front (empty pendingViews, since discovery hasn't run yet) and shared by both createTecodeApi and the Shell's render props, so extension registerView() calls are visible to the same registry the Shell subscribes to; onDidChange makes late arrival after activation work correctly. Documented deviation: manifest views not eagerly activated get no lazy placeholder in this wiring, since pendingViews can only be known after first frame. - The render seam (renderShell.tsx) defaults to a real @opentui/react render, with a headless no-op used under TECODE_HEADLESS=1 or whenever stdout isn't a real TTY, so tests and CI never open a TTY. - extensionRecords.ts is cli's sanctioned composition-layer dynamic import() of extension index.ts/js files, documented inline (mirroring discovery.ts's manifest-load convention) rather than needing an eslint-disable — the layering rule's selector only matches literal "@tecode/core" specifiers. - A subprocess integration test proves ordering end-to-end: a disposable fixture extension logs its own module-load time, compared against the first-frame timing line emitted by the real child process. Also: packages/builtin/index.ts adds the (currently empty) built-in manifest aggregator; @tecode/core's barrel re-exports getUserConfigDir for extension storage-path wiring; root package.json gains a `cli` script. bun test: 519 pass (up from the 493 baseline), bun run lint clean, bunx tsc --noEmit clean. Measured first-frame time: ~6-15ms in this sandbox, well under the 100ms budget. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- bun.lock | 7 + package.json | 3 +- packages/builtin/index.ts | 29 ++ packages/builtin/package.json | 2 + packages/cli/package.json | 9 +- packages/cli/src/argv.test.ts | 92 ++++ packages/cli/src/argv.ts | 90 ++++ packages/cli/src/extensionRecords.test.ts | 126 ++++++ packages/cli/src/extensionRecords.ts | 102 +++++ packages/cli/src/keymapState.test.ts | 59 +++ packages/cli/src/keymapState.ts | 79 ++++ packages/cli/src/main.integration.test.ts | 131 ++++++ packages/cli/src/main.test.ts | 190 +++++++- packages/cli/src/main.ts | 408 ++++++++++++++++-- packages/cli/src/renderShell.test.ts | 41 ++ packages/cli/src/renderShell.tsx | 82 ++++ packages/cli/src/terminalCapabilities.test.ts | 15 + packages/cli/src/terminalCapabilities.ts | 39 ++ packages/core/src/index.ts | 1 + 19 files changed, 1460 insertions(+), 45 deletions(-) create mode 100644 packages/builtin/index.ts create mode 100644 packages/cli/src/argv.test.ts create mode 100644 packages/cli/src/argv.ts create mode 100644 packages/cli/src/extensionRecords.test.ts create mode 100644 packages/cli/src/extensionRecords.ts create mode 100644 packages/cli/src/keymapState.test.ts create mode 100644 packages/cli/src/keymapState.ts create mode 100644 packages/cli/src/main.integration.test.ts create mode 100644 packages/cli/src/renderShell.test.ts create mode 100644 packages/cli/src/renderShell.tsx create mode 100644 packages/cli/src/terminalCapabilities.test.ts create mode 100644 packages/cli/src/terminalCapabilities.ts diff --git a/bun.lock b/bun.lock index de65a37..f855468 100644 --- a/bun.lock +++ b/bun.lock @@ -30,8 +30,15 @@ "tecode": "src/main.ts", }, "dependencies": { + "@opentui/core": "^0.1.107", + "@opentui/react": "^0.1.107", "@tecode/api": "workspace:*", + "@tecode/builtin": "workspace:*", "@tecode/core": "workspace:*", + "react": "^19.0.0", + }, + "devDependencies": { + "@types/react": "^19.0.0", }, }, "packages/core": { diff --git a/package.json b/package.json index 68260e7..d9d889b 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ ], "scripts": { "test": "bun test", - "lint": "eslint ." + "lint": "eslint .", + "cli": "bun packages/cli/src/main.ts" }, "devDependencies": { "@eslint/js": "^9.19.0", diff --git a/packages/builtin/index.ts b/packages/builtin/index.ts new file mode 100644 index 0000000..769dc1c --- /dev/null +++ b/packages/builtin/index.ts @@ -0,0 +1,29 @@ +/** + * Aggregates every built-in extension's manifest into one array, for + * `@tecode/core`'s `loadExtensions({ builtins, ... })` dependency (Req 2.1; + * design.md §4.1, §4.4: "Built-ins are compiled into the binary as + * ordinary imports [...] their manifest data in a static registry"). + * + * `discovery.ts` never scans this package's directories off disk — a + * built-in's manifest reaches the host as a plain compiled-in `import`, + * not through the `user`/`workspace` filesystem-scanning path (which is + * exactly why `discovery.ts`'s `DiscoveryDeps.builtins` exists as a + * separate parameter rather than a third scanned directory). + * + * **Today this is `[]`.** Every `packages/builtin/*` package + * (`command-palette`, `editor-core`, `explorer`, `keybindings-editor`, + * `languages-basic`, `statusbar`, `themes-default`) is still a placeholder + * with no `manifest.ts` (each is its own later task — see tasks.md's Phase + * 2/3 built-in tasks). This module is `packages/cli`'s one composition + * point for the "compiled-in built-ins" list (Task 1.15) so wiring a real + * built-in later is exactly "add its manifest import and push it into + * `builtinManifests` below," not a new call site or a new dependency for + * `cli` to pick up. + */ + +import type { Manifest } from "@tecode/api"; + +/** Every built-in extension's manifest, compiled in as a static import + * (this module's TSDoc). Empty until a `packages/builtin/*` package gains + * a real `manifest.ts` and is added here. */ +export const builtinManifests: Manifest[] = []; diff --git a/packages/builtin/package.json b/packages/builtin/package.json index a884a3b..fccb5f7 100644 --- a/packages/builtin/package.json +++ b/packages/builtin/package.json @@ -3,6 +3,8 @@ "version": "0.1.0", "private": true, "type": "module", + "main": "index.ts", + "types": "index.ts", "dependencies": { "@tecode/api": "workspace:*" } diff --git a/packages/cli/package.json b/packages/cli/package.json index 819d667..3250a89 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -9,6 +9,13 @@ }, "dependencies": { "@tecode/api": "workspace:*", - "@tecode/core": "workspace:*" + "@tecode/builtin": "workspace:*", + "@tecode/core": "workspace:*", + "@opentui/core": "^0.1.107", + "@opentui/react": "^0.1.107", + "react": "^19.0.0" + }, + "devDependencies": { + "@types/react": "^19.0.0" } } diff --git a/packages/cli/src/argv.test.ts b/packages/cli/src/argv.test.ts new file mode 100644 index 0000000..e013e53 --- /dev/null +++ b/packages/cli/src/argv.test.ts @@ -0,0 +1,92 @@ +import { afterEach, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { createHostLog, type HostLogEntry } from "@tecode/core"; +import { resolveStartupTarget } from "./argv"; + +let dir: string | undefined; + +afterEach(async () => { + if (dir) await rm(dir, { recursive: true, force: true }); + dir = undefined; +}); + +test("no positional argument resolves to cwd with no initial file", async () => { + const log = createHostLog(); + const target = await resolveStartupTarget([], "/some/cwd", log); + expect(target).toEqual({ workspaceRoot: "/some/cwd" }); + expect(log.entries()).toEqual([]); +}); + +test("a directory argument becomes workspaceRoot with no initial file", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + const log = createHostLog(); + const target = await resolveStartupTarget([dir], "/irrelevant", log); + expect(target).toEqual({ workspaceRoot: dir }); +}); + +test("a file argument's parent directory becomes workspaceRoot, and the file is the initial file", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + const filePath = join(dir, "notes.txt"); + await writeFile(filePath, "hello", "utf8"); + + const log = createHostLog(); + const target = await resolveStartupTarget([filePath], "/irrelevant", log); + expect(target).toEqual({ workspaceRoot: dir, initialFilePath: filePath }); +}); + +test("a relative path argument resolves against cwd", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + await mkdir(join(dir, "sub"), { recursive: true }); + + const log = createHostLog(); + const target = await resolveStartupTarget(["sub"], dir, log); + expect(target).toEqual({ workspaceRoot: join(dir, "sub") }); +}); + +test("a nonexistent path logs a warning and falls back to cwd", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + const missing = join(dir, "does-not-exist"); + + const log = createHostLog(); + const target = await resolveStartupTarget([missing], "/fallback-cwd", log); + expect(target).toEqual({ workspaceRoot: "/fallback-cwd" }); + + const entries: readonly HostLogEntry[] = log.entries(); + expect(entries.length).toBe(1); + expect(entries[0]?.level).toBe("warning"); + expect(entries[0]?.error.message).toContain(missing); +}); + +test("only the first non-flag token is treated as the positional argument", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + const log = createHostLog(); + const target = await resolveStartupTarget(["--verbose", dir], "/irrelevant", log); + expect(target).toEqual({ workspaceRoot: dir }); +}); + +test("uses the injected fs seam instead of touching real disk", async () => { + const log = createHostLog(); + const fakeFs = { + stat: async (path: string) => { + expect(path).toBe(join("/cwd", "project")); + return { isDirectory: () => true }; + }, + }; + const target = await resolveStartupTarget(["project"], "/cwd", log, fakeFs); + expect(target).toEqual({ workspaceRoot: join("/cwd", "project") }); +}); + +test("parent directory of a nested file resolves correctly", async () => { + dir = await mkdtemp(join(tmpdir(), "tecode-argv-")); + const nested = join(dir, "a", "b"); + await mkdir(nested, { recursive: true }); + const filePath = join(nested, "file.ts"); + await writeFile(filePath, "", "utf8"); + + const log = createHostLog(); + const target = await resolveStartupTarget([filePath], "/irrelevant", log); + expect(target.workspaceRoot).toBe(dirname(filePath)); + expect(target.initialFilePath).toBe(filePath); +}); diff --git a/packages/cli/src/argv.ts b/packages/cli/src/argv.ts new file mode 100644 index 0000000..ebeccaa --- /dev/null +++ b/packages/cli/src/argv.ts @@ -0,0 +1,90 @@ +/** + * Argv parsing and file/directory resolution for the CLI's startup + * sequence (Req 12.1; design.md §3, §17: "parse argv" is the sync phase's + * first step; tasks.md's Task 1.15 "Argv parsing (file/directory)"). + * `--version` is handled by `main.ts` itself, before this module is even + * reached (it must not touch the filesystem or build any services). + */ + +import { stat as nodeStat } from "node:fs/promises"; +import { dirname, resolve as resolvePath } from "node:path"; +import type { HostLog } from "@tecode/core"; + +/** Where {@link resolveStartupTarget} landed for one CLI invocation. */ +export interface StartupTarget { + /** The directory `ConfigService`/`discover()`/`tecode.workspace.rootUri` + * treat as the open workspace. */ + workspaceRoot: string; + /** Absolute path to open once the deferred phase's document manager is + * ready (design.md §3's "open the file/directory from argv" step) — + * `undefined` for a directory argument or a no-argument launch. */ + initialFilePath?: string; +} + +/** The narrow filesystem seam {@link resolveStartupTarget} needs — + * exists as an injectable seam (matches every `core` service's + * `*Fs`-suffixed dependency convention) so tests can simulate a path that + * exists/doesn't without depending on real disk state. Defaults to + * `node:fs/promises`. */ +export interface ArgvResolutionFs { + stat(path: string): Promise<{ isDirectory(): boolean }>; +} + +function createNodeArgvFs(): ArgvResolutionFs { + return { + stat: async (path) => { + const stats = await nodeStat(path); + return { isDirectory: () => stats.isDirectory() }; + }, + }; +} + +/** Render a caught `unknown` value as a message string without risking a + * second throw (matches `core`'s `describeError` convention). */ +function describeError(err: unknown): string { + try { + if (err instanceof Error) return err.message; + return String(err); + } catch { + return "Unknown error"; + } +} + +/** + * Resolve the CLI's one positional argument (CodeRabbit's Phase 1 plan): a + * directory becomes `workspaceRoot` with no initial document; a file's + * parent directory becomes `workspaceRoot` and the file itself is opened + * in the deferred phase; no argument at all defaults to `cwd`. `argv` here + * is expected to already have flags like `--version` handled/stripped by + * the caller — this function only ever looks for the first token that + * does not start with `-`. + * + * Never throws (matches `core`'s never-throwing service boundaries): a + * path that does not exist, or can't be `stat`-ed, is reported to `log` as + * a warning and treated as if no argument had been given (`cwd`) — a + * typo'd path should degrade to an empty workspace rather than abort + * startup, the same "continue starting up" spirit Req 2.4 applies to a bad + * extension. + */ +export async function resolveStartupTarget( + argv: readonly string[], + cwd: string, + log: HostLog, + fs: ArgvResolutionFs = createNodeArgvFs(), +): Promise { + const positional = argv.find((arg) => !arg.startsWith("-")); + if (!positional) return { workspaceRoot: cwd }; + + const resolved = resolvePath(cwd, positional); + try { + const stats = await fs.stat(resolved); + if (stats.isDirectory()) return { workspaceRoot: resolved }; + return { workspaceRoot: dirname(resolved), initialFilePath: resolved }; + } catch (cause) { + log.append("warning", { + message: `Startup path "${resolved}" does not exist or could not be read (${describeError(cause)}); starting with no workspace.`, + path: resolved, + }); + return { workspaceRoot: cwd }; + } +} diff --git a/packages/cli/src/extensionRecords.test.ts b/packages/cli/src/extensionRecords.test.ts new file mode 100644 index 0000000..c0819e2 --- /dev/null +++ b/packages/cli/src/extensionRecords.test.ts @@ -0,0 +1,126 @@ +import { afterEach, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import type { Manifest } from "@tecode/api"; +import type { LoadedExtension } from "@tecode/core"; +import { buildExtensionRecord, buildExtensionRecords } from "./extensionRecords"; + +let tempDirs: string[] = []; + +async function makeTempDir(): Promise { + const dir = await mkdtemp(join(tmpdir(), "tecode-ext-records-")); + tempDirs.push(dir); + return dir; +} + +afterEach(async () => { + await Promise.all(tempDirs.map((dir) => rm(dir, { recursive: true, force: true }))); + tempDirs = []; +}); + +function fixtureManifest(id: string): Manifest { + return { id, version: "0.0.1", apiVersion: "1.0", activationEvents: ["onStartup"], contributes: {} }; +} + +test("a user/workspace extension's extensionUri/storagePath derive from its real directory", async () => { + const extensionsDir = await makeTempDir(); + const extensionDir = join(extensionsDir, "demo"); + await mkdir(extensionDir, { recursive: true }); + const manifestPath = join(extensionDir, "manifest.ts"); + await writeFile(manifestPath, "export default {}\n", "utf8"); + + const loaded: LoadedExtension = { + extensionId: "demo", + manifest: fixtureManifest("demo"), + source: "user", + sourcePath: manifestPath, + }; + + const record = buildExtensionRecord(loaded); + expect(record.id).toBe("demo"); + expect(record.extensionUri).toBe(pathToFileURL(extensionDir).href); + expect(record.storagePath.endsWith(join("extension-storage", "demo"))).toBe(true); +}); + +test("loadModule() dynamically imports index.ts when no index.js exists", async () => { + const extensionsDir = await makeTempDir(); + const extensionDir = join(extensionsDir, "demo"); + await mkdir(extensionDir, { recursive: true }); + const manifestPath = join(extensionDir, "manifest.ts"); + await writeFile(manifestPath, "export default {}\n", "utf8"); + await writeFile( + join(extensionDir, "index.ts"), + 'export const MARKER = "index-ts-loaded";\nexport function activate() {}\n', + "utf8", + ); + + const loaded: LoadedExtension = { + extensionId: "demo", + manifest: fixtureManifest("demo"), + source: "user", + sourcePath: manifestPath, + }; + + const record = buildExtensionRecord(loaded); + const mod = (await record.loadModule()) as { MARKER: string; activate: () => void }; + expect(mod.MARKER).toBe("index-ts-loaded"); + expect(typeof mod.activate).toBe("function"); +}); + +test("loadModule() prefers a pre-bundled index.js over index.ts (design.md §4.4)", async () => { + const extensionsDir = await makeTempDir(); + const extensionDir = join(extensionsDir, "demo"); + await mkdir(extensionDir, { recursive: true }); + const manifestPath = join(extensionDir, "manifest.ts"); + await writeFile(manifestPath, "export default {}\n", "utf8"); + await writeFile(join(extensionDir, "index.ts"), 'export const MARKER = "ts";\n', "utf8"); + await writeFile(join(extensionDir, "index.js"), 'export const MARKER = "js";\n', "utf8"); + + const loaded: LoadedExtension = { + extensionId: "demo", + manifest: fixtureManifest("demo"), + source: "workspace", + sourcePath: manifestPath, + }; + + const record = buildExtensionRecord(loaded); + const mod = (await record.loadModule()) as { MARKER: string }; + expect(mod.MARKER).toBe("js"); +}); + +test("a builtin extension's loadModule() rejects with a clear, documented error", async () => { + const loaded: LoadedExtension = { + extensionId: "fake-builtin", + manifest: fixtureManifest("fake-builtin"), + source: "builtin", + sourcePath: "/fake-builtin", + }; + + const record = buildExtensionRecord(loaded); + expect(record.extensionUri).toBe("/fake-builtin"); + await expect(record.loadModule()).rejects.toThrow(/No static module wiring/); +}); + +test("buildExtensionRecords maps every LoadedExtension", async () => { + const extensionsDir = await makeTempDir(); + const records = await Promise.all( + ["a", "b"].map(async (id) => { + const extensionDir = join(extensionsDir, id); + await mkdir(extensionDir, { recursive: true }); + const manifestPath = join(extensionDir, "manifest.ts"); + await writeFile(manifestPath, "export default {}\n", "utf8"); + const loaded: LoadedExtension = { + extensionId: id, + manifest: fixtureManifest(id), + source: "user", + sourcePath: manifestPath, + }; + return loaded; + }), + ); + + const built = buildExtensionRecords(records); + expect(built.map((r) => r.id)).toEqual(["a", "b"]); +}); diff --git a/packages/cli/src/extensionRecords.ts b/packages/cli/src/extensionRecords.ts new file mode 100644 index 0000000..3bedd01 --- /dev/null +++ b/packages/cli/src/extensionRecords.ts @@ -0,0 +1,102 @@ +/** + * Builds `@tecode/core`'s `ExtensionRecord[]` from `loadExtensions`'s + * `LoadedExtension[]` (Req 2.5, 2.6; design.md §4.2, §4.4) — the one piece + * `host/activation.ts`'s own TSDoc calls out as deliberately *not* its + * job: "the *how* [of loading an extension's implementation module] ... + * is injected via `ExtensionRecord.loadModule` rather than performed here + * ... production wiring of that closure is the ... assembly task." PR #53 + * moved the UI shell in ahead of that TSDoc's original guess at which task + * would land it; this module is `packages/cli`'s Task 1.15 fulfilling it. + * + * **`loadModule()` performs a real dynamic `import()` of the extension's + * `index.ts`/`.js`.** This is the designed composition-layer load path + * (design.md §4.2, §4.4) — extension code can only be named once discovery + * has scanned the filesystem and registration has validated the manifest, + * exactly the same shape of necessity `discovery.ts`'s + * `importManifestModule` documents for `manifest.ts`. `packages/cli` is + * the one package the root `eslint.config.mjs` layering rule exempts from + * its `@tecode/core` import/dynamic-import ban (`ignores: + * ["packages/cli/**"]`), and that rule's `no-restricted-syntax` selector + * only ever matches a dynamic import of the literal `"@tecode/core"` + * specifier — a `file://` URL built from a real path found on disk during + * this same startup's discovery scan does not match it — so this call + * site needs no `eslint-disable` (one would be flagged as unused besides). + */ + +import { stat as nodeStat } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { getUserConfigDir, type ExtensionRecord, type LoadedExtension } from "@tecode/core"; + +async function pathExists(path: string): Promise { + try { + await nodeStat(path); + return true; + } catch { + return false; + } +} + +/** + * Composition-layer extension-module load site (mirrors `discovery.ts`'s + * `importManifestModule` TSDoc convention). `extensionDir` always comes + * from a `LoadedExtension.sourcePath` this same process's `discover()` + * call found on disk — never external/untrusted input passed straight + * through from argv or a network source. Prefers a pre-bundled + * `index.js` over `index.ts` when both exist (design.md §4.4: "Extensions + * with npm dependencies ship a pre-bundled `index.js`; the host prefers + * `index.js` over `index.ts` when both exist"). + */ +async function loadUserOrWorkspaceModule(extensionDir: string): Promise { + const jsPath = join(extensionDir, "index.js"); + const target = (await pathExists(jsPath)) ? jsPath : join(extensionDir, "index.ts"); + return import(pathToFileURL(target).href); +} + +/** + * Build one {@link ExtensionRecord} from a `registration.ts` + * `LoadedExtension`. + * + * - `user`/`workspace` extensions: `extensionUri`/`storagePath` derive from + * the extension's real directory (`dirname` of its manifest path), and + * `loadModule` dynamically imports `index.js`/`index.ts` from that same + * directory (this module's TSDoc). + * - `builtin` extensions: `discovery.ts` gives these a synthetic + * `/` `sourcePath` (no real directory — design.md §4.4 + * compiles built-ins in as static imports instead). `loadModule` for a + * builtin therefore has no generic dynamic-import implementation to fall + * back to; it exists here as a documented placeholder that reports a + * clear error rather than dynamically importing a path that was never a + * real file. `packages/builtin/index.ts`'s `builtinManifests` is `[]` + * today, so this branch cannot currently fire in practice — it is + * structurally ready for the first built-in that adds itself there and + * needs its own static-import case added alongside. + */ +export function buildExtensionRecord(extension: LoadedExtension): ExtensionRecord { + const isBuiltin = extension.source === "builtin"; + const extensionDir = isBuiltin ? extension.sourcePath : dirname(extension.sourcePath); + const extensionUri = isBuiltin ? extension.sourcePath : pathToFileURL(extensionDir).href; + const storagePath = join(getUserConfigDir(), "extension-storage", extension.extensionId); + + return { + id: extension.extensionId, + manifest: extension.manifest, + extensionUri, + storagePath, + loadModule: () => + isBuiltin + ? Promise.reject( + new Error( + `No static module wiring for built-in extension "${extension.extensionId}" yet ` + + `(packages/builtin/index.ts's builtinManifests is empty today — see extensionRecords.ts's TSDoc).`, + ), + ) + : loadUserOrWorkspaceModule(extensionDir), + }; +} + +/** Build every {@link ExtensionRecord} for {@link createExtensionHost} + * (`@tecode/core`) from `loadExtensions`'s `LoadedExtension[]`. */ +export function buildExtensionRecords(loaded: readonly LoadedExtension[]): ExtensionRecord[] { + return loaded.map(buildExtensionRecord); +} diff --git a/packages/cli/src/keymapState.test.ts b/packages/cli/src/keymapState.test.ts new file mode 100644 index 0000000..fe27bae --- /dev/null +++ b/packages/cli/src/keymapState.test.ts @@ -0,0 +1,59 @@ +import { expect, test } from "bun:test"; +import { createHostLog } from "@tecode/core"; +import { createKeymapState } from "./keymapState"; + +test("starts with an empty table (no defaults/fallback/extension/user layers yet)", () => { + const log = createHostLog(); + const state = createKeymapState(log); + expect(state.getTable().entries().size).toBe(0); + expect(state.getTable().lookup("ctrl+s", () => undefined)).toBeUndefined(); +}); + +test("setUserEntries rebuilds the table with the user layer", () => { + const log = createHostLog(); + const state = createKeymapState(log); + state.setUserEntries([{ key: "ctrl+s", command: "workspace.save" }]); + + const resolved = state.getTable().lookup("ctrl+s", () => undefined); + expect(resolved?.command).toBe("workspace.save"); + expect(resolved?.layer).toBe("user"); +}); + +test("setExtensionEntries rebuilds the table with the extension layer", () => { + const log = createHostLog(); + const state = createKeymapState(log); + state.setExtensionEntries([{ key: "ctrl+alt+t", command: "fixture.hello" }]); + + const resolved = state.getTable().lookup("ctrl+alt+t", () => undefined); + expect(resolved?.command).toBe("fixture.hello"); + expect(resolved?.layer).toBe("extension"); +}); + +test("user entries outrank extension entries on the same key (design.md §6.2 precedence)", () => { + const log = createHostLog(); + const state = createKeymapState(log); + state.setExtensionEntries([{ key: "ctrl+s", command: "extension.save" }]); + state.setUserEntries([{ key: "ctrl+s", command: "user.save" }]); + + const resolved = state.getTable().lookup("ctrl+s", () => undefined); + expect(resolved?.command).toBe("user.save"); + expect(resolved?.layer).toBe("user"); +}); + +test("a malformed raw user entry is skipped rather than thrown", () => { + const log = createHostLog(); + const state = createKeymapState(log); + expect(() => state.setUserEntries([{ key: 42, command: "bad" }, "not even an object"])).not.toThrow(); + expect(state.getTable().entries().size).toBe(0); + expect(log.entries().some((e) => e.level === "warning")).toBe(true); +}); + +test("later setUserEntries calls fully replace the previous user layer", () => { + const log = createHostLog(); + const state = createKeymapState(log); + state.setUserEntries([{ key: "ctrl+s", command: "workspace.save" }]); + state.setUserEntries([{ key: "ctrl+w", command: "workspace.close" }]); + + expect(state.getTable().lookup("ctrl+s", () => undefined)).toBeUndefined(); + expect(state.getTable().lookup("ctrl+w", () => undefined)?.command).toBe("workspace.close"); +}); diff --git a/packages/cli/src/keymapState.ts b/packages/cli/src/keymapState.ts new file mode 100644 index 0000000..a3cf711 --- /dev/null +++ b/packages/cli/src/keymapState.ts @@ -0,0 +1,79 @@ +/** + * Keeps the layered `BindingTable` up to date across the CLI's startup + * phases (Req 4.1-4.3; design.md §6.2; CodeRabbit's Phase 2 plan): the + * sync phase builds one with whatever is known synchronously (nothing — + * `defaults`/`fallback` have no source yet, see below), `ConfigService`'s + * `onKeybindingsChange` hook rebuilds it with the `user` layer once the + * user's `keybindings.json` has loaded (and again on every live reload), + * and the deferred phase rebuilds it again once `loadExtensions`'s + * `extensionKeybindings` are known. + * + * **`defaults`/`fallback` are `[]` today, deliberately.** `KeymapLayers` + * (`@tecode/core`'s `bindingTable.ts`) requires all four layers regardless + * of which are populated yet: + * - `defaults` — core commands' own default bindings. No core command + * contributes one yet (editor-core's movement/editing commands are + * Phase 2 tasks, command-palette's `ctrl+shift+p`/`ctrl+p` are Phase 3) — + * there is nothing to seed this layer with until those land. + * - `fallback` — the terminal-capability fallback overlay (Req 4.7). + * `terminalCapabilities.ts`'s stub result feeds this once Task 4.2 wires + * real detection; until then it stays empty, exactly like + * `bindingTable.ts`'s own TSDoc says it may. + * + * `@tecode/core` has no OpenTUI key-event pipeline consuming this table + * yet (routing key input into editing is tasks.md's Task 2.2) — this + * module's job for Task 1.15 is only to keep the table itself correctly + * assembled and rebuildable end to end, the same way `ui/slotRegistry.ts` + * is kept live before any view consumes it. + */ + +import { createBindingTable, type BindingTable, type HostLog } from "@tecode/core"; +import type { KeybindingContribution } from "@tecode/api"; + +/** The mutable keymap-table holder {@link createKeymapState} returns. */ +export interface KeymapState { + /** The current binding table — always up to date as of the last + * {@link setUserEntries}/{@link setExtensionEntries} call. */ + getTable(): BindingTable; + /** + * Rebuild with a new `user` layer — wired as `ConfigService`'s + * `onKeybindingsChange` hook (`config/service.ts`). Entries are raw, + * unvalidated JSON (`ConfigService` "does not interpret keybinding + * entries" — its own TSDoc): `createBindingTable`'s `compileEntry` + * already guards every field defensively (a non-string `key`/`command` + * is skipped and logged, not trusted blindly), so casting here is safe. + */ + setUserEntries(entries: readonly unknown[]): void; + /** Rebuild with a new `extension` layer — called once from the deferred + * phase with `LoadExtensionsResult.extensionKeybindings` once discovery + * and registration have run. */ + setExtensionEntries(entries: readonly KeybindingContribution[]): void; +} + +/** Build a {@link KeymapState} (Req 4.1-4.3). Starts with every layer + * empty; `getTable()` is always safe to call, even before either setter + * has ever run. */ +export function createKeymapState(log: HostLog): KeymapState { + let userEntries: KeybindingContribution[] = []; + let extensionEntries: KeybindingContribution[] = []; + let table = build(); + + function build(): BindingTable { + return createBindingTable( + { defaults: [], fallback: [], extension: extensionEntries, user: userEntries }, + { log }, + ); + } + + return { + getTable: () => table, + setUserEntries(entries) { + userEntries = entries as KeybindingContribution[]; + table = build(); + }, + setExtensionEntries(entries) { + extensionEntries = entries.slice(); + table = build(); + }, + }; +} diff --git a/packages/cli/src/main.integration.test.ts b/packages/cli/src/main.integration.test.ts new file mode 100644 index 0000000..51be937 --- /dev/null +++ b/packages/cli/src/main.integration.test.ts @@ -0,0 +1,131 @@ +/** + * Subprocess startup-sequence integration test (Req 12.1, 12.2; design.md + * §3, §15, §16; tasks.md's Task 1.15: "Integration test: startup renders + * before any extension activates; timing check with headroom over + * 100ms"). + * + * Spawns the real `packages/cli/src/main.ts` entry point as a genuine + * child process (`Bun.spawn`, matching `layering.test.ts`'s + * spawn-and-parse pattern) with `TECODE_HEADLESS=1` — never grabs a real + * TTY — and a disposable on-disk fixture extension + temp `HOME`, always + * cleaned up in `finally`. + * + * **Proving ordering without a shared clock**: the fixture extension's + * `index.ts` logs its own module-load time as one JSON line on stdout, in + * the very same child process `main.ts` runs in — so its `performance.now()` + * reading is directly comparable to the `tecode.timing` first-frame + * event's `ts` (both measured from the same process-start reference + * point), with no cross-process clock synchronization needed. + */ + +import { expect, setDefaultTimeout, test } from "bun:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// Spawning bun as a subprocess (cold module resolution/transpilation) can +// exceed bun:test's 5s default, independent of the app's own <100ms +// startup budget being asserted below. +setDefaultTimeout(30_000); + +interface JsonLine { + event: string; + [key: string]: unknown; +} + +function parseJsonLines(output: string): JsonLine[] { + const lines: JsonLine[] = []; + for (const line of output.split("\n")) { + const trimmed = line.trim(); + if (!trimmed.startsWith("{")) continue; + try { + lines.push(JSON.parse(trimmed) as JsonLine); + } catch { + // Not one of our structured lines (e.g. a stray console warning) — + // ignore rather than fail the whole parse. + } + } + return lines; +} + +test("headless startup renders the shell before any extension's index.ts loads, and reports first-frame timing", async () => { + const homeDir = await mkdtemp(join(tmpdir(), "tecode-integration-home-")); + const workspaceDir = await mkdtemp(join(tmpdir(), "tecode-integration-ws-")); + + try { + const extensionDir = join(workspaceDir, ".tecode", "extensions", "fixture"); + await mkdir(extensionDir, { recursive: true }); + await writeFile( + join(extensionDir, "manifest.ts"), + `export default { + id: "fixture.startup-order", + version: "0.0.1", + apiVersion: "1.0", + activationEvents: ["onStartup"], + contributes: {}, + };\n`, + "utf8", + ); + await writeFile( + join(extensionDir, "index.ts"), + `console.log(JSON.stringify({ event: "fixture.moduleLoaded", ts: performance.now() })); + export function activate() { + console.log(JSON.stringify({ event: "fixture.activated", ts: performance.now() })); + }\n`, + "utf8", + ); + + const mainPath = join(import.meta.dir, "main.ts"); + const proc = Bun.spawn({ + cmd: ["bun", "run", mainPath, workspaceDir], + env: { + ...process.env, + HOME: homeDir, + APPDATA: homeDir, + TECODE_HEADLESS: "1", + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + expect(exitCode).toBe(0); + + const lines = parseJsonLines(stdout); + const firstFrame = lines.find((l) => l.event === "tecode.timing" && l.phase === "first-frame"); + const moduleLoaded = lines.find((l) => l.event === "fixture.moduleLoaded"); + const activated = lines.find((l) => l.event === "fixture.activated"); + const headlessExit = lines.find((l) => l.event === "tecode.headlessExit"); + + expect(firstFrame, `expected a tecode.timing first-frame line; stderr:\n${stderr}`).toBeDefined(); + expect(moduleLoaded, `expected the fixture's own module-load line; stderr:\n${stderr}`).toBeDefined(); + expect(activated).toBeDefined(); + expect(headlessExit).toBeDefined(); + + // The core ordering assertion (Req 12.1, 12.2): the shell's first + // frame happened strictly before the extension's index.ts was ever + // imported, and before it activated. + const firstFrameTs = firstFrame?.["ts"] as number; + const moduleLoadedTs = moduleLoaded?.["ts"] as number; + const activatedTs = activated?.["ts"] as number; + expect(firstFrameTs).toBeLessThan(moduleLoadedTs); + expect(moduleLoadedTs).toBeLessThanOrEqual(activatedTs); + + // Timing budget (design.md §15's <100ms, with generous headroom to + // avoid CI flakiness — tasks.md's own "timing check with headroom + // over 100ms"). + const firstFrameMs = firstFrame?.["ms"] as number; + expect(firstFrameMs).toBeGreaterThanOrEqual(0); + expect(firstFrameMs).toBeLessThan(5_000); + + expect(headlessExit?.["loaded"]).toBe(1); + expect(headlessExit?.["skipped"]).toBe(0); + } finally { + await rm(homeDir, { recursive: true, force: true }); + await rm(workspaceDir, { recursive: true, force: true }); + } +}); diff --git a/packages/cli/src/main.test.ts b/packages/cli/src/main.test.ts index ff6219f..fef3240 100644 --- a/packages/cli/src/main.test.ts +++ b/packages/cli/src/main.test.ts @@ -1,10 +1,34 @@ import { expect, test } from "bun:test"; -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdir, mkdtemp, readdir as nodeReaddir, rm, stat as nodeStat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { pathToUri } from "@tecode/core"; +import { getUserExtensionsDir, pathToUri, type DiscoveryFs } from "@tecode/core"; import pkg from "../package.json"; -import { buildAssemblyRoot } from "./main"; +import { buildAssemblyRoot, runDeferredPhase } from "./main"; + +/** A {@link DiscoveryFs} backed by the real filesystem, except the real + * user extensions directory, which is always reported as missing + * (matches `packages/core/src/host/discovery.test.ts`'s `createHermeticFs` + * — Bun's `os.homedir()` does not honor a runtime `process.env.HOME` + * mutation, so an in-process test cannot rely on the HOME-redirect trick + * below to keep `discover()`'s `user` layer scan off the real machine's + * `~/.config/tecode/extensions`; this blocks that one path explicitly + * instead). */ +function createHermeticDiscoveryFs(): DiscoveryFs { + const blockedUserDir = getUserExtensionsDir(); + return { + async readdir(path) { + if (path === blockedUserDir) { + throw Object.assign(new Error("ENOENT (blocked for test hermeticity)"), { code: "ENOENT" }); + } + return nodeReaddir(path); + }, + async stat(path) { + const stats = await nodeStat(path); + return { isDirectory: () => stats.isDirectory() }; + }, + }; +} test("--version prints the package version and exits 0", async () => { const proc = Bun.spawn(["bun", "run", `${import.meta.dir}/main.ts`, "--version"], { @@ -59,6 +83,14 @@ test("buildAssemblyRoot wires every core service and registers the 'tecode' modu expect(root.api.workspace.rootUri).toBe(pathToUri(dir)); expect(Object.isFrozen(root.api)).toBe(true); + // New in Task 1.15: the UI/keymap wiring buildAssemblyRoot now adds + // alongside Task 1.13's api assembly. + expect(root.slotRegistry).toBeDefined(); + expect(root.layoutState).toBeDefined(); + expect(root.theme.colors).toBeDefined(); + expect(root.keymap.getTable().entries().size).toBe(0); + expect(root.hostRef.current).toBeUndefined(); + // buildAssemblyRoot's own TSDoc documents that registerTecodeAlias runs // as its last step; `create.contract.test.ts` is where the resulting // `"tecode"` module-alias resolution is exercised end-to-end (the one @@ -69,3 +101,155 @@ test("buildAssemblyRoot wires every core service and registers the 'tecode' modu await rm(dir, { recursive: true, force: true }); } }); + +test("forward-referenced activateExtension is a safe no-op before the deferred phase assigns hostRef", async () => { + const dir = await mkdtemp(join(tmpdir(), "tecode-cli-root-")); + const savedHome = process.env["HOME"]; + const savedAppData = process.env["APPDATA"]; + process.env["HOME"] = dir; + process.env["APPDATA"] = dir; + let root: ReturnType; + try { + root = buildAssemblyRoot(dir); + } finally { + if (savedHome === undefined) delete process.env["HOME"]; + else process.env["HOME"] = savedHome; + if (savedAppData === undefined) delete process.env["APPDATA"]; + else process.env["APPDATA"] = savedAppData; + } + + try { + await root.config.ready; + root.commands.registerLazy("fixture.stillLazy", { extensionId: "nobody-home" }); + // No host has been assigned yet (hostRef.current is undefined) — + // execute() must resolve (never hang/throw) and report "not activated + // yet" rather than crash on a missing activateExtension hook. + const result = await root.commands.execute("fixture.stillLazy"); + expect(result).toBeUndefined(); + } finally { + root.config.dispose(); + await rm(dir, { recursive: true, force: true }); + } +}); + +test("runDeferredPhase loads a workspace extension, activates it on startup, wires its keybindings, and opens the initial file", async () => { + const homeDir = await mkdtemp(join(tmpdir(), "tecode-cli-home-")); + const workspaceDir = await mkdtemp(join(tmpdir(), "tecode-cli-ws-")); + const savedHome = process.env["HOME"]; + const savedAppData = process.env["APPDATA"]; + process.env["HOME"] = homeDir; + process.env["APPDATA"] = homeDir; + let root: ReturnType; + try { + root = buildAssemblyRoot(workspaceDir); + } finally { + if (savedHome === undefined) delete process.env["HOME"]; + else process.env["HOME"] = savedHome; + if (savedAppData === undefined) delete process.env["APPDATA"]; + else process.env["APPDATA"] = savedAppData; + } + + try { + await root.config.ready; + + const extensionDir = join(workspaceDir, ".tecode", "extensions", "fixture"); + await mkdir(extensionDir, { recursive: true }); + await writeFile( + join(extensionDir, "manifest.ts"), + `export default { + id: "fixture.startup-order", + version: "0.0.1", + apiVersion: "1.0", + activationEvents: ["onStartup"], + contributes: { + commands: [{ id: "fixture.hello", title: "Fixture Hello" }], + keybindings: [{ key: "ctrl+alt+t", command: "fixture.hello" }], + }, + };\n`, + "utf8", + ); + await writeFile( + join(extensionDir, "index.ts"), + `export function activate(ctx) { + ctx.subscriptions.push( + ctx.api.commands.register("fixture.hello", () => "hello-from-fixture"), + ); + }\n`, + "utf8", + ); + + const targetFile = join(workspaceDir, "notes.txt"); + await writeFile(targetFile, "hello", "utf8"); + + const { extensionHost, loadResult } = await runDeferredPhase(root, { + initialFilePath: targetFile, + fs: createHermeticDiscoveryFs(), + }); + + expect(loadResult.loaded.map((e) => e.extensionId)).toEqual(["fixture.startup-order"]); + expect(loadResult.skipped).toEqual([]); + expect(extensionHost.getState("fixture.startup-order")).toBe("active"); + // hostRef is now fulfilled — commands/documents/slotRegistry's forward + // references reach the real host. + expect(root.hostRef.current).toBe(extensionHost); + + // The extension's activate(ctx) registered a real command. + expect(await root.api.commands.execute("fixture.hello")).toBe("hello-from-fixture"); + + // Its contributes.keybindings landed in the keymap's extension layer. + const resolved = root.keymap.getTable().lookup("ctrl+alt+t", () => undefined); + expect(resolved?.command).toBe("fixture.hello"); + expect(resolved?.layer).toBe("extension"); + + // The argv-resolved initial file was opened. + expect(root.documents.documents.some((d) => d.uri === pathToUri(targetFile))).toBe(true); + + await extensionHost.disposeAll(); + } finally { + root.config.dispose(); + await rm(homeDir, { recursive: true, force: true }); + await rm(workspaceDir, { recursive: true, force: true }); + } +}, 15_000); + +test("runDeferredPhase reports a bad extension without failing startup (Req 2.4)", async () => { + const homeDir = await mkdtemp(join(tmpdir(), "tecode-cli-home-")); + const workspaceDir = await mkdtemp(join(tmpdir(), "tecode-cli-ws-")); + const savedHome = process.env["HOME"]; + const savedAppData = process.env["APPDATA"]; + process.env["HOME"] = homeDir; + process.env["APPDATA"] = homeDir; + let root: ReturnType; + try { + root = buildAssemblyRoot(workspaceDir); + } finally { + if (savedHome === undefined) delete process.env["HOME"]; + else process.env["HOME"] = savedHome; + if (savedAppData === undefined) delete process.env["APPDATA"]; + else process.env["APPDATA"] = savedAppData; + } + + try { + await root.config.ready; + + const extensionDir = join(workspaceDir, ".tecode", "extensions", "broken"); + await mkdir(extensionDir, { recursive: true }); + // Missing required fields ("version", "apiVersion", ...) — validation + // should skip it, not throw. + await writeFile(join(extensionDir, "manifest.ts"), "export default { id: 'broken' };\n", "utf8"); + + const { extensionHost, loadResult } = await runDeferredPhase(root, { + fs: createHermeticDiscoveryFs(), + }); + + expect(loadResult.loaded).toEqual([]); + expect(loadResult.skipped.length).toBe(1); + expect(loadResult.skipped[0]?.extensionId).toBe("broken"); + await extensionHost.disposeAll(); + } finally { + root.config.dispose(); + await rm(homeDir, { recursive: true, force: true }); + await rm(workspaceDir, { recursive: true, force: true }); + } +}, 15_000); + diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index c7a623d..6048e71 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -1,30 +1,45 @@ import pkg from "../package.json"; -import type { FileSystem, Tecode } from "@tecode/api"; +import type { FileSystem, Manifest, ResolvedTheme, Tecode } from "@tecode/api"; import { createCommandRegistry, createConfigService, createContextService, createDocumentManager, + createExtensionHost, createFileSystem, createHostLog, + createLayoutStateService, createNoopStatusSink, + createBaseTheme, + createSlotRegistry, createTecodeApi, + loadExtensions, pathToUri, registerTecodeAlias, type CommandRegistry, type ConfigService, type ContextService, + type DiscoveryFs, type DocumentManager, + type ExtensionHost, type HostLog, + type LayoutStateService, + type LoadExtensionsResult, + type SlotRegistry, type StatusSink, } from "@tecode/core"; +import { builtinManifests } from "@tecode/builtin"; +import { resolveStartupTarget, type StartupTarget } from "./argv"; +import { buildExtensionRecords } from "./extensionRecords"; +import { createKeymapState, type KeymapState } from "./keymapState"; +import { renderShellHeadless, renderShellToTerminal, type RenderShell } from "./renderShell"; +import { detectTerminalCapabilities } from "./terminalCapabilities"; /** * Every core service {@link buildAssemblyRoot} wires together, plus the - * assembled `tecode` object itself — returned so a caller (currently just - * this module's own `main`; Task 1.15's startup sequence next) can hold - * onto `config` for `ready`/`dispose()` without reaching back into the - * module's internals. + * assembled `tecode` object itself (Req 10.1, 10.2; design.md §12, §17; + * Task 1.13's "Bun module alias registration" note, extended by Task + * 1.15's full startup sequence below). */ export interface AssemblyRoot { log: HostLog; @@ -35,44 +50,103 @@ export interface AssemblyRoot { config: ConfigService; context: ContextService; api: Tecode; + /** The live slot registry backing both `tecode.ui.registerView` (via + * `api`) and the rendered Shell — see this module's TSDoc on why both + * must share the exact same instance. */ + slotRegistry: SlotRegistry; + layoutState: LayoutStateService; + /** The active theme (Task 1.14's hardcoded base palette — a real theme + * loader is Task 2.6's job). */ + theme: ResolvedTheme; + /** The resolved workspace root this root was built for. */ + workspaceRoot: string; + /** The layered keybinding table, kept up to date across every startup + * phase — see `keymapState.ts`'s TSDoc. */ + keymap: KeymapState; + /** + * Forward-reference box for the extension host (this module's TSDoc, + * "Forward-referenced host wiring"): `undefined` until the deferred + * phase ({@link runDeferredPhase}) assigns it. `commands`/`documents`/ + * `slotRegistry` above all close over `hostRef.current` rather than a + * host instance directly, so they can be built in the sync phase, before + * the host exists, and still reach it once it does. + */ + hostRef: { current?: ExtensionHost }; } /** * Build the `tecode` composition root and register the `"tecode"` module - * alias (Req 10.1, 10.2; design.md §12, §17; Task 1.13's "Bun module alias - * registration" note). `packages/cli` is the one place allowed to import - * `@tecode/core` directly (`eslint.config.mjs`'s layering rule) — this - * function is that wiring. + * alias (Req 10.1, 10.2, 1.4; design.md §2, §12, §17). `packages/cli` is + * the one place allowed to import `@tecode/core` directly + * (`eslint.config.mjs`'s layering rule) — this function is that wiring, + * and the sync phase of Task 1.15's startup sequence (everything up to, + * but not including, rendering the Shell) is entirely this function's + * body. * - * **This is deliberately a small slice of design.md §17's full startup - * sequence**, not that sequence itself: argv parsing (file vs. directory), - * the sync-before-first-frame phase, rendering the UI shell, deferred - * extension discovery/activation, the initial file open, and startup-timing - * instrumentation are all Task 1.15's job. That task should *call* this - * function (or extend it) rather than duplicate its ordering — the one - * invariant it establishes and Task 1.15 must preserve is - * {@link registerTecodeAlias} running immediately after - * {@link createTecodeApi} and strictly before any extension module is - * imported (Req 1.4, design.md §2): an extension's `import ... from - * "tecode"` resolves only once the alias is registered. + * **Forward-referenced host wiring**: the extension host + * ({@link ExtensionHost}, `@tecode/core`'s `createExtensionHost`) is built + * in the *deferred* phase ({@link runDeferredPhase}), after discovery — + * which itself needs `commands`/`slotRegistry` already registered so + * lazy commands/views exist to attach to. But `commands.execute` on a lazy + * command, `documents`' `onLanguage:*` firing, and `slotRegistry`'s lazy + * view activation all need to reach the host *from here*, in the sync + * phase. `hostRef` — a plain mutable box, read through an optional-chained + * closure — is exactly the pattern `host/activation.ts`'s own TSDoc + * documents for this: build the services that need the host first with a + * closure over `hostRef.current`, build the host once discovery has run, + * then assign it. Every call through `hostRef.current` before the deferred + * phase assigns it is a documented, safe no-op (each dependency's own + * `activateExtension?`/`onLanguageActivation?` is already optional and + * guarded for exactly this "no host yet" case). * - * `workspaceRoot` defaults to `process.cwd()` as a placeholder for Task - * 1.15's real argv-driven file/directory resolution (design.md §17's - * "Argv parsing (file/directory)" step) — nothing here interprets `argv` - * yet. + * `registerTecodeAlias` runs immediately after {@link createTecodeApi} and + * strictly before any extension module import — {@link runDeferredPhase}'s + * `loadModule()` closures (`extensionRecords.ts`) are the first (and only) + * dynamic imports of extension code, always strictly after this function + * returns. */ -export function buildAssemblyRoot(workspaceRoot: string = process.cwd()): AssemblyRoot { - const log = createHostLog(); - // No UI shell exists yet (Task 1.14) to back a real StatusSink — matches - // every other core composition point that hasn't reached its UI task. +export function buildAssemblyRoot( + workspaceRoot: string = process.cwd(), + deps: { log?: HostLog } = {}, +): AssemblyRoot { + const log = deps.log ?? createHostLog(); + // The UI shell is real now (PR #53), but nothing wires host/command + // errors into it yet — that is the statusbar built-in's job + // (`packages/builtin/statusbar`, still a placeholder) or a later + // notification-area task, not Task 1.15's. Every other composition point + // in `core` stays a no-op sink until one of those lands. const sink = createNoopStatusSink(); - const commands = createCommandRegistry({ log, sink }); - const documents = createDocumentManager({ log, sink }); + const hostRef: { current?: ExtensionHost } = {}; + + const commands = createCommandRegistry({ + log, + sink, + activateExtension: (id) => hostRef.current?.activateExtension(id) ?? Promise.resolve(), + }); + const documents = createDocumentManager({ + log, + sink, + onLanguageActivation: (id) => hostRef.current?.onLanguage(id), + }); const fs = createFileSystem({ log }); - const config = createConfigService({ log, sink, workspaceRoot }); + + const keymap = createKeymapState(log); + const config = createConfigService({ + log, + sink, + workspaceRoot, + onKeybindingsChange: (entries) => keymap.setUserEntries(entries), + }); const context = createContextService(); + const slotRegistry = createSlotRegistry({ + log, + activateExtension: (id) => hostRef.current?.activateExtension(id) ?? Promise.resolve(), + }); + const layoutState = createLayoutStateService({ log, sink }); + const theme = createBaseTheme(); + const api = createTecodeApi({ commands, documents, @@ -81,30 +155,284 @@ export function buildAssemblyRoot(workspaceRoot: string = process.cwd()): Assemb config, context, sink, + slotRegistry, }); // Must run before any extension module is imported (see this function's - // TSDoc) — no extension loading exists yet (Task 1.15/2.x), so this is - // simply the last step here today. + // TSDoc). registerTecodeAlias(api); - return { log, sink, commands, documents, fs, config, context, api }; + return { + log, + sink, + commands, + documents, + fs, + config, + context, + api, + slotRegistry, + layoutState, + theme, + workspaceRoot, + keymap, + hostRef, + }; +} + +/** What {@link runDeferredPhase} produced. */ +export interface DeferredStartupResult { + extensionHost: ExtensionHost; + loadResult: LoadExtensionsResult; +} + +/** Options for {@link runDeferredPhase}. */ +export interface RunDeferredPhaseOptions { + /** The argv-resolved file to open once extensions have registered (Req + * 12.1's "open the initial file"). `undefined` for a directory/no-arg + * launch. */ + initialFilePath?: string; + /** Built-in manifests to discover alongside `user`/`workspace` + * extensions. Defaults to `@tecode/builtin`'s `builtinManifests` + * (currently `[]` — see that module's TSDoc); overridable for tests. */ + builtins?: Manifest[]; + /** Discovery's filesystem seam passthrough — production never sets + * this; tests use it for hermeticity (matches `discovery.test.ts`'s + * `createHermeticFs`, which blocks scanning the *real* user extensions + * directory during an in-process test). */ + fs?: DiscoveryFs; +} + +/** + * Task 1.15's deferred phase (design.md §3's step 2, scheduled via + * `queueMicrotask` by {@link runTecode} after the first frame): discover → + * validate → register every extension (`loadExtensions`), fire + * `onStartup` activations, then open the argv-resolved initial file + * (firing `onLanguage:*` via `documents.openDocument`). + * + * Exported separately from {@link runTecode} (which drives the full CLI, + * including `process.exit` in headless mode) so it can be exercised + * in-process in tests without any risk of exiting the test runner itself. + */ +export async function runDeferredPhase( + root: AssemblyRoot, + options: RunDeferredPhaseOptions = {}, +): Promise { + const loadResult = await loadExtensions({ + log: root.log, + sink: root.sink, + commands: root.commands, + configRegistrar: root.config, + builtins: options.builtins ?? builtinManifests, + workspaceRoot: root.workspaceRoot, + fs: options.fs, + }); + + // Feed registration's extension keybindings into the keymap's extension + // layer now that they are known (Phase 2's plan) — the user layer was + // already wired synchronously via ConfigService's onKeybindingsChange. + root.keymap.setExtensionEntries(loadResult.extensionKeybindings); + + const extensionHost = createExtensionHost({ + extensions: buildExtensionRecords(loadResult.loaded), + api: root.api, + log: root.log, + sink: root.sink, + }); + // Fulfills every forward reference `buildAssemblyRoot` built + // (`hostRef`) — commands/documents/slotRegistry can now actually reach + // activation. + root.hostRef.current = extensionHost; + + await extensionHost.activateStartupExtensions(); + + if (options.initialFilePath) { + // Fires onLanguage:* via DocumentManagerDeps.onLanguageActivation, + // which is exactly hostRef.current.onLanguage now that it is assigned + // above. + await root.documents.openDocument(pathToUri(options.initialFilePath)); + } + + return { extensionHost, loadResult }; +} + +/** Emit one structured, single-line JSON metric to stdout — the + * CI-parseable timing/order channel this task's plan calls for. + * `HostLog` (`@tecode/core`'s `host/errors.ts`) has no dedicated + * metrics/info level (only `error`/`warning`), so shoehorning a timing + * line into it would misuse that schema; stdout is what both the manual + * smoke check and the subprocess integration test actually parse. */ +function emitMetric(event: string, fields: Record = {}): void { + console.log(JSON.stringify({ event, ...fields })); +} + +/** Emit a step marker, only when `TECODE_VERBOSE=1` (this task's plan: + * "verbose behind an env flag") — the baseline `tecode.timing` first-frame + * metric below is always emitted regardless. */ +function emitVerboseStep(startedAt: number, step: string): void { + if (process.env["TECODE_VERBOSE"] !== "1") return; + emitMetric("tecode.step", { step, ms: performance.now() - startedAt }); } -function main(argv: string[]): void { +/** Options for {@link runTecode}. */ +export interface RunTecodeOptions { + /** Forces headless mode on/off. Defaults to `TECODE_HEADLESS=1` or "no + * real TTY on stdout" (this task's adaptation) — see this module's + * TSDoc. */ + headless?: boolean; + /** Overrides the render seam — tests substitute their own to observe + * ordering without a real terminal OR the built-in headless no-op. + * Defaults to {@link renderShellHeadless} when `headless`, else + * {@link renderShellToTerminal}. */ + renderShell?: RenderShell; + /** Overrides the built-in manifest list passed to `loadExtensions` — + * tests only; production always uses `@tecode/builtin`'s + * `builtinManifests`. */ + builtins?: Manifest[]; + /** Overrides `process.cwd()` — tests only. */ + cwd?: string; +} + +/** Sets up graceful-shutdown handling (Phase 3's "wire process-exit + * disposeAll"). A synchronous Node/Bun `"exit"` handler cannot await async + * cleanup, so this listens for `SIGINT`/`SIGTERM` instead — the standard + * pattern for a CLI that needs to flush/dispose before actually exiting — + * and calls `process.exit(0)` itself once cleanup settles. Idempotent: a + * second signal while shutdown is already in flight is a no-op. */ +function wireProcessExit(root: AssemblyRoot): void { + let shuttingDown = false; + const shutdown = async (): Promise => { + if (shuttingDown) return; + shuttingDown = true; + await root.layoutState.flush(); + root.config.dispose(); + await root.hostRef.current?.disposeAll(); + }; + for (const signal of ["SIGINT", "SIGTERM"] as const) { + process.once(signal, () => { + void shutdown().finally(() => process.exit(0)); + }); + } +} + +/** What {@link runTecode} produced, for a non-headless (real) run — a + * headless run instead calls `process.exit(0)` itself once the deferred + * phase completes and never returns this. */ +export interface RunTecodeResult { + root: AssemblyRoot; + extensionHost: ExtensionHost; + loadResult: LoadExtensionsResult; + firstFrameMs: number; +} + +/** + * Run the full startup sequence (Req 12.1, 12.2; design.md §3, §15): the + * sync phase (argv → {@link buildAssemblyRoot} → config ready → terminal + * capabilities → render the Shell = "first frame"), then the deferred + * phase ({@link runDeferredPhase}, scheduled via `queueMicrotask` so it + * never runs synchronously ahead of the first frame above it), with + * startup-to-first-frame timing instrumentation throughout. + * + * **Headless mode** (`TECODE_HEADLESS=1`, or no real TTY on stdout at + * all): the render seam defaults to {@link renderShellHeadless} (never + * opens a TTY) and, once the deferred phase completes, this function + * flushes layout state, disposes the config service's file watchers and + * the extension host, emits a final `tecode.headlessExit` metric, and + * calls `process.exit(0)` itself — `ConfigService`'s real `fs.watch` + * handles would otherwise keep the event loop (and the process) alive + * forever with no UI to justify it. This is what makes `TECODE_HEADLESS=1 + * bun packages/cli/src/main.ts ` usable as a scriptable smoke check + * and what the subprocess integration test relies on to observe a clean + * exit. + */ +export async function runTecode( + argv: readonly string[], + options: RunTecodeOptions = {}, +): Promise { + const startedAt = performance.now(); + const headless = options.headless ?? (process.env["TECODE_HEADLESS"] === "1" || !process.stdout.isTTY); + + const log = createHostLog(); + const cwd = options.cwd ?? process.cwd(); + const target: StartupTarget = await resolveStartupTarget(argv, cwd, log); + + const root = buildAssemblyRoot(target.workspaceRoot, { log }); + await root.config.ready; + emitVerboseStep(startedAt, "config-ready"); + + // Sync-phase terminal capability detection (design.md §3) — a stub + // (Task 4.2 owns the real probe; see terminalCapabilities.ts's TSDoc for + // why nothing consumes the result yet). + detectTerminalCapabilities(); + + wireProcessExit(root); + + const renderShell = options.renderShell ?? (headless ? renderShellHeadless : renderShellToTerminal); + await renderShell({ + slotRegistry: root.slotRegistry, + layoutState: root.layoutState, + context: root.context, + commands: root.commands, + theme: root.theme, + }); + + const firstFrameMs = performance.now() - startedAt; + root.log.append("warning", { + message: `[tecode:timing] first-frame ${firstFrameMs.toFixed(2)}ms`, + }); + // `ts` is a raw performance.now() reading — directly comparable, within + // this same process, against any other same-process reading (e.g. the + // subprocess integration test's fixture extension logging its own + // module-load time) without needing a shared wall-clock epoch. + emitMetric("tecode.timing", { phase: "first-frame", ms: firstFrameMs, ts: performance.now() }); + + // Deferred phase (design.md §3's step 2): queueMicrotask guarantees this + // never runs synchronously ahead of the render above — the microtask + // queue only drains after the current synchronous stack (including the + // `await renderShell(...)` continuation) has yielded. + const deferred = await new Promise((resolveDeferred, rejectDeferred) => { + queueMicrotask(() => { + runDeferredPhase(root, { + initialFilePath: target.initialFilePath, + builtins: options.builtins, + }).then(resolveDeferred, rejectDeferred); + }); + }); + emitVerboseStep(startedAt, "deferred-complete"); + + if (headless) { + emitMetric("tecode.headlessExit", { + loaded: deferred.loadResult.loaded.length, + skipped: deferred.loadResult.skipped.length, + ms: performance.now() - startedAt, + }); + await root.layoutState.flush(); + root.config.dispose(); + await deferred.extensionHost.disposeAll(); + process.exit(0); + } + + return { root, extensionHost: deferred.extensionHost, loadResult: deferred.loadResult, firstFrameMs }; +} + +async function main(argv: string[]): Promise { if (argv.includes("--version")) { console.log(pkg.version); process.exit(0); } - buildAssemblyRoot(); + await runTecode(argv); } // `import.meta.main` is Bun's "am I the entry point" check (true only when // this file itself was executed, e.g. `bun run main.ts`; false when another // module — such as this file's own test — imports it). Without this guard, -// importing `main.ts` for testing `buildAssemblyRoot` would also run -// `main(process.argv.slice(2))` as an unwanted side effect, against the -// *importing* process's real argv and real `HOME`. +// importing `main.ts` for testing `buildAssemblyRoot`/`runDeferredPhase` +// would also run `main(process.argv.slice(2))` as an unwanted side effect, +// against the *importing* process's real argv, real `HOME`, and (in +// non-headless mode) a real TTY. if (import.meta.main) { - main(process.argv.slice(2)); + main(process.argv.slice(2)).catch((cause: unknown) => { + console.error("tecode failed to start:", cause); + process.exit(1); + }); } diff --git a/packages/cli/src/renderShell.test.ts b/packages/cli/src/renderShell.test.ts new file mode 100644 index 0000000..aa8f02e --- /dev/null +++ b/packages/cli/src/renderShell.test.ts @@ -0,0 +1,41 @@ +import { expect, test } from "bun:test"; +import { + createContextService, + createHostLog, + createLayoutStateService, + createNoopStatusSink, + createSlotRegistry, + createCommandRegistry, +} from "@tecode/core"; +import { createBaseTheme } from "@tecode/core"; +import { renderShellHeadless } from "./renderShell"; + +// renderShellToTerminal is intentionally NOT exercised here: it opens a +// real @opentui/core CliRenderer/TTY, which bun test's sandboxed, non-TTY +// stdout cannot provide (and must never attempt to — see renderShell.tsx's +// TSDoc and this task's TECODE_HEADLESS adaptation). `bunx tsc --noEmit` +// is what proves it type-checks; this file only proves the headless seam +// used by every other test (and TECODE_HEADLESS=1) behaves. + +test("renderShellHeadless resolves without touching a real terminal", async () => { + const log = createHostLog(); + const sink = createNoopStatusSink(); + const deps = { + slotRegistry: createSlotRegistry({ log }), + layoutState: createLayoutStateService({ + log, + sink, + path: "/dev/null/unused-in-this-test", + fs: { + readFile: () => Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })), + mkdir: () => Promise.resolve(), + writeFile: () => Promise.resolve(), + }, + }), + context: createContextService(), + commands: createCommandRegistry({ log, sink }), + theme: createBaseTheme(), + }; + + await expect(renderShellHeadless(deps)).resolves.toBeUndefined(); +}); diff --git a/packages/cli/src/renderShell.tsx b/packages/cli/src/renderShell.tsx new file mode 100644 index 0000000..e9a401e --- /dev/null +++ b/packages/cli/src/renderShell.tsx @@ -0,0 +1,82 @@ +/** + * The shell-render seam (Req 12.1, 12.2; design.md §3, §8.1; tasks.md's + * Task 1.15: "shell render" as the sync phase's "first frame" step). + * + * `runTecode` (`main.ts`) never calls `@opentui/core`/`@opentui/react` + * directly — it calls `deps.renderShell(...)`, injectable so tests (and + * `TECODE_HEADLESS=1`) can substitute {@link renderShellHeadless} and never + * open a real TTY. {@link renderShellToTerminal} is the seam's default, + * real implementation. + */ + +import { createCliRenderer } from "@opentui/core"; +import { createRoot } from "@opentui/react"; +import type { ResolvedTheme } from "@tecode/api"; +import { + ContextFocusTracker, + Shell, + ThemeProvider, + type CommandRegistry, + type ContextService, + type LayoutStateService, + type SlotRegistry, +} from "@tecode/core"; + +/** Everything one `renderShell` call needs to mount the Shell (this + * module's TSDoc) — exactly the live services `main.ts`'s sync phase has + * already built by the time it calls this. */ +export interface ShellRenderDeps { + slotRegistry: SlotRegistry; + layoutState: LayoutStateService; + context: ContextService; + commands: CommandRegistry; + theme: ResolvedTheme; +} + +/** The render seam's shape: resolves once "first frame" has happened (see + * each implementation's TSDoc for what that means for it). Guarded at its + * one call site in `main.ts` — an implementation that throws still leaves + * `runTecode`'s own never-throwing startup contract to that call site, not + * to this type. */ +export type RenderShell = (deps: ShellRenderDeps) => Promise; + +/** + * The real implementation: mounts ` + * ` (design.md §8.1's + * component tree) onto a real `CliRenderer`, opening the actual terminal. + * + * **"First frame" here is a documented approximation.** `createRoot(...).render()` + * synchronously commits the initial React tree onto OpenTUI's host config; + * `CliRenderer` then runs its own frame loop to actually draw to the + * terminal. Precisely awaiting "pixels reached the terminal" would need a + * lower level `CliRenderer` render-loop hook this task did not need to + * reverse-engineer for a <100ms sanity budget (design.md §15) — yielding + * one microtask after the synchronous commit is a defensible, cheap proxy + * for "the shell has painted its first frame," consistent with this task's + * other documented stubs (`terminalCapabilities.ts`). + */ +export const renderShellToTerminal: RenderShell = async (deps) => { + const renderer = await createCliRenderer(); + const root = createRoot(renderer); + root.render( + + + + + , + ); + await Promise.resolve(); +}; + +/** + * The headless/no-op implementation (the adaptation this task's plan + * requires): never touches `@opentui/core`/opens a TTY. Used whenever + * `TECODE_HEADLESS=1` is set (or stdout is not a TTY at all), and by every + * test that exercises `runTecode`/`main` without a real terminal. "First + * frame" for a headless run is simply the moment this resolves — there is + * no terminal to paint, but the deferred phase still needs a well-defined + * point to measure startup timing from and to start after. + */ +export const renderShellHeadless: RenderShell = async () => { + // Intentionally does nothing — see this module's TSDoc. +}; diff --git a/packages/cli/src/terminalCapabilities.test.ts b/packages/cli/src/terminalCapabilities.test.ts new file mode 100644 index 0000000..7f6236d --- /dev/null +++ b/packages/cli/src/terminalCapabilities.test.ts @@ -0,0 +1,15 @@ +import { expect, test } from "bun:test"; +import { detectTerminalCapabilities } from "./terminalCapabilities"; + +test("returns fixed conservative defaults (real detection is Task 4.2)", () => { + expect(detectTerminalCapabilities()).toEqual({ + kittyKeyboardProtocol: false, + colorDepth: "truecolor", + }); +}); + +test("is a pure, side-effect-free, never-throwing call", () => { + expect(() => detectTerminalCapabilities()).not.toThrow(); + // Same result on every call — nothing here reads process state (yet). + expect(detectTerminalCapabilities()).toEqual(detectTerminalCapabilities()); +}); diff --git a/packages/cli/src/terminalCapabilities.ts b/packages/cli/src/terminalCapabilities.ts new file mode 100644 index 0000000..3979ec2 --- /dev/null +++ b/packages/cli/src/terminalCapabilities.ts @@ -0,0 +1,39 @@ +/** + * Terminal-capability detection stub (Req 4.7, design.md §3, §6.5; + * tasks.md's Task 1.15: "terminal capability detection stub"). Real + * detection — querying the Kitty Keyboard Protocol, sniffing + * `COLORTERM`/terminfo for color depth — is Task 4.2's job. This stub + * returns fixed, conservative defaults so the sync phase has something to + * call here without adding real I/O: a genuine terminal query is a + * request/response round-trip that cannot be awaited synchronously inside + * the sync phase's <100ms first-frame budget (design.md §15), so it is + * deliberately deferred rather than half-implemented here. + * + * Nothing downstream consumes the result yet: the keymap fallback layer + * (`@tecode/core`'s `KeymapLayers.fallback`, Req 4.7) stays empty until + * Task 4.2 wires real detection into it (`packages/cli/src/keymapState.ts` + * documents the same deferral). This function exists now so that wiring is + * "populate `fallback` from this result," not "add a new call site to the + * startup sequence." + */ + +/** What the sync phase can currently learn about the host terminal — see + * this module's TSDoc for why every value here is a fixed placeholder. */ +export interface TerminalCapabilities { + /** Whether the Kitty Keyboard Protocol is assumed available (Req 4.7). + * Always `false` until Task 4.2's real probe lands — the conservative + * assumption, since treating an unsupported terminal as Kitty-capable + * would silently break otherwise-indistinguishable combinations (e.g. + * `ctrl+shift+*`) that Req 4.7's fallback keymap exists to cover. */ + kittyKeyboardProtocol: boolean; + /** Assumed terminal color depth. Always `"truecolor"` until Task 4.2's + * real probe lands and Task 2.6's theme quantization can react to a + * downgraded value. */ + colorDepth: "truecolor" | "256" | "16"; +} + +/** Detect the host terminal's capabilities (Req 4.7). See this module's + * TSDoc: returns fixed defaults today; never throws. */ +export function detectTerminalCapabilities(): TerminalCapabilities { + return { kittyKeyboardProtocol: false, colorDepth: "truecolor" }; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9adc153..8dc6636 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,6 +5,7 @@ export { createHostLog, createNoopStatusSink, discover, + getUserConfigDir, getUserExtensionsDir, getWorkspaceExtensionsDir, HOST_PLACEHOLDER, From f87944bfd60ab547b064c1c578f96fa76db04505 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 06:46:32 +0000 Subject: [PATCH 2/2] Await renderer.idle() for first frame; tighten startup budget assertion CodeRabbit review round 1 on PR #54: - renderShellToTerminal now resolves via renderer.idle() so first frame means the renderer actually finished its scheduled draw, not just a microtask after the React commit (demand-driven mode: no live loop is requested by the Shell's initial tree, so idle() cannot hang). - Integration test timing bound tightened 5000ms -> 1000ms: tasks.md's Task 1.15 explicitly asks for headroom over the 100ms budget (CI flake margin), so exactly 100 would contradict the spec, but 10x the budget still catches any real startup regression (measured ~6-15ms). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/cli/src/main.integration.test.ts | 10 ++++++---- packages/cli/src/renderShell.tsx | 19 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/main.integration.test.ts b/packages/cli/src/main.integration.test.ts index 51be937..881573f 100644 --- a/packages/cli/src/main.integration.test.ts +++ b/packages/cli/src/main.integration.test.ts @@ -115,12 +115,14 @@ test("headless startup renders the shell before any extension's index.ts loads, expect(firstFrameTs).toBeLessThan(moduleLoadedTs); expect(moduleLoadedTs).toBeLessThanOrEqual(activatedTs); - // Timing budget (design.md §15's <100ms, with generous headroom to - // avoid CI flakiness — tasks.md's own "timing check with headroom - // over 100ms"). + // Timing budget (design.md §15's <100ms). tasks.md's Task 1.15 asks + // for a "timing check with headroom over 100 ms" — a strict <100 + // bound would flake on loaded CI runners, so this enforces 10x the + // budget (measured locally: ~6–15ms), tight enough to catch any real + // startup regression. const firstFrameMs = firstFrame?.["ms"] as number; expect(firstFrameMs).toBeGreaterThanOrEqual(0); - expect(firstFrameMs).toBeLessThan(5_000); + expect(firstFrameMs).toBeLessThan(1_000); expect(headlessExit?.["loaded"]).toBe(1); expect(headlessExit?.["skipped"]).toBe(0); diff --git a/packages/cli/src/renderShell.tsx b/packages/cli/src/renderShell.tsx index e9a401e..2f239ab 100644 --- a/packages/cli/src/renderShell.tsx +++ b/packages/cli/src/renderShell.tsx @@ -45,15 +45,14 @@ export type RenderShell = (deps: ShellRenderDeps) => Promise; * ` (design.md §8.1's * component tree) onto a real `CliRenderer`, opening the actual terminal. * - * **"First frame" here is a documented approximation.** `createRoot(...).render()` - * synchronously commits the initial React tree onto OpenTUI's host config; - * `CliRenderer` then runs its own frame loop to actually draw to the - * terminal. Precisely awaiting "pixels reached the terminal" would need a - * lower level `CliRenderer` render-loop hook this task did not need to - * reverse-engineer for a <100ms sanity budget (design.md §15) — yielding - * one microtask after the synchronous commit is a defensible, cheap proxy - * for "the shell has painted its first frame," consistent with this task's - * other documented stubs (`terminalCapabilities.ts`). + * "First frame" resolves via `renderer.idle()`: `createRoot(...).render()` + * commits the initial React tree onto OpenTUI's host config and schedules + * the actual terminal draw; `idle()` resolves once the renderer has no + * pending draw work left (it resolves immediately when nothing is + * scheduled yet, so this never waits longer than the real first draw). + * The demand-driven `CliRenderer` only runs a continuous loop when a + * component requests live mode — the Shell's initial tree requests none, + * so `idle()` cannot hang here. */ export const renderShellToTerminal: RenderShell = async (deps) => { const renderer = await createCliRenderer(); @@ -65,7 +64,7 @@ export const renderShellToTerminal: RenderShell = async (deps) => { , ); - await Promise.resolve(); + await renderer.idle(); }; /**