Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/explicit-extension-session-owner.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
2 changes: 1 addition & 1 deletion benchmarks/highlight-prefetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/huge-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/interaction-latency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/large-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions benchmarks/lib/appHost.tsx
Original file line number Diff line number Diff line change
@@ -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<ComponentProps<typeof AppHost>, "bootstrap">) {
const [extensionSession] = useState(() =>
createExtensionSession(
(bootstrap.extensions as ReturnType<typeof createEmptyExtensionLoadResult> | undefined) ??
createEmptyExtensionLoadResult(bootstrap.reloadContext.cwd),
bootstrap.reloadContext.cwd,
),
);
useEffect(
() => () => {
void extensionSession.shutdown();
},
[extensionSession],
);
return (
<AppHost
bootstrap={bootstrap}
extensionSession={extensionSession}
extensionOwnership="owned"
onRequestSessionShutdown={() => extensionSession.shutdown()}
/>
);
}
2 changes: 1 addition & 1 deletion benchmarks/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/navigation-memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/non-ascii-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/resize-memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/wrapped-cjk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
8 changes: 6 additions & 2 deletions docs/extension-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions docs/extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions docs/source-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions packages/hunk/src/app/historyBootstrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
23 changes: 9 additions & 14 deletions packages/hunk/src/app/historyBootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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(
Expand Down Expand Up @@ -75,6 +74,7 @@ export async function loadHistoryBootstrap({
baseVcsCatalog,
previousLoad,
});
const extensionSession = createExtensionSession(resolved.extensions, cwd);
const extensionAdapters = resolveExtensionVcsAdapters(
resolved.extensions.registry,
baseVcsCatalog,
Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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(
Expand Down Expand Up @@ -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();
},
};
}
32 changes: 28 additions & 4 deletions packages/hunk/src/app/historyReview.test.ts
Original file line number Diff line number Diff line change
@@ -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"),
Expand All @@ -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,
};
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/hunk/src/app/historyReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -76,6 +76,6 @@ export async function prepareEmbeddedHistoryReview(
plan.controllingTerminal?.close();
return {
bootstrap: plan.bootstrap as AppBootstrap<ExtensionLoadResult>,
borrowsExtensions: plan.bootstrap.extensions === request.extensionSession,
borrowsExtensions: plan.bootstrap.extensions?.registry === request.extensionSession?.registry,
};
}
Loading
Loading