-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(view): display workflow status in active changes #807
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ import * as path from 'path'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import chalk from 'chalk'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { MarkdownParser } from './parsers/markdown-parser.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { loadChangeContext, formatChangeStatus } from './artifact-graph/index.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class ViewCommand { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async execute(targetPath: string = '.'): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -46,6 +47,11 @@ export class ViewCommand { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ` ${chalk.yellow('◉')} ${chalk.bold(change.name.padEnd(30))} ${progressBar} ${chalk.dim(`${percentage}%`)}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (change.workflowStatus) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { schema, artifacts } = change.workflowStatus; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(` ${chalk.dim(`└─ [${schema}] ${artifacts}`)}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+54
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. Pre-colored
A safer approach is to keep the prefix dim and leave the pre-formatted
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -80,17 +86,26 @@ export class ViewCommand { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private async getChangesData(openspecDir: string): Promise<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| draft: Array<{ name: string }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| active: Array<{ name: string; progress: { total: number; completed: number } }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| active: Array<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| progress: { total: number; completed: number }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflowStatus?: { schema: string; artifacts: string } | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completed: Array<{ name: string }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const changesDir = path.join(openspecDir, 'changes'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const projectRoot = path.dirname(openspecDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!fs.existsSync(changesDir)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { draft: [], active: [], completed: [] }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const draft: Array<{ name: string }> = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const active: Array<{ name: string; progress: { total: number; completed: number } }> = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const active: Array<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| progress: { total: number; completed: number }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflowStatus?: { schema: string; artifacts: string } | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }> = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const completed: Array<{ name: string }> = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const entries = fs.readdirSync(changesDir, { withFileTypes: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -107,7 +122,19 @@ export class ViewCommand { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completed.push({ name: entry.name }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Has tasks but not all complete | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| active.push({ name: entry.name, progress }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try to load workflow status | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let workflowStatus: { schema: string; artifacts: string } | null = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const context = loadChangeContext(projectRoot, entry.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const status = formatChangeStatus(context); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflowStatus = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| schema: status.schemaName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| artifacts: this.formatWorkflowArtifacts(status.artifacts), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Change doesn't have valid workflow metadata, skip workflow status | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+134
to
+136
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. Overly broad silent catch may hide unexpected errors The catch block's comment says "Change doesn't have valid workflow metadata, skip workflow status", but it silently swallows all errors — including unexpected runtime failures (e.g., a crash inside Consider logging at debug/warn level for unexpected error types:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| active.push({ name: entry.name, progress, workflowStatus }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -206,14 +233,35 @@ export class ViewCommand { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private createProgressBar(completed: number, total: number, width: number = 20): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (total === 0) return chalk.dim('─'.repeat(width)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const percentage = completed / total; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filled = Math.round(percentage * width); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const empty = width - filled; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filledBar = chalk.green('█'.repeat(filled)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const emptyBar = chalk.dim('░'.repeat(empty)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `[${filledBar}${emptyBar}]`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Formats workflow artifact status into a compact string. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Examples: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - "proposal ✓ specs → design tasks" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - "proposal ✓ specs ✓ design → tasks" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - "proposal → specs design tasks" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+248
to
+253
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. JSDoc examples don't match actual output format The docstring examples show a space before the status symbol (e.g.,
Suggested change
Comment on lines
+247
to
+253
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. Docstring examples don't match actual output format. The examples show spaces before the symbols ( 📝 Fix documentation to match implementation /**
* Formats workflow artifact status into a compact string.
* Examples:
- * - "proposal ✓ specs → design tasks"
- * - "proposal ✓ specs ✓ design → tasks"
- * - "proposal → specs design tasks"
+ * - "proposal✓ specs→ design tasks"
+ * - "proposal✓ specs✓ design→ tasks"
+ * - "proposal→ specs design tasks"
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private formatWorkflowArtifacts(artifacts: Array<{ id: string; status: 'done' | 'ready' | 'blocked' }>): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return artifacts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((artifact) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (artifact.status === 'done') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `${artifact.id}${chalk.green('✓')}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (artifact.status === 'ready') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `${artifact.id}${chalk.cyan('→')}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return artifact.id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .join(' '); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Unrelated
packageManagerfield addedThis PR is described as adding workflow status to the view command, but it also adds a
packageManagerfield committing the repository topnpm@10.30.3. This is an infrastructure-level change (it will causenpm/yarninvocations to warn or fail in some environments) and should be in a separate PR with explicit discussion. If this was intentional, a note in the PR description would help reviewers understand the intent.