From 9af12a51a0b896a74bca33a4d094827b676d300c Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sat, 5 Sep 2026 23:42:28 -0400 Subject: [PATCH] refactor(extensions): centralize session ownership --- .../explicit-extension-session-owner.md | 2 + benchmarks/highlight-prefetch.ts | 2 +- benchmarks/huge-stream.ts | 2 +- benchmarks/interaction-latency.ts | 2 +- benchmarks/large-stream.ts | 2 +- benchmarks/lib/appHost.tsx | 29 +++ benchmarks/memory.ts | 2 +- benchmarks/navigation-memory.ts | 2 +- benchmarks/non-ascii-stream.ts | 2 +- benchmarks/resize-memory.ts | 2 +- benchmarks/wrapped-cjk.ts | 2 +- docs/extension-architecture.md | 8 +- docs/extensions.md | 5 + docs/source-architecture.md | 7 +- .../hunk/src/app/historyBootstrap.test.ts | 4 + packages/hunk/src/app/historyBootstrap.ts | 23 +-- packages/hunk/src/app/historyReview.test.ts | 32 +++- packages/hunk/src/app/historyReview.ts | 4 +- packages/hunk/src/extensions/session.test.ts | 150 ++++++++++++++++ packages/hunk/src/extensions/session.ts | 135 ++++++++++++++ .../hunk/src/ui/AppHost.cursor-line.test.tsx | 2 +- .../src/ui/AppHost.dynamic-mount.test.tsx | 2 +- .../src/ui/AppHost.edit-in-editor.test.tsx | 2 +- .../src/ui/AppHost.extension-dialogs.test.tsx | 2 +- .../ui/AppHost.extension-navigation.test.tsx | 2 +- .../src/ui/AppHost.extension-sidebar.test.tsx | 2 +- .../hunk/src/ui/AppHost.extensions.test.tsx | 2 +- .../src/ui/AppHost.file-view-modes.test.tsx | 2 +- .../hunk/src/ui/AppHost.file-views.test.tsx | 2 +- .../hunk/src/ui/AppHost.interactions.test.tsx | 2 +- .../hunk/src/ui/AppHost.key-routing.test.tsx | 2 +- .../hunk/src/ui/AppHost.keybindings.test.tsx | 7 +- .../src/ui/AppHost.keyboard-modes.test.tsx | 2 +- packages/hunk/src/ui/AppHost.reload.test.tsx | 2 +- .../hunk/src/ui/AppHost.responsive.test.tsx | 2 +- .../src/ui/AppHost.review-metadata.test.tsx | 2 +- .../src/ui/AppHost.scroll-regression.test.tsx | 2 +- .../hunk/src/ui/AppHost.selection.test.tsx | 2 +- .../src/ui/AppHost.sidebar-resize.test.tsx | 2 +- .../ui/AppHost.sidebar-visibility.test.tsx | 2 +- packages/hunk/src/ui/AppHost.tsx | 170 ++++++------------ packages/hunk/src/ui/AppHost.watch.test.tsx | 2 +- .../hunk/src/ui/AppHost.workspace.test.tsx | 2 +- .../scrollbar/VerticalScrollbar.test.tsx | 2 +- .../src/ui/components/ui-components.test.tsx | 2 +- .../src/ui/history/runStaticHistory.test.ts | 43 ++++- .../hunk/src/ui/history/runStaticHistory.ts | 6 +- packages/hunk/src/ui/history/types.ts | 8 +- packages/hunk/src/ui/log/controller.test.ts | 2 + .../hunk/src/ui/log/runInteractiveLog.test.ts | 51 +++++- .../hunk/src/ui/log/runInteractiveLog.tsx | 22 ++- .../hunk/src/ui/runInteractiveApp.test.tsx | 29 ++- packages/hunk/src/ui/runInteractiveApp.tsx | 21 ++- .../src/ui/session/HunkSessionHost.test.tsx | 116 +++++++++--- .../hunk/src/ui/session/HunkSessionHost.tsx | 38 +++- scripts/test-large-untracked-render.tsx | 20 ++- test/helpers/app-host.tsx | 54 ++++++ test/helpers/extension-session.ts | 7 + 58 files changed, 815 insertions(+), 242 deletions(-) create mode 100644 .changeset/explicit-extension-session-owner.md create mode 100644 benchmarks/lib/appHost.tsx create mode 100644 packages/hunk/src/extensions/session.test.ts create mode 100644 packages/hunk/src/extensions/session.ts create mode 100644 test/helpers/app-host.tsx create mode 100644 test/helpers/extension-session.ts diff --git a/.changeset/explicit-extension-session-owner.md b/.changeset/explicit-extension-session-owner.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/explicit-extension-session-owner.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/benchmarks/highlight-prefetch.ts b/benchmarks/highlight-prefetch.ts index 340e14338..082a411aa 100644 --- a/benchmarks/highlight-prefetch.ts +++ b/benchmarks/highlight-prefetch.ts @@ -5,7 +5,7 @@ import React from "react"; import { testRender } from "@opentui/react/test-utils"; import { parseDiffFromFile } from "@pierre/diffs"; import { act } from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import type { AppBootstrap } from "../packages/hunk/src/core/bootstrap"; import type { DiffFile } from "../packages/hunk/src/core/changeset/model"; diff --git a/benchmarks/huge-stream.ts b/benchmarks/huge-stream.ts index 1d96bc196..f525336f2 100644 --- a/benchmarks/huge-stream.ts +++ b/benchmarks/huge-stream.ts @@ -7,7 +7,7 @@ import { performance } from "node:perf_hooks"; import { testRender } from "@opentui/react/test-utils"; import React from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createHugeStreamBootstrap, GIANT_SINGLE_FILE_LINES, diff --git a/benchmarks/interaction-latency.ts b/benchmarks/interaction-latency.ts index 6fc0ed7e8..cdc782db4 100644 --- a/benchmarks/interaction-latency.ts +++ b/benchmarks/interaction-latency.ts @@ -4,7 +4,7 @@ import { performance } from "node:perf_hooks"; import { testRender } from "@opentui/react/test-utils"; import React from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createLargeSplitStreamBootstrap, DEFAULT_FILE_COUNT, diff --git a/benchmarks/large-stream.ts b/benchmarks/large-stream.ts index 1c460762d..1d5bb0685 100644 --- a/benchmarks/large-stream.ts +++ b/benchmarks/large-stream.ts @@ -2,7 +2,7 @@ import { performance } from "perf_hooks"; import React from "react"; import { testRender } from "@opentui/react/test-utils"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { VIEWPORT_READ_COALESCE_MS } from "../packages/hunk/src/ui/lib/viewportTiming"; import { createLargeSplitStreamBootstrap, diff --git a/benchmarks/lib/appHost.tsx b/benchmarks/lib/appHost.tsx new file mode 100644 index 000000000..7e3c3f3e8 --- /dev/null +++ b/benchmarks/lib/appHost.tsx @@ -0,0 +1,29 @@ +import { useEffect, useState, type ComponentProps } from "react"; +import { createExtensionSession } from "../../packages/hunk/src/extensions/session"; +import { createEmptyExtensionLoadResult } from "../../packages/hunk/src/extensions/types"; +import { AppHost } from "../../packages/hunk/src/ui/AppHost"; + +/** Supply explicit session ownership to an isolated benchmark AppHost mount. */ +export function BenchmarkAppHost({ bootstrap }: Pick, "bootstrap">) { + const [extensionSession] = useState(() => + createExtensionSession( + (bootstrap.extensions as ReturnType | undefined) ?? + createEmptyExtensionLoadResult(bootstrap.reloadContext.cwd), + bootstrap.reloadContext.cwd, + ), + ); + useEffect( + () => () => { + void extensionSession.shutdown(); + }, + [extensionSession], + ); + return ( + extensionSession.shutdown()} + /> + ); +} diff --git a/benchmarks/memory.ts b/benchmarks/memory.ts index 34fb51d16..d341c618f 100644 --- a/benchmarks/memory.ts +++ b/benchmarks/memory.ts @@ -6,7 +6,7 @@ import { act } from "react"; import { buildSplitRows } from "../packages/hunk/src/ui/diff/diffRows"; import { buildReviewRenderPlan } from "../packages/hunk/src/ui/diff/reviewRenderPlan"; import { resolveTheme } from "../packages/hunk/src/ui/themes"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createLargeSplitStreamBootstrap } from "./large-stream-fixture"; const viewport = { width: 240, height: 28 } as const; diff --git a/benchmarks/navigation-memory.ts b/benchmarks/navigation-memory.ts index a6193feec..a1fe71d94 100644 --- a/benchmarks/navigation-memory.ts +++ b/benchmarks/navigation-memory.ts @@ -3,7 +3,7 @@ import { testRender } from "@opentui/react/test-utils"; import { performance } from "node:perf_hooks"; import React from "react"; import { act } from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createLargeSplitStreamBootstrap } from "./large-stream-fixture"; type MemorySample = { diff --git a/benchmarks/non-ascii-stream.ts b/benchmarks/non-ascii-stream.ts index 9c73ed441..578d021ea 100644 --- a/benchmarks/non-ascii-stream.ts +++ b/benchmarks/non-ascii-stream.ts @@ -5,7 +5,7 @@ import { performance } from "node:perf_hooks"; import { testRender } from "@opentui/react/test-utils"; import React from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createLargeSplitStreamBootstrap } from "./large-stream-fixture"; import { destroyRenderer, diff --git a/benchmarks/resize-memory.ts b/benchmarks/resize-memory.ts index 8e3840c19..6bf8731a1 100644 --- a/benchmarks/resize-memory.ts +++ b/benchmarks/resize-memory.ts @@ -4,7 +4,7 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { dirname, resolve } from "node:path"; import { performance } from "node:perf_hooks"; import React from "react"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { createLargeSplitStreamBootstrap } from "./large-stream-fixture"; type MemorySample = { diff --git a/benchmarks/wrapped-cjk.ts b/benchmarks/wrapped-cjk.ts index efe32beb7..02ef298ba 100644 --- a/benchmarks/wrapped-cjk.ts +++ b/benchmarks/wrapped-cjk.ts @@ -6,7 +6,7 @@ import { testRender } from "@opentui/react/test-utils"; import React, { act } from "react"; import type { AppBootstrap } from "../packages/hunk/src/core/bootstrap"; import type { DiffFile } from "../packages/hunk/src/core/changeset/model"; -import { AppHost } from "../packages/hunk/src/ui/AppHost"; +import { BenchmarkAppHost as AppHost } from "./lib/appHost"; import { prefetchHighlightedDiff } from "../packages/hunk/src/ui/diff/useHighlightedDiff"; import { VIEWPORT_READ_COALESCE_MS } from "../packages/hunk/src/ui/lib/viewportTiming"; import { resolveTheme } from "../packages/hunk/src/ui/themes"; diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index bb7ec36e3..8e97efb0e 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -55,7 +55,10 @@ and restores the previous generation if any pre-commit step fails. Staged extern retains the provisional candidate/config snapshot: a final pass that only appends repo candidates extends the same registry, while a changed prefix receives bounded `shutdown` before being rebuilt. Live registry replacement uses -the same shutdown/startup lifecycle. A factory that throws is rolled back to its +the same shutdown/startup lifecycle. `src/extensions/session.ts` owns active, provisional, and +retiring registries by identity; it synchronously closes authority at adoption/shutdown and drains +all known bounded retirements. Surfaces borrow that session and cannot retire it independently. +A factory that throws is rolled back to its pre-run registration counts (`runExtension.ts`); failures cost a warning, not the session. @@ -270,7 +273,8 @@ same guarded live navigation commands use. They can also request a current-input the current-review controller registers the latest reloadable descriptor, then AppHost resolves that descriptor at queue execution and coalesces extension requests while serializing them with manual, watch, workspace, and daemon reloads. `App` installs these controls through the -per-extension event-context provider, while `AppHost` publishes mounted +per-extension event-context provider, while `AppHost` keeps the content/broker/React commit gate +and asks the owning `ExtensionSession` to adopt only after those facts agree. `AppHost` publishes mounted lifecycle order (`startup`, then `changeset_loaded`; reloads add `session_reload`) only after the matching child commit. Headless or pre-mount delivery resolves dialogs to their cancel values and refuses navigation with a warning. diff --git a/docs/extensions.md b/docs/extensions.md index 4bc4875f1..f2889d27a 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -278,6 +278,11 @@ start watchers, processes, connections, and other long-lived resources from `startup`, and release them from `shutdown`. Extension-registry reloads create new instances and run that shutdown/startup pair around the replacement. +One host-owned `ExtensionSession` holds active, provisional, and retiring registries for a +command lifetime. It revokes replaced authority at the review commit gate, drains bounded shutdown +handlers before terminal teardown, and prevents surfaces from independently replacing or retiring +the shared registry. + An interactive history workspace owns one extension instance for its complete lifetime. Opening a commit review inside that workspace borrows the same instance: the factory and `startup` do not run again, and returning to history diff --git a/docs/source-architecture.md b/docs/source-architecture.md index b3b34f6fe..d7d6c0c34 100644 --- a/docs/source-architecture.md +++ b/docs/source-architecture.md @@ -63,8 +63,11 @@ Callers retain their distinct lifecycle work (extension rediscovery, notices, an 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`. +restoring the terminal. `HunkSessionHost` routes only the current history and review surfaces and +passes one `extensions/session.ts` owner through them. That owner holds registry authority and +retirement; review content loading, broker publication, React commit, and post-child-layout event +ordering remain with `AppHost`. Static history uses the same owner without importing OpenTUI and +closes its provider cursor before extension shutdown. ## Migration policy diff --git a/packages/hunk/src/app/historyBootstrap.test.ts b/packages/hunk/src/app/historyBootstrap.test.ts index b8f937161..e25487906 100644 --- a/packages/hunk/src/app/historyBootstrap.test.ts +++ b/packages/hunk/src/app/historyBootstrap.test.ts @@ -73,6 +73,10 @@ describe("history bootstrap cursor ownership", () => { await bootstrap.close(); await bootstrap.close(); expect(closeCounts).toEqual([1, 1]); + expect(bootstrap.extensionSession.current.registry.eventBusPhase).toBe("ready"); + await bootstrap.extensionSession.shutdown(); + await bootstrap.extensionSession.shutdown(); + expect(bootstrap.extensionSession.current.registry.eventBusPhase).toBe("closed"); } finally { rmSync(cwd, { recursive: true, force: true }); rmSync(configHome, { recursive: true, force: true }); diff --git a/packages/hunk/src/app/historyBootstrap.ts b/packages/hunk/src/app/historyBootstrap.ts index 1e362c940..08bb25b7e 100644 --- a/packages/hunk/src/app/historyBootstrap.ts +++ b/packages/hunk/src/app/historyBootstrap.ts @@ -17,8 +17,8 @@ import { } from "../core/vcs"; import type { VcsCatalog, VcsHistorySource } from "../core/vcs/types"; import { resolveExtensionVcsAdapters, resolveSessionVcsId } from "../extensions/apply"; -import { emitExtensionEvent, retireExtensionLoadResult } from "../extensions/events"; import { mergeStartupNotices } from "../extensions/startup"; +import { createExtensionSession, type ExtensionSession } from "../extensions/session"; import type { ExtensionLoadResult } from "../extensions/types"; import { resolveConfiguredExtensions } from "./extensionBootstrap"; @@ -30,9 +30,8 @@ export interface HistoryBootstrap { providerName: string; startupCwd: string; repoRoot: string; - extensions: ExtensionLoadResult; - /** History-owned extension authority borrowed by embedded reviews. */ - extensionSession: ExtensionLoadResult; + /** Command-owned extension authority borrowed by embedded reviews. */ + extensionSession: ExtensionSession; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; planReview( @@ -75,6 +74,7 @@ export async function loadHistoryBootstrap({ baseVcsCatalog, previousLoad, }); + const extensionSession = createExtensionSession(resolved.extensions, cwd); const extensionAdapters = resolveExtensionVcsAdapters( resolved.extensions.registry, baseVcsCatalog, @@ -92,7 +92,7 @@ export async function loadHistoryBootstrap({ try { adapter = getVcsAdapter(providerId, catalog); } catch (error) { - await retireExtensionLoadResult(resolved.extensions); + await extensionSession.shutdown(); throw error; } let selectedDetection; @@ -119,9 +119,9 @@ export async function loadHistoryBootstrap({ let source: VcsHistorySource; try { source = await openSource(); - emitExtensionEvent(resolved.extensions, "startup", { cwd }); + extensionSession.startCurrent(cwd); } catch (error) { - await retireExtensionLoadResult(resolved.extensions); + await extensionSession.shutdown(); throw error; } @@ -134,8 +134,7 @@ export async function loadHistoryBootstrap({ providerName: sanitizeTerminalLine(adapter.name), startupCwd: cwd, repoRoot, - extensions: resolved.extensions, - extensionSession: resolved.extensions, + extensionSession, customThemes: sessionThemes.themes, notices: [ ...(mergeStartupNotices(resolved.configured.startupNotices, resolved.extensions) ?? []).map( @@ -169,11 +168,7 @@ export async function loadHistoryBootstrap({ async close() { if (closed) return; closed = true; - try { - await source.close(); - } finally { - await retireExtensionLoadResult(resolved.extensions); - } + await source.close(); }, }; } diff --git a/packages/hunk/src/app/historyReview.test.ts b/packages/hunk/src/app/historyReview.test.ts index dc8a4c2df..652ca1042 100644 --- a/packages/hunk/src/app/historyReview.test.ts +++ b/packages/hunk/src/app/historyReview.test.ts @@ -1,11 +1,11 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, mock, test } from "bun:test"; import { resolve } from "node:path"; -import type { ExtensionLoadResult } from "../extensions/types"; +import { createEmptyExtensionLoadResult } from "../extensions/types"; import { prepareEmbeddedHistoryReview } from "./historyReview"; /** Provide only the provider-neutral fields embedded review startup consumes. */ function createTestRequest() { - const extensionSession = { registry: {} } as unknown as ExtensionLoadResult; + const extensionSession = createEmptyExtensionLoadResult(resolve("invocation")); return { action: { kind: "revision-show", revisionId: "--opaque:id" } as const, startupCwd: resolve("invocation"), @@ -30,7 +30,7 @@ describe("embedded history review bootstrap", () => { captured = { argv, deps }; return { kind: "app", - bootstrap: { extensions: request.extensionSession }, + bootstrap: { extensions: { ...request.extensionSession } }, cliInput: {}, controllingTerminal: null, }; @@ -50,6 +50,30 @@ describe("embedded history review bootstrap", () => { expect(captured?.argv.join(" ")).not.toContain("--opaque:id"); }); + test("does not retire an aliased borrowed registry when cancellation wins after startup", async () => { + const abort = new AbortController(); + const request = createTestRequest(); + const close = mock(() => undefined); + + await expect( + prepareEmbeddedHistoryReview(request, { + signal: abort.signal, + prepareStartupPlanImpl: (async () => { + abort.abort(); + return { + kind: "app", + bootstrap: { extensions: { ...request.extensionSession } }, + cliInput: {}, + controllingTerminal: { close }, + }; + }) as never, + }), + ).rejects.toThrow(); + + expect(close).toHaveBeenCalledTimes(1); + expect(request.extensionSession.registry.retirementPromise).toBeUndefined(); + }); + test("refuses an already-cancelled bootstrap before startup", async () => { const abort = new AbortController(); abort.abort(); diff --git a/packages/hunk/src/app/historyReview.ts b/packages/hunk/src/app/historyReview.ts index b6ad0cd46..068364d66 100644 --- a/packages/hunk/src/app/historyReview.ts +++ b/packages/hunk/src/app/historyReview.ts @@ -65,7 +65,7 @@ export async function prepareEmbeddedHistoryReview( }); if (signal?.aborted && plan.kind === "app") { plan.controllingTerminal?.close(); - if (plan.bootstrap.extensions !== request.extensionSession) { + if (plan.bootstrap.extensions?.registry !== request.extensionSession?.registry) { await retireExtensionLoadResult(plan.bootstrap.extensions); } signal.throwIfAborted(); @@ -76,6 +76,6 @@ export async function prepareEmbeddedHistoryReview( plan.controllingTerminal?.close(); return { bootstrap: plan.bootstrap as AppBootstrap, - borrowsExtensions: plan.bootstrap.extensions === request.extensionSession, + borrowsExtensions: plan.bootstrap.extensions?.registry === request.extensionSession?.registry, }; } diff --git a/packages/hunk/src/extensions/session.test.ts b/packages/hunk/src/extensions/session.test.ts new file mode 100644 index 000000000..3a386df8e --- /dev/null +++ b/packages/hunk/src/extensions/session.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, test } from "bun:test"; +import { createExtensionNotificationHub } from "./notifications"; +import { createExtensionSession } from "./session"; +import { + createEmptyExtensionLoadResult, + type ExtensionEventHandler, + type ExtensionLoadResult, +} from "./types"; + +/** Build a registry with observable startup and shutdown handlers. */ +function loadResult( + id: string, + events: string[], + shutdown?: () => void | Promise, +): ExtensionLoadResult { + const result = createEmptyExtensionLoadResult("/repo", createExtensionNotificationHub()); + result.registry.eventHandlers.startup.push({ + extensionId: id, + handler: ((payload: { cwd: string }) => { + events.push(`${id}:start:${payload.cwd}`); + }) as ExtensionEventHandler, + }); + result.registry.eventHandlers.shutdown.push({ + extensionId: id, + handler: async () => { + events.push(`${id}:stop`); + await shutdown?.(); + }, + }); + return result; +} + +describe("ExtensionSession", () => { + test("starts each adopted registry once and revokes its predecessor before exposing replacement", async () => { + const events: string[] = []; + let release!: () => void; + const pending = new Promise((resolve) => { + release = resolve; + }); + const initial = loadResult("initial", events, () => pending); + const replacement = loadResult("replacement", events); + const session = createExtensionSession(initial, "/one"); + + expect(session.startCurrent()).toBe(true); + expect(session.startCurrent()).toBe(false); + session.trackPrepared(replacement); + const retirement = session.adoptPrepared(replacement, "/two"); + + expect(initial.registry.eventBusPhase).toBe("closed"); + expect(session.current).toBe(replacement); + expect(session.cwd).toBe("/two"); + expect(session.startCurrent()).toBe(true); + expect(events).toEqual(["initial:start:/one", "initial:stop", "replacement:start:/two"]); + release(); + await retirement; + await session.shutdown(); + expect(events).toEqual([ + "initial:start:/one", + "initial:stop", + "replacement:start:/two", + "replacement:stop", + ]); + }); + + test("owns aliases by registry identity and retires multiple provisional registries once", async () => { + const events: string[] = []; + const initial = loadResult("initial", events); + const first = loadResult("first", events); + const alias = { ...first }; + const second = loadResult("second", events); + const session = createExtensionSession(initial, "/repo"); + + session.trackPrepared(first); + session.trackPrepared(alias); + session.trackPrepared(second); + await session.retirePrepared(); + await session.retirePrepared(first); + await session.shutdown(); + await session.shutdown(); + + expect(events.filter((event) => event === "first:stop")).toHaveLength(1); + expect(events.filter((event) => event === "second:stop")).toHaveLength(1); + expect(events.filter((event) => event === "initial:stop")).toHaveLength(1); + }); + + test("closes synchronously and drains provisional authority published during shutdown", async () => { + const events: string[] = []; + let releaseInitial!: () => void; + let releaseLate!: () => void; + const initialWait = new Promise((resolve) => { + releaseInitial = resolve; + }); + const lateWait = new Promise((resolve) => { + releaseLate = resolve; + }); + const initial = loadResult("initial", events, () => initialWait); + const late = loadResult("late", events, () => lateWait); + const session = createExtensionSession(initial, "/repo"); + + const shutdown = session.shutdown(); + expect(session.closing).toBe(true); + expect(session.startCurrent()).toBe(false); + session.trackPrepared(late); + expect(late.registry.eventBusPhase).toBe("closed"); + + let settled = false; + void shutdown.then(() => { + settled = true; + }); + releaseInitial(); + await Promise.resolve(); + expect(settled).toBe(false); + releaseLate(); + await shutdown; + expect(events).toEqual(["initial:stop", "late:stop"]); + }); + + test("a later shutdown call drains provisional work tracked after shutdown settled", async () => { + const events: string[] = []; + let releaseLate!: () => void; + const lateWait = new Promise((resolve) => { + releaseLate = resolve; + }); + const session = createExtensionSession(loadResult("initial", events), "/repo"); + await session.shutdown(); + + const late = loadResult("late", events, () => lateWait); + session.trackPrepared(late); + let settled = false; + const barrier = session.shutdown().then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + expect(late.registry.eventBusPhase).toBe("closed"); + + releaseLate(); + await barrier; + expect(events).toEqual(["initial:stop", "late:stop"]); + }); + + test("rejects adoption after closing and registries not staged by the session", async () => { + const events: string[] = []; + const session = createExtensionSession(loadResult("initial", events), "/repo"); + const foreign = loadResult("foreign", events); + expect(() => session.adoptPrepared(foreign, "/other")).toThrow("not prepared"); + await session.shutdown(); + expect(() => session.adoptPrepared(foreign, "/other")).toThrow("shutting down"); + }); +}); diff --git a/packages/hunk/src/extensions/session.ts b/packages/hunk/src/extensions/session.ts new file mode 100644 index 000000000..25504de84 --- /dev/null +++ b/packages/hunk/src/extensions/session.ts @@ -0,0 +1,135 @@ +import { emitExtensionEvent, retireExtensionLoadResult } from "./events"; +import type { ExtensionLoadResult, ExtensionRegistry } from "./types"; + +/** Owns extension registries for one command or interactive Hunk session. */ +export interface ExtensionSession { + readonly current: ExtensionLoadResult; + readonly cwd: string; + readonly closing: boolean; + startCurrent(cwd?: string): boolean; + trackPrepared(result: ExtensionLoadResult): void; + retirePrepared(result?: ExtensionLoadResult): Promise; + adoptPrepared(result: ExtensionLoadResult, cwd: string): Promise; + shutdown(): Promise; +} + +/** Coordinates active, provisional, and retiring extension registries by registry identity. */ +class ExtensionSessionImpl implements ExtensionSession { + #current: ExtensionLoadResult; + #cwd: string; + #closing = false; + #prepared = new Map(); + #started = new WeakSet(); + #retirements = new Set>(); + #shutdownPromise: Promise | undefined; + + constructor(initial: ExtensionLoadResult, cwd: string) { + this.#current = initial; + this.#cwd = cwd; + } + + get current() { + return this.#current; + } + + get cwd() { + return this.#cwd; + } + + get closing() { + return this.#closing; + } + + /** Emit startup once for each registry that reaches active authority. */ + startCurrent(cwd = this.#cwd) { + if (this.#closing) return false; + const { registry } = this.#current; + if (this.#started.has(registry)) return false; + this.#started.add(registry); + emitExtensionEvent(this.#current, "startup", { cwd }); + return true; + } + + /** Own a provisional registry before asynchronous loading can suspend. */ + trackPrepared(result: ExtensionLoadResult) { + if (result.registry === this.#current.registry) return; + if (this.#closing) { + void this.#retire(result); + return; + } + this.#prepared.set(result.registry, result); + } + + /** Retire one staged registry, or every registry that has not been adopted. */ + async retirePrepared(result?: ExtensionLoadResult) { + if (result) { + if (result.registry === this.#current.registry) return; + this.#prepared.delete(result.registry); + await this.#retire(result); + return; + } + const prepared = [...this.#prepared.values()]; + this.#prepared.clear(); + await Promise.allSettled(prepared.map((entry) => this.#retire(entry))); + } + + /** Adopt a staged registry at the caller's content commit gate and retire its predecessor. */ + adoptPrepared(result: ExtensionLoadResult, cwd: string) { + if (this.#closing) throw new Error("The extension session is shutting down."); + if (result.registry !== this.#current.registry && !this.#prepared.has(result.registry)) { + throw new Error("The extension registry was not prepared by this session."); + } + if (result.registry === this.#current.registry) { + this.#cwd = cwd; + return Promise.resolve(); + } + + const previous = this.#current; + this.#prepared.delete(result.registry); + // Retirement revokes the old event/control authority synchronously before the swap is visible. + const retirement = this.#retire(previous); + this.#current = result; + this.#cwd = cwd; + return retirement; + } + + /** Revoke every owned registry and wait for all retirement work known to this call. */ + shutdown() { + if (!this.#closing) { + this.#closing = true; + void this.#retire(this.#current); + const prepared = [...this.#prepared.values()]; + this.#prepared.clear(); + for (const result of prepared) void this.#retire(result); + } + + const previous = this.#shutdownPromise; + const shutdown = (async () => { + await previous; + while (this.#retirements.size > 0) { + await Promise.allSettled(this.#retirements); + } + })(); + this.#shutdownPromise = shutdown; + return shutdown; + } + + /** Track one registry's shared retirement completion without duplicating shutdown. */ + #retire(result: ExtensionLoadResult) { + const retirement = retireExtensionLoadResult(result); + this.#retirements.add(retirement); + void retirement.then( + () => this.#retirements.delete(retirement), + () => this.#retirements.delete(retirement), + ); + return retirement; + } +} + +/** Create one explicit owner for a loaded extension registry. */ +export function createExtensionSession( + initial: ExtensionLoadResult, + cwd: string, +): ExtensionSession { + return new ExtensionSessionImpl(initial, cwd); +} diff --git a/packages/hunk/src/ui/AppHost.cursor-line.test.tsx b/packages/hunk/src/ui/AppHost.cursor-line.test.tsx index 88c33dde7..94361e86a 100644 --- a/packages/hunk/src/ui/AppHost.cursor-line.test.tsx +++ b/packages/hunk/src/ui/AppHost.cursor-line.test.tsx @@ -4,7 +4,7 @@ import { act } from "react"; import type { CursorLine } from "../core/run/commandInputs"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile, lines } from "../../../../test/helpers/diff-helpers"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; const BEFORE = lines( "const alpha = 1;", diff --git a/packages/hunk/src/ui/AppHost.dynamic-mount.test.tsx b/packages/hunk/src/ui/AppHost.dynamic-mount.test.tsx index 9b265475b..9147f27a8 100644 --- a/packages/hunk/src/ui/AppHost.dynamic-mount.test.tsx +++ b/packages/hunk/src/ui/AppHost.dynamic-mount.test.tsx @@ -3,7 +3,7 @@ import { testRender } from "@opentui/react/test-utils"; import { act, useState } from "react"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile } from "../../../../test/helpers/diff-helpers"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; mock.restore(); diff --git a/packages/hunk/src/ui/AppHost.edit-in-editor.test.tsx b/packages/hunk/src/ui/AppHost.edit-in-editor.test.tsx index 25c8ab198..566590de7 100644 --- a/packages/hunk/src/ui/AppHost.edit-in-editor.test.tsx +++ b/packages/hunk/src/ui/AppHost.edit-in-editor.test.tsx @@ -8,7 +8,7 @@ import type { AppBootstrap } from "../core/bootstrap"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile, lines } from "../../../../test/helpers/diff-helpers"; -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); const WIDE = { width: 200, height: 24 }; diff --git a/packages/hunk/src/ui/AppHost.extension-dialogs.test.tsx b/packages/hunk/src/ui/AppHost.extension-dialogs.test.tsx index 9e8cc5316..a2d5ca6ef 100644 --- a/packages/hunk/src/ui/AppHost.extension-dialogs.test.tsx +++ b/packages/hunk/src/ui/AppHost.extension-dialogs.test.tsx @@ -13,7 +13,7 @@ import { getBundledVcsCatalog } from "../app/vcsCatalog"; import type { CliInput } from "../core/run/commandInputs"; import type { HunkSessionBrokerClient } from "../session/broker/brokerClient"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** Specialize the core loader result with extension state assigned by these tests. */ function loadAppBootstrap(...args: Parameters): Promise { diff --git a/packages/hunk/src/ui/AppHost.extension-navigation.test.tsx b/packages/hunk/src/ui/AppHost.extension-navigation.test.tsx index 2ae051598..f2e0b3b46 100644 --- a/packages/hunk/src/ui/AppHost.extension-navigation.test.tsx +++ b/packages/hunk/src/ui/AppHost.extension-navigation.test.tsx @@ -11,7 +11,7 @@ import { loadAppBootstrap as loadCoreAppBootstrap } from "../core/changeset/load import type { AppBootstrap } from "../app/types"; import { getBundledVcsCatalog } from "../app/vcsCatalog"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** Specialize the core loader result with extension state assigned by these tests. */ function loadAppBootstrap(...args: Parameters): Promise { diff --git a/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx b/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx index a4d28ad33..9448d0ba5 100644 --- a/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx +++ b/packages/hunk/src/ui/AppHost.extension-sidebar.test.tsx @@ -12,7 +12,7 @@ import { loadAppBootstrap as loadCoreAppBootstrap } from "../core/changeset/load import type { AppBootstrap } from "../app/types"; import { getBundledVcsCatalog } from "../app/vcsCatalog"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** Specialize the core loader result with extension state assigned by these tests. */ function loadAppBootstrap(...args: Parameters): Promise { diff --git a/packages/hunk/src/ui/AppHost.extensions.test.tsx b/packages/hunk/src/ui/AppHost.extensions.test.tsx index 0c6b06c25..c1dd465d8 100644 --- a/packages/hunk/src/ui/AppHost.extensions.test.tsx +++ b/packages/hunk/src/ui/AppHost.extensions.test.tsx @@ -27,7 +27,7 @@ import { } from "../extensions/apply"; import { loadStartupExtensions } from "../extensions/startup"; import { emitExtensionCustomEvent } from "../extensions/events"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** Specialize the core loader result with extension state assigned by these tests. */ function loadAppBootstrap(...args: Parameters): Promise { diff --git a/packages/hunk/src/ui/AppHost.file-view-modes.test.tsx b/packages/hunk/src/ui/AppHost.file-view-modes.test.tsx index 299782034..6db9bdfb1 100644 --- a/packages/hunk/src/ui/AppHost.file-view-modes.test.tsx +++ b/packages/hunk/src/ui/AppHost.file-view-modes.test.tsx @@ -10,7 +10,7 @@ import { createTestDiffFile } from "../../../../test/helpers/diff-helpers"; import { createWatchTestRuntime } from "../../../../test/helpers/watchTest"; import { loadAppBootstrap } from "../core/changeset/loaders"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; const tempDirs: string[] = []; setDefaultTimeout(20_000); diff --git a/packages/hunk/src/ui/AppHost.file-views.test.tsx b/packages/hunk/src/ui/AppHost.file-views.test.tsx index df6017d53..9696492c6 100644 --- a/packages/hunk/src/ui/AppHost.file-views.test.tsx +++ b/packages/hunk/src/ui/AppHost.file-views.test.tsx @@ -7,7 +7,7 @@ import { act } from "react"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile, createTestSourceFetcher } from "../../../../test/helpers/diff-helpers"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; const JSX_FILE_VIEW_EXTENSION = join( import.meta.dir, diff --git a/packages/hunk/src/ui/AppHost.interactions.test.tsx b/packages/hunk/src/ui/AppHost.interactions.test.tsx index 48c7fcb01..a302bad03 100644 --- a/packages/hunk/src/ui/AppHost.interactions.test.tsx +++ b/packages/hunk/src/ui/AppHost.interactions.test.tsx @@ -26,7 +26,7 @@ import { App } from "./App"; import { availableThemes, resolveTheme } from "./themes"; const { loadAppBootstrap } = await import("../core/changeset/loaders"); -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); const TEST_KEY_PAGE_UP = "\x1B[5~"; const TEST_KEY_PAGE_DOWN = "\x1B[6~"; diff --git a/packages/hunk/src/ui/AppHost.key-routing.test.tsx b/packages/hunk/src/ui/AppHost.key-routing.test.tsx index c24f65711..5b99e6b99 100644 --- a/packages/hunk/src/ui/AppHost.key-routing.test.tsx +++ b/packages/hunk/src/ui/AppHost.key-routing.test.tsx @@ -12,7 +12,7 @@ import { loadStartupExtensions } from "../extensions/startup"; mock.restore(); -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); /** * Key-ownership routing between the global handler chain and a focused diff --git a/packages/hunk/src/ui/AppHost.keybindings.test.tsx b/packages/hunk/src/ui/AppHost.keybindings.test.tsx index aec70b382..cebbe1bf2 100644 --- a/packages/hunk/src/ui/AppHost.keybindings.test.tsx +++ b/packages/hunk/src/ui/AppHost.keybindings.test.tsx @@ -11,8 +11,9 @@ import { getBundledVcsCatalog } from "../app/vcsCatalog"; import { loadAppBootstrap } from "../core/changeset/loaders"; import type { AppBootstrap } from "../core/bootstrap"; import { retireExtensionLoadResult } from "../extensions/events"; +import { createExtensionSession, type ExtensionSession } from "../extensions/session"; import { createEmptyExtensionLoadResult } from "../extensions/types"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** * User keybindings, end to end. @@ -99,6 +100,7 @@ async function withAppHost( body: (setup: Awaited>, quits: () => number) => Promise, externalQuitSignal?: AbortSignal, extensionOwnership: "owned" | "borrowed" = "owned", + extensionSession?: ExtensionSession, ) { let quitCount = 0; const setup = await testRender( @@ -107,6 +109,7 @@ async function withAppHost( externalQuitSignal={externalQuitSignal} onQuit={() => (quitCount += 1)} extensionOwnership={extensionOwnership} + {...(extensionSession ? { extensionSession } : {})} />, { width: 120, height: 24 }, ); @@ -349,6 +352,7 @@ describe("user keybindings", () => { }, }); bootstrap.extensions = extensions; + const extensionSession = createExtensionSession(extensions, repo); for (let generation = 0; generation < 2; generation += 1) { await withAppHost( @@ -360,6 +364,7 @@ describe("user keybindings", () => { }, undefined, "borrowed", + extensionSession, ); } diff --git a/packages/hunk/src/ui/AppHost.keyboard-modes.test.tsx b/packages/hunk/src/ui/AppHost.keyboard-modes.test.tsx index 8256da386..afac9ef37 100644 --- a/packages/hunk/src/ui/AppHost.keyboard-modes.test.tsx +++ b/packages/hunk/src/ui/AppHost.keyboard-modes.test.tsx @@ -8,7 +8,7 @@ import { act } from "react"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile } from "../../../../test/helpers/diff-helpers"; import { loadStartupExtensions } from "../extensions/startup"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; const tempDirs: string[] = []; setDefaultTimeout(20_000); diff --git a/packages/hunk/src/ui/AppHost.reload.test.tsx b/packages/hunk/src/ui/AppHost.reload.test.tsx index e2abf32f2..4e073867e 100644 --- a/packages/hunk/src/ui/AppHost.reload.test.tsx +++ b/packages/hunk/src/ui/AppHost.reload.test.tsx @@ -16,7 +16,7 @@ import type { const { getBundledVcsCatalog } = await import("../app/vcsCatalog"); const { loadAppBootstrap } = await import("../core/changeset/loaders"); -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); /** Stand in for the session daemon so a test can send the commands agents send. */ function createTestHostClient() { diff --git a/packages/hunk/src/ui/AppHost.responsive.test.tsx b/packages/hunk/src/ui/AppHost.responsive.test.tsx index da121099b..29311f0f2 100644 --- a/packages/hunk/src/ui/AppHost.responsive.test.tsx +++ b/packages/hunk/src/ui/AppHost.responsive.test.tsx @@ -6,7 +6,7 @@ import type { LayoutMode } from "../core/run/commandInputs"; import { createTestVcsAppBootstrap } from "../../../../test/helpers/app-bootstrap"; import { createTestDiffFile } from "../../../../test/helpers/diff-helpers"; -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); function createBootstrap(initialMode: LayoutMode = "auto", pager = false): AppBootstrap { return createTestVcsAppBootstrap({ diff --git a/packages/hunk/src/ui/AppHost.review-metadata.test.tsx b/packages/hunk/src/ui/AppHost.review-metadata.test.tsx index cf4dea5cb..29ec30dd3 100644 --- a/packages/hunk/src/ui/AppHost.review-metadata.test.tsx +++ b/packages/hunk/src/ui/AppHost.review-metadata.test.tsx @@ -15,7 +15,7 @@ import type { HunkSessionServerMessage, HunkSessionSnapshot, } from "../session/types"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; /** Stand in for the daemon so a mounted AppHost can receive unrelated reloads. */ function createTestHostClient() { diff --git a/packages/hunk/src/ui/AppHost.scroll-regression.test.tsx b/packages/hunk/src/ui/AppHost.scroll-regression.test.tsx index 2f344dc78..df062decc 100644 --- a/packages/hunk/src/ui/AppHost.scroll-regression.test.tsx +++ b/packages/hunk/src/ui/AppHost.scroll-regression.test.tsx @@ -7,7 +7,7 @@ import { createTestDiffFile } from "../../../../test/helpers/diff-helpers"; mock.restore(); -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); function createScrollBootstrap(): AppBootstrap { const before = Array.from( diff --git a/packages/hunk/src/ui/AppHost.selection.test.tsx b/packages/hunk/src/ui/AppHost.selection.test.tsx index b6552ab91..46cdec06a 100644 --- a/packages/hunk/src/ui/AppHost.selection.test.tsx +++ b/packages/hunk/src/ui/AppHost.selection.test.tsx @@ -122,7 +122,7 @@ async function renderSelectionApp( osc52 = true, }: { width?: number; height?: number; osc52?: boolean } = {}, ) { - const { AppHost } = await import("./AppHost"); + const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); const setup = await testRender(, { width, height }); const copied: string[] = []; diff --git a/packages/hunk/src/ui/AppHost.sidebar-resize.test.tsx b/packages/hunk/src/ui/AppHost.sidebar-resize.test.tsx index 8f9d70dc1..e771823fa 100644 --- a/packages/hunk/src/ui/AppHost.sidebar-resize.test.tsx +++ b/packages/hunk/src/ui/AppHost.sidebar-resize.test.tsx @@ -9,7 +9,7 @@ import { } from "../../../../test/helpers/diff-helpers"; import { createEmptyExtensionLoadResult } from "../extensions/types"; -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); /** A wide terminal so the responsive layout always shows the resizable sidebar. */ const WIDE = { width: 240, height: 24 }; diff --git a/packages/hunk/src/ui/AppHost.sidebar-visibility.test.tsx b/packages/hunk/src/ui/AppHost.sidebar-visibility.test.tsx index 091206a73..5166a3035 100644 --- a/packages/hunk/src/ui/AppHost.sidebar-visibility.test.tsx +++ b/packages/hunk/src/ui/AppHost.sidebar-visibility.test.tsx @@ -11,7 +11,7 @@ import { import { HUNK_FILES_PANE_KEY } from "../extensions/extensionIds"; import { createEmptyExtensionLoadResult } from "../extensions/types"; -const { AppHost } = await import("./AppHost"); +const { TestAppHost: AppHost } = await import("../../../../test/helpers/app-host"); /** Wide enough for the responsive layout to show the sidebar on its own. */ const WIDE = { width: 240, height: 24 }; diff --git a/packages/hunk/src/ui/AppHost.tsx b/packages/hunk/src/ui/AppHost.tsx index 959973c4b..5743634e4 100644 --- a/packages/hunk/src/ui/AppHost.tsx +++ b/packages/hunk/src/ui/AppHost.tsx @@ -17,7 +17,8 @@ import { reportExtensionApplyIssues, resolveExtensionVcsAdapters, } from "../extensions/apply"; -import { emitExtensionEvent, retireExtensionLoadResult } from "../extensions/events"; +import { emitExtensionEvent } from "../extensions/events"; +import type { ExtensionSession } from "../extensions/session"; import { extendVcsCatalog } from "../core/vcs"; import { createInitialSessionSnapshot, @@ -39,11 +40,6 @@ import type { import { assertReliableWatchRuntime } from "../core/watch/runtime"; import type { WatchedInputRuntime } from "./hooks/useWatchedInput"; -/** A replacement registry prepared for adoption and optionally already retiring. */ -interface PendingExtensionReplacement { - result: ExtensionLoadResult; -} - /** Build the stable refusal returned once quit becomes terminal for reload coordination. */ function reloadRefusedDuringShutdown() { return new Error("The review session is shutting down and cannot reload."); @@ -64,7 +60,9 @@ export function AppHost({ onActiveBootstrapChange, onFirstFrameReady, returnToHistory = false, - extensionOwnership = "owned", + extensionSession, + extensionOwnership, + onRequestSessionShutdown, reviewProducer, startupNoticeResolver, watchRuntime, @@ -82,8 +80,12 @@ export function AppHost({ onFirstFrameReady?: () => void; /** Present quit as returning to an owning history surface. */ returnToHistory?: boolean; - /** Whether this surface may retire and restart its initial extension authority. */ - extensionOwnership?: "owned" | "borrowed"; + /** Session authority shared by every routed surface in this process. */ + extensionSession: ExtensionSession; + /** Whether this surface may ask the session to adopt a replacement registry. */ + extensionOwnership: "owned" | "borrowed"; + /** Ask the process owner to retire global extension authority before review teardown. */ + onRequestSessionShutdown: () => Promise; /** * The producer whose generations this host publishes. Supplied by the process that * built the initial registration from its first publication; a host mounted without one @@ -105,6 +107,10 @@ export function AppHost({ vcsCatalog: getBundledVcsCatalog(), }, }; + const [activeExtensionSession] = useState(extensionSession); + // Direct renderer harnesses can mount extension-free bootstraps while still supplying an + // explicit empty owner. Production startup always attaches the owner's current result. + const extensionLifecycleEnabled = initialBootstrap.extensions !== undefined; const [activeBootstrap, setActiveBootstrap] = useState(initialBootstrap); const reviewIdentityRef = useRef({ input: initialBootstrap.input, @@ -120,14 +126,6 @@ export function AppHost({ }), ); const [appVersion, setAppVersion] = useState(0); - // Extensions outlive App remounts. Standalone hosts own replacement and shutdown; - // embedded reviews borrow the history workspace's initial authority. - const extensionsRef = useRef(initialBootstrap.extensions as ExtensionLoadResult | undefined); - const borrowedExtensionRegistryRef = useRef( - extensionOwnership === "borrowed" - ? (initialBootstrap.extensions as ExtensionLoadResult | undefined)?.registry - : undefined, - ); // Experimental capabilities are launch authority: remote/watch reloads may replace content, // but opting in or out requires starting a new Hunk process. const launchExperimental = initialBootstrap.input.options.experimental === true; @@ -145,13 +143,6 @@ export function AppHost({ const [sessionFileBounds] = useState(() => createSessionReloadBounds(initialBootstrap, { cwd: initialBootstrap.reloadContext.cwd }), ); - // Which working directory the current extension set was discovered for. - // Discovery is cwd-relative, so a reload that moves the session to another - // repository has to re-run it: that repo's extensions — and the trust - // question they raise — belong to it, not to the one Hunk launched in. Seeded - // from the bounds' cwd so it compares against the same resolved form reloads - // produce, and a same-directory reload is not mistaken for a move. - const extensionsCwdRef = useRef(sessionFileBounds.defaultCwd); const initialExtensionStartupPendingRef = useRef(true); const reloadTailRef = useRef>(Promise.resolve()); const pendingExtensionReloadRef = useRef<{ @@ -159,8 +150,6 @@ export function AppHost({ promise: Promise; } | null>(null); const quitRequestedRef = useRef(false); - const pendingExtensionReplacementRef = useRef(undefined); - const pendingExtensionRetirementsRef = useRef>>(new Set()); const pendingWorkspaceWritesRef = useRef>>(new Set()); const workspaceRefreshRequestRef = useRef(undefined); const pendingReloadLifecycleRef = useRef<{ @@ -186,14 +175,16 @@ export function AppHost({ // leases are live here; passive UI events still wait until this order lands. if (initialExtensionStartupPendingRef.current) { initialExtensionStartupPendingRef.current = false; - if (extensionOwnership === "owned") { - emitExtensionEvent(extensionsRef.current, "startup", { - cwd: initialBootstrap.reloadContext.cwd, - }); + if (extensionLifecycleEnabled && extensionOwnership === "owned") { + activeExtensionSession.startCurrent(initialBootstrap.reloadContext.cwd); } - emitExtensionEvent(extensionsRef.current, "changeset_loaded", { - changeset: initialBootstrap.changeset, - }); + emitExtensionEvent( + extensionLifecycleEnabled ? activeExtensionSession.current : undefined, + "changeset_loaded", + { + changeset: initialBootstrap.changeset, + }, + ); return; } @@ -203,7 +194,7 @@ export function AppHost({ } pendingReloadLifecycleRef.current = null; if (pending.emitStartup) { - emitExtensionEvent(pending.extensions, "startup", { cwd: pending.cwd }); + activeExtensionSession.startCurrent(pending.cwd); } emitExtensionEvent(pending.extensions, "changeset_loaded", { changeset: pending.changeset, @@ -213,57 +204,13 @@ export function AppHost({ reason: pending.reason, }); pending.resolveMounted(); - }, [activeBootstrap, extensionOwnership, initialBootstrap.reloadContext.cwd]); - - /** Track one prepared registry until it is either adopted or fully retired. */ - const trackPreparedExtensionReplacement = useCallback((result: ExtensionLoadResult) => { - pendingExtensionReplacementRef.current = { result }; - }, []); - - /** Track every registry retirement until its shared shutdown completion settles. */ - const retireOwnedExtensionLoadResult = useCallback((result: ExtensionLoadResult | undefined) => { - if (result?.registry === borrowedExtensionRegistryRef.current) return Promise.resolve(); - const retirement = retireExtensionLoadResult(result); - pendingExtensionRetirementsRef.current.add(retirement); - void retirement.then( - () => pendingExtensionRetirementsRef.current.delete(retirement), - () => pendingExtensionRetirementsRef.current.delete(retirement), - ); - return retirement; - }, []); - - /** Retire one prepared registry through a shared promise so quit and reload cleanup agree. */ - const retirePreparedExtensionReplacement = useCallback( - (result: ExtensionLoadResult | undefined): Promise => { - if (!result) return Promise.resolve(); - const pending = pendingExtensionReplacementRef.current; - if (!pending || pending.result !== result) return retireOwnedExtensionLoadResult(result); - return retireOwnedExtensionLoadResult(result).finally(() => { - if (pendingExtensionReplacementRef.current === pending) { - pendingExtensionReplacementRef.current = undefined; - } - }); - }, - [retireOwnedExtensionLoadResult], - ); - - /** Own provisional loader authority immediately, including work exposed after quit. */ - const ownProvisionalExtensionReplacement = useCallback( - (result: ExtensionLoadResult) => { - trackPreparedExtensionReplacement(result); - if (quitRequestedRef.current) { - void retirePreparedExtensionReplacement(result); - } - }, - [retirePreparedExtensionReplacement, trackPreparedExtensionReplacement], - ); - - /** Clear prepared ownership only when this exact registry becomes the mounted authority. */ - const adoptPreparedExtensionReplacement = useCallback((result: ExtensionLoadResult) => { - if (pendingExtensionReplacementRef.current?.result === result) { - pendingExtensionReplacementRef.current = undefined; - } - }, []); + }, [ + activeBootstrap, + extensionLifecycleEnabled, + extensionOwnership, + activeExtensionSession, + initialBootstrap.reloadContext.cwd, + ]); /** Start one irreversible write atomically with host tracking, unless quit already won. */ const runWorkspaceWrite = useCallback(async (write) => { @@ -300,7 +247,9 @@ export function AppHost({ sourcePath: options?.sourcePath, }); const baseVcsCatalog = getBundledVcsCatalog(); - const currentExtensions = extensionsRef.current; + const currentExtensions = extensionLifecycleEnabled + ? activeExtensionSession.current + : undefined; const currentAdapters = currentExtensions ? resolveExtensionVcsAdapters(currentExtensions.registry, baseVcsCatalog).adapters : []; @@ -316,7 +265,7 @@ export function AppHost({ if ( extensionOwnership === "owned" && - (options?.reloadExtensions || cwd !== extensionsCwdRef.current) + (options?.reloadExtensions || cwd !== activeExtensionSession.cwd) ) { try { const resolvedExtensions = await resolveConfiguredExtensions({ @@ -327,23 +276,23 @@ export function AppHost({ discoveryCatalog, // Reuse the session hub so the mounted toast surface keeps receiving notifications. notifications: currentExtensions?.notifications, - onProvisionalLoad: ownProvisionalExtensionReplacement, + onProvisionalLoad: (result) => activeExtensionSession.trackPrepared(result), assertActive: () => { if (quitRequestedRef.current) throw reloadRefusedDuringShutdown(); }, }); configured = resolvedExtensions.configured; replacementExtensions = resolvedExtensions.extensions; - trackPreparedExtensionReplacement(replacementExtensions); + activeExtensionSession.trackPrepared(replacementExtensions); } catch (error) { // The resolver may fail after publishing a provisional registry but // before returning it. Clear host ownership through the same shared // retirement used by quit and the ordinary reload failure paths. - await retirePreparedExtensionReplacement(pendingExtensionReplacementRef.current?.result); + await activeExtensionSession.retirePrepared(); throw error; } if (quitRequestedRef.current) { - await retirePreparedExtensionReplacement(replacementExtensions); + await activeExtensionSession.retirePrepared(replacementExtensions); throw reloadRefusedDuringShutdown(); } } @@ -359,7 +308,7 @@ export function AppHost({ baseVcsCatalog, }); } catch (error) { - await retirePreparedExtensionReplacement(replacementExtensions); + await activeExtensionSession.retirePrepared(replacementExtensions); throw error; } @@ -368,7 +317,7 @@ export function AppHost({ // Quit therefore linearizes either wholly before or wholly after adoption. if (quitRequestedRef.current) { restoreFileLanguageRegistrations(loaded.previousFileLanguages); - await retirePreparedExtensionReplacement(replacementExtensions); + await activeExtensionSession.retirePrepared(replacementExtensions); throw reloadRefusedDuringShutdown(); } @@ -424,20 +373,15 @@ export function AppHost({ } } catch (error) { restoreFileLanguageRegistrations(loaded.previousFileLanguages); - await retirePreparedExtensionReplacement(replacementExtensions); + await activeExtensionSession.retirePrepared(replacementExtensions); throw error; } let currentExtensionsRetired: Promise | undefined; if (replacementExtensions) { - // Only retire the visible runtime after its replacement review is known-good. - // Revocation is synchronous so mounted controls and modes become inert - // before shutdown starts; React then tears them down on this state update. - // `retireExtensionLoadResult` revokes synchronously before its first await. - currentExtensionsRetired = retireOwnedExtensionLoadResult(currentExtensions); - extensionsRef.current = replacementExtensions; - extensionsCwdRef.current = cwd; - adoptPreparedExtensionReplacement(replacementExtensions); + // Adopt only after the review and broker publication are known-good. Adoption revokes + // the previous registry synchronously before exposing the replacement. + currentExtensionsRetired = activeExtensionSession.adoptPrepared(replacementExtensions, cwd); } const reloadMounted = extensions ? new Promise((resolveMounted) => { @@ -479,19 +423,16 @@ export function AppHost({ }; }, [ - adoptPreparedExtensionReplacement, + extensionLifecycleEnabled, extensionOwnership, + activeExtensionSession, hostClient, launchExperimental, launchFast, launchExtensionsEnabled, launchExtensionPaths, - ownProvisionalExtensionReplacement, producer, - retireOwnedExtensionLoadResult, - retirePreparedExtensionReplacement, sessionFileBounds, - trackPreparedExtensionReplacement, ], ); @@ -596,23 +537,12 @@ export function AppHost({ if (quitRequestedRef.current) return; quitRequestedRef.current = true; queueMicrotask(() => { - const preparedReplacement = pendingExtensionReplacementRef.current?.result; const startedWrites = [...pendingWorkspaceWritesRef.current]; - void retireOwnedExtensionLoadResult(extensionsRef.current); - void retirePreparedExtensionReplacement(preparedReplacement); - - /** Drain known retirement work; cancelled loaders cannot create a later staged registry. */ - const settleExtensionRetirements = async () => { - while (pendingExtensionRetirementsRef.current.size > 0) { - await Promise.allSettled(pendingExtensionRetirementsRef.current); - } - }; - - void Promise.all([settleExtensionRetirements(), Promise.allSettled(startedWrites)]).finally( + void Promise.all([onRequestSessionShutdown(), Promise.allSettled(startedWrites)]).finally( onQuit, ); }); - }, [onQuit, retireOwnedExtensionLoadResult, retirePreparedExtensionReplacement]); + }, [onQuit, onRequestSessionShutdown]); useEffect(() => { if (!externalQuitSignal) return; diff --git a/packages/hunk/src/ui/AppHost.watch.test.tsx b/packages/hunk/src/ui/AppHost.watch.test.tsx index 7f6d5ee1d..e1e2c6330 100644 --- a/packages/hunk/src/ui/AppHost.watch.test.tsx +++ b/packages/hunk/src/ui/AppHost.watch.test.tsx @@ -6,7 +6,7 @@ import { act } from "react"; import { capturedTestColorToHex } from "../../../../test/helpers/test-color-helpers"; import { createWatchTestRuntime } from "../../../../test/helpers/watchTest"; import { loadAppBootstrap } from "../core/changeset/loaders"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; import { resolveTheme } from "./themes"; async function flush(setup: Awaited>) { diff --git a/packages/hunk/src/ui/AppHost.workspace.test.tsx b/packages/hunk/src/ui/AppHost.workspace.test.tsx index 361660821..382768003 100644 --- a/packages/hunk/src/ui/AppHost.workspace.test.tsx +++ b/packages/hunk/src/ui/AppHost.workspace.test.tsx @@ -20,7 +20,7 @@ import { getBundledVcsCatalog } from "../app/vcsCatalog"; import type { CliInput } from "../core/run/commandInputs"; import { loadStartupExtensions } from "../extensions/startup"; import type { HunkSessionBrokerClient } from "../session/broker/brokerClient"; -import { AppHost } from "./AppHost"; +import { TestAppHost as AppHost } from "../../../../test/helpers/app-host"; import type { WorkspaceFileWriter } from "./hooks/useExtensionWorkspaceControls"; /** Specialize the core loader result with extension state assigned by these tests. */ diff --git a/packages/hunk/src/ui/components/scrollbar/VerticalScrollbar.test.tsx b/packages/hunk/src/ui/components/scrollbar/VerticalScrollbar.test.tsx index 46afaa9f6..386266341 100644 --- a/packages/hunk/src/ui/components/scrollbar/VerticalScrollbar.test.tsx +++ b/packages/hunk/src/ui/components/scrollbar/VerticalScrollbar.test.tsx @@ -12,7 +12,7 @@ import { type VerticalScrollbarScheduler, } from "./VerticalScrollbar"; -const { AppHost } = await import("../../AppHost"); +const { TestAppHost: AppHost } = await import("../../../../../../test/helpers/app-host"); function createDiffFile(id: string, path: string, before: string, after: string): DiffFile { const metadata = parseDiffFromFile( diff --git a/packages/hunk/src/ui/components/ui-components.test.tsx b/packages/hunk/src/ui/components/ui-components.test.tsx index cc7879f7b..f13bea031 100644 --- a/packages/hunk/src/ui/components/ui-components.test.tsx +++ b/packages/hunk/src/ui/components/ui-components.test.tsx @@ -24,7 +24,7 @@ import { resolveCommandKeys } from "../lib/keymap"; import type { CurrentLineAlignment, LineRevealPlacement } from "../lib/hunkScroll"; import type { LineCursor } from "../lib/lineCursors"; -const { AppHost } = await import("../AppHost"); +const { TestAppHost: AppHost } = await import("../../../../../test/helpers/app-host"); const { toReadOnlyFileViews } = await import("../../extensions/events"); const { FlexFileSidebar } = await import("../../extensions/default/ui/sidebar"); const { HelpDialog } = await import("./chrome/HelpDialog"); diff --git a/packages/hunk/src/ui/history/runStaticHistory.test.ts b/packages/hunk/src/ui/history/runStaticHistory.test.ts index 1ac5a2071..8d9379b82 100644 --- a/packages/hunk/src/ui/history/runStaticHistory.test.ts +++ b/packages/hunk/src/ui/history/runStaticHistory.test.ts @@ -1,14 +1,20 @@ import { describe, expect, test } from "bun:test"; import { EventEmitter } from "node:events"; import type { HistoryCommit } from "../../core/history/types"; +import { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; import type { HistoryRuntime } from "./types"; import { runStaticHistory } from "./runStaticHistory"; /** Create a page-backed runtime and expose whether cleanup ran. */ -function runtime(commits: HistoryCommit[], maxCount?: number) { +function runtime(commits: HistoryCommit[], maxCount?: number, closeFailure?: Error) { let offset = 0; let closed = 0; let reads = 0; + const cleanup: string[] = []; + const extensionSession = createTestExtensionSession(); + extensionSession.shutdown = async () => { + cleanup.push("extensions"); + }; const value: HistoryRuntime = { input: { kind: "history", @@ -20,6 +26,7 @@ function runtime(commits: HistoryCommit[], maxCount?: number) { extensionPaths: [], ...(maxCount !== undefined ? { maxCount } : {}), }, + extensionSession, providerId: "test", providerName: "Test", repoRoot: "/repo", @@ -42,9 +49,11 @@ function runtime(commits: HistoryCommit[], maxCount?: number) { }, async close() { closed += 1; + cleanup.push("cursor"); + if (closeFailure) throw closeFailure; }, }; - return { value, closed: () => closed, reads: () => reads }; + return { value, closed: () => closed, reads: () => reads, cleanup }; } const commits: HistoryCommit[] = ["a", "b"].map((id, index) => ({ @@ -79,6 +88,36 @@ describe("static history runner", () => { expect(output).toContain("Commit b"); expect(paged).toBe(""); expect(history.closed()).toBe(1); + expect(history.cleanup).toEqual(["cursor", "extensions"]); + }); + + test("closes the cursor before extensions when pager startup fails", async () => { + const history = runtime(commits); + await expect( + runStaticHistory(history.value, { + stdout: { isTTY: true, columns: 80, rows: 2, write: () => true }, + stderr: { write: () => true }, + env: { TERM: "xterm" }, + pageText: async () => { + throw new Error("pager failed"); + }, + }), + ).rejects.toThrow("pager failed"); + expect(history.cleanup).toEqual(["cursor", "extensions"]); + }); + + test("still shuts down extensions when cursor close rejects", async () => { + const failure = new Error("cursor close failed"); + const history = runtime(commits, undefined, failure); + await expect( + runStaticHistory(history.value, { + stdout: { isTTY: false, columns: 80, rows: 24, write: () => true }, + stderr: { write: () => true }, + env: {}, + pageText: async () => {}, + }), + ).rejects.toBe(failure); + expect(history.cleanup).toEqual(["cursor", "extensions"]); }); test("treats a downstream EPIPE as normal and still closes the source", async () => { diff --git a/packages/hunk/src/ui/history/runStaticHistory.ts b/packages/hunk/src/ui/history/runStaticHistory.ts index 3527d4c09..cdd8b56e0 100644 --- a/packages/hunk/src/ui/history/runStaticHistory.ts +++ b/packages/hunk/src/ui/history/runStaticHistory.ts @@ -133,7 +133,11 @@ export async function runStaticHistory( await pager?.close(); } finally { deps.stdout.off?.("error", onOutputError); - await bootstrap.close(); + try { + await bootstrap.close(); + } finally { + await bootstrap.extensionSession.shutdown(); + } } } } diff --git a/packages/hunk/src/ui/history/types.ts b/packages/hunk/src/ui/history/types.ts index 045b9916d..169cf8c94 100644 --- a/packages/hunk/src/ui/history/types.ts +++ b/packages/hunk/src/ui/history/types.ts @@ -1,6 +1,6 @@ import type { HistoryCommandInput } from "../../core/run/commandInputs"; import type { VcsHistorySource } from "../../core/vcs/types"; -import type { ExtensionLoadResult } from "../../extensions/types"; +import type { ExtensionSession } from "../../extensions/session"; import type { ExtensionVcsHistoryCommit, ExtensionVcsHistoryReviewAction, @@ -8,7 +8,7 @@ import type { NamedCustomThemeConfig, } from "../../extension-api/types"; -/** Renderer-facing history resources, excluding app and extension ownership details. */ +/** Renderer-facing history resources with cursor data and command-owned extension authority. */ export interface HistoryRuntime { input: HistoryCommandInput; source: VcsHistorySource; @@ -19,8 +19,8 @@ export interface HistoryRuntime { repoRoot: string; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; - /** History-owned extension authority borrowed by embedded reviews. */ - extensionSession?: ExtensionLoadResult; + /** Command-owned extension authority borrowed by embedded reviews. */ + extensionSession: ExtensionSession; planReview( commit: ExtensionVcsHistoryCommit, options?: ExtensionVcsHistoryReviewOptions, diff --git a/packages/hunk/src/ui/log/controller.test.ts b/packages/hunk/src/ui/log/controller.test.ts index 9125f979a..bdcbd09f2 100644 --- a/packages/hunk/src/ui/log/controller.test.ts +++ b/packages/hunk/src/ui/log/controller.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; import type { HistoryRuntime } from "../history/types"; import { LogController } from "./controller"; @@ -36,6 +37,7 @@ function createRuntime(subjects = ["first", "second", "third"]) { extensionPaths: [], }, source, + extensionSession: createTestExtensionSession(), providerId: "test", providerName: "Test", repoRoot: "/repo", diff --git a/packages/hunk/src/ui/log/runInteractiveLog.test.ts b/packages/hunk/src/ui/log/runInteractiveLog.test.ts index 10159d1fe..088c5a698 100644 --- a/packages/hunk/src/ui/log/runInteractiveLog.test.ts +++ b/packages/hunk/src/ui/log/runInteractiveLog.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, test } from "bun:test"; -import { logSignalExitCode } from "./runInteractiveLog"; +import { describe, expect, mock, test } from "bun:test"; +import type { HistoryRuntime } from "../history/types"; +import { logSignalExitCode, runInteractiveLog } from "./runInteractiveLog"; describe("interactive log lifecycle", () => { test("preserves conventional signal exit codes after cleanup", () => { @@ -7,4 +8,50 @@ describe("interactive log lifecycle", () => { expect(logSignalExitCode("SIGHUP")).toBe(129); expect(logSignalExitCode("SIGTERM")).toBe(143); }); + + test("closes cursor then extension authority when interactive input has no terminal", async () => { + const events: string[] = []; + const runtime = { + close: mock(async () => { + events.push("cursor"); + }), + extensionSession: { + shutdown: mock(async () => { + events.push("extensions"); + }), + }, + } as unknown as HistoryRuntime; + + await expect( + runInteractiveLog(runtime, { + stdin: { isTTY: false } as never, + stdout: { isTTY: true } as never, + }), + ).rejects.toThrow("requires a terminal"); + expect(events).toEqual(["cursor", "extensions"]); + }); + + test("still shuts down extensions when non-terminal cursor cleanup rejects", async () => { + const events: string[] = []; + const failure = new Error("cursor close failed"); + const runtime = { + close: mock(async () => { + events.push("cursor"); + throw failure; + }), + extensionSession: { + shutdown: mock(async () => { + events.push("extensions"); + }), + }, + } as unknown as HistoryRuntime; + + await expect( + runInteractiveLog(runtime, { + stdin: { isTTY: false } as never, + stdout: { isTTY: true } as never, + }), + ).rejects.toBe(failure); + expect(events).toEqual(["cursor", "extensions"]); + }); }); diff --git a/packages/hunk/src/ui/log/runInteractiveLog.tsx b/packages/hunk/src/ui/log/runInteractiveLog.tsx index 730a71c32..6af189a6c 100644 --- a/packages/hunk/src/ui/log/runInteractiveLog.tsx +++ b/packages/hunk/src/ui/log/runInteractiveLog.tsx @@ -23,7 +23,11 @@ export async function runInteractiveLog( }: { stdin?: NodeJS.ReadStream; stdout?: NodeJS.WriteStream } = {}, ) { if (!stdin.isTTY || !stdout.isTTY || typeof stdin.setRawMode !== "function") { - await runtime.close(); + try { + await runtime.close(); + } finally { + await runtime.extensionSession.shutdown(); + } throw new HunkUserError("The `hunk log` browser requires a terminal.", [ "Use `hunk log --static` to force scrollback output.", ]); @@ -43,7 +47,13 @@ export async function runInteractiveLog( signals: LOG_SHUTDOWN_SIGNALS, signalExitCode: logSignalExitCode, interruptExitCode: 130, - beforeTeardown: () => controller.close(), + beforeTeardown: async () => { + try { + await controller.close(); + } finally { + await runtime.extensionSession.shutdown(); + } + }, render: ({ externalQuitSignal, finish }) => ( , + handler: () => void | Promise, +) { + bootstrap.extensions ??= createEmptyExtensionLoadResult(bootstrap.reloadContext.cwd); + (bootstrap.extensions as ExtensionLoadResult).registry.eventHandlers.shutdown.push({ + extensionId: "test", + handler, + }); +} + 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 shutdown = mock(async () => undefined); + onShutdown(bootstrap, shutdown); const runSession = mock(async () => undefined); const failure = new Error("registration failed"); @@ -26,14 +40,12 @@ test("retires extensions and closes the controlling terminal when review runtime 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(shutdown).toHaveBeenCalledTimes(1); expect(close).toHaveBeenCalledTimes(1); }); @@ -45,11 +57,12 @@ test("stops the broker and retires extensions before exceptional renderer teardo const events: string[] = []; const failure = new Error("render failed"); const stop = mock(() => events.push("stop")); - const retireExtensions = mock(async () => { + const shutdown = mock(async () => { events.push("retire-start"); await Promise.resolve(); events.push("retire-finish"); }); + onShutdown(bootstrap, shutdown); const runSession = mock( async (options: Parameters[0]) => { await options.onFailure?.(failure); @@ -68,14 +81,13 @@ test("stops the broker and retires extensions before exceptional renderer teardo 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); + expect(shutdown).toHaveBeenCalledTimes(1); }); test("retries broker cleanup before teardown when the exceptional stop attempt fails", async () => { @@ -91,7 +103,7 @@ test("retries broker cleanup before teardown when the exceptional stop attempt f events.push(`stop-${stopAttempts}`); if (stopAttempts === 1) throw new Error("socket close failed"); }); - const retireExtensions = mock(async () => { + onShutdown(bootstrap, async () => { events.push("retire"); }); const runSession = mock( @@ -115,7 +127,6 @@ test("retries broker cleanup before teardown when the exceptional stop attempt f stop, })) as never, runSession: runSession as never, - retireExtensions: retireExtensions as never, }, ), ).rejects.toBe(failure); diff --git a/packages/hunk/src/ui/runInteractiveApp.tsx b/packages/hunk/src/ui/runInteractiveApp.tsx index 2f44b331c..8dde06571 100644 --- a/packages/hunk/src/ui/runInteractiveApp.tsx +++ b/packages/hunk/src/ui/runInteractiveApp.tsx @@ -2,7 +2,8 @@ import { shouldUseMouseForApp, type ControllingTerminal } from "../core/process/ import type { AppBootstrap } from "../core/bootstrap"; import { resolveStartupUpdateNotice } from "../core/process/updateNotice"; import { createReviewSessionRuntime } from "../app/session/reviewRuntime"; -import { retireExtensionLoadResult } from "../extensions/events"; +import { createSessionReloadBounds } from "../app/session/reloadBounds"; +import { createExtensionSession } from "../extensions/session"; import type { ExtensionLoadResult } from "../extensions/types"; import { HunkSessionHost, type StandaloneReviewSurfaceRoute } from "./session/HunkSessionHost"; import { runHunkSession } from "./session/runHunkSession"; @@ -15,7 +16,6 @@ export interface InteractiveAppInput { export interface InteractiveAppDeps { createReviewRuntime?: typeof createReviewSessionRuntime; runSession?: typeof runHunkSession; - retireExtensions?: typeof retireExtensionLoadResult; } // Leave fatal process faults to their default OS disposition. @@ -31,7 +31,6 @@ export async function runInteractiveApp( ): Promise { const createReviewRuntime = deps.createReviewRuntime ?? createReviewSessionRuntime; const runSession = deps.runSession ?? runHunkSession; - const retireExtensions = deps.retireExtensions ?? retireExtensionLoadResult; const rendererStdin = controllingTerminal?.stdin ?? process.stdin; let terminalClosed = false; const closeTerminal = () => { @@ -39,6 +38,14 @@ export async function runInteractiveApp( terminalClosed = true; controllingTerminal?.close(); }; + if (!bootstrap.extensions) { + controllingTerminal?.close(); + throw new Error("Interactive review startup did not provide extension authority."); + } + const extensionSession = createExtensionSession( + bootstrap.extensions, + createSessionReloadBounds(bootstrap, { cwd: bootstrap.reloadContext.cwd }).defaultCwd, + ); let reviewRuntime: ReturnType | undefined; let runnerOwnsFailureCleanup = false; let runtimeCleanupAttempted = false; @@ -50,6 +57,7 @@ export async function runInteractiveApp( instanceId: 1, bootstrap, runtime: reviewRuntime, + extensionSession, }; runnerOwnsFailureCleanup = true; await runSession({ @@ -65,10 +73,11 @@ export async function runInteractiveApp( reviewRuntime?.stop(); runtimeCleanupAttempted = true; } finally { - await retireExtensions(bootstrap.extensions); + await extensionSession.shutdown(); } }, - beforeTeardown: () => { + beforeTeardown: async () => { + await extensionSession.shutdown(); if (runtimeCleanupAttempted) return; reviewRuntime?.stop(); runtimeCleanupAttempted = true; @@ -83,7 +92,7 @@ export async function runInteractiveApp( ), }); } catch (error) { - if (!runnerOwnsFailureCleanup) await retireExtensions(bootstrap.extensions); + if (!runnerOwnsFailureCleanup) await extensionSession.shutdown(); throw error; } finally { if (!runtimeCleanupAttempted) reviewRuntime?.stop(); diff --git a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx index 369d2b83e..031b66656 100644 --- a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx +++ b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx @@ -3,6 +3,8 @@ 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 { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; +import { createEmptyExtensionLoadResult } from "../../extensions/types"; import type { HistoryRuntime } from "../history/types"; import { LogController } from "../log/controller"; import { @@ -25,6 +27,7 @@ async function createHistoryRoute() { extensionsEnabled: false, extensionPaths: [], }, + extensionSession: createTestExtensionSession(), source: { async read() { return { @@ -84,13 +87,14 @@ test("routes repeated history reviews through fresh runtimes and returns instead const stops: Array> = []; let instance = 0; const deps: HunkSessionHostDeps = { - prepareReview: (async () => ({ - bootstrap: createTestVcsAppBootstrap({ + prepareReview: (async () => { + const bootstrap = createTestVcsAppBootstrap({ changesetId: `review-${++instance}`, files: [createTestDiffFile({ id: "review.ts", path: "review.ts" })], - }), - borrowsExtensions: true, - })) as never, + }); + bootstrap.extensions = history.runtime.extensionSession.current; + return { bootstrap, borrowsExtensions: true }; + }) as never, createReviewRuntime: (() => { const stop = mock(() => undefined); stops.push(stop); @@ -137,20 +141,24 @@ test("quits the session from a standalone review route", async () => { const quit = mock(() => undefined); const stop = mock(() => undefined); const abort = new AbortController(); + const extensionSession = createTestExtensionSession(); + const bootstrap = createTestVcsAppBootstrap({ + changesetId: "standalone-review", + files: [createTestDiffFile({ id: "standalone.ts", path: "standalone.ts" })], + }); + bootstrap.extensions = extensionSession.current; const setup = await testRender( throw new Error("broker close failed"); }); const abort = new AbortController(); + const extensionSession = createTestExtensionSession(); + const bootstrap = createTestVcsAppBootstrap({ + changesetId: "failing-broker-review", + files: [createTestDiffFile({ id: "failing.ts", path: "failing.ts" })], + }); + bootstrap.extensions = extensionSession.current; const setup = await testRender( { + const history = await createHistoryRoute(); + const foreign = createEmptyExtensionLoadResult("/repo"); + const createReviewRuntime = mock(() => { + throw new Error("nested runtime mounted"); + }); + const setup = await testRender( + undefined} + deps={{ + prepareReview: (async () => ({ + bootstrap: { + ...createTestVcsAppBootstrap({ + changesetId: "foreign-review", + files: [createTestDiffFile({ id: "foreign.ts", path: "foreign.ts" })], + }), + extensions: foreign, + }, + borrowsExtensions: false, + })) as never, + createReviewRuntime: createReviewRuntime 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("cannot replace"); + expect(createReviewRuntime).not.toHaveBeenCalled(); + expect(foreign.registry.eventBusPhase).toBe("closed"); + expect(history.runtime.extensionSession.closing).toBe(false); + } 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); @@ -333,17 +387,16 @@ test("defers menu quit until cancelled preparation and retirement settle", async const preparation = new Promise((resolve) => { resolvePreparation = resolve; }); + history.runtime.extensionSession.trackPrepared = mock(() => undefined); + history.runtime.extensionSession.retirePrepared = mock(async () => { + events.push("retire"); + }); const setup = await testRender( preparation) as never, - retirePreparedExtensions: (async () => { - events.push("retire"); - }) as never, - }} + deps={{ prepareReview: (() => preparation) as never }} />, { width: 100, height: 20 }, ); @@ -355,10 +408,13 @@ test("defers menu quit until cancelled preparation and retirement settle", async expect(quit).not.toHaveBeenCalled(); resolvePreparation({ - bootstrap: createTestVcsAppBootstrap({ - changesetId: "cancelled-review", - files: [createTestDiffFile({ id: "cancelled.ts", path: "cancelled.ts" })], - }), + bootstrap: { + ...createTestVcsAppBootstrap({ + changesetId: "cancelled-review", + files: [createTestDiffFile({ id: "cancelled.ts", path: "cancelled.ts" })], + }), + extensions: createEmptyExtensionLoadResult("/repo"), + }, borrowsExtensions: false, }); await settle(setup); @@ -387,6 +443,8 @@ test("cancels stale preparation, retires its owned registry, and quits once", as resolvePreparation = resolve; }); const abort = new AbortController(); + history.runtime.extensionSession.trackPrepared = mock(() => undefined); + history.runtime.extensionSession.retirePrepared = retire as never; const setup = await testRender( { throw new Error("stale review mounted"); }) as never, - retirePreparedExtensions: retire as never, }} />, { width: 100, height: 20 }, @@ -409,10 +466,13 @@ test("cancels stale preparation, retires its owned registry, and quits once", as act(() => abort.abort()); expect(quit).not.toHaveBeenCalled(); resolvePreparation({ - bootstrap: createTestVcsAppBootstrap({ - changesetId: "stale-review", - files: [createTestDiffFile({ id: "stale.ts", path: "stale.ts" })], - }), + bootstrap: { + ...createTestVcsAppBootstrap({ + changesetId: "stale-review", + files: [createTestDiffFile({ id: "stale.ts", path: "stale.ts" })], + }), + extensions: createEmptyExtensionLoadResult("/repo"), + }, borrowsExtensions: false, }); await act(async () => { diff --git a/packages/hunk/src/ui/session/HunkSessionHost.tsx b/packages/hunk/src/ui/session/HunkSessionHost.tsx index e3a4135c8..1814885ec 100644 --- a/packages/hunk/src/ui/session/HunkSessionHost.tsx +++ b/packages/hunk/src/ui/session/HunkSessionHost.tsx @@ -10,7 +10,7 @@ import { } from "../../app/session/reviewRuntime"; import type { StartupNotice } from "../../core/process/startupNotice"; import type { AppBootstrap } from "../../core/bootstrap"; -import { retireExtensionLoadResult } from "../../extensions/events"; +import type { ExtensionSession } from "../../extensions/session"; import type { ExtensionLoadResult } from "../../extensions/types"; import { AppHost } from "../AppHost"; import type { HistoryRuntime } from "../history/types"; @@ -29,6 +29,7 @@ export interface StandaloneReviewSurfaceRoute { instanceId: number; bootstrap: AppBootstrap; runtime: ReviewSessionRuntime; + extensionSession: ExtensionSession; } export type HunkSurfaceRoute = HistorySurfaceRoute | StandaloneReviewSurfaceRoute; @@ -45,7 +46,6 @@ type ActiveSurfaceRoute = HistorySurfaceRoute | ActiveReviewSurfaceRoute; export interface HunkSessionHostDeps { prepareReview?: typeof prepareEmbeddedHistoryReview; createReviewRuntime?: typeof createReviewSessionRuntime; - retirePreparedExtensions?: typeof retireExtensionLoadResult; } /** @@ -70,7 +70,6 @@ export function HunkSessionHost({ }) { const prepareReview = deps.prepareReview ?? prepareEmbeddedHistoryReview; const createReviewRuntime = deps.createReviewRuntime ?? createReviewSessionRuntime; - const retirePreparedExtensions = deps.retirePreparedExtensions ?? retireExtensionLoadResult; const [route, setRoute] = useState(() => initialRoute.kind === "history" ? initialRoute @@ -177,11 +176,14 @@ export function HunkSessionHost({ startupCwd, extensionsEnabled: historyRoute.runtime.input.extensionsEnabled, extensionPaths: historyRoute.runtime.input.extensionPaths, - extensionSession: historyRoute.runtime.extensionSession, + extensionSession: historyRoute.runtime.extensionSession.current, themeId: outcome.themeId, themeMode: outcome.themeMode, }; plan = await prepareReview(request, { signal }); + if (!plan.bootstrap.extensions) { + throw new Error("Embedded review startup did not provide extension authority."); + } signal.throwIfAborted(); if ( !mountedRef.current || @@ -189,17 +191,24 @@ export function HunkSessionHost({ routeRef.current !== historyRoute ) { if (!plan.borrowsExtensions) { - await retirePreparedExtensions(plan.bootstrap.extensions); + historyRoute.runtime.extensionSession.trackPrepared(plan.bootstrap.extensions); + await historyRoute.runtime.extensionSession.retirePrepared(plan.bootstrap.extensions); } return; } + if (!plan.borrowsExtensions) { + historyRoute.runtime.extensionSession.trackPrepared(plan.bootstrap.extensions); + await historyRoute.runtime.extensionSession.retirePrepared(plan.bootstrap.extensions); + throw new Error("An embedded review cannot replace the owning extension session."); + } const reviewRuntime = createReviewRuntime(plan.bootstrap, startupCwd); const reviewRoute: ActiveReviewSurfaceRoute = { kind: "review", instanceId: nextInstanceRef.current++, bootstrap: plan.bootstrap, runtime: reviewRuntime, - extensionOwnership: plan.borrowsExtensions ? "borrowed" : "owned", + extensionSession: historyRoute.runtime.extensionSession, + extensionOwnership: "borrowed", quitBehavior: "return-to-history", mountMode: "dynamic", returnRoute: historyRoute, @@ -207,8 +216,15 @@ export function HunkSessionHost({ routeRef.current = reviewRoute; setRoute(reviewRoute); } catch (error) { - if (plan && !plan.borrowsExtensions && routeRef.current.kind !== "review") { - await retirePreparedExtensions(plan.bootstrap.extensions); + const preparedExtensions = plan?.bootstrap.extensions; + if ( + plan && + preparedExtensions && + !plan.borrowsExtensions && + routeRef.current.kind !== "review" + ) { + historyRoute.runtime.extensionSession.trackPrepared(preparedExtensions); + await historyRoute.runtime.extensionSession.retirePrepared(preparedExtensions); } if (!signal.aborted) throw error; } finally { @@ -256,7 +272,13 @@ export function HunkSessionHost({ onQuit={retireReview} {...(route.mountMode === "dynamic" ? { onFirstFrameReady: () => undefined } : {})} returnToHistory={route.quitBehavior === "return-to-history"} + extensionSession={route.extensionSession} extensionOwnership={route.extensionOwnership} + onRequestSessionShutdown={ + route.extensionOwnership === "owned" + ? () => route.extensionSession.shutdown() + : async () => undefined + } reviewProducer={route.runtime.reviewProducer} startupNoticeResolver={startupNoticeResolver} /> diff --git a/scripts/test-large-untracked-render.tsx b/scripts/test-large-untracked-render.tsx index c756fe88b..90d013ee7 100644 --- a/scripts/test-large-untracked-render.tsx +++ b/scripts/test-large-untracked-render.tsx @@ -3,7 +3,10 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { act } from "react"; +import { getBundledVcsCatalog } from "../packages/hunk/src/app/vcsCatalog"; import { loadAppBootstrap } from "../packages/hunk/src/core/changeset/loaders"; +import { createExtensionSession } from "../packages/hunk/src/extensions/session"; +import { createEmptyExtensionLoadResult } from "../packages/hunk/src/extensions/types"; import { AppHost } from "../packages/hunk/src/ui/AppHost"; function runGit(cwd: string, ...args: string[]) { @@ -56,9 +59,21 @@ try { const bootstrap = await loadAppBootstrap( { kind: "vcs", staged: false, options: { mode: "stack" } }, - { cwd: repo }, + { cwd: repo, vcsCatalog: getBundledVcsCatalog() }, + ); + const extensionSession = createExtensionSession( + bootstrap.extensions ?? createEmptyExtensionLoadResult(repo), + repo, + ); + const setup = await testRender( + extensionSession.shutdown()} + />, + { width: 120, height: 30 }, ); - const setup = await testRender(, { width: 120, height: 30 }); try { await act(async () => { @@ -85,6 +100,7 @@ try { ), ); } finally { + await extensionSession.shutdown(); await act(async () => { setup.renderer.destroy(); }); diff --git a/test/helpers/app-host.tsx b/test/helpers/app-host.tsx new file mode 100644 index 000000000..0fbc65688 --- /dev/null +++ b/test/helpers/app-host.tsx @@ -0,0 +1,54 @@ +import { useEffect, useState, type ComponentProps } from "react"; +import { createSessionReloadBounds } from "../../packages/hunk/src/app/session/reloadBounds"; +import { + createExtensionSession, + type ExtensionSession, +} from "../../packages/hunk/src/extensions/session"; +import { createEmptyExtensionLoadResult } from "../../packages/hunk/src/extensions/types"; +import { AppHost } from "../../packages/hunk/src/ui/AppHost"; + +type TestAppHostProps = Omit< + ComponentProps, + "extensionSession" | "extensionOwnership" | "onRequestSessionShutdown" +> & { + extensionSession?: ExtensionSession; + extensionOwnership?: "owned" | "borrowed"; +}; + +/** Supply explicit test-owned extension authority to an isolated AppHost mount. */ +export function TestAppHost({ + extensionSession: suppliedExtensionSession, + extensionOwnership = "owned", + ...props +}: TestAppHostProps) { + const [createdExtensionSession] = useState(() => + createExtensionSession( + (props.bootstrap.extensions as + | ReturnType + | undefined) ?? createEmptyExtensionLoadResult(props.bootstrap.reloadContext.cwd), + createSessionReloadBounds(props.bootstrap, { + cwd: props.bootstrap.reloadContext.cwd, + }).defaultCwd, + ), + ); + const extensionSession = suppliedExtensionSession ?? createdExtensionSession; + useEffect( + () => () => { + if (extensionOwnership === "owned") void extensionSession.shutdown(); + }, + [extensionOwnership, extensionSession], + ); + if (extensionOwnership === "borrowed" && !suppliedExtensionSession) { + throw new Error("Borrowed TestAppHost mounts require an explicit extension session owner."); + } + return ( + extensionSession.shutdown() : async () => undefined + } + /> + ); +} diff --git a/test/helpers/extension-session.ts b/test/helpers/extension-session.ts new file mode 100644 index 000000000..1b7849600 --- /dev/null +++ b/test/helpers/extension-session.ts @@ -0,0 +1,7 @@ +import { createExtensionSession } from "../../packages/hunk/src/extensions/session"; +import { createEmptyExtensionLoadResult } from "../../packages/hunk/src/extensions/types"; + +/** Create an empty explicitly owned extension session for lifecycle tests. */ +export function createTestExtensionSession(cwd = "/repo") { + return createExtensionSession(createEmptyExtensionLoadResult(cwd), cwd); +}