diff --git a/.changeset/shared-hunk-session-router.md b/.changeset/shared-hunk-session-router.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/shared-hunk-session-router.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.dependency-cruiser.cjs b/.dependency-cruiser.cjs index 150226b52..1c76d9457 100644 --- a/.dependency-cruiser.cjs +++ b/.dependency-cruiser.cjs @@ -8,13 +8,14 @@ * `bun run deps:check` fails on any violation not in the baseline. */ -// UI files allowed to couple to packages/hunk/src/app and packages/hunk/src/session: the composition shell, the two -// named session adapter hooks, and the session-navigation resolution helper those hooks -// share. Everything else in packages/hunk/src/ui stays presentation-only. +// UI files allowed to couple to packages/hunk/src/app and packages/hunk/src/session: App/AppHost, +// HunkSessionHost, runInteractiveApp, the named session adapter hooks, and their shared navigation +// helper. Everything else in packages/hunk/src/ui stays presentation-only. const UI_SESSION_ADAPTERS = [ "^packages/hunk/src/ui/App\\.tsx$", "^packages/hunk/src/ui/AppHost\\.tsx$", "^packages/hunk/src/ui/runInteractiveApp\\.tsx$", + "^packages/hunk/src/ui/session/HunkSessionHost\\.tsx$", "^packages/hunk/src/ui/hooks/useHunkSessionBridge\\.ts$", "^packages/hunk/src/ui/hooks/useTerminalReview\\.ts$", "^packages/hunk/src/ui/lib/reviewState\\.ts$", diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index f4f633929..bb7ec36e3 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -347,8 +347,10 @@ second root/config pass when a global, config-path, or CLI adapter recognizes a repository unavailable to the bundled catalog. `hunk log` follows the same boundary. Core/app and `src/ui/history/` own the built-in command, -validated graph planning, presentation, themes, paging, terminal lifecycle, and child-process -orchestration. The selected adapter's public `history` capability owns traversal, filtering, +validated graph planning, presentation, themes, paging, and child-process orchestration. The shared +`src/ui/session/` runner owns the process-level renderer/root lifetime, while its closed host routes +retained history and fresh review surfaces without giving either surface terminal ownership. The +selected adapter's public `history` capability owns traversal, filtering, immutable revision and parent identities, structured decorations, and the declarative review action for a selected item. The host treats those ids as opaque and never constructs provider revision syntax or decides root/merge comparison semantics. History pages remain child-before-parent across diff --git a/docs/module-boundaries.md b/docs/module-boundaries.md index 95df201bc..115530f43 100644 --- a/docs/module-boundaries.md +++ b/docs/module-boundaries.md @@ -31,7 +31,7 @@ packages/hunk/src/session daemon/broker transport + protocol; consume packages/hunk/src/app startup composition: CLI parsing plus the wiring of core, extensions, and the session broker; no rendering packages/hunk/src/ui terminal surface; only the composition shell (App, AppHost, - runInteractiveApp), the named session adapter hooks + runInteractiveApp, session/HunkSessionHost), the named session adapter hooks (useTerminalReview, useHunkSessionBridge), and their shared navigation helper (ui/lib/reviewState) may import app/session packages/hunk/src/opentui published facade re-exporting ui/core pieces for `hunkdiff/opentui` diff --git a/docs/source-architecture.md b/docs/source-architecture.md index 2c85c9627..b3b34f6fe 100644 --- a/docs/source-architecture.md +++ b/docs/source-architecture.md @@ -8,7 +8,7 @@ Use it when adding a new module or deciding where an existing responsibility bel ```text packages/hunk/src/app/ executable composition: CLI parsing, startup plans, and shared session bootstrap -packages/hunk/src/app/session/ mounted-review registration, bridge, and reload authorization +packages/hunk/src/app/session/ mounted-review runtime, registration, bridge, and reload authorization packages/hunk/src/core/ review model, patch handling, VCS contracts, configuration, and runtime primitives packages/hunk/src/core/changeset/ the changeset model and the pipeline that acquires one: loaders, @@ -27,6 +27,7 @@ packages/hunk/src/session/client/ shared session-daemon HTTP and compatibility packages/hunk/src/session/agent/ agent-facing session CLI, command manifest, errors, and formatting packages/hunk/src/session/broker/ local daemon transport, launcher, Hunk broker state, wire parsing, projections packages/hunk/src/ui/ interactive review application, rendering, interaction, and chrome +packages/hunk/src/ui/session/ one OpenTUI renderer/root lifetime and the closed history/review surface router packages/hunk/src/extension-api/ public `hunkdiff/extension` declaration and runtime boundary packages/hunk/src/opentui/ public `hunkdiff/opentui` component boundary packages/hunk/src/lib/ small product-wide utilities with no feature ownership @@ -58,8 +59,12 @@ and bundled-provider -> core boundaries, including the public extension-barrel r Initial launch and live-session reload use `app/sessionBootstrap.ts`. That service is the one place that applies extension registrations, resolves extension-aware VCS selection, loads the normalized changeset, applies changeset transforms, and attaches session theme/config state. -Callers retain their distinct lifecycle work (terminal setup, extension rediscovery, notices, -and mounted-app state), but must not recreate this ordering. +Callers retain their distinct lifecycle work (extension rediscovery, notices, and mounted-app +state), but must not recreate this ordering. Interactive entry adapters supply their terminal, +signal, mouse, and exit-status policies to `ui/session/runHunkSession`; that runner owns one +renderer/root lifetime and waits for `HunkSessionHost` to finish graceful surface cleanup before +restoring the terminal. `HunkSessionHost` routes only the current history and review surfaces; +review reload and extension-event commit ordering remain with `AppHost`. ## Migration policy diff --git a/packages/hunk/src/ui/log/embeddedReview.test.ts b/packages/hunk/src/app/historyReview.test.ts similarity index 71% rename from packages/hunk/src/ui/log/embeddedReview.test.ts rename to packages/hunk/src/app/historyReview.test.ts index 539a776fc..dc8a4c2df 100644 --- a/packages/hunk/src/ui/log/embeddedReview.test.ts +++ b/packages/hunk/src/app/historyReview.test.ts @@ -1,38 +1,36 @@ import { describe, expect, test } from "bun:test"; import { resolve } from "node:path"; -import type { HistoryRuntime } from "../history/types"; -import { prepareEmbeddedHistoryReview } from "../runInteractiveApp"; +import type { ExtensionLoadResult } from "../extensions/types"; +import { prepareEmbeddedHistoryReview } from "./historyReview"; /** Provide only the provider-neutral fields embedded review startup consumes. */ -function createTestRuntime() { - const extensionSession = { registry: {} }; +function createTestRequest() { + const extensionSession = { registry: {} } as unknown as ExtensionLoadResult; return { - repoRoot: resolve("repository"), + action: { kind: "revision-show", revisionId: "--opaque:id" } as const, startupCwd: resolve("invocation"), providerId: "opaque-vcs", - input: { extensionPaths: ["extensions/provider.ts"], extensionsEnabled: true }, + extensionPaths: ["extensions/provider.ts"], + extensionsEnabled: true, extensionSession, - } as unknown as HistoryRuntime; + }; } describe("embedded history review bootstrap", () => { test("preserves opaque actions, invocation-relative extensions, cwd, theme, and signal", async () => { const abort = new AbortController(); let captured: { argv: string[]; deps: Record } | undefined; - const runtime = createTestRuntime(); + const request = createTestRequest(); const result = await prepareEmbeddedHistoryReview( - runtime, - { kind: "revision-show", revisionId: "--opaque:id" }, + { ...request, themeId: "github-dark", themeMode: "dark" }, { - themeId: "github-dark", - themeMode: "dark", signal: abort.signal, env: {}, prepareStartupPlanImpl: (async (argv: string[], deps: Record) => { captured = { argv, deps }; return { kind: "app", - bootstrap: { extensions: runtime.extensionSession }, + bootstrap: { extensions: request.extensionSession }, cliInput: {}, controllingTerminal: null, }; @@ -47,7 +45,7 @@ describe("embedded history review bootstrap", () => { terminalThemeMode: "dark", signal: abort.signal, }); - expect(captured?.deps.borrowedExtensionLoad).toBe(runtime.extensionSession); + expect(captured?.deps.borrowedExtensionLoad).toBe(request.extensionSession); expect(result.borrowsExtensions).toBe(true); expect(captured?.argv.join(" ")).not.toContain("--opaque:id"); }); @@ -58,8 +56,7 @@ describe("embedded history review bootstrap", () => { let called = false; await expect( prepareEmbeddedHistoryReview( - createTestRuntime(), - { kind: "revision-show", revisionId: "opaque" }, + { ...createTestRequest(), action: { kind: "revision-show", revisionId: "opaque" } }, { signal: abort.signal, prepareStartupPlanImpl: (async () => { diff --git a/packages/hunk/src/app/historyReview.ts b/packages/hunk/src/app/historyReview.ts new file mode 100644 index 000000000..b6ad0cd46 --- /dev/null +++ b/packages/hunk/src/app/historyReview.ts @@ -0,0 +1,81 @@ +import { resolve } from "node:path"; +import type { AppBootstrap } from "../core/bootstrap"; +import type { TerminalThemeMode } from "../core/theme/detection"; +import type { ExtensionVcsHistoryReviewAction } from "../extension-api/types"; +import { retireExtensionLoadResult } from "../extensions/events"; +import type { ExtensionLoadResult } from "../extensions/types"; +import { prepareStartupPlan } from "./startup"; + +export interface EmbeddedHistoryReviewRequest { + action: ExtensionVcsHistoryReviewAction; + providerId: string; + startupCwd: string; + extensionsEnabled: boolean; + extensionPaths: readonly string[]; + extensionSession?: ExtensionLoadResult; + themeId?: string; + themeMode?: TerminalThemeMode; +} + +export interface EmbeddedHistoryReview { + bootstrap: AppBootstrap; + /** The history session owns this bootstrap's extension registry. */ + borrowsExtensions: boolean; +} + +/** Convert a provider-owned review declaration into one option-safe internal invocation. */ +export function historyReviewArgs(action: ExtensionVcsHistoryReviewAction) { + const payload = Buffer.from(JSON.stringify(action), "utf8").toString("base64url"); + return [action.kind === "revision-range" ? "diff" : "show", "--history-review", payload]; +} + +/** Bootstrap one provider-planned history review without creating or claiming a renderer. */ +export async function prepareEmbeddedHistoryReview( + request: EmbeddedHistoryReviewRequest, + { + signal, + env = process.env, + prepareStartupPlanImpl = prepareStartupPlan, + }: { + signal?: AbortSignal; + env?: NodeJS.ProcessEnv; + prepareStartupPlanImpl?: typeof prepareStartupPlan; + } = {}, +): Promise { + signal?.throwIfAborted(); + const extensionArgs = request.extensionPaths.flatMap((path) => [ + "--extension", + resolve(request.startupCwd, path), + ]); + const args = [ + ...historyReviewArgs(request.action), + "--vcs", + request.providerId, + ...(request.themeId ? ["--theme", request.themeId] : []), + ...(request.extensionsEnabled ? extensionArgs : ["--no-extensions"]), + ]; + const plan = await prepareStartupPlanImpl(["hunk", "hunk", ...args], { + cwd: request.startupCwd, + env, + signal, + borrowedExtensionLoad: request.extensionSession, + stdinIsTTY: true, + stdoutIsTTY: true, + terminalThemeMode: request.themeMode, + }); + if (signal?.aborted && plan.kind === "app") { + plan.controllingTerminal?.close(); + if (plan.bootstrap.extensions !== request.extensionSession) { + await retireExtensionLoadResult(plan.bootstrap.extensions); + } + signal.throwIfAborted(); + } + if (plan.kind !== "app") { + throw new Error("The selected commit did not produce an interactive review."); + } + plan.controllingTerminal?.close(); + return { + bootstrap: plan.bootstrap as AppBootstrap, + borrowsExtensions: plan.bootstrap.extensions === request.extensionSession, + }; +} diff --git a/packages/hunk/src/app/session/reviewRuntime.ts b/packages/hunk/src/app/session/reviewRuntime.ts new file mode 100644 index 000000000..061187f96 --- /dev/null +++ b/packages/hunk/src/app/session/reviewRuntime.ts @@ -0,0 +1,41 @@ +import { createNativeSessionBrokerLifecycleClock } from "@hunk/session-broker"; +import type { AppBootstrap } from "../../core/bootstrap"; +import { SessionBrokerClient } from "../../session/broker/brokerClient"; +import { reportHunkSessionBrokerLifecycleDefect } from "../../session/broker/lifecycleDefect"; +import { ReviewProducer } from "../review/producer"; +import { createInitialSessionSnapshot, createSessionRegistration } from "./registration"; + +export interface ReviewSessionRuntime { + hostClient: SessionBrokerClient; + reviewProducer: ReviewProducer; + stop(): void; +} + +/** Create broker and producer resources for one independently mountable review surface. */ +export function createReviewSessionRuntime( + bootstrap: AppBootstrap, + cwd = process.cwd(), +): ReviewSessionRuntime { + const reviewProducer = new ReviewProducer({ + files: bootstrap.changeset.files, + sourceLabel: bootstrap.changeset.sourceLabel, + }); + const publication = reviewProducer.getPublication(); + const lifecycleClock = createNativeSessionBrokerLifecycleClock(); + const hostClient = new SessionBrokerClient( + createSessionRegistration(bootstrap, publication, cwd), + createInitialSessionSnapshot(bootstrap, publication), + { lifecycleClock, onDefect: reportHunkSessionBrokerLifecycleDefect }, + ); + hostClient.start(); + let stopped = false; + return { + hostClient, + reviewProducer, + stop() { + if (stopped) return; + hostClient.stop(); + stopped = true; + }, + }; +} diff --git a/packages/hunk/src/core/process/shutdown.test.ts b/packages/hunk/src/core/process/shutdown.test.ts deleted file mode 100644 index 97d8b7eae..000000000 --- a/packages/hunk/src/core/process/shutdown.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { describe, expect, mock, test } from "bun:test"; -import { shutdownSession } from "./shutdown"; - -describe("shutdownSession", () => { - test("unmounts, destroys, and exits without clearing the restored screen", () => { - const events: string[] = []; - const exit = mock((code: number) => { - events.push(`exit:${code}`); - }); - - shutdownSession({ - root: { - unmount: () => events.push("unmount"), - }, - renderer: { - destroy: () => events.push("destroy"), - }, - exit, - }); - - expect(events).toEqual(["unmount", "destroy", "exit:0"]); - expect(exit).toHaveBeenCalledWith(0); - }); -}); diff --git a/packages/hunk/src/core/process/shutdown.ts b/packages/hunk/src/core/process/shutdown.ts deleted file mode 100644 index 17719c566..000000000 --- a/packages/hunk/src/core/process/shutdown.ts +++ /dev/null @@ -1,27 +0,0 @@ -/** Minimal root contract needed for app shutdown. */ -export interface ShutdownRoot { - unmount: () => void; -} - -/** Minimal renderer contract needed for app shutdown. */ -export interface ShutdownRenderer { - destroy: () => void; -} - -/** - * Tear down the TUI session and let the renderer restore the previous terminal screen. - * The caller owns any once-only guard around this helper. - */ -export function shutdownSession({ - root, - renderer, - exit = (code: number) => process.exit(code), -}: { - root: ShutdownRoot; - renderer: ShutdownRenderer; - exit?: (code: number) => never | void; -}) { - root.unmount(); - renderer.destroy(); - exit(0); -} diff --git a/packages/hunk/src/main.tsx b/packages/hunk/src/main.tsx index 9ebf7fa7f..34ca1c93a 100644 --- a/packages/hunk/src/main.tsx +++ b/packages/hunk/src/main.tsx @@ -128,19 +128,10 @@ async function main() { } // OpenTUI stays behind the interactive plan so headless commands never materialize its embedded - // native library. The highlighting client starts the compiled worker only when an opted-in, - // eligible diff needs it, so normal sessions do not pay its startup cost. The interactive app - // owns that worker's disposal: this call returns once the app is mounted, not once it exits. + // native library. The shared interactive runner owns the highlighting worker and terminal until + // the mounted surface acknowledges graceful shutdown. const { runInteractiveApp } = await import("./ui/runInteractiveApp"); - try { - await runInteractiveApp(startupPlan); - } catch (error) { - startupPlan.controllingTerminal?.close(); - await ( - await import("./extensions/events") - ).retireExtensionLoadResult(startupPlan.bootstrap.extensions); - throw error; - } + await runInteractiveApp(startupPlan); } await main().catch((error) => { diff --git a/packages/hunk/src/ui/history/runInteractiveHistory.test.ts b/packages/hunk/src/ui/history/runInteractiveHistory.test.ts index 9d91e6cad..c4bbac973 100644 --- a/packages/hunk/src/ui/history/runInteractiveHistory.test.ts +++ b/packages/hunk/src/ui/history/runInteractiveHistory.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { historyReviewArgs } from "./runInteractiveHistory"; +import { historyReviewArgs } from "../../app/historyReview"; describe("history review child arguments", () => { test("encodes provider-owned opaque actions without exposing ids to CLI option parsing", () => { diff --git a/packages/hunk/src/ui/history/runInteractiveHistory.ts b/packages/hunk/src/ui/history/runInteractiveHistory.ts index bf698746d..592e81278 100644 --- a/packages/hunk/src/ui/history/runInteractiveHistory.ts +++ b/packages/hunk/src/ui/history/runInteractiveHistory.ts @@ -1,3 +1,2 @@ -/** Preserve the original import seam while interactive history moves into the log feature folder. */ -export { historyReviewArgs } from "../log/reviewLaunch"; +/** Preserve the original runner import seam while interactive history lives in the log feature. */ export { runInteractiveLog as runInteractiveHistory } from "../log/runInteractiveLog"; diff --git a/packages/hunk/src/ui/log/LogApp.tsx b/packages/hunk/src/ui/log/LogApp.tsx index b03bc09e6..51ead2de5 100644 --- a/packages/hunk/src/ui/log/LogApp.tsx +++ b/packages/hunk/src/ui/log/LogApp.tsx @@ -2,7 +2,7 @@ import type { KeyEvent, MouseEvent as TuiMouseEvent } from "@opentui/core"; import { useKeyboard, useRenderer, useTerminalDimensions } from "@opentui/react"; import { basename } from "node:path"; import { useEffect, useRef, useState, useSyncExternalStore } from "react"; -import type { ExtensionVcsHistoryReviewAction } from "../../extension-api/types"; +import type { ExtensionVcsHistoryCommit } from "../../extension-api/types"; import { sanitizeTerminalLine } from "../../lib/terminalText"; import { HelpDialog } from "../components/chrome/HelpDialog"; import { MenuBar } from "../components/chrome/MenuBar"; @@ -30,7 +30,8 @@ export type LogAppOutcome = | { kind: "quit"; exitCode?: number } | { kind: "open-review"; - action: ExtensionVcsHistoryReviewAction; + commit: ExtensionVcsHistoryCommit; + parentRevisionId?: string; themeId: string; themeMode: "dark" | "light"; }; @@ -55,9 +56,10 @@ export function LogApp({ const [transientNotice, setTransientNotice] = useState(""); const [openingCommit, setOpeningCommit] = useState<{ id: string; subject: string } | null>(null); const lastClick = useRef({ index: -1, at: 0 }); - // Lock synchronously before awaiting provider planning so coalesced Enter+q input cannot - // quit the log or leak the trailing command into the child review. + // Lock synchronously before requesting review preparation so coalesced input cannot + // open two child reviews. Quit remains available while the host settles pending work. const reviewPending = useRef(false); + const reviewQuitEnabled = useRef(false); const themeController = useThemeSelectorController({ customThemes: runtime.customThemes, initialTheme: snapshot.themeId, @@ -89,30 +91,29 @@ export function LogApp({ }; const openSelected = async (parentRevisionId?: string) => { if (reviewPending.current) return; - const planned = controller.planSelectedReview(parentRevisionId); - if (!planned) return; - reviewPending.current = true; const currentRow = controller.getSelectedRow(); - setOpeningCommit( - currentRow - ? { - id: sanitizeTerminalLine(currentRow.commit.displayId), - subject: sanitizeTerminalLine(currentRow.commit.subject), - } - : null, - ); + if (!currentRow) return; + reviewPending.current = true; + reviewQuitEnabled.current = false; + setOpeningCommit({ + id: sanitizeTerminalLine(currentRow.commit.displayId), + subject: sanitizeTerminalLine(currentRow.commit.subject), + }); try { - const action = await planned; - // Commit the loading surface before in-process provider startup performs synchronous probes. + // Commit the loading surface and consume input coalesced with the opening key/click before + // provider planning starts. Deliberate quit input remains available after this boundary. await new Promise((resolve) => setImmediate(resolve)); + reviewQuitEnabled.current = true; await onOutcome({ kind: "open-review", - action, + commit: currentRow.commit, + ...(parentRevisionId === undefined ? {} : { parentRevisionId }), themeId: themeController.themeId, themeMode: terminalThemeMode, }); } catch (error) { reviewPending.current = false; + reviewQuitEnabled.current = false; setOpeningCommit(null); controller.setNotice(error instanceof Error ? error.message : String(error)); } @@ -273,12 +274,40 @@ export function LogApp({ key.preventDefault(); key.stopPropagation(); }; + const name = key.name; + const sequence = key.sequence ?? ""; if (reviewPending.current) { + if (!reviewQuitEnabled.current) { + consume(); + return; + } + if (menu.getActiveMenuId()) { + if (name === "escape") menu.closeMenu(); + else if (name === "left") menu.switchMenu(-1); + else if (name === "right" || name === "tab") menu.switchMenu(1); + else if (name === "up") menu.moveMenuItem(-1); + else if (name === "down") menu.moveMenuItem(1); + else if (name === "return" || name === "enter") menu.activateCurrentMenuItem(); + else { + const command = matchLogCommand(key); + if (command === "quit") { + menu.closeMenu(); + executeCommand(command, key.ctrl && key.name === "c" ? 130 : undefined); + } + } + consume(); + return; + } + if (name === "f10") menu.openMenu("file"); + else { + const command = matchLogCommand(key); + if (command === "quit") { + executeCommand(command, key.ctrl && key.name === "c" ? 130 : undefined); + } + } consume(); return; } - const name = key.name; - const sequence = key.sequence ?? ""; if (parentSelectorIndex !== null) { const parents = controller.getSelectedRow()?.commit.parentRevisionIds ?? []; if (name === "escape") setParentSelectorIndex(null); diff --git a/packages/hunk/src/ui/log/LogSessionHost.tsx b/packages/hunk/src/ui/log/LogSessionHost.tsx deleted file mode 100644 index 86c91bc07..000000000 --- a/packages/hunk/src/ui/log/LogSessionHost.tsx +++ /dev/null @@ -1,143 +0,0 @@ -import { useCallback, useEffect, useRef, useState } from "react"; -import { retireExtensionLoadResult } from "../../extensions/events"; -import { resolveStartupUpdateNotice } from "../../core/process/updateNotice"; -import type { HistoryRuntime } from "../history/types"; -import { AppHost } from "../AppHost"; -import { - createReviewSessionRuntime, - prepareEmbeddedHistoryReview, - type EmbeddedHistoryReview, - type ReviewSessionRuntime, -} from "../runInteractiveApp"; -import { interactiveLogUsesColor } from "./colorPolicy"; -import { LogApp, type LogAppOutcome } from "./LogApp"; -import { LogController } from "./controller"; - -interface MountedReview { - plan: EmbeddedHistoryReview; - runtime: ReviewSessionRuntime; - instanceId: number; -} - -/** Route history and fresh review sessions through one stable React and terminal renderer root. */ -export function LogSessionHost({ - controller, - runtime, - externalQuitSignal, - onQuit, -}: { - controller: LogController; - runtime: HistoryRuntime; - externalQuitSignal: AbortSignal; - onQuit: (exitCode?: number) => void; -}) { - const [review, setReview] = useState(null); - const reviewRef = useRef(review); - reviewRef.current = review; - const [preparing, setPreparing] = useState(false); - const preparingRef = useRef(preparing); - preparingRef.current = preparing; - const nextInstanceRef = useRef(1); - const preparationControllerRef = useRef(null); - - const retireReview = useCallback(() => { - const current = reviewRef.current; - if (!current) return; - reviewRef.current = null; - current.runtime.stop(); - setReview(null); - if (externalQuitSignal.aborted) onQuit(); - }, [externalQuitSignal, onQuit]); - - const handleLogOutcome = async (outcome: LogAppOutcome) => { - if (outcome.kind === "quit") { - preparationControllerRef.current?.abort( - new Error("History review preparation was cancelled."), - ); - onQuit(outcome.exitCode); - return; - } - if (preparingRef.current || reviewRef.current) return; - preparingRef.current = true; - setPreparing(true); - const preparationController = new AbortController(); - preparationControllerRef.current = preparationController; - const preparationSignal = AbortSignal.any([externalQuitSignal, preparationController.signal]); - let plan: EmbeddedHistoryReview | undefined; - try { - plan = await prepareEmbeddedHistoryReview(runtime, outcome.action, { - themeId: outcome.themeId, - themeMode: outcome.themeMode, - signal: preparationSignal, - }); - preparationSignal.throwIfAborted(); - const reviewRuntime = createReviewSessionRuntime( - plan.bootstrap, - runtime.startupCwd ?? runtime.repoRoot, - ); - const mounted = { - plan, - runtime: reviewRuntime, - instanceId: nextInstanceRef.current++, - } satisfies MountedReview; - reviewRef.current = mounted; - setReview(mounted); - } catch (error) { - if (plan && !plan.borrowsExtensions && !reviewRef.current) { - await retireExtensionLoadResult(plan.bootstrap.extensions); - } - if (preparationSignal.aborted) { - if (externalQuitSignal.aborted) onQuit(); - } else throw error; - } finally { - if (preparationControllerRef.current === preparationController) { - preparationControllerRef.current = null; - } - preparingRef.current = false; - setPreparing(false); - } - }; - - useEffect( - () => () => { - preparationControllerRef.current?.abort( - new Error("History review host unmounted during preparation."), - ); - }, - [], - ); - - useEffect(() => { - if (reviewRef.current || preparing) return; - const requestQuit = () => onQuit(); - if (externalQuitSignal.aborted) requestQuit(); - else externalQuitSignal.addEventListener("abort", requestQuit, { once: true }); - return () => externalQuitSignal.removeEventListener("abort", requestQuit); - }, [externalQuitSignal, onQuit, preparing, review]); - - if (review) { - return ( - undefined} - returnToHistory - extensionOwnership={review.plan.borrowsExtensions ? "borrowed" : "owned"} - reviewProducer={review.runtime.reviewProducer} - startupNoticeResolver={resolveStartupUpdateNotice} - /> - ); - } - - return ( - - ); -} diff --git a/packages/hunk/src/ui/log/controller.ts b/packages/hunk/src/ui/log/controller.ts index dbae4c3f7..fa5a04eee 100644 --- a/packages/hunk/src/ui/log/controller.ts +++ b/packages/hunk/src/ui/log/controller.ts @@ -1,6 +1,5 @@ import { createHistoryLaneCheckpoint, planHistoryPage } from "../../core/history/lanePlanner"; import type { HistoryGraphRow, HistoryLaneCheckpoint } from "../../core/history/types"; -import type { ExtensionVcsHistoryReviewAction } from "../../extension-api/types"; import { sanitizeTerminalLine } from "../../lib/terminalText"; import type { HistoryRuntime } from "../history/types"; @@ -313,21 +312,11 @@ export class LogController { this.setNotice("History refreshed."); } - /** Ask the selected provider to describe the review without interpreting revision syntax. */ + /** Return the currently selected immutable provider history row. */ getSelectedRow() { return this.snapshot.rows[this.snapshot.selected]; } - planSelectedReview(parentRevisionId?: string): Promise | null { - const commit = this.getSelectedRow()?.commit; - return commit - ? this.runtime.planReview( - commit, - parentRevisionId === undefined ? undefined : { parentRevisionId }, - ) - : null; - } - async close() { if (this.closed) return; this.closed = true; diff --git a/packages/hunk/src/ui/log/reviewLaunch.ts b/packages/hunk/src/ui/log/reviewLaunch.ts deleted file mode 100644 index ddbcd01cd..000000000 --- a/packages/hunk/src/ui/log/reviewLaunch.ts +++ /dev/null @@ -1,7 +0,0 @@ -import type { ExtensionVcsHistoryReviewAction } from "../../extension-api/types"; - -/** Convert a provider-owned review declaration into one option-safe internal invocation. */ -export function historyReviewArgs(action: ExtensionVcsHistoryReviewAction) { - const payload = Buffer.from(JSON.stringify(action), "utf8").toString("base64url"); - return [action.kind === "revision-range" ? "diff" : "show", "--history-review", payload]; -} diff --git a/packages/hunk/src/ui/log/runInteractiveLog.tsx b/packages/hunk/src/ui/log/runInteractiveLog.tsx index bdb59d737..730a71c32 100644 --- a/packages/hunk/src/ui/log/runInteractiveLog.tsx +++ b/packages/hunk/src/ui/log/runInteractiveLog.tsx @@ -1,23 +1,10 @@ -import { createCliRenderer } from "@opentui/core"; -import { createRoot } from "@opentui/react"; -import { - installJobControlInterruptSupport, - installJobControlSuspendSupport, - type JobControlInterruptSupport, - type JobControlSuspendSupport, -} from "../../core/process/jobControl"; -import { shutdownSession } from "../../core/process/shutdown"; import { HunkUserError } from "../../core/run/errors"; -import { - installTerminalDisconnectSupport, - type TerminalDisconnectSupport, -} from "../../core/process/terminal"; -import { disposeHighlightWorker } from "../diff/worker"; import type { HistoryRuntime } from "../history/types"; +import { HunkSessionHost, type HistorySurfaceRoute } from "../session/HunkSessionHost"; +import { runHunkSession } from "../session/runHunkSession"; import { LogController } from "./controller"; -import { LogSessionHost } from "./LogSessionHost"; -const LOG_SHUTDOWN_SIGNALS: NodeJS.Signals[] = +export const LOG_SHUTDOWN_SIGNALS: NodeJS.Signals[] = process.platform === "win32" ? ["SIGINT", "SIGTERM", "SIGBREAK"] : ["SIGINT", "SIGTERM", "SIGHUP"]; @@ -43,70 +30,30 @@ export async function runInteractiveLog( } const controller = new LogController(runtime); - const quitController = new AbortController(); - let renderer: Awaited> | undefined; - let root: ReturnType | undefined; - let interrupt: JobControlInterruptSupport = { dispose: () => undefined }; - let suspend: JobControlSuspendSupport = { dispose: () => undefined }; - let disconnect: TerminalDisconnectSupport = { dispose: () => undefined }; - let settled = false; - let finish!: (exitCode?: number) => void; - const outcome = new Promise((resolve) => { - finish = (exitCode) => { - if (settled) return; - settled = true; - resolve(exitCode); - }; - }); - const requestQuit = () => quitController.abort(); - const requestInterrupt = () => { - process.exitCode = 130; - quitController.abort(); - }; - const signalHandlers = new Map void>( - LOG_SHUTDOWN_SIGNALS.map((signal) => [ - signal, - () => { - process.exitCode = logSignalExitCode(signal); - quitController.abort(); - }, - ]), - ); + const initialRoute: HistorySurfaceRoute = { kind: "history", controller, runtime }; + let runnerOwnsCleanup = false; try { await controller.loadMore(); - renderer = await createCliRenderer({ + runnerOwnsCleanup = true; + const exitCode = await runHunkSession({ stdin, stdout, useMouse: true, - screenMode: "alternate-screen", - exitOnCtrlC: false, - exitSignals: [], - openConsoleOnError: true, + signals: LOG_SHUTDOWN_SIGNALS, + signalExitCode: logSignalExitCode, + interruptExitCode: 130, + beforeTeardown: () => controller.close(), + render: ({ externalQuitSignal, finish }) => ( + + ), }); - root = createRoot(renderer); - interrupt = installJobControlInterruptSupport(renderer, requestInterrupt); - suspend = installJobControlSuspendSupport(renderer); - disconnect = installTerminalDisconnectSupport(stdin, requestQuit); - for (const [signal, handler] of signalHandlers) process.once(signal, handler); - root.render( - , - ); - const exitCode = await outcome; if (exitCode !== undefined) process.exitCode = exitCode; } finally { - for (const [signal, handler] of signalHandlers) process.off(signal, handler); - interrupt.dispose(); - suspend.dispose(); - disconnect.dispose(); - disposeHighlightWorker(); - if (root && renderer) shutdownSession({ root, renderer, exit: () => undefined }); - else renderer?.destroy(); - await controller.close(); + if (!runnerOwnsCleanup) await controller.close(); } } diff --git a/packages/hunk/src/ui/runInteractiveApp.test.tsx b/packages/hunk/src/ui/runInteractiveApp.test.tsx new file mode 100644 index 000000000..991b7c4cd --- /dev/null +++ b/packages/hunk/src/ui/runInteractiveApp.test.tsx @@ -0,0 +1,125 @@ +import { expect, mock, test } from "bun:test"; +import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; +import { runInteractiveApp } from "./runInteractiveApp"; + +test("retires extensions and closes the controlling terminal when review runtime creation fails", async () => { + const bootstrap = createTestVcsAppBootstrap({ + changesetId: "runtime-failure", + files: [], + }); + const close = mock(() => undefined); + const retireExtensions = mock(async () => undefined); + const runSession = mock(async () => undefined); + const failure = new Error("registration failed"); + + await expect( + runInteractiveApp( + { + bootstrap: bootstrap as never, + controllingTerminal: { + stdin: { isTTY: true } as never, + close, + }, + }, + { + createReviewRuntime: mock(() => { + throw failure; + }) as never, + runSession: runSession as never, + retireExtensions: retireExtensions as never, + }, + ), + ).rejects.toBe(failure); + + expect(runSession).not.toHaveBeenCalled(); + expect(retireExtensions).toHaveBeenCalledTimes(1); + expect(retireExtensions).toHaveBeenCalledWith(bootstrap.extensions); + expect(close).toHaveBeenCalledTimes(1); +}); + +test("stops the broker and retires extensions before exceptional renderer teardown", async () => { + const bootstrap = createTestVcsAppBootstrap({ + changesetId: "renderer-failure", + files: [], + }); + const events: string[] = []; + const failure = new Error("render failed"); + const stop = mock(() => events.push("stop")); + const retireExtensions = mock(async () => { + events.push("retire-start"); + await Promise.resolve(); + events.push("retire-finish"); + }); + const runSession = mock( + async (options: Parameters[0]) => { + await options.onFailure?.(failure); + events.push("destroy"); + throw failure; + }, + ); + + await expect( + runInteractiveApp( + { bootstrap: bootstrap as never, controllingTerminal: null }, + { + createReviewRuntime: (() => ({ + hostClient: undefined, + reviewProducer: undefined, + stop, + })) as never, + runSession: runSession as never, + retireExtensions: retireExtensions as never, + }, + ), + ).rejects.toBe(failure); + + expect(events).toEqual(["stop", "retire-start", "retire-finish", "destroy"]); + expect(stop).toHaveBeenCalledTimes(1); + expect(retireExtensions).toHaveBeenCalledTimes(1); +}); + +test("retries broker cleanup before teardown when the exceptional stop attempt fails", async () => { + const bootstrap = createTestVcsAppBootstrap({ + changesetId: "broker-stop-failure", + files: [], + }); + const events: string[] = []; + const failure = new Error("render failed"); + let stopAttempts = 0; + const stop = mock(() => { + stopAttempts += 1; + events.push(`stop-${stopAttempts}`); + if (stopAttempts === 1) throw new Error("socket close failed"); + }); + const retireExtensions = mock(async () => { + events.push("retire"); + }); + const runSession = mock( + async (options: Parameters[0]) => { + try { + await options.onFailure?.(failure); + } catch {} + await options.beforeTeardown?.(); + events.push("destroy"); + throw failure; + }, + ); + + await expect( + runInteractiveApp( + { bootstrap: bootstrap as never, controllingTerminal: null }, + { + createReviewRuntime: (() => ({ + hostClient: undefined, + reviewProducer: undefined, + stop, + })) as never, + runSession: runSession as never, + retireExtensions: retireExtensions as never, + }, + ), + ).rejects.toBe(failure); + + expect(events).toEqual(["stop-1", "retire", "stop-2", "destroy"]); + expect(stop).toHaveBeenCalledTimes(2); +}); diff --git a/packages/hunk/src/ui/runInteractiveApp.tsx b/packages/hunk/src/ui/runInteractiveApp.tsx index 05af1108f..2f44b331c 100644 --- a/packages/hunk/src/ui/runInteractiveApp.tsx +++ b/packages/hunk/src/ui/runInteractiveApp.tsx @@ -1,249 +1,92 @@ -import { createNativeSessionBrokerLifecycleClock } from "@hunk/session-broker"; -import { createCliRenderer } from "@opentui/core"; -import { createRoot } from "@opentui/react"; -import { resolve } from "node:path"; -import { - installJobControlInterruptSupport, - installJobControlSuspendSupport, - type JobControlInterruptSupport, - type JobControlSuspendSupport, -} from "../core/process/jobControl"; -import { shutdownSession } from "../core/process/shutdown"; -import { - installTerminalDisconnectSupport, - shouldUseMouseForApp, - type ControllingTerminal, - type TerminalDisconnectSupport, -} from "../core/process/terminal"; +import { shouldUseMouseForApp, type ControllingTerminal } from "../core/process/terminal"; import type { AppBootstrap } from "../core/bootstrap"; import { resolveStartupUpdateNotice } from "../core/process/updateNotice"; -import { prepareStartupPlan } from "../app/startup"; -import { ReviewProducer } from "../app/review/producer"; -import { - createInitialSessionSnapshot, - createSessionRegistration, -} from "../app/session/registration"; -import { SessionBrokerClient } from "../session/broker/brokerClient"; -import { reportHunkSessionBrokerLifecycleDefect } from "../session/broker/lifecycleDefect"; -import type { ExtensionVcsHistoryReviewAction } from "../extension-api/types"; -import type { HistoryRuntime } from "./history/types"; -import { historyReviewArgs } from "./log/reviewLaunch"; -import { AppHost } from "./AppHost"; -import { disposeHighlightWorker } from "./diff/worker"; +import { createReviewSessionRuntime } from "../app/session/reviewRuntime"; import { retireExtensionLoadResult } from "../extensions/events"; import type { ExtensionLoadResult } from "../extensions/types"; +import { HunkSessionHost, type StandaloneReviewSurfaceRoute } from "./session/HunkSessionHost"; +import { runHunkSession } from "./session/runHunkSession"; export interface InteractiveAppInput { bootstrap: AppBootstrap; controllingTerminal: ControllingTerminal | null; } -export interface ReviewSessionRuntime { - hostClient: SessionBrokerClient; - reviewProducer: ReviewProducer; - stop(): void; -} - -export interface EmbeddedHistoryReview { - bootstrap: AppBootstrap; - /** The history runtime owns this bootstrap's extension registry. */ - borrowsExtensions: boolean; -} - -/** Create broker and producer resources for one independently mountable review surface. */ -export function createReviewSessionRuntime( - bootstrap: AppBootstrap, - cwd = process.cwd(), -): ReviewSessionRuntime { - const reviewProducer = new ReviewProducer({ - files: bootstrap.changeset.files, - sourceLabel: bootstrap.changeset.sourceLabel, - }); - const publication = reviewProducer.getPublication(); - const lifecycleClock = createNativeSessionBrokerLifecycleClock(); - const hostClient = new SessionBrokerClient( - createSessionRegistration(bootstrap, publication, cwd), - createInitialSessionSnapshot(bootstrap, publication), - { lifecycleClock, onDefect: reportHunkSessionBrokerLifecycleDefect }, - ); - hostClient.start(); - let stopped = false; - return { - hostClient, - reviewProducer, - stop() { - if (stopped) return; - stopped = true; - hostClient.stop(); - }, - }; -} - -/** Bootstrap one provider-planned history review without creating or claiming a renderer. */ -export async function prepareEmbeddedHistoryReview( - runtime: HistoryRuntime, - action: ExtensionVcsHistoryReviewAction, - { - themeId, - themeMode, - signal, - env = process.env, - prepareStartupPlanImpl = prepareStartupPlan, - }: { - themeId?: string; - themeMode?: "dark" | "light"; - signal?: AbortSignal; - env?: NodeJS.ProcessEnv; - prepareStartupPlanImpl?: typeof prepareStartupPlan; - } = {}, -): Promise { - signal?.throwIfAborted(); - const startupCwd = runtime.startupCwd ?? runtime.repoRoot; - const extensionArgs = runtime.input.extensionPaths.flatMap((path) => [ - "--extension", - resolve(startupCwd, path), - ]); - const args = [ - ...historyReviewArgs(action), - "--vcs", - runtime.providerId, - ...(themeId ? ["--theme", themeId] : []), - ...(runtime.input.extensionsEnabled ? extensionArgs : ["--no-extensions"]), - ]; - const plan = await prepareStartupPlanImpl(["hunk", "hunk", ...args], { - cwd: startupCwd, - env, - signal, - borrowedExtensionLoad: runtime.extensionSession, - stdinIsTTY: true, - stdoutIsTTY: true, - terminalThemeMode: themeMode, - }); - if (signal?.aborted && plan.kind === "app") { - plan.controllingTerminal?.close(); - if (plan.bootstrap.extensions !== runtime.extensionSession) { - await retireExtensionLoadResult(plan.bootstrap.extensions); - } - signal.throwIfAborted(); - } - if (plan.kind !== "app") { - throw new Error("The selected commit did not produce an interactive review."); - } - plan.controllingTerminal?.close(); - return { - bootstrap: plan.bootstrap as AppBootstrap, - borrowsExtensions: plan.bootstrap.extensions === runtime.extensionSession, - }; +export interface InteractiveAppDeps { + createReviewRuntime?: typeof createReviewSessionRuntime; + runSession?: typeof runHunkSession; + retireExtensions?: typeof retireExtensionLoadResult; } // Leave fatal process faults to their default OS disposition. -const APP_SHUTDOWN_SIGNALS: NodeJS.Signals[] = +export const APP_SHUTDOWN_SIGNALS: NodeJS.Signals[] = process.platform === "win32" ? ["SIGINT", "SIGTERM", "SIGBREAK"] : ["SIGINT", "SIGTERM", "SIGHUP", "SIGQUIT", "SIGPIPE"]; /** Load and run the OpenTUI review app after startup has selected an interactive plan. */ -export async function runInteractiveApp({ - bootstrap, - controllingTerminal, -}: InteractiveAppInput): Promise { - const reviewSession = createReviewSessionRuntime(bootstrap, bootstrap.reloadContext.cwd); - const { hostClient, reviewProducer } = reviewSession; - - // Keep OpenTUI's platform-safe threading default (enabled on macOS, disabled on Linux). +export async function runInteractiveApp( + { bootstrap, controllingTerminal }: InteractiveAppInput, + deps: InteractiveAppDeps = {}, +): Promise { + const createReviewRuntime = deps.createReviewRuntime ?? createReviewSessionRuntime; + const runSession = deps.runSession ?? runHunkSession; + const retireExtensions = deps.retireExtensions ?? retireExtensionLoadResult; const rendererStdin = controllingTerminal?.stdin ?? process.stdin; - let renderer: Awaited>; + let terminalClosed = false; + const closeTerminal = () => { + if (terminalClosed) return; + terminalClosed = true; + controllingTerminal?.close(); + }; + let reviewRuntime: ReturnType | undefined; + let runnerOwnsFailureCleanup = false; + let runtimeCleanupAttempted = false; + try { - renderer = await createCliRenderer({ + reviewRuntime = createReviewRuntime(bootstrap, bootstrap.reloadContext.cwd); + const initialRoute: StandaloneReviewSurfaceRoute = { + kind: "review", + instanceId: 1, + bootstrap, + runtime: reviewRuntime, + }; + runnerOwnsFailureCleanup = true; + await runSession({ stdin: rendererStdin, stdout: process.stdout, useMouse: shouldUseMouseForApp({ hasControllingTerminal: Boolean(controllingTerminal), }), - screenMode: "alternate-screen", - exitOnCtrlC: false, - // OpenTUI's destroy-only handlers can strand sessions with active broker handles. - exitSignals: [], - openConsoleOnError: true, - onDestroy: () => controllingTerminal?.close(), - }); - } catch (error) { - reviewSession.stop(); - controllingTerminal?.close(); - await retireExtensionLoadResult(bootstrap.extensions); - throw error; - } - - const appRenderer = renderer; - let root: ReturnType; - try { - root = createRoot(appRenderer); - } catch (error) { - reviewSession.stop(); - appRenderer.destroy(); - controllingTerminal?.close(); - await retireExtensionLoadResult(bootstrap.extensions); - throw error; - } - const externalQuitController = new AbortController(); - let shuttingDown = false; - let jobControlSuspendSupport: JobControlSuspendSupport = { dispose: () => undefined }; - let jobControlInterruptSupport: JobControlInterruptSupport = { dispose: () => undefined }; - let terminalDisconnectSupport: TerminalDisconnectSupport = { dispose: () => undefined }; - - /** Ask AppHost to retire extension authority before tearing down the terminal. */ - function requestQuit() { - externalQuitController.abort(); - } - - /** Tear down the renderer before exit so the primary terminal screen comes back cleanly. */ - function shutdown(exitProcess = true) { - if (shuttingDown) { - return; - } - - shuttingDown = true; - for (const signal of APP_SHUTDOWN_SIGNALS) { - process.off(signal, requestQuit); - } - jobControlInterruptSupport.dispose(); - jobControlSuspendSupport.dispose(); - terminalDisconnectSupport.dispose(); - reviewSession.stop(); - // Release the syntax worker here rather than from the executable entrypoint: this function - // returns once the app is mounted, so an entrypoint-side dispose would fire before the first - // eligible diff ever asked for the worker. - disposeHighlightWorker(); - shutdownSession({ - root, - renderer: appRenderer, - ...(exitProcess ? {} : { exit: () => undefined }), + signals: APP_SHUTDOWN_SIGNALS, + onRendererDestroy: closeTerminal, + onFailure: async () => { + try { + reviewRuntime?.stop(); + runtimeCleanupAttempted = true; + } finally { + await retireExtensions(bootstrap.extensions); + } + }, + beforeTeardown: () => { + if (runtimeCleanupAttempted) return; + reviewRuntime?.stop(); + runtimeCleanupAttempted = true; + }, + render: ({ externalQuitSignal, finish }) => ( + + ), }); - } - - try { - for (const signal of APP_SHUTDOWN_SIGNALS) { - process.once(signal, requestQuit); - } - // Install after the renderer so a disconnect closes the live session instead of racing startup. - terminalDisconnectSupport = installTerminalDisconnectSupport(rendererStdin, requestQuit); - jobControlInterruptSupport = installJobControlInterruptSupport(appRenderer, requestQuit); - jobControlSuspendSupport = installJobControlSuspendSupport(appRenderer); - - // The app owns the full alternate screen session from this point on. - root.render( - , - ); } catch (error) { - shutdown(false); - await retireExtensionLoadResult(bootstrap.extensions); + if (!runnerOwnsFailureCleanup) await retireExtensions(bootstrap.extensions); throw error; + } finally { + if (!runtimeCleanupAttempted) reviewRuntime?.stop(); + closeTerminal(); } } diff --git a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx new file mode 100644 index 000000000..369d2b83e --- /dev/null +++ b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx @@ -0,0 +1,432 @@ +import { expect, mock, test } from "bun:test"; +import { testRender } from "@opentui/react/test-utils"; +import { act } from "react"; +import { createTestVcsAppBootstrap } from "../../../../../test/helpers/app-bootstrap"; +import { createTestDiffFile } from "../../../../../test/helpers/diff-helpers"; +import type { HistoryRuntime } from "../history/types"; +import { LogController } from "../log/controller"; +import { + HunkSessionHost, + type HistorySurfaceRoute, + type HunkSessionHostDeps, +} from "./HunkSessionHost"; + +mock.restore(); + +/** Create a loaded one-row history route for session-host navigation tests. */ +async function createHistoryRoute() { + const runtime: HistoryRuntime = { + input: { + kind: "history", + color: "never", + format: "compact", + ascii: false, + static: false, + extensionsEnabled: false, + extensionPaths: [], + }, + source: { + async read() { + return { + commits: [ + { + revisionId: "revision-a", + displayId: "revision", + parentRevisionIds: [], + subject: "History row", + authorName: "Ada", + authoredAt: "2026-01-01T00:00:00Z", + decorations: [], + }, + ], + done: true, + }; + }, + async close() {}, + }, + providerId: "test", + providerName: "Test", + repoRoot: "/repo", + notices: [], + customThemes: [], + async planReview() { + return { kind: "revision-show", revisionId: "revision-a" }; + }, + async reopenSource() { + return this.source; + }, + async close() {}, + }; + const controller = new LogController(runtime); + await controller.loadMore(); + return { kind: "history", controller, runtime } satisfies HistorySurfaceRoute; +} + +/** Select Quit through the live history menu while review preparation owns the input lock. */ +async function selectHistoryMenuQuit(setup: Awaited>) { + await act(async () => setup.mockInput.pressKey("f10")); + await act(async () => setup.mockInput.pressKey("q")); +} + +/** Flush async history planning and mounted review shutdown work. */ +async function settle(setup: Awaited>) { + await act(async () => { + await Bun.sleep(20); + await setup.renderOnce(); + await Bun.sleep(20); + await setup.renderOnce(); + }); +} + +test("routes repeated history reviews through fresh runtimes and returns instead of quitting", async () => { + const history = await createHistoryRoute(); + const quit = mock(() => undefined); + const stops: Array> = []; + let instance = 0; + const deps: HunkSessionHostDeps = { + prepareReview: (async () => ({ + bootstrap: createTestVcsAppBootstrap({ + changesetId: `review-${++instance}`, + files: [createTestDiffFile({ id: "review.ts", path: "review.ts" })], + }), + borrowsExtensions: true, + })) as never, + createReviewRuntime: (() => { + const stop = mock(() => undefined); + stops.push(stop); + return { hostClient: undefined, reviewProducer: undefined, stop }; + }) as never, + }; + const abort = new AbortController(); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("History row"); + + await act(async () => setup.mockInput.pressEnter()); + await settle(setup); + expect(setup.captureCharFrame()).not.toContain("History row"); + await act(async () => setup.mockInput.pressKey("q")); + await settle(setup); + expect(setup.captureCharFrame()).toContain("History row"); + + await act(async () => setup.mockInput.pressEnter()); + await settle(setup); + await act(async () => setup.mockInput.pressKey("q")); + await settle(setup); + + expect(stops).toHaveLength(2); + expect(stops[0]).toHaveBeenCalledTimes(1); + expect(stops[1]).toHaveBeenCalledTimes(1); + expect(quit).not.toHaveBeenCalled(); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("quits the session from a standalone review route", async () => { + const quit = mock(() => undefined); + const stop = mock(() => undefined); + const abort = new AbortController(); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressKey("q")); + await settle(setup); + expect(stop).toHaveBeenCalledTimes(1); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + } +}); + +test("finishes review navigation even when broker shutdown throws", async () => { + const quit = mock(() => undefined); + const stop = mock(() => { + throw new Error("broker close failed"); + }); + const abort = new AbortController(); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressKey("q")); + await settle(setup); + expect(stop).toHaveBeenCalledTimes(1); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + } +}); + +test("does not start provider planning after shutdown wins the pre-dispatch window", async () => { + const history = await createHistoryRoute(); + const planReview = mock(history.runtime.planReview); + history.runtime.planReview = planReview; + const abort = new AbortController(); + const quit = mock(() => undefined); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => { + setup.mockInput.pressEnter(); + abort.abort(); + }); + await settle(setup); + expect(planReview).not.toHaveBeenCalled(); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("keeps history mounted when review preparation fails", async () => { + const history = await createHistoryRoute(); + const quit = mock(() => undefined); + const setup = await testRender( + { + throw new Error("provider failed"); + }) as never, + }} + />, + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await settle(setup); + expect(setup.captureCharFrame()).toContain("History row"); + expect(setup.captureCharFrame()).toContain("provider failed"); + expect(quit).not.toHaveBeenCalled(); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("waits for non-cooperative provider planning before menu quit", async () => { + const history = await createHistoryRoute(); + let resolvePlanning!: (value: { kind: "revision-show"; revisionId: string }) => void; + const planning = new Promise<{ kind: "revision-show"; revisionId: string }>((resolve) => { + resolvePlanning = resolve; + }); + history.runtime.planReview = mock(() => planning); + const prepareReview = mock(async () => { + throw new Error("cancelled planning reached preparation"); + }); + const quit = mock(() => undefined); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await Bun.sleep(10); + await selectHistoryMenuQuit(setup); + expect(quit).not.toHaveBeenCalled(); + + resolvePlanning({ kind: "revision-show", revisionId: "revision-a" }); + await settle(setup); + expect(prepareReview).not.toHaveBeenCalled(); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("waits for non-cooperative provider planning after an external signal", async () => { + const history = await createHistoryRoute(); + let resolvePlanning!: (value: { kind: "revision-show"; revisionId: string }) => void; + const planning = new Promise<{ kind: "revision-show"; revisionId: string }>((resolve) => { + resolvePlanning = resolve; + }); + history.runtime.planReview = mock(() => planning); + const abort = new AbortController(); + const quit = mock(() => undefined); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await Bun.sleep(10); + act(() => abort.abort()); + expect(quit).not.toHaveBeenCalled(); + + resolvePlanning({ kind: "revision-show", revisionId: "revision-a" }); + await settle(setup); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("defers menu quit until cancelled preparation and retirement settle", async () => { + const history = await createHistoryRoute(); + const events: string[] = []; + const quit = mock(() => events.push("quit")); + let resolvePreparation!: (value: unknown) => void; + const preparation = new Promise((resolve) => { + resolvePreparation = resolve; + }); + const setup = await testRender( + preparation) as never, + retirePreparedExtensions: (async () => { + events.push("retire"); + }) as never, + }} + />, + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await Bun.sleep(10); + await selectHistoryMenuQuit(setup); + expect(quit).not.toHaveBeenCalled(); + + resolvePreparation({ + bootstrap: createTestVcsAppBootstrap({ + changesetId: "cancelled-review", + files: [createTestDiffFile({ id: "cancelled.ts", path: "cancelled.ts" })], + }), + borrowsExtensions: false, + }); + await settle(setup); + expect(events).toEqual(["retire", "quit"]); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + +test("cancels stale preparation, retires its owned registry, and quits once", async () => { + const history = await createHistoryRoute(); + const events: string[] = []; + const quit = mock(() => events.push("quit")); + let resolveRetirement!: () => void; + const retirement = new Promise((resolve) => { + resolveRetirement = resolve; + }); + const retire = mock(async () => { + events.push("retire-start"); + await retirement; + events.push("retire-finish"); + }); + let resolvePreparation!: (value: unknown) => void; + const preparation = new Promise((resolve) => { + resolvePreparation = resolve; + }); + const abort = new AbortController(); + const setup = await testRender( + preparation) as never, + createReviewRuntime: mock(() => { + throw new Error("stale review mounted"); + }) as never, + retirePreparedExtensions: retire as never, + }} + />, + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await Bun.sleep(10); + act(() => abort.abort()); + expect(quit).not.toHaveBeenCalled(); + resolvePreparation({ + bootstrap: createTestVcsAppBootstrap({ + changesetId: "stale-review", + files: [createTestDiffFile({ id: "stale.ts", path: "stale.ts" })], + }), + borrowsExtensions: false, + }); + await act(async () => { + await Bun.sleep(10); + await setup.renderOnce(); + }); + expect(retire).toHaveBeenCalledTimes(1); + expect(quit).not.toHaveBeenCalled(); + resolveRetirement(); + await settle(setup); + expect(quit).toHaveBeenCalledTimes(1); + expect(events).toEqual(["retire-start", "retire-finish", "quit"]); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); diff --git a/packages/hunk/src/ui/session/HunkSessionHost.tsx b/packages/hunk/src/ui/session/HunkSessionHost.tsx new file mode 100644 index 000000000..e3a4135c8 --- /dev/null +++ b/packages/hunk/src/ui/session/HunkSessionHost.tsx @@ -0,0 +1,274 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { + prepareEmbeddedHistoryReview, + type EmbeddedHistoryReview, + type EmbeddedHistoryReviewRequest, +} from "../../app/historyReview"; +import { + createReviewSessionRuntime, + type ReviewSessionRuntime, +} from "../../app/session/reviewRuntime"; +import type { StartupNotice } from "../../core/process/startupNotice"; +import type { AppBootstrap } from "../../core/bootstrap"; +import { retireExtensionLoadResult } from "../../extensions/events"; +import type { ExtensionLoadResult } from "../../extensions/types"; +import { AppHost } from "../AppHost"; +import type { HistoryRuntime } from "../history/types"; +import { interactiveLogUsesColor } from "../log/colorPolicy"; +import { LogApp, type LogAppOutcome } from "../log/LogApp"; +import type { LogController } from "../log/controller"; + +export interface HistorySurfaceRoute { + kind: "history"; + controller: LogController; + runtime: HistoryRuntime; +} + +export interface StandaloneReviewSurfaceRoute { + kind: "review"; + instanceId: number; + bootstrap: AppBootstrap; + runtime: ReviewSessionRuntime; +} + +export type HunkSurfaceRoute = HistorySurfaceRoute | StandaloneReviewSurfaceRoute; + +interface ActiveReviewSurfaceRoute extends StandaloneReviewSurfaceRoute { + extensionOwnership: "owned" | "borrowed"; + quitBehavior: "return-to-history" | "quit-session"; + mountMode: "initial" | "dynamic"; + returnRoute?: HistorySurfaceRoute; +} + +type ActiveSurfaceRoute = HistorySurfaceRoute | ActiveReviewSurfaceRoute; + +export interface HunkSessionHostDeps { + prepareReview?: typeof prepareEmbeddedHistoryReview; + createReviewRuntime?: typeof createReviewSessionRuntime; + retirePreparedExtensions?: typeof retireExtensionLoadResult; +} + +/** + * Route retained history and fresh review surfaces inside one stable React root. + * + * History selections and standalone review startup converge here. The host owns route preparation + * and review-surface disposal; `runHunkSession` retains terminal ownership, while `AppHost` retains + * review reload and extension-event commit ordering. + */ +export function HunkSessionHost({ + initialRoute, + externalQuitSignal, + onQuit, + startupNoticeResolver, + deps = {}, +}: { + initialRoute: HunkSurfaceRoute; + externalQuitSignal: AbortSignal; + onQuit: (exitCode?: number) => void; + startupNoticeResolver?: () => Promise; + deps?: HunkSessionHostDeps; +}) { + const prepareReview = deps.prepareReview ?? prepareEmbeddedHistoryReview; + const createReviewRuntime = deps.createReviewRuntime ?? createReviewSessionRuntime; + const retirePreparedExtensions = deps.retirePreparedExtensions ?? retireExtensionLoadResult; + const [route, setRoute] = useState(() => + initialRoute.kind === "history" + ? initialRoute + : { + ...initialRoute, + extensionOwnership: "owned", + quitBehavior: "quit-session", + mountMode: "initial", + }, + ); + const routeRef = useRef(route); + routeRef.current = route; + const mountedRef = useRef(true); + const preparingRef = useRef(false); + const preparationControllerRef = useRef(null); + const preparationGenerationRef = useRef(0); + const nextInstanceRef = useRef(initialRoute.kind === "review" ? initialRoute.instanceId + 1 : 1); + const quitRequestedRef = useRef(false); + const shutdownPendingRef = useRef(false); + const pendingExitCodeRef = useRef(undefined); + const failedReviewStopsRef = useRef(new Set()); + + /** Attempt broker cleanup and retain failed runtimes for the final unmount retry. */ + const stopReviewRuntime = useCallback((runtime: ReviewSessionRuntime) => { + try { + runtime.stop(); + failedReviewStopsRef.current.delete(runtime); + } catch { + failedReviewStopsRef.current.add(runtime); + } + }, []); + + const completeQuit = useCallback(() => { + if (quitRequestedRef.current) return; + quitRequestedRef.current = true; + onQuit(pendingExitCodeRef.current); + }, [onQuit]); + + const requestQuit = useCallback( + (exitCode?: number) => { + shutdownPendingRef.current = true; + if (pendingExitCodeRef.current === undefined) pendingExitCodeRef.current = exitCode; + preparationGenerationRef.current += 1; + preparationControllerRef.current?.abort( + new Error("Hunk surface preparation was cancelled during shutdown."), + ); + if (routeRef.current.kind === "history" && !preparingRef.current) completeQuit(); + }, + [completeQuit], + ); + + const retireReview = useCallback(() => { + const current = routeRef.current; + if (current.kind !== "review") return; + stopReviewRuntime(current.runtime); + if (externalQuitSignal.aborted || current.quitBehavior === "quit-session") { + completeQuit(); + return; + } + const returnRoute = current.returnRoute; + if (!returnRoute) { + completeQuit(); + return; + } + routeRef.current = returnRoute; + setRoute(returnRoute); + }, [completeQuit, externalQuitSignal, stopReviewRuntime]); + + const handleHistoryOutcome = async ( + historyRoute: HistorySurfaceRoute, + outcome: LogAppOutcome, + ) => { + if (outcome.kind === "quit") { + requestQuit(outcome.exitCode); + return; + } + if ( + !mountedRef.current || + shutdownPendingRef.current || + externalQuitSignal.aborted || + preparingRef.current || + routeRef.current.kind !== "history" + ) { + return; + } + preparingRef.current = true; + const generation = ++preparationGenerationRef.current; + const preparationController = new AbortController(); + preparationControllerRef.current = preparationController; + const signal = AbortSignal.any([externalQuitSignal, preparationController.signal]); + const startupCwd = historyRoute.runtime.startupCwd ?? historyRoute.runtime.repoRoot; + let plan: EmbeddedHistoryReview | undefined; + try { + const action = await historyRoute.runtime.planReview( + outcome.commit, + outcome.parentRevisionId === undefined + ? undefined + : { parentRevisionId: outcome.parentRevisionId }, + ); + signal.throwIfAborted(); + const request: EmbeddedHistoryReviewRequest = { + action, + providerId: historyRoute.runtime.providerId, + startupCwd, + extensionsEnabled: historyRoute.runtime.input.extensionsEnabled, + extensionPaths: historyRoute.runtime.input.extensionPaths, + extensionSession: historyRoute.runtime.extensionSession, + themeId: outcome.themeId, + themeMode: outcome.themeMode, + }; + plan = await prepareReview(request, { signal }); + signal.throwIfAborted(); + if ( + !mountedRef.current || + generation !== preparationGenerationRef.current || + routeRef.current !== historyRoute + ) { + if (!plan.borrowsExtensions) { + await retirePreparedExtensions(plan.bootstrap.extensions); + } + return; + } + const reviewRuntime = createReviewRuntime(plan.bootstrap, startupCwd); + const reviewRoute: ActiveReviewSurfaceRoute = { + kind: "review", + instanceId: nextInstanceRef.current++, + bootstrap: plan.bootstrap, + runtime: reviewRuntime, + extensionOwnership: plan.borrowsExtensions ? "borrowed" : "owned", + quitBehavior: "return-to-history", + mountMode: "dynamic", + returnRoute: historyRoute, + }; + routeRef.current = reviewRoute; + setRoute(reviewRoute); + } catch (error) { + if (plan && !plan.borrowsExtensions && routeRef.current.kind !== "review") { + await retirePreparedExtensions(plan.bootstrap.extensions); + } + if (!signal.aborted) throw error; + } finally { + if (preparationControllerRef.current === preparationController) { + preparationControllerRef.current = null; + } + preparingRef.current = false; + if (shutdownPendingRef.current && routeRef.current.kind === "history") { + completeQuit(); + } + } + }; + + useEffect(() => { + const requestExternalQuit = () => requestQuit(); + if (externalQuitSignal.aborted) requestExternalQuit(); + else + externalQuitSignal.addEventListener("abort", requestExternalQuit, { + once: true, + }); + return () => externalQuitSignal.removeEventListener("abort", requestExternalQuit); + }, [externalQuitSignal, requestQuit]); + + useEffect( + () => () => { + mountedRef.current = false; + preparationGenerationRef.current += 1; + preparationControllerRef.current?.abort( + new Error("Hunk session host unmounted during surface preparation."), + ); + const current = routeRef.current; + if (current.kind === "review") stopReviewRuntime(current.runtime); + for (const runtime of failedReviewStopsRef.current) stopReviewRuntime(runtime); + }, + [stopReviewRuntime], + ); + + if (route.kind === "review") { + return ( + undefined } : {})} + returnToHistory={route.quitBehavior === "return-to-history"} + extensionOwnership={route.extensionOwnership} + reviewProducer={route.runtime.reviewProducer} + startupNoticeResolver={startupNoticeResolver} + /> + ); + } + + return ( + handleHistoryOutcome(route, outcome)} + /> + ); +} diff --git a/packages/hunk/src/ui/session/runHunkSession.test.tsx b/packages/hunk/src/ui/session/runHunkSession.test.tsx new file mode 100644 index 000000000..5e4f148a6 --- /dev/null +++ b/packages/hunk/src/ui/session/runHunkSession.test.tsx @@ -0,0 +1,282 @@ +import { describe, expect, mock, test } from "bun:test"; +import type { CliRenderer } from "@opentui/core"; +import type { createRoot } from "@opentui/react"; +import { runHunkSession, type HunkSessionRunnerDeps } from "./runHunkSession"; + +/** Build injected renderer/root dependencies without starting a terminal. */ +function createTestDeps(events: string[]) { + const renderer = { + isDestroyed: false, + keyInput: { on: mock(() => undefined), off: mock(() => undefined) }, + suspend: mock(() => undefined), + resume: mock(() => undefined), + destroy: mock(() => events.push("destroy")), + } as unknown as CliRenderer; + const root = { + render: mock(() => events.push("render")), + unmount: mock(() => events.push("unmount")), + } as unknown as ReturnType; + const support = (name: string) => ({ dispose: () => events.push(name) }); + const deps: HunkSessionRunnerDeps = { + createRenderer: (async () => renderer) as never, + createReactRoot: (() => root) as never, + installInterrupt: (() => support("interrupt")) as never, + installSuspend: (() => support("suspend")) as never, + installDisconnect: (() => support("disconnect")) as never, + disposeWorker: () => events.push("worker"), + onSignal: () => undefined, + offSignal: () => undefined, + }; + return { deps, renderer, root }; +} + +const stdin = { isTTY: true } as unknown as NodeJS.ReadStream & { + on: never; + off: never; +}; +const stdout = {} as NodeJS.WriteStream; + +describe("runHunkSession", () => { + test("waits for host completion and tears down every process resource once", async () => { + const events: string[] = []; + const { deps } = createTestDeps(events); + let finish: ((exitCode?: number) => void) | undefined; + const running = runHunkSession( + { + stdin, + stdout, + useMouse: true, + signals: ["SIGTERM"], + beforeTeardown: async () => { + events.push("cleanup-start"); + await Promise.resolve(); + events.push("cleanup-finish"); + }, + render: (context) => { + finish = context.finish; + return null; + }, + }, + deps, + ); + await Promise.resolve(); + expect(events).toEqual(["render"]); + + finish?.(7); + finish?.(9); + expect(await running).toBe(7); + expect(events).toEqual([ + "render", + "cleanup-start", + "cleanup-finish", + "interrupt", + "suspend", + "disconnect", + "worker", + "unmount", + "destroy", + ]); + }); + + test("requests graceful completion on a signal without tearing down early", async () => { + const events: string[] = []; + const { deps } = createTestDeps(events); + let signalHandler: (() => void) | undefined; + deps.onSignal = (_signal, listener) => { + signalHandler = listener; + }; + let finish: (() => void) | undefined; + let quitSignal: AbortSignal | undefined; + const running = runHunkSession( + { + stdin, + stdout, + useMouse: true, + signals: ["SIGTERM"], + signalExitCode: () => 143, + render: (context) => { + finish = context.finish; + quitSignal = context.externalQuitSignal; + return null; + }, + }, + deps, + ); + await Promise.resolve(); + + signalHandler?.(); + expect(quitSignal?.aborted).toBe(true); + expect(events).toEqual(["render"]); + finish?.(); + expect(await running).toBe(143); + }); + + test("still tears down the terminal when awaited session cleanup fails", async () => { + const events: string[] = []; + const { deps } = createTestDeps(events); + let finish: (() => void) | undefined; + const running = runHunkSession( + { + stdin, + stdout, + useMouse: true, + signals: [], + beforeTeardown: async () => { + events.push("cleanup"); + throw new Error("cleanup failed"); + }, + render: (context) => { + finish = context.finish; + return null; + }, + }, + deps, + ); + await Promise.resolve(); + finish?.(); + + await expect(running).rejects.toThrow("cleanup failed"); + expect(events).toEqual([ + "render", + "cleanup", + "interrupt", + "suspend", + "disconnect", + "worker", + "unmount", + "destroy", + ]); + }); + + test("isolates disposer and unmount failures so renderer cleanup still completes", async () => { + const events: string[] = []; + const { deps, renderer, root } = createTestDeps(events); + deps.installInterrupt = (() => ({ + dispose() { + events.push("interrupt"); + throw new Error("interrupt failed"); + }, + })) as never; + root.unmount = mock(() => { + events.push("unmount"); + throw new Error("unmount failed"); + }); + renderer.destroy = mock(() => { + events.push("destroy"); + throw new Error("destroy failed"); + }); + const rendererDestroyed = mock(() => events.push("renderer-callback")); + let finish: (() => void) | undefined; + const running = runHunkSession( + { + stdin, + stdout, + useMouse: true, + signals: [], + onRendererDestroy: rendererDestroyed, + render(context) { + finish = context.finish; + return null; + }, + }, + deps, + ); + await Promise.resolve(); + finish?.(); + + await expect(running).rejects.toThrow("interrupt failed"); + expect(events).toEqual([ + "render", + "interrupt", + "suspend", + "disconnect", + "worker", + "unmount", + "destroy", + "renderer-callback", + ]); + expect(rendererDestroyed).toHaveBeenCalledTimes(1); + }); + + test("runs failure ownership cleanup before attempting terminal teardown", async () => { + const events: string[] = []; + const { deps, root } = createTestDeps(events); + root.render = mock(() => { + events.push("render"); + throw new Error("render failed"); + }); + + await expect( + runHunkSession( + { + stdin, + stdout, + useMouse: false, + signals: [], + onFailure: async () => { + events.push("failure-start"); + await Promise.resolve(); + events.push("failure-finish"); + }, + render: () => null, + }, + deps, + ), + ).rejects.toThrow("render failed"); + expect(events).toEqual([ + "render", + "failure-start", + "failure-finish", + "interrupt", + "suspend", + "disconnect", + "worker", + "unmount", + "destroy", + ]); + }); + + test("destroys a renderer when root creation fails", async () => { + const events: string[] = []; + const { deps } = createTestDeps(events); + deps.createReactRoot = (() => { + throw new Error("root failed"); + }) as never; + + await expect( + runHunkSession( + { + stdin, + stdout, + useMouse: false, + signals: [], + render: () => null, + }, + deps, + ), + ).rejects.toThrow("root failed"); + expect(events).toEqual(["worker", "destroy"]); + }); + + test("disposes process-lifetime state when renderer creation fails", async () => { + const events: string[] = []; + const { deps } = createTestDeps(events); + deps.createRenderer = (async () => { + throw new Error("renderer failed"); + }) as never; + + await expect( + runHunkSession( + { + stdin, + stdout, + useMouse: false, + signals: [], + render: () => null, + }, + deps, + ), + ).rejects.toThrow("renderer failed"); + expect(events).toEqual(["worker"]); + }); +}); diff --git a/packages/hunk/src/ui/session/runHunkSession.tsx b/packages/hunk/src/ui/session/runHunkSession.tsx new file mode 100644 index 000000000..c5529691e --- /dev/null +++ b/packages/hunk/src/ui/session/runHunkSession.tsx @@ -0,0 +1,171 @@ +import { createCliRenderer } from "@opentui/core"; +import { createRoot } from "@opentui/react"; +import type { ReactNode } from "react"; +import { + installJobControlInterruptSupport, + installJobControlSuspendSupport, + type JobControlInterruptSupport, + type JobControlSuspendSupport, +} from "../../core/process/jobControl"; +import { + installTerminalDisconnectSupport, + type TerminalDisconnectSupport, + type TerminalInputEvents, +} from "../../core/process/terminal"; +import { disposeHighlightWorker } from "../diff/worker"; + +type Renderer = Awaited>; +type Root = ReturnType; +type SignalListener = () => void; + +export interface HunkSessionRenderContext { + externalQuitSignal: AbortSignal; + finish(exitCode?: number): void; +} + +export interface HunkSessionRunnerOptions { + stdin: NodeJS.ReadStream & TerminalInputEvents; + stdout: NodeJS.WriteStream; + useMouse: boolean; + signals: readonly NodeJS.Signals[]; + /** Preserve the entry surface's exit status when an OS signal asks it to quit. */ + signalExitCode?: (signal: NodeJS.Signals) => number | undefined; + /** Preserve the entry surface's exit status when raw-mode Ctrl-C asks it to quit. */ + interruptExitCode?: number; + onRendererDestroy?: () => void; + /** Settle ownership that must transfer only when session setup or rendering fails. */ + onFailure?: (error: unknown) => void | Promise; + /** Settle process-level resources that must close while the terminal surface is still mounted. */ + beforeTeardown?: () => void | Promise; + render(context: HunkSessionRenderContext): ReactNode; +} + +export interface HunkSessionRunnerDeps { + createRenderer?: typeof createCliRenderer; + createReactRoot?: typeof createRoot; + installInterrupt?: typeof installJobControlInterruptSupport; + installSuspend?: typeof installJobControlSuspendSupport; + installDisconnect?: typeof installTerminalDisconnectSupport; + disposeWorker?: typeof disposeHighlightWorker; + onSignal?: (signal: NodeJS.Signals, listener: SignalListener) => unknown; + offSignal?: (signal: NodeJS.Signals, listener: SignalListener) => unknown; +} + +/** + * Run one interactive Hunk session through one renderer and one React root. + * + * Surface hosts acknowledge graceful completion with `finish`; signals only request that + * completion, so extension retirement and started writes settle before terminal teardown. + */ +export async function runHunkSession( + options: HunkSessionRunnerOptions, + deps: HunkSessionRunnerDeps = {}, +): Promise { + const createRenderer = deps.createRenderer ?? createCliRenderer; + const createReactRoot = deps.createReactRoot ?? createRoot; + const installInterrupt = deps.installInterrupt ?? installJobControlInterruptSupport; + const installSuspend = deps.installSuspend ?? installJobControlSuspendSupport; + const installDisconnect = deps.installDisconnect ?? installTerminalDisconnectSupport; + const disposeWorker = deps.disposeWorker ?? disposeHighlightWorker; + const onSignal = deps.onSignal ?? process.once.bind(process); + const offSignal = deps.offSignal ?? process.off.bind(process); + + const quitController = new AbortController(); + let renderer: Renderer | undefined; + let root: Root | undefined; + let interrupt: JobControlInterruptSupport = { dispose: () => undefined }; + let suspend: JobControlSuspendSupport = { dispose: () => undefined }; + let disconnect: TerminalDisconnectSupport = { dispose: () => undefined }; + let settled = false; + let requestedExitCode: number | undefined; + let finishOutcome!: (exitCode?: number) => void; + const outcome = new Promise((resolve) => { + finishOutcome = (exitCode) => { + if (settled) return; + settled = true; + resolve(exitCode ?? requestedExitCode); + }; + }); + const requestQuit = (exitCode?: number) => { + if (requestedExitCode === undefined) requestedExitCode = exitCode; + if (!quitController.signal.aborted) quitController.abort(); + }; + const signalHandlers = new Map( + options.signals.map((signal) => [signal, () => requestQuit(options.signalExitCode?.(signal))]), + ); + let rendererDestroyNotified = false; + const notifyRendererDestroy = () => { + if (rendererDestroyNotified) return; + rendererDestroyNotified = true; + options.onRendererDestroy?.(); + }; + let result: number | undefined; + let failure: unknown; + let failed = false; + const recordFailure = (error: unknown) => { + if (failed) return; + failed = true; + failure = error; + }; + const attempt = (action: () => void) => { + try { + action(); + } catch (error) { + recordFailure(error); + } + }; + + try { + renderer = await createRenderer({ + stdin: options.stdin, + stdout: options.stdout, + useMouse: options.useMouse, + screenMode: "alternate-screen", + exitOnCtrlC: false, + // OpenTUI's destroy-only handlers can strand sessions with active broker handles. + exitSignals: [], + openConsoleOnError: true, + onDestroy: notifyRendererDestroy, + }); + root = createReactRoot(renderer); + interrupt = installInterrupt(renderer, () => requestQuit(options.interruptExitCode)); + suspend = installSuspend(renderer); + disconnect = installDisconnect(options.stdin, () => requestQuit()); + for (const [signal, handler] of signalHandlers) onSignal(signal, handler); + root.render( + options.render({ + externalQuitSignal: quitController.signal, + finish: finishOutcome, + }), + ); + result = await outcome; + } catch (error) { + recordFailure(error); + try { + await options.onFailure?.(error); + } catch (cleanupError) { + recordFailure(cleanupError); + } + } finally { + try { + await options.beforeTeardown?.(); + } catch (error) { + recordFailure(error); + } + for (const [signal, handler] of signalHandlers) { + attempt(() => offSignal(signal, handler)); + } + attempt(() => interrupt.dispose()); + attempt(() => suspend.dispose()); + attempt(() => disconnect.dispose()); + attempt(disposeWorker); + const mountedRoot = root; + const activeRenderer = renderer; + if (mountedRoot) attempt(() => mountedRoot.unmount()); + if (activeRenderer) attempt(() => activeRenderer.destroy()); + attempt(notifyRendererDestroy); + } + + if (failed) throw failure; + return result; +} diff --git a/test/cli/startup-graph.test.ts b/test/cli/startup-graph.test.ts index fb4ff6e00..d188d40bb 100644 --- a/test/cli/startup-graph.test.ts +++ b/test/cli/startup-graph.test.ts @@ -99,15 +99,13 @@ describe("CLI startup graph", () => { expect(eagerlyDeferred).toEqual([]); }); - test("worker disposal stays with the interactive app rather than the entrypoint", () => { - // The entrypoint resolves once the app is mounted, so disposing from there would terminate the - // worker before the first large diff requested it. - const interactiveAppSource = readFileSync( - join(REPO_ROOT, "packages/hunk/src/ui/runInteractiveApp.tsx"), + test("worker disposal stays with the shared interactive session runner", () => { + const sessionRunnerSource = readFileSync( + join(REPO_ROOT, "packages/hunk/src/ui/session/runHunkSession.tsx"), "utf8", ); expect(readModuleSource(ENTRYPOINT).includes("disposeHighlightWorker")).toBe(false); - expect(interactiveAppSource.includes("disposeHighlightWorker()")).toBe(true); + expect(sessionRunnerSource.includes("disposeHighlightWorker")).toBe(true); }); });