diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b2d8fb6c3..f70b1a8a0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added a cancellable `session_before_shutdown` extension lifecycle event so interactive quit flows can be safely aborted before Atomic tears down UI/runtime state ([#1378](https://github.com/bastani-inc/atomic/issues/1378)). + ### Fixed - Fixed custom tool renderer disposal to honor renderer-owned cleanup callbacks, preventing stale animation registry entries after terminal workflow tool rows are finalized ([#1518](https://github.com/bastani-inc/atomic/issues/1518)). diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 904d0acc7..d3631d409 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -344,7 +344,11 @@ user sends another prompt ◄───────────────── thinking level changes (settings, keybinding, pi.setThinkingLevel()) └─► thinking_level_select -exit (CTRL+C, CTRL+D, SIGHUP, SIGTERM) +exit (CTRL+C, CTRL+D) + ├─► session_before_shutdown (can cancel; reason: "quit") + └─► session_shutdown + +signal exit (SIGHUP, SIGTERM) └─► session_shutdown ``` @@ -494,9 +498,23 @@ pi.on("session_tree", async (event, ctx) => { }); ``` +#### session_before_shutdown + +Fired before an interactive quit shutdown. Return `{ cancel: true }` to abort the quit before Atomic stops the UI or disposes the runtime. Signal shutdowns (`SIGHUP`, `SIGTERM`) skip this cancellable prompt path so process teardown remains non-interactive. Reloads and session replacement flows are not cancellable through this hook; use `session_shutdown` for terminal cleanup in those paths. + +```typescript +pi.on("session_before_shutdown", async (event, ctx) => { + // event.reason - "quit" + if (ctx.hasUI) { + const ok = await ctx.ui.confirm("Quit?", "Stop background work and exit?"); + if (!ok) return { cancel: true }; + } +}); +``` + #### session_shutdown -Fired before a started session runtime is torn down. Use this to clean up resources opened from `session_start` or other session-scoped hooks. +Fired before a started session runtime is torn down, after any cancellable `session_before_shutdown` handlers have allowed the shutdown to proceed. Use this to clean up resources opened from `session_start` or other session-scoped hooks. ```typescript pi.on("session_shutdown", async (event, ctx) => { @@ -1006,7 +1024,7 @@ Request a graceful shutdown of Atomic. - **RPC mode:** Deferred until the next idle state (after completing the current command response, when waiting for the next command). - **Print mode:** No-op. The process exits automatically when all prompts are processed. -Emits `session_shutdown` event to all extensions before exiting. Available in all contexts (event handlers, tools, commands, shortcuts). +Interactive quit requests first emit cancellable `session_before_shutdown`; when not cancelled, Atomic emits `session_shutdown` before exiting. Available in all contexts (event handlers, tools, commands, shortcuts). ```typescript pi.on("tool_call", (event, ctx) => { diff --git a/packages/coding-agent/docs/keybindings.md b/packages/coding-agent/docs/keybindings.md index 7b4d5588d..2bc890f15 100644 --- a/packages/coding-agent/docs/keybindings.md +++ b/packages/coding-agent/docs/keybindings.md @@ -84,7 +84,7 @@ Modifier combinations: `ctrl+shift+x`, `alt+ctrl+x`, `ctrl+shift+alt+x`, `ctrl+1 |--------|---------|-------------| | `app.interrupt` | `escape` | Cancel / abort | | `app.clear` | `ctrl+c` | Clear editor | -| `app.exit` | `ctrl+d` | Exit (when editor empty) | +| `app.exit` | `ctrl+d` | Exit (when editor empty; active workflows prompt to confirm) | | `app.suspend` | `ctrl+z` (none on Windows) | Suspend to background | | `app.editor.external` | `ctrl+g` | Open in external editor (`$VISUAL` or `$EDITOR`) | | `app.clipboard.pasteImage` | `ctrl+v` (`alt+v` on Windows) | Paste image from clipboard | diff --git a/packages/coding-agent/docs/usage.md b/packages/coding-agent/docs/usage.md index 8c86430b2..1debbd22d 100644 --- a/packages/coding-agent/docs/usage.md +++ b/packages/coding-agent/docs/usage.md @@ -58,6 +58,8 @@ Type `/` in the editor to open command completion. Extensions can register custo | `/exit` | Exit Atomic | | `/quit` | Quit Atomic | +When workflows are active, `/exit`, `/quit`, and empty-editor Ctrl+D show an exit confirmation. The prompt defaults to cancel so active workflow runs continue unless you explicitly confirm; confirming quits Atomic and cleans up active workflow work during shutdown. + ## Message Queue You can submit messages while the agent is still working: diff --git a/packages/coding-agent/src/core/extensions/api-types.ts b/packages/coding-agent/src/core/extensions/api-types.ts index 3efa317e1..5ac76b6a1 100644 --- a/packages/coding-agent/src/core/extensions/api-types.ts +++ b/packages/coding-agent/src/core/extensions/api-types.ts @@ -39,6 +39,7 @@ import type { MessageEndEventResult, SessionBeforeCompactResult, SessionBeforeForkResult, + SessionBeforeShutdownResult, SessionBeforeSwitchResult, SessionBeforeTreeResult, ToolCallEventResult, @@ -52,6 +53,7 @@ import type { ResourcesDiscoverResult, SessionBeforeCompactEvent, SessionBeforeForkEvent, + SessionBeforeShutdownEvent, SessionBeforeSwitchEvent, SessionBeforeTreeEvent, SessionCompactEvent, @@ -86,6 +88,10 @@ export interface ExtensionAPI { handler: ExtensionHandler, ): void; on(event: "session_compact", handler: ExtensionHandler): void; + on( + event: "session_before_shutdown", + handler: ExtensionHandler, + ): void; on(event: "session_shutdown", handler: ExtensionHandler): void; on(event: "session_before_tree", handler: ExtensionHandler): void; on(event: "session_tree", handler: ExtensionHandler): void; diff --git a/packages/coding-agent/src/core/extensions/event-results.ts b/packages/coding-agent/src/core/extensions/event-results.ts index 972eca2fb..9d8fafdee 100644 --- a/packages/coding-agent/src/core/extensions/event-results.ts +++ b/packages/coding-agent/src/core/extensions/event-results.ts @@ -56,6 +56,10 @@ export interface SessionBeforeCompactResult { deletionRequest?: ContextDeletionRequest; } +export interface SessionBeforeShutdownResult { + cancel?: boolean; +} + export interface SessionBeforeTreeResult { cancel?: boolean; summary?: { diff --git a/packages/coding-agent/src/core/extensions/index.ts b/packages/coding-agent/src/core/extensions/index.ts index 4d8181485..f7ecf6513 100644 --- a/packages/coding-agent/src/core/extensions/index.ts +++ b/packages/coding-agent/src/core/extensions/index.ts @@ -19,7 +19,7 @@ export type { ShutdownHandler, SwitchSessionHandler, } from "./runner.ts"; -export { ExtensionRunner } from "./runner.ts"; +export { emitSessionBeforeShutdownEvent, ExtensionRunner } from "./runner.ts"; export type { AfterProviderResponseEvent, AgentEndEvent, @@ -127,6 +127,8 @@ export type { SessionBeforeCompactResult, SessionBeforeForkEvent, SessionBeforeForkResult, + SessionBeforeShutdownEvent, + SessionBeforeShutdownResult, SessionBeforeSwitchEvent, SessionBeforeSwitchResult, SessionBeforeTreeEvent, diff --git a/packages/coding-agent/src/core/extensions/runner-events.ts b/packages/coding-agent/src/core/extensions/runner-events.ts index a94b4993a..f6d71a863 100644 --- a/packages/coding-agent/src/core/extensions/runner-events.ts +++ b/packages/coding-agent/src/core/extensions/runner-events.ts @@ -21,6 +21,7 @@ import type { ResourcesDiscoverResult, SessionBeforeCompactResult, SessionBeforeForkResult, + SessionBeforeShutdownResult, SessionBeforeSwitchResult, SessionBeforeTreeResult, ToolCallEvent, @@ -60,13 +61,14 @@ export type RunnerEmitEvent = Exclude< type SessionBeforeEvent = Extract< RunnerEmitEvent, - { type: "session_before_switch" | "session_before_fork" | "session_before_compact" | "session_before_tree" } + { type: "session_before_switch" | "session_before_fork" | "session_before_compact" | "session_before_shutdown" | "session_before_tree" } >; type SessionBeforeEventResult = | SessionBeforeSwitchResult | SessionBeforeForkResult | SessionBeforeCompactResult + | SessionBeforeShutdownResult | SessionBeforeTreeResult; export type RunnerEmitResult = TEvent extends { type: "session_before_switch" } @@ -75,9 +77,11 @@ export type RunnerEmitResult = TEvent extends { ? SessionBeforeForkResult | undefined : TEvent extends { type: "session_before_compact" } ? SessionBeforeCompactResult | undefined - : TEvent extends { type: "session_before_tree" } - ? SessionBeforeTreeResult | undefined - : undefined; + : TEvent extends { type: "session_before_shutdown" } + ? SessionBeforeShutdownResult | undefined + : TEvent extends { type: "session_before_tree" } + ? SessionBeforeTreeResult | undefined + : undefined; type EmitExtensionError = (error: ExtensionError) => void; @@ -85,6 +89,7 @@ const isSessionBeforeEvent = (event: RunnerEmitEvent): event is SessionBeforeEve event.type === "session_before_switch" || event.type === "session_before_fork" || event.type === "session_before_compact" || + event.type === "session_before_shutdown" || event.type === "session_before_tree"; const emitCaughtError = ( diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index 678f1c10c..06115fe15 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -74,6 +74,7 @@ import type { RegisteredTool, ResolvedCommand, ResourcesDiscoverEvent, + SessionBeforeShutdownEvent, SessionShutdownEvent, ToolCallEvent, ToolCallEventResult, @@ -94,6 +95,21 @@ export type { } from "./runner-handlers.ts"; export { emitProjectTrustEvent } from "./runner-project-trust.ts"; +/** + * Helper function to emit session_before_shutdown event to extensions. + * Returns cancellation and emission status. + */ +export async function emitSessionBeforeShutdownEvent( + extensionRunner: ExtensionRunner, + event: SessionBeforeShutdownEvent, +): Promise<{ cancelled: boolean; emitted: boolean }> { + if (!extensionRunner.hasHandlers("session_before_shutdown")) { + return { cancelled: false, emitted: false }; + } + const result = await extensionRunner.emit(event); + return { cancelled: result?.cancel === true, emitted: true }; +} + /** * Helper function to emit session_shutdown event to extensions. * Returns true if the event was emitted, false if there were no handlers. diff --git a/packages/coding-agent/src/core/extensions/session-events.ts b/packages/coding-agent/src/core/extensions/session-events.ts index d1dc06648..b1811003f 100644 --- a/packages/coding-agent/src/core/extensions/session-events.ts +++ b/packages/coding-agent/src/core/extensions/session-events.ts @@ -66,6 +66,12 @@ export interface SessionCompactEvent { fromExtension: boolean; } +/** Fired before an interactive quit shutdown (can be cancelled). */ +export interface SessionBeforeShutdownEvent { + type: "session_before_shutdown"; + reason: "quit"; +} + /** Fired before an extension runtime is torn down due to quit, reload, or session replacement. */ export interface SessionShutdownEvent { type: "session_shutdown"; @@ -111,6 +117,7 @@ export type SessionEvent = | SessionBeforeForkEvent | SessionBeforeCompactEvent | SessionCompactEvent + | SessionBeforeShutdownEvent | SessionShutdownEvent | SessionBeforeTreeEvent | SessionTreeEvent; diff --git a/packages/coding-agent/src/core/index.ts b/packages/coding-agent/src/core/index.ts index 36cbf942c..e73080b5c 100644 --- a/packages/coding-agent/src/core/index.ts +++ b/packages/coding-agent/src/core/index.ts @@ -60,6 +60,8 @@ export { type RegisteredCommand, type SessionBeforeCompactEvent, type SessionBeforeForkEvent, + type SessionBeforeShutdownEvent, + type SessionBeforeShutdownResult, type SessionBeforeSwitchEvent, type SessionBeforeTreeEvent, type SessionCompactEvent, diff --git a/packages/coding-agent/src/index-extensions.ts b/packages/coding-agent/src/index-extensions.ts index 4a8fd7fe7..36173bfa3 100644 --- a/packages/coding-agent/src/index-extensions.ts +++ b/packages/coding-agent/src/index-extensions.ts @@ -71,6 +71,8 @@ export type { ResolvedCommand, SessionBeforeCompactEvent, SessionBeforeForkEvent, + SessionBeforeShutdownEvent, + SessionBeforeShutdownResult, SessionBeforeSwitchEvent, SessionBeforeTreeEvent, SessionCompactEvent, diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode-base.ts b/packages/coding-agent/src/modes/interactive/interactive-mode-base.ts index 8c75fce42..92ef28d4f 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode-base.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode-base.ts @@ -393,4 +393,5 @@ export class InteractiveModeBase { * repaint the final frame while the process is exiting. */ isShuttingDown = false; + shutdownConfirmationPending = false; } diff --git a/packages/coding-agent/src/modes/interactive/interactive-process-lifecycle.ts b/packages/coding-agent/src/modes/interactive/interactive-process-lifecycle.ts index 2afbcd4b0..f9d54f216 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-process-lifecycle.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-process-lifecycle.ts @@ -1,3 +1,4 @@ +import { emitSessionBeforeShutdownEvent } from "../../core/extensions/index.ts"; import { InteractiveModeBase } from "./interactive-mode-base.ts"; import { chalk, killTrackedDetachedChildren } from "./interactive-mode-deps.ts"; import { formatResumeCommand, isDeadTerminalError } from "./interactive-mode-helpers.ts"; @@ -19,6 +20,29 @@ InteractiveModeBase.prototype.handleCtrlD = function(this: InteractiveModeBase): InteractiveModeBase.prototype.shutdown = async function(this: InteractiveModeBase, options?: { fromSignal?: boolean }): Promise { if (this.isShuttingDown) return; + + if (!options?.fromSignal) { + // While a cancellable quit prompt is mounted, further in-process quit + // requests (including double Ctrl+C) are owned by that overlay. Real + // process signals still bypass this path through `fromSignal`. + if (this.shutdownConfirmationPending) return; + this.shutdownConfirmationPending = true; + let beforeShutdown: Awaited>; + try { + beforeShutdown = await emitSessionBeforeShutdownEvent(this.session.extensionRunner, { + type: "session_before_shutdown", + reason: "quit", + }); + } finally { + this.shutdownConfirmationPending = false; + } + if (beforeShutdown.cancelled) { + this.shutdownRequested = false; + return; + } + if (this.isShuttingDown) return; + } + this.isShuttingDown = true; // Keep signal handlers registered until terminal cleanup has completed. // `signal-exit` checks the listener list during the same SIGTERM/SIGHUP diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts index f69afbbaf..93cc643b5 100644 --- a/packages/coding-agent/test/extensions-runner.test.ts +++ b/packages/coding-agent/test/extensions-runner.test.ts @@ -7,4 +7,5 @@ import "./extensions-runner/shortcut-conflicts.suite.ts"; import "./extensions-runner/tool-command-collection.suite.ts"; import "./extensions-runner/context-error-renderer-flags.suite.ts"; import "./extensions-runner/lifecycle-tool-result.suite.ts"; +import "./extensions-runner/session-shutdown.suite.ts"; import "./extensions-runner/provider-command-handlers.suite.ts"; diff --git a/packages/coding-agent/test/extensions-runner/session-shutdown.suite.ts b/packages/coding-agent/test/extensions-runner/session-shutdown.suite.ts new file mode 100644 index 000000000..8c14e71d3 --- /dev/null +++ b/packages/coding-agent/test/extensions-runner/session-shutdown.suite.ts @@ -0,0 +1,57 @@ +/** Tests for ExtensionRunner session shutdown lifecycle helpers. */ + +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { AuthStorage } from "../../src/core/auth-storage.ts"; +import { loadExtensions } from "../../src/core/extensions/loader.ts"; +import { emitSessionBeforeShutdownEvent, ExtensionRunner } from "../../src/core/extensions/runner.ts"; +import { ModelRegistry } from "../../src/core/model-registry.ts"; +import { SessionManager } from "../../src/core/session-manager.ts"; + +describe("ExtensionRunner session_before_shutdown", () => { + let tempDir: string; + let extensionsDir: string; + let sessionManager: SessionManager; + let modelRegistry: ModelRegistry; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-runner-session-shutdown-test-")); + extensionsDir = path.join(tempDir, "extensions"); + fs.mkdirSync(extensionsDir); + sessionManager = SessionManager.inMemory(); + const authStorage = AuthStorage.create(path.join(tempDir, "auth.json")); + modelRegistry = ModelRegistry.create(authStorage); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it("returns cancellation from pre-shutdown handlers", async () => { + const extPath = path.join(extensionsDir, "before-shutdown.ts"); + fs.writeFileSync( + extPath, + `export default function(pi) { + pi.on("session_before_shutdown", (event) => ({ cancel: event.reason === "quit" })); +}`, + ); + + const result = await loadExtensions([extPath], tempDir); + const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry); + + expect( + await emitSessionBeforeShutdownEvent(runner, { type: "session_before_shutdown", reason: "quit" }), + ).toEqual({ cancelled: true, emitted: true }); + }); + + it("reports no emission when no pre-shutdown handlers are registered", async () => { + const result = await loadExtensions([], tempDir); + const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry); + + expect( + await emitSessionBeforeShutdownEvent(runner, { type: "session_before_shutdown", reason: "quit" }), + ).toEqual({ cancelled: false, emitted: false }); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts b/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts index 771ba68e4..c460fab9a 100644 --- a/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts +++ b/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts @@ -18,11 +18,19 @@ import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode type ShutdownThis = { isShuttingDown: boolean; + shutdownRequested: boolean; + shutdownConfirmationPending: boolean; unregisterSignalHandlers: () => void; runtimeHost: { dispose: () => Promise }; ui: { terminal: { drainInput: (ms: number) => Promise } }; themeController: { disableAutoSync: () => void }; stop: () => void; + session: { + extensionRunner: { + hasHandlers: (eventType: string) => boolean; + emit?: (event: { type: "session_before_shutdown"; reason: "quit" }) => Promise<{ cancel?: boolean }>; + }; + }; sessionManager: SessionManager; }; @@ -69,6 +77,8 @@ function restoreStdoutIsTTY(): void { function createContext(order: string[], sessionManager = createSessionManager()): ShutdownThis { return { isShuttingDown: false, + shutdownRequested: false, + shutdownConfirmationPending: false, unregisterSignalHandlers: vi.fn(), runtimeHost: { dispose: vi.fn(async () => { @@ -86,6 +96,11 @@ function createContext(order: string[], sessionManager = createSessionManager()) stop: vi.fn(() => { order.push("stop"); }), + session: { + extensionRunner: { + hasHandlers: vi.fn(() => false), + }, + }, sessionManager, }; } @@ -130,6 +145,7 @@ describe("InteractiveMode.shutdown ordering (#5080)", () => { await callShutdown(context); expect(order).toEqual(["drainInput", "stop", "dispose"]); + expect(context.shutdownConfirmationPending).toBe(false); }); test("interactive quit prints a resume hint for persisted sessions", async () => { @@ -169,6 +185,76 @@ describe("InteractiveMode.shutdown ordering (#5080)", () => { } }); + test("cancelled interactive shutdown clears pending shutdown request", async () => { + const order: string[] = []; + const context = createContext(order); + context.shutdownRequested = true; + context.session.extensionRunner = { + hasHandlers: vi.fn((eventType) => eventType === "session_before_shutdown"), + emit: vi.fn(async () => ({ cancel: true })), + }; + + await callShutdown(context); + + expect(context.shutdownRequested).toBe(false); + expect(context.isShuttingDown).toBe(false); + expect(context.shutdownConfirmationPending).toBe(false); + expect(order).toEqual([]); + expect(context.session.extensionRunner.emit).toHaveBeenCalledWith({ + type: "session_before_shutdown", + reason: "quit", + }); + }); + + test("pending interactive shutdown confirmation suppresses duplicate pre-shutdown prompts", async () => { + const order: string[] = []; + const context = createContext(order); + context.shutdownRequested = true; + let resolvePrompt!: (value: { cancel?: boolean }) => void; + const prompt = new Promise<{ cancel?: boolean }>((resolve) => { + resolvePrompt = resolve; + }); + context.session.extensionRunner = { + hasHandlers: vi.fn((eventType) => eventType === "session_before_shutdown"), + emit: vi.fn(() => prompt), + }; + + const first = callShutdown(context); + await Promise.resolve(); + expect(context.shutdownConfirmationPending).toBe(true); + + await callShutdown(context); + expect(context.session.extensionRunner.emit).toHaveBeenCalledTimes(1); + expect(context.shutdownRequested).toBe(true); + expect(context.isShuttingDown).toBe(false); + + resolvePrompt({ cancel: true }); + await first; + + expect(context.shutdownConfirmationPending).toBe(false); + expect(context.shutdownRequested).toBe(false); + expect(order).toEqual([]); + }); + + test("failed pre-shutdown confirmation clears the pending guard", async () => { + // ExtensionRunner normally catches handler failures; this synthetic reject + // validates the shutdown guard contract if the emit helper ever fails. + const order: string[] = []; + const context = createContext(order); + context.session.extensionRunner = { + hasHandlers: vi.fn((eventType) => eventType === "session_before_shutdown"), + emit: vi.fn(async () => { + throw new Error("prompt failed"); + }), + }; + + await expect(callShutdown(context)).rejects.toThrow("prompt failed"); + + expect(context.shutdownConfirmationPending).toBe(false); + expect(context.isShuttingDown).toBe(false); + expect(order).toEqual([]); + }); + test("re-entrant shutdown is a no-op", async () => { vi.spyOn(process, "exit").mockImplementation((() => { throw new ProcessExitError(); diff --git a/packages/workflows/CHANGELOG.md b/packages/workflows/CHANGELOG.md index 5d3481529..cf3298f59 100644 --- a/packages/workflows/CHANGELOG.md +++ b/packages/workflows/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- Fixed interactive quit and graph `q` handling for active workflows: Atomic now shows a host-themed, default-cancel confirmation before performing a resumable workflow quit/detach, and the graph overlay no longer closes speculatively before confirmation. `/workflow kill` remains the explicit non-resumable disposal path. ([#1378](https://github.com/bastani-inc/atomic/issues/1378)) - Added direct mouse-click activation for visible workflow graph nodes: clicking a node now focuses it and opens/attaches it through the same path as pressing Enter, with hit-testing based on the rendered node rectangles so overlay chrome/scroll geometry stays in sync while wheel scrolling and empty-space/chrome clicks remain safe no-ops. ([#1521](https://github.com/bastani-inc/atomic/issues/1521)) - Fixed workflow stage chats to leave terminal text selection enabled by default, so users can drag-select and copy workflow prompts, logs, command text, and model responses from attached workflow chats; `ctrl+t` now explicitly toggles temporary mouse-scroll capture for users who prefer mouse/trackpad wheel scrolling. ([#1519](https://github.com/bastani-inc/atomic/issues/1519)) - Routed idle `StageContext.sendUserMessage()` turns through the workflow stage guard so first-call injected turns record normal stage lifecycle and all idle injected turns observe abort/kill wiring, MCP scope, readiness handling, and the concurrency limiter. ([#1520](https://github.com/bastani-inc/atomic/issues/1520)) diff --git a/packages/workflows/README.md b/packages/workflows/README.md index 8b0f20e9b..0b692a5f9 100644 --- a/packages/workflows/README.md +++ b/packages/workflows/README.md @@ -44,6 +44,8 @@ Set `enabled` to `false` to disable all lifecycle notices, or narrow `notifyOn` When a stage human-in-the-loop prompt is answered from the workflow TUI/stage chat, workflows also emits a separate display-only `workflows:hil-answer-notice` custom message. It records the answer for user-visible audit, but it does not wake the main agent, enter LLM context, or authorize answering later workflow prompts. Answers sent by the main-chat `workflow` tool do not emit this notice because the tool result already tells the main agent what happened. +When you quit Atomic while top-level workflow runs are still in flight, the extension shows a destructive quit confirmation in the TUI. **Cancel** is focused by default. Confirming quit lets the normal shutdown path kill active workflow work and retain killed runs for later status/history inspection; cancelling leaves Atomic and the workflows running. Headless or degraded UI shutdowns fail open so automation cannot wedge on a prompt. + --- ## Authoring API diff --git a/packages/workflows/src/extension/extension-lifecycle.ts b/packages/workflows/src/extension/extension-lifecycle.ts index 0dcaff215..daf81954a 100644 --- a/packages/workflows/src/extension/extension-lifecycle.ts +++ b/packages/workflows/src/extension/extension-lifecycle.ts @@ -3,9 +3,12 @@ import { quitAllRuns } from "../runs/background/quit.js"; import { cancellationRegistry } from "../runs/background/cancellation-registry.js"; import { stageControlRegistry } from "../runs/foreground/stage-control-registry.js"; import { store } from "../shared/store.js"; +import { topLevelWorkflowRuns } from "../shared/run-visibility.js"; import { restoreOnSessionStart } from "../shared/persistence-restore.js"; import { installCompactionHook } from "../shared/persistence-compaction-policy.js"; import { clearForms } from "../tui/inline-form-store.js"; +import { deriveGraphTheme } from "../tui/graph-theme.js"; +import { openWorkflowQuitConfirm } from "../tui/session-overlays.js"; import { installStoreWidget } from "../tui/store-widget-installer.js"; import { registerIntercomParentSession } from "../intercom/intercom-bridge.js"; import { @@ -18,6 +21,13 @@ import type { ExtensionAPI } from "./public-types.js"; import type { WorkflowExtensionRuntimeState } from "./extension-runtime-state.js"; import { deAdvertiseAskUserQuestionWhenHeadless, formatStartupDiagnostics } from "./workflow-command-surfaces.js"; import { inFlightRunCount } from "./workflow-targets.js"; +import type { RunSnapshot } from "../shared/store-types.js"; + +function blocksAppShutdown(run: RunSnapshot): boolean { + if (run.endedAt !== undefined) return false; + if (run.exitReason === "quit" && run.status === "paused" && run.resumable === true) return false; + return true; +} export interface WorkflowLifecycleRegistrationDeps { runtimeState: WorkflowExtensionRuntimeState; @@ -57,6 +67,30 @@ export function registerWorkflowLifecycleHandlers( return { cancel: true }; }); + pi.on("session_before_shutdown", async (event, ctx) => { + const reason = typeof event === "object" && event !== null && "reason" in event + ? (event as { readonly reason?: string }).reason + : undefined; + if (reason !== "quit") return undefined; + + const inFlightRuns = topLevelWorkflowRuns(store.runs()).filter(blocksAppShutdown); + if (inFlightRuns.length === 0) return undefined; + + if (ctx?.hasUI === false || typeof ctx?.ui?.custom !== "function") return undefined; + + let shouldQuit: boolean | undefined; + try { + shouldQuit = await openWorkflowQuitConfirm(ctx.ui, inFlightRuns, deriveGraphTheme({})); + } catch { + return undefined; + } + if (shouldQuit !== false) return undefined; + + const workflowNoun = inFlightRuns.length === 1 ? "workflow" : "workflows"; + ctx?.ui?.notify?.(`Quit cancelled; ${inFlightRuns.length} in-flight ${workflowNoun} left running.`, "info"); + return { cancel: true }; + }); + pi.on("session_start", async (_event, ctx) => { deAdvertiseAskUserQuestionWhenHeadless(pi, ctx?.hasUI); killAllRuns({ store, cancellation: cancellationRegistry, persistence: runtimeState.persistenceRef.current }); diff --git a/packages/workflows/src/tui/graph-theme.ts b/packages/workflows/src/tui/graph-theme.ts index 9ad20ebc4..2e7242d2a 100644 --- a/packages/workflows/src/tui/graph-theme.ts +++ b/packages/workflows/src/tui/graph-theme.ts @@ -205,23 +205,24 @@ function parsePiAnsiToHex(ansi: string | undefined): string | undefined { * throw to overlay mount. */ function tryPiAccessor( + theme: PiRuntimeTheme, fn: ((color: string) => string) | undefined, color: string, ): string | undefined { if (typeof fn !== "function") return undefined; try { - return fn(color); + return fn.call(theme, color); } catch { return undefined; } } function fgHex(theme: PiRuntimeTheme, color: string): string | undefined { - return parsePiAnsiToHex(tryPiAccessor(theme.getFgAnsi, color)); + return parsePiAnsiToHex(tryPiAccessor(theme, theme.getFgAnsi, color)); } function bgHex(theme: PiRuntimeTheme, color: string): string | undefined { - return parsePiAnsiToHex(tryPiAccessor(theme.getBgAnsi, color)); + return parsePiAnsiToHex(tryPiAccessor(theme, theme.getBgAnsi, color)); } /** diff --git a/packages/workflows/src/tui/graph-view-input.ts b/packages/workflows/src/tui/graph-view-input.ts index 34fd4f2e2..449348f94 100644 --- a/packages/workflows/src/tui/graph-view-input.ts +++ b/packages/workflows/src/tui/graph-view-input.ts @@ -144,16 +144,21 @@ export abstract class GraphViewInputController extends GraphViewRenderer { } return true; } - // `q` quits/detaches the orchestrator view without authoritatively - // killing the workflow. The workflow remains resumable via - // `/workflow resume`; use `/workflow kill` for non-resumable disposal. + // `q` requests quit confirmation for a live run. The host closes only + // after confirmation; when there is no live target, restore the legacy + // close/fall-through behavior instead of swallowing the key. This is a + // resumable quit/detach, not the `/workflow kill` terminal path. if (matchesKey(data, "q")) { const run = this._getCurrentRun(); if (run && run.endedAt === undefined && this.onQuit) { this.onQuit(run.id); + return true; } - this.onClose?.(); - return true; + if (this.onClose) { + this.onClose(); + return true; + } + return false; } if (matchesKey(data, "h") && this.onHide) { this.onHide(); diff --git a/packages/workflows/src/tui/graph-view-types.ts b/packages/workflows/src/tui/graph-view-types.ts index ee206a61b..4c239eb47 100644 --- a/packages/workflows/src/tui/graph-view-types.ts +++ b/packages/workflows/src/tui/graph-view-types.ts @@ -10,9 +10,10 @@ export interface GraphViewOpts { graphTheme: GraphTheme; onClose?: () => void; /** - * Invoked when the user presses `q` inside the pane. This quits/detaches - * the orchestrator view and leaves the workflow resumable; it must not use - * the `/workflow kill` terminal path. + * Invoked when the user presses `q` inside the pane on an in-flight + * run. The host owns quit confirmation and closes the pane only after + * a confirmed resumable quit; GraphView does not close speculatively. + * This must not use the `/workflow kill` terminal path. */ onQuit?: (runId: string) => void; /** diff --git a/packages/workflows/src/tui/overlay-adapter.ts b/packages/workflows/src/tui/overlay-adapter.ts index 1b8dc2699..61f179639 100644 --- a/packages/workflows/src/tui/overlay-adapter.ts +++ b/packages/workflows/src/tui/overlay-adapter.ts @@ -16,9 +16,10 @@ */ import type { Store } from "../shared/store.js"; -import type { StoreSnapshot } from "../shared/store-types.js"; +import type { RunSnapshot, StoreSnapshot } from "../shared/store-types.js"; import type { ChatMessageRenderOptions, ReadonlyFooterDataProvider } from "@bastani/atomic"; import { WorkflowAttachPane } from "./workflow-attach-pane.js"; +import { openWorkflowQuitConfirm } from "./session-overlays.js"; import { WORKFLOW_STATUS_KEY } from "./workflow-status.js"; import { deriveGraphThemeFromPiTheme } from "./graph-theme.js"; import { quitRun as defaultQuitRun } from "../runs/background/quit.js"; @@ -50,6 +51,7 @@ export interface OverlayUISurface { getChatRenderSettings?: () => OverlayChatRenderSettings | undefined; getFooterDataProvider?: () => ReadonlyFooterDataProvider; setStatus?: (key: string, value: string | undefined) => void; + confirm?: (title: string, message: string) => Promise; } export interface OverlayPiSurface { @@ -318,16 +320,37 @@ export function buildGraphOverlayAdapter( clearHostCustomUiObservation(); done(undefined); }; + const graphTheme = deriveGraphThemeFromPiTheme(theme); + let quitConfirmationPending = false; + const requestQuit = (targetRunId: string): void => { + if (quitConfirmationPending) return; + const targetRun: RunSnapshot | undefined = store.runs() + .find((candidate) => candidate.id === targetRunId); + if (!targetRun || targetRun.endedAt !== undefined) return; + quitConfirmationPending = true; + void openWorkflowQuitConfirm(ui ?? {}, [targetRun], graphTheme) + .then((confirmed) => { + if (confirmed === false) return; + const liveTarget: RunSnapshot | undefined = store.runs() + .find((candidate) => candidate.id === targetRunId); + if (!liveTarget || liveTarget.endedAt !== undefined) return; + quitRun(targetRunId); + finish(); + }) + .finally(() => { + quitConfirmationPending = false; + }); + }; const view = new WorkflowAttachPane({ store, - graphTheme: deriveGraphThemeFromPiTheme(theme), + graphTheme, runId, stageControlRegistry: registry, stageUiBroker, uiStatus, onClose: finish, onHide: hideMounted, - onQuit: quitRun, + onQuit: requestQuit, initialAttachStageId: stageId, piTui: tui, piTheme: theme, diff --git a/packages/workflows/src/tui/session-confirm.ts b/packages/workflows/src/tui/session-confirm.ts index cea420f9c..35ada64c6 100644 --- a/packages/workflows/src/tui/session-confirm.ts +++ b/packages/workflows/src/tui/session-confirm.ts @@ -45,6 +45,7 @@ export interface KillConfirmRenderOpts { } const TITLE = "Kill workflow run?"; +const QUIT_TITLE = "Quit with active workflows?"; function padTo(s: string, width: number): string { const vis = visibleWidth(s); @@ -52,18 +53,27 @@ function padTo(s: string, width: number): string { return s + " ".repeat(width - vis); } -function renderHeader(width: number, theme: GraphTheme): string { +function renderTitledHeader(width: number, theme: GraphTheme, title: string): string { const inner = Math.max(4, width - 2); const border = hexToAnsi(theme.border); const error = hexToAnsi(theme.error); - const padded = ` ${TITLE} `; + const padded = ` ${title} `; const padLen = Math.max(0, inner - visibleWidth(padded)); const left = Math.min(2, padLen); const right = padLen - left; return `${border}╭${"─".repeat(left)}${RESET}${error}${BOLD}${padded}${RESET}${border}${"─".repeat(right)}╮${RESET}`; } -function renderFooter(width: number, theme: GraphTheme): string { +function renderHeader(width: number, theme: GraphTheme): string { + return renderTitledHeader(width, theme, TITLE); +} + +function renderFooter( + width: number, + theme: GraphTheme, + confirmLabel = "Kill", + focusedButton: KillConfirmState["focusedButton"] = 0, +): string { const inner = Math.max(4, width - 2); const border = hexToAnsi(theme.border); const dim = hexToAnsi(theme.dim); @@ -71,10 +81,11 @@ function renderFooter(width: number, theme: GraphTheme): string { const muted = hexToAnsi(theme.textMuted); const sep = `${dim} \u00b7 ${RESET}`; const hint = (key: string, label: string) => `${text}${key}${RESET} ${muted}${label}${RESET}`; + const enterLabel = focusedButton === 1 ? confirmLabel : "Cancel"; const line = [ - hint("y", "Kill"), + hint("y", confirmLabel), hint("n", "Cancel"), - hint(keyText("tui.select.confirm"), "Confirm"), + hint(keyText("tui.select.confirm"), enterLabel), hint(keyText("tui.select.cancel"), "Cancel"), ].join(sep); const leftRule = "\u2500\u2500 "; @@ -85,9 +96,9 @@ function renderFooter(width: number, theme: GraphTheme): string { return `${border}\u2570${RESET}${padTo(innerContent, inner)}${border}\u256f${RESET}`; } -function renderBlankRow(inner: number, theme: GraphTheme): string { +function renderBlankRow(inner: number, theme: GraphTheme, background = theme.bg): string { const border = hexToAnsi(theme.border); - const panelBg = hexBg(theme.bg); + const panelBg = hexBg(background); return `${border}│${RESET}${panelBg}${" ".repeat(inner)}${RESET}${border}│${RESET}`; } @@ -95,9 +106,10 @@ function renderTextRow( inner: number, theme: GraphTheme, content: string, + background = theme.bg, ): string { const border = hexToAnsi(theme.border); - const panelBg = hexBg(theme.bg); + const panelBg = hexBg(background); return `${border}│${RESET}${panelBg}${padTo(content, inner)}${RESET}${border}│${RESET}`; } @@ -109,7 +121,7 @@ function renderButton(label: string, focused: boolean, destructive: boolean, the return `${bg}${fg}${BOLD}${inner}${RESET}`; } const fg = destructive ? hexToAnsi(theme.error) : hexToAnsi(theme.textMuted); - const bg = hexBg(theme.surface); + const bg = hexBg(theme.backgroundElement); return `${bg}${fg}${inner}${RESET}`; } @@ -127,11 +139,11 @@ export function renderKillConfirm(opts: KillConfirmRenderOpts): string[] { const text = hexToAnsi(theme.text); const dim = hexToAnsi(theme.dim); const muted = hexToAnsi(theme.textMuted); - const panelBg = hexBg(theme.bg); + const panelBg = hexBg(theme.backgroundPanel); const lines: string[] = []; lines.push(renderHeader(width, theme)); - lines.push(renderBlankRow(inner, theme)); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); // Identity row: ⚠ · . Keep the destructive dialog // width-safe even for wide workflow names. @@ -141,27 +153,29 @@ export function renderKillConfirm(opts: KillConfirmRenderOpts): string[] { const name = truncateToWidth(run.name, nameBudget, "…"); const identity = ` ${warning}\u26a0${RESET}${panelBg} ${text}${BOLD}${name}${RESET}${panelBg} ${dim}\u00b7${RESET}${panelBg} ${muted}${idShort}${RESET}`; - lines.push(renderTextRow(inner, theme, identity)); + lines.push(renderTextRow(inner, theme, identity, theme.backgroundPanel)); // Status sub-line. const statusLine = run.endedAt === undefined ? ` ${muted}in-flight ${elapsed}, ${stagesRunning}/${stagesTotal} stages running${RESET}` : ` ${muted}${run.status} after ${elapsed}, ${stagesTotal} stages${RESET}`; - lines.push(renderTextRow(inner, theme, statusLine)); - lines.push(renderBlankRow(inner, theme)); + lines.push(renderTextRow(inner, theme, statusLine, theme.backgroundPanel)); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); // Body copy. lines.push(renderTextRow( inner, theme, ` ${muted}Aborts in-flight work and marks the run killed.${RESET}`, + theme.backgroundPanel, )); lines.push(renderTextRow( inner, theme, ` ${muted}Retains it in history/status for inspection.${RESET}`, + theme.backgroundPanel, )); - lines.push(renderBlankRow(inner, theme)); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); // Buttons row, centered. const cancelBtn = renderButton("Cancel", state.focusedButton === 0, false, theme); @@ -173,9 +187,86 @@ export function renderKillConfirm(opts: KillConfirmRenderOpts): string[] { `${" ".repeat(leftPad)}${cancelBtn}${panelBg} ${RESET}${killBtn}${panelBg}${" ".repeat(rightPad)}${RESET}`; const border = hexToAnsi(theme.border); lines.push(`${border}│${RESET}${panelBg}${padTo(buttonsRow, inner)}${RESET}${border}│${RESET}`); - lines.push(renderBlankRow(inner, theme)); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); + + lines.push(renderFooter(width, theme, "Kill", state.focusedButton)); + return lines; +} + +export interface WorkflowQuitConfirmRenderOpts { + width: number; + theme: GraphTheme; + runs: RunSnapshot[]; + state: KillConfirmState; + now?: number; +} + +export function renderWorkflowQuitConfirm(opts: WorkflowQuitConfirmRenderOpts): string[] { + const { width, theme, runs, state } = opts; + const now = opts.now ?? Date.now(); + const inner = Math.max(50, width - 2); + const count = runs.length; + const workflowNoun = count === 1 ? "workflow" : "workflows"; + const runningStages = runs.reduce((sum, run) => sum + run.stages.filter((s) => s.status === "running").length, 0); + const totalStages = runs.reduce((sum, run) => sum + run.stages.length, 0); + const oldestStartedAt = Math.min(...runs.map((run) => run.startedAt)); + const elapsed = Number.isFinite(oldestStartedAt) ? fmtDuration(Math.max(0, now - oldestStartedAt)) : "0s"; + + const warning = hexToAnsi(theme.warning); + const text = hexToAnsi(theme.text); + const muted = hexToAnsi(theme.textMuted); + const dim = hexToAnsi(theme.dim); + const panelBg = hexBg(theme.backgroundPanel); - lines.push(renderFooter(width, theme)); + const lines: string[] = []; + lines.push(renderTitledHeader(width, theme, QUIT_TITLE)); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); + lines.push(renderTextRow( + inner, + theme, + ` ${warning}\u26a0${RESET}${panelBg} ${text}${BOLD}${count} in-flight ${workflowNoun}${RESET}${panelBg} ${dim}\u00b7${RESET}${panelBg} ${muted}${runningStages}/${totalStages} stages running${RESET}`, + theme.backgroundPanel, + )); + lines.push(renderTextRow( + inner, + theme, + ` ${muted}Oldest active run has been running for ${elapsed}.${RESET}`, + theme.backgroundPanel, + )); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); + lines.push(renderTextRow( + inner, + theme, + ` ${muted}Quitting Atomic will abort active workflow work.${RESET}`, + theme.backgroundPanel, + )); + lines.push(renderTextRow( + inner, + theme, + ` ${muted}Killed runs are retained in workflow history/status.${RESET}`, + theme.backgroundPanel, + )); + const names = runs.slice(0, 3).map((run) => `${run.name} (${run.id.slice(0, 8)})`).join(", "); + const suffix = runs.length > 3 ? `, +${runs.length - 3} more` : ""; + lines.push(renderTextRow( + inner, + theme, + ` ${muted}${truncateToWidth(`Active: ${names}${suffix}`, Math.max(1, inner - 6), "…")}${RESET}`, + theme.backgroundPanel, + )); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); + + const cancelBtn = renderButton("Cancel", state.focusedButton === 0, false, theme); + const quitBtn = renderButton("\u25c6 Quit & kill", state.focusedButton === 1, true, theme); + const buttonsVis = visibleWidth(" Cancel ") + 3 + visibleWidth(" \u25c6 Quit & kill "); + const leftPad = Math.max(2, Math.floor((inner - buttonsVis) / 2)); + const rightPad = Math.max(0, inner - leftPad - buttonsVis); + const buttonsRow = + `${" ".repeat(leftPad)}${cancelBtn}${panelBg} ${RESET}${quitBtn}${panelBg}${" ".repeat(rightPad)}${RESET}`; + const border = hexToAnsi(theme.border); + lines.push(`${border}│${RESET}${panelBg}${padTo(buttonsRow, inner)}${RESET}${border}│${RESET}`); + lines.push(renderBlankRow(inner, theme, theme.backgroundPanel)); + lines.push(renderFooter(width, theme, "Quit & kill", state.focusedButton)); return lines; } @@ -188,6 +279,14 @@ export type KillConfirmAction = | { kind: "cancel" } | { kind: "confirm" }; +function isCtrlC(data: string): boolean { + return ( + matchesKey(data, Key.ctrl("c")) || + data === "ctrl+C" || + data === "\u0003" + ); +} + export function handleKillConfirmInput( data: string, state: KillConfirmState, @@ -195,7 +294,7 @@ export function handleKillConfirmInput( // Direct shortcuts bypass focus. if (matchesKey(data, "y") || matchesKey(data, Key.shift("y"))) return { kind: "confirm" }; if (matchesKey(data, "n") || matchesKey(data, Key.shift("n"))) return { kind: "cancel" }; - if (matchesKey(data, Key.escape)) return { kind: "cancel" }; + if (matchesKey(data, Key.escape) || isCtrlC(data)) return { kind: "cancel" }; // Tab / arrows toggle focus. if ( diff --git a/packages/workflows/src/tui/session-overlays.ts b/packages/workflows/src/tui/session-overlays.ts index 8591f7871..02b00a3aa 100644 --- a/packages/workflows/src/tui/session-overlays.ts +++ b/packages/workflows/src/tui/session-overlays.ts @@ -38,7 +38,7 @@ import type { PiOverlayOptions, } from "../extension/wiring.js"; import type { Store } from "../shared/store.js"; -import type { GraphTheme } from "./graph-theme.js"; +import { deriveGraphThemeFromPiTheme, type GraphTheme } from "./graph-theme.js"; import { createSessionPickerState, handleSessionPickerInput, @@ -49,6 +49,7 @@ import { createKillConfirmState, handleKillConfirmInput, renderKillConfirm, + renderWorkflowQuitConfirm, } from "./session-confirm.js"; import type { RunSnapshot } from "../shared/store-types.js"; @@ -179,6 +180,36 @@ export interface ConfirmUiSurface extends UiSurface { confirm?: (title: string, message: string) => Promise; } +function hasPiRuntimeTheme(theme: unknown): boolean { + if (!theme || typeof theme !== "object") return false; + const candidate = theme as { getFgAnsi?: unknown; getBgAnsi?: unknown }; + return typeof candidate.getFgAnsi === "function" || typeof candidate.getBgAnsi === "function"; +} + +function observeCustomMount( + mount: () => unknown, + factoryInvoked: () => boolean, + settleHostFailure: () => void, +): void { + let result: unknown; + try { + result = mount(); + } catch { + settleHostFailure(); + return; + } + + // Atomic/pi custom overlays invoke the component factory synchronously while + // mounting. If the mount promise settles before that happens, no overlay can + // receive input, so callers should apply their degraded-host fallback. + Promise.resolve(result).then( + () => { + if (!factoryInvoked()) settleHostFailure(); + }, + () => settleHostFailure(), + ); +} + export function openKillConfirm( ui: ConfirmUiSurface, run: RunSnapshot, @@ -201,6 +232,13 @@ export function openKillConfirm( const state = createKillConfirmState(); let settled = false; + let factoryInvoked = false; + + const settle = (result: boolean): void => { + if (settled) return; + settled = true; + resolve(result); + }; const factory = ( tui: PiCustomOverlayFactoryTui, @@ -208,6 +246,7 @@ export function openKillConfirm( _keys: unknown, done: (r: undefined) => void, ): PiCustomComponent => { + factoryInvoked = true; const finish = (result: boolean): void => { if (settled) return; settled = true; @@ -225,15 +264,83 @@ export function openKillConfirm( finish(action.kind === "confirm"); }, invalidate: () => tui.requestRender?.(), - dispose: () => { - if (!settled) { - settled = true; - resolve(false); + dispose: () => settle(false), + }; + }; + + observeCustomMount( + () => custom(factory, { overlay: true, overlayOptions: CONFIRM_OVERLAY }), + () => factoryInvoked, + () => settle(false), + ); + }); +} + +/** + * Mount a safe default-cancel quit confirmation for active workflows. + * Returns `undefined` when custom UI is unavailable so callers can fail open + * for headless/automation paths instead of relying on a generic yes-default + * confirm implementation. + */ +export function openWorkflowQuitConfirm( + ui: UiSurface, + runs: RunSnapshot[], + theme: GraphTheme, +): Promise { + return new Promise((resolve) => { + const custom = ui.custom; + if (typeof custom !== "function") { + resolve(undefined); + return; + } + + const state = createKillConfirmState(); + let settled = false; + let factoryInvoked = false; + + const settle = (result: boolean | undefined): void => { + if (settled) return; + settled = true; + resolve(result); + }; + + const factory = ( + tui: PiCustomOverlayFactoryTui, + piTheme: unknown, + _keys: unknown, + done: (r: undefined) => void, + ): PiCustomComponent => { + factoryInvoked = true; + const overlayTheme = hasPiRuntimeTheme(piTheme) + ? deriveGraphThemeFromPiTheme(piTheme) + : theme; + const finish = (result: boolean): void => { + if (settled) return; + settled = true; + done(undefined); + resolve(result); + }; + return { + render: (width: number) => renderWorkflowQuitConfirm({ width, theme: overlayTheme, runs, state }), + handleInput: (data: string) => { + const action = handleKillConfirmInput(data, state); + if (action.kind === "noop") { + tui.requestRender?.(); + return; } + finish(action.kind === "confirm"); }, + invalidate: () => tui.requestRender?.(), + // Once the prompt mounted, host disposal is equivalent to user cancel; + // only never-mounted hosts fail open to avoid wedging headless quits. + dispose: () => settle(false), }; }; - void custom(factory, { overlay: true, overlayOptions: CONFIRM_OVERLAY }); + observeCustomMount( + () => custom(factory, { overlay: true, overlayOptions: CONFIRM_OVERLAY }), + () => factoryInvoked, + () => settle(undefined), + ); }); } diff --git a/packages/workflows/src/tui/workflow-attach-pane-types.ts b/packages/workflows/src/tui/workflow-attach-pane-types.ts index 414d51de0..e67b25f55 100644 --- a/packages/workflows/src/tui/workflow-attach-pane-types.ts +++ b/packages/workflows/src/tui/workflow-attach-pane-types.ts @@ -29,7 +29,7 @@ export interface WorkflowAttachPaneOpts { onClose: () => void; /** Called when the user requests the host to hide the popup. */ onHide?: () => void; - /** Called when the user quits/detaches the active run (q in graph mode). */ + /** Called when the user requests quitting/detaching the active run (q in graph mode); host confirms and closes after quit. */ onQuit?: (runId: string) => void; /** Called when the user resolves a HIL prompt via the graph view. */ onPromptResolve?: (runId: string, promptId: string, response: unknown) => void; diff --git a/packages/workflows/src/tui/workflow-attach-pane.ts b/packages/workflows/src/tui/workflow-attach-pane.ts index 42347db7d..54f4f2605 100644 --- a/packages/workflows/src/tui/workflow-attach-pane.ts +++ b/packages/workflows/src/tui/workflow-attach-pane.ts @@ -38,15 +38,9 @@ import type { StageUiBroker } from "../shared/stage-ui-broker.js"; import type { StageSnapshot, StoreSnapshot } from "../shared/store-types.js"; import { expandWorkflowGraph } from "../shared/expanded-workflow-graph.js"; import { WORKFLOW_STATUS_KEY } from "./workflow-status.js"; -/** - * Surface used to write Pi's footer/status tag while the attach pane is - * mounted. Passing `undefined` clears the slot — required on dispose so - * the `pi-workflows/[/]` tag does NOT linger in every - * subsequent chat message after the overlay is closed. - * cross-ref: @bastani/atomic docs/extensions.md - * §Widgets, Status, and Footer (`ctx.ui.setStatus`). - */ import type { AttachUiStatusSurface, WorkflowAttachPaneMode, WorkflowAttachPaneOpts } from "./workflow-attach-pane-types.js"; +export type { AttachUiStatusSurface, WorkflowAttachPaneMode, WorkflowAttachPaneOpts } from "./workflow-attach-pane-types.js"; + const ENTER_TRANSITION_QUARANTINE_MS = 200; export class WorkflowAttachPane implements Component { private store: Store; diff --git a/test/integration/overlay-entrypoints-hide-keys.test.ts b/test/integration/overlay-entrypoints-hide-keys.test.ts index 2941cb73c..59c735151 100644 --- a/test/integration/overlay-entrypoints-hide-keys.test.ts +++ b/test/integration/overlay-entrypoints-hide-keys.test.ts @@ -130,7 +130,7 @@ describe("buildGraphOverlayAdapter — Ctrl+D / h non-destructive hide", () => { assert.equal(run!.endedAt, undefined); }); - test("`q` on a real custom mount quits and retains the active run as resumable", () => { + test("`q` on a real custom mount confirms, quits, and retains the active run as resumable", async () => { const runId = `q-quit-${Date.now()}`; const store = createStore(); store.recordRunStart({ @@ -142,7 +142,7 @@ describe("buildGraphOverlayAdapter — Ctrl+D / h non-destructive hide", () => { startedAt: Date.now(), }); - let capturedComponent: PiCustomComponent | undefined; + const capturedComponents: PiCustomComponent[] = []; const customFn: PiCustomOverlayFunction = (factoryArg, options) => { const { handle } = buildOverlayHandle(); options.onHandle?.(handle); @@ -151,14 +151,19 @@ describe("buildGraphOverlayAdapter — Ctrl+D / h non-destructive hide", () => { }; const component = factoryArg(tui, {}, {}, () => undefined); if (component instanceof Promise) throw new Error("expected sync factory"); - capturedComponent = component; + capturedComponents.push(component); return undefined; }; const adapter = buildGraphOverlayAdapter({ ui: { custom: customFn } }, store); adapter.open(runId); - capturedComponent!.handleInput!("q"); + capturedComponents[0]!.handleInput!("q"); + assert.equal(capturedComponents.length, 2, "q must mount the confirmation overlay first"); + assert.notEqual(store.runs().find((r) => r.id === runId)?.status, "killed"); + + capturedComponents[1]!.handleInput!("y"); + await Promise.resolve(); const run = store.runs().find((r) => r.id === runId); assert.ok(run, "`q` must retain the run in live history/status for inspection"); diff --git a/test/unit/extension-shutdown.test.ts b/test/unit/extension-shutdown.test.ts new file mode 100644 index 000000000..7d17d8f1a --- /dev/null +++ b/test/unit/extension-shutdown.test.ts @@ -0,0 +1,109 @@ +import { beforeEach, test } from "bun:test"; +import assert from "node:assert/strict"; +import factory, { type ExtensionAPI } from "../../packages/workflows/src/extension/index.js"; +import { store } from "../../packages/workflows/src/shared/store.js"; +import type { RunSnapshot } from "../../packages/workflows/src/shared/store-types.js"; +import type { PiCustomOverlayFactoryTui } from "../../packages/workflows/src/extension/wiring.js"; + +type SessionBeforeShutdownHandler = (event?: { readonly reason?: string }, ctx?: ShutdownContext) => Promise | unknown; +interface CustomComponent { + handleInput?: (data: string) => void; +} + +interface ShutdownContext { + readonly hasUI?: boolean; + readonly ui?: { + readonly custom?: ( + factory: ( + tui: PiCustomOverlayFactoryTui, + theme: unknown, + keys: unknown, + done: (result: undefined) => void, + ) => CustomComponent, + options?: { readonly overlay?: boolean; readonly overlayOptions?: object }, + ) => unknown; + readonly notify?: (message: string, type?: string) => void; + }; +} + +function workflowRun(overrides: Partial = {}): RunSnapshot { + return { + id: "run-1", + name: "Test workflow", + inputs: {}, + status: "running", + stages: [], + startedAt: Date.now(), + ...overrides, + }; +} + +function sessionBeforeShutdownHandler(): SessionBeforeShutdownHandler { + const handlers = new Map(); + const pi: ExtensionAPI = { + registerTool: () => undefined, + registerCommand: () => undefined, + registerMessageRenderer: () => undefined, + registerFlag: () => undefined, + registerShortcut: () => undefined, + on: (event, handler) => { + handlers.set(event, handler as SessionBeforeShutdownHandler); + }, + disableAsyncDiscovery: true, + }; + factory(pi); + const handler = handlers.get("session_before_shutdown"); + if (handler === undefined) assert.fail("session_before_shutdown handler was not registered"); + return handler; +} + +beforeEach(() => { + store.clear(); +}); + +test("session_before_shutdown does not prompt for a graph-quit resumable run", async () => { + store.recordRunStart(workflowRun({ + status: "paused", + exitReason: "quit", + resumable: true, + })); + const handler = sessionBeforeShutdownHandler(); + let customCalls = 0; + + const result = await handler({ reason: "quit" }, { + ui: { + custom: () => { + customCalls += 1; + return undefined; + }, + notify: () => undefined, + }, + }); + + assert.equal(result, undefined); + assert.equal(customCalls, 0); +}); + +test("session_before_shutdown still prompts for paused runs that are not graph-quit resumable", async () => { + store.recordRunStart(workflowRun({ + status: "paused", + resumable: true, + })); + const handler = sessionBeforeShutdownHandler(); + let customCalls = 0; + + const result = await handler({ reason: "quit" }, { + ui: { + custom: (componentFactory) => { + customCalls += 1; + const component: CustomComponent = componentFactory({ requestRender: () => undefined }, {}, {}, () => undefined); + component.handleInput?.("n"); + return undefined; + }, + notify: () => undefined, + }, + }); + + assert.deepEqual(result, { cancel: true }); + assert.equal(customCalls, 1); +}); diff --git a/test/unit/extension.test.ts b/test/unit/extension.test.ts index 414ab4e96..357656933 100644 --- a/test/unit/extension.test.ts +++ b/test/unit/extension.test.ts @@ -74,6 +74,14 @@ function getSessionBeforeSwitchHandler(): SessionBeforeSwitchHandler { return handler; } +function getSessionBeforeShutdownHandler(): SessionBeforeSwitchHandler { + const handler = captureHandlers().get("session_before_shutdown"); + if (handler === undefined) { + assert.fail("session_before_shutdown handler was not registered"); + } + return handler; +} + beforeEach(() => { stageControlRegistry.clear(); store.clear(); @@ -185,6 +193,76 @@ test("session_before_switch cancels /new and /resume when warning is declined", } }); +test("session_before_shutdown confirms quit with active workflows and cancels by default", async () => { + store.recordRunStart(workflowRun({ id: "run-1", stages: [{ id: "s1", name: "plan", status: "running", parentIds: [], toolEvents: [] }] })); + const handler = getSessionBeforeShutdownHandler(); + const notifications: Array<{ message: string; type?: string }> = []; + let customMounted = 0; + + const result = await handler({ reason: "quit" }, { + ui: { + custom: (factory: Function) => { + customMounted += 1; + const component = factory({ requestRender: () => undefined }, {}, {}, () => undefined); + component.handleInput("n"); + return undefined; + }, + notify: (message: string, type?: string) => notifications.push({ message, type }), + }, + }); + + assert.deepEqual(result, { cancel: true }); + assert.equal(customMounted, 1); + assert.equal(notifications.at(-1)?.type, "info"); + assert.match(notifications.at(-1)?.message ?? "", /quit cancelled/i); + assert.equal(store.runs()[0]?.endedAt, undefined); +}); + +test("session_before_shutdown allows quit after custom confirmation", async () => { + store.recordRunStart(workflowRun({ id: "run-1" })); + const handler = getSessionBeforeShutdownHandler(); + + const result = await handler({ reason: "quit" }, { + ui: { + custom: (factory: Function) => { + const component = factory({ requestRender: () => undefined }, {}, {}, () => undefined); + component.handleInput("y"); + return undefined; + }, + notify: () => undefined, + }, + }); + + assert.equal(result, undefined); +}); + +test("session_before_shutdown fails open without custom UI", async () => { + store.recordRunStart(workflowRun({ id: "run-1" })); + const handler = getSessionBeforeShutdownHandler(); + + assert.equal(await handler({ reason: "quit" }, { ui: { notify: () => undefined } }), undefined); +}); + +test("session_before_shutdown fails open when hasUI is false even with no-op custom UI", async () => { + store.recordRunStart(workflowRun({ id: "run-1" })); + const handler = getSessionBeforeShutdownHandler(); + let customCalls = 0; + + const result = await handler({ reason: "quit" }, { + hasUI: false, + ui: { + custom: () => { + customCalls += 1; + return undefined; + }, + notify: () => undefined, + }, + }); + + assert.equal(result, undefined); + assert.equal(customCalls, 0); +}); + test("session_before_switch does not prompt without in-flight workflows", async () => { store.clear(); try { diff --git a/test/unit/graph-theme.test.ts b/test/unit/graph-theme.test.ts index a5c9b756e..d6e3b388c 100644 --- a/test/unit/graph-theme.test.ts +++ b/test/unit/graph-theme.test.ts @@ -48,6 +48,38 @@ describe("deriveGraphThemeFromPiTheme", () => { assert.deepEqual(out, MOCHA_DEFAULTS); }); + test("maps truecolor Pi tokens from receiver-dependent Pi methods", () => { + const piTheme = { + fgMap: { + border: truecolorFg(84, 125, 167), + warning: truecolorFg(154, 115, 38), + error: truecolorFg(170, 85, 85), + }, + bgMap: { + customMessageBg: truecolorBg(244, 244, 245), + toolPendingBg: truecolorBg(250, 250, 250), + }, + getFgAnsi(color: string): string { + const out = this.fgMap[color as keyof typeof this.fgMap]; + if (!out) throw new Error(`Unknown theme color: ${color}`); + return out; + }, + getBgAnsi(color: string): string { + const out = this.bgMap[color as keyof typeof this.bgMap]; + if (!out) throw new Error(`Unknown theme background color: ${color}`); + return out; + }, + }; + + const theme = deriveGraphThemeFromPiTheme(piTheme); + + assert.equal(theme.border, "#547da7"); + assert.equal(theme.warning, "#9a7326"); + assert.equal(theme.error, "#aa5555"); + assert.equal(theme.backgroundPanel, "#fafafa"); + assert.equal(theme.backgroundElement, "#f4f4f5"); + }); + test("maps truecolor Pi tokens onto GraphTheme roles", () => { const fgMap: Record = { accent: truecolorFg(0x12, 0x34, 0x56), diff --git a/test/unit/overlay-graph-quit.test.ts b/test/unit/overlay-graph-quit.test.ts new file mode 100644 index 000000000..a8800629b --- /dev/null +++ b/test/unit/overlay-graph-quit.test.ts @@ -0,0 +1,56 @@ +import { describe, it, mock } from "bun:test"; +import assert from "node:assert/strict"; +import { GraphView } from "../../packages/workflows/src/tui/graph-view.js"; +import * as h from "./overlay-graph-helpers.js"; + +function makeGraphView(opts: { + ended?: boolean; + onClose?: () => void; + onQuit?: (runId: string) => void; +} = {}): GraphView { + const stages = [h.makeStage("A")]; + const baseRun = h.makeRun(stages); + const snap = opts.ended === true + ? { + runs: [{ ...baseRun, status: "completed" as const, endedAt: Date.now() }], + notices: [], + version: 1, + } + : h.makeSnap(stages); + return new GraphView({ + mode: "overlay", + runId: "run-1", + store: h.makeStore(snap), + graphTheme: h.defaultTheme, + onClose: opts.onClose, + onQuit: opts.onQuit, + }); +} + +describe("GraphView q quit handling", () => { + it("requests live-run quit without speculatively closing", () => { + const onClose = mock(() => {}); + const quit: string[] = []; + const view = makeGraphView({ + onClose, + onQuit: (runId) => quit.push(runId), + }); + + assert.equal(view.handleInput("q"), true); + assert.deepEqual(quit, ["run-1"]); + assert.equal(onClose.mock.calls.length, 0); + view.dispose(); + }); + + it("falls through when q has no live run and no close handler", () => { + const quit: string[] = []; + const view = makeGraphView({ + ended: true, + onQuit: (runId) => quit.push(runId), + }); + + assert.equal(view.handleInput("q"), false); + assert.deepEqual(quit, []); + view.dispose(); + }); +}); diff --git a/test/unit/session-confirm-list.test.ts b/test/unit/session-confirm-list.test.ts index 1f34c0629..8bc26847b 100644 --- a/test/unit/session-confirm-list.test.ts +++ b/test/unit/session-confirm-list.test.ts @@ -9,12 +9,16 @@ import { handleKillConfirmInput, renderKillConfirm, renderWorkflowKilledNotice, + renderWorkflowQuitConfirm, } from "../../packages/workflows/src/tui/session-confirm.ts"; import { renderSessionList } from "../../packages/workflows/src/tui/session-list.ts"; +import { openKillConfirm, openWorkflowQuitConfirm } from "../../packages/workflows/src/tui/session-overlays.ts"; import { deriveGraphTheme } from "../../packages/workflows/src/tui/graph-theme.ts"; import type { RunSnapshot } from "../../packages/workflows/src/shared/store-types.ts"; import { visibleWidth } from "../../packages/workflows/src/tui/text-helpers.ts"; +const ANSI_RE = /\x1b\[[0-9;]*m/g; +const stripAnsi = (s: string): string => s.replace(ANSI_RE, ""); function makeRun(over: Partial): RunSnapshot { return { @@ -31,12 +35,15 @@ function makeRun(over: Partial): RunSnapshot { }; } -test("kill confirm: y always confirms, n / esc variants cancel", () => { +test("kill confirm: y always confirms, n / esc / Ctrl+C variants cancel", () => { const s = createKillConfirmState(); assert.deepEqual(handleKillConfirmInput("y", s), { kind: "confirm" }); assert.deepEqual(handleKillConfirmInput("Y", s), { kind: "confirm" }); assert.deepEqual(handleKillConfirmInput("n", s), { kind: "cancel" }); assert.deepEqual(handleKillConfirmInput(Key.escape, s), { kind: "cancel" }); + for (const key of [Key.ctrl("c"), "ctrl+C", "\x03", "\x1b[99;5u", "\x1b[99;5:1u", "\x1b[27;5;99~"]) { + assert.deepEqual(handleKillConfirmInput(key, s), { kind: "cancel" }); + } }); test("kill confirm: tab toggles focus, enter commits focused button", () => { @@ -70,6 +77,10 @@ test("kill confirm renders run identity and button row", () => { assert.match(joined, /abc12345/); assert.match(joined, /Cancel/); assert.match(joined, /Kill run/); + const plain = stripAnsi(joined); + assert.match(plain, /y Kill/); + assert.match(plain, /enter Cancel/); + assert.doesNotMatch(plain, /enter Confirm/); assert.match(joined, /1\/2 stages running/); assert.match(joined, /marks the run killed/); assert.match(joined, /Retains it in history\/status for inspection/); @@ -94,6 +105,166 @@ test("kill confirm clamps long and wide workflow names to the dialog width", () assert.match(lines.join("\n"), /…/); }); +test("confirm modal rows use panel tokens instead of graph canvas tokens", () => { + const theme = deriveGraphTheme({ + bg: "#010203", + surface: "#020304", + backgroundPanel: "#fafafa", + backgroundElement: "#f4f4f5", + }); + const state = createKillConfirmState(); + const run = makeRun({ + id: "abc12345-0000-0000-0000-000000000000", + name: "panel-theme", + status: "running", + startedAt: 1000, + stages: [{ id: "s1", name: "plan", status: "running", parentIds: [], toolEvents: [] }], + }); + + const kill = renderKillConfirm({ width: 70, theme, run, state, now: 5000 }).join("\n"); + const quit = renderWorkflowQuitConfirm({ width: 76, theme, state, now: 5000, runs: [run] }).join("\n"); + + for (const rendered of [kill, quit]) { + assert.match(rendered, /\x1b\[48;2;250;250;250m/); + assert.match(rendered, /\x1b\[48;2;244;244;245m/); + assert.doesNotMatch(rendered, /\x1b\[48;2;1;2;3m/); + assert.doesNotMatch(rendered, /\x1b\[48;2;2;3;4m/); + } +}); + +test("confirm footers describe Enter as the currently focused button", () => { + const theme = deriveGraphTheme({}); + const run = makeRun({ + id: "abc12345-0000-0000-0000-000000000000", + name: "footer-theme", + stages: [{ id: "s1", name: "plan", status: "running", parentIds: [], toolEvents: [] }], + }); + + const killDefault = createKillConfirmState(); + const killDefaultPlain = stripAnsi(renderKillConfirm({ width: 80, theme, run, state: killDefault }).join("\n")); + assert.match(killDefaultPlain, /enter Cancel/); + assert.doesNotMatch(killDefaultPlain, /enter Confirm/); + + const killFocused = createKillConfirmState(); + handleKillConfirmInput(Key.tab, killFocused); + const killFocusedPlain = stripAnsi(renderKillConfirm({ width: 80, theme, run, state: killFocused }).join("\n")); + assert.match(killFocusedPlain, /enter Kill/); + + const quitDefault = createKillConfirmState(); + const quitDefaultPlain = stripAnsi(renderWorkflowQuitConfirm({ width: 84, theme, runs: [run], state: quitDefault }).join("\n")); + assert.match(quitDefaultPlain, /enter Cancel/); + assert.doesNotMatch(quitDefaultPlain, /enter Confirm/); + + const quitFocused = createKillConfirmState(); + handleKillConfirmInput(Key.tab, quitFocused); + const quitFocusedPlain = stripAnsi(renderWorkflowQuitConfirm({ width: 84, theme, runs: [run], state: quitFocused }).join("\n")); + assert.match(quitFocusedPlain, /enter Quit & kill/); +}); + +test("workflow quit confirm defaults to cancel and renders active-run summary", () => { + const theme = deriveGraphTheme({}); + const state = createKillConfirmState(); + const lines = renderWorkflowQuitConfirm({ + width: 76, + theme, + state, + now: 61_000, + runs: [ + makeRun({ + id: "abc12345-0000-0000-0000-000000000000", + name: "alpha", + startedAt: 1_000, + stages: [{ id: "s1", name: "plan", status: "running", parentIds: [], toolEvents: [] }], + }), + makeRun({ + id: "def67890-0000-0000-0000-000000000000", + name: "beta", + startedAt: 31_000, + stages: [{ id: "s2", name: "build", status: "pending", parentIds: [], toolEvents: [] }], + }), + ], + }); + + assert.equal(state.focusedButton, 0); + assert.deepEqual(handleKillConfirmInput(Key.enter, state), { kind: "cancel" }); + const joined = lines.join("\n"); + const plain = stripAnsi(joined); + assert.match(joined, /Quit with active workflows/); + assert.match(joined, /2 in-flight workflows/); + assert.match(joined, /Quit & kill/); + assert.match(plain, /y Quit & kill/); + assert.match(plain, /enter Cancel/); + assert.doesNotMatch(plain, /enter Confirm/); + assert.doesNotMatch(plain, /y Kill/); + assert.match(joined, /Killed runs are retained/); + assert.match(joined, /alpha/); + assert.match(joined, /beta/); +}); + +test("workflow quit confirm fails open when custom UI rejects or never mounts", async () => { + const theme = deriveGraphTheme({}); + const runs = [makeRun({ id: "run-quit" })]; + + assert.equal( + await openWorkflowQuitConfirm( + { + custom: () => Promise.reject(new Error("custom unavailable")), + }, + runs, + theme, + ), + undefined, + ); + + let factoryCalls = 0; + assert.equal( + await openWorkflowQuitConfirm( + { + custom: () => { + factoryCalls += 1; + return undefined; + }, + }, + runs, + theme, + ), + undefined, + ); + assert.equal(factoryCalls, 1); +}); + +test("kill confirm cancels safely when custom UI rejects or never mounts", async () => { + const theme = deriveGraphTheme({}); + const run = makeRun({ id: "run-kill" }); + + assert.equal( + await openKillConfirm( + { + custom: () => Promise.reject(new Error("custom unavailable")), + }, + run, + theme, + ), + false, + ); + + let factoryCalls = 0; + assert.equal( + await openKillConfirm( + { + custom: () => { + factoryCalls += 1; + return undefined; + }, + }, + run, + theme, + ), + false, + ); + assert.equal(factoryCalls, 1); +}); + test("workflow killed notice renders transparent completion details", () => { const theme = deriveGraphTheme({}); const width = 72; diff --git a/test/unit/session-overlays.test.ts b/test/unit/session-overlays.test.ts new file mode 100644 index 000000000..db3eb4dc0 --- /dev/null +++ b/test/unit/session-overlays.test.ts @@ -0,0 +1,127 @@ +import { test } from "bun:test"; +import assert from "node:assert/strict"; +import { + openKillConfirm, + openWorkflowQuitConfirm, + type ConfirmUiSurface, + type UiSurface, +} from "../../packages/workflows/src/tui/session-overlays.js"; +import { deriveGraphTheme } from "../../packages/workflows/src/tui/graph-theme.js"; +import type { RunSnapshot } from "../../packages/workflows/src/shared/store-types.js"; + +function workflowRun(overrides: Partial = {}): RunSnapshot { + return { + id: "run-12345678", + name: "Test workflow", + inputs: {}, + status: "running", + stages: [], + startedAt: Date.now(), + ...overrides, + }; +} + +const theme = deriveGraphTheme({}); + +test("openWorkflowQuitConfirm fail-opens when custom UI rejects before mounting", async () => { + const ui: UiSurface = { + custom: () => Promise.reject(new Error("custom unavailable")), + }; + + assert.equal(await openWorkflowQuitConfirm(ui, [workflowRun()], theme), undefined); +}); + +test("openWorkflowQuitConfirm fail-opens when custom UI resolves without invoking the factory", async () => { + let customCalls = 0; + const ui: UiSurface = { + custom: async (factory, options) => { + customCalls += 1; + assert.equal(options.overlay, true); + void factory; + return undefined; + }, + }; + + const result = await openWorkflowQuitConfirm(ui, [workflowRun()], theme); + + assert.equal(result, undefined); + assert.equal(customCalls, 1); +}); + +test("openWorkflowQuitConfirm keeps mounted custom UI cancel-by-default behavior", async () => { + const ui: UiSurface = { + custom: (factory) => { + const component = factory({ requestRender: () => undefined }, {}, {}, () => undefined); + if (component instanceof Promise) throw new Error("test factory should be sync"); + if (typeof component.handleInput !== "function") throw new Error("test component should handle input"); + component.handleInput("enter"); + return undefined; + }, + }; + + assert.equal(await openWorkflowQuitConfirm(ui, [workflowRun()], theme), false); +}); + +test("openWorkflowQuitConfirm renders with receiver-dependent host custom UI runtime theme", async () => { + let rendered = ""; + const runtimeTheme = { + fgMap: { + error: "\x1b[38;2;1;2;3m", + border: "\x1b[38;2;84;125;167m", + warning: "\x1b[38;2;154;115;38m", + }, + bgMap: { + toolPendingBg: "\x1b[48;2;250;250;250m", + customMessageBg: "\x1b[48;2;244;244;245m", + }, + getFgAnsi(color: string): string { + const out = this.fgMap[color as keyof typeof this.fgMap]; + if (!out) throw new Error(`unknown foreground: ${color}`); + return out; + }, + getBgAnsi(color: string): string { + const out = this.bgMap[color as keyof typeof this.bgMap]; + if (!out) throw new Error(`unknown background: ${color}`); + return out; + }, + }; + const ui: UiSurface = { + custom: (factory) => { + const component = factory({ requestRender: () => undefined }, runtimeTheme, {}, () => undefined); + if (component instanceof Promise) throw new Error("test factory should be sync"); + rendered = component.render(80).join("\n"); + component.dispose?.(); + return undefined; + }, + }; + + assert.equal(await openWorkflowQuitConfirm(ui, [workflowRun()], theme), false); + assert.match(rendered, /\x1b\[38;2;1;2;3m/); + assert.match(rendered, /\x1b\[38;2;84;125;167m/); + assert.match(rendered, /\x1b\[48;2;250;250;250m/); + assert.match(rendered, /\x1b\[48;2;244;244;245m/); + assert.doesNotMatch(rendered, /\x1b\[38;2;243;139;168m/); + assert.doesNotMatch(rendered, /\x1b\[48;2;30;30;46m/); +}); + +test("openKillConfirm resolves false when custom UI rejects before mounting", async () => { + const ui: ConfirmUiSurface = { + custom: () => Promise.reject(new Error("custom unavailable")), + }; + + assert.equal(await openKillConfirm(ui, workflowRun(), theme), false); +}); + +test("openKillConfirm resolves false when custom UI resolves without invoking the factory", async () => { + let confirmCalls = 0; + const ui: ConfirmUiSurface = { + custom: async () => undefined, + confirm: async () => { + confirmCalls += 1; + return true; + }, + }; + + assert.equal(await openKillConfirm(ui, workflowRun(), theme), false); + assert.equal(confirmCalls, 0); +}); diff --git a/test/unit/workflow-attach-pane-09.test.ts b/test/unit/workflow-attach-pane-09.test.ts index bc6913f98..926d47132 100644 --- a/test/unit/workflow-attach-pane-09.test.ts +++ b/test/unit/workflow-attach-pane-09.test.ts @@ -230,11 +230,10 @@ function assertNextGraphEnterAttaches( } describe("WorkflowAttachPane", () => { - test("q quit delegates to onQuit (resumable) and closes without killing", () => { + test("q requests host quit confirmation without closing speculatively", () => { const store = createStore(); setupRun(store, "run-1", [{ id: "stage-a", name: "A" }]); let quitRunId: string | undefined; - let killRunId: string | undefined; let closed = 0; const pane = new WorkflowAttachPane({ store, @@ -243,24 +242,20 @@ describe("WorkflowAttachPane", () => { onQuit: (runId) => { quitRunId = runId; }, - onKill: (runId) => { - killRunId = runId; - store.removeRun(runId); - }, onClose: () => { closed += 1; }, getViewportRows: () => 36, }); - pane.handleInput("q"); + assert.equal(pane.handleInput("q"), true); - // `q` is a resumable quit/detach, not an authoritative kill. The run is - // left in place so `/workflow resume` can restore it; only - // `/workflow kill` removes a run as non-resumable. + // `q` requests a resumable quit/detach and lets the host confirm it. + // The pane must not close speculatively before that confirmation path + // decides to quit and dismiss the overlay. assert.equal(quitRunId, "run-1"); - assert.equal(killRunId, undefined); - assert.equal(closed, 1); + assert.equal(closed, 0); + assert.equal(store.runs().find((run) => run.id === "run-1")?.status, "running"); assert.equal(pane._mode, "graph"); pane.dispose(); });