From e897c2a34ff4e23b858b99f33bc243d4889930ab Mon Sep 17 00:00:00 2001 From: Marcelito Date: Sun, 27 Sep 2026 10:54:56 -0300 Subject: [PATCH 1/2] feat(sidebar): integrate RDD lifecycle with session resets --- extensions/gentle-ai.ts | 17 ++++--- extensions/gentle-shell.ts | 27 +++++++++++ tests/gentle-shell.test.ts | 91 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 6 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index dbae1730b..543a89e1f 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -1,5 +1,6 @@ import { appendSystemPromptOnce } from "../lib/append-system-prompt.ts"; import { consumeReviewMutation, pendingReviewMutation, recordReviewMutation } from "../lib/review-reminder-receipt.ts"; +import { createReviewSidebarPublisher } from "../lib/review-sidebar-state.ts"; import { isOddPhase, oddPhaseRegistry, ODD_PHASES } from "../lib/odd-phase.ts"; import { resolveSessionWorktree } from "../lib/session-worktree-registry.ts"; import { declareReviewRelayHandshake } from "../lib/review-relay-contract.ts"; @@ -8768,9 +8769,12 @@ function createGentleAiExtensionForTesting( return revoked; }; + const reviewSidebar = createReviewSidebarPublisher(pi); + pi.on("session_tree", (_event, ctx) => reviewSidebar.reset(ctx)); let reminderSessionActive = true; let reminderEpoch = 0; pi.on("session_shutdown", (event, context) => { + reviewSidebar.reset(); reminderSessionActive = false; reminderEpoch += 1; // Pi tears down this registry on reload as well as session replacement/quit. @@ -8905,7 +8909,7 @@ function createGentleAiExtensionForTesting( return named.length === 0 ? operation : `${operation} · ${named.join(" · ")}`; }; - pi.registerTool({ + pi.registerTool(reviewSidebar.tool({ name: "gentle_review_capture_group", renderShell: "self", label: "Gentle Review Capture Group", @@ -8943,9 +8947,9 @@ function createGentleAiExtensionForTesting( ); return { content: [{ type: "text", text: JSON.stringify(details) }], details }; }, - }); + })); - pi.registerTool({ + pi.registerTool(reviewSidebar.tool({ name: "gentle_review_capture", renderShell: "self", label: "Gentle Review Capture", @@ -8989,9 +8993,9 @@ function createGentleAiExtensionForTesting( details, }; }, - }); + })); - pi.registerTool({ + pi.registerTool(reviewSidebar.tool({ name: "gentle_review", renderShell: "self", label: "Gentle Review Controller", @@ -9137,9 +9141,10 @@ function createGentleAiExtensionForTesting( details, }; }, - }); + })); pi.on("session_start", async (event, ctx) => { + reviewSidebar.reset(ctx); elapsedTiming = new GentleAiElapsedTimingLedger(ctx.sessionManager, pi); reminderSessionActive = true; reminderEpoch += 1; diff --git a/extensions/gentle-shell.ts b/extensions/gentle-shell.ts index f6418b2eb..e337c609b 100644 --- a/extensions/gentle-shell.ts +++ b/extensions/gentle-shell.ts @@ -110,6 +110,7 @@ import { UsageView } from "../lib/shell-usage-view.ts"; import { sidebarHeader, sidebarPart, sidebarState, VISUAL_SETTINGS_CHANGED } from "../lib/shell-sidebar.ts"; import { installSidebar, invalidateSidebar, narrowStatusOwner, STATUS_OWNER } from "../lib/shell-sidebar-layout.ts"; import { SessionChanges, SESSION_CHANGE_EVENT } from "../lib/session-changes.ts"; +import { REVIEW_SIDEBAR_EVENT, isReviewSidebarSnapshot, type ReviewSidebarSnapshot } from "../lib/review-sidebar-state.ts"; import { installSessionChangeCapture } from "../lib/session-change-capture.ts"; import { SelectionEngine } from "../lib/selection-engine.ts"; import { withOverlayRepaint } from "../lib/overlay-repaint.ts"; @@ -1547,6 +1548,22 @@ export default function gentleShell(pi: ExtensionAPI, env: NodeJS.ProcessEnv = p let changes: SessionChanges | undefined; let registry: SessionWorktreeRegistry | undefined; let currentContext: ExtensionContext | undefined; + let review: ReviewSidebarSnapshot | undefined; + const redrawReview = () => { + renderHost?.invalidateSidebar?.(); + renderHost?.requestRender(); + }; + const unsubscribeReview = pi.events.on(REVIEW_SIDEBAR_EVENT, (value) => { + const event = value as { sessionId?: unknown; snapshot?: unknown } | undefined; + if (!currentContext || event?.sessionId !== currentContext.sessionManager.getSessionId()) return; + if (!isReviewSidebarSnapshot(event.snapshot)) return; + review = { state: event.snapshot.state, scope: event.snapshot.scope }; + redrawReview(); + }); + pi.on("session_tree", () => { + review = undefined; + redrawReview(); + }); let shown = ""; const applyChanges = (ctx: ExtensionContext, model: ChangesModel) => { const fingerprint = changesFingerprint(model); @@ -1581,6 +1598,10 @@ export default function gentleShell(pi: ExtensionAPI, env: NodeJS.ProcessEnv = p }, }); pi.on("session_start", async (_event, ctx) => { + if (review) { + review = undefined; + redrawReview(); + } stopProfilePoll(); registry?.close(); currentContext = ctx; @@ -1614,6 +1635,7 @@ export default function gentleShell(pi: ExtensionAPI, env: NodeJS.ProcessEnv = p const footerModel = (): ShellBarModel => ({ ...buildShellBarModel(pi, ctx, footerData, { dirty: tracker.model.files.length, usage: usage.get(ctx.model?.provider ?? ""), profile: deps.activeProfile() }), changes: { files: tracker.model.files.length, added: tracker.model.added, deleted: tracker.model.deleted, notice: tracker.model.notice }, + review, }); // At narrow fullscreen widths only one status row paints: a top header // suppresses the bottom bar in the layout, and otherwise the bottom bar @@ -1684,6 +1706,11 @@ export default function gentleShell(pi: ExtensionAPI, env: NodeJS.ProcessEnv = p applyChanges(ctx, tracker.model); }); pi.on("session_shutdown", (_event, ctx) => { + if (review) { + review = undefined; + redrawReview(); + } + unsubscribeReview(); stopProfilePoll(); oddPhaseRegistry.clear(ctx.sessionManager.getSessionId()); oddPhaseRegistry.clearRenderRequest(ctx.sessionManager.getSessionId()); diff --git a/tests/gentle-shell.test.ts b/tests/gentle-shell.test.ts index b71e27150..f1674c636 100644 --- a/tests/gentle-shell.test.ts +++ b/tests/gentle-shell.test.ts @@ -14,6 +14,9 @@ import { CHANGE_STATUS } from "../lib/shell-changes.ts"; import { sidebarState, type SidebarRail } from "../lib/shell-sidebar.ts"; import type { ShellBarTheme } from "../lib/shell-bar.ts"; import { stripAnsi } from "../lib/terminal-theme.ts"; +import { createGentleAiExtension } from "../extensions/gentle-ai.ts"; +import { decodeReviewStatusV3 } from "../lib/review-integration-v2.ts"; +import type { NativeReviewCli } from "../lib/native-review-cli.ts"; import { resolveVisualSettings, writeVisualSettings } from "../lib/visual-customization-policy.ts"; import { resolveAnimationPolicy } from "../lib/animation-policy.ts"; import { resolveVimPolicy, writeVimPolicy } from "../lib/vim-policy.ts"; @@ -285,6 +288,94 @@ test("gentleShell installs the footer on session_start when a UI exists", () => assert.match(lines[0], /main ⟡ gpt-5\.5 · medium/); }); +test("review publisher rejects stale completion after session_start and accepts the new session", async () => { + const { pi, handlers, tools } = fakePi(); + gentleShell(pi, { GENTLE_PI_SHELL_CHANGES_WATCH_MS: "off" }, { activeProfile: () => undefined }); + const { ctx, ui } = fakeContext(); + ctx.cwd = process.cwd(); + let sessionId = "shell-session"; + ctx.sessionManager.getSessionId = () => sessionId; + const raw = JSON.parse(readFileSync(new URL("./fixtures/devbinary/status-v5-capture-result-submission.captured.json", import.meta.url), "utf8")); + raw.action = "stop"; + raw.projection.paths = ["src/fresh.ts"]; + const status = decodeReviewStatusV3(raw); + let finishStart!: (result: typeof status) => void; + let finishTree!: (result: typeof status) => void; + let calls = 0; + const native = { targetStatus: async () => { + calls += 1; + if (calls === 1) return new Promise((resolve) => { finishStart = resolve; }); + if (calls === 3) return new Promise((resolve) => { finishTree = resolve; }); + return status; + } } as unknown as NativeReviewCli; + const published: unknown[] = []; + const bus = pi.events; + const observedPi = { ...pi, events: { + ...bus, + emit(name: string, value: unknown) { + if (name === "gentle-ai:review-sidebar") published.push(value); + bus.emit(name, value); + }, + } } as ExtensionAPI; + const producerHooks = new Map unknown>>(); + createGentleAiExtension({ nativeReviewCli: native, candidateViews: null, processEnv: {} })({ + ...observedPi, + on(name: string, handler: (event: unknown, context: ExtensionContext) => unknown) { + producerHooks.set(name, [...(producerHooks.get(name) ?? []), handler]); + }, + } as ExtensionAPI); + const startProducer = async () => { + for (const hook of producerHooks.get("session_start") ?? []) await hook({}, ctx); + }; + await fire(handlers, "session_start", ctx); + await startProducer(); + const tui = { terminal: { rows: 40, columns: 160 }, requestRender() {} }; + const factory = ui.footerFactory as (tui: unknown, theme: ShellBarTheme, footerData: unknown) => { dispose(): void }; + const component = factory(tui, plainTheme, { getGitBranch: () => "main", getExtensionStatuses: () => new Map(), getAvailableProviderCount: () => 1, onBranchChange: () => () => {} }); + const rail = sidebarState(tui as unknown as TUI).parts.get("footer") as SidebarRail; + const text = () => rail.render(60).join("\n"); + const run = () => tools.get("gentle_review")!.execute("reset-test", { operation: "status", lineageId: status.authority!.lineageId }, undefined, undefined, ctx); + try { + const pending = run(); + assert.match(text(), /Checking/); + sessionId = "next-session"; + await fire(handlers, "session_start", ctx); + await startProducer(); + assert.doesNotMatch(text(), /RDD/); + const beforeStaleStart = published.length; + finishStart(status); + await pending; + assert.equal(published.length, beforeStaleStart, "producer must not publish the old session completion"); + assert.doesNotMatch(text(), /RDD/, "old completion must not repaint the new session"); + await run(); + assert.match(text(), /fresh\.ts/); + const pendingTree = run(); + assert.match(text(), /Checking/); + await fire(handlers, "session_tree", ctx); + for (const hook of producerHooks.get("session_tree") ?? []) await hook({}, ctx); + assert.doesNotMatch(text(), /RDD/, "tree navigation clears the visible snapshot"); + const beforeStaleTree = published.length; + finishTree(status); + await pendingTree; + assert.equal(published.length, beforeStaleTree, "producer must not publish the old tree completion with the same session ID"); + assert.doesNotMatch(text(), /RDD/); + await run(); + assert.match(text(), /fresh\.ts/, "a fresh tree-bound result is accepted"); + assert.equal(calls, 4); + const previous = rail.digest?.(); + pi.events.emit("gentle-ai:review-sidebar", { sessionId: "foreign", snapshot: { state: "closed", scope: "foreign.ts" } }); + assert.equal(rail.digest?.(), previous, "foreign events cannot replace the current snapshot"); + await fire(handlers, "session_shutdown", ctx); + assert.doesNotMatch(text(), /RDD/); + pi.events.emit("gentle-ai:review-sidebar", { sessionId, snapshot: { state: "closed", scope: "late.ts" } }); + assert.doesNotMatch(text(), /RDD/, "shutdown unsubscribes the consumer"); + } finally { + for (const hook of producerHooks.get("session_shutdown") ?? []) await hook({ reason: "quit" }, ctx); + await fire(handlers, "session_shutdown", ctx); + component.dispose(); + } +}); + test("the fullscreen Status rail carries a live digest so a profile switch refreshes it", async () => { const { pi, handlers } = fakePi(); let profile: string | undefined = "team"; From c87efa9209542c7976e980f40f864de55fc5502c Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Sun, 27 Sep 2026 17:16:38 +0200 Subject: [PATCH 2/2] test(sidebar): split RDD lifecycle integration into focused tests Share one harness across stale session_start, stale session_tree, and foreign/shutdown cases; use REVIEW_SIDEBAR_EVENT and explicit holds instead of a call count. Document why the shutdown unsubscribe is safe. --- extensions/gentle-shell.ts | 2 + tests/gentle-shell.test.ts | 143 ++++++++++++++++++++++++------------- 2 files changed, 96 insertions(+), 49 deletions(-) diff --git a/extensions/gentle-shell.ts b/extensions/gentle-shell.ts index e337c609b..99c694871 100644 --- a/extensions/gentle-shell.ts +++ b/extensions/gentle-shell.ts @@ -1710,6 +1710,8 @@ export default function gentleShell(pi: ExtensionAPI, env: NodeJS.ProcessEnv = p review = undefined; redrawReview(); } + // Pi rebuilds the extension runtime after every shutdown (reload, replacement, + // fork, quit), so the factory-level subscription never needs to be restored. unsubscribeReview(); stopProfilePoll(); oddPhaseRegistry.clear(ctx.sessionManager.getSessionId()); diff --git a/tests/gentle-shell.test.ts b/tests/gentle-shell.test.ts index f1674c636..ebc729fd8 100644 --- a/tests/gentle-shell.test.ts +++ b/tests/gentle-shell.test.ts @@ -16,6 +16,7 @@ import type { ShellBarTheme } from "../lib/shell-bar.ts"; import { stripAnsi } from "../lib/terminal-theme.ts"; import { createGentleAiExtension } from "../extensions/gentle-ai.ts"; import { decodeReviewStatusV3 } from "../lib/review-integration-v2.ts"; +import { REVIEW_SIDEBAR_EVENT } from "../lib/review-sidebar-state.ts"; import type { NativeReviewCli } from "../lib/native-review-cli.ts"; import { resolveVisualSettings, writeVisualSettings } from "../lib/visual-customization-policy.ts"; import { resolveAnimationPolicy } from "../lib/animation-policy.ts"; @@ -288,7 +289,7 @@ test("gentleShell installs the footer on session_start when a UI exists", () => assert.match(lines[0], /main ⟡ gpt-5\.5 · medium/); }); -test("review publisher rejects stale completion after session_start and accepts the new session", async () => { +async function reviewSidebarHarness() { const { pi, handlers, tools } = fakePi(); gentleShell(pi, { GENTLE_PI_SHELL_CHANGES_WATCH_MS: "off" }, { activeProfile: () => undefined }); const { ctx, ui } = fakeContext(); @@ -299,21 +300,23 @@ test("review publisher rejects stale completion after session_start and accepts raw.action = "stop"; raw.projection.paths = ["src/fresh.ts"]; const status = decodeReviewStatusV3(raw); - let finishStart!: (result: typeof status) => void; - let finishTree!: (result: typeof status) => void; - let calls = 0; + // Each hold() parks the next native status call until the returned release runs. + const held: Array<(release: (result: typeof status) => void) => void> = []; const native = { targetStatus: async () => { - calls += 1; - if (calls === 1) return new Promise((resolve) => { finishStart = resolve; }); - if (calls === 3) return new Promise((resolve) => { finishTree = resolve; }); - return status; + const park = held.shift(); + return park ? new Promise((resolve) => park(resolve)) : status; } } as unknown as NativeReviewCli; + const hold = () => { + let release!: () => void; + held.push((resolve) => { release = () => resolve(status); }); + return () => release(); + }; const published: unknown[] = []; const bus = pi.events; const observedPi = { ...pi, events: { ...bus, emit(name: string, value: unknown) { - if (name === "gentle-ai:review-sidebar") published.push(value); + if (name === REVIEW_SIDEBAR_EVENT) published.push(value); bus.emit(name, value); }, } } as ExtensionAPI; @@ -324,55 +327,97 @@ test("review publisher rejects stale completion after session_start and accepts producerHooks.set(name, [...(producerHooks.get(name) ?? []), handler]); }, } as ExtensionAPI); - const startProducer = async () => { - for (const hook of producerHooks.get("session_start") ?? []) await hook({}, ctx); + const produce = async (name: string, event: unknown = {}) => { + for (const hook of producerHooks.get(name) ?? []) await hook(event, ctx); }; await fire(handlers, "session_start", ctx); - await startProducer(); + await produce("session_start"); const tui = { terminal: { rows: 40, columns: 160 }, requestRender() {} }; const factory = ui.footerFactory as (tui: unknown, theme: ShellBarTheme, footerData: unknown) => { dispose(): void }; const component = factory(tui, plainTheme, { getGitBranch: () => "main", getExtensionStatuses: () => new Map(), getAvailableProviderCount: () => 1, onBranchChange: () => () => {} }); const rail = sidebarState(tui as unknown as TUI).parts.get("footer") as SidebarRail; - const text = () => rail.render(60).join("\n"); - const run = () => tools.get("gentle_review")!.execute("reset-test", { operation: "status", lineageId: status.authority!.lineageId }, undefined, undefined, ctx); + return { + pi, + rail, + published, + hold, + text: () => rail.render(60).join("\n"), + sessionId: () => sessionId, + run: () => tools.get("gentle_review")!.execute("reset-test", { operation: "status", lineageId: status.authority!.lineageId }, undefined, undefined, ctx), + async startSession(next: string) { + sessionId = next; + await fire(handlers, "session_start", ctx); + await produce("session_start"); + }, + async navigateTree() { + await fire(handlers, "session_tree", ctx); + await produce("session_tree"); + }, + shutdownShell: () => fire(handlers, "session_shutdown", ctx), + async dispose() { + await produce("session_shutdown", { reason: "quit" }); + await fire(handlers, "session_shutdown", ctx); + component.dispose(); + }, + }; +} + +test("review sidebar ignores a stale completion after session_start and accepts the new session", async () => { + const harness = await reviewSidebarHarness(); try { - const pending = run(); - assert.match(text(), /Checking/); - sessionId = "next-session"; - await fire(handlers, "session_start", ctx); - await startProducer(); - assert.doesNotMatch(text(), /RDD/); - const beforeStaleStart = published.length; - finishStart(status); + const release = harness.hold(); + const pending = harness.run(); + assert.match(harness.text(), /Checking/); + await harness.startSession("next-session"); + assert.doesNotMatch(harness.text(), /RDD/); + const before = harness.published.length; + release(); await pending; - assert.equal(published.length, beforeStaleStart, "producer must not publish the old session completion"); - assert.doesNotMatch(text(), /RDD/, "old completion must not repaint the new session"); - await run(); - assert.match(text(), /fresh\.ts/); - const pendingTree = run(); - assert.match(text(), /Checking/); - await fire(handlers, "session_tree", ctx); - for (const hook of producerHooks.get("session_tree") ?? []) await hook({}, ctx); - assert.doesNotMatch(text(), /RDD/, "tree navigation clears the visible snapshot"); - const beforeStaleTree = published.length; - finishTree(status); - await pendingTree; - assert.equal(published.length, beforeStaleTree, "producer must not publish the old tree completion with the same session ID"); - assert.doesNotMatch(text(), /RDD/); - await run(); - assert.match(text(), /fresh\.ts/, "a fresh tree-bound result is accepted"); - assert.equal(calls, 4); - const previous = rail.digest?.(); - pi.events.emit("gentle-ai:review-sidebar", { sessionId: "foreign", snapshot: { state: "closed", scope: "foreign.ts" } }); - assert.equal(rail.digest?.(), previous, "foreign events cannot replace the current snapshot"); - await fire(handlers, "session_shutdown", ctx); - assert.doesNotMatch(text(), /RDD/); - pi.events.emit("gentle-ai:review-sidebar", { sessionId, snapshot: { state: "closed", scope: "late.ts" } }); - assert.doesNotMatch(text(), /RDD/, "shutdown unsubscribes the consumer"); + assert.equal(harness.published.length, before, "producer must not publish the old session completion"); + assert.doesNotMatch(harness.text(), /RDD/, "old completion must not repaint the new session"); + await harness.run(); + assert.match(harness.text(), /fresh\.ts/, "a fresh result in the new session is accepted"); } finally { - for (const hook of producerHooks.get("session_shutdown") ?? []) await hook({ reason: "quit" }, ctx); - await fire(handlers, "session_shutdown", ctx); - component.dispose(); + await harness.dispose(); + } +}); + +test("review sidebar ignores a stale completion after same-session tree navigation", async () => { + const harness = await reviewSidebarHarness(); + try { + await harness.run(); + assert.match(harness.text(), /fresh\.ts/); + const release = harness.hold(); + const pending = harness.run(); + assert.match(harness.text(), /Checking/); + await harness.navigateTree(); + assert.doesNotMatch(harness.text(), /RDD/, "tree navigation clears the visible snapshot"); + const before = harness.published.length; + release(); + await pending; + assert.equal(harness.published.length, before, "producer must not publish the old tree completion with the same session ID"); + assert.doesNotMatch(harness.text(), /RDD/); + await harness.run(); + assert.match(harness.text(), /fresh\.ts/, "a fresh tree-bound result is accepted"); + } finally { + await harness.dispose(); + } +}); + +test("review sidebar rejects foreign-session events and unsubscribes on shutdown", async () => { + const harness = await reviewSidebarHarness(); + try { + await harness.run(); + assert.match(harness.text(), /fresh\.ts/); + const previous = harness.rail.digest?.(); + harness.pi.events.emit(REVIEW_SIDEBAR_EVENT, { sessionId: "foreign", snapshot: { state: "closed", scope: "foreign.ts" } }); + assert.equal(harness.rail.digest?.(), previous, "foreign events cannot replace the current snapshot"); + await harness.shutdownShell(); + assert.doesNotMatch(harness.text(), /RDD/); + harness.pi.events.emit(REVIEW_SIDEBAR_EVENT, { sessionId: harness.sessionId(), snapshot: { state: "closed", scope: "late.ts" } }); + assert.doesNotMatch(harness.text(), /RDD/, "shutdown unsubscribes the consumer"); + } finally { + await harness.dispose(); } });