From fa163007cb5a536dd4097a76613bb43d9cc21f79 Mon Sep 17 00:00:00 2001 From: Justin Thurman Date: Thu, 4 Jun 2026 11:25:25 -0400 Subject: [PATCH] Add support for mid-task UI updating --- node-src/lib/tasks.ts | 5 ++ node-src/renderer/engine/index.test.ts | 107 +++++++++++++++++++++++++ node-src/renderer/engine/index.ts | 36 ++++++++- node-src/types.ts | 36 ++++++++- 4 files changed, 177 insertions(+), 7 deletions(-) diff --git a/node-src/lib/tasks.ts b/node-src/lib/tasks.ts index 13c992caa..2774995a3 100644 --- a/node-src/lib/tasks.ts +++ b/node-src/lib/tasks.ts @@ -39,6 +39,11 @@ export const buildDeps = (ctx: Context): Deps => ({ pkg: ctx.pkg, sessionId: ctx.sessionId, packageJson: ctx.packageJson, + // No-op by default; the new engine's `runTask` overrides this with a renderer-aware reporter. + // Under Listr there is no reporter: legacy tasks mutate the Listr task object directly, and + // adapted tasks have no mid-task channel here — so a task must not adopt `deps.report` until it + // has been migrated off Listr onto `runTask`, where the override makes it live. + report: () => {}, }); async function applyAdaptedResult( diff --git a/node-src/renderer/engine/index.test.ts b/node-src/renderer/engine/index.test.ts index e82219427..c63ba09de 100644 --- a/node-src/renderer/engine/index.test.ts +++ b/node-src/renderer/engine/index.test.ts @@ -181,4 +181,111 @@ describe('runTask', () => { const fail = calls.find((c) => c.hook === 'fail'); expect(fail?.state.title).toBe('Authentication failed'); }); + + describe('deps.report (mid-task updates)', () => { + it('routes a textual update to renderer.update without calling onTaskProgress', async () => { + const onTaskProgress = vi.fn(); + const ctx = fakeContext({ options: { experimental_onTaskProgress: onTaskProgress } as any }); + const { renderer, calls } = recordingRenderer(); + + const config: TaskConfig = { + name: 'gitInfo', + title: 'Retrieving git information', + transitions, + extractInput: () => ({}), + run: async (deps) => { + deps.report({ title: 'Skipping build', output: 'no changes since last build' }); + return { kind: 'continue', output: {} }; + }, + }; + + await runTask(ctx, config, renderer); + + const update = calls.find((c) => c.hook === 'update'); + expect(update?.state.title).toBe('Skipping build'); + expect(update?.state.output).toBe('no changes since last build'); + expect(update?.state.progress).toBeUndefined(); + expect(onTaskProgress).not.toHaveBeenCalled(); + }); + + it('fans a numeric update out to both renderer.update and the public onTaskProgress hook', async () => { + const onTaskProgress = vi.fn(); + const ctx = fakeContext({ options: { experimental_onTaskProgress: onTaskProgress } as any }); + const { renderer, calls } = recordingRenderer(); + + const config: TaskConfig = { + name: 'upload', + title: 'Uploading', + transitions, + extractInput: () => ({}), + run: async (deps) => { + deps.report({ + output: 'uploading', + progress: { progress: 512, total: 1024, unit: 'bytes' }, + }); + return { kind: 'continue', output: {} }; + }, + }; + + await runTask(ctx, config, renderer); + + const update = calls.find((c) => c.hook === 'update'); + expect(update?.state.progress).toEqual({ progress: 512, total: 1024, unit: 'bytes' }); + // No textual report preceded this one, so the update inherits the task's starting title. + expect(update?.state.title).toBe('Uploading'); + + expect(onTaskProgress).toHaveBeenCalledExactlyOnceWith(expect.anything(), { + progress: 512, + total: 1024, + unit: 'bytes', + }); + }); + + it('passes a numeric-only report through with no output', async () => { + const ctx = fakeContext({ options: {} as any }); + const { renderer, calls } = recordingRenderer(); + + const config: TaskConfig = { + name: 'upload', + title: 'Uploading', + transitions, + extractInput: () => ({}), + run: async (deps) => { + deps.report({ progress: { progress: 512, total: 1024, unit: 'bytes' } }); + return { kind: 'continue', output: {} }; + }, + }; + + await runTask(ctx, config, renderer); + + const update = calls.find((c) => c.hook === 'update'); + expect(update?.state.output).toBeUndefined(); + expect(update?.state.progress).toEqual({ progress: 512, total: 1024, unit: 'bytes' }); + }); + + it('makes a textual update sticky: a later title-less report inherits the current title', async () => { + const ctx = fakeContext({ options: {} as any }); + const { renderer, calls } = recordingRenderer(); + + const config: TaskConfig = { + name: 'upload', + title: 'Uploading', + transitions, + extractInput: () => ({}), + run: async (deps) => { + deps.report({ title: 'Finalizing' }); + deps.report({ progress: { progress: 512, total: 1024, unit: 'bytes' } }); + return { kind: 'continue', output: {} }; + }, + }; + + await runTask(ctx, config, renderer); + + const updates = calls.filter((c) => c.hook === 'update'); + expect(updates[0].state.title).toBe('Finalizing'); + // The numeric-only report carries no title of its own, so it renders with the title set by + // the preceding textual report. + expect(updates[1].state.title).toBe('Finalizing'); + }); + }); }); diff --git a/node-src/renderer/engine/index.ts b/node-src/renderer/engine/index.ts index 094f6929e..cdbf7d7e1 100644 --- a/node-src/renderer/engine/index.ts +++ b/node-src/renderer/engine/index.ts @@ -3,7 +3,7 @@ // alone for now to avoid a noisy diff. import { buildDeps } from '@cli/tasks'; -import { Context, Deps, Task, TaskFunction, TaskName, TaskResult } from '../../types'; +import { Context, Deps, Task, TaskFunction, TaskName, TaskReporter, TaskResult } from '../../types'; /** * A `TaskRenderer` consumes the `Task` state objects emitted by a task's @@ -15,7 +15,10 @@ import { Context, Deps, Task, TaskFunction, TaskName, TaskResult } from '../../t export interface TaskRenderer { /** Task has begun. `state` is the `pending` transition. */ start: (state: Task) => void; - /** Progress update while the task body runs. `state.output` contains the message. */ + /** + * Progress update while the task body runs, pushed via `deps.report`. `state.output` carries the + * message; `state.progress` carries optional numeric progress for renderers that draw a bar. + */ update: (state: Task) => void; /** Task finished successfully. `state` is the `success` or `partial` transition. */ succeed: (state: Task) => void; @@ -70,7 +73,8 @@ export async function runTask( try { const input = config.extractInput(ctx); - const result = await config.run(buildDeps(ctx), input); + const deps = { ...buildDeps(ctx), report: makeReporter(ctx, renderer) }; + const result = await config.run(deps, input); await applyResult(ctx, config, result, renderer); } catch (error) { renderer.fail(failureState(ctx, config, error as Error, pending.title)); @@ -80,6 +84,32 @@ export async function runTask( ctx.options.experimental_onTaskComplete?.({ ...ctx }); } +/** + * Build the reporter injected as `deps.report`, used to enable mid-task UI updates through the + * renderer's `update` hook. These updates are also fanned out to `experimental_onTaskProgress` + * (this callback only accepts numeric progress, not textual updates). + * + * @param ctx The CLI context (its `title` is updated when a textual update arrives). + * @param renderer The active renderer for the running task. + * + * @returns A `TaskReporter` closed over the current task's renderer. + */ +function makeReporter(ctx: Context, renderer: TaskRenderer): TaskReporter { + return (update) => { + if (update.title !== undefined) ctx.title = update.title; + + renderer.update({ + title: ctx.title, + output: update.output, + progress: update.progress, + }); + + if (update.progress) { + ctx.options.experimental_onTaskProgress?.({ ...ctx }, update.progress); + } + }; +} + /** * Maps task results to context mutations and UI state objects, as well as calling the underlying renderer to * drive the UI. diff --git a/node-src/types.ts b/node-src/types.ts index 5a09522c6..5ea17487d 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -129,10 +129,7 @@ export interface Options extends Configuration { ) => void; /** A callback that is called during tasks that have incremental progress */ - experimental_onTaskProgress?: ( - ctx: Context, - status: { progress: number; total: number; unit: string } - ) => void; + experimental_onTaskProgress?: (ctx: Context, status: TaskProgress) => void; /** A callback that is called at the completion of each task */ experimental_onTaskComplete?: (ctx: Context) => void; @@ -291,6 +288,12 @@ export interface Deps { pkg: Context['pkg']; sessionId: string; packageJson: Record; + /** + * Push a mid-task UI update from inside the task body. The base `buildDeps` provides a no-op; + * `runTask` overrides it with a renderer-aware reporter that fans out to the active renderer and + * the public `experimental_onTaskProgress` hook. + */ + report: TaskReporter; } export type TaskResult = @@ -462,8 +465,33 @@ export interface Task { status?: string; title: string; output?: string; + /** + * Optional numeric progress. Renderers that can draw a progress bar (e.g. the Clack progress-bar + * renderer) consume this; renderers that can't simply ignore it and render the text. + */ + progress?: TaskProgress; +} + +export interface TaskProgress { + progress: number; + total: number; + unit: string; } +/** + * A mid-task update pushed from inside a task body via `deps.report`. This is the channel for any + * UI change that happens *during* the business logic (a textual phase change like gitInfo's + * "skipping build", or numeric progress like upload bytes / snapshot counts) — as opposed to the + * `start`/`succeed`/`fail` brackets the engine drives on its own. + */ +export interface TaskUpdate { + title?: string; + output?: string; + progress?: TaskProgress; +} + +export type TaskReporter = (update: TaskUpdate) => void; + export interface Reason { moduleName: string; }