From cd006476159963c9e6cc574ca46e1b7c5bd188b4 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 01:01:48 -0400 Subject: [PATCH 01/16] feat(embedded): add host API for mounting Hunk --- package.json | 4 + scripts/build-npm.ts | 24 ++ src/embedded/embedded.test.ts | 141 ++++++++++++ src/embedded/index.d.ts | 73 ++++++ src/embedded/index.tsx | 289 ++++++++++++++++++++++++ src/hunk-session/sessionRegistration.ts | 7 +- src/ui/App.tsx | 3 + src/ui/AppHost.interactions.test.tsx | 53 +++++ src/ui/AppHost.tsx | 9 +- src/ui/hooks/useAppKeyboardShortcuts.ts | 8 + src/ui/hooks/useReviewController.ts | 4 +- 11 files changed, 610 insertions(+), 5 deletions(-) create mode 100644 src/embedded/embedded.test.ts create mode 100644 src/embedded/index.d.ts create mode 100644 src/embedded/index.tsx diff --git a/package.json b/package.json index 8092aed1..033485a9 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,10 @@ "types": "./dist/npm/opentui/index.d.ts", "import": "./dist/npm/opentui/index.js" }, + "./embedded": { + "types": "./dist/npm/embedded/index.d.ts", + "import": "./dist/npm/embedded/index.js" + }, "./package.json": "./package.json" }, "publishConfig": { diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index c4be9c6d..2e95cb4c 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -8,6 +8,7 @@ const outdir = path.join(repoRoot, "dist", "npm"); const typesOutdir = path.join(repoRoot, "dist", "npm-types"); const opentuiOutdir = path.join(outdir, "opentui"); const opentuiTypesDir = path.join(typesOutdir, "opentui"); +const embeddedOutdir = path.join(outdir, "embedded"); const bunEnv = { ...process.env, @@ -32,6 +33,7 @@ function runBun(args: string[]) { rmSync(outdir, { recursive: true, force: true }); rmSync(typesOutdir, { recursive: true, force: true }); mkdirSync(opentuiOutdir, { recursive: true }); +mkdirSync(embeddedOutdir, { recursive: true }); runBun([ "build", @@ -81,6 +83,22 @@ runBun([ "index.js", ]); +const embeddedBuild = await Bun.build({ + entrypoints: [path.join(repoRoot, "src", "embedded", "index.tsx")], + target: "node", + format: "esm", + outdir: embeddedOutdir, + naming: { entry: "index.js" }, + external: ["@opentui/core", "@opentui/react", "@pierre/diffs", "react", "react/jsx-runtime"], +}); + +if (!embeddedBuild.success) { + for (const log of embeddedBuild.logs) { + console.error(log.message); + } + throw new Error("Failed to build embedded Hunk export."); +} + runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.opentui.json")]); for (const entry of readdirSync(opentuiTypesDir)) { @@ -89,7 +107,13 @@ for (const entry of readdirSync(opentuiTypesDir)) { } } +copyFileSync( + path.join(repoRoot, "src", "embedded", "index.d.ts"), + path.join(embeddedOutdir, "index.d.ts"), +); + rmSync(typesOutdir, { recursive: true, force: true }); console.log(`Built ${mainJs}`); console.log(`Built ${path.join(opentuiOutdir, "index.js")}`); +console.log(`Built ${path.join(embeddedOutdir, "index.js")}`); diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts new file mode 100644 index 00000000..250926c2 --- /dev/null +++ b/src/embedded/embedded.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createEmbeddedHunkSession, embeddedSourceToCliInput } from "./index"; + +const patchText = [ + "diff --git a/example.ts b/example.ts", + "--- a/example.ts", + "+++ b/example.ts", + "@@ -1 +1 @@", + "-const value = 1;", + "+const value = 2;", + "", +].join("\n"); + +describe("embeddedSourceToCliInput", () => { + test("maps embedded review sources to Hunk CLI inputs", () => { + expect(embeddedSourceToCliInput({ kind: "worktree", options: { theme: "midnight" } })).toEqual({ + kind: "vcs", + staged: false, + options: { theme: "midnight" }, + }); + expect(embeddedSourceToCliInput({ kind: "staged" })).toEqual({ + kind: "vcs", + staged: true, + options: {}, + }); + expect(embeddedSourceToCliInput({ kind: "show", ref: "HEAD~1" })).toEqual({ + kind: "show", + ref: "HEAD~1", + options: {}, + }); + expect( + embeddedSourceToCliInput({ + kind: "vcs", + range: "main", + staged: false, + pathspecs: ["src/app.ts"], + options: { vcs: "jj" }, + }), + ).toEqual({ + kind: "vcs", + range: "main", + staged: false, + pathspecs: ["src/app.ts"], + options: { vcs: "jj" }, + }); + expect( + embeddedSourceToCliInput({ kind: "diff", left: "before.ts", right: "after.ts" }), + ).toEqual({ + kind: "diff", + left: "before.ts", + right: "after.ts", + options: {}, + }); + expect(embeddedSourceToCliInput({ kind: "stash-show", ref: "stash@{1}" })).toEqual({ + kind: "stash-show", + ref: "stash@{1}", + options: {}, + }); + expect(embeddedSourceToCliInput({ kind: "patch", file: "changes.patch" })).toEqual({ + kind: "patch", + file: "changes.patch", + options: {}, + }); + expect( + embeddedSourceToCliInput({ + kind: "difftool", + left: "left.ts", + right: "right.ts", + path: "src/app.ts", + }), + ).toEqual({ + kind: "difftool", + left: "left.ts", + right: "right.ts", + path: "src/app.ts", + options: {}, + }); + }); + + test("loads embedded sessions through Hunk config resolution", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-config-")); + const previousXdgConfigHome = process.env.XDG_CONFIG_HOME; + + try { + const configHome = join(root, "config"); + mkdirSync(join(configHome, "hunk"), { recursive: true }); + writeFileSync( + join(configHome, "hunk", "config.toml"), + ['theme = "midnight"', 'mode = "stack"', "line_numbers = false"].join("\n"), + ); + process.env.XDG_CONFIG_HOME = configHome; + + const session = await createEmbeddedHunkSession({ + cwd: root, + source: { kind: "patch", text: patchText, options: { theme: "paper" } }, + }); + const snapshot = session.getSnapshot(); + + expect(snapshot.status).toBe("ready"); + if (snapshot.status !== "ready") throw new Error("Expected embedded session to load."); + expect(snapshot.bootstrap.initialMode).toBe("stack"); + expect(snapshot.bootstrap.initialShowLineNumbers).toBe(false); + expect(snapshot.bootstrap.initialTheme).toBe("paper"); + + session.dispose(); + } finally { + if (previousXdgConfigHome === undefined) { + delete process.env.XDG_CONFIG_HOME; + } else { + process.env.XDG_CONFIG_HOME = previousXdgConfigHome; + } + rmSync(root, { force: true, recursive: true }); + } + }); + + test("keeps the previous source and reports errors when reload fails", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-error-")); + + try { + const initialSource = { kind: "patch", text: patchText, label: "initial patch" } as const; + const session = await createEmbeddedHunkSession({ + cwd: root, + source: initialSource, + }); + + await expect(session.load({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); + + expect(session.source).toEqual(initialSource); + const snapshot = session.getSnapshot(); + expect(snapshot.status).toBe("error"); + expect(snapshot.bootstrap).toBeDefined(); + + session.dispose(); + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); +}); diff --git a/src/embedded/index.d.ts b/src/embedded/index.d.ts new file mode 100644 index 00000000..cf920500 --- /dev/null +++ b/src/embedded/index.d.ts @@ -0,0 +1,73 @@ +import type { CliRenderer, Renderable } from "@opentui/core"; + +export interface EmbeddedHunkOptions { + mode?: "auto" | "split" | "stack"; + vcs?: "git" | "jj"; + theme?: string; + watch?: boolean; + excludeUntracked?: boolean; + lineNumbers?: boolean; + wrapLines?: boolean; + hunkHeaders?: boolean; + agentNotes?: boolean; +} + +export type EmbeddedHunkSource = + | { kind: "worktree"; pathspecs?: string[]; options?: EmbeddedHunkOptions } + | { kind: "staged"; pathspecs?: string[]; options?: EmbeddedHunkOptions } + | { + kind: "vcs"; + range?: string; + staged: boolean; + pathspecs?: string[]; + options?: EmbeddedHunkOptions; + } + | { kind: "show"; ref?: string; pathspecs?: string[]; options?: EmbeddedHunkOptions } + | { kind: "stash-show"; ref?: string; options?: EmbeddedHunkOptions } + | { kind: "diff"; left: string; right: string; options?: EmbeddedHunkOptions } + | { + kind: "patch"; + file?: string; + text?: string; + label?: string; + options?: EmbeddedHunkOptions; + } + | { + kind: "difftool"; + left: string; + right: string; + path?: string; + options?: EmbeddedHunkOptions; + }; + +export type EmbeddedHunkSnapshot = + | { status: "loading"; bootstrap?: unknown; error?: undefined } + | { status: "ready"; bootstrap: unknown; error?: undefined } + | { status: "error"; bootstrap?: unknown; error: string }; + +export interface EmbeddedHunkSession { + readonly cwd: string; + readonly source: EmbeddedHunkSource; + getSnapshot(): EmbeddedHunkSnapshot; + load(source: EmbeddedHunkSource): Promise; + subscribe(listener: () => void): () => void; + dispose(): void; +} + +export interface EmbeddedHunkMount { + update(options: { active: boolean; onQuit: () => void }): void; + unmount(): void; +} + +export declare function embeddedSourceToCliInput(source: EmbeddedHunkSource): unknown; +export declare function createEmbeddedHunkSession(input: { + cwd?: string; + source: EmbeddedHunkSource; +}): Promise; +export declare function mountEmbeddedHunkApp(input: { + active: boolean; + container: Renderable; + onQuit: () => void; + renderer: CliRenderer; + session: EmbeddedHunkSession; +}): EmbeddedHunkMount; diff --git a/src/embedded/index.tsx b/src/embedded/index.tsx new file mode 100644 index 00000000..1fc5073c --- /dev/null +++ b/src/embedded/index.tsx @@ -0,0 +1,289 @@ +import type { CliRenderer, Renderable } from "@opentui/core"; +import { createRoot } from "@opentui/react"; +import { useSyncExternalStore } from "react"; +import { resolveConfiguredCliInput } from "../core/config"; +import { loadAppBootstrap } from "../core/loaders"; +import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; +import { + createInitialSessionSnapshot, + createSessionRegistration, + updateSessionRegistration, +} from "../hunk-session/sessionRegistration"; +import type { HunkSessionBrokerClient } from "../hunk-session/types"; +import { SessionBrokerClient } from "../session-broker/brokerClient"; +import { AppHost } from "../ui/AppHost"; + +export type EmbeddedHunkSource = + | { kind: "worktree"; pathspecs?: string[]; options?: CommonOptions } + | { kind: "staged"; pathspecs?: string[]; options?: CommonOptions } + | { + kind: "vcs"; + range?: string; + staged: boolean; + pathspecs?: string[]; + options?: CommonOptions; + } + | { kind: "show"; ref?: string; pathspecs?: string[]; options?: CommonOptions } + | { kind: "stash-show"; ref?: string; options?: CommonOptions } + | { kind: "diff"; left: string; right: string; options?: CommonOptions } + | { kind: "patch"; file?: string; text?: string; label?: string; options?: CommonOptions } + | { kind: "difftool"; left: string; right: string; path?: string; options?: CommonOptions }; + +export type EmbeddedHunkSnapshot = + | { status: "loading"; bootstrap?: AppBootstrap; error?: undefined } + | { status: "ready"; bootstrap: AppBootstrap; error?: undefined } + | { status: "error"; bootstrap?: AppBootstrap; error: string }; + +export interface EmbeddedHunkSession { + readonly cwd: string; + readonly source: EmbeddedHunkSource; + getSnapshot(): EmbeddedHunkSnapshot; + load(source: EmbeddedHunkSource): Promise; + subscribe(listener: () => void): () => void; + dispose(): void; +} + +interface EmbeddedHunkSessionHandle extends EmbeddedHunkSession { + readonly hostClient: HunkSessionBrokerClient; +} + +export interface EmbeddedHunkMount { + update(options: { active: boolean; onQuit: () => void }): void; + unmount(): void; +} + +export function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { + switch (source.kind) { + case "worktree": + return { + kind: "vcs", + staged: false, + pathspecs: source.pathspecs, + options: source.options ?? {}, + }; + case "staged": + return { + kind: "vcs", + staged: true, + pathspecs: source.pathspecs, + options: source.options ?? {}, + }; + case "vcs": + return { + kind: "vcs", + range: source.range, + staged: source.staged, + pathspecs: source.pathspecs, + options: source.options ?? {}, + }; + case "show": + return { + kind: "show", + ref: source.ref, + pathspecs: source.pathspecs, + options: source.options ?? {}, + }; + case "stash-show": + return { + kind: "stash-show", + ref: source.ref, + options: source.options ?? {}, + }; + case "diff": + return { + kind: "diff", + left: source.left, + right: source.right, + options: source.options ?? {}, + }; + case "patch": + return { + kind: "patch", + text: source.text, + file: source.file ?? source.label, + options: source.options ?? {}, + }; + case "difftool": + return { + kind: "difftool", + left: source.left, + right: source.right, + path: source.path, + options: source.options ?? {}, + }; + } +} + +function resolveEmbeddedCliInput(source: EmbeddedHunkSource, cwd: string) { + return resolveConfiguredCliInput(embeddedSourceToCliInput(source), { cwd }).input; +} + +function errorMessage(error: unknown) { + if (error instanceof Error && error.message) return error.message; + return String(error || "Failed to load Hunk."); +} + +class EmbeddedHunkSessionImpl implements EmbeddedHunkSessionHandle { + private listeners = new Set<() => void>(); + private disposed = false; + private snapshot: EmbeddedHunkSnapshot; + + readonly hostClient: HunkSessionBrokerClient; + + private constructor( + readonly cwd: string, + public source: EmbeddedHunkSource, + bootstrap: AppBootstrap, + ) { + this.snapshot = { status: "ready", bootstrap }; + this.hostClient = new SessionBrokerClient( + createSessionRegistration(bootstrap, { cwd }), + createInitialSessionSnapshot(bootstrap), + ); + this.hostClient.start(); + } + + static async create({ cwd, source }: { cwd: string; source: EmbeddedHunkSource }) { + const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, cwd), { cwd }); + return new EmbeddedHunkSessionImpl(cwd, source, bootstrap); + } + + getSnapshot = () => this.snapshot; + + subscribe = (listener: () => void) => { + this.listeners.add(listener); + return () => this.listeners.delete(listener); + }; + + async load(source: EmbeddedHunkSource) { + if (this.disposed) return; + this.setSnapshot({ status: "loading", bootstrap: this.snapshot.bootstrap }); + + try { + const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, this.cwd), { + cwd: this.cwd, + }); + this.source = source; + this.hostClient.replaceSession( + updateSessionRegistration(this.hostClient.getRegistration(), bootstrap), + createInitialSessionSnapshot(bootstrap), + ); + this.setSnapshot({ status: "ready", bootstrap }); + } catch (error) { + const message = errorMessage(error); + this.setSnapshot({ + status: "error", + bootstrap: this.snapshot.bootstrap, + error: message, + }); + throw error instanceof Error ? error : new Error(message); + } + } + + dispose() { + this.disposed = true; + this.hostClient.stop(); + this.listeners.clear(); + } + + private setSnapshot(snapshot: EmbeddedHunkSnapshot) { + this.snapshot = snapshot; + for (const listener of this.listeners) listener(); + } +} + +/** Resolve the internal broker client owned by sessions created through this entrypoint. */ +function sessionHostClient(session: EmbeddedHunkSession) { + const hostClient = (session as Partial).hostClient; + if (!hostClient) { + throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); + } + return hostClient; +} + +function EmbeddedHunkRoot({ + active, + onQuit, + session, +}: { + active: boolean; + onQuit: () => void; + session: EmbeddedHunkSession; +}) { + const snapshot = useSyncExternalStore( + session.subscribe, + session.getSnapshot, + session.getSnapshot, + ); + + if (snapshot.status === "loading" && !snapshot.bootstrap) { + return ( + + Loading Hunk... + + ); + } + + if (snapshot.status === "error" && !snapshot.bootstrap) { + return ( + + {`Hunk failed: ${snapshot.error}`} + + ); + } + + return ( + null} + /> + ); +} + +function scopedRenderer(renderer: CliRenderer, root: Renderable) { + const scoped = Object.create(renderer) as CliRenderer; + Object.defineProperty(scoped, "root", { value: root }); + return scoped; +} + +export async function createEmbeddedHunkSession({ + cwd = process.cwd(), + source, +}: { + cwd?: string; + source: EmbeddedHunkSource; +}): Promise { + return EmbeddedHunkSessionImpl.create({ cwd, source }); +} + +export function mountEmbeddedHunkApp({ + active, + container, + onQuit, + renderer, + session, +}: { + active: boolean; + container: Renderable; + onQuit: () => void; + renderer: CliRenderer; + session: EmbeddedHunkSession; +}): EmbeddedHunkMount { + const root = createRoot(scopedRenderer(renderer, container)); + + const render = (next: { active: boolean; onQuit: () => void }) => { + root.render(); + }; + + render({ active, onQuit }); + + return { + update: render, + unmount() { + root.unmount(); + }, + }; +} diff --git a/src/hunk-session/sessionRegistration.ts b/src/hunk-session/sessionRegistration.ts index 958d93cc..96c695dd 100644 --- a/src/hunk-session/sessionRegistration.ts +++ b/src/hunk-session/sessionRegistration.ts @@ -52,14 +52,17 @@ function buildSessionFiles(bootstrap: AppBootstrap): SessionReviewFile[] { } /** Build the broker-facing envelope for one live Hunk review session. */ -export function createSessionRegistration(bootstrap: AppBootstrap): HunkSessionRegistration { +export function createSessionRegistration( + bootstrap: AppBootstrap, + options: { cwd?: string } = {}, +): HunkSessionRegistration { const terminal = resolveSessionTerminalMetadata({ tty: ttyname() }); return { registrationVersion: SESSION_BROKER_REGISTRATION_VERSION, sessionId: randomUUID(), pid: process.pid, - cwd: process.cwd(), + cwd: options.cwd ?? process.cwd(), repoRoot: inferRepoRoot(bootstrap), launchedAt: new Date().toISOString(), terminal, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 1bc36ce0..a6c194ec 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -75,12 +75,14 @@ function withCurrentViewOptions( /** Orchestrate global app state, layout, navigation, and pane coordination. */ export function App({ + active = true, bootstrap, hostClient, noticeText, onQuit = () => process.exit(0), onReloadSession, }: { + active?: boolean; bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; noticeText?: string | null; @@ -629,6 +631,7 @@ export function App({ } = useMenuController(menus); useAppKeyboardShortcuts({ + active, activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 99dc227e..c7a66512 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -167,6 +167,16 @@ function createLineScrollBootstrap(pager = false): AppBootstrap { }); } +function createLiveCommentScrollBootstrap(): AppBootstrap { + const before = lines(...createNumberedAssignmentLines(1, 18)); + const after = lines(...createNumberedAssignmentLines(1, 18, 100)); + + return createTestVcsAppBootstrap({ + changesetId: "changeset:app-live-comment-scroll", + files: [createTestDiffFile("scroll-live", "scroll-live.ts", before, after)], + }); +} + /** Build a two-hunk fixture with a deep inline note for CLI comment-navigation scroll tests. */ function createDeepNoteBootstrap(): AppBootstrap { const beforeLines = Array.from( @@ -2137,6 +2147,49 @@ describe("App interactions", () => { } }); + test("focused live comments scroll the new inline note into view", async () => { + const { dispatchCommand, hostClient } = createMockHostClient(); + const setup = await testRender( + , + { + width: 104, + height: 12, + }, + ); + + try { + await flush(setup); + + let frame = setup.captureCharFrame(); + expect(frame).not.toContain("Live note anchored near the bottom."); + + await act(async () => { + await dispatchCommand({ + type: "command", + requestId: "comment-1", + command: "comment", + input: { + sessionId: "session-1", + filePath: "scroll-live.ts", + side: "new", + line: 12, + summary: "Live note anchored near the bottom.", + reveal: true, + }, + }); + }); + + frame = await waitForFrame(setup, (currentFrame) => + currentFrame.includes("Live note anchored near the bottom."), + ); + expect(frame).toContain("Live note anchored near the bottom."); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("menu navigation wraps across the first and last top-level menus", async () => { const setup = await testRender(, { width: 220, diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index 96aa9cf5..1c2a05e1 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -1,4 +1,4 @@ -import { useCallback, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { resolveConfiguredCliInput } from "../core/config"; import { loadAppBootstrap } from "../core/loaders"; import { resolveRuntimeCliInput } from "../core/terminal"; @@ -14,11 +14,13 @@ import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; /** Keep one live Hunk app mounted while allowing daemon-driven session reloads. */ export function AppHost({ + active = true, bootstrap, hostClient, onQuit = () => process.exit(0), startupNoticeResolver, }: { + active?: boolean; bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; onQuit?: () => void; @@ -31,6 +33,10 @@ export function AppHost({ resolver: startupNoticeResolver, }); + useEffect(() => { + setActiveBootstrap(bootstrap); + }, [bootstrap]); + const reloadSession = useCallback( async (nextInput: CliInput, options?: { resetApp?: boolean; sourcePath?: string }) => { // Re-run the same startup normalization pipeline used on first launch so reloads honor @@ -81,6 +87,7 @@ export function AppHost({ return ( void; canRefreshCurrentInput: boolean; @@ -77,6 +78,7 @@ export interface UseAppKeyboardShortcutsOptions { /** Register the app's scoped keyboard handling while keeping mode precedence explicit. */ export function useAppKeyboardShortcuts({ + active = true, activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, @@ -110,11 +112,13 @@ export function useAppKeyboardShortcuts({ triggerEditSelectedFile, triggerRefreshCurrentInput, }: UseAppKeyboardShortcutsOptions) { + const activeRef = useRef(active); const activeMenuIdRef = useRef(activeMenuId); const focusAreaRef = useRef(focusArea); const pagerModeRef = useRef(pagerMode); const showHelpRef = useRef(showHelp); + activeRef.current = active; activeMenuIdRef.current = activeMenuId; focusAreaRef.current = focusArea; pagerModeRef.current = pagerMode; @@ -487,6 +491,10 @@ export function useAppKeyboardShortcuts({ }; useKeyboard((key: KeyEvent) => { + if (!activeRef.current) { + return; + } + if (handleMenuToggleShortcut(key)) { return; } diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index f8c1b9ea..3b1ff431 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -394,7 +394,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon })); if (options?.reveal ?? false) { - selectHunk(file.id, target.hunkIndex); + selectHunk(file.id, target.hunkIndex, { scrollToNote: true }); } return { @@ -453,7 +453,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon if (options?.revealMode === "first" && prepared.length > 0) { const first = prepared[0]!; - selectHunk(first.file.id, first.target.hunkIndex); + selectHunk(first.file.id, first.target.hunkIndex, { scrollToNote: true }); } return { From 01eac6e021d5fba51d5f7aa815722b836a685e03 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 01:23:24 -0400 Subject: [PATCH 02/16] feat: add embedded Hunk export for Opencode --- scripts/build-npm.ts | 78 ++++++++++++++--------------- src/embedded/embedded.test.ts | 72 ++------------------------- src/embedded/index.d.ts | 5 +- src/embedded/index.tsx | 93 +++++++---------------------------- src/ui/AppHost.tsx | 9 +++- 5 files changed, 68 insertions(+), 189 deletions(-) diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index 2e95cb4c..7821b86c 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -9,6 +9,16 @@ const typesOutdir = path.join(repoRoot, "dist", "npm-types"); const opentuiOutdir = path.join(outdir, "opentui"); const opentuiTypesDir = path.join(typesOutdir, "opentui"); const embeddedOutdir = path.join(outdir, "embedded"); +const libraryExternals = [ + "react", + "react/jsx-runtime", + "react/jsx-dev-runtime", + "@opentui/core", + "@opentui/react", + "@opentui/react/jsx-runtime", + "@opentui/react/jsx-dev-runtime", + "@pierre/diffs", +]; const bunEnv = { ...process.env, @@ -30,6 +40,24 @@ function runBun(args: string[]) { } } +async function buildLibraryExport(name: string, entrypoint: string, outputDirectory: string) { + const build = await Bun.build({ + entrypoints: [entrypoint], + target: "node", + format: "esm", + outdir: outputDirectory, + naming: { entry: "index.js" }, + external: libraryExternals, + }); + + if (!build.success) { + for (const log of build.logs) { + console.error(log.message); + } + throw new Error(`Failed to build ${name} export.`); + } +} + rmSync(outdir, { recursive: true, force: true }); rmSync(typesOutdir, { recursive: true, force: true }); mkdirSync(opentuiOutdir, { recursive: true }); @@ -54,50 +82,16 @@ if (process.platform !== "win32") { chmodSync(mainJs, 0o755); } -runBun([ - "build", +await buildLibraryExport( + "OpenTUI", path.join(repoRoot, "src", "opentui", "index.ts"), - "--target", - "node", - "--format", - "esm", - "--external", - "react", - "--external", - "react/jsx-runtime", - "--external", - "react/jsx-dev-runtime", - "--external", - "@opentui/core", - "--external", - "@opentui/react", - "--external", - "@opentui/react/jsx-runtime", - "--external", - "@opentui/react/jsx-dev-runtime", - "--external", - "@pierre/diffs", - "--outdir", opentuiOutdir, - "--entry-naming", - "index.js", -]); - -const embeddedBuild = await Bun.build({ - entrypoints: [path.join(repoRoot, "src", "embedded", "index.tsx")], - target: "node", - format: "esm", - outdir: embeddedOutdir, - naming: { entry: "index.js" }, - external: ["@opentui/core", "@opentui/react", "@pierre/diffs", "react", "react/jsx-runtime"], -}); - -if (!embeddedBuild.success) { - for (const log of embeddedBuild.logs) { - console.error(log.message); - } - throw new Error("Failed to build embedded Hunk export."); -} +); +await buildLibraryExport( + "embedded Hunk", + path.join(repoRoot, "src", "embedded", "index.tsx"), + embeddedOutdir, +); runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.opentui.json")]); diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 250926c2..6cbd81ab 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { createEmbeddedHunkSession, embeddedSourceToCliInput } from "./index"; +import { createEmbeddedHunkSession } from "./index"; const patchText = [ "diff --git a/example.ts b/example.ts", @@ -14,72 +14,7 @@ const patchText = [ "", ].join("\n"); -describe("embeddedSourceToCliInput", () => { - test("maps embedded review sources to Hunk CLI inputs", () => { - expect(embeddedSourceToCliInput({ kind: "worktree", options: { theme: "midnight" } })).toEqual({ - kind: "vcs", - staged: false, - options: { theme: "midnight" }, - }); - expect(embeddedSourceToCliInput({ kind: "staged" })).toEqual({ - kind: "vcs", - staged: true, - options: {}, - }); - expect(embeddedSourceToCliInput({ kind: "show", ref: "HEAD~1" })).toEqual({ - kind: "show", - ref: "HEAD~1", - options: {}, - }); - expect( - embeddedSourceToCliInput({ - kind: "vcs", - range: "main", - staged: false, - pathspecs: ["src/app.ts"], - options: { vcs: "jj" }, - }), - ).toEqual({ - kind: "vcs", - range: "main", - staged: false, - pathspecs: ["src/app.ts"], - options: { vcs: "jj" }, - }); - expect( - embeddedSourceToCliInput({ kind: "diff", left: "before.ts", right: "after.ts" }), - ).toEqual({ - kind: "diff", - left: "before.ts", - right: "after.ts", - options: {}, - }); - expect(embeddedSourceToCliInput({ kind: "stash-show", ref: "stash@{1}" })).toEqual({ - kind: "stash-show", - ref: "stash@{1}", - options: {}, - }); - expect(embeddedSourceToCliInput({ kind: "patch", file: "changes.patch" })).toEqual({ - kind: "patch", - file: "changes.patch", - options: {}, - }); - expect( - embeddedSourceToCliInput({ - kind: "difftool", - left: "left.ts", - right: "right.ts", - path: "src/app.ts", - }), - ).toEqual({ - kind: "difftool", - left: "left.ts", - right: "right.ts", - path: "src/app.ts", - options: {}, - }); - }); - +describe("embedded Hunk sessions", () => { test("loads embedded sessions through Hunk config resolution", async () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-config-")); const previousXdgConfigHome = process.env.XDG_CONFIG_HOME; @@ -131,7 +66,8 @@ describe("embeddedSourceToCliInput", () => { expect(session.source).toEqual(initialSource); const snapshot = session.getSnapshot(); expect(snapshot.status).toBe("error"); - expect(snapshot.bootstrap).toBeDefined(); + if (snapshot.status !== "error") throw new Error("Expected embedded reload to fail."); + expect(snapshot.error).toContain("missing.patch"); session.dispose(); } finally { diff --git a/src/embedded/index.d.ts b/src/embedded/index.d.ts index cf920500..113bc3a2 100644 --- a/src/embedded/index.d.ts +++ b/src/embedded/index.d.ts @@ -41,9 +41,9 @@ export type EmbeddedHunkSource = }; export type EmbeddedHunkSnapshot = - | { status: "loading"; bootstrap?: unknown; error?: undefined } + | { status: "loading"; bootstrap: unknown; error?: undefined } | { status: "ready"; bootstrap: unknown; error?: undefined } - | { status: "error"; bootstrap?: unknown; error: string }; + | { status: "error"; bootstrap: unknown; error: string }; export interface EmbeddedHunkSession { readonly cwd: string; @@ -59,7 +59,6 @@ export interface EmbeddedHunkMount { unmount(): void; } -export declare function embeddedSourceToCliInput(source: EmbeddedHunkSource): unknown; export declare function createEmbeddedHunkSession(input: { cwd?: string; source: EmbeddedHunkSource; diff --git a/src/embedded/index.tsx b/src/embedded/index.tsx index 1fc5073c..1eb9c511 100644 --- a/src/embedded/index.tsx +++ b/src/embedded/index.tsx @@ -30,9 +30,9 @@ export type EmbeddedHunkSource = | { kind: "difftool"; left: string; right: string; path?: string; options?: CommonOptions }; export type EmbeddedHunkSnapshot = - | { status: "loading"; bootstrap?: AppBootstrap; error?: undefined } + | { status: "loading"; bootstrap: AppBootstrap; error?: undefined } | { status: "ready"; bootstrap: AppBootstrap; error?: undefined } - | { status: "error"; bootstrap?: AppBootstrap; error: string }; + | { status: "error"; bootstrap: AppBootstrap; error: string }; export interface EmbeddedHunkSession { readonly cwd: string; @@ -43,74 +43,38 @@ export interface EmbeddedHunkSession { dispose(): void; } -interface EmbeddedHunkSessionHandle extends EmbeddedHunkSession { - readonly hostClient: HunkSessionBrokerClient; -} - export interface EmbeddedHunkMount { update(options: { active: boolean; onQuit: () => void }): void; unmount(): void; } -export function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { +function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { + const options = source.options ?? {}; + switch (source.kind) { case "worktree": return { kind: "vcs", staged: false, pathspecs: source.pathspecs, - options: source.options ?? {}, + options, }; case "staged": return { kind: "vcs", staged: true, pathspecs: source.pathspecs, - options: source.options ?? {}, - }; - case "vcs": - return { - kind: "vcs", - range: source.range, - staged: source.staged, - pathspecs: source.pathspecs, - options: source.options ?? {}, - }; - case "show": - return { - kind: "show", - ref: source.ref, - pathspecs: source.pathspecs, - options: source.options ?? {}, - }; - case "stash-show": - return { - kind: "stash-show", - ref: source.ref, - options: source.options ?? {}, - }; - case "diff": - return { - kind: "diff", - left: source.left, - right: source.right, - options: source.options ?? {}, + options, }; case "patch": return { kind: "patch", text: source.text, file: source.file ?? source.label, - options: source.options ?? {}, - }; - case "difftool": - return { - kind: "difftool", - left: source.left, - right: source.right, - path: source.path, - options: source.options ?? {}, + options, }; + default: + return { ...source, options } as CliInput; } } @@ -123,14 +87,14 @@ function errorMessage(error: unknown) { return String(error || "Failed to load Hunk."); } -class EmbeddedHunkSessionImpl implements EmbeddedHunkSessionHandle { +class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); private disposed = false; private snapshot: EmbeddedHunkSnapshot; readonly hostClient: HunkSessionBrokerClient; - private constructor( + constructor( readonly cwd: string, public source: EmbeddedHunkSource, bootstrap: AppBootstrap, @@ -143,11 +107,6 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSessionHandle { this.hostClient.start(); } - static async create({ cwd, source }: { cwd: string; source: EmbeddedHunkSource }) { - const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, cwd), { cwd }); - return new EmbeddedHunkSessionImpl(cwd, source, bootstrap); - } - getSnapshot = () => this.snapshot; subscribe = (listener: () => void) => { @@ -194,11 +153,10 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSessionHandle { /** Resolve the internal broker client owned by sessions created through this entrypoint. */ function sessionHostClient(session: EmbeddedHunkSession) { - const hostClient = (session as Partial).hostClient; - if (!hostClient) { - throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); + if (session instanceof EmbeddedHunkSessionImpl) { + return session.hostClient; } - return hostClient; + throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); } function EmbeddedHunkRoot({ @@ -216,26 +174,10 @@ function EmbeddedHunkRoot({ session.getSnapshot, ); - if (snapshot.status === "loading" && !snapshot.bootstrap) { - return ( - - Loading Hunk... - - ); - } - - if (snapshot.status === "error" && !snapshot.bootstrap) { - return ( - - {`Hunk failed: ${snapshot.error}`} - - ); - } - return ( null} @@ -256,7 +198,8 @@ export async function createEmbeddedHunkSession({ cwd?: string; source: EmbeddedHunkSource; }): Promise { - return EmbeddedHunkSessionImpl.create({ cwd, source }); + const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, cwd), { cwd }); + return new EmbeddedHunkSessionImpl(cwd, source, bootstrap); } export function mountEmbeddedHunkApp({ diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index 1c2a05e1..37970796 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { resolveConfiguredCliInput } from "../core/config"; import { loadAppBootstrap } from "../core/loaders"; import { resolveRuntimeCliInput } from "../core/terminal"; @@ -28,13 +28,20 @@ export function AppHost({ }) { const [activeBootstrap, setActiveBootstrap] = useState(bootstrap); const [appVersion, setAppVersion] = useState(0); + const previousBootstrapRef = useRef(bootstrap); const startupNoticeText = useStartupUpdateNotice({ enabled: !bootstrap.input.options.pager, resolver: startupNoticeResolver, }); useEffect(() => { + if (previousBootstrapRef.current === bootstrap) { + return; + } + + previousBootstrapRef.current = bootstrap; setActiveBootstrap(bootstrap); + setAppVersion((current) => current + 1); }, [bootstrap]); const reloadSession = useCallback( From 7a4ec086e927f366e3397b9114a2bf0b4c52362a Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 02:31:26 -0400 Subject: [PATCH 03/16] refactor(embedded): split session lifecycle and npm exports --- scripts/build-npm.ts | 14 +- scripts/check-pack.ts | 2 + scripts/check-prebuilt-pack.ts | 2 + src/embedded/embedded.test.ts | 111 ++++++++- src/embedded/index.ts | 29 +++ src/embedded/index.tsx | 232 ------------------ src/embedded/mount.tsx | 142 +++++++++++ src/embedded/session.ts | 162 ++++++++++++ src/embedded/source.ts | 103 ++++++++ src/embedded/{index.d.ts => types.ts} | 48 ++-- ....opentui.json => tsconfig.npm-exports.json | 4 +- 11 files changed, 581 insertions(+), 268 deletions(-) create mode 100644 src/embedded/index.ts delete mode 100644 src/embedded/index.tsx create mode 100644 src/embedded/mount.tsx create mode 100644 src/embedded/session.ts create mode 100644 src/embedded/source.ts rename src/embedded/{index.d.ts => types.ts} (65%) rename tsconfig.opentui.json => tsconfig.npm-exports.json (70%) diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index 7821b86c..c7c0c03d 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -7,8 +7,9 @@ const repoRoot = path.resolve(import.meta.dir, ".."); const outdir = path.join(repoRoot, "dist", "npm"); const typesOutdir = path.join(repoRoot, "dist", "npm-types"); const opentuiOutdir = path.join(outdir, "opentui"); -const opentuiTypesDir = path.join(typesOutdir, "opentui"); +const opentuiTypesDir = path.join(typesOutdir, "src", "opentui"); const embeddedOutdir = path.join(outdir, "embedded"); +const embeddedTypesDir = path.join(typesOutdir, "src", "embedded"); const libraryExternals = [ "react", "react/jsx-runtime", @@ -89,11 +90,11 @@ await buildLibraryExport( ); await buildLibraryExport( "embedded Hunk", - path.join(repoRoot, "src", "embedded", "index.tsx"), + path.join(repoRoot, "src", "embedded", "index.ts"), embeddedOutdir, ); -runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.opentui.json")]); +runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.npm-exports.json")]); for (const entry of readdirSync(opentuiTypesDir)) { if (entry.endsWith(".d.ts")) { @@ -101,10 +102,9 @@ for (const entry of readdirSync(opentuiTypesDir)) { } } -copyFileSync( - path.join(repoRoot, "src", "embedded", "index.d.ts"), - path.join(embeddedOutdir, "index.d.ts"), -); +for (const entry of ["index.d.ts", "types.d.ts"]) { + copyFileSync(path.join(embeddedTypesDir, entry), path.join(embeddedOutdir, entry)); +} rmSync(typesOutdir, { recursive: true, force: true }); diff --git a/scripts/check-pack.ts b/scripts/check-pack.ts index d0baae92..cf43f1c0 100644 --- a/scripts/check-pack.ts +++ b/scripts/check-pack.ts @@ -48,6 +48,8 @@ const publishedPaths = new Set(pack.files.map((file) => file.path)); const requiredPaths = [ "bin/hunk.cjs", "dist/npm/main.js", + "dist/npm/embedded/index.d.ts", + "dist/npm/embedded/index.js", "dist/npm/opentui/index.d.ts", "dist/npm/opentui/index.js", "README.md", diff --git a/scripts/check-prebuilt-pack.ts b/scripts/check-prebuilt-pack.ts index 808e6c40..745065ab 100644 --- a/scripts/check-prebuilt-pack.ts +++ b/scripts/check-prebuilt-pack.ts @@ -67,6 +67,8 @@ const metaPack = runPackDryRun(metaDir); assertPaths(metaPack, [ "bin/hunk.cjs", "dist/npm/main.js", + "dist/npm/embedded/index.d.ts", + "dist/npm/embedded/index.js", "dist/npm/opentui/index.d.ts", "dist/npm/opentui/index.js", "skills/hunk-review/SKILL.md", diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 6cbd81ab..94f55579 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -1,8 +1,11 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createEmbeddedHunkSession } from "./index"; +import { createScopedKeyInput } from "./mount"; +import { embeddedHunkSessionInternals } from "./session"; +import type { EmbeddedHunkSession } from "./types"; const patchText = [ "diff --git a/example.ts b/example.ts", @@ -14,7 +17,34 @@ const patchText = [ "", ].join("\n"); +let previousHunkMcpDisable: string | undefined; + +/** Return the private app bootstrap for assertions that public snapshots intentionally hide. */ +function renderBootstrap(session: EmbeddedHunkSession) { + return embeddedHunkSessionInternals(session).getRenderSnapshot().bootstrap; +} + +/** Return the loaded patch text for one embedded session. */ +function loadedPatch(session: EmbeddedHunkSession) { + return renderBootstrap(session) + .changeset.files.map((file) => file.patch) + .join("\n"); +} + describe("embedded Hunk sessions", () => { + beforeEach(() => { + previousHunkMcpDisable = process.env.HUNK_MCP_DISABLE; + process.env.HUNK_MCP_DISABLE = "1"; + }); + + afterEach(() => { + if (previousHunkMcpDisable === undefined) { + delete process.env.HUNK_MCP_DISABLE; + } else { + process.env.HUNK_MCP_DISABLE = previousHunkMcpDisable; + } + }); + test("loads embedded sessions through Hunk config resolution", async () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-config-")); const previousXdgConfigHome = process.env.XDG_CONFIG_HOME; @@ -36,9 +66,14 @@ describe("embedded Hunk sessions", () => { expect(snapshot.status).toBe("ready"); if (snapshot.status !== "ready") throw new Error("Expected embedded session to load."); - expect(snapshot.bootstrap.initialMode).toBe("stack"); - expect(snapshot.bootstrap.initialShowLineNumbers).toBe(false); - expect(snapshot.bootstrap.initialTheme).toBe("paper"); + expect("bootstrap" in snapshot).toBe(false); + expect(snapshot.title).toBe("Patch review: stdin patch"); + expect(snapshot.fileCount).toBe(1); + + const bootstrap = renderBootstrap(session); + expect(bootstrap.initialMode).toBe("stack"); + expect(bootstrap.initialShowLineNumbers).toBe(false); + expect(bootstrap.initialTheme).toBe("paper"); session.dispose(); } finally { @@ -51,7 +86,37 @@ describe("embedded Hunk sessions", () => { } }); - test("keeps the previous source and reports errors when reload fails", async () => { + test("open reuses the loaded review when source identity has not changed", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-open-same-source-")); + const left = join(root, "before.ts"); + const right = join(root, "after.ts"); + + try { + writeFileSync(left, "export const value = 1;\n"); + writeFileSync(right, "export const value = 2;\nexport const first = true;\n"); + + const source = { kind: "diff", left, right } as const; + const session = await createEmbeddedHunkSession({ cwd: root, source }); + expect(loadedPatch(session)).toContain("first"); + + writeFileSync(right, "export const value = 2;\nexport const second = true;\n"); + await session.open(source); + + expect(loadedPatch(session)).toContain("first"); + expect(loadedPatch(session)).not.toContain("second"); + + await session.reload(); + + expect(loadedPatch(session)).toContain("second"); + expect(loadedPatch(session)).not.toContain("first"); + + session.dispose(); + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + + test("keeps the previous source and reports errors when open fails", async () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-error-")); try { @@ -61,17 +126,49 @@ describe("embedded Hunk sessions", () => { source: initialSource, }); - await expect(session.load({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); + await expect(session.open({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); - expect(session.source).toEqual(initialSource); + expect(session.source).toMatchObject(initialSource); const snapshot = session.getSnapshot(); expect(snapshot.status).toBe("error"); if (snapshot.status !== "error") throw new Error("Expected embedded reload to fail."); expect(snapshot.error).toContain("missing.patch"); + expect(snapshot.title).toBe("Patch review: initial patch"); + expect("bootstrap" in snapshot).toBe(false); session.dispose(); } finally { rmSync(root, { force: true, recursive: true }); } }); + + test("scopes embedded key input to the active mount", () => { + const sourceListeners = new Map void>>(); + const source = { + on(event: string, listener: (...args: unknown[]) => void) { + const listeners = sourceListeners.get(event) ?? new Set(); + listeners.add(listener); + sourceListeners.set(event, listeners); + }, + off(event: string, listener: (...args: unknown[]) => void) { + sourceListeners.get(event)?.delete(listener); + }, + }; + let active = false; + const scoped = createScopedKeyInput(source, () => active); + const received: unknown[] = []; + + scoped.keyInput.on("keypress", (event: unknown) => { + received.push(event); + }); + + sourceListeners.get("keypress")?.forEach((listener) => listener("hidden")); + active = true; + sourceListeners.get("keypress")?.forEach((listener) => listener("visible")); + + expect(received).toEqual(["visible"]); + + scoped.dispose(); + expect(sourceListeners.get("keypress")?.size).toBe(0); + }); }); diff --git a/src/embedded/index.ts b/src/embedded/index.ts new file mode 100644 index 00000000..762ec3b5 --- /dev/null +++ b/src/embedded/index.ts @@ -0,0 +1,29 @@ +import { mountEmbeddedHunkApp as mountEmbeddedHunkAppImpl } from "./mount"; +import { createEmbeddedHunkSession as createEmbeddedHunkSessionImpl } from "./session"; +export type { + CreateEmbeddedHunkSessionInput, + EmbeddedHunkMount, + EmbeddedHunkOptions, + EmbeddedHunkSession, + EmbeddedHunkSnapshot, + EmbeddedHunkSource, + MountEmbeddedHunkAppInput, +} from "./types"; +import type { + CreateEmbeddedHunkSessionInput, + EmbeddedHunkMount, + EmbeddedHunkSession, + MountEmbeddedHunkAppInput, +} from "./types"; + +/** Create one embedded Hunk review session from a public embedded source. */ +export function createEmbeddedHunkSession( + input: CreateEmbeddedHunkSessionInput, +): Promise { + return createEmbeddedHunkSessionImpl(input); +} + +/** Mount one embedded Hunk app into a host-owned OpenTUI container. */ +export function mountEmbeddedHunkApp(input: MountEmbeddedHunkAppInput): EmbeddedHunkMount { + return mountEmbeddedHunkAppImpl(input); +} diff --git a/src/embedded/index.tsx b/src/embedded/index.tsx deleted file mode 100644 index 1eb9c511..00000000 --- a/src/embedded/index.tsx +++ /dev/null @@ -1,232 +0,0 @@ -import type { CliRenderer, Renderable } from "@opentui/core"; -import { createRoot } from "@opentui/react"; -import { useSyncExternalStore } from "react"; -import { resolveConfiguredCliInput } from "../core/config"; -import { loadAppBootstrap } from "../core/loaders"; -import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; -import { - createInitialSessionSnapshot, - createSessionRegistration, - updateSessionRegistration, -} from "../hunk-session/sessionRegistration"; -import type { HunkSessionBrokerClient } from "../hunk-session/types"; -import { SessionBrokerClient } from "../session-broker/brokerClient"; -import { AppHost } from "../ui/AppHost"; - -export type EmbeddedHunkSource = - | { kind: "worktree"; pathspecs?: string[]; options?: CommonOptions } - | { kind: "staged"; pathspecs?: string[]; options?: CommonOptions } - | { - kind: "vcs"; - range?: string; - staged: boolean; - pathspecs?: string[]; - options?: CommonOptions; - } - | { kind: "show"; ref?: string; pathspecs?: string[]; options?: CommonOptions } - | { kind: "stash-show"; ref?: string; options?: CommonOptions } - | { kind: "diff"; left: string; right: string; options?: CommonOptions } - | { kind: "patch"; file?: string; text?: string; label?: string; options?: CommonOptions } - | { kind: "difftool"; left: string; right: string; path?: string; options?: CommonOptions }; - -export type EmbeddedHunkSnapshot = - | { status: "loading"; bootstrap: AppBootstrap; error?: undefined } - | { status: "ready"; bootstrap: AppBootstrap; error?: undefined } - | { status: "error"; bootstrap: AppBootstrap; error: string }; - -export interface EmbeddedHunkSession { - readonly cwd: string; - readonly source: EmbeddedHunkSource; - getSnapshot(): EmbeddedHunkSnapshot; - load(source: EmbeddedHunkSource): Promise; - subscribe(listener: () => void): () => void; - dispose(): void; -} - -export interface EmbeddedHunkMount { - update(options: { active: boolean; onQuit: () => void }): void; - unmount(): void; -} - -function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { - const options = source.options ?? {}; - - switch (source.kind) { - case "worktree": - return { - kind: "vcs", - staged: false, - pathspecs: source.pathspecs, - options, - }; - case "staged": - return { - kind: "vcs", - staged: true, - pathspecs: source.pathspecs, - options, - }; - case "patch": - return { - kind: "patch", - text: source.text, - file: source.file ?? source.label, - options, - }; - default: - return { ...source, options } as CliInput; - } -} - -function resolveEmbeddedCliInput(source: EmbeddedHunkSource, cwd: string) { - return resolveConfiguredCliInput(embeddedSourceToCliInput(source), { cwd }).input; -} - -function errorMessage(error: unknown) { - if (error instanceof Error && error.message) return error.message; - return String(error || "Failed to load Hunk."); -} - -class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { - private listeners = new Set<() => void>(); - private disposed = false; - private snapshot: EmbeddedHunkSnapshot; - - readonly hostClient: HunkSessionBrokerClient; - - constructor( - readonly cwd: string, - public source: EmbeddedHunkSource, - bootstrap: AppBootstrap, - ) { - this.snapshot = { status: "ready", bootstrap }; - this.hostClient = new SessionBrokerClient( - createSessionRegistration(bootstrap, { cwd }), - createInitialSessionSnapshot(bootstrap), - ); - this.hostClient.start(); - } - - getSnapshot = () => this.snapshot; - - subscribe = (listener: () => void) => { - this.listeners.add(listener); - return () => this.listeners.delete(listener); - }; - - async load(source: EmbeddedHunkSource) { - if (this.disposed) return; - this.setSnapshot({ status: "loading", bootstrap: this.snapshot.bootstrap }); - - try { - const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, this.cwd), { - cwd: this.cwd, - }); - this.source = source; - this.hostClient.replaceSession( - updateSessionRegistration(this.hostClient.getRegistration(), bootstrap), - createInitialSessionSnapshot(bootstrap), - ); - this.setSnapshot({ status: "ready", bootstrap }); - } catch (error) { - const message = errorMessage(error); - this.setSnapshot({ - status: "error", - bootstrap: this.snapshot.bootstrap, - error: message, - }); - throw error instanceof Error ? error : new Error(message); - } - } - - dispose() { - this.disposed = true; - this.hostClient.stop(); - this.listeners.clear(); - } - - private setSnapshot(snapshot: EmbeddedHunkSnapshot) { - this.snapshot = snapshot; - for (const listener of this.listeners) listener(); - } -} - -/** Resolve the internal broker client owned by sessions created through this entrypoint. */ -function sessionHostClient(session: EmbeddedHunkSession) { - if (session instanceof EmbeddedHunkSessionImpl) { - return session.hostClient; - } - throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); -} - -function EmbeddedHunkRoot({ - active, - onQuit, - session, -}: { - active: boolean; - onQuit: () => void; - session: EmbeddedHunkSession; -}) { - const snapshot = useSyncExternalStore( - session.subscribe, - session.getSnapshot, - session.getSnapshot, - ); - - return ( - null} - /> - ); -} - -function scopedRenderer(renderer: CliRenderer, root: Renderable) { - const scoped = Object.create(renderer) as CliRenderer; - Object.defineProperty(scoped, "root", { value: root }); - return scoped; -} - -export async function createEmbeddedHunkSession({ - cwd = process.cwd(), - source, -}: { - cwd?: string; - source: EmbeddedHunkSource; -}): Promise { - const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, cwd), { cwd }); - return new EmbeddedHunkSessionImpl(cwd, source, bootstrap); -} - -export function mountEmbeddedHunkApp({ - active, - container, - onQuit, - renderer, - session, -}: { - active: boolean; - container: Renderable; - onQuit: () => void; - renderer: CliRenderer; - session: EmbeddedHunkSession; -}): EmbeddedHunkMount { - const root = createRoot(scopedRenderer(renderer, container)); - - const render = (next: { active: boolean; onQuit: () => void }) => { - root.render(); - }; - - render({ active, onQuit }); - - return { - update: render, - unmount() { - root.unmount(); - }, - }; -} diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx new file mode 100644 index 00000000..22897f6b --- /dev/null +++ b/src/embedded/mount.tsx @@ -0,0 +1,142 @@ +import type { CliRenderer, Renderable } from "@opentui/core"; +import { createRoot } from "@opentui/react"; +import { useSyncExternalStore } from "react"; +import { AppHost } from "../ui/AppHost"; +import { embeddedHunkSessionInternals } from "./session"; +import type { EmbeddedHunkMount, EmbeddedHunkSession, MountEmbeddedHunkAppInput } from "./types"; + +const scopedKeyInputEvents = ["keypress", "keyrelease", "paste"] as const; + +type ScopedKeyInputEvent = (typeof scopedKeyInputEvents)[number]; +type KeyInputListener = (...args: unknown[]) => void; +type KeyInputSource = Readonly<{ + on: (event: string, listener: KeyInputListener) => unknown; + off: (event: string, listener: KeyInputListener) => unknown; +}>; + +/** Return whether one renderer input event should be visibility-scoped for embedded Hunk. */ +function isScopedKeyInputEvent(event: string): event is ScopedKeyInputEvent { + return scopedKeyInputEvents.includes(event as ScopedKeyInputEvent); +} + +/** Scope Hunk keyboard and paste listeners so inactive embedded mounts stay alive but quiet. */ +export function createScopedKeyInput(source: KeyInputSource, enabled: () => boolean) { + const listeners = new Map>(); + const forwarders = new Map(); + + for (const event of scopedKeyInputEvents) { + listeners.set(event, new Set()); + forwarders.set(event, (...args: unknown[]) => { + if (!enabled()) return; + for (const listener of listeners.get(event) ?? []) listener(...args); + }); + } + + const scoped = { + on(event: string, listener: KeyInputListener) { + if (!isScopedKeyInputEvent(event)) { + source.on(event, listener); + return scoped; + } + + const eventListeners = listeners.get(event)!; + if (eventListeners.size === 0) source.on(event, forwarders.get(event)!); + eventListeners.add(listener); + return scoped; + }, + off(event: string, listener: KeyInputListener) { + if (!isScopedKeyInputEvent(event)) { + source.off(event, listener); + return scoped; + } + + const eventListeners = listeners.get(event)!; + eventListeners.delete(listener); + if (eventListeners.size === 0) source.off(event, forwarders.get(event)!); + return scoped; + }, + }; + + return { + keyInput: scoped as CliRenderer["keyInput"], + dispose() { + for (const event of scopedKeyInputEvents) { + const forwarder = forwarders.get(event); + if (forwarder) source.off(event, forwarder); + listeners.get(event)?.clear(); + } + }, + }; +} + +/** Scope the renderer root and input stream to the host-provided embedded container. */ +function scopedRenderer( + renderer: CliRenderer, + root: Renderable, + keyInput: CliRenderer["keyInput"], +) { + const scoped = Object.create(renderer) as CliRenderer; + Object.defineProperty(scoped, "root", { value: root }); + Object.defineProperty(scoped, "keyInput", { value: keyInput }); + Object.defineProperty(scoped, "intermediateRender", { + value() { + if (!renderer.isDestroyed) renderer.requestRender(); + }, + }); + return scoped; +} + +function EmbeddedHunkRoot({ + active, + onQuit, + session, +}: { + active: boolean; + onQuit: () => void; + session: EmbeddedHunkSession; +}) { + const internals = embeddedHunkSessionInternals(session); + const snapshot = useSyncExternalStore( + session.subscribe, + internals.getRenderSnapshot, + internals.getRenderSnapshot, + ); + + return ( + null} + /> + ); +} + +/** Mount one embedded Hunk app into a host-owned OpenTUI container. */ +export function mountEmbeddedHunkApp({ + active, + container, + onQuit, + renderer, + session, +}: MountEmbeddedHunkAppInput): EmbeddedHunkMount { + let currentActive = active; + const scopedKeyInput = createScopedKeyInput(renderer.keyInput, () => currentActive); + const root = createRoot(scopedRenderer(renderer, container, scopedKeyInput.keyInput)); + + const render = (next: { active: boolean; onQuit: () => void }) => { + currentActive = next.active; + root.render(); + }; + + render({ active, onQuit }); + + return { + update: render, + unmount() { + root.unmount(); + scopedKeyInput.dispose(); + }, + }; +} diff --git a/src/embedded/session.ts b/src/embedded/session.ts new file mode 100644 index 00000000..acd2fd4c --- /dev/null +++ b/src/embedded/session.ts @@ -0,0 +1,162 @@ +import { loadAppBootstrap } from "../core/loaders"; +import type { AppBootstrap } from "../core/types"; +import { + createInitialSessionSnapshot, + createSessionRegistration, + updateSessionRegistration, +} from "../hunk-session/sessionRegistration"; +import type { HunkSessionBrokerClient } from "../hunk-session/types"; +import { SessionBrokerClient } from "../session-broker/brokerClient"; +import { + embeddedHunkSourcesEqual, + normalizeEmbeddedHunkSource, + resolveEmbeddedCliInput, +} from "./source"; +import type { + CreateEmbeddedHunkSessionInput, + EmbeddedHunkSession, + EmbeddedHunkSnapshot, + EmbeddedHunkSource, +} from "./types"; + +export type EmbeddedHunkRenderSnapshot = + | { status: "loading"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error?: undefined } + | { status: "ready"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error?: undefined } + | { status: "error"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error: string }; + +/** Convert unknown thrown values into stable user-facing error text. */ +function errorMessage(error: unknown) { + if (error instanceof Error && error.message) return error.message; + return String(error || "Failed to load Hunk."); +} + +/** Build the host-facing embedded snapshot without exposing app bootstrap internals. */ +function publicSnapshot(snapshot: EmbeddedHunkRenderSnapshot): EmbeddedHunkSnapshot { + switch (snapshot.status) { + case "loading": + return { status: "loading", source: snapshot.source }; + case "ready": + return { + status: "ready", + source: snapshot.source, + title: snapshot.bootstrap.changeset.title, + fileCount: snapshot.bootstrap.changeset.files.length, + }; + case "error": + return { + status: "error", + source: snapshot.source, + title: snapshot.bootstrap.changeset.title, + fileCount: snapshot.bootstrap.changeset.files.length, + error: snapshot.error, + }; + } +} + +/** Own one embedded Hunk review session, including source identity and broker registration. */ +class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { + private listeners = new Set<() => void>(); + private disposed = false; + private renderSnapshot: EmbeddedHunkRenderSnapshot; + private snapshot: EmbeddedHunkSnapshot; + + readonly hostClient: HunkSessionBrokerClient; + + constructor( + readonly cwd: string, + public source: EmbeddedHunkSource, + bootstrap: AppBootstrap, + ) { + this.renderSnapshot = { status: "ready", source, bootstrap }; + this.snapshot = publicSnapshot(this.renderSnapshot); + this.hostClient = new SessionBrokerClient( + createSessionRegistration(bootstrap, { cwd }), + createInitialSessionSnapshot(bootstrap), + ); + this.hostClient.start(); + } + + getSnapshot = () => this.snapshot; + + getRenderSnapshot = () => this.renderSnapshot; + + subscribe = (listener: () => void) => { + this.listeners.add(listener); + return () => this.listeners.delete(listener); + }; + + async open(source: EmbeddedHunkSource) { + const nextSource = normalizeEmbeddedHunkSource(source); + if (this.disposed || embeddedHunkSourcesEqual(this.source, nextSource)) return; + await this.load(nextSource, { updateSource: true }); + } + + async reload() { + if (this.disposed) return; + await this.load(this.source, { updateSource: false }); + } + + dispose() { + this.disposed = true; + this.hostClient.stop(); + this.listeners.clear(); + } + + private async load(source: EmbeddedHunkSource, { updateSource }: { updateSource: boolean }) { + this.setRenderSnapshot({ + status: "loading", + source: this.source, + bootstrap: this.renderSnapshot.bootstrap, + }); + + try { + const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, this.cwd), { + cwd: this.cwd, + }); + if (updateSource) { + this.source = source; + } + this.hostClient.replaceSession( + updateSessionRegistration(this.hostClient.getRegistration(), bootstrap), + createInitialSessionSnapshot(bootstrap), + ); + this.setRenderSnapshot({ status: "ready", source: this.source, bootstrap }); + } catch (error) { + const message = errorMessage(error); + this.setRenderSnapshot({ + status: "error", + source: this.source, + bootstrap: this.renderSnapshot.bootstrap, + error: message, + }); + throw error instanceof Error ? error : new Error(message); + } + } + + private setRenderSnapshot(snapshot: EmbeddedHunkRenderSnapshot) { + this.renderSnapshot = snapshot; + this.snapshot = publicSnapshot(snapshot); + for (const listener of this.listeners) listener(); + } +} + +/** Resolve private render state for sessions created by this embedded entrypoint. */ +export function embeddedHunkSessionInternals(session: EmbeddedHunkSession) { + if (session instanceof EmbeddedHunkSessionImpl) { + return { + getRenderSnapshot: session.getRenderSnapshot, + hostClient: session.hostClient, + }; + } + throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); +} + +/** Create one embedded Hunk review session from a public embedded source. */ +export async function createEmbeddedHunkSession({ + cwd = process.cwd(), + source, +}: CreateEmbeddedHunkSessionInput): Promise { + const normalizedSource = normalizeEmbeddedHunkSource(source); + const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(normalizedSource, cwd), { cwd }); + return new EmbeddedHunkSessionImpl(cwd, normalizedSource, bootstrap); +} diff --git a/src/embedded/source.ts b/src/embedded/source.ts new file mode 100644 index 00000000..cc423ebc --- /dev/null +++ b/src/embedded/source.ts @@ -0,0 +1,103 @@ +import { isDeepStrictEqual } from "node:util"; +import { resolveConfiguredCliInput } from "../core/config"; +import type { CliInput, CommonOptions } from "../core/types"; +import type { EmbeddedHunkOptions, EmbeddedHunkSource } from "./types"; + +/** Return a defensive copy of embedded options. */ +function normalizeOptions(options: EmbeddedHunkOptions | undefined): CommonOptions { + return options ? { ...options } : {}; +} + +/** Return a copy of optional pathspecs so source identity cannot be mutated externally. */ +function normalizePathspecs(pathspecs: string[] | undefined) { + return pathspecs ? [...pathspecs] : undefined; +} + +/** Normalize one embedded source into the canonical shape Hunk stores on a session. */ +export function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): EmbeddedHunkSource { + const options = normalizeOptions(source.options); + + switch (source.kind) { + case "worktree": + return { kind: "worktree", pathspecs: normalizePathspecs(source.pathspecs), options }; + case "staged": + return { kind: "staged", pathspecs: normalizePathspecs(source.pathspecs), options }; + case "vcs": + return { + kind: "vcs", + range: source.range, + staged: source.staged, + pathspecs: normalizePathspecs(source.pathspecs), + options, + }; + case "show": + return { + kind: "show", + ref: source.ref, + pathspecs: normalizePathspecs(source.pathspecs), + options, + }; + case "stash-show": + return { kind: "stash-show", ref: source.ref, options }; + case "diff": + return { kind: "diff", left: source.left, right: source.right, options }; + case "patch": + return { + kind: "patch", + file: source.file, + text: source.text, + label: source.label, + options, + }; + case "difftool": + return { + kind: "difftool", + left: source.left, + right: source.right, + path: source.path, + options, + }; + } +} + +/** Adapt a public embedded source into the internal CLI input pipeline. */ +export function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { + const normalized = normalizeEmbeddedHunkSource(source); + const options = normalized.options ?? {}; + + switch (normalized.kind) { + case "worktree": + return { + kind: "vcs", + staged: false, + pathspecs: normalized.pathspecs, + options, + }; + case "staged": + return { + kind: "vcs", + staged: true, + pathspecs: normalized.pathspecs, + options, + }; + case "patch": + return { + kind: "patch", + text: normalized.text, + file: normalized.file ?? normalized.label, + options, + }; + default: + return { ...normalized, options } as CliInput; + } +} + +/** Resolve embedded input through the same config layers as the CLI. */ +export function resolveEmbeddedCliInput(source: EmbeddedHunkSource, cwd: string) { + return resolveConfiguredCliInput(embeddedSourceToCliInput(source), { cwd }).input; +} + +/** Return whether two embedded sources resolve to the same review identity. */ +export function embeddedHunkSourcesEqual(left: EmbeddedHunkSource, right: EmbeddedHunkSource) { + return isDeepStrictEqual(normalizeEmbeddedHunkSource(left), normalizeEmbeddedHunkSource(right)); +} diff --git a/src/embedded/index.d.ts b/src/embedded/types.ts similarity index 65% rename from src/embedded/index.d.ts rename to src/embedded/types.ts index 113bc3a2..be768758 100644 --- a/src/embedded/index.d.ts +++ b/src/embedded/types.ts @@ -25,31 +25,38 @@ export type EmbeddedHunkSource = | { kind: "show"; ref?: string; pathspecs?: string[]; options?: EmbeddedHunkOptions } | { kind: "stash-show"; ref?: string; options?: EmbeddedHunkOptions } | { kind: "diff"; left: string; right: string; options?: EmbeddedHunkOptions } + | { kind: "patch"; file?: string; text?: string; label?: string; options?: EmbeddedHunkOptions } + | { kind: "difftool"; left: string; right: string; path?: string; options?: EmbeddedHunkOptions }; + +export type EmbeddedHunkSnapshot = | { - kind: "patch"; - file?: string; - text?: string; - label?: string; - options?: EmbeddedHunkOptions; + status: "loading"; + source: EmbeddedHunkSource; + error?: undefined; + fileCount?: undefined; + title?: undefined; } | { - kind: "difftool"; - left: string; - right: string; - path?: string; - options?: EmbeddedHunkOptions; + status: "ready"; + source: EmbeddedHunkSource; + error?: undefined; + fileCount: number; + title: string; + } + | { + status: "error"; + source: EmbeddedHunkSource; + error: string; + fileCount: number; + title: string; }; -export type EmbeddedHunkSnapshot = - | { status: "loading"; bootstrap: unknown; error?: undefined } - | { status: "ready"; bootstrap: unknown; error?: undefined } - | { status: "error"; bootstrap: unknown; error: string }; - export interface EmbeddedHunkSession { readonly cwd: string; readonly source: EmbeddedHunkSource; getSnapshot(): EmbeddedHunkSnapshot; - load(source: EmbeddedHunkSource): Promise; + open(source: EmbeddedHunkSource): Promise; + reload(): Promise; subscribe(listener: () => void): () => void; dispose(): void; } @@ -59,14 +66,15 @@ export interface EmbeddedHunkMount { unmount(): void; } -export declare function createEmbeddedHunkSession(input: { +export interface CreateEmbeddedHunkSessionInput { cwd?: string; source: EmbeddedHunkSource; -}): Promise; -export declare function mountEmbeddedHunkApp(input: { +} + +export interface MountEmbeddedHunkAppInput { active: boolean; container: Renderable; onQuit: () => void; renderer: CliRenderer; session: EmbeddedHunkSession; -}): EmbeddedHunkMount; +} diff --git a/tsconfig.opentui.json b/tsconfig.npm-exports.json similarity index 70% rename from tsconfig.opentui.json rename to tsconfig.npm-exports.json index e1b363a3..33044a77 100644 --- a/tsconfig.opentui.json +++ b/tsconfig.npm-exports.json @@ -5,8 +5,8 @@ "declaration": true, "emitDeclarationOnly": true, "outDir": "./dist/npm-types", - "rootDir": "./src" + "rootDir": "." }, "include": [], - "files": ["src/opentui/index.ts"] + "files": ["src/opentui/index.ts", "src/embedded/index.ts"] } From 6b9750b422a79a2b78d1889fbab56b783ef08563 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 02:59:41 -0400 Subject: [PATCH 04/16] refactor: refine embedded Hunk exports and input scoping --- src/embedded/index.ts | 11 +-- src/embedded/mount.tsx | 24 ++---- src/embedded/session.ts | 106 ++++++++++++++++-------- src/embedded/source.ts | 103 ----------------------- src/embedded/types.ts | 24 +----- src/ui/App.tsx | 3 - src/ui/AppHost.tsx | 3 - src/ui/hooks/useAppKeyboardShortcuts.ts | 8 -- 8 files changed, 88 insertions(+), 194 deletions(-) delete mode 100644 src/embedded/source.ts diff --git a/src/embedded/index.ts b/src/embedded/index.ts index 762ec3b5..a5cbab7b 100644 --- a/src/embedded/index.ts +++ b/src/embedded/index.ts @@ -17,13 +17,10 @@ import type { } from "./types"; /** Create one embedded Hunk review session from a public embedded source. */ -export function createEmbeddedHunkSession( +export const createEmbeddedHunkSession = ( input: CreateEmbeddedHunkSessionInput, -): Promise { - return createEmbeddedHunkSessionImpl(input); -} +): Promise => createEmbeddedHunkSessionImpl(input); /** Mount one embedded Hunk app into a host-owned OpenTUI container. */ -export function mountEmbeddedHunkApp(input: MountEmbeddedHunkAppInput): EmbeddedHunkMount { - return mountEmbeddedHunkAppImpl(input); -} +export const mountEmbeddedHunkApp = (input: MountEmbeddedHunkAppInput): EmbeddedHunkMount => + mountEmbeddedHunkAppImpl(input); diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index 22897f6b..79afed03 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -14,11 +14,6 @@ type KeyInputSource = Readonly<{ off: (event: string, listener: KeyInputListener) => unknown; }>; -/** Return whether one renderer input event should be visibility-scoped for embedded Hunk. */ -function isScopedKeyInputEvent(event: string): event is ScopedKeyInputEvent { - return scopedKeyInputEvents.includes(event as ScopedKeyInputEvent); -} - /** Scope Hunk keyboard and paste listeners so inactive embedded mounts stay alive but quiet. */ export function createScopedKeyInput(source: KeyInputSource, enabled: () => boolean) { const listeners = new Map>(); @@ -34,25 +29,27 @@ export function createScopedKeyInput(source: KeyInputSource, enabled: () => bool const scoped = { on(event: string, listener: KeyInputListener) { - if (!isScopedKeyInputEvent(event)) { + if (!scopedKeyInputEvents.includes(event as ScopedKeyInputEvent)) { source.on(event, listener); return scoped; } - const eventListeners = listeners.get(event)!; - if (eventListeners.size === 0) source.on(event, forwarders.get(event)!); + const scopedEvent = event as ScopedKeyInputEvent; + const eventListeners = listeners.get(scopedEvent)!; + if (eventListeners.size === 0) source.on(scopedEvent, forwarders.get(scopedEvent)!); eventListeners.add(listener); return scoped; }, off(event: string, listener: KeyInputListener) { - if (!isScopedKeyInputEvent(event)) { + if (!scopedKeyInputEvents.includes(event as ScopedKeyInputEvent)) { source.off(event, listener); return scoped; } - const eventListeners = listeners.get(event)!; + const scopedEvent = event as ScopedKeyInputEvent; + const eventListeners = listeners.get(scopedEvent)!; eventListeners.delete(listener); - if (eventListeners.size === 0) source.off(event, forwarders.get(event)!); + if (eventListeners.size === 0) source.off(scopedEvent, forwarders.get(scopedEvent)!); return scoped; }, }; @@ -87,11 +84,9 @@ function scopedRenderer( } function EmbeddedHunkRoot({ - active, onQuit, session, }: { - active: boolean; onQuit: () => void; session: EmbeddedHunkSession; }) { @@ -104,7 +99,6 @@ function EmbeddedHunkRoot({ return ( void }) => { currentActive = next.active; - root.render(); + root.render(); }; render({ active, onQuit }); diff --git a/src/embedded/session.ts b/src/embedded/session.ts index acd2fd4c..6744b57e 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -1,5 +1,7 @@ +import { isDeepStrictEqual } from "node:util"; +import { resolveConfiguredCliInput } from "../core/config"; import { loadAppBootstrap } from "../core/loaders"; -import type { AppBootstrap } from "../core/types"; +import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; import { createInitialSessionSnapshot, createSessionRegistration, @@ -7,11 +9,6 @@ import { } from "../hunk-session/sessionRegistration"; import type { HunkSessionBrokerClient } from "../hunk-session/types"; import { SessionBrokerClient } from "../session-broker/brokerClient"; -import { - embeddedHunkSourcesEqual, - normalizeEmbeddedHunkSource, - resolveEmbeddedCliInput, -} from "./source"; import type { CreateEmbeddedHunkSessionInput, EmbeddedHunkSession, @@ -20,9 +17,11 @@ import type { } from "./types"; export type EmbeddedHunkRenderSnapshot = - | { status: "loading"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error?: undefined } - | { status: "ready"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error?: undefined } - | { status: "error"; source: EmbeddedHunkSource; bootstrap: AppBootstrap; error: string }; + | { status: "loading"; bootstrap: AppBootstrap; error?: undefined } + | { status: "ready"; bootstrap: AppBootstrap; error?: undefined } + | { status: "error"; bootstrap: AppBootstrap; error: string }; + +type NormalizedEmbeddedHunkSource = EmbeddedHunkSource & { options: CommonOptions }; /** Convert unknown thrown values into stable user-facing error text. */ function errorMessage(error: unknown) { @@ -30,35 +29,76 @@ function errorMessage(error: unknown) { return String(error || "Failed to load Hunk."); } -/** Build the host-facing embedded snapshot without exposing app bootstrap internals. */ -function publicSnapshot(snapshot: EmbeddedHunkRenderSnapshot): EmbeddedHunkSnapshot { - switch (snapshot.status) { - case "loading": - return { status: "loading", source: snapshot.source }; - case "ready": +/** Return a session-owned source copy with normalized options and pathspec identity. */ +function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): NormalizedEmbeddedHunkSource { + const pathspecs = "pathspecs" in source ? source.pathspecs : undefined; + return { + ...source, + ...(pathspecs ? { pathspecs: [...pathspecs] } : {}), + options: { ...(source.options ?? {}) }, + } as NormalizedEmbeddedHunkSource; +} + +/** Adapt a public embedded source into the internal CLI input pipeline. */ +function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { + const normalized = normalizeEmbeddedHunkSource(source); + + switch (normalized.kind) { + case "worktree": return { - status: "ready", - source: snapshot.source, - title: snapshot.bootstrap.changeset.title, - fileCount: snapshot.bootstrap.changeset.files.length, + kind: "vcs", + staged: false, + pathspecs: normalized.pathspecs, + options: normalized.options, }; - case "error": + case "staged": return { - status: "error", - source: snapshot.source, - title: snapshot.bootstrap.changeset.title, - fileCount: snapshot.bootstrap.changeset.files.length, - error: snapshot.error, + kind: "vcs", + staged: true, + pathspecs: normalized.pathspecs, + options: normalized.options, + }; + case "patch": + return { + kind: "patch", + text: normalized.text, + file: normalized.file ?? normalized.label, + options: normalized.options, }; + default: + return normalized as CliInput; } } +/** Resolve embedded input through the same config layers as the CLI. */ +function resolveEmbeddedCliInput(source: EmbeddedHunkSource, cwd: string) { + return resolveConfiguredCliInput(embeddedSourceToCliInput(source), { cwd }).input; +} + +/** Build the host-facing embedded snapshot without exposing app bootstrap internals. */ +function publicSnapshot( + source: EmbeddedHunkSource, + snapshot: EmbeddedHunkRenderSnapshot, +): EmbeddedHunkSnapshot { + if (snapshot.status === "loading") { + return { status: "loading", source }; + } + + const base = { + source, + title: snapshot.bootstrap.changeset.title, + fileCount: snapshot.bootstrap.changeset.files.length, + }; + return snapshot.status === "error" + ? { ...base, status: "error", error: snapshot.error } + : { ...base, status: "ready" }; +} + /** Own one embedded Hunk review session, including source identity and broker registration. */ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); private disposed = false; private renderSnapshot: EmbeddedHunkRenderSnapshot; - private snapshot: EmbeddedHunkSnapshot; readonly hostClient: HunkSessionBrokerClient; @@ -67,8 +107,7 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { public source: EmbeddedHunkSource, bootstrap: AppBootstrap, ) { - this.renderSnapshot = { status: "ready", source, bootstrap }; - this.snapshot = publicSnapshot(this.renderSnapshot); + this.renderSnapshot = { status: "ready", bootstrap }; this.hostClient = new SessionBrokerClient( createSessionRegistration(bootstrap, { cwd }), createInitialSessionSnapshot(bootstrap), @@ -76,7 +115,7 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.hostClient.start(); } - getSnapshot = () => this.snapshot; + getSnapshot = () => publicSnapshot(this.source, this.renderSnapshot); getRenderSnapshot = () => this.renderSnapshot; @@ -87,7 +126,9 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { async open(source: EmbeddedHunkSource) { const nextSource = normalizeEmbeddedHunkSource(source); - if (this.disposed || embeddedHunkSourcesEqual(this.source, nextSource)) return; + if (this.disposed || isDeepStrictEqual(normalizeEmbeddedHunkSource(this.source), nextSource)) { + return; + } await this.load(nextSource, { updateSource: true }); } @@ -105,7 +146,6 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private async load(source: EmbeddedHunkSource, { updateSource }: { updateSource: boolean }) { this.setRenderSnapshot({ status: "loading", - source: this.source, bootstrap: this.renderSnapshot.bootstrap, }); @@ -120,12 +160,11 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { updateSessionRegistration(this.hostClient.getRegistration(), bootstrap), createInitialSessionSnapshot(bootstrap), ); - this.setRenderSnapshot({ status: "ready", source: this.source, bootstrap }); + this.setRenderSnapshot({ status: "ready", bootstrap }); } catch (error) { const message = errorMessage(error); this.setRenderSnapshot({ status: "error", - source: this.source, bootstrap: this.renderSnapshot.bootstrap, error: message, }); @@ -135,7 +174,6 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private setRenderSnapshot(snapshot: EmbeddedHunkRenderSnapshot) { this.renderSnapshot = snapshot; - this.snapshot = publicSnapshot(snapshot); for (const listener of this.listeners) listener(); } } diff --git a/src/embedded/source.ts b/src/embedded/source.ts deleted file mode 100644 index cc423ebc..00000000 --- a/src/embedded/source.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { isDeepStrictEqual } from "node:util"; -import { resolveConfiguredCliInput } from "../core/config"; -import type { CliInput, CommonOptions } from "../core/types"; -import type { EmbeddedHunkOptions, EmbeddedHunkSource } from "./types"; - -/** Return a defensive copy of embedded options. */ -function normalizeOptions(options: EmbeddedHunkOptions | undefined): CommonOptions { - return options ? { ...options } : {}; -} - -/** Return a copy of optional pathspecs so source identity cannot be mutated externally. */ -function normalizePathspecs(pathspecs: string[] | undefined) { - return pathspecs ? [...pathspecs] : undefined; -} - -/** Normalize one embedded source into the canonical shape Hunk stores on a session. */ -export function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): EmbeddedHunkSource { - const options = normalizeOptions(source.options); - - switch (source.kind) { - case "worktree": - return { kind: "worktree", pathspecs: normalizePathspecs(source.pathspecs), options }; - case "staged": - return { kind: "staged", pathspecs: normalizePathspecs(source.pathspecs), options }; - case "vcs": - return { - kind: "vcs", - range: source.range, - staged: source.staged, - pathspecs: normalizePathspecs(source.pathspecs), - options, - }; - case "show": - return { - kind: "show", - ref: source.ref, - pathspecs: normalizePathspecs(source.pathspecs), - options, - }; - case "stash-show": - return { kind: "stash-show", ref: source.ref, options }; - case "diff": - return { kind: "diff", left: source.left, right: source.right, options }; - case "patch": - return { - kind: "patch", - file: source.file, - text: source.text, - label: source.label, - options, - }; - case "difftool": - return { - kind: "difftool", - left: source.left, - right: source.right, - path: source.path, - options, - }; - } -} - -/** Adapt a public embedded source into the internal CLI input pipeline. */ -export function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { - const normalized = normalizeEmbeddedHunkSource(source); - const options = normalized.options ?? {}; - - switch (normalized.kind) { - case "worktree": - return { - kind: "vcs", - staged: false, - pathspecs: normalized.pathspecs, - options, - }; - case "staged": - return { - kind: "vcs", - staged: true, - pathspecs: normalized.pathspecs, - options, - }; - case "patch": - return { - kind: "patch", - text: normalized.text, - file: normalized.file ?? normalized.label, - options, - }; - default: - return { ...normalized, options } as CliInput; - } -} - -/** Resolve embedded input through the same config layers as the CLI. */ -export function resolveEmbeddedCliInput(source: EmbeddedHunkSource, cwd: string) { - return resolveConfiguredCliInput(embeddedSourceToCliInput(source), { cwd }).input; -} - -/** Return whether two embedded sources resolve to the same review identity. */ -export function embeddedHunkSourcesEqual(left: EmbeddedHunkSource, right: EmbeddedHunkSource) { - return isDeepStrictEqual(normalizeEmbeddedHunkSource(left), normalizeEmbeddedHunkSource(right)); -} diff --git a/src/embedded/types.ts b/src/embedded/types.ts index be768758..21a8dbac 100644 --- a/src/embedded/types.ts +++ b/src/embedded/types.ts @@ -29,27 +29,9 @@ export type EmbeddedHunkSource = | { kind: "difftool"; left: string; right: string; path?: string; options?: EmbeddedHunkOptions }; export type EmbeddedHunkSnapshot = - | { - status: "loading"; - source: EmbeddedHunkSource; - error?: undefined; - fileCount?: undefined; - title?: undefined; - } - | { - status: "ready"; - source: EmbeddedHunkSource; - error?: undefined; - fileCount: number; - title: string; - } - | { - status: "error"; - source: EmbeddedHunkSource; - error: string; - fileCount: number; - title: string; - }; + | { status: "loading"; source: EmbeddedHunkSource } + | { status: "ready"; source: EmbeddedHunkSource; fileCount: number; title: string } + | { status: "error"; source: EmbeddedHunkSource; fileCount: number; title: string; error: string }; export interface EmbeddedHunkSession { readonly cwd: string; diff --git a/src/ui/App.tsx b/src/ui/App.tsx index a6c194ec..1bc36ce0 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -75,14 +75,12 @@ function withCurrentViewOptions( /** Orchestrate global app state, layout, navigation, and pane coordination. */ export function App({ - active = true, bootstrap, hostClient, noticeText, onQuit = () => process.exit(0), onReloadSession, }: { - active?: boolean; bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; noticeText?: string | null; @@ -631,7 +629,6 @@ export function App({ } = useMenuController(menus); useAppKeyboardShortcuts({ - active, activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index 37970796..ed4fe1ad 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -14,13 +14,11 @@ import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; /** Keep one live Hunk app mounted while allowing daemon-driven session reloads. */ export function AppHost({ - active = true, bootstrap, hostClient, onQuit = () => process.exit(0), startupNoticeResolver, }: { - active?: boolean; bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; onQuit?: () => void; @@ -94,7 +92,6 @@ export function AppHost({ return ( void; canRefreshCurrentInput: boolean; @@ -78,7 +77,6 @@ export interface UseAppKeyboardShortcutsOptions { /** Register the app's scoped keyboard handling while keeping mode precedence explicit. */ export function useAppKeyboardShortcuts({ - active = true, activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, @@ -112,13 +110,11 @@ export function useAppKeyboardShortcuts({ triggerEditSelectedFile, triggerRefreshCurrentInput, }: UseAppKeyboardShortcutsOptions) { - const activeRef = useRef(active); const activeMenuIdRef = useRef(activeMenuId); const focusAreaRef = useRef(focusArea); const pagerModeRef = useRef(pagerMode); const showHelpRef = useRef(showHelp); - activeRef.current = active; activeMenuIdRef.current = activeMenuId; focusAreaRef.current = focusArea; pagerModeRef.current = pagerMode; @@ -491,10 +487,6 @@ export function useAppKeyboardShortcuts({ }; useKeyboard((key: KeyEvent) => { - if (!activeRef.current) { - return; - } - if (handleMenuToggleShortcut(key)) { return; } From d61aa0f26438c09d315763b252351b3039eba89c Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 03:39:47 -0400 Subject: [PATCH 05/16] feat(embedded): launch broker from bundled Hunk binary Add an embedded session broker availability adapter so embedded sessions start the daemon through the package-provided Hunk CLI. Return snapshots from embedded session open and reload calls so callers can observe reused or refreshed review state directly. --- src/embedded/daemon.test.ts | 36 ++++++++++++++++++++ src/embedded/daemon.ts | 45 +++++++++++++++++++++++++ src/embedded/embedded.test.ts | 10 ++++-- src/embedded/session.ts | 22 ++++++++---- src/embedded/types.ts | 15 +++++++-- src/session-broker/brokerClient.test.ts | 28 +++++++++++++++ src/session-broker/brokerClient.ts | 25 +++++++++----- 7 files changed, 162 insertions(+), 19 deletions(-) create mode 100644 src/embedded/daemon.test.ts create mode 100644 src/embedded/daemon.ts diff --git a/src/embedded/daemon.test.ts b/src/embedded/daemon.test.ts new file mode 100644 index 00000000..5820123f --- /dev/null +++ b/src/embedded/daemon.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, test } from "bun:test"; +import { createEmbeddedSessionBrokerAvailability } from "./daemon"; +import type { EnsureSessionBrokerAvailableOptions } from "../session-broker/brokerLauncher"; + +const testConfig = { + host: "127.0.0.1", + port: 47657, + httpOrigin: "http://127.0.0.1:47657", + wsOrigin: "ws://127.0.0.1:47657", +}; + +describe("embedded session broker daemon launcher", () => { + test("passes Hunk package-bin launch options through the broker availability adapter", async () => { + let captured: EnsureSessionBrokerAvailableOptions | undefined; + const ensureBroker = createEmbeddedSessionBrokerAvailability({ + cwd: "/repo", + env: { HUNK_MCP_PORT: "48658" }, + hunkCliPath: "/deps/hunkdiff/bin/hunk.cjs", + timeoutMs: 1234, + ensureAvailable: async (options) => { + captured = options; + }, + }); + + await ensureBroker(testConfig); + + expect(captured).toEqual({ + argv: ["/deps/hunkdiff/bin/hunk.cjs"], + config: testConfig, + cwd: "/repo", + env: { HUNK_MCP_PORT: "48658" }, + execPath: "/deps/hunkdiff/bin/hunk.cjs", + timeoutMs: 1234, + }); + }); +}); diff --git a/src/embedded/daemon.ts b/src/embedded/daemon.ts new file mode 100644 index 00000000..c3c23e26 --- /dev/null +++ b/src/embedded/daemon.ts @@ -0,0 +1,45 @@ +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; +import { + ensureSessionBrokerAvailable, + type EnsureSessionBrokerAvailableOptions, +} from "../session-broker/brokerLauncher"; +import type { ResolvedSessionBrokerConfig } from "../session-broker/brokerConfig"; +import type { EnsureSessionBrokerAdapter } from "../session-broker/brokerClient"; + +const require = createRequire(import.meta.url); + +type EmbeddedEnsureSessionBroker = typeof ensureSessionBrokerAvailable; + +export interface EmbeddedSessionBrokerAvailabilityOptions { + cwd: string; + env?: NodeJS.ProcessEnv; + ensureAvailable?: EmbeddedEnsureSessionBroker; + hunkCliPath?: string; + timeoutMs?: number; +} + +/** Create the embedded broker availability adapter used by embedded Hunk sessions. */ +export function createEmbeddedSessionBrokerAvailability({ + cwd, + env = process.env, + ensureAvailable = ensureSessionBrokerAvailable, + hunkCliPath = join(dirname(require.resolve("hunkdiff/package.json")), "bin", "hunk.cjs"), + timeoutMs, +}: EmbeddedSessionBrokerAvailabilityOptions): EnsureSessionBrokerAdapter { + return (config: ResolvedSessionBrokerConfig) => { + const options: EnsureSessionBrokerAvailableOptions = { + argv: [hunkCliPath], + config, + cwd, + env, + execPath: hunkCliPath, + }; + + if (timeoutMs !== undefined) { + options.timeoutMs = timeoutMs; + } + + return ensureAvailable(options); + }; +} diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 94f55579..eeef8d31 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -100,13 +100,19 @@ describe("embedded Hunk sessions", () => { expect(loadedPatch(session)).toContain("first"); writeFileSync(right, "export const value = 2;\nexport const second = true;\n"); - await session.open(source); + const reusedSnapshot = await session.open(source); + expect(reusedSnapshot.status).toBe("ready"); + if (reusedSnapshot.status !== "ready") throw new Error("Expected reused snapshot."); + expect(reusedSnapshot.source).toEqual(session.source); expect(loadedPatch(session)).toContain("first"); expect(loadedPatch(session)).not.toContain("second"); - await session.reload(); + const reloadedSnapshot = await session.reload(); + expect(reloadedSnapshot.status).toBe("ready"); + if (reloadedSnapshot.status !== "ready") throw new Error("Expected reloaded snapshot."); + expect(reloadedSnapshot.source).toEqual(session.source); expect(loadedPatch(session)).toContain("second"); expect(loadedPatch(session)).not.toContain("first"); diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 6744b57e..14fd9ad1 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -9,6 +9,7 @@ import { } from "../hunk-session/sessionRegistration"; import type { HunkSessionBrokerClient } from "../hunk-session/types"; import { SessionBrokerClient } from "../session-broker/brokerClient"; +import { createEmbeddedSessionBrokerAvailability } from "./daemon"; import type { CreateEmbeddedHunkSessionInput, EmbeddedHunkSession, @@ -35,7 +36,7 @@ function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): NormalizedEmbe return { ...source, ...(pathspecs ? { pathspecs: [...pathspecs] } : {}), - options: { ...(source.options ?? {}) }, + options: { ...source.options }, } as NormalizedEmbeddedHunkSource; } @@ -111,6 +112,9 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.hostClient = new SessionBrokerClient( createSessionRegistration(bootstrap, { cwd }), createInitialSessionSnapshot(bootstrap), + { + ensureBrokerAvailable: createEmbeddedSessionBrokerAvailability({ cwd }), + }, ); this.hostClient.start(); } @@ -124,17 +128,19 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { return () => this.listeners.delete(listener); }; + /** Open a source idempotently; callers can re-open without learning source identity rules. */ async open(source: EmbeddedHunkSource) { const nextSource = normalizeEmbeddedHunkSource(source); if (this.disposed || isDeepStrictEqual(normalizeEmbeddedHunkSource(this.source), nextSource)) { - return; + return this.getSnapshot(); } - await this.load(nextSource, { updateSource: true }); + return this.load(nextSource, { updateSource: true }); } + /** Reload the currently loaded source, preserving source identity for the host. */ async reload() { - if (this.disposed) return; - await this.load(this.source, { updateSource: false }); + if (this.disposed) return this.getSnapshot(); + return this.load(this.source, { updateSource: false }); } dispose() { @@ -143,7 +149,10 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.listeners.clear(); } - private async load(source: EmbeddedHunkSource, { updateSource }: { updateSource: boolean }) { + private async load( + source: EmbeddedHunkSource, + { updateSource }: { updateSource: boolean }, + ): Promise { this.setRenderSnapshot({ status: "loading", bootstrap: this.renderSnapshot.bootstrap, @@ -161,6 +170,7 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { createInitialSessionSnapshot(bootstrap), ); this.setRenderSnapshot({ status: "ready", bootstrap }); + return this.getSnapshot(); } catch (error) { const message = errorMessage(error); this.setRenderSnapshot({ diff --git a/src/embedded/types.ts b/src/embedded/types.ts index 21a8dbac..3e5b6841 100644 --- a/src/embedded/types.ts +++ b/src/embedded/types.ts @@ -31,14 +31,23 @@ export type EmbeddedHunkSource = export type EmbeddedHunkSnapshot = | { status: "loading"; source: EmbeddedHunkSource } | { status: "ready"; source: EmbeddedHunkSource; fileCount: number; title: string } - | { status: "error"; source: EmbeddedHunkSource; fileCount: number; title: string; error: string }; + | { + status: "error"; + source: EmbeddedHunkSource; + fileCount: number; + title: string; + error: string; + }; export interface EmbeddedHunkSession { readonly cwd: string; + /** The currently loaded source. Failed opens keep the previous source. */ readonly source: EmbeddedHunkSource; getSnapshot(): EmbeddedHunkSnapshot; - open(source: EmbeddedHunkSource): Promise; - reload(): Promise; + /** Open a source idempotently; same-source opens reuse the current review. */ + open(source: EmbeddedHunkSource): Promise; + /** Refresh the currently loaded source contents without changing source identity. */ + reload(): Promise; subscribe(listener: () => void): () => void; dispose(): void; } diff --git a/src/session-broker/brokerClient.test.ts b/src/session-broker/brokerClient.test.ts index 21372976..9609cce6 100644 --- a/src/session-broker/brokerClient.test.ts +++ b/src/session-broker/brokerClient.test.ts @@ -76,6 +76,34 @@ afterEach(() => { }); describe("Hunk session daemon client", () => { + test("uses an injected broker availability adapter during startup", async () => { + delete process.env.HUNK_MCP_DISABLE; + const ensuredOrigins: string[] = []; + const messages: string[] = []; + console.error = (...args: unknown[]) => { + messages.push(args.map((value) => String(value)).join(" ")); + }; + const client = new SessionBrokerClient(createRegistration(), createSnapshot(), { + ensureBrokerAvailable: async (resolvedConfig) => { + ensuredOrigins.push(resolvedConfig.httpOrigin); + throw new Error("custom broker availability failed"); + }, + }); + + try { + client.start(); + await waitUntil( + "custom broker availability adapter", + () => ensuredOrigins.length === 1 && messages.length === 1, + ); + + expect(ensuredOrigins).toEqual(["http://127.0.0.1:47657"]); + expect(messages[0]).toContain("[session:broker] custom broker availability failed"); + } finally { + client.stop(); + } + }); + test("logs one actionable warning when the session daemon is configured for a non-loopback host without opt-in", async () => { process.env.HUNK_MCP_HOST = "0.0.0.0"; process.env.HUNK_MCP_PORT = "47657"; diff --git a/src/session-broker/brokerClient.ts b/src/session-broker/brokerClient.ts index 3e960781..f29a2a18 100644 --- a/src/session-broker/brokerClient.ts +++ b/src/session-broker/brokerClient.ts @@ -37,6 +37,12 @@ type SessionAppBridge< Result = unknown, > = SessionBrokerConnectionBridge; +export type EnsureSessionBrokerAdapter = (config: ResolvedSessionBrokerConfig) => Promise; + +export interface SessionBrokerClientOptions { + ensureBrokerAvailable?: EnsureSessionBrokerAdapter; +} + /** Keep one running app session registered with the local session broker daemon. */ export class SessionBrokerClient< Info = unknown, @@ -60,6 +66,7 @@ export class SessionBrokerClient< constructor( private registration: SessionRegistration, private snapshot: SessionSnapshot, + private options: SessionBrokerClientOptions = {}, ) {} start() { @@ -117,18 +124,20 @@ export class SessionBrokerClient< } private async ensureDaemonAvailable(config: ResolvedSessionBrokerConfig) { - await ensureSessionBrokerAvailable({ - config, - timeoutMs: DAEMON_STARTUP_TIMEOUT_MS, - }); + const ensureBrokerAvailable = + this.options.ensureBrokerAvailable ?? + ((resolvedConfig: ResolvedSessionBrokerConfig) => + ensureSessionBrokerAvailable({ + config: resolvedConfig, + timeoutMs: DAEMON_STARTUP_TIMEOUT_MS, + })); + + await ensureBrokerAvailable(config); const capabilities = await readHunkSessionDaemonCapabilities(config); if (!capabilities) { await this.restartIncompatibleDaemon(config); - await ensureSessionBrokerAvailable({ - config, - timeoutMs: DAEMON_STARTUP_TIMEOUT_MS, - }); + await ensureBrokerAvailable(config); if (!(await readHunkSessionDaemonCapabilities(config))) { throw new Error( From b81b810ec6cd59fb761ae66d0cf4f61e7f2ad788 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Mon, 18 May 2026 03:59:13 -0400 Subject: [PATCH 06/16] fix(embedded): normalize source identity and daemon launch --- src/embedded/daemon.test.ts | 24 ++++++++++++++++++++++-- src/embedded/daemon.ts | 10 ++++++++-- src/embedded/embedded.test.ts | 11 ++++++++--- src/embedded/session.ts | 19 +++++++++++++++---- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/embedded/daemon.test.ts b/src/embedded/daemon.test.ts index 5820123f..671d60b4 100644 --- a/src/embedded/daemon.test.ts +++ b/src/embedded/daemon.test.ts @@ -16,6 +16,7 @@ describe("embedded session broker daemon launcher", () => { cwd: "/repo", env: { HUNK_MCP_PORT: "48658" }, hunkCliPath: "/deps/hunkdiff/bin/hunk.cjs", + runtimePath: "/usr/local/bin/node", timeoutMs: 1234, ensureAvailable: async (options) => { captured = options; @@ -25,12 +26,31 @@ describe("embedded session broker daemon launcher", () => { await ensureBroker(testConfig); expect(captured).toEqual({ - argv: ["/deps/hunkdiff/bin/hunk.cjs"], + argv: ["/usr/local/bin/node", "/deps/hunkdiff/bin/hunk.cjs"], config: testConfig, cwd: "/repo", env: { HUNK_MCP_PORT: "48658" }, - execPath: "/deps/hunkdiff/bin/hunk.cjs", + execPath: "/usr/local/bin/node", timeoutMs: 1234, }); }); + + test("passes direct Hunk executable paths through without a runtime wrapper", async () => { + let captured: EnsureSessionBrokerAvailableOptions | undefined; + const ensureBroker = createEmbeddedSessionBrokerAvailability({ + cwd: "/repo", + hunkCliPath: "/deps/hunkdiff/bin/hunk", + runtimePath: "/usr/local/bin/node", + ensureAvailable: async (options) => { + captured = options; + }, + }); + + await ensureBroker(testConfig); + + expect(captured).toMatchObject({ + argv: ["/deps/hunkdiff/bin/hunk"], + execPath: "/deps/hunkdiff/bin/hunk", + }); + }); }); diff --git a/src/embedded/daemon.ts b/src/embedded/daemon.ts index c3c23e26..f574ccde 100644 --- a/src/embedded/daemon.ts +++ b/src/embedded/daemon.ts @@ -8,6 +8,7 @@ import type { ResolvedSessionBrokerConfig } from "../session-broker/brokerConfig import type { EnsureSessionBrokerAdapter } from "../session-broker/brokerClient"; const require = createRequire(import.meta.url); +const JAVASCRIPT_ENTRYPOINT_PATTERN = /\.(?:[cm]?js|tsx?)$/; type EmbeddedEnsureSessionBroker = typeof ensureSessionBrokerAvailable; @@ -16,6 +17,7 @@ export interface EmbeddedSessionBrokerAvailabilityOptions { env?: NodeJS.ProcessEnv; ensureAvailable?: EmbeddedEnsureSessionBroker; hunkCliPath?: string; + runtimePath?: string; timeoutMs?: number; } @@ -25,15 +27,19 @@ export function createEmbeddedSessionBrokerAvailability({ env = process.env, ensureAvailable = ensureSessionBrokerAvailable, hunkCliPath = join(dirname(require.resolve("hunkdiff/package.json")), "bin", "hunk.cjs"), + runtimePath = process.execPath, timeoutMs, }: EmbeddedSessionBrokerAvailabilityOptions): EnsureSessionBrokerAdapter { return (config: ResolvedSessionBrokerConfig) => { + // The published package bin is a JS wrapper, so launch it through the active runtime instead + // of spawning the script path directly. Direct executable overrides still run as-is. + const scriptEntrypoint = JAVASCRIPT_ENTRYPOINT_PATTERN.test(hunkCliPath); const options: EnsureSessionBrokerAvailableOptions = { - argv: [hunkCliPath], + argv: scriptEntrypoint ? [runtimePath, hunkCliPath] : [hunkCliPath], config, cwd, env, - execPath: hunkCliPath, + execPath: scriptEntrypoint ? runtimePath : hunkCliPath, }; if (timeoutMs !== undefined) { diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index eeef8d31..c6b640e1 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -86,7 +86,7 @@ describe("embedded Hunk sessions", () => { } }); - test("open reuses the loaded review when source identity has not changed", async () => { + test("open reuses the loaded review when source identity is equivalent", async () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-open-same-source-")); const left = join(root, "before.ts"); const right = join(root, "after.ts"); @@ -95,12 +95,17 @@ describe("embedded Hunk sessions", () => { writeFileSync(left, "export const value = 1;\n"); writeFileSync(right, "export const value = 2;\nexport const first = true;\n"); - const source = { kind: "diff", left, right } as const; + const source = { + kind: "diff", + left, + right, + options: { wrapLines: undefined }, + } as const; const session = await createEmbeddedHunkSession({ cwd: root, source }); expect(loadedPatch(session)).toContain("first"); writeFileSync(right, "export const value = 2;\nexport const second = true;\n"); - const reusedSnapshot = await session.open(source); + const reusedSnapshot = await session.open({ kind: "diff", left, right }); expect(reusedSnapshot.status).toBe("ready"); if (reusedSnapshot.status !== "ready") throw new Error("Expected reused snapshot."); diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 14fd9ad1..714e8f98 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -30,14 +30,25 @@ function errorMessage(error: unknown) { return String(error || "Failed to load Hunk."); } +/** Copy only explicitly defined fields so source identity has one canonical shape. */ +function compactRecord(source: T): T { + const next = {} as T; + for (const [key, value] of Object.entries(source) as [keyof T, T[keyof T]][]) { + if (value !== undefined) { + next[key] = value; + } + } + return next; +} + /** Return a session-owned source copy with normalized options and pathspec identity. */ function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): NormalizedEmbeddedHunkSource { const pathspecs = "pathspecs" in source ? source.pathspecs : undefined; - return { + return compactRecord({ ...source, - ...(pathspecs ? { pathspecs: [...pathspecs] } : {}), - options: { ...source.options }, - } as NormalizedEmbeddedHunkSource; + ...(pathspecs !== undefined ? { pathspecs: [...pathspecs] } : {}), + options: compactRecord({ ...source.options }), + }) as NormalizedEmbeddedHunkSource; } /** Adapt a public embedded source into the internal CLI input pipeline. */ From 1cf0e2c7e7e84edb302fbd4b89ced04b27d454b7 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Tue, 19 May 2026 12:54:40 -0400 Subject: [PATCH 07/16] feat(embedded): scope renderer state to host container --- src/embedded/embedded.test.ts | 83 +++++++++++++++++++++++++++-------- src/embedded/mount.tsx | 71 +++++++++++++++++++++++++----- src/embedded/session.ts | 42 +++++++++--------- 3 files changed, 146 insertions(+), 50 deletions(-) diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index c6b640e1..3f8ffca2 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -1,13 +1,15 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { BoxRenderable } from "@opentui/core"; +import { createTestRenderer } from "@opentui/core/testing"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createEmbeddedHunkSession } from "./index"; -import { createScopedKeyInput } from "./mount"; +import { createEmbeddedRendererScope, createScopedKeyInput } from "./mount"; import { embeddedHunkSessionInternals } from "./session"; import type { EmbeddedHunkSession } from "./types"; -const patchText = [ +const testPatchText = [ "diff --git a/example.ts b/example.ts", "--- a/example.ts", "+++ b/example.ts", @@ -19,15 +21,11 @@ const patchText = [ let previousHunkMcpDisable: string | undefined; -/** Return the private app bootstrap for assertions that public snapshots intentionally hide. */ -function renderBootstrap(session: EmbeddedHunkSession) { - return embeddedHunkSessionInternals(session).getRenderSnapshot().bootstrap; -} - /** Return the loaded patch text for one embedded session. */ -function loadedPatch(session: EmbeddedHunkSession) { - return renderBootstrap(session) - .changeset.files.map((file) => file.patch) +function getTestLoadedPatch(session: EmbeddedHunkSession) { + return embeddedHunkSessionInternals(session) + .getRenderSnapshot() + .bootstrap.changeset.files.map((file) => file.patch) .join("\n"); } @@ -60,7 +58,7 @@ describe("embedded Hunk sessions", () => { const session = await createEmbeddedHunkSession({ cwd: root, - source: { kind: "patch", text: patchText, options: { theme: "paper" } }, + source: { kind: "patch", text: testPatchText, options: { theme: "paper" } }, }); const snapshot = session.getSnapshot(); @@ -70,7 +68,7 @@ describe("embedded Hunk sessions", () => { expect(snapshot.title).toBe("Patch review: stdin patch"); expect(snapshot.fileCount).toBe(1); - const bootstrap = renderBootstrap(session); + const bootstrap = embeddedHunkSessionInternals(session).getRenderSnapshot().bootstrap; expect(bootstrap.initialMode).toBe("stack"); expect(bootstrap.initialShowLineNumbers).toBe(false); expect(bootstrap.initialTheme).toBe("paper"); @@ -102,7 +100,7 @@ describe("embedded Hunk sessions", () => { options: { wrapLines: undefined }, } as const; const session = await createEmbeddedHunkSession({ cwd: root, source }); - expect(loadedPatch(session)).toContain("first"); + expect(getTestLoadedPatch(session)).toContain("first"); writeFileSync(right, "export const value = 2;\nexport const second = true;\n"); const reusedSnapshot = await session.open({ kind: "diff", left, right }); @@ -110,16 +108,16 @@ describe("embedded Hunk sessions", () => { expect(reusedSnapshot.status).toBe("ready"); if (reusedSnapshot.status !== "ready") throw new Error("Expected reused snapshot."); expect(reusedSnapshot.source).toEqual(session.source); - expect(loadedPatch(session)).toContain("first"); - expect(loadedPatch(session)).not.toContain("second"); + expect(getTestLoadedPatch(session)).toContain("first"); + expect(getTestLoadedPatch(session)).not.toContain("second"); const reloadedSnapshot = await session.reload(); expect(reloadedSnapshot.status).toBe("ready"); if (reloadedSnapshot.status !== "ready") throw new Error("Expected reloaded snapshot."); expect(reloadedSnapshot.source).toEqual(session.source); - expect(loadedPatch(session)).toContain("second"); - expect(loadedPatch(session)).not.toContain("first"); + expect(getTestLoadedPatch(session)).toContain("second"); + expect(getTestLoadedPatch(session)).not.toContain("first"); session.dispose(); } finally { @@ -131,13 +129,13 @@ describe("embedded Hunk sessions", () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-error-")); try { - const initialSource = { kind: "patch", text: patchText, label: "initial patch" } as const; + const initialSource = { kind: "patch", text: testPatchText, label: "initial patch" } as const; const session = await createEmbeddedHunkSession({ cwd: root, source: initialSource, }); - await expect(session.open({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); + expect(session.open({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); expect(session.source).toMatchObject(initialSource); const snapshot = session.getSnapshot(); @@ -182,4 +180,51 @@ describe("embedded Hunk sessions", () => { scoped.dispose(); expect(sourceListeners.get("keypress")?.size).toBe(0); }); + + test("sizes embedded renderer reads and resize events from the host container", async () => { + const setup = await createTestRenderer({ width: 120, height: 40 }); + + try { + const container = new BoxRenderable(setup.renderer, { + height: 12, + id: "embedded-container", + width: 60, + }); + setup.renderer.root.add(container); + await setup.renderOnce(); + + const scope = createEmbeddedRendererScope(setup.renderer, container, setup.renderer.keyInput); + const resizes: Array<{ height: number; width: number }> = []; + const onResize = (width: unknown, height: unknown) => { + resizes.push({ height: Number(height), width: Number(width) }); + }; + + try { + scope.renderer.on("resize", onResize); + + expect(scope.renderer.width).toBe(60); + expect(scope.renderer.height).toBe(12); + expect(scope.renderer.terminalWidth).toBe(60); + expect(scope.renderer.terminalHeight).toBe(12); + + container.width = 48; + container.height = 9; + await setup.renderOnce(); + + expect(scope.renderer.width).toBe(48); + expect(scope.renderer.height).toBe(9); + expect(resizes).toEqual([{ height: 9, width: 48 }]); + + scope.renderer.off("resize", onResize); + container.width = 36; + await setup.renderOnce(); + + expect(resizes).toEqual([{ height: 9, width: 48 }]); + } finally { + scope.dispose(); + } + } finally { + setup.renderer.destroy(); + } + }); }); diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index 79afed03..dadbb210 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -13,6 +13,11 @@ type KeyInputSource = Readonly<{ on: (event: string, listener: KeyInputListener) => unknown; off: (event: string, listener: KeyInputListener) => unknown; }>; +type RendererListener = (...args: unknown[]) => void; +type ScopedRendererScope = { + renderer: CliRenderer; + dispose(): void; +}; /** Scope Hunk keyboard and paste listeners so inactive embedded mounts stay alive but quiet. */ export function createScopedKeyInput(source: KeyInputSource, enabled: () => boolean) { @@ -66,21 +71,65 @@ export function createScopedKeyInput(source: KeyInputSource, enabled: () => bool }; } -/** Scope the renderer root and input stream to the host-provided embedded container. */ -function scopedRenderer( +/** Scope renderer APIs that embedded Hunk reads to the host-provided container. */ +export function createEmbeddedRendererScope( renderer: CliRenderer, root: Renderable, keyInput: CliRenderer["keyInput"], -) { +): ScopedRendererScope { const scoped = Object.create(renderer) as CliRenderer; - Object.defineProperty(scoped, "root", { value: root }); - Object.defineProperty(scoped, "keyInput", { value: keyInput }); - Object.defineProperty(scoped, "intermediateRender", { - value() { - if (!renderer.isDestroyed) renderer.requestRender(); + const resizeListeners = new Set(); + const readWidth = () => Math.max(1, root.width); + const readHeight = () => Math.max(1, root.height); + const emitResize = () => { + for (const listener of resizeListeners) listener(readWidth(), readHeight()); + }; + + root.on("resize", emitResize); + + Object.defineProperties(scoped, { + height: { get: readHeight }, + intermediateRender: { + value() { + if (!renderer.isDestroyed) renderer.requestRender(); + }, + }, + keyInput: { value: keyInput }, + off: { + value(event: string | symbol, listener: RendererListener) { + if (event === "resize") { + resizeListeners.delete(listener); + return scoped; + } + + renderer.off(event, listener); + return scoped; + }, + }, + on: { + value(event: string | symbol, listener: RendererListener) { + if (event === "resize") { + resizeListeners.add(listener); + return scoped; + } + + renderer.on(event, listener); + return scoped; + }, }, + root: { value: root }, + terminalHeight: { get: readHeight }, + terminalWidth: { get: readWidth }, + width: { get: readWidth }, }); - return scoped; + + return { + renderer: scoped, + dispose() { + root.off("resize", emitResize); + resizeListeners.clear(); + }, + }; } function EmbeddedHunkRoot({ @@ -117,7 +166,8 @@ export function mountEmbeddedHunkApp({ }: MountEmbeddedHunkAppInput): EmbeddedHunkMount { let currentActive = active; const scopedKeyInput = createScopedKeyInput(renderer.keyInput, () => currentActive); - const root = createRoot(scopedRenderer(renderer, container, scopedKeyInput.keyInput)); + const scopedRenderer = createEmbeddedRendererScope(renderer, container, scopedKeyInput.keyInput); + const root = createRoot(scopedRenderer.renderer); const render = (next: { active: boolean; onQuit: () => void }) => { currentActive = next.active; @@ -130,6 +180,7 @@ export function mountEmbeddedHunkApp({ update: render, unmount() { root.unmount(); + scopedRenderer.dispose(); scopedKeyInput.dispose(); }, }; diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 714e8f98..33d38a1c 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -24,31 +24,31 @@ export type EmbeddedHunkRenderSnapshot = type NormalizedEmbeddedHunkSource = EmbeddedHunkSource & { options: CommonOptions }; -/** Convert unknown thrown values into stable user-facing error text. */ -function errorMessage(error: unknown) { - if (error instanceof Error && error.message) return error.message; - return String(error || "Failed to load Hunk."); -} - -/** Copy only explicitly defined fields so source identity has one canonical shape. */ -function compactRecord(source: T): T { - const next = {} as T; - for (const [key, value] of Object.entries(source) as [keyof T, T[keyof T]][]) { - if (value !== undefined) { - next[key] = value; - } +/** Drop undefined option entries so equivalent embedded sources compare the same. */ +function normalizeEmbeddedOptions(options: EmbeddedHunkSource["options"] = {}): CommonOptions { + const normalized = { ...options }; + for (const key of Object.keys(normalized) as Array) { + if (normalized[key] === undefined) delete normalized[key]; } - return next; + return normalized; } /** Return a session-owned source copy with normalized options and pathspec identity. */ function normalizeEmbeddedHunkSource(source: EmbeddedHunkSource): NormalizedEmbeddedHunkSource { - const pathspecs = "pathspecs" in source ? source.pathspecs : undefined; - return compactRecord({ + const normalized = { ...source, - ...(pathspecs !== undefined ? { pathspecs: [...pathspecs] } : {}), - options: compactRecord({ ...source.options }), - }) as NormalizedEmbeddedHunkSource; + options: normalizeEmbeddedOptions(source.options), + } as NormalizedEmbeddedHunkSource; + + if ("pathspecs" in normalized) { + if (normalized.pathspecs === undefined) { + delete normalized.pathspecs; + } else { + normalized.pathspecs = [...normalized.pathspecs]; + } + } + + return normalized; } /** Adapt a public embedded source into the internal CLI input pipeline. */ @@ -183,13 +183,13 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.setRenderSnapshot({ status: "ready", bootstrap }); return this.getSnapshot(); } catch (error) { - const message = errorMessage(error); + const message = error instanceof Error ? error.message : String(error); this.setRenderSnapshot({ status: "error", bootstrap: this.renderSnapshot.bootstrap, error: message, }); - throw error instanceof Error ? error : new Error(message); + throw error; } } From ec16aded463406308cf22e7d1fd5aa3260a055aa Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Wed, 20 May 2026 12:19:32 -0400 Subject: [PATCH 08/16] fix(build): surface Bun export diagnostics in one error --- scripts/build-npm.ts | 57 ++++++++++++++++++++++++++--------- src/embedded/daemon.test.ts | 8 ++--- src/embedded/embedded.test.ts | 34 ++++++++++++--------- 3 files changed, 64 insertions(+), 35 deletions(-) diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index c7c0c03d..7ddde453 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -27,6 +27,19 @@ const bunEnv = { BUN_INSTALL: path.join(repoRoot, ".bun-install"), }; +type LibraryBuildLog = Awaited>["logs"][number]; + +interface BuildLibraryExportOptions { + entrypoint: string; + name: string; + outputDirectory: string; +} + +interface FormatBuildLibraryExportErrorOptions { + logs: readonly LibraryBuildLog[]; + name: string; +} + function runBun(args: string[]) { const proc = Bun.spawnSync(["bun", ...args], { cwd: repoRoot, @@ -41,7 +54,24 @@ function runBun(args: string[]) { } } -async function buildLibraryExport(name: string, entrypoint: string, outputDirectory: string) { +/** Format a Bun.build failure so the runtime reports the build diagnostics once. */ +function formatBuildLibraryExportError({ logs, name }: FormatBuildLibraryExportErrorOptions) { + const details = logs + .map((log) => log.message) + .filter((message) => message.length > 0) + .join("\n"); + + return details + ? `Failed to build ${name} export:\n${details}` + : `Failed to build ${name} export.`; +} + +/** Build one npm package subpath export. */ +async function buildLibraryExport({ + entrypoint, + name, + outputDirectory, +}: BuildLibraryExportOptions) { const build = await Bun.build({ entrypoints: [entrypoint], target: "node", @@ -52,10 +82,7 @@ async function buildLibraryExport(name: string, entrypoint: string, outputDirect }); if (!build.success) { - for (const log of build.logs) { - console.error(log.message); - } - throw new Error(`Failed to build ${name} export.`); + throw new Error(formatBuildLibraryExportError({ logs: build.logs, name })); } } @@ -83,16 +110,16 @@ if (process.platform !== "win32") { chmodSync(mainJs, 0o755); } -await buildLibraryExport( - "OpenTUI", - path.join(repoRoot, "src", "opentui", "index.ts"), - opentuiOutdir, -); -await buildLibraryExport( - "embedded Hunk", - path.join(repoRoot, "src", "embedded", "index.ts"), - embeddedOutdir, -); +await buildLibraryExport({ + entrypoint: path.join(repoRoot, "src", "opentui", "index.ts"), + name: "OpenTUI", + outputDirectory: opentuiOutdir, +}); +await buildLibraryExport({ + entrypoint: path.join(repoRoot, "src", "embedded", "index.ts"), + name: "embedded Hunk", + outputDirectory: embeddedOutdir, +}); runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.npm-exports.json")]); diff --git a/src/embedded/daemon.test.ts b/src/embedded/daemon.test.ts index 671d60b4..118b238d 100644 --- a/src/embedded/daemon.test.ts +++ b/src/embedded/daemon.test.ts @@ -1,13 +1,9 @@ import { describe, expect, test } from "bun:test"; import { createEmbeddedSessionBrokerAvailability } from "./daemon"; +import { resolveSessionBrokerConfig } from "../session-broker/brokerConfig"; import type { EnsureSessionBrokerAvailableOptions } from "../session-broker/brokerLauncher"; -const testConfig = { - host: "127.0.0.1", - port: 47657, - httpOrigin: "http://127.0.0.1:47657", - wsOrigin: "ws://127.0.0.1:47657", -}; +const testConfig = resolveSessionBrokerConfig({ HUNK_MCP_PORT: "47657" }); describe("embedded session broker daemon launcher", () => { test("passes Hunk package-bin launch options through the broker availability adapter", async () => { diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 3f8ffca2..cae2d158 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -7,7 +7,7 @@ import { join } from "node:path"; import { createEmbeddedHunkSession } from "./index"; import { createEmbeddedRendererScope, createScopedKeyInput } from "./mount"; import { embeddedHunkSessionInternals } from "./session"; -import type { EmbeddedHunkSession } from "./types"; +import type { EmbeddedHunkSession, EmbeddedHunkSnapshot } from "./types"; const testPatchText = [ "diff --git a/example.ts b/example.ts", @@ -29,6 +29,18 @@ function getTestLoadedPatch(session: EmbeddedHunkSession) { .join("\n"); } +/** Expect a snapshot to be ready and narrow it for the rest of the test. */ +function expectTestReadySnapshot(snapshot: EmbeddedHunkSnapshot) { + expect(snapshot.status).toBe("ready"); + return snapshot as Extract; +} + +/** Expect a snapshot to be errored and narrow it for the rest of the test. */ +function expectTestErrorSnapshot(snapshot: EmbeddedHunkSnapshot) { + expect(snapshot.status).toBe("error"); + return snapshot as Extract; +} + describe("embedded Hunk sessions", () => { beforeEach(() => { previousHunkMcpDisable = process.env.HUNK_MCP_DISABLE; @@ -60,10 +72,8 @@ describe("embedded Hunk sessions", () => { cwd: root, source: { kind: "patch", text: testPatchText, options: { theme: "paper" } }, }); - const snapshot = session.getSnapshot(); + const snapshot = expectTestReadySnapshot(session.getSnapshot()); - expect(snapshot.status).toBe("ready"); - if (snapshot.status !== "ready") throw new Error("Expected embedded session to load."); expect("bootstrap" in snapshot).toBe(false); expect(snapshot.title).toBe("Patch review: stdin patch"); expect(snapshot.fileCount).toBe(1); @@ -103,18 +113,16 @@ describe("embedded Hunk sessions", () => { expect(getTestLoadedPatch(session)).toContain("first"); writeFileSync(right, "export const value = 2;\nexport const second = true;\n"); - const reusedSnapshot = await session.open({ kind: "diff", left, right }); + const reusedSnapshot = expectTestReadySnapshot( + await session.open({ kind: "diff", left, right }), + ); - expect(reusedSnapshot.status).toBe("ready"); - if (reusedSnapshot.status !== "ready") throw new Error("Expected reused snapshot."); expect(reusedSnapshot.source).toEqual(session.source); expect(getTestLoadedPatch(session)).toContain("first"); expect(getTestLoadedPatch(session)).not.toContain("second"); - const reloadedSnapshot = await session.reload(); + const reloadedSnapshot = expectTestReadySnapshot(await session.reload()); - expect(reloadedSnapshot.status).toBe("ready"); - if (reloadedSnapshot.status !== "ready") throw new Error("Expected reloaded snapshot."); expect(reloadedSnapshot.source).toEqual(session.source); expect(getTestLoadedPatch(session)).toContain("second"); expect(getTestLoadedPatch(session)).not.toContain("first"); @@ -135,12 +143,10 @@ describe("embedded Hunk sessions", () => { source: initialSource, }); - expect(session.open({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); + await expect(session.open({ kind: "patch", file: "missing.patch" })).rejects.toThrow(); expect(session.source).toMatchObject(initialSource); - const snapshot = session.getSnapshot(); - expect(snapshot.status).toBe("error"); - if (snapshot.status !== "error") throw new Error("Expected embedded reload to fail."); + const snapshot = expectTestErrorSnapshot(session.getSnapshot()); expect(snapshot.error).toContain("missing.patch"); expect(snapshot.title).toBe("Patch review: initial patch"); expect("bootstrap" in snapshot).toBe(false); From 4bac6f5b76255aa7ce37abf89eedeb20bcafb3ca Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Fri, 22 May 2026 01:29:34 -0400 Subject: [PATCH 09/16] fix: persist embedded session state and ignore stale prebuilt bins --- bin/hunk.cjs | 18 +- src/embedded/embedded.test.ts | 67 ++++- src/embedded/mount.tsx | 1 + src/embedded/session.ts | 446 +++++++++++++++++++++++++++- src/ui/App.tsx | 19 +- src/ui/AppHost.tsx | 5 +- src/ui/hooks/useReviewController.ts | 53 +++- test/cli/entrypoint.test.ts | 98 +++++- 8 files changed, 688 insertions(+), 19 deletions(-) diff --git a/bin/hunk.cjs b/bin/hunk.cjs index 19e94359..949b14df 100755 --- a/bin/hunk.cjs +++ b/bin/hunk.cjs @@ -72,15 +72,31 @@ function hostCandidates() { return []; } +function readPackageVersion(packageRoot) { + try { + const packageJson = JSON.parse(fs.readFileSync(path.join(packageRoot, "package.json"), "utf8")); + return typeof packageJson.version === "string" ? packageJson.version : null; + } catch { + return null; + } +} + function findInstalledBinary(startDir) { + const expectedVersion = readPackageVersion(path.join(__dirname, "..")); let current = startDir; for (;;) { const modulesDir = path.join(current, "node_modules"); if (fs.existsSync(modulesDir)) { for (const candidate of hostCandidates()) { - const resolved = path.join(modulesDir, candidate.packageName, "bin", candidate.binary); + const packageRoot = path.join(modulesDir, candidate.packageName); + const resolved = path.join(packageRoot, "bin", candidate.binary); if (fs.existsSync(resolved)) { + const installedVersion = readPackageVersion(packageRoot); + if (expectedVersion && installedVersion && installedVersion !== expectedVersion) { + continue; + } + return resolved; } } diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index cae2d158..553cee33 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -4,7 +4,7 @@ import { createTestRenderer } from "@opentui/core/testing"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { createEmbeddedHunkSession } from "./index"; +import { createEmbeddedHunkSession, mountEmbeddedHunkApp } from "./index"; import { createEmbeddedRendererScope, createScopedKeyInput } from "./mount"; import { embeddedHunkSessionInternals } from "./session"; import type { EmbeddedHunkSession, EmbeddedHunkSnapshot } from "./types"; @@ -157,6 +157,71 @@ describe("embedded Hunk sessions", () => { } }); + test("preserves headless agent notes for the next embedded mount", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-headless-notes-")); + const noteSummary = "Persisted agent note from hidden Hunk"; + + try { + const session = await createEmbeddedHunkSession({ + cwd: root, + source: { kind: "patch", text: testPatchText }, + }); + const internals = embeddedHunkSessionInternals(session); + + await internals.dispatchCommand({ + type: "command", + requestId: "comment-batch-1", + command: "comment_batch", + input: { + comments: [ + { + filePath: "example.ts", + hunkIndex: 0, + summary: noteSummary, + }, + ], + revealMode: "first", + }, + }); + + expect(internals.getSessionSnapshot().state.showAgentNotes).toBe(true); + expect(internals.getSessionSnapshot().state.liveCommentCount).toBe(1); + + const setup = await createTestRenderer({ width: 120, height: 24 }); + const container = new BoxRenderable(setup.renderer, { + height: 18, + id: "embedded-hunk", + width: 100, + }); + setup.renderer.root.add(container); + + const mount = mountEmbeddedHunkApp({ + active: true, + container, + onQuit: () => undefined, + renderer: setup.renderer, + session, + }); + + try { + let frame = ""; + for (let attempt = 0; attempt < 5; attempt += 1) { + await setup.renderOnce(); + frame = setup.captureCharFrame(); + if (frame.includes(noteSummary)) break; + await Bun.sleep(0); + } + expect(frame).toContain(noteSummary); + } finally { + mount.unmount(); + setup.renderer.destroy(); + session.dispose(); + } + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + test("scopes embedded key input to the active mount", () => { const sourceListeners = new Map void>>(); const source = { diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index dadbb210..cc686f3a 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -150,6 +150,7 @@ function EmbeddedHunkRoot({ null} /> diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 33d38a1c..10cc1e3a 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -1,14 +1,37 @@ import { isDeepStrictEqual } from "node:util"; import { resolveConfiguredCliInput } from "../core/config"; +import { + buildLiveComment, + findDiffFileByPath, + hunkLineRange, + resolveCommentTarget, +} from "../core/liveComments"; import { loadAppBootstrap } from "../core/loaders"; import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; +import { createHunkSessionBridge } from "../hunk-session/bridge"; import { createInitialSessionSnapshot, createSessionRegistration, updateSessionRegistration, } from "../hunk-session/sessionRegistration"; -import type { HunkSessionBrokerClient } from "../hunk-session/types"; +import type { + AppliedCommentBatchResult, + AppliedCommentResult, + ClearedCommentsResult, + HunkSessionBrokerClient, + HunkSessionCommandResult, + HunkSessionRegistration, + HunkSessionServerMessage, + HunkSessionSnapshot, + LiveComment, + NavigatedSelectionResult, + RemovedCommentResult, + SessionLiveCommentSummary, + SessionReviewNoteSummary, +} from "../hunk-session/types"; import { SessionBrokerClient } from "../session-broker/brokerClient"; +import { reviewNoteSource } from "../ui/lib/agentAnnotations"; +import { buildSelectedHunkSummary, resolveReviewNavigationTarget } from "../ui/lib/reviewState"; import { createEmbeddedSessionBrokerAvailability } from "./daemon"; import type { CreateEmbeddedHunkSessionInput, @@ -106,12 +129,67 @@ function publicSnapshot( : { ...base, status: "ready" }; } +/** Convert one live comment into the broker snapshot's summary shape. */ +function summarizeLiveComment(comment: LiveComment): SessionLiveCommentSummary { + return { + commentId: comment.id, + filePath: comment.filePath, + hunkIndex: comment.hunkIndex, + side: comment.side, + line: comment.line, + summary: comment.summary, + rationale: comment.rationale, + author: comment.author, + createdAt: comment.createdAt, + }; +} + +/** Rehydrate broker snapshot comments into the session-owned live annotation map. */ +function liveCommentsByFileFromSnapshot( + bootstrap: AppBootstrap, + comments: SessionLiveCommentSummary[], +) { + const byFileId: Record = {}; + comments.forEach((comment) => { + const file = findDiffFileByPath(bootstrap.changeset.files, comment.filePath); + if (!file) { + return; + } + + byFileId[file.id] = [ + ...(byFileId[file.id] ?? []), + { + id: comment.commentId, + source: "mcp", + filePath: comment.filePath, + hunkIndex: comment.hunkIndex, + side: comment.side, + line: comment.line, + summary: comment.summary, + rationale: comment.rationale, + author: comment.author, + createdAt: comment.createdAt, + oldRange: comment.side === "old" ? [comment.line, comment.line] : undefined, + newRange: comment.side === "new" ? [comment.line, comment.line] : undefined, + tags: ["mcp"], + confidence: "high", + }, + ]; + }); + + return byFileId; +} + /** Own one embedded Hunk review session, including source identity and broker registration. */ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); private disposed = false; private renderSnapshot: EmbeddedHunkRenderSnapshot; + private sessionSnapshot: HunkSessionSnapshot; + private liveCommentsByFileId: Record = {}; + private mountedBridge: Parameters[0] = null; + readonly brokerClient: HunkSessionBrokerClient; readonly hostClient: HunkSessionBrokerClient; constructor( @@ -120,20 +198,27 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { bootstrap: AppBootstrap, ) { this.renderSnapshot = { status: "ready", bootstrap }; - this.hostClient = new SessionBrokerClient( + this.sessionSnapshot = createInitialSessionSnapshot(bootstrap); + this.brokerClient = new SessionBrokerClient( createSessionRegistration(bootstrap, { cwd }), - createInitialSessionSnapshot(bootstrap), + this.sessionSnapshot, { ensureBrokerAvailable: createEmbeddedSessionBrokerAvailability({ cwd }), }, ); - this.hostClient.start(); + this.brokerClient.setBridge({ + dispatchCommand: (message) => this.dispatchCommand(message), + }); + this.hostClient = this.createMountedHostClient(); + this.brokerClient.start(); } getSnapshot = () => publicSnapshot(this.source, this.renderSnapshot); getRenderSnapshot = () => this.renderSnapshot; + getSessionSnapshot = () => this.sessionSnapshot; + subscribe = (listener: () => void) => { this.listeners.add(listener); return () => this.listeners.delete(listener); @@ -154,9 +239,46 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { return this.load(this.source, { updateSource: false }); } + /** Dispatch session-broker commands through the mounted UI when available, otherwise headlessly. */ + async dispatchCommand(message: HunkSessionServerMessage): Promise { + if (this.mountedBridge) { + return this.mountedBridge.dispatchCommand(message); + } + + const bridge = createHunkSessionBridge({ + addLiveComment: (input, commentId, options) => + this.addHeadlessLiveComment(input, commentId, options), + addLiveCommentBatch: (inputs, requestId, options) => + this.addHeadlessLiveCommentBatch(inputs, requestId, options), + clearLiveComments: (filePath) => this.clearHeadlessLiveComments(filePath), + navigateToLocation: (input) => this.navigateHeadless(input), + openAgentNotes: () => this.setHeadlessAgentNotesVisible(true), + reloadSession: async (nextInput) => { + const result = await this.load(nextInput as EmbeddedHunkSource, { + updateSource: true, + }); + return { + sessionId: this.brokerClient.getRegistration().sessionId, + inputKind: this.renderSnapshot.bootstrap.input.kind, + title: + result.status === "ready" + ? result.title + : this.renderSnapshot.bootstrap.changeset.title, + sourceLabel: this.renderSnapshot.bootstrap.changeset.sourceLabel, + fileCount: this.renderSnapshot.bootstrap.changeset.files.length, + selectedFilePath: this.sessionSnapshot.state.selectedFilePath, + selectedHunkIndex: this.sessionSnapshot.state.selectedHunkIndex, + }; + }, + removeLiveComment: (commentId) => this.removeHeadlessLiveComment(commentId), + }); + + return bridge.dispatchCommand(message); + } + dispose() { this.disposed = true; - this.hostClient.stop(); + this.brokerClient.stop(); this.listeners.clear(); } @@ -176,9 +298,11 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { if (updateSource) { this.source = source; } - this.hostClient.replaceSession( - updateSessionRegistration(this.hostClient.getRegistration(), bootstrap), - createInitialSessionSnapshot(bootstrap), + this.liveCommentsByFileId = {}; + this.sessionSnapshot = createInitialSessionSnapshot(bootstrap); + this.brokerClient.replaceSession( + updateSessionRegistration(this.brokerClient.getRegistration(), bootstrap), + this.sessionSnapshot, ); this.setRenderSnapshot({ status: "ready", bootstrap }); return this.getSnapshot(); @@ -197,13 +321,319 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.renderSnapshot = snapshot; for (const listener of this.listeners) listener(); } + + /** Build the host client facade used by mounted React apps without giving up headless handling. */ + private createMountedHostClient(): HunkSessionBrokerClient { + return { + getRegistration: () => this.brokerClient.getRegistration(), + replaceSession: (registration: HunkSessionRegistration, snapshot: HunkSessionSnapshot) => { + this.persistSessionSnapshot(snapshot); + this.brokerClient.replaceSession(registration, snapshot); + }, + setBridge: (bridge: Parameters[0]) => { + this.mountedBridge = bridge; + }, + start: () => this.brokerClient.start(), + stop: () => this.brokerClient.stop(), + updateSnapshot: (snapshot: HunkSessionSnapshot) => { + this.persistSessionSnapshot(snapshot); + this.brokerClient.updateSnapshot(snapshot); + }, + } as HunkSessionBrokerClient; + } + + /** Persist the latest mounted-app snapshot so future embedded mounts start from it. */ + private persistSessionSnapshot(snapshot: HunkSessionSnapshot) { + this.sessionSnapshot = snapshot; + this.liveCommentsByFileId = liveCommentsByFileFromSnapshot( + this.renderSnapshot.bootstrap, + snapshot.state.liveComments, + ); + } + + /** Return all session-owned live comments in file order. */ + private liveCommentSummaries() { + return this.renderSnapshot.bootstrap.changeset.files.flatMap((file) => + (this.liveCommentsByFileId[file.id] ?? []).map(summarizeLiveComment), + ); + } + + /** Return all review notes visible to session commands, including headless live comments. */ + private reviewNoteSummaries(): SessionReviewNoteSummary[] { + const summaries: SessionReviewNoteSummary[] = []; + + this.renderSnapshot.bootstrap.changeset.files.forEach((file) => { + (file.agent?.annotations ?? []).forEach((annotation, index) => { + const source = reviewNoteSource(annotation); + summaries.push({ + noteId: annotation.id ?? `${source}:${file.id}:${index}`, + source, + filePath: file.path, + oldRange: annotation.oldRange, + newRange: annotation.newRange, + body: [annotation.summary, annotation.rationale].filter(Boolean).join("\n\n"), + title: annotation.title, + author: annotation.author, + createdAt: annotation.createdAt ?? "1970-01-01T00:00:00.000Z", + updatedAt: annotation.updatedAt, + editable: false, + }); + }); + + (this.liveCommentsByFileId[file.id] ?? []).forEach((comment) => { + summaries.push({ + noteId: comment.id, + source: "agent", + filePath: file.path, + hunkIndex: comment.hunkIndex, + oldRange: comment.oldRange, + newRange: comment.newRange, + body: [comment.summary, comment.rationale].filter(Boolean).join("\n\n"), + author: comment.author, + createdAt: comment.createdAt, + editable: false, + }); + }); + }); + + return summaries; + } + + /** Publish the current session-owned review state to the daemon. */ + private updateHeadlessSnapshot() { + const liveComments = this.liveCommentSummaries(); + const reviewNotes = this.reviewNoteSummaries(); + this.sessionSnapshot = { + updatedAt: new Date().toISOString(), + state: { + ...this.sessionSnapshot.state, + liveCommentCount: liveComments.length, + liveComments, + reviewNoteCount: reviewNotes.length, + reviewNotes, + }, + }; + this.brokerClient.updateSnapshot(this.sessionSnapshot); + for (const listener of this.listeners) listener(); + } + + /** Update the persisted agent-note visibility bit. */ + private setHeadlessAgentNotesVisible(visible: boolean) { + this.sessionSnapshot = { + ...this.sessionSnapshot, + state: { + ...this.sessionSnapshot.state, + showAgentNotes: visible, + }, + }; + this.updateHeadlessSnapshot(); + } + + /** Add one live agent comment to the session-owned review state. */ + private addHeadlessLiveComment( + input: Extract["input"], + commentId: string, + options?: { reveal?: boolean }, + ): AppliedCommentResult { + const file = findDiffFileByPath(this.renderSnapshot.bootstrap.changeset.files, input.filePath); + if (!file) { + throw new Error(`No diff file matches ${input.filePath}.`); + } + + const target = resolveCommentTarget(file, input); + const liveComment = buildLiveComment( + { + ...input, + side: target.side, + line: target.line, + }, + commentId, + new Date().toISOString(), + target.hunkIndex, + ); + this.liveCommentsByFileId[file.id] = [ + ...(this.liveCommentsByFileId[file.id] ?? []), + liveComment, + ]; + + if (options?.reveal ?? false) { + this.selectHeadlessHunk(file.id, file.path, target.hunkIndex); + this.sessionSnapshot.state.showAgentNotes = true; + } + + this.updateHeadlessSnapshot(); + return { + commentId, + fileId: file.id, + filePath: file.path, + hunkIndex: target.hunkIndex, + side: target.side, + line: target.line, + }; + } + + /** Apply a validated batch of live comments to the session-owned review state. */ + private addHeadlessLiveCommentBatch( + inputs: Extract["input"]["comments"], + requestId: string, + options?: { revealMode?: "none" | "first" }, + ): AppliedCommentBatchResult { + const createdAt = new Date().toISOString(); + const prepared = inputs.map((input, index) => { + const file = findDiffFileByPath( + this.renderSnapshot.bootstrap.changeset.files, + input.filePath, + ); + if (!file) { + throw new Error(`No diff file matches ${input.filePath}.`); + } + + const target = resolveCommentTarget(file, input); + return { + file, + target, + liveComment: buildLiveComment( + { + ...input, + side: target.side, + line: target.line, + }, + `mcp:${requestId}:${index}`, + createdAt, + target.hunkIndex, + ), + }; + }); + + prepared.forEach(({ file, liveComment }) => { + this.liveCommentsByFileId[file.id] = [ + ...(this.liveCommentsByFileId[file.id] ?? []), + liveComment, + ]; + }); + + if (options?.revealMode === "first" && prepared.length > 0) { + const first = prepared[0]!; + this.selectHeadlessHunk(first.file.id, first.file.path, first.target.hunkIndex); + this.sessionSnapshot.state.showAgentNotes = true; + } + + this.updateHeadlessSnapshot(); + return { + applied: prepared.map(({ file, target, liveComment }) => ({ + commentId: liveComment.id, + fileId: file.id, + filePath: file.path, + hunkIndex: target.hunkIndex, + side: target.side, + line: target.line, + })), + }; + } + + /** Navigate the persisted hidden-session selection. */ + private navigateHeadless( + input: Extract["input"], + ): NavigatedSelectionResult { + const files = this.renderSnapshot.bootstrap.changeset.files; + const target = resolveReviewNavigationTarget({ + allFiles: files, + currentFileId: this.sessionSnapshot.state.selectedFileId, + currentHunkIndex: this.sessionSnapshot.state.selectedHunkIndex, + input, + visibleFiles: files, + }); + this.selectHeadlessHunk(target.file.id, target.file.path, target.hunkIndex); + this.updateHeadlessSnapshot(); + return { + fileId: target.file.id, + filePath: target.file.path, + hunkIndex: target.hunkIndex, + selectedHunk: buildSelectedHunkSummary(target.file, target.hunkIndex), + }; + } + + /** Remove one persisted live comment. */ + private removeHeadlessLiveComment(commentId: string): RemovedCommentResult { + let removed = false; + let remainingCommentCount = 0; + const next: Record = {}; + + for (const [fileId, comments] of Object.entries(this.liveCommentsByFileId)) { + const filtered = comments.filter((comment) => comment.id !== commentId); + if (filtered.length !== comments.length) { + removed = true; + } + if (filtered.length > 0) { + next[fileId] = filtered; + remainingCommentCount += filtered.length; + } + } + + if (!removed) { + throw new Error(`No live comment matches id ${commentId}.`); + } + + this.liveCommentsByFileId = next; + this.updateHeadlessSnapshot(); + return { commentId, removed: true, remainingCommentCount }; + } + + /** Clear persisted live comments, optionally scoped to one file. */ + private clearHeadlessLiveComments(filePath?: string): ClearedCommentsResult { + let removedCount = 0; + let remainingCommentCount = 0; + + if (filePath) { + const file = findDiffFileByPath(this.renderSnapshot.bootstrap.changeset.files, filePath); + if (!file) { + throw new Error(`No diff file matches ${filePath}.`); + } + + const next: Record = {}; + for (const [fileId, comments] of Object.entries(this.liveCommentsByFileId)) { + if (fileId === file.id) { + removedCount = comments.length; + continue; + } + next[fileId] = comments; + remainingCommentCount += comments.length; + } + this.liveCommentsByFileId = next; + } else { + removedCount = Object.values(this.liveCommentsByFileId).reduce( + (sum, comments) => sum + comments.length, + 0, + ); + this.liveCommentsByFileId = {}; + } + + this.updateHeadlessSnapshot(); + return { removedCount, remainingCommentCount, filePath }; + } + + /** Update the persisted selection fields from one file/hunk target. */ + private selectHeadlessHunk(fileId: string, filePath: string, hunkIndex: number) { + const file = this.renderSnapshot.bootstrap.changeset.files.find( + (candidate) => candidate.id === fileId, + ); + const hunk = file?.metadata.hunks[hunkIndex]; + const range = hunk ? hunkLineRange(hunk) : null; + this.sessionSnapshot.state.selectedFileId = fileId; + this.sessionSnapshot.state.selectedFilePath = filePath; + this.sessionSnapshot.state.selectedHunkIndex = hunkIndex; + this.sessionSnapshot.state.selectedHunkOldRange = range?.oldRange; + this.sessionSnapshot.state.selectedHunkNewRange = range?.newRange; + } } /** Resolve private render state for sessions created by this embedded entrypoint. */ export function embeddedHunkSessionInternals(session: EmbeddedHunkSession) { if (session instanceof EmbeddedHunkSessionImpl) { return { + dispatchCommand: session.dispatchCommand.bind(session), getRenderSnapshot: session.getRenderSnapshot, + getSessionSnapshot: session.getSessionSnapshot, hostClient: session.hostClient, }; } diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 1bc36ce0..620da618 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -7,7 +7,11 @@ import { useRenderer, useTerminalDimensions } from "@opentui/react"; import { Suspense, lazy, useCallback, useEffect, useMemo, useState, useRef } from "react"; import type { AppBootstrap, CliInput, LayoutMode, UserNoteLineTarget } from "../core/types"; import { canReloadInput, computeWatchSignature } from "../core/watch"; -import type { HunkSessionBrokerClient, ReloadedSessionResult } from "../hunk-session/types"; +import type { + HunkSessionBrokerClient, + HunkSessionState, + ReloadedSessionResult, +} from "../hunk-session/types"; import { MenuBar } from "./components/chrome/MenuBar"; import { StatusBar } from "./components/chrome/StatusBar"; import { DiffPane } from "./components/panes/DiffPane"; @@ -77,12 +81,14 @@ function withCurrentViewOptions( export function App({ bootstrap, hostClient, + initialSessionState, noticeText, onQuit = () => process.exit(0), onReloadSession, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; + initialSessionState?: HunkSessionState; noticeText?: string | null; onQuit?: () => void; onReloadSession: ( @@ -112,7 +118,9 @@ export function App({ ); // Soft reloads replace bootstrap without re-running startup terminal theme detection. const [detectedThemeMode] = useState(() => bootstrap.initialThemeMode); - const [showAgentNotes, setShowAgentNotes] = useState(bootstrap.initialShowAgentNotes ?? false); + const [showAgentNotes, setShowAgentNotes] = useState( + initialSessionState?.showAgentNotes ?? bootstrap.initialShowAgentNotes ?? false, + ); const [showLineNumbers, setShowLineNumbers] = useState(bootstrap.initialShowLineNumbers ?? true); const [wrapLines, setWrapLines] = useState(bootstrap.initialWrapLines ?? false); const [codeHorizontalOffset, setCodeHorizontalOffset] = useState(0); @@ -129,7 +137,12 @@ export function App({ const sessionNoticeTimeoutRef = useRef | null>(null); const activeTheme = resolveTheme(themeId, detectedThemeMode ?? null); - const review = useReviewController({ files: bootstrap.changeset.files }); + const review = useReviewController({ + files: bootstrap.changeset.files, + initialLiveComments: initialSessionState?.liveComments, + initialSelectedFileId: initialSessionState?.selectedFileId, + initialSelectedHunkIndex: initialSessionState?.selectedHunkIndex, + }); const filteredFiles = review.visibleFiles; const selectedFile = review.selectedFile; const selectedHunkIndex = review.selectedHunkIndex; diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index ed4fe1ad..6fd5841a 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -8,7 +8,7 @@ import { createInitialSessionSnapshot, updateSessionRegistration, } from "../hunk-session/sessionRegistration"; -import type { HunkSessionBrokerClient } from "../hunk-session/types"; +import type { HunkSessionBrokerClient, HunkSessionState } from "../hunk-session/types"; import { App } from "./App"; import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; @@ -16,11 +16,13 @@ import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; export function AppHost({ bootstrap, hostClient, + initialSessionState, onQuit = () => process.exit(0), startupNoticeResolver, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; + initialSessionState?: HunkSessionState; onQuit?: () => void; startupNoticeResolver?: () => Promise; }) { @@ -95,6 +97,7 @@ export function AppHost({ key={appVersion} bootstrap={activeBootstrap} hostClient={hostClient} + initialSessionState={initialSessionState} noticeText={startupNoticeText} onQuit={onQuit} onReloadSession={reloadSession} diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index 3b1ff431..a147c378 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -146,16 +146,61 @@ export interface ReviewController { updateDraftNote: (body: string) => void; } +/** Rehydrate daemon-snapshot live comments into the review controller's annotation map. */ +function liveCommentsByFileFromSummaries( + files: DiffFile[], + summaries: SessionLiveCommentSummary[] = [], +): Record { + const byFileId: Record = {}; + + summaries.forEach((summary) => { + const file = findDiffFileByPath(files, summary.filePath); + if (!file) { + return; + } + + const comment: LiveComment = { + id: summary.commentId, + source: "mcp", + filePath: summary.filePath, + hunkIndex: summary.hunkIndex, + side: summary.side, + line: summary.line, + summary: summary.summary, + rationale: summary.rationale, + author: summary.author, + createdAt: summary.createdAt, + oldRange: summary.side === "old" ? [summary.line, summary.line] : undefined, + newRange: summary.side === "new" ? [summary.line, summary.line] : undefined, + tags: ["mcp"], + confidence: "high", + }; + byFileId[file.id] = [...(byFileId[file.id] ?? []), comment]; + }); + + return byFileId; +} + /** Own the shared review stream state used by both the UI and session bridge. */ -export function useReviewController({ files }: { files: DiffFile[] }): ReviewController { +export function useReviewController({ + files, + initialLiveComments, + initialSelectedFileId, + initialSelectedHunkIndex, +}: { + files: DiffFile[]; + initialLiveComments?: SessionLiveCommentSummary[]; + initialSelectedFileId?: string; + initialSelectedHunkIndex?: number; +}): ReviewController { const [filter, setFilter] = useState(""); - const [selectedFileId, setSelectedFileId] = useState(files[0]?.id ?? ""); - const [selectedHunkIndex, setSelectedHunkIndex] = useState(0); + const [selectedFileId, setSelectedFileId] = useState(initialSelectedFileId ?? files[0]?.id ?? ""); + const [selectedHunkIndex, setSelectedHunkIndex] = useState(initialSelectedHunkIndex ?? 0); const [selectedFileTopAlignRequestId, setSelectedFileTopAlignRequestId] = useState(0); const [selectedHunkRevealRequestId, setSelectedHunkRevealRequestId] = useState(0); const [scrollToNote, setScrollToNote] = useState(false); const [liveCommentsByFileId, setLiveCommentsByFileId] = useState>( - {}, + () => liveCommentsByFileFromSummaries(files, initialLiveComments), ); const [userNotesByFileId, setUserNotesByFileId] = useState>({}); const [draftNote, setDraftNote] = useState(null); diff --git a/test/cli/entrypoint.test.ts b/test/cli/entrypoint.test.ts index 923521c6..b609952d 100644 --- a/test/cli/entrypoint.test.ts +++ b/test/cli/entrypoint.test.ts @@ -1,5 +1,13 @@ import { describe, expect, test } from "bun:test"; -import { copyFileSync, existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -18,6 +26,30 @@ function git(cwd: string, ...args: string[]) { } } +/** Return the optional prebuilt package name for the current test host. */ +function currentPlatformPackageName() { + const platformMap: Partial> = { + darwin: "darwin", + linux: "linux", + win32: "windows", + }; + const archMap: Partial> = { + arm64: "arm64", + x64: "x64", + }; + const platform = platformMap[process.platform]; + const arch = archMap[process.arch]; + return platform && arch ? `hunkdiff-${platform}-${arch}` : null; +} + +/** Write a small executable JS file for wrapper integration tests. */ +function writeExecutableScript(path: string, source: string) { + writeFileSync(path, source); + if (process.platform !== "win32") { + chmodSync(path, 0o755); + } +} + describe("CLI entrypoint contracts", () => { test("bare hunk prints standard help without terminal takeover sequences", () => { const proc = Bun.spawnSync(["bun", "run", "src/main.tsx"], { @@ -204,6 +236,70 @@ describe("CLI entrypoint contracts", () => { } }); + test("bin wrapper ignores a mismatched prebuilt package and falls back to bundled JS", () => { + if (process.platform === "win32") { + // This test builds fake shebang executables, which Windows cannot launch directly. + return; + } + + const platformPackageName = currentPlatformPackageName(); + if (!platformPackageName) { + return; + } + + const tempDir = mkdtempSync(join(tmpdir(), "hunk-wrapper-stale-prebuilt-")); + const packageRoot = join(tempDir, "hunkdiff"); + const tempBinDir = join(packageRoot, "bin"); + const stalePackageRoot = join(packageRoot, "node_modules", platformPackageName); + const staleBinDir = join(stalePackageRoot, "bin"); + const fakeBunDir = join(packageRoot, "node_modules", "bun", "bin"); + + try { + mkdirSync(tempBinDir, { recursive: true }); + mkdirSync(join(packageRoot, "dist", "npm"), { recursive: true }); + mkdirSync(staleBinDir, { recursive: true }); + mkdirSync(fakeBunDir, { recursive: true }); + copyFileSync(join(process.cwd(), "bin", "hunk.cjs"), join(tempBinDir, "hunk.cjs")); + writeFileSync(join(packageRoot, "package.json"), JSON.stringify({ version: "2.0.0" })); + writeFileSync(join(stalePackageRoot, "package.json"), JSON.stringify({ version: "1.0.0" })); + writeExecutableScript( + join(staleBinDir, "hunk"), + "#!/usr/bin/env node\nconsole.log('stale prebuilt');\n", + ); + writeExecutableScript( + join(fakeBunDir, "bun.exe"), + [ + "#!/usr/bin/env node", + 'const { spawnSync } = require("node:child_process");', + "const result = spawnSync(process.execPath, process.argv.slice(2), { stdio: 'inherit' });", + "process.exit(typeof result.status === 'number' ? result.status : 1);", + "", + ].join("\n"), + ); + writeExecutableScript( + join(packageRoot, "dist", "npm", "main.js"), + "#!/usr/bin/env node\nconsole.log('fallback bundled js');\n", + ); + + const proc = Bun.spawnSync(["node", join(tempBinDir, "hunk.cjs"), "--version"], { + cwd: tempDir, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + env: process.env, + }); + + const stdout = Buffer.from(proc.stdout).toString("utf8"); + const stderr = Buffer.from(proc.stderr).toString("utf8"); + + expect(proc.exitCode).toBe(0); + expect(stdout).toBe("fallback bundled js\n"); + expect(stderr).toBe(""); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + test("general pager mode falls back to plain text for non-diff stdin", () => { const proc = Bun.spawnSync(["bun", "run", "src/main.tsx", "pager"], { cwd: process.cwd(), From e99a09269aa9a9f42aed74687f5569b1e68632c6 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Fri, 22 May 2026 16:38:32 -0400 Subject: [PATCH 10/16] refactor(embedded): simplify npm export implementation --- bin/hunk.cjs | 37 ++++---------- scripts/build-npm.ts | 59 +++++++--------------- src/embedded/daemon.ts | 23 +++------ src/embedded/index.ts | 19 +------ src/embedded/session.ts | 64 ++++-------------------- src/hunk-session/liveCommentSummaries.ts | 55 ++++++++++++++++++++ src/ui/hooks/useReviewController.ts | 53 +++----------------- 7 files changed, 107 insertions(+), 203 deletions(-) create mode 100644 src/hunk-session/liveCommentSummaries.ts diff --git a/bin/hunk.cjs b/bin/hunk.cjs index 949b14df..ebe3e135 100755 --- a/bin/hunk.cjs +++ b/bin/hunk.cjs @@ -41,35 +41,18 @@ function run(target, args) { } function hostCandidates() { - const platformMap = { - darwin: "darwin", - linux: "linux", - win32: "windows", - }; - const archMap = { - x64: "x64", - arm64: "arm64", - }; - - const platform = platformMap[os.platform()] || os.platform(); - const arch = archMap[os.arch()] || os.arch(); - const binary = platform === "windows" ? "hunk.exe" : "hunk"; - - if (platform === "darwin") { - if (arch === "arm64") return [{ packageName: "hunkdiff-darwin-arm64", binary }]; - if (arch === "x64") return [{ packageName: "hunkdiff-darwin-x64", binary }]; + const platform = { darwin: "darwin", linux: "linux", win32: "windows" }[os.platform()]; + const arch = { x64: "x64", arm64: "arm64" }[os.arch()]; + if (!platform || !arch || (platform === "windows" && arch !== "x64")) { + return []; } - if (platform === "linux") { - if (arch === "arm64") return [{ packageName: "hunkdiff-linux-arm64", binary }]; - if (arch === "x64") return [{ packageName: "hunkdiff-linux-x64", binary }]; - } - - if (platform === "windows") { - if (arch === "x64") return [{ packageName: "hunkdiff-windows-x64", binary }]; - } - - return []; + return [ + { + packageName: `hunkdiff-${platform}-${arch}`, + binary: platform === "windows" ? "hunk.exe" : "hunk", + }, + ]; } function readPackageVersion(packageRoot) { diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index 7ddde453..fd80713d 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -27,19 +27,6 @@ const bunEnv = { BUN_INSTALL: path.join(repoRoot, ".bun-install"), }; -type LibraryBuildLog = Awaited>["logs"][number]; - -interface BuildLibraryExportOptions { - entrypoint: string; - name: string; - outputDirectory: string; -} - -interface FormatBuildLibraryExportErrorOptions { - logs: readonly LibraryBuildLog[]; - name: string; -} - function runBun(args: string[]) { const proc = Bun.spawnSync(["bun", ...args], { cwd: repoRoot, @@ -54,24 +41,8 @@ function runBun(args: string[]) { } } -/** Format a Bun.build failure so the runtime reports the build diagnostics once. */ -function formatBuildLibraryExportError({ logs, name }: FormatBuildLibraryExportErrorOptions) { - const details = logs - .map((log) => log.message) - .filter((message) => message.length > 0) - .join("\n"); - - return details - ? `Failed to build ${name} export:\n${details}` - : `Failed to build ${name} export.`; -} - /** Build one npm package subpath export. */ -async function buildLibraryExport({ - entrypoint, - name, - outputDirectory, -}: BuildLibraryExportOptions) { +async function buildLibraryExport(entrypoint: string, name: string, outputDirectory: string) { const build = await Bun.build({ entrypoints: [entrypoint], target: "node", @@ -82,7 +53,13 @@ async function buildLibraryExport({ }); if (!build.success) { - throw new Error(formatBuildLibraryExportError({ logs: build.logs, name })); + const details = build.logs + .map((log) => log.message) + .filter((message) => message.length > 0) + .join("\n"); + throw new Error( + details ? `Failed to build ${name} export:\n${details}` : `Failed to build ${name} export.`, + ); } } @@ -110,16 +87,16 @@ if (process.platform !== "win32") { chmodSync(mainJs, 0o755); } -await buildLibraryExport({ - entrypoint: path.join(repoRoot, "src", "opentui", "index.ts"), - name: "OpenTUI", - outputDirectory: opentuiOutdir, -}); -await buildLibraryExport({ - entrypoint: path.join(repoRoot, "src", "embedded", "index.ts"), - name: "embedded Hunk", - outputDirectory: embeddedOutdir, -}); +await buildLibraryExport( + path.join(repoRoot, "src", "opentui", "index.ts"), + "OpenTUI", + opentuiOutdir, +); +await buildLibraryExport( + path.join(repoRoot, "src", "embedded", "index.ts"), + "embedded Hunk", + embeddedOutdir, +); runBun(["x", "tsc", "-p", path.join(repoRoot, "tsconfig.npm-exports.json")]); diff --git a/src/embedded/daemon.ts b/src/embedded/daemon.ts index f574ccde..2b56c35e 100644 --- a/src/embedded/daemon.ts +++ b/src/embedded/daemon.ts @@ -1,21 +1,15 @@ import { createRequire } from "node:module"; import { dirname, join } from "node:path"; -import { - ensureSessionBrokerAvailable, - type EnsureSessionBrokerAvailableOptions, -} from "../session-broker/brokerLauncher"; -import type { ResolvedSessionBrokerConfig } from "../session-broker/brokerConfig"; +import { ensureSessionBrokerAvailable } from "../session-broker/brokerLauncher"; import type { EnsureSessionBrokerAdapter } from "../session-broker/brokerClient"; const require = createRequire(import.meta.url); const JAVASCRIPT_ENTRYPOINT_PATTERN = /\.(?:[cm]?js|tsx?)$/; -type EmbeddedEnsureSessionBroker = typeof ensureSessionBrokerAvailable; - export interface EmbeddedSessionBrokerAvailabilityOptions { cwd: string; env?: NodeJS.ProcessEnv; - ensureAvailable?: EmbeddedEnsureSessionBroker; + ensureAvailable?: typeof ensureSessionBrokerAvailable; hunkCliPath?: string; runtimePath?: string; timeoutMs?: number; @@ -30,22 +24,17 @@ export function createEmbeddedSessionBrokerAvailability({ runtimePath = process.execPath, timeoutMs, }: EmbeddedSessionBrokerAvailabilityOptions): EnsureSessionBrokerAdapter { - return (config: ResolvedSessionBrokerConfig) => { + return (config) => { // The published package bin is a JS wrapper, so launch it through the active runtime instead // of spawning the script path directly. Direct executable overrides still run as-is. const scriptEntrypoint = JAVASCRIPT_ENTRYPOINT_PATTERN.test(hunkCliPath); - const options: EnsureSessionBrokerAvailableOptions = { + return ensureAvailable({ argv: scriptEntrypoint ? [runtimePath, hunkCliPath] : [hunkCliPath], config, cwd, env, execPath: scriptEntrypoint ? runtimePath : hunkCliPath, - }; - - if (timeoutMs !== undefined) { - options.timeoutMs = timeoutMs; - } - - return ensureAvailable(options); + ...(timeoutMs === undefined ? {} : { timeoutMs }), + }); }; } diff --git a/src/embedded/index.ts b/src/embedded/index.ts index a5cbab7b..525b430b 100644 --- a/src/embedded/index.ts +++ b/src/embedded/index.ts @@ -1,5 +1,5 @@ -import { mountEmbeddedHunkApp as mountEmbeddedHunkAppImpl } from "./mount"; -import { createEmbeddedHunkSession as createEmbeddedHunkSessionImpl } from "./session"; +export { mountEmbeddedHunkApp } from "./mount"; +export { createEmbeddedHunkSession } from "./session"; export type { CreateEmbeddedHunkSessionInput, EmbeddedHunkMount, @@ -9,18 +9,3 @@ export type { EmbeddedHunkSource, MountEmbeddedHunkAppInput, } from "./types"; -import type { - CreateEmbeddedHunkSessionInput, - EmbeddedHunkMount, - EmbeddedHunkSession, - MountEmbeddedHunkAppInput, -} from "./types"; - -/** Create one embedded Hunk review session from a public embedded source. */ -export const createEmbeddedHunkSession = ( - input: CreateEmbeddedHunkSessionInput, -): Promise => createEmbeddedHunkSessionImpl(input); - -/** Mount one embedded Hunk app into a host-owned OpenTUI container. */ -export const mountEmbeddedHunkApp = (input: MountEmbeddedHunkAppInput): EmbeddedHunkMount => - mountEmbeddedHunkAppImpl(input); diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 10cc1e3a..7f5b49ff 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -9,6 +9,10 @@ import { import { loadAppBootstrap } from "../core/loaders"; import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; import { createHunkSessionBridge } from "../hunk-session/bridge"; +import { + liveCommentsByFileFromSummaries, + summarizeLiveComment, +} from "../hunk-session/liveCommentSummaries"; import { createInitialSessionSnapshot, createSessionRegistration, @@ -26,7 +30,6 @@ import type { LiveComment, NavigatedSelectionResult, RemovedCommentResult, - SessionLiveCommentSummary, SessionReviewNoteSummary, } from "../hunk-session/types"; import { SessionBrokerClient } from "../session-broker/brokerClient"; @@ -129,57 +132,6 @@ function publicSnapshot( : { ...base, status: "ready" }; } -/** Convert one live comment into the broker snapshot's summary shape. */ -function summarizeLiveComment(comment: LiveComment): SessionLiveCommentSummary { - return { - commentId: comment.id, - filePath: comment.filePath, - hunkIndex: comment.hunkIndex, - side: comment.side, - line: comment.line, - summary: comment.summary, - rationale: comment.rationale, - author: comment.author, - createdAt: comment.createdAt, - }; -} - -/** Rehydrate broker snapshot comments into the session-owned live annotation map. */ -function liveCommentsByFileFromSnapshot( - bootstrap: AppBootstrap, - comments: SessionLiveCommentSummary[], -) { - const byFileId: Record = {}; - comments.forEach((comment) => { - const file = findDiffFileByPath(bootstrap.changeset.files, comment.filePath); - if (!file) { - return; - } - - byFileId[file.id] = [ - ...(byFileId[file.id] ?? []), - { - id: comment.commentId, - source: "mcp", - filePath: comment.filePath, - hunkIndex: comment.hunkIndex, - side: comment.side, - line: comment.line, - summary: comment.summary, - rationale: comment.rationale, - author: comment.author, - createdAt: comment.createdAt, - oldRange: comment.side === "old" ? [comment.line, comment.line] : undefined, - newRange: comment.side === "new" ? [comment.line, comment.line] : undefined, - tags: ["mcp"], - confidence: "high", - }, - ]; - }); - - return byFileId; -} - /** Own one embedded Hunk review session, including source identity and broker registration. */ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); @@ -345,8 +297,8 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { /** Persist the latest mounted-app snapshot so future embedded mounts start from it. */ private persistSessionSnapshot(snapshot: HunkSessionSnapshot) { this.sessionSnapshot = snapshot; - this.liveCommentsByFileId = liveCommentsByFileFromSnapshot( - this.renderSnapshot.bootstrap, + this.liveCommentsByFileId = liveCommentsByFileFromSummaries( + this.renderSnapshot.bootstrap.changeset.files, snapshot.state.liveComments, ); } @@ -354,7 +306,9 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { /** Return all session-owned live comments in file order. */ private liveCommentSummaries() { return this.renderSnapshot.bootstrap.changeset.files.flatMap((file) => - (this.liveCommentsByFileId[file.id] ?? []).map(summarizeLiveComment), + (this.liveCommentsByFileId[file.id] ?? []).map((comment) => + summarizeLiveComment(file.path, comment), + ), ); } diff --git a/src/hunk-session/liveCommentSummaries.ts b/src/hunk-session/liveCommentSummaries.ts new file mode 100644 index 00000000..fd497dad --- /dev/null +++ b/src/hunk-session/liveCommentSummaries.ts @@ -0,0 +1,55 @@ +import { findDiffFileByPath, type LiveComment } from "../core/liveComments"; +import type { DiffFile } from "../core/types"; +import type { SessionLiveCommentSummary } from "./types"; + +/** Convert a live annotation into the broker snapshot's compact summary shape. */ +export function summarizeLiveComment( + filePath: string, + comment: LiveComment, +): SessionLiveCommentSummary { + return { + commentId: comment.id, + filePath, + hunkIndex: comment.hunkIndex, + side: comment.side, + line: comment.line, + summary: comment.summary, + rationale: comment.rationale, + author: comment.author, + createdAt: comment.createdAt, + }; +} + +/** Rehydrate one broker snapshot comment into a live annotation. */ +function liveCommentFromSummary(summary: SessionLiveCommentSummary): LiveComment { + return { + id: summary.commentId, + source: "mcp", + filePath: summary.filePath, + hunkIndex: summary.hunkIndex, + side: summary.side, + line: summary.line, + summary: summary.summary, + rationale: summary.rationale, + author: summary.author, + createdAt: summary.createdAt, + oldRange: summary.side === "old" ? [summary.line, summary.line] : undefined, + newRange: summary.side === "new" ? [summary.line, summary.line] : undefined, + tags: ["mcp"], + confidence: "high", + }; +} + +/** Rehydrate broker snapshot comments into a file-id keyed annotation map. */ +export function liveCommentsByFileFromSummaries( + files: DiffFile[], + summaries: SessionLiveCommentSummary[] = [], +) { + const byFileId: Record = {}; + summaries.forEach((summary) => { + const file = findDiffFileByPath(files, summary.filePath); + if (!file) return; + byFileId[file.id] = [...(byFileId[file.id] ?? []), liveCommentFromSummary(summary)]; + }); + return byFileId; +} diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index a147c378..25a2b263 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -21,6 +21,10 @@ import { resolveCommentTarget, } from "../../core/liveComments"; import type { AgentAnnotation, DiffFile, UserNoteLineTarget } from "../../core/types"; +import { + liveCommentsByFileFromSummaries, + summarizeLiveComment, +} from "../../hunk-session/liveCommentSummaries"; import type { AppliedCommentBatchResult, AppliedCommentResult, @@ -146,41 +150,6 @@ export interface ReviewController { updateDraftNote: (body: string) => void; } -/** Rehydrate daemon-snapshot live comments into the review controller's annotation map. */ -function liveCommentsByFileFromSummaries( - files: DiffFile[], - summaries: SessionLiveCommentSummary[] = [], -): Record { - const byFileId: Record = {}; - - summaries.forEach((summary) => { - const file = findDiffFileByPath(files, summary.filePath); - if (!file) { - return; - } - - const comment: LiveComment = { - id: summary.commentId, - source: "mcp", - filePath: summary.filePath, - hunkIndex: summary.hunkIndex, - side: summary.side, - line: summary.line, - summary: summary.summary, - rationale: summary.rationale, - author: summary.author, - createdAt: summary.createdAt, - oldRange: summary.side === "old" ? [summary.line, summary.line] : undefined, - newRange: summary.side === "new" ? [summary.line, summary.line] : undefined, - tags: ["mcp"], - confidence: "high", - }; - byFileId[file.id] = [...(byFileId[file.id] ?? []), comment]; - }); - - return byFileId; -} - /** Own the shared review stream state used by both the UI and session bridge. */ export function useReviewController({ files, @@ -768,17 +737,9 @@ export function useReviewController({ const liveCommentSummaries = useMemo( () => allFiles.flatMap((file) => - (liveCommentsByFileId[file.id] ?? []).map((comment) => ({ - commentId: comment.id, - filePath: file.path, - hunkIndex: comment.hunkIndex, - side: comment.side, - line: comment.line, - summary: comment.summary, - rationale: comment.rationale, - author: comment.author, - createdAt: comment.createdAt, - })), + (liveCommentsByFileId[file.id] ?? []).map((comment) => + summarizeLiveComment(file.path, comment), + ), ), [allFiles, liveCommentsByFileId], ); From 0df8add79bd6aba6c2edbcd045061d5c4d75416b Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sat, 23 May 2026 00:39:08 -0400 Subject: [PATCH 11/16] refactor(review): centralize shared review command state Move selection, live comments, user notes, and session snapshots into a single review-command state layer shared by the UI and embedded session. --- src/embedded/session.ts | 348 ++++---------- src/hunk-session/liveCommentSummaries.ts | 55 --- src/hunk-session/reviewCommandState.ts | 498 +++++++++++++++++++ src/ui/App.tsx | 27 +- src/ui/hooks/useHunkSessionBridge.ts | 57 +-- src/ui/hooks/useReviewController.test.tsx | 37 ++ src/ui/hooks/useReviewController.ts | 553 +++++++--------------- src/ui/lib/reviewState.ts | 18 +- 8 files changed, 806 insertions(+), 787 deletions(-) delete mode 100644 src/hunk-session/liveCommentSummaries.ts create mode 100644 src/hunk-session/reviewCommandState.ts diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 7f5b49ff..687a1f13 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -1,20 +1,21 @@ import { isDeepStrictEqual } from "node:util"; import { resolveConfiguredCliInput } from "../core/config"; -import { - buildLiveComment, - findDiffFileByPath, - hunkLineRange, - resolveCommentTarget, -} from "../core/liveComments"; import { loadAppBootstrap } from "../core/loaders"; import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; import { createHunkSessionBridge } from "../hunk-session/bridge"; import { - liveCommentsByFileFromSummaries, - summarizeLiveComment, -} from "../hunk-session/liveCommentSummaries"; + addReviewLiveComment, + addReviewLiveCommentBatch, + buildReviewSessionSnapshot, + clearReviewLiveComments, + createReviewCommandState, + navigateReviewCommandState, + reviewCommandFiles, + removeReviewLiveComment, + setReviewAgentNotesVisible, + type ReviewCommandState, +} from "../hunk-session/reviewCommandState"; import { - createInitialSessionSnapshot, createSessionRegistration, updateSessionRegistration, } from "../hunk-session/sessionRegistration"; @@ -27,14 +28,10 @@ import type { HunkSessionRegistration, HunkSessionServerMessage, HunkSessionSnapshot, - LiveComment, NavigatedSelectionResult, RemovedCommentResult, - SessionReviewNoteSummary, } from "../hunk-session/types"; import { SessionBrokerClient } from "../session-broker/brokerClient"; -import { reviewNoteSource } from "../ui/lib/agentAnnotations"; -import { buildSelectedHunkSummary, resolveReviewNavigationTarget } from "../ui/lib/reviewState"; import { createEmbeddedSessionBrokerAvailability } from "./daemon"; import type { CreateEmbeddedHunkSessionInput, @@ -137,8 +134,8 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); private disposed = false; private renderSnapshot: EmbeddedHunkRenderSnapshot; + private reviewState: ReviewCommandState; private sessionSnapshot: HunkSessionSnapshot; - private liveCommentsByFileId: Record = {}; private mountedBridge: Parameters[0] = null; readonly brokerClient: HunkSessionBrokerClient; @@ -150,7 +147,11 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { bootstrap: AppBootstrap, ) { this.renderSnapshot = { status: "ready", bootstrap }; - this.sessionSnapshot = createInitialSessionSnapshot(bootstrap); + this.reviewState = createReviewCommandState({ + files: bootstrap.changeset.files, + initialShowAgentNotes: bootstrap.initialShowAgentNotes ?? false, + }); + this.sessionSnapshot = this.buildSessionSnapshot(); this.brokerClient = new SessionBrokerClient( createSessionRegistration(bootstrap, { cwd }), this.sessionSnapshot, @@ -198,12 +199,10 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { } const bridge = createHunkSessionBridge({ - addLiveComment: (input, commentId, options) => - this.addHeadlessLiveComment(input, commentId, options), - addLiveCommentBatch: (inputs, requestId, options) => - this.addHeadlessLiveCommentBatch(inputs, requestId, options), - clearLiveComments: (filePath) => this.clearHeadlessLiveComments(filePath), - navigateToLocation: (input) => this.navigateHeadless(input), + addLiveComment: this.addHeadlessLiveComment.bind(this), + addLiveCommentBatch: this.addHeadlessLiveCommentBatch.bind(this), + clearLiveComments: this.clearHeadlessLiveComments.bind(this), + navigateToLocation: this.navigateHeadless.bind(this), openAgentNotes: () => this.setHeadlessAgentNotesVisible(true), reloadSession: async (nextInput) => { const result = await this.load(nextInput as EmbeddedHunkSource, { @@ -222,7 +221,7 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { selectedHunkIndex: this.sessionSnapshot.state.selectedHunkIndex, }; }, - removeLiveComment: (commentId) => this.removeHeadlessLiveComment(commentId), + removeLiveComment: this.removeHeadlessLiveComment.bind(this), }); return bridge.dispatchCommand(message); @@ -250,8 +249,11 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { if (updateSource) { this.source = source; } - this.liveCommentsByFileId = {}; - this.sessionSnapshot = createInitialSessionSnapshot(bootstrap); + this.reviewState = createReviewCommandState({ + files: bootstrap.changeset.files, + initialShowAgentNotes: bootstrap.initialShowAgentNotes ?? false, + }); + this.sessionSnapshot = this.buildSessionSnapshot(bootstrap); this.brokerClient.replaceSession( updateSessionRegistration(this.brokerClient.getRegistration(), bootstrap), this.sessionSnapshot, @@ -274,6 +276,15 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { for (const listener of this.listeners) listener(); } + /** Build the broker-facing snapshot from the current command state. */ + private buildSessionSnapshot(bootstrap = this.renderSnapshot.bootstrap) { + return buildReviewSessionSnapshot({ + files: bootstrap.changeset.files, + state: this.reviewState, + now: new Date().toISOString(), + }); + } + /** Build the host client facade used by mounted React apps without giving up headless handling. */ private createMountedHostClient(): HunkSessionBrokerClient { return { @@ -297,89 +308,29 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { /** Persist the latest mounted-app snapshot so future embedded mounts start from it. */ private persistSessionSnapshot(snapshot: HunkSessionSnapshot) { this.sessionSnapshot = snapshot; - this.liveCommentsByFileId = liveCommentsByFileFromSummaries( - this.renderSnapshot.bootstrap.changeset.files, - snapshot.state.liveComments, - ); - } - - /** Return all session-owned live comments in file order. */ - private liveCommentSummaries() { - return this.renderSnapshot.bootstrap.changeset.files.flatMap((file) => - (this.liveCommentsByFileId[file.id] ?? []).map((comment) => - summarizeLiveComment(file.path, comment), - ), - ); - } - - /** Return all review notes visible to session commands, including headless live comments. */ - private reviewNoteSummaries(): SessionReviewNoteSummary[] { - const summaries: SessionReviewNoteSummary[] = []; - - this.renderSnapshot.bootstrap.changeset.files.forEach((file) => { - (file.agent?.annotations ?? []).forEach((annotation, index) => { - const source = reviewNoteSource(annotation); - summaries.push({ - noteId: annotation.id ?? `${source}:${file.id}:${index}`, - source, - filePath: file.path, - oldRange: annotation.oldRange, - newRange: annotation.newRange, - body: [annotation.summary, annotation.rationale].filter(Boolean).join("\n\n"), - title: annotation.title, - author: annotation.author, - createdAt: annotation.createdAt ?? "1970-01-01T00:00:00.000Z", - updatedAt: annotation.updatedAt, - editable: false, - }); - }); - - (this.liveCommentsByFileId[file.id] ?? []).forEach((comment) => { - summaries.push({ - noteId: comment.id, - source: "agent", - filePath: file.path, - hunkIndex: comment.hunkIndex, - oldRange: comment.oldRange, - newRange: comment.newRange, - body: [comment.summary, comment.rationale].filter(Boolean).join("\n\n"), - author: comment.author, - createdAt: comment.createdAt, - editable: false, - }); - }); + this.reviewState = createReviewCommandState({ + files: this.renderSnapshot.bootstrap.changeset.files, + initialSessionState: snapshot.state, }); - - return summaries; } /** Publish the current session-owned review state to the daemon. */ private updateHeadlessSnapshot() { - const liveComments = this.liveCommentSummaries(); - const reviewNotes = this.reviewNoteSummaries(); - this.sessionSnapshot = { - updatedAt: new Date().toISOString(), - state: { - ...this.sessionSnapshot.state, - liveCommentCount: liveComments.length, - liveComments, - reviewNoteCount: reviewNotes.length, - reviewNotes, - }, - }; + this.sessionSnapshot = this.buildSessionSnapshot(); this.brokerClient.updateSnapshot(this.sessionSnapshot); for (const listener of this.listeners) listener(); } + /** Apply a headless review-state transition and publish it to the daemon. */ + private applyHeadlessTransition(transition: { state: ReviewCommandState; result: T }): T { + this.reviewState = transition.state; + this.updateHeadlessSnapshot(); + return transition.result; + } + /** Update the persisted agent-note visibility bit. */ private setHeadlessAgentNotesVisible(visible: boolean) { - this.sessionSnapshot = { - ...this.sessionSnapshot, - state: { - ...this.sessionSnapshot.state, - showAgentNotes: visible, - }, - }; + this.reviewState = setReviewAgentNotesVisible(this.reviewState, visible); this.updateHeadlessSnapshot(); } @@ -389,41 +340,16 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { commentId: string, options?: { reveal?: boolean }, ): AppliedCommentResult { - const file = findDiffFileByPath(this.renderSnapshot.bootstrap.changeset.files, input.filePath); - if (!file) { - throw new Error(`No diff file matches ${input.filePath}.`); - } - - const target = resolveCommentTarget(file, input); - const liveComment = buildLiveComment( - { - ...input, - side: target.side, - line: target.line, - }, - commentId, - new Date().toISOString(), - target.hunkIndex, + return this.applyHeadlessTransition( + addReviewLiveComment({ + files: this.renderSnapshot.bootstrap.changeset.files, + state: this.reviewState, + input, + commentId, + now: new Date().toISOString(), + options, + }), ); - this.liveCommentsByFileId[file.id] = [ - ...(this.liveCommentsByFileId[file.id] ?? []), - liveComment, - ]; - - if (options?.reveal ?? false) { - this.selectHeadlessHunk(file.id, file.path, target.hunkIndex); - this.sessionSnapshot.state.showAgentNotes = true; - } - - this.updateHeadlessSnapshot(); - return { - commentId, - fileId: file.id, - filePath: file.path, - hunkIndex: target.hunkIndex, - side: target.side, - line: target.line, - }; } /** Apply a validated batch of live comments to the session-owned review state. */ @@ -432,152 +358,50 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { requestId: string, options?: { revealMode?: "none" | "first" }, ): AppliedCommentBatchResult { - const createdAt = new Date().toISOString(); - const prepared = inputs.map((input, index) => { - const file = findDiffFileByPath( - this.renderSnapshot.bootstrap.changeset.files, - input.filePath, - ); - if (!file) { - throw new Error(`No diff file matches ${input.filePath}.`); - } - - const target = resolveCommentTarget(file, input); - return { - file, - target, - liveComment: buildLiveComment( - { - ...input, - side: target.side, - line: target.line, - }, - `mcp:${requestId}:${index}`, - createdAt, - target.hunkIndex, - ), - }; - }); - - prepared.forEach(({ file, liveComment }) => { - this.liveCommentsByFileId[file.id] = [ - ...(this.liveCommentsByFileId[file.id] ?? []), - liveComment, - ]; - }); - - if (options?.revealMode === "first" && prepared.length > 0) { - const first = prepared[0]!; - this.selectHeadlessHunk(first.file.id, first.file.path, first.target.hunkIndex); - this.sessionSnapshot.state.showAgentNotes = true; - } - - this.updateHeadlessSnapshot(); - return { - applied: prepared.map(({ file, target, liveComment }) => ({ - commentId: liveComment.id, - fileId: file.id, - filePath: file.path, - hunkIndex: target.hunkIndex, - side: target.side, - line: target.line, - })), - }; + return this.applyHeadlessTransition( + addReviewLiveCommentBatch({ + files: this.renderSnapshot.bootstrap.changeset.files, + state: this.reviewState, + inputs, + requestId, + now: new Date().toISOString(), + options, + }), + ); } /** Navigate the persisted hidden-session selection. */ private navigateHeadless( input: Extract["input"], ): NavigatedSelectionResult { - const files = this.renderSnapshot.bootstrap.changeset.files; - const target = resolveReviewNavigationTarget({ - allFiles: files, - currentFileId: this.sessionSnapshot.state.selectedFileId, - currentHunkIndex: this.sessionSnapshot.state.selectedHunkIndex, - input, - visibleFiles: files, - }); - this.selectHeadlessHunk(target.file.id, target.file.path, target.hunkIndex); - this.updateHeadlessSnapshot(); - return { - fileId: target.file.id, - filePath: target.file.path, - hunkIndex: target.hunkIndex, - selectedHunk: buildSelectedHunkSummary(target.file, target.hunkIndex), - }; + const files = reviewCommandFiles( + this.renderSnapshot.bootstrap.changeset.files, + this.reviewState, + ); + return this.applyHeadlessTransition( + navigateReviewCommandState({ + allFiles: files, + visibleFiles: files, + state: this.reviewState, + input, + }), + ); } /** Remove one persisted live comment. */ private removeHeadlessLiveComment(commentId: string): RemovedCommentResult { - let removed = false; - let remainingCommentCount = 0; - const next: Record = {}; - - for (const [fileId, comments] of Object.entries(this.liveCommentsByFileId)) { - const filtered = comments.filter((comment) => comment.id !== commentId); - if (filtered.length !== comments.length) { - removed = true; - } - if (filtered.length > 0) { - next[fileId] = filtered; - remainingCommentCount += filtered.length; - } - } - - if (!removed) { - throw new Error(`No live comment matches id ${commentId}.`); - } - - this.liveCommentsByFileId = next; - this.updateHeadlessSnapshot(); - return { commentId, removed: true, remainingCommentCount }; + return this.applyHeadlessTransition(removeReviewLiveComment(this.reviewState, commentId)); } /** Clear persisted live comments, optionally scoped to one file. */ private clearHeadlessLiveComments(filePath?: string): ClearedCommentsResult { - let removedCount = 0; - let remainingCommentCount = 0; - - if (filePath) { - const file = findDiffFileByPath(this.renderSnapshot.bootstrap.changeset.files, filePath); - if (!file) { - throw new Error(`No diff file matches ${filePath}.`); - } - - const next: Record = {}; - for (const [fileId, comments] of Object.entries(this.liveCommentsByFileId)) { - if (fileId === file.id) { - removedCount = comments.length; - continue; - } - next[fileId] = comments; - remainingCommentCount += comments.length; - } - this.liveCommentsByFileId = next; - } else { - removedCount = Object.values(this.liveCommentsByFileId).reduce( - (sum, comments) => sum + comments.length, - 0, - ); - this.liveCommentsByFileId = {}; - } - - this.updateHeadlessSnapshot(); - return { removedCount, remainingCommentCount, filePath }; - } - - /** Update the persisted selection fields from one file/hunk target. */ - private selectHeadlessHunk(fileId: string, filePath: string, hunkIndex: number) { - const file = this.renderSnapshot.bootstrap.changeset.files.find( - (candidate) => candidate.id === fileId, + return this.applyHeadlessTransition( + clearReviewLiveComments({ + files: this.renderSnapshot.bootstrap.changeset.files, + state: this.reviewState, + filePath, + }), ); - const hunk = file?.metadata.hunks[hunkIndex]; - const range = hunk ? hunkLineRange(hunk) : null; - this.sessionSnapshot.state.selectedFileId = fileId; - this.sessionSnapshot.state.selectedFilePath = filePath; - this.sessionSnapshot.state.selectedHunkIndex = hunkIndex; - this.sessionSnapshot.state.selectedHunkOldRange = range?.oldRange; - this.sessionSnapshot.state.selectedHunkNewRange = range?.newRange; } } diff --git a/src/hunk-session/liveCommentSummaries.ts b/src/hunk-session/liveCommentSummaries.ts deleted file mode 100644 index fd497dad..00000000 --- a/src/hunk-session/liveCommentSummaries.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { findDiffFileByPath, type LiveComment } from "../core/liveComments"; -import type { DiffFile } from "../core/types"; -import type { SessionLiveCommentSummary } from "./types"; - -/** Convert a live annotation into the broker snapshot's compact summary shape. */ -export function summarizeLiveComment( - filePath: string, - comment: LiveComment, -): SessionLiveCommentSummary { - return { - commentId: comment.id, - filePath, - hunkIndex: comment.hunkIndex, - side: comment.side, - line: comment.line, - summary: comment.summary, - rationale: comment.rationale, - author: comment.author, - createdAt: comment.createdAt, - }; -} - -/** Rehydrate one broker snapshot comment into a live annotation. */ -function liveCommentFromSummary(summary: SessionLiveCommentSummary): LiveComment { - return { - id: summary.commentId, - source: "mcp", - filePath: summary.filePath, - hunkIndex: summary.hunkIndex, - side: summary.side, - line: summary.line, - summary: summary.summary, - rationale: summary.rationale, - author: summary.author, - createdAt: summary.createdAt, - oldRange: summary.side === "old" ? [summary.line, summary.line] : undefined, - newRange: summary.side === "new" ? [summary.line, summary.line] : undefined, - tags: ["mcp"], - confidence: "high", - }; -} - -/** Rehydrate broker snapshot comments into a file-id keyed annotation map. */ -export function liveCommentsByFileFromSummaries( - files: DiffFile[], - summaries: SessionLiveCommentSummary[] = [], -) { - const byFileId: Record = {}; - summaries.forEach((summary) => { - const file = findDiffFileByPath(files, summary.filePath); - if (!file) return; - byFileId[file.id] = [...(byFileId[file.id] ?? []), liveCommentFromSummary(summary)]; - }); - return byFileId; -} diff --git a/src/hunk-session/reviewCommandState.ts b/src/hunk-session/reviewCommandState.ts new file mode 100644 index 00000000..ed4ca181 --- /dev/null +++ b/src/hunk-session/reviewCommandState.ts @@ -0,0 +1,498 @@ +import { + buildLiveComment, + findDiffFileByPath, + hunkLineRange, + resolveCommentTarget, + type LiveComment, +} from "../core/liveComments"; +import type { AgentAnnotation, DiffFile } from "../core/types"; +import { reviewNoteSource } from "../ui/lib/agentAnnotations"; +import { mergeFileAnnotationsByFileId } from "../ui/lib/files"; +import { buildSelectedHunkSummary, resolveReviewNavigationTarget } from "../ui/lib/reviewState"; +import type { + AppliedCommentBatchResult, + AppliedCommentResult, + ClearedCommentsResult, + CommentBatchItemInput, + CommentToolInput, + HunkSessionSnapshot, + HunkSessionState, + NavigateToHunkToolInput, + NavigatedSelectionResult, + RemovedCommentResult, + SessionLiveCommentSummary, + SessionReviewNoteSummary, +} from "./types"; + +export interface SavedUserReviewNote extends AgentAnnotation { + id: string; + source: "user"; + fileId: string; + filePath: string; + hunkIndex: number; + side: "old" | "new"; + line: number; + author: string; + createdAt: string; + editable: true; +} + +export interface ReviewCommandState { + selectedFileId: string; + selectedHunkIndex: number; + showAgentNotes: boolean; + liveCommentsByFileId: Record; + userNotesByFileId: Record; +} + +function countFileMapItems(byFileId: Record) { + return Object.values(byFileId).reduce((sum, items) => sum + items.length, 0); +} + +function appendMapItem(byFileId: Record, fileId: string, item: T) { + return { ...byFileId, [fileId]: [...(byFileId[fileId] ?? []), item] }; +} + +function removeMapItem(byFileId: Record, id: string) { + let removed = false; + const next: Record = {}; + + for (const [fileId, items] of Object.entries(byFileId)) { + const filtered = items.filter((item) => item.id !== id); + removed ||= filtered.length !== items.length; + if (filtered.length > 0) next[fileId] = filtered; + } + + return { next, removed, remainingCount: countFileMapItems(next) }; +} + +function liveCommentsByFileFromSummaries( + files: DiffFile[], + summaries: SessionLiveCommentSummary[] = [], +) { + const byFileId: Record = {}; + summaries.forEach((summary) => { + const file = findDiffFileByPath(files, summary.filePath); + if (!file) return; + const comments = (byFileId[file.id] ??= []); + comments.push( + buildLiveComment(summary, summary.commentId, summary.createdAt, summary.hunkIndex), + ); + }); + return byFileId; +} + +function summarizeLiveComment(filePath: string, comment: LiveComment): SessionLiveCommentSummary { + return { + commentId: comment.id, + filePath, + hunkIndex: comment.hunkIndex, + side: comment.side, + line: comment.line, + summary: comment.summary, + rationale: comment.rationale, + author: comment.author, + createdAt: comment.createdAt, + }; +} + +/** Create command state from a fresh review, optionally rehydrating a broker snapshot. */ +export function createReviewCommandState({ + files, + initialSessionState, + initialShowAgentNotes = false, +}: { + files: DiffFile[]; + initialSessionState?: HunkSessionState; + initialShowAgentNotes?: boolean; +}): ReviewCommandState { + const selectedFile = + files.find((file) => file.id === initialSessionState?.selectedFileId) ?? + (initialSessionState?.selectedFilePath + ? findDiffFileByPath(files, initialSessionState.selectedFilePath) + : undefined) ?? + files[0]; + + return reconcileReviewCommandSelection({ + allFiles: files, + visibleFiles: files, + state: { + selectedFileId: selectedFile?.id ?? "", + selectedHunkIndex: initialSessionState?.selectedHunkIndex ?? 0, + showAgentNotes: initialSessionState?.showAgentNotes ?? initialShowAgentNotes, + liveCommentsByFileId: liveCommentsByFileFromSummaries( + files, + initialSessionState?.liveComments, + ), + userNotesByFileId: {}, + }, + }); +} + +function reviewCommandAnnotationsByFileId(state: ReviewCommandState) { + const next: Record = { ...state.liveCommentsByFileId }; + for (const [fileId, annotations] of Object.entries(state.userNotesByFileId)) { + next[fileId] = [...(next[fileId] ?? []), ...annotations]; + } + return next; +} + +/** Merge command-owned annotations into review files for command navigation. */ +export function reviewCommandFiles(files: DiffFile[], state: ReviewCommandState): DiffFile[] { + return mergeFileAnnotationsByFileId(files, reviewCommandAnnotationsByFileId(state)); +} + +/** Keep command selection anchored to the currently available review stream. */ +export function reconcileReviewCommandSelection({ + allFiles, + state, + visibleFiles, +}: { + allFiles: DiffFile[]; + state: ReviewCommandState; + visibleFiles: DiffFile[]; +}): ReviewCommandState { + if (visibleFiles.length === 0) { + return state; + } + + const selectedFile = allFiles.find((file) => file.id === state.selectedFileId); + const selectedVisible = selectedFile + ? visibleFiles.some((file) => file.id === selectedFile.id) + : false; + + if (!state.selectedFileId || !selectedFile || !selectedVisible) { + const firstVisibleFile = visibleFiles[0]!; + return selectReviewHunk(state, { fileId: firstVisibleFile.id, hunkIndex: 0 }); + } + + const maxIndex = Math.max(0, selectedFile.metadata.hunks.length - 1); + const selectedHunkIndex = Math.min(Math.max(state.selectedHunkIndex, 0), maxIndex); + return selectedHunkIndex === state.selectedHunkIndex ? state : { ...state, selectedHunkIndex }; +} + +/** Select one file and hunk in command state. */ +export function selectReviewHunk( + state: ReviewCommandState, + { fileId, hunkIndex }: { fileId: string; hunkIndex: number }, +): ReviewCommandState { + return state.selectedFileId === fileId && state.selectedHunkIndex === hunkIndex + ? state + : { ...state, selectedFileId: fileId, selectedHunkIndex: hunkIndex }; +} + +/** Toggle the persistent review-note visibility bit in command state. */ +export function setReviewAgentNotesVisible( + state: ReviewCommandState, + visible: boolean, +): ReviewCommandState { + return state.showAgentNotes === visible ? state : { ...state, showAgentNotes: visible }; +} + +function addLiveComments({ + files, + inputs, + now, + revealFirst, + state, +}: { + files: DiffFile[]; + state: ReviewCommandState; + inputs: Array<{ commentId: string; input: CommentBatchItemInput }>; + now: string; + revealFirst: boolean; +}): { applied: AppliedCommentResult[]; state: ReviewCommandState } { + const prepared = inputs.map(({ commentId, input }) => { + const file = findDiffFileByPath(files, input.filePath); + if (!file) throw new Error(`No diff file matches ${input.filePath}.`); + + const target = resolveCommentTarget(file, input); + const liveComment = buildLiveComment( + { ...input, side: target.side, line: target.line }, + commentId, + now, + target.hunkIndex, + ); + return { file, liveComment, target }; + }); + + let liveCommentsByFileId = state.liveCommentsByFileId; + prepared.forEach(({ file, liveComment }) => { + liveCommentsByFileId = appendMapItem(liveCommentsByFileId, file.id, liveComment); + }); + + let nextState: ReviewCommandState = { ...state, liveCommentsByFileId }; + if (revealFirst && prepared[0]) { + nextState = setReviewAgentNotesVisible( + selectReviewHunk(nextState, { + fileId: prepared[0].file.id, + hunkIndex: prepared[0].target.hunkIndex, + }), + true, + ); + } + + return { + state: nextState, + applied: prepared.map(({ file, liveComment, target }) => ({ + commentId: liveComment.id, + fileId: file.id, + filePath: file.path, + hunkIndex: target.hunkIndex, + side: target.side, + line: target.line, + })), + }; +} + +/** Add one live agent comment to command state. */ +export function addReviewLiveComment({ + commentId, + files, + input, + now, + options = {}, + state, +}: { + files: DiffFile[]; + state: ReviewCommandState; + input: CommentToolInput; + commentId: string; + now: string; + options?: { reveal?: boolean }; +}): { state: ReviewCommandState; result: AppliedCommentResult } { + const next = addLiveComments({ + files, + state, + inputs: [{ commentId, input }], + now, + revealFirst: options.reveal ?? false, + }); + return { state: next.state, result: next.applied[0]! }; +} + +/** Apply several live comments after validating every target against the same input state. */ +export function addReviewLiveCommentBatch({ + files, + inputs, + now, + options = {}, + requestId, + state, +}: { + files: DiffFile[]; + state: ReviewCommandState; + inputs: CommentBatchItemInput[]; + requestId: string; + now: string; + options?: { revealMode?: "none" | "first" }; +}): { state: ReviewCommandState; result: AppliedCommentBatchResult } { + const next = addLiveComments({ + files, + state, + inputs: inputs.map((input, index) => ({ commentId: `mcp:${requestId}:${index}`, input })), + now, + revealFirst: options.revealMode === "first", + }); + return { state: next.state, result: { applied: next.applied } }; +} + +/** Remove one live comment by id. */ +export function removeReviewLiveComment( + state: ReviewCommandState, + commentId: string, +): { state: ReviewCommandState; result: RemovedCommentResult } { + const removed = removeMapItem(state.liveCommentsByFileId, commentId); + if (!removed.removed) throw new Error(`No live comment matches id ${commentId}.`); + + return { + state: { ...state, liveCommentsByFileId: removed.next }, + result: { commentId, removed: true, remainingCommentCount: removed.remainingCount }, + }; +} + +/** Clear all live comments, or only the comments attached to one file. */ +export function clearReviewLiveComments({ + filePath, + files, + state, +}: { + files: DiffFile[]; + state: ReviewCommandState; + filePath?: string; +}): { state: ReviewCommandState; result: ClearedCommentsResult } { + let liveCommentsByFileId: Record = {}; + let removedCount = countFileMapItems(state.liveCommentsByFileId); + + if (filePath) { + const file = findDiffFileByPath(files, filePath); + if (!file) throw new Error(`No diff file matches ${filePath}.`); + + liveCommentsByFileId = { ...state.liveCommentsByFileId }; + removedCount = liveCommentsByFileId[file.id]?.length ?? 0; + delete liveCommentsByFileId[file.id]; + } + + const remainingCommentCount = countFileMapItems(liveCommentsByFileId); + return { + state: removedCount === 0 ? state : { ...state, liveCommentsByFileId }, + result: { removedCount, remainingCommentCount, filePath }, + }; +} + +/** Add one saved user review note to command state. */ +export function addSavedUserReviewNote( + state: ReviewCommandState, + note: SavedUserReviewNote, +): ReviewCommandState { + return { + ...state, + userNotesByFileId: appendMapItem(state.userNotesByFileId, note.fileId, note), + }; +} + +/** Remove one saved user review note by id. */ +export function removeSavedUserReviewNote( + state: ReviewCommandState, + noteId: string, +): ReviewCommandState { + const removed = removeMapItem(state.userNotesByFileId, noteId); + if (!removed.removed) throw new Error(`No user note matches id ${noteId}.`); + return { ...state, userNotesByFileId: removed.next }; +} + +/** Navigate persistent command selection using a session-daemon navigation request. */ +export function navigateReviewCommandState({ + allFiles, + input, + state, + visibleFiles, +}: { + allFiles: DiffFile[]; + visibleFiles: DiffFile[]; + state: ReviewCommandState; + input: NavigateToHunkToolInput; +}): { state: ReviewCommandState; result: NavigatedSelectionResult; scrollToNote: boolean } { + const target = resolveReviewNavigationTarget({ + allFiles, + currentFileId: state.selectedFileId, + currentHunkIndex: state.selectedHunkIndex, + input, + visibleFiles, + }); + + return { + state: selectReviewHunk(state, { fileId: target.file.id, hunkIndex: target.hunkIndex }), + scrollToNote: target.scrollToNote, + result: { + fileId: target.file.id, + filePath: target.file.path, + hunkIndex: target.hunkIndex, + selectedHunk: buildSelectedHunkSummary(target.file, target.hunkIndex), + }, + }; +} + +/** Return all session-owned live comments in file order. */ +function reviewLiveCommentSummaries( + files: DiffFile[], + state: ReviewCommandState, +): SessionLiveCommentSummary[] { + return files.flatMap((file) => + (state.liveCommentsByFileId[file.id] ?? []).map((comment) => + summarizeLiveComment(file.path, comment), + ), + ); +} + +/** Return all review notes visible to session commands and review exports. */ +function reviewNoteSummaries( + files: DiffFile[], + state: ReviewCommandState, +): SessionReviewNoteSummary[] { + const summaries: SessionReviewNoteSummary[] = []; + + files.forEach((file) => { + (file.agent?.annotations ?? []).forEach((annotation, index) => { + const source = reviewNoteSource(annotation); + summaries.push({ + noteId: annotation.id ?? `${source}:${file.id}:${index}`, + source, + filePath: file.path, + oldRange: annotation.oldRange, + newRange: annotation.newRange, + body: [annotation.summary, annotation.rationale].filter(Boolean).join("\n\n"), + title: annotation.title, + author: annotation.author, + createdAt: annotation.createdAt ?? "1970-01-01T00:00:00.000Z", + updatedAt: annotation.updatedAt, + editable: false, + }); + }); + + (state.liveCommentsByFileId[file.id] ?? []).forEach((comment) => { + summaries.push({ + noteId: comment.id, + source: "agent", + filePath: file.path, + hunkIndex: comment.hunkIndex, + oldRange: comment.oldRange, + newRange: comment.newRange, + body: [comment.summary, comment.rationale].filter(Boolean).join("\n\n"), + author: comment.author, + createdAt: comment.createdAt, + editable: false, + }); + }); + + (state.userNotesByFileId[file.id] ?? []).forEach((note) => { + summaries.push({ + noteId: note.id, + source: "user", + filePath: file.path, + hunkIndex: note.hunkIndex, + oldRange: note.oldRange, + newRange: note.newRange, + body: note.summary, + author: note.author, + createdAt: note.createdAt, + editable: true, + }); + }); + }); + + return summaries; +} + +/** Build the broker-facing snapshot for the current command state. */ +export function buildReviewSessionSnapshot({ + files, + now, + state, +}: { + files: DiffFile[]; + state: ReviewCommandState; + now: string; +}): HunkSessionSnapshot { + const selectedFile = files.find((file) => file.id === state.selectedFileId); + const selectedHunk = selectedFile?.metadata.hunks[state.selectedHunkIndex]; + const selectedRange = selectedHunk ? hunkLineRange(selectedHunk) : undefined; + const liveComments = reviewLiveCommentSummaries(files, state); + const reviewNotes = reviewNoteSummaries(files, state); + + return { + updatedAt: now, + state: { + selectedFileId: selectedFile?.id, + selectedFilePath: selectedFile?.path, + selectedHunkIndex: state.selectedHunkIndex, + selectedHunkOldRange: selectedRange?.oldRange, + selectedHunkNewRange: selectedRange?.newRange, + showAgentNotes: state.showAgentNotes, + liveCommentCount: liveComments.length, + liveComments, + reviewNoteCount: reviewNotes.length, + reviewNotes, + }, + }; +} diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 299d0357..e689a99d 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -120,9 +120,6 @@ export function App({ ); // Soft reloads replace bootstrap without re-running startup terminal theme detection. const [detectedThemeMode] = useState(() => bootstrap.initialThemeMode); - const [showAgentNotes, setShowAgentNotes] = useState( - initialSessionState?.showAgentNotes ?? bootstrap.initialShowAgentNotes ?? false, - ); const [showLineNumbers, setShowLineNumbers] = useState(bootstrap.initialShowLineNumbers ?? true); const [wrapLines, setWrapLines] = useState(bootstrap.initialWrapLines ?? false); const [copyDecorations, setCopyDecorations] = useState(bootstrap.initialCopyDecorations ?? false); @@ -142,9 +139,8 @@ export function App({ const activeTheme = resolveTheme(themeId, detectedThemeMode ?? null); const review = useReviewController({ files: bootstrap.changeset.files, - initialLiveComments: initialSessionState?.liveComments, - initialSelectedFileId: initialSessionState?.selectedFileId, - initialSelectedHunkIndex: initialSessionState?.selectedHunkIndex, + initialSessionState, + initialShowAgentNotes: bootstrap.initialShowAgentNotes ?? false, }); const filteredFiles = review.visibleFiles; const selectedFile = review.selectedFile; @@ -163,8 +159,8 @@ export function App({ ); const openAgentNotes = useCallback(() => { - setShowAgentNotes(true); - }, []); + review.setAgentNotesVisible(true); + }, [review.setAgentNotesVisible]); const showSessionNotice = useCallback((message: string) => { setSessionNoticeText(message); @@ -191,18 +187,11 @@ export function App({ addLiveCommentBatch: review.addLiveCommentBatch, clearLiveComments: review.clearLiveComments, hostClient, - liveCommentCount: review.liveCommentCount, - liveCommentSummaries: review.liveCommentSummaries, navigateToLocation: review.navigateToLocation, openAgentNotes, reloadSession: onReloadSession, removeLiveComment: review.removeLiveComment, - reviewNoteCount: review.reviewNoteCount, - reviewNoteSummaries: review.reviewNoteSummaries, - selectedFile, - selectedHunk: review.selectedHunk, - selectedHunkIndex, - showAgentNotes, + sessionSnapshot: review.sessionSnapshot, }); const bodyPadding = pagerMode ? 0 : BODY_PADDING; @@ -331,10 +320,8 @@ export function App({ setLayoutMode(mode); }, []); - /** Toggle the global agent note layer on or off. */ - const toggleAgentNotes = () => { - setShowAgentNotes((current) => !current); - }; + const showAgentNotes = review.showAgentNotes; + const toggleAgentNotes = review.toggleAgentNotes; /** Toggle line-number gutters without changing the diff content itself. */ const toggleLineNumbers = () => { diff --git a/src/ui/hooks/useHunkSessionBridge.ts b/src/ui/hooks/useHunkSessionBridge.ts index d7c9d690..7c76dab8 100644 --- a/src/ui/hooks/useHunkSessionBridge.ts +++ b/src/ui/hooks/useHunkSessionBridge.ts @@ -1,12 +1,10 @@ import { useEffect, useMemo } from "react"; -import type { CliInput, DiffFile } from "../../core/types"; -import { hunkLineRange } from "../../core/liveComments"; +import type { CliInput } from "../../core/types"; import { createHunkSessionBridge } from "../../hunk-session/bridge"; import type { HunkSessionBrokerClient, + HunkSessionSnapshot, ReloadedSessionResult, - SessionLiveCommentSummary, - SessionReviewNoteSummary, } from "../../hunk-session/types"; import type { ReviewController } from "./useReviewController"; @@ -16,25 +14,16 @@ export function useHunkSessionBridge({ addLiveCommentBatch, clearLiveComments, hostClient, - liveCommentCount, - liveCommentSummaries, navigateToLocation, openAgentNotes, reloadSession, removeLiveComment, - reviewNoteCount, - reviewNoteSummaries, - selectedFile, - selectedHunk, - selectedHunkIndex, - showAgentNotes, + sessionSnapshot, }: { addLiveComment: ReviewController["addLiveComment"]; addLiveCommentBatch: ReviewController["addLiveCommentBatch"]; clearLiveComments: ReviewController["clearLiveComments"]; hostClient?: HunkSessionBrokerClient; - liveCommentCount: number; - liveCommentSummaries: SessionLiveCommentSummary[]; navigateToLocation: ReviewController["navigateToLocation"]; openAgentNotes: () => void; reloadSession: ( @@ -42,12 +31,7 @@ export function useHunkSessionBridge({ options?: { resetApp?: boolean; sourcePath?: string }, ) => Promise; removeLiveComment: ReviewController["removeLiveComment"]; - reviewNoteCount: number; - reviewNoteSummaries: SessionReviewNoteSummary[]; - selectedFile: DiffFile | undefined; - selectedHunk: DiffFile["metadata"]["hunks"][number] | undefined; - selectedHunkIndex: number; - showAgentNotes: boolean; + sessionSnapshot: HunkSessionSnapshot; }) { const bridge = useMemo( () => @@ -57,7 +41,7 @@ export function useHunkSessionBridge({ clearLiveComments, navigateToLocation, openAgentNotes, - reloadSession: (nextInput, options) => reloadSession(nextInput, { ...options }), + reloadSession, removeLiveComment, }), [ @@ -84,33 +68,6 @@ export function useHunkSessionBridge({ }, [bridge, hostClient]); useEffect(() => { - const selectedRange = selectedHunk ? hunkLineRange(selectedHunk) : undefined; - - hostClient?.updateSnapshot({ - updatedAt: new Date().toISOString(), - state: { - selectedFileId: selectedFile?.id, - selectedFilePath: selectedFile?.path, - selectedHunkIndex, - selectedHunkOldRange: selectedRange?.oldRange, - selectedHunkNewRange: selectedRange?.newRange, - showAgentNotes, - liveCommentCount, - liveComments: liveCommentSummaries, - reviewNoteCount, - reviewNotes: reviewNoteSummaries, - }, - }); - }, [ - hostClient, - liveCommentCount, - liveCommentSummaries, - reviewNoteCount, - reviewNoteSummaries, - selectedFile?.id, - selectedFile?.path, - selectedHunk, - selectedHunkIndex, - showAgentNotes, - ]); + hostClient?.updateSnapshot(sessionSnapshot); + }, [hostClient, sessionSnapshot]); } diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index 246462cc..4061ad84 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -361,6 +361,43 @@ describe("useReviewController", () => { } }); + test("explicit session navigation reveals the selected hunk", async () => { + const controllerRef: { current: ReviewController | null } = { current: null }; + const setup = await testRender( + { + controllerRef.current = nextController; + }} + />, + { width: 80, height: 4 }, + ); + + try { + await flush(setup); + const initialRevealRequestId = expectValue(controllerRef.current).selectedHunkRevealRequestId; + + await act(async () => { + const result = expectValue(controllerRef.current).navigateToLocation({ + filePath: "alpha.ts", + hunkIndex: 1, + }); + expect(result).toMatchObject({ filePath: "alpha.ts", hunkIndex: 1 }); + }); + await flush(setup); + + expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(1); + expect(expectValue(controllerRef.current).selectedHunkRevealRequestId).toBe( + initialRevealRequestId + 1, + ); + expect(expectValue(controllerRef.current).scrollToNote).toBe(false); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("batch live comments validate together and reveal the first applied hunk", async () => { const controllerRef: { current: ReviewController | null } = { current: null }; const setup = await testRender( diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index 25a2b263..7b38e9f8 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -6,31 +6,34 @@ * navigation. `App` uses it for rendering and keyboard or menu actions, while * the session bridge uses the same state and actions for daemon-driven navigation. */ +import { useCallback, useDeferredValue, useEffect, useMemo, useRef, useState } from "react"; +import { firstCommentTargetForHunk } from "../../core/liveComments"; +import type { DiffFile, UserNoteLineTarget } from "../../core/types"; import { - startTransition, - useCallback, - useDeferredValue, - useEffect, - useMemo, - useState, -} from "react"; -import { - buildLiveComment, - findDiffFileByPath, - firstCommentTargetForHunk, - resolveCommentTarget, -} from "../../core/liveComments"; -import type { AgentAnnotation, DiffFile, UserNoteLineTarget } from "../../core/types"; -import { - liveCommentsByFileFromSummaries, - summarizeLiveComment, -} from "../../hunk-session/liveCommentSummaries"; + addReviewLiveComment, + addReviewLiveCommentBatch, + addSavedUserReviewNote, + buildReviewSessionSnapshot, + clearReviewLiveComments, + createReviewCommandState, + navigateReviewCommandState, + reconcileReviewCommandSelection, + removeReviewLiveComment, + removeSavedUserReviewNote, + reviewCommandFiles, + selectReviewHunk, + setReviewAgentNotesVisible, + type ReviewCommandState, + type SavedUserReviewNote, +} from "../../hunk-session/reviewCommandState"; import type { AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, CommentBatchItemInput, CommentToolInput, + HunkSessionSnapshot, + HunkSessionState, LiveComment, NavigateToHunkToolInput, NavigatedSelectionResult, @@ -39,47 +42,7 @@ import type { SessionReviewNoteSummary, } from "../../hunk-session/types"; import { findNextHunkCursor } from "../lib/hunks"; -import { reviewNoteSource } from "../lib/agentAnnotations"; -import { - buildReviewState, - buildSelectedHunkSummary, - findNextAnnotatedFile, - type ReviewState, - resolveReviewNavigationTarget, -} from "../lib/reviewState"; - -/** Clamp one numeric index into an inclusive range. */ -function clamp(value: number, min: number, max: number) { - return Math.min(Math.max(value, min), max); -} - -/** Merge file-id keyed annotation maps without losing their concrete item types. */ -function mergeAnnotationMaps( - first: Record, - second: Record, -): Record> { - const next: Record> = {}; - for (const [fileId, annotations] of Object.entries(first)) { - next[fileId] = [...annotations]; - } - for (const [fileId, annotations] of Object.entries(second)) { - next[fileId] = [...(next[fileId] ?? []), ...annotations]; - } - return next; -} - -export interface UserReviewNote extends AgentAnnotation { - id: string; - source: "user"; - filePath: string; - hunkIndex: number; - side: "old" | "new"; - line: number; - summary: string; - author: string; - createdAt: string; - editable: true; -} +import { buildReviewState, findNextAnnotatedFile, type ReviewState } from "../lib/reviewState"; export interface DraftReviewNote { id: string; @@ -108,7 +71,9 @@ export interface ReviewController { liveCommentsByFileId: Record; reviewNoteCount: number; reviewNoteSummaries: SessionReviewNoteSummary[]; - userNotesByFileId: Record; + sessionSnapshot: HunkSessionSnapshot; + showAgentNotes: boolean; + userNotesByFileId: Record; moveToAnnotatedFile: (delta: number) => void; moveToAnnotatedHunk: (delta: number) => void; moveToFile: (delta: number) => void; @@ -138,7 +103,7 @@ export interface ReviewController { removeLiveComment: (commentId: string) => RemovedCommentResult; cancelDraftNote: () => void; removeUserNote: (noteId: string) => void; - saveDraftNote: () => UserReviewNote | null; + saveDraftNote: () => SavedUserReviewNote | null; selectFile: (fileId: string, nextHunkIndex?: number, options?: ReviewSelectionOptions) => void; selectHunk: (fileId: string, hunkIndex: number, options?: ReviewSelectionOptions) => void; startUserNote: ( @@ -146,35 +111,60 @@ export interface ReviewController { hunkIndex?: number, target?: UserNoteLineTarget, ) => DraftReviewNote | null; + setAgentNotesVisible: (visible: boolean) => void; setFilter: (value: string) => void; + toggleAgentNotes: () => void; updateDraftNote: (body: string) => void; } /** Own the shared review stream state used by both the UI and session bridge. */ export function useReviewController({ files, - initialLiveComments, - initialSelectedFileId, - initialSelectedHunkIndex, + initialSessionState, + initialShowAgentNotes, }: { files: DiffFile[]; - initialLiveComments?: SessionLiveCommentSummary[]; - initialSelectedFileId?: string; - initialSelectedHunkIndex?: number; + initialSessionState?: HunkSessionState; + initialShowAgentNotes?: boolean; }): ReviewController { const [filter, setFilter] = useState(""); - const [selectedFileId, setSelectedFileId] = useState(initialSelectedFileId ?? files[0]?.id ?? ""); - const [selectedHunkIndex, setSelectedHunkIndex] = useState(initialSelectedHunkIndex ?? 0); + const [commandState, setCommandState] = useState(() => + createReviewCommandState({ files, initialSessionState, initialShowAgentNotes }), + ); + const commandStateRef = useRef(commandState); const [selectedFileTopAlignRequestId, setSelectedFileTopAlignRequestId] = useState(0); const [selectedHunkRevealRequestId, setSelectedHunkRevealRequestId] = useState(0); const [scrollToNote, setScrollToNote] = useState(false); - const [liveCommentsByFileId, setLiveCommentsByFileId] = useState>( - () => liveCommentsByFileFromSummaries(files, initialLiveComments), - ); - const [userNotesByFileId, setUserNotesByFileId] = useState>({}); const [draftNote, setDraftNote] = useState(null); const deferredFilter = useDeferredValue(filter); + useEffect(() => { + commandStateRef.current = commandState; + }, [commandState]); + + /** Update command state and its imperative mirror together. */ + const updateCommandState = useCallback( + (updater: (state: ReviewCommandState) => ReviewCommandState) => { + setCommandState((current) => { + const next = updater(current); + commandStateRef.current = next; + return next; + }); + }, + [], + ); + + /** Apply a command-state transition synchronously so session command replies match state. */ + const applyCommandStateTransition = useCallback( + (transition: (state: ReviewCommandState) => { state: ReviewCommandState; result: T }) => { + const next = transition(commandStateRef.current); + commandStateRef.current = next.state; + setCommandState(next.state); + return next.result; + }, + [], + ); + const { allFiles, visibleFiles, @@ -186,27 +176,23 @@ export function useReviewController({ } = useMemo( () => buildReviewState({ - files, - liveCommentsByFileId: mergeAnnotationMaps(liveCommentsByFileId, userNotesByFileId), + files: reviewCommandFiles(files, commandState), filterQuery: deferredFilter, - selectedFileId, - selectedHunkIndex, + selectedFileId: commandState.selectedFileId, + selectedHunkIndex: commandState.selectedHunkIndex, }), - [ - deferredFilter, - files, - liveCommentsByFileId, - selectedFileId, - selectedHunkIndex, - userNotesByFileId, - ], + [deferredFilter, files, commandState], ); + const selectedFileId = commandState.selectedFileId; + const selectedHunkIndex = commandState.selectedHunkIndex; + const showAgentNotes = commandState.showAgentNotes; + const liveCommentsByFileId = commandState.liveCommentsByFileId; + const userNotesByFileId = commandState.userNotesByFileId; /** Update the selection and reveal intent together so diff scrolling stays explicit. */ const selectHunk = useCallback( (fileId: string, hunkIndex: number, options?: ReviewSelectionOptions) => { - setSelectedFileId(fileId); - setSelectedHunkIndex(hunkIndex); + updateCommandState((current) => selectReviewHunk(current, { fileId, hunkIndex })); setScrollToNote(Boolean(options?.scrollToNote)); if (options?.alignFileHeaderTop) { @@ -218,7 +204,7 @@ export function useReviewController({ setSelectedHunkRevealRequestId((current) => current + 1); } }, - [], + [updateCommandState], ); /** Select one file and optionally one specific hunk within it. */ @@ -229,47 +215,11 @@ export function useReviewController({ [selectHunk], ); - /** Reset selection to the first visible file when the current target disappears from the review stream. */ - const reselectFirstVisibleFile = useCallback(() => { - startTransition(() => { - setSelectedFileId(visibleFiles[0]!.id); - setSelectedHunkIndex(0); - }); - }, [visibleFiles]); - - /** Keep the selected file anchored to the current visible review stream as filters and reloads change it. */ - const reconcileSelectedFile = useCallback(() => { - if (visibleFiles.length === 0) { - return; - } - - if (!selectedFileId || !allFiles.some((file) => file.id === selectedFileId)) { - reselectFirstVisibleFile(); - return; - } - - if (selectedFile && !visibleFiles.some((file) => file.id === selectedFile.id)) { - reselectFirstVisibleFile(); - } - }, [allFiles, reselectFirstVisibleFile, selectedFile, selectedFileId, visibleFiles]); - - /** Clamp the selected hunk index after reloads or filter changes shrink the selected file's hunk list. */ - const reconcileSelectedHunkIndex = useCallback(() => { - if (!selectedFile) { - return; - } - - const maxIndex = Math.max(0, selectedFile.metadata.hunks.length - 1); - setSelectedHunkIndex((current) => clamp(current, 0, maxIndex)); - }, [selectedFile]); - - useEffect(() => { - reconcileSelectedFile(); - }, [reconcileSelectedFile]); - useEffect(() => { - reconcileSelectedHunkIndex(); - }, [reconcileSelectedHunkIndex]); + updateCommandState((current) => + reconcileReviewCommandSelection({ allFiles, visibleFiles, state: current }), + ); + }, [allFiles, updateCommandState, visibleFiles]); /** Move through the full visible review stream one hunk at a time. */ const moveToHunk = useCallback( @@ -336,7 +286,7 @@ export function useReviewController({ return; } - const nextIndex = clamp(currentIndex + delta, 0, visibleFiles.length - 1); + const nextIndex = Math.min(Math.max(currentIndex + delta, 0), visibleFiles.length - 1); if (nextIndex === currentIndex) { return; } @@ -359,23 +309,22 @@ export function useReviewController({ /** Resolve one session-daemon navigation request against the current review state and select it. */ const navigateToLocation = useCallback( (input: NavigateToHunkToolInput): NavigatedSelectionResult => { - const target = resolveReviewNavigationTarget({ - allFiles, - currentFileId: selectedFile?.id, - currentHunkIndex: selectedHunkIndex, - input, - visibleFiles, + let scrollToNoteAfterNavigation = false; + const result = applyCommandStateTransition((state) => { + const transition = navigateReviewCommandState({ + allFiles, + visibleFiles, + state, + input, + }); + scrollToNoteAfterNavigation = transition.scrollToNote; + return transition; }); - - selectHunk(target.file.id, target.hunkIndex, { scrollToNote: target.scrollToNote }); - return { - fileId: target.file.id, - filePath: target.file.path, - hunkIndex: target.hunkIndex, - selectedHunk: buildSelectedHunkSummary(target.file, target.hunkIndex), - }; + setScrollToNote(scrollToNoteAfterNavigation); + setSelectedHunkRevealRequestId((current) => current + 1); + return result; }, - [allFiles, selectHunk, selectedFile?.id, selectedHunkIndex, visibleFiles], + [allFiles, applyCommandStateTransition, visibleFiles], ); /** Add one live comment, optionally revealing its hunk in the active review. */ @@ -385,42 +334,26 @@ export function useReviewController({ commentId: string, options?: { reveal?: boolean }, ): AppliedCommentResult => { - const file = findDiffFileByPath(allFiles, input.filePath); - if (!file) { - throw new Error(`No diff file matches ${input.filePath}.`); - } - - const target = resolveCommentTarget(file, input); - - const liveComment = buildLiveComment( - { - ...input, - side: target.side, - line: target.line, - }, - commentId, - new Date().toISOString(), - target.hunkIndex, + const now = new Date().toISOString(); + const result = applyCommandStateTransition((state) => + addReviewLiveComment({ + files: allFiles, + state, + input, + commentId, + now, + options, + }), ); - setLiveCommentsByFileId((current) => ({ - ...current, - [file.id]: [...(current[file.id] ?? []), liveComment], - })); if (options?.reveal ?? false) { - selectHunk(file.id, target.hunkIndex, { scrollToNote: true }); + setScrollToNote(true); + setSelectedHunkRevealRequestId((current) => current + 1); } - return { - commentId, - fileId: file.id, - filePath: file.path, - hunkIndex: target.hunkIndex, - side: target.side, - line: target.line, - }; + return result; }, - [allFiles, selectHunk], + [allFiles, applyCommandStateTransition], ); /** Apply several live comments together after validating every target first. */ @@ -430,136 +363,48 @@ export function useReviewController({ requestId: string, options?: { revealMode?: "none" | "first" }, ): AppliedCommentBatchResult => { - const createdAt = new Date().toISOString(); - const prepared = inputs.map((input, index) => { - const file = findDiffFileByPath(allFiles, input.filePath); - if (!file) { - throw new Error(`No diff file matches ${input.filePath}.`); - } - - const target = resolveCommentTarget(file, input); - return { - file, - target, - liveComment: buildLiveComment( - { - ...input, - side: target.side, - line: target.line, - }, - `mcp:${requestId}:${index}`, - createdAt, - target.hunkIndex, - ), - }; - }); - - if (prepared.length > 0) { - setLiveCommentsByFileId((current) => { - const next = { ...current }; - for (const entry of prepared) { - next[entry.file.id] = [...(next[entry.file.id] ?? []), entry.liveComment]; - } - - return next; - }); - } + const now = new Date().toISOString(); + const result = applyCommandStateTransition((state) => + addReviewLiveCommentBatch({ + files: allFiles, + state, + inputs, + requestId, + now, + options, + }), + ); - if (options?.revealMode === "first" && prepared.length > 0) { - const first = prepared[0]!; - selectHunk(first.file.id, first.target.hunkIndex, { scrollToNote: true }); + if (options?.revealMode === "first" && result.applied.length > 0) { + setScrollToNote(true); + setSelectedHunkRevealRequestId((current) => current + 1); } - return { - applied: prepared.map(({ file, target, liveComment }) => ({ - commentId: liveComment.id, - fileId: file.id, - filePath: file.path, - hunkIndex: target.hunkIndex, - side: target.side, - line: target.line, - })), - }; + return result; }, - [allFiles, selectHunk], + [allFiles, applyCommandStateTransition], ); /** Remove one live comment by id and report how many remain. */ const removeLiveComment = useCallback( (commentId: string): RemovedCommentResult => { - let removed = false; - let remainingCommentCount = 0; - const next: Record = {}; - - for (const [fileId, comments] of Object.entries(liveCommentsByFileId)) { - const filtered = comments.filter((comment) => comment.id !== commentId); - if (filtered.length !== comments.length) { - removed = true; - } - - if (filtered.length > 0) { - next[fileId] = filtered; - remainingCommentCount += filtered.length; - } - } - - if (!removed) { - throw new Error(`No live comment matches id ${commentId}.`); - } - - setLiveCommentsByFileId(next); - return { - commentId, - removed: true, - remainingCommentCount, - }; + return applyCommandStateTransition((state) => removeReviewLiveComment(state, commentId)); }, - [liveCommentsByFileId], + [applyCommandStateTransition], ); /** Clear all live comments, or only the comments attached to one specific file. */ const clearLiveComments = useCallback( (filePath?: string): ClearedCommentsResult => { - let removedCount = 0; - let remainingCommentCount = 0; - - if (filePath) { - const file = findDiffFileByPath(allFiles, filePath); - if (!file) { - throw new Error(`No diff file matches ${filePath}.`); - } - - const next: Record = {}; - for (const [fileId, comments] of Object.entries(liveCommentsByFileId)) { - if (fileId === file.id) { - removedCount = comments.length; - continue; - } - - next[fileId] = comments; - remainingCommentCount += comments.length; - } - - if (removedCount > 0) { - setLiveCommentsByFileId(next); - } - } else { - removedCount = Object.values(liveCommentsByFileId).reduce( - (sum, comments) => sum + comments.length, - 0, - ); - if (removedCount > 0) { - setLiveCommentsByFileId({}); - } - } - - return { - removedCount, - remainingCommentCount, - filePath, - }; + return applyCommandStateTransition((state) => + clearReviewLiveComments({ + files: allFiles, + state, + filePath, + }), + ); }, - [allFiles, liveCommentsByFileId], + [allFiles, applyCommandStateTransition], ); /** Start a human-authored draft note at the selected or requested hunk. */ @@ -609,7 +454,7 @@ export function useReviewController({ }, []); /** Persist the active draft into the in-memory user note collection. */ - const saveDraftNote = useCallback((): UserReviewNote | null => { + const saveDraftNote = useCallback((): SavedUserReviewNote | null => { if (!draftNote) { return null; } @@ -620,9 +465,10 @@ export function useReviewController({ return null; } - const savedNote: UserReviewNote = { + const savedNote: SavedUserReviewNote = { id: `user:${Date.now()}`, source: "user", + fileId: draftNote.fileId, filePath: draftNote.filePath, hunkIndex: draftNote.hunkIndex, side: draftNote.side, @@ -635,114 +481,43 @@ export function useReviewController({ editable: true, }; - setUserNotesByFileId((notesByFile) => ({ - ...notesByFile, - [draftNote.fileId]: [...(notesByFile[draftNote.fileId] ?? []), savedNote], - })); + updateCommandState((current) => addSavedUserReviewNote(current, savedNote)); setDraftNote(null); return savedNote; - }, [draftNote]); + }, [draftNote, updateCommandState]); /** Remove one in-memory user note by id. */ - const removeUserNote = useCallback( - (noteId: string) => { - let removed = false; - const next: Record = {}; - - for (const [fileId, notes] of Object.entries(userNotesByFileId)) { - const filtered = notes.filter((note) => note.id !== noteId); - if (filtered.length !== notes.length) { - removed = true; - } - if (filtered.length > 0) { - next[fileId] = filtered; - } - } - - if (!removed) { - throw new Error(`No user note matches id ${noteId}.`); - } + const removeUserNote = useCallback((noteId: string) => { + commandStateRef.current = removeSavedUserReviewNote(commandStateRef.current, noteId); + setCommandState(commandStateRef.current); + }, []); - setUserNotesByFileId(next); + const setAgentNotesVisible = useCallback( + (visible: boolean) => { + updateCommandState((current) => setReviewAgentNotesVisible(current, visible)); }, - [userNotesByFileId], + [updateCommandState], ); - /** Count all currently tracked live comments, including ones hidden by the active filter. */ - const liveCommentCount = useMemo( - () => Object.values(liveCommentsByFileId).reduce((sum, comments) => sum + comments.length, 0), - [liveCommentsByFileId], - ); + const toggleAgentNotes = useCallback(() => { + updateCommandState((current) => setReviewAgentNotesVisible(current, !current.showAgentNotes)); + }, [updateCommandState]); - /** Format current inline notes for daemon snapshots without exposing UI-only objects. */ - const reviewNoteSummaries = useMemo(() => { - const noteSummaries: SessionReviewNoteSummary[] = []; - - files.forEach((file) => { - (file.agent?.annotations ?? []).forEach((annotation, index) => { - const source = reviewNoteSource(annotation); - noteSummaries.push({ - noteId: annotation.id ?? `${source}:${file.id}:${index}`, - source, - filePath: file.path, - oldRange: annotation.oldRange, - newRange: annotation.newRange, - body: [annotation.summary, annotation.rationale].filter(Boolean).join("\n\n"), - title: annotation.title, - author: annotation.author, - createdAt: annotation.createdAt ?? "1970-01-01T00:00:00.000Z", - updatedAt: annotation.updatedAt, - editable: false, - }); - }); - - (liveCommentsByFileId[file.id] ?? []).forEach((comment) => { - noteSummaries.push({ - noteId: comment.id, - source: "agent", - filePath: file.path, - hunkIndex: comment.hunkIndex, - oldRange: comment.oldRange, - newRange: comment.newRange, - body: [comment.summary, comment.rationale].filter(Boolean).join("\n\n"), - author: comment.author, - createdAt: comment.createdAt, - editable: false, - }); - }); - - (userNotesByFileId[file.id] ?? []).forEach((note) => { - noteSummaries.push({ - noteId: note.id, - source: "user", - filePath: file.path, - hunkIndex: note.hunkIndex, - oldRange: note.oldRange, - newRange: note.newRange, - body: note.summary, - author: note.author, - createdAt: note.createdAt, - editable: true, - }); - }); - }); - - return noteSummaries; - }, [files, liveCommentsByFileId, userNotesByFileId]); - - /** Count all currently tracked review notes, including AI, agent, and user notes. */ - const reviewNoteCount = reviewNoteSummaries.length; - - /** Format current live comments for daemon snapshots without exposing merged UI-only objects. */ - const liveCommentSummaries = useMemo( + const sessionSnapshot = useMemo( () => - allFiles.flatMap((file) => - (liveCommentsByFileId[file.id] ?? []).map((comment) => - summarizeLiveComment(file.path, comment), - ), - ), - [allFiles, liveCommentsByFileId], + buildReviewSessionSnapshot({ + files, + state: commandState, + now: new Date().toISOString(), + }), + [files, commandState], ); + const { + liveCommentCount, + liveComments: liveCommentSummaries, + reviewNoteCount = 0, + reviewNotes: reviewNoteSummaries = [], + } = sessionSnapshot.state; return { allFiles, @@ -753,6 +528,8 @@ export function useReviewController({ liveCommentsByFileId, reviewNoteCount, reviewNoteSummaries, + sessionSnapshot, + showAgentNotes, userNotesByFileId, scrollToNote, selectedFile, @@ -779,7 +556,9 @@ export function useReviewController({ selectFile, selectHunk, startUserNote, + setAgentNotesVisible, setFilter, + toggleAgentNotes, updateDraftNote, }; } diff --git a/src/ui/lib/reviewState.ts b/src/ui/lib/reviewState.ts index 794a74da..aa771b47 100644 --- a/src/ui/lib/reviewState.ts +++ b/src/ui/lib/reviewState.ts @@ -7,14 +7,9 @@ * tested without React state in the loop. */ import { findDiffFileByPath, findHunkIndexForLine, hunkLineRange } from "../../core/liveComments"; -import type { AgentAnnotation, DiffFile } from "../../core/types"; +import type { DiffFile } from "../../core/types"; import type { NavigateToHunkToolInput, SelectedHunkSummary } from "../../hunk-session/types"; -import { - buildSidebarEntries, - filterReviewFiles, - mergeFileAnnotationsByFileId, - type SidebarEntry, -} from "./files"; +import { buildSidebarEntries, filterReviewFiles, type SidebarEntry } from "./files"; import { buildAnnotatedHunkCursors, buildHunkCursors, @@ -24,7 +19,6 @@ import { export interface BuildReviewStateOptions { files: DiffFile[]; - liveCommentsByFileId: Record; filterQuery: string; selectedFileId: string; selectedHunkIndex: number; @@ -49,17 +43,15 @@ export interface ReviewNavigationTarget { /** Build the derived review stream state from files, filter text, and selection. */ export function buildReviewState({ files, - liveCommentsByFileId, filterQuery, selectedFileId, selectedHunkIndex, }: BuildReviewStateOptions): ReviewState { - const allFiles = mergeFileAnnotationsByFileId(files, liveCommentsByFileId); - const visibleFiles = filterReviewFiles(allFiles, filterQuery); - const selectedFile = resolveSelectedFile(allFiles, visibleFiles, selectedFileId); + const visibleFiles = filterReviewFiles(files, filterQuery); + const selectedFile = resolveSelectedFile(files, visibleFiles, selectedFileId); return { - allFiles, + allFiles: files, visibleFiles, sidebarEntries: buildSidebarEntries(visibleFiles), selectedFile, From e115c4f87bbb5c3487469fd6099b15a78dbcd166 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sat, 23 May 2026 01:22:25 -0400 Subject: [PATCH 12/16] fix(embedded): preserve remounted review state --- src/embedded/session.ts | 89 ++++++++++++++++- src/hunk-session/reviewCommandState.test.ts | 105 ++++++++++++++++++++ src/hunk-session/reviewCommandState.ts | 46 ++++++++- src/ui/hooks/useReviewController.test.tsx | 2 + src/ui/hooks/useReviewController.ts | 14 ++- 5 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 src/hunk-session/reviewCommandState.test.ts diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 687a1f13..0f0408d6 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -93,6 +93,34 @@ function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { pathspecs: normalized.pathspecs, options: normalized.options, }; + case "vcs": + return { + kind: "vcs", + range: normalized.range, + staged: normalized.staged, + pathspecs: normalized.pathspecs, + options: normalized.options, + }; + case "show": + return { + kind: "show", + ref: normalized.ref, + pathspecs: normalized.pathspecs, + options: normalized.options, + }; + case "stash-show": + return { + kind: "stash-show", + ref: normalized.ref, + options: normalized.options, + }; + case "diff": + return { + kind: "diff", + left: normalized.left, + right: normalized.right, + options: normalized.options, + }; case "patch": return { kind: "patch", @@ -100,8 +128,63 @@ function embeddedSourceToCliInput(source: EmbeddedHunkSource): CliInput { file: normalized.file ?? normalized.label, options: normalized.options, }; - default: - return normalized as CliInput; + case "difftool": + return { + kind: "difftool", + left: normalized.left, + right: normalized.right, + path: normalized.path, + options: normalized.options, + }; + } +} + +/** Adapt daemon reload input back into a normalized embedded source without unsafe casts. */ +function cliInputToEmbeddedSource(input: CliInput): NormalizedEmbeddedHunkSource { + switch (input.kind) { + case "vcs": + return normalizeEmbeddedHunkSource({ + kind: "vcs", + range: input.range, + staged: input.staged, + pathspecs: input.pathspecs, + options: input.options, + }); + case "show": + return normalizeEmbeddedHunkSource({ + kind: "show", + ref: input.ref, + pathspecs: input.pathspecs, + options: input.options, + }); + case "stash-show": + return normalizeEmbeddedHunkSource({ + kind: "stash-show", + ref: input.ref, + options: input.options, + }); + case "diff": + return normalizeEmbeddedHunkSource({ + kind: "diff", + left: input.left, + right: input.right, + options: input.options, + }); + case "patch": + return normalizeEmbeddedHunkSource({ + kind: "patch", + file: input.file, + text: input.text, + options: input.options, + }); + case "difftool": + return normalizeEmbeddedHunkSource({ + kind: "difftool", + left: input.left, + right: input.right, + path: input.path, + options: input.options, + }); } } @@ -205,7 +288,7 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { navigateToLocation: this.navigateHeadless.bind(this), openAgentNotes: () => this.setHeadlessAgentNotesVisible(true), reloadSession: async (nextInput) => { - const result = await this.load(nextInput as EmbeddedHunkSource, { + const result = await this.load(cliInputToEmbeddedSource(nextInput), { updateSource: true, }); return { diff --git a/src/hunk-session/reviewCommandState.test.ts b/src/hunk-session/reviewCommandState.test.ts new file mode 100644 index 00000000..60412be6 --- /dev/null +++ b/src/hunk-session/reviewCommandState.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, test } from "bun:test"; +import { parseDiffFromFile } from "@pierre/diffs"; +import type { DiffFile } from "../core/types"; +import { buildReviewSessionSnapshot, createReviewCommandState } from "./reviewCommandState"; + +/** Build a minimal diff file with real parsed hunk metadata. */ +function createDiffFile(id: string, path: string, before: string, after: string): DiffFile { + const metadata = parseDiffFromFile( + { name: path, contents: before, cacheKey: `${id}:before` }, + { name: path, contents: after, cacheKey: `${id}:after` }, + { context: 3 }, + true, + ); + + return { + id, + path, + patch: "", + language: "typescript", + stats: { additions: 1, deletions: 1 }, + metadata, + agent: null, + }; +} + +describe("review command state", () => { + test("rehydrates editable user notes from session snapshots", () => { + const file = createDiffFile( + "alpha", + "alpha.ts", + "export const value = 1;\n", + "export const value = 2;\n", + ); + + const state = createReviewCommandState({ + files: [file], + initialSessionState: { + selectedFileId: "alpha", + selectedFilePath: "alpha.ts", + selectedHunkIndex: 0, + showAgentNotes: true, + liveCommentCount: 0, + liveComments: [], + reviewNoteCount: 2, + reviewNotes: [ + { + noteId: "user:1", + source: "user", + filePath: "alpha.ts", + hunkIndex: 0, + newRange: [1, 1], + body: "Keep this user note after remount.", + author: "user", + createdAt: "2026-05-23T00:00:00.000Z", + editable: true, + }, + { + noteId: "agent:1", + source: "agent", + filePath: "alpha.ts", + hunkIndex: 0, + newRange: [1, 1], + body: "Live agent note is rehydrated from liveComments instead.", + author: "agent", + createdAt: "2026-05-23T00:00:00.000Z", + editable: false, + }, + ], + }, + }); + + expect(state.userNotesByFileId.alpha).toMatchObject([ + { + id: "user:1", + source: "user", + fileId: "alpha", + filePath: "alpha.ts", + hunkIndex: 0, + side: "new", + line: 1, + summary: "Keep this user note after remount.", + editable: true, + }, + ]); + + const snapshot = buildReviewSessionSnapshot({ + files: [file], + state, + now: "2026-05-23T00:00:01.000Z", + }); + + expect(snapshot.state.reviewNoteCount).toBe(1); + expect(snapshot.state.reviewNotes).toMatchObject([ + { + noteId: "user:1", + source: "user", + filePath: "alpha.ts", + hunkIndex: 0, + newRange: [1, 1], + body: "Keep this user note after remount.", + editable: true, + }, + ]); + }); +}); diff --git a/src/hunk-session/reviewCommandState.ts b/src/hunk-session/reviewCommandState.ts index ed4ca181..25405635 100644 --- a/src/hunk-session/reviewCommandState.ts +++ b/src/hunk-session/reviewCommandState.ts @@ -82,6 +82,50 @@ function liveCommentsByFileFromSummaries( return byFileId; } +/** Resolve a persisted review-note summary back to its editable line target. */ +function reviewNoteTarget(summary: SessionReviewNoteSummary, file: DiffFile) { + if (summary.newRange) return { side: "new" as const, line: summary.newRange[0] }; + if (summary.oldRange) return { side: "old" as const, line: summary.oldRange[0] }; + + const hunk = summary.hunkIndex === undefined ? undefined : file.metadata.hunks[summary.hunkIndex]; + const range = hunk ? hunkLineRange(hunk) : undefined; + return { side: "new" as const, line: range?.newRange[0] ?? 0 }; +} + +/** Rehydrate editable user notes from a broker snapshot for remounted sessions. */ +function userNotesByFileFromSummaries( + files: DiffFile[], + summaries: SessionReviewNoteSummary[] = [], +) { + const byFileId: Record = {}; + summaries.forEach((summary) => { + if (summary.source !== "user" || !summary.editable) return; + const file = findDiffFileByPath(files, summary.filePath); + if (!file) return; + + const target = reviewNoteTarget(summary, file); + const notes = (byFileId[file.id] ??= []); + notes.push({ + id: summary.noteId, + source: "user", + fileId: file.id, + filePath: file.path, + hunkIndex: summary.hunkIndex ?? 0, + side: target.side, + line: target.line, + oldRange: summary.oldRange, + newRange: summary.newRange, + summary: summary.body, + title: summary.title, + author: summary.author ?? "user", + createdAt: summary.createdAt, + updatedAt: summary.updatedAt, + editable: true, + }); + }); + return byFileId; +} + function summarizeLiveComment(filePath: string, comment: LiveComment): SessionLiveCommentSummary { return { commentId: comment.id, @@ -124,7 +168,7 @@ export function createReviewCommandState({ files, initialSessionState?.liveComments, ), - userNotesByFileId: {}, + userNotesByFileId: userNotesByFileFromSummaries(files, initialSessionState?.reviewNotes), }, }); } diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index 4061ad84..9291b3b6 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -595,10 +595,12 @@ describe("useReviewController", () => { ]); await act(async () => { + expectValue(controllerRef.current).setAgentNotesVisible(true); expectValue(controllerRef.current).removeUserNote(savedNoteId); }); await flush(setup); + expect(expectValue(controllerRef.current).showAgentNotes).toBe(true); expect(expectValue(controllerRef.current).userNotesByFileId.alpha).toBeUndefined(); expect(expectValue(controllerRef.current).reviewNoteSummaries).toEqual([]); } finally { diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index 7b38e9f8..9c6a9429 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -138,10 +138,6 @@ export function useReviewController({ const [draftNote, setDraftNote] = useState(null); const deferredFilter = useDeferredValue(filter); - useEffect(() => { - commandStateRef.current = commandState; - }, [commandState]); - /** Update command state and its imperative mirror together. */ const updateCommandState = useCallback( (updater: (state: ReviewCommandState) => ReviewCommandState) => { @@ -487,10 +483,12 @@ export function useReviewController({ }, [draftNote, updateCommandState]); /** Remove one in-memory user note by id. */ - const removeUserNote = useCallback((noteId: string) => { - commandStateRef.current = removeSavedUserReviewNote(commandStateRef.current, noteId); - setCommandState(commandStateRef.current); - }, []); + const removeUserNote = useCallback( + (noteId: string) => { + updateCommandState((current) => removeSavedUserReviewNote(current, noteId)); + }, + [updateCommandState], + ); const setAgentNotesVisible = useCallback( (visible: boolean) => { From b2f62876576de35608657f1b390fc454f0a718a1 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sat, 23 May 2026 16:18:44 -0400 Subject: [PATCH 13/16] fix(embedded): scope host cursor to active embedded sessions --- src/embedded/embedded.test.ts | 53 ++++++++++++++++++++++++++++++++++- src/embedded/mount.tsx | 37 +++++++++++++++++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 553cee33..34fa8b55 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { BoxRenderable } from "@opentui/core"; +import { BoxRenderable, InputRenderable } from "@opentui/core"; import { createTestRenderer } from "@opentui/core/testing"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -298,4 +298,55 @@ describe("embedded Hunk sessions", () => { setup.renderer.destroy(); } }); + + test("hides host cursor only while the embedded scope is active", async () => { + const setup = await createTestRenderer({ width: 80, height: 24 }); + + try { + const hostInput = new InputRenderable(setup.renderer, { + id: "host-input", + left: 4, + position: "absolute", + top: 2, + value: "host", + width: 20, + }); + const container = new BoxRenderable(setup.renderer, { + height: 10, + id: "embedded-container", + width: 40, + }); + setup.renderer.root.add(hostInput); + setup.renderer.root.add(container); + hostInput.focus(); + await setup.renderOnce(); + expect(setup.renderer.getCursorState().visible).toBe(true); + + let active = false; + const scope = createEmbeddedRendererScope( + setup.renderer, + container, + setup.renderer.keyInput, + () => active, + ); + + try { + await setup.renderOnce(); + expect(setup.renderer.getCursorState().visible).toBe(true); + + active = true; + await setup.renderOnce(); + expect(setup.renderer.getCursorState().visible).toBe(false); + + scope.renderer.setCursorPosition(11, 12, true); + setup.renderer.setCursorPosition(2, 3, true); + await setup.renderOnce(); + expect(setup.renderer.getCursorState()).toMatchObject({ x: 11, y: 12, visible: true }); + } finally { + scope.dispose(); + } + } finally { + setup.renderer.destroy(); + } + }); }); diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index cc686f3a..a98c963d 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -76,16 +76,34 @@ export function createEmbeddedRendererScope( renderer: CliRenderer, root: Renderable, keyInput: CliRenderer["keyInput"], + active: () => boolean = () => true, ): ScopedRendererScope { const scoped = Object.create(renderer) as CliRenderer; const resizeListeners = new Set(); + let embeddedCursor: { x: number; y: number; visible: boolean } | undefined; + let wasActive = active(); const readWidth = () => Math.max(1, root.width); const readHeight = () => Math.max(1, root.height); const emitResize = () => { for (const listener of resizeListeners) listener(readWidth(), readHeight()); }; + const hideHostCursor = () => renderer.setCursorPosition(0, 0, false); + const enforceCursorScope: Parameters[0] = () => { + const isActive = active(); + if (isActive) { + if (embeddedCursor?.visible) + renderer.setCursorPosition(embeddedCursor.x, embeddedCursor.y, true); + else hideHostCursor(); + } else if (wasActive) { + hideHostCursor(); + } + + embeddedCursor = undefined; + wasActive = isActive; + }; root.on("resize", emitResize); + renderer.addPostProcessFn(enforceCursorScope); Object.defineProperties(scoped, { height: { get: readHeight }, @@ -95,6 +113,17 @@ export function createEmbeddedRendererScope( }, }, keyInput: { value: keyInput }, + setCursorPosition: { + value(x: number, y: number, visible = true) { + if (!active()) { + embeddedCursor = undefined; + return; + } + + embeddedCursor = { x, y, visible }; + renderer.setCursorPosition(x, y, visible); + }, + }, off: { value(event: string | symbol, listener: RendererListener) { if (event === "resize") { @@ -127,6 +156,7 @@ export function createEmbeddedRendererScope( renderer: scoped, dispose() { root.off("resize", emitResize); + renderer.removePostProcessFn(enforceCursorScope); resizeListeners.clear(); }, }; @@ -167,7 +197,12 @@ export function mountEmbeddedHunkApp({ }: MountEmbeddedHunkAppInput): EmbeddedHunkMount { let currentActive = active; const scopedKeyInput = createScopedKeyInput(renderer.keyInput, () => currentActive); - const scopedRenderer = createEmbeddedRendererScope(renderer, container, scopedKeyInput.keyInput); + const scopedRenderer = createEmbeddedRendererScope( + renderer, + container, + scopedKeyInput.keyInput, + () => currentActive, + ); const root = createRoot(scopedRenderer.renderer); const render = (next: { active: boolean; onQuit: () => void }) => { From 297ec1a674b8f23d22af26143090bde896bf9277 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sat, 23 May 2026 17:48:19 -0400 Subject: [PATCH 14/16] fix(embedded): keep a single React root for mounted sessions Add regression coverage for mount updates without stacking sibling roots. --- src/embedded/embedded.test.ts | 76 +++++++++++++++++++++++++++++++++++ src/embedded/mount.tsx | 16 ++++---- 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 34fa8b55..38f0e3a3 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -29,6 +29,32 @@ function getTestLoadedPatch(session: EmbeddedHunkSession) { .join("\n"); } +/** Count non-overlapping occurrences of a text fragment in a rendered frame. */ +function countTestFrameOccurrences(frame: string, text: string) { + return frame.split(text).length - 1; +} + +/** Flush enough render cycles for embedded React updates to reach the test frame. */ +async function flushTestRenderer(setup: Awaited>) { + await setup.renderOnce(); + await Bun.sleep(0); + await setup.renderOnce(); +} + +/** Render until an expected frame fragment appears, or return the last captured frame. */ +async function captureSettledTestFrame( + setup: Awaited>, + expectedText: string, +) { + let frame = ""; + for (let attempt = 0; attempt < 10; attempt += 1) { + await flushTestRenderer(setup); + frame = setup.captureCharFrame(); + if (frame.includes(expectedText)) break; + } + return frame; +} + /** Expect a snapshot to be ready and narrow it for the rest of the test. */ function expectTestReadySnapshot(snapshot: EmbeddedHunkSnapshot) { expect(snapshot.status).toBe("ready"); @@ -222,6 +248,56 @@ describe("embedded Hunk sessions", () => { } }); + test("updates an embedded mount without stacking app roots", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-mount-update-")); + const labels = ["alpha embedded source", "beta embedded source", "gamma embedded source"]; + + try { + const session = await createEmbeddedHunkSession({ + cwd: root, + source: { kind: "patch", text: testPatchText, label: labels[0] }, + }); + const setup = await createTestRenderer({ width: 140, height: 24 }); + const container = new BoxRenderable(setup.renderer, { + height: 20, + id: "embedded-hunk", + width: 120, + }); + setup.renderer.root.add(container); + + const mount = mountEmbeddedHunkApp({ + active: true, + container, + onQuit: () => undefined, + renderer: setup.renderer, + session, + }); + + try { + let frame = await captureSettledTestFrame(setup, labels[0]!); + expect(countTestFrameOccurrences(frame, labels[0]!)).toBe(1); + + for (const label of [labels[1]!, labels[2]!, labels[0]!, labels[1]!]) { + mount.update({ active: false, onQuit: () => undefined }); + await session.open({ kind: "patch", text: testPatchText, label }); + mount.update({ active: true, onQuit: () => undefined }); + + frame = await captureSettledTestFrame(setup, label); + expect(countTestFrameOccurrences(frame, label)).toBe(1); + for (const otherLabel of labels.filter((candidate) => candidate !== label)) { + expect(countTestFrameOccurrences(frame, otherLabel)).toBe(0); + } + } + } finally { + mount.unmount(); + setup.renderer.destroy(); + session.dispose(); + } + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + test("scopes embedded key input to the active mount", () => { const sourceListeners = new Map void>>(); const source = { diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index a98c963d..8007f3a5 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -196,6 +196,8 @@ export function mountEmbeddedHunkApp({ session, }: MountEmbeddedHunkAppInput): EmbeddedHunkMount { let currentActive = active; + let currentOnQuit = onQuit; + const handleQuit = () => currentOnQuit(); const scopedKeyInput = createScopedKeyInput(renderer.keyInput, () => currentActive); const scopedRenderer = createEmbeddedRendererScope( renderer, @@ -205,15 +207,15 @@ export function mountEmbeddedHunkApp({ ); const root = createRoot(scopedRenderer.renderer); - const render = (next: { active: boolean; onQuit: () => void }) => { - currentActive = next.active; - root.render(); - }; - - render({ active, onQuit }); + // Keep one React root mounted; repeated root.render calls leave sibling OpenTUI trees behind. + root.render(); return { - update: render, + update(next) { + currentActive = next.active; + currentOnQuit = next.onQuit; + if (!renderer.isDestroyed) renderer.requestRender(); + }, unmount() { root.unmount(); scopedRenderer.dispose(); From 4478175dd62bd7deb7b5a109fccb95d6aa49c11e Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sat, 23 May 2026 18:16:41 -0400 Subject: [PATCH 15/16] fix(embedded): sync mounted reloads into session state --- src/embedded/embedded.test.ts | 69 +++++++++++++++++++++++++++++++++++ src/embedded/index.ts | 24 +++++++++++- src/embedded/mount.tsx | 3 ++ src/embedded/session.ts | 9 +++++ src/ui/AppHost.tsx | 13 ++++++- 5 files changed, 114 insertions(+), 4 deletions(-) diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 38f0e3a3..2a337c7f 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { BoxRenderable, InputRenderable } from "@opentui/core"; import { createTestRenderer } from "@opentui/core/testing"; +import { spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -41,6 +42,17 @@ async function flushTestRenderer(setup: Awaited>, @@ -298,6 +310,63 @@ describe("embedded Hunk sessions", () => { } }); + test("mounted session reloads update the session-owned review", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-mounted-reload-")); + const initialPatchPath = join(root, "initial.patch"); + const nextPatchText = testPatchText.replace("value = 2", "value = 3"); + + try { + runTestGit(root, "init"); + writeFileSync(initialPatchPath, testPatchText); + const session = await createEmbeddedHunkSession({ + cwd: root, + source: { kind: "patch", file: initialPatchPath }, + }); + const setup = await createTestRenderer({ width: 140, height: 24 }); + const container = new BoxRenderable(setup.renderer, { + height: 20, + id: "embedded-hunk", + width: 120, + }); + setup.renderer.root.add(container); + + const mount = mountEmbeddedHunkApp({ + active: true, + container, + onQuit: () => undefined, + renderer: setup.renderer, + session, + }); + + try { + await flushTestRenderer(setup); + const internals = embeddedHunkSessionInternals(session); + + await internals.dispatchCommand({ + type: "command", + requestId: "reload-1", + command: "reload_session", + input: { + sessionId: "session-1", + nextInput: { kind: "patch", text: nextPatchText, options: {} }, + }, + }); + + expect(getTestLoadedPatch(session)).toContain("+const value = 3;"); + expect(session.getSnapshot().source).toMatchObject({ + kind: "patch", + text: nextPatchText, + }); + } finally { + mount.unmount(); + setup.renderer.destroy(); + session.dispose(); + } + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + test("scopes embedded key input to the active mount", () => { const sourceListeners = new Map void>>(); const source = { diff --git a/src/embedded/index.ts b/src/embedded/index.ts index 525b430b..06a54c6e 100644 --- a/src/embedded/index.ts +++ b/src/embedded/index.ts @@ -1,5 +1,25 @@ -export { mountEmbeddedHunkApp } from "./mount"; -export { createEmbeddedHunkSession } from "./session"; +import { mountEmbeddedHunkApp as mountEmbeddedHunkAppImpl } from "./mount"; +import { createEmbeddedHunkSession as createEmbeddedHunkSessionImpl } from "./session"; +import type { + CreateEmbeddedHunkSessionInput, + EmbeddedHunkMount, + EmbeddedHunkSession, + MountEmbeddedHunkAppInput, +} from "./types"; + +// Keep the published index.d.ts self-contained; the npm package only copies index/types here. +/** Create one embedded Hunk review session. */ +export function createEmbeddedHunkSession( + input: CreateEmbeddedHunkSessionInput, +): Promise { + return createEmbeddedHunkSessionImpl(input); +} + +/** Mount an embedded Hunk app into a host-owned OpenTUI container. */ +export function mountEmbeddedHunkApp(input: MountEmbeddedHunkAppInput): EmbeddedHunkMount { + return mountEmbeddedHunkAppImpl(input); +} + export type { CreateEmbeddedHunkSessionInput, EmbeddedHunkMount, diff --git a/src/embedded/mount.tsx b/src/embedded/mount.tsx index 8007f3a5..b14de827 100644 --- a/src/embedded/mount.tsx +++ b/src/embedded/mount.tsx @@ -181,6 +181,9 @@ function EmbeddedHunkRoot({ bootstrap={snapshot.bootstrap} hostClient={internals.hostClient} initialSessionState={internals.getSessionSnapshot().state} + onSessionReloaded={({ bootstrap, snapshot }) => + internals.syncMountedReload(bootstrap, snapshot) + } onQuit={onQuit} startupNoticeResolver={async () => null} /> diff --git a/src/embedded/session.ts b/src/embedded/session.ts index 0f0408d6..fb602253 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -397,6 +397,14 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { }); } + /** Sync session-owned state after the mounted AppHost loads a replacement review. */ + syncMountedReload(bootstrap: AppBootstrap, snapshot: HunkSessionSnapshot) { + this.source = cliInputToEmbeddedSource(bootstrap.input); + this.renderSnapshot = { status: "ready", bootstrap }; + this.persistSessionSnapshot(snapshot); + for (const listener of this.listeners) listener(); + } + /** Publish the current session-owned review state to the daemon. */ private updateHeadlessSnapshot() { this.sessionSnapshot = this.buildSessionSnapshot(); @@ -496,6 +504,7 @@ export function embeddedHunkSessionInternals(session: EmbeddedHunkSession) { getRenderSnapshot: session.getRenderSnapshot, getSessionSnapshot: session.getSessionSnapshot, hostClient: session.hostClient, + syncMountedReload: session.syncMountedReload.bind(session), }; } throw new Error("mountEmbeddedHunkApp requires a session from createEmbeddedHunkSession."); diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index d824608f..c625bc88 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -12,7 +12,11 @@ import { createSessionReloadBounds, validateSessionReloadWithinBounds, } from "../hunk-session/sessionFileBounds"; -import type { HunkSessionBrokerClient, HunkSessionState } from "../hunk-session/types"; +import type { + HunkSessionBrokerClient, + HunkSessionSnapshot, + HunkSessionState, +} from "../hunk-session/types"; import { App } from "./App"; import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; @@ -21,12 +25,14 @@ export function AppHost({ bootstrap, hostClient, initialSessionState, + onSessionReloaded, onQuit = () => process.exit(0), startupNoticeResolver, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; initialSessionState?: HunkSessionState; + onSessionReloaded?: (event: { bootstrap: AppBootstrap; snapshot: HunkSessionSnapshot }) => void; onQuit?: () => void; startupNoticeResolver?: () => Promise; }) { @@ -70,6 +76,9 @@ export function AppHost({ }); const nextSnapshot = createInitialSessionSnapshot(nextBootstrap); + previousBootstrapRef.current = nextBootstrap; + onSessionReloaded?.({ bootstrap: nextBootstrap, snapshot: nextSnapshot }); + let sessionId = "local-session"; if (hostClient) { // Keep the daemon-facing session registration in sync with whatever the UI is about to @@ -100,7 +109,7 @@ export function AppHost({ selectedHunkIndex: nextSnapshot.state.selectedHunkIndex, }; }, - [hostClient, sessionFileBounds], + [hostClient, onSessionReloaded, sessionFileBounds], ); return ( From b3825a4f884a9ea1b84b70b3de6a1884bb32b996 Mon Sep 17 00:00:00 2001 From: Khoa Huynh Date: Sun, 24 May 2026 01:31:37 -0400 Subject: [PATCH 16/16] feat(embedded): add reload-safe embedded session API Validate reloads stay within the initial root and ignore stale loads. --- CHANGELOG.md | 1 + src/embedded/embedded.test.ts | 104 +++++++++++++++++++++++++++++++++- src/embedded/session.ts | 70 +++++++++++++++++++++-- 3 files changed, 169 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc200d5f..b8040caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable user-visible changes to Hunk are documented in this file. ### Added +- Added a supported `hunkdiff/embedded` API for host OpenTUI apps to create embedded Hunk sessions and mount the review UI inside another terminal app. - Added Catppuccin Latte and Mocha as built-in themes. - Added mouse-drag text selection in diff views that copies selected rows to the system clipboard via OSC 52. A `View > Copy decorations` toggle (or `copy_decorations` config) controls whether the clipboard includes diff rails, gutters, and file headers or only the changed code. - Added inline expansion for collapsed unchanged file content. Click an unchanged-context row (`▾ N unchanged lines` when expandable, otherwise the static `··· N unchanged lines ···` form) or press `e` while a hunk is selected to reveal surrounding and trailing file lines without leaving the review. The affordance is shown only for input modes that have reachable source content (`hunk diff`, `show`, `stash show`, file-pair `diff` and `difftool`, untracked files); raw `hunk patch` input still renders as before. Failed and in-flight loads surface a one-line status ("Loading…", "Could not load N unchanged lines") on the gap row. Expanded context rows use the same syntax highlighting as the surrounding diff. diff --git a/src/embedded/embedded.test.ts b/src/embedded/embedded.test.ts index 2a337c7f..fb87254b 100644 --- a/src/embedded/embedded.test.ts +++ b/src/embedded/embedded.test.ts @@ -5,9 +5,10 @@ import { spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { loadAppBootstrap } from "../core/loaders"; import { createEmbeddedHunkSession, mountEmbeddedHunkApp } from "./index"; import { createEmbeddedRendererScope, createScopedKeyInput } from "./mount"; -import { embeddedHunkSessionInternals } from "./session"; +import { createEmbeddedHunkSessionForTest, embeddedHunkSessionInternals } from "./session"; import type { EmbeddedHunkSession, EmbeddedHunkSnapshot } from "./types"; const testPatchText = [ @@ -171,6 +172,63 @@ describe("embedded Hunk sessions", () => { } }); + test("ignores stale embedded loads that finish after a newer open", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-embedded-stale-load-")); + const slowPatchText = testPatchText.replace("value = 2", "slow = true"); + const fastPatchText = testPatchText.replace("value = 2", "fast = true"); + let markSlowStarted: () => void = () => undefined; + let releaseSlow: () => void = () => undefined; + const slowStarted = new Promise((resolve) => { + markSlowStarted = resolve; + }); + const slowReleased = new Promise((resolve) => { + releaseSlow = resolve; + }); + let session: EmbeddedHunkSession | undefined; + + try { + const loadBootstrap: typeof loadAppBootstrap = async (input, options) => { + if (input.kind === "patch" && input.text === slowPatchText) { + markSlowStarted(); + await slowReleased; + } + + return loadAppBootstrap(input, options); + }; + session = await createEmbeddedHunkSessionForTest( + { + cwd: root, + source: { kind: "patch", text: testPatchText, label: "initial.patch" }, + }, + loadBootstrap, + ); + + const slowOpen = session.open({ + kind: "patch", + text: slowPatchText, + label: "slow.patch", + }); + await slowStarted; + + const fastSnapshot = expectTestReadySnapshot( + await session.open({ kind: "patch", text: fastPatchText, label: "fast.patch" }), + ); + + expect(fastSnapshot.title).toBe("Patch review: fast.patch"); + releaseSlow(); + const staleSnapshot = expectTestReadySnapshot(await slowOpen); + + expect(staleSnapshot.title).toBe("Patch review: fast.patch"); + expect(session.getSnapshot().source).toMatchObject({ kind: "patch", label: "fast.patch" }); + expect(getTestLoadedPatch(session)).toContain("+const fast = true;"); + expect(getTestLoadedPatch(session)).not.toContain("+const slow = true;"); + } finally { + releaseSlow(); + session?.dispose(); + rmSync(root, { force: true, recursive: true }); + } + }); + test("keeps the previous source and reports errors when open fails", async () => { const root = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-error-")); @@ -367,6 +425,50 @@ describe("embedded Hunk sessions", () => { } }); + test("headless session reloads reject file reads outside the initial root", async () => { + const repo = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-root-")); + const outside = mkdtempSync(join(tmpdir(), "hunk-embedded-reload-outside-")); + const patch = join(repo, "changes.patch"); + const left = join(outside, "before.ts"); + const right = join(outside, "after.ts"); + let session: EmbeddedHunkSession | undefined; + + try { + mkdirSync(join(repo, ".git")); + writeFileSync(patch, testPatchText); + writeFileSync(left, "export const secret = 1;\n"); + writeFileSync(right, "export const secret = 2;\n"); + + session = await createEmbeddedHunkSession({ + cwd: repo, + source: { kind: "patch", file: patch }, + }); + const internals = embeddedHunkSessionInternals(session); + + await expect( + internals.dispatchCommand({ + type: "command", + requestId: "reload-outside-root", + command: "reload_session", + input: { + sessionId: "session-1", + nextInput: { + kind: "diff", + left, + right, + options: {}, + }, + sourcePath: outside, + }, + }), + ).rejects.toThrow("outside the initial Hunk root"); + } finally { + session?.dispose(); + rmSync(repo, { force: true, recursive: true }); + rmSync(outside, { force: true, recursive: true }); + } + }); + test("scopes embedded key input to the active mount", () => { const sourceListeners = new Map void>>(); const source = { diff --git a/src/embedded/session.ts b/src/embedded/session.ts index fb602253..86d5c65d 100644 --- a/src/embedded/session.ts +++ b/src/embedded/session.ts @@ -1,6 +1,7 @@ import { isDeepStrictEqual } from "node:util"; import { resolveConfiguredCliInput } from "../core/config"; import { loadAppBootstrap } from "../core/loaders"; +import { resolveRuntimeCliInput } from "../core/terminal"; import type { AppBootstrap, CliInput, CommonOptions } from "../core/types"; import { createHunkSessionBridge } from "../hunk-session/bridge"; import { @@ -19,6 +20,11 @@ import { createSessionRegistration, updateSessionRegistration, } from "../hunk-session/sessionRegistration"; +import { + createSessionReloadBounds, + validateSessionReloadWithinBounds, + type SessionReloadBounds, +} from "../hunk-session/sessionFileBounds"; import type { AppliedCommentBatchResult, AppliedCommentResult, @@ -46,6 +52,7 @@ export type EmbeddedHunkRenderSnapshot = | { status: "error"; bootstrap: AppBootstrap; error: string }; type NormalizedEmbeddedHunkSource = EmbeddedHunkSource & { options: CommonOptions }; +type LoadEmbeddedAppBootstrap = typeof loadAppBootstrap; /** Drop undefined option entries so equivalent embedded sources compare the same. */ function normalizeEmbeddedOptions(options: EmbeddedHunkSource["options"] = {}): CommonOptions { @@ -216,9 +223,12 @@ function publicSnapshot( class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { private listeners = new Set<() => void>(); private disposed = false; + private nextLoadRequestId = 1; + private activeLoadRequestId = 0; private renderSnapshot: EmbeddedHunkRenderSnapshot; private reviewState: ReviewCommandState; private sessionSnapshot: HunkSessionSnapshot; + private sessionFileBounds: SessionReloadBounds; private mountedBridge: Parameters[0] = null; readonly brokerClient: HunkSessionBrokerClient; @@ -228,8 +238,10 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { readonly cwd: string, public source: EmbeddedHunkSource, bootstrap: AppBootstrap, + private readonly loadBootstrap: LoadEmbeddedAppBootstrap = loadAppBootstrap, ) { this.renderSnapshot = { status: "ready", bootstrap }; + this.sessionFileBounds = createSessionReloadBounds(bootstrap, { cwd }); this.reviewState = createReviewCommandState({ files: bootstrap.changeset.files, initialShowAgentNotes: bootstrap.initialShowAgentNotes ?? false, @@ -287,8 +299,15 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { clearLiveComments: this.clearHeadlessLiveComments.bind(this), navigateToLocation: this.navigateHeadless.bind(this), openAgentNotes: () => this.setHeadlessAgentNotesVisible(true), - reloadSession: async (nextInput) => { - const result = await this.load(cliInputToEmbeddedSource(nextInput), { + reloadSession: async (nextInput, options) => { + const runtimeInput = resolveRuntimeCliInput(nextInput); + const { cwd } = validateSessionReloadWithinBounds(this.sessionFileBounds, runtimeInput, { + sourcePath: options?.sourcePath, + }); + const configured = resolveConfiguredCliInput(runtimeInput, { cwd }); + const result = await this.loadResolvedCliInput(configured.input, { + cwd, + source: cliInputToEmbeddedSource(configured.input), updateSource: true, }); return { @@ -320,15 +339,42 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { source: EmbeddedHunkSource, { updateSource }: { updateSource: boolean }, ): Promise { + return this.loadResolvedCliInput(resolveEmbeddedCliInput(source, this.cwd), { + cwd: this.cwd, + source, + updateSource, + }); + } + + private isCurrentLoadRequest(requestId: number) { + return !this.disposed && this.activeLoadRequestId === requestId; + } + + private async loadResolvedCliInput( + input: CliInput, + { + cwd, + source, + updateSource, + }: { cwd: string; source: EmbeddedHunkSource; updateSource: boolean }, + ): Promise { + if (this.disposed) return this.getSnapshot(); + + const requestId = this.nextLoadRequestId; + this.nextLoadRequestId += 1; + this.activeLoadRequestId = requestId; + this.setRenderSnapshot({ status: "loading", bootstrap: this.renderSnapshot.bootstrap, }); try { - const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(source, this.cwd), { - cwd: this.cwd, - }); + const bootstrap = await this.loadBootstrap(input, { cwd }); + if (!this.isCurrentLoadRequest(requestId)) { + return this.getSnapshot(); + } + if (updateSource) { this.source = source; } @@ -344,6 +390,10 @@ class EmbeddedHunkSessionImpl implements EmbeddedHunkSession { this.setRenderSnapshot({ status: "ready", bootstrap }); return this.getSnapshot(); } catch (error) { + if (!this.isCurrentLoadRequest(requestId)) { + return this.getSnapshot(); + } + const message = error instanceof Error ? error.message : String(error); this.setRenderSnapshot({ status: "error", @@ -519,3 +569,13 @@ export async function createEmbeddedHunkSession({ const bootstrap = await loadAppBootstrap(resolveEmbeddedCliInput(normalizedSource, cwd), { cwd }); return new EmbeddedHunkSessionImpl(cwd, normalizedSource, bootstrap); } + +/** Create an embedded session with a test-controlled bootstrap loader. */ +export async function createEmbeddedHunkSessionForTest( + { cwd = process.cwd(), source }: CreateEmbeddedHunkSessionInput, + loadBootstrap: LoadEmbeddedAppBootstrap, +): Promise { + const normalizedSource = normalizeEmbeddedHunkSource(source); + const bootstrap = await loadBootstrap(resolveEmbeddedCliInput(normalizedSource, cwd), { cwd }); + return new EmbeddedHunkSessionImpl(cwd, normalizedSource, bootstrap, loadBootstrap); +}