-
Notifications
You must be signed in to change notification settings - Fork 9
feat: enrich install completion summary and stream agent file ops #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||
| import type { InstallerAdapter, AdapterConfig } from './types.js'; | ||||||||||||||||||||
| import type { InstallerEventEmitter, InstallerEvents } from '../events.js'; | ||||||||||||||||||||
| import { relative } from 'node:path'; | ||||||||||||||||||||
| import clack from '../../utils/clack.js'; | ||||||||||||||||||||
| import chalk from 'chalk'; | ||||||||||||||||||||
| import { getConfig } from '../settings.js'; | ||||||||||||||||||||
|
|
@@ -35,9 +36,14 @@ export class CLIAdapter implements InstallerAdapter { | |||||||||||||||||||
| // SIGINT handler for cleanup | ||||||||||||||||||||
| private sigIntHandler: (() => void) | null = null; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Long-running agent update interval | ||||||||||||||||||||
| // Long-running agent update interval (kept as a no-op field; never started) | ||||||||||||||||||||
| private agentUpdateInterval: NodeJS.Timeout | null = null; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Last phase message shown on the agent spinner, restored after logging above it. | ||||||||||||||||||||
| private lastAgentMessage = 'Running AI agent...'; | ||||||||||||||||||||
| // Last file path rendered as a step line, to dedupe consecutive same-path ops. | ||||||||||||||||||||
| private lastFileOp: string | null = null; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| constructor(config: AdapterConfig) { | ||||||||||||||||||||
| this.emitter = config.emitter; | ||||||||||||||||||||
| this.sendEvent = config.sendEvent; | ||||||||||||||||||||
|
|
@@ -112,6 +118,10 @@ export class CLIAdapter implements InstallerAdapter { | |||||||||||||||||||
| this.subscribe('config:complete', this.handleConfigComplete); | ||||||||||||||||||||
| this.subscribe('agent:start', this.handleAgentStart); | ||||||||||||||||||||
| this.subscribe('agent:progress', this.handleAgentProgress); | ||||||||||||||||||||
| // Persistent, append-only log of file operations + tool calls above the spinner. | ||||||||||||||||||||
| this.subscribe('file:write', this.handleFileWrite); | ||||||||||||||||||||
| this.subscribe('file:edit', this.handleFileEdit); | ||||||||||||||||||||
| this.subscribe('agent:tool', this.handleAgentTool); | ||||||||||||||||||||
| this.subscribe('validation:start', this.handleValidationStart); | ||||||||||||||||||||
| this.subscribe('validation:issues', this.handleValidationIssues); | ||||||||||||||||||||
| this.subscribe('validation:complete', this.handleValidationComplete); | ||||||||||||||||||||
|
|
@@ -359,22 +369,52 @@ export class CLIAdapter implements InstallerAdapter { | |||||||||||||||||||
|
|
||||||||||||||||||||
| private handleAgentStart = (): void => { | ||||||||||||||||||||
| this.spinner = clack.spinner(); | ||||||||||||||||||||
| this.spinner.start('Running AI agent...'); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Periodic status updates for long-running operations | ||||||||||||||||||||
| let dots = 0; | ||||||||||||||||||||
| this.agentUpdateInterval = setInterval(() => { | ||||||||||||||||||||
| dots = (dots + 1) % 4; | ||||||||||||||||||||
| const dotStr = '.'.repeat(dots + 1); | ||||||||||||||||||||
| this.spinner?.message(`Running AI agent${dotStr}`); | ||||||||||||||||||||
| }, 2000); | ||||||||||||||||||||
| this.spinner.start(this.lastAgentMessage); | ||||||||||||||||||||
| // No setInterval: clack animates its own frames, and the old 2s reset | ||||||||||||||||||||
| // clobbered the current phase text set by handleAgentProgress. | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleAgentProgress = ({ step, detail }: InstallerEvents['agent:progress']): void => { | ||||||||||||||||||||
| const message = detail ? `${step}: ${detail}` : step; | ||||||||||||||||||||
| this.lastAgentMessage = message; | ||||||||||||||||||||
| this.spinner?.message(message); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Render a persistent line above the running spinner: stop the spinner to | ||||||||||||||||||||
| * finalize its line, emit the log, then restart it on the last phase message. | ||||||||||||||||||||
| * Mirrors the existing stop→log and stop→start-new-spinner precedents. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| private logAboveSpinner(render: () => void): void { | ||||||||||||||||||||
| const wasRunning = this.spinner !== null; | ||||||||||||||||||||
| this.spinner?.stop(); | ||||||||||||||||||||
| this.spinner = null; | ||||||||||||||||||||
| render(); | ||||||||||||||||||||
| if (wasRunning) { | ||||||||||||||||||||
| this.spinner = clack.spinner(); | ||||||||||||||||||||
| this.spinner.start(this.lastAgentMessage); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleFileWrite = ({ path }: InstallerEvents['file:write']): void => { | ||||||||||||||||||||
| if (path === this.lastFileOp) return; // dedupe consecutive same-path ops | ||||||||||||||||||||
| this.lastFileOp = path; | ||||||||||||||||||||
| const rel = relative(process.cwd(), path); | ||||||||||||||||||||
| this.logAboveSpinner(() => clack.log.step(`Creating ${chalk.dim(rel)}`)); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleFileEdit = ({ path }: InstallerEvents['file:edit']): void => { | ||||||||||||||||||||
| if (path === this.lastFileOp) return; | ||||||||||||||||||||
| this.lastFileOp = path; | ||||||||||||||||||||
| const rel = relative(process.cwd(), path); | ||||||||||||||||||||
| this.logAboveSpinner(() => clack.log.step(`Editing ${chalk.dim(rel)}`)); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
Comment on lines
+399
to
+411
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Cross-type file dedup: a write followed by an edit to the same path is suppressed The Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleAgentTool = ({ detail }: InstallerEvents['agent:tool']): void => { | ||||||||||||||||||||
| const cmd = detail.length > 80 ? `${detail.slice(0, 77)}…` : detail; | ||||||||||||||||||||
| this.logAboveSpinner(() => clack.log.step(`Running ${chalk.dim(cmd)}`)); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
Comment on lines
+413
to
+416
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 File operation notifications are silently suppressed when a tool call intervenes between two operations on the same file The deduplication of file-operation notifications does not reset when a tool-call event fires ( Impact: Users miss that a file was edited after an intervening command, making the install log incomplete. Mechanism: shared lastFileOp is never cleared by tool-call handlers
The comment at line 400 says "dedupe consecutive same-path ops", but the tool call breaks consecutiveness without resetting the dedup state.
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleValidationStart = (): void => { | ||||||||||||||||||||
| this.stopAgentUpdates(); | ||||||||||||||||||||
| this.stopSpinner('Agent completed'); | ||||||||||||||||||||
|
|
@@ -401,12 +441,12 @@ export class CLIAdapter implements InstallerAdapter { | |||||||||||||||||||
| } | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private handleComplete = ({ success, summary }: InstallerEvents['complete']): void => { | ||||||||||||||||||||
| private handleComplete = ({ success, summary, completion }: InstallerEvents['complete']): void => { | ||||||||||||||||||||
| this.stopAgentUpdates(); | ||||||||||||||||||||
| this.stopSpinner(success ? 'Done' : 'Failed'); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| console.log(''); | ||||||||||||||||||||
| console.log(renderCompletionSummary(success, summary)); | ||||||||||||||||||||
| console.log(renderCompletionSummary(success, summary, completion)); | ||||||||||||||||||||
| console.log(''); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // When we scaffolded a fresh app, the install ran in the current dir, so | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Spinner stopped without a message — inconsistent with all other call sites
Every other call to
spinner.stop()in the CLI adapter passes an explicit message string (e.g.stopSpinner('Agent completed'),this.spinner.stop('Cancelled'),this.spinner.stop('Authenticated')). The newlogAboveSpinnermethod atsrc/lib/adapters/cli-adapter.ts:390callsthis.spinner?.stop()without a message. Depending on clack's internal behavior, this may print the spinner's last message as a "completed" line (with a checkmark), creating a misleading visual artifact where the current phase (e.g. "Configuring middleware") appears completed before the spinner immediately restarts with the same message. The test mocksstopas a no-op so this visual behavior is untested. Consider passing an empty string or a neutral message tostop()to control the output.Was this helpful? React with 👍 or 👎 to provide feedback.