Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,6 @@
"posthog-node": "^5.20.0",
"yaml": "^2.8.2",
"zod": "^4.0.17"
}
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated packageManager field added

This PR is described as adding workflow status to the view command, but it also adds a packageManager field committing the repository to pnpm@10.30.3. This is an infrastructure-level change (it will cause npm / yarn invocations 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.

"packageManager": "pnpm@10.30.3+sha512.c961d1e0a2d8e354ecaa5166b822516668b7f44cb5bd95122d590dd81922f606f5473b6d23ec4a5be05e7fcd18e8488d47d978bbe981872f1145d06e9a740017"
}
60 changes: 54 additions & 6 deletions src/core/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-colored artifacts string embedded inside chalk.dim()

this.formatWorkflowArtifacts(...) returns a string that already contains ANSI color escape sequences (from chalk.green('✓') and chalk.cyan('→')). These pre-built sequences are then embedded inside a new chalk.dim(...) call. While chalk generally handles nesting of its own calls correctly, here the artifacts string is a pre-rendered string with raw escape codes — not a chalk builder chain — so chalk's nesting/reset logic doesn't apply. The dim styling may be interrupted after each color-reset sequence (\u001B[39m).

A safer approach is to keep the prefix dim and leave the pre-formatted artifacts string outside of chalk.dim:

Suggested change
if (change.workflowStatus) {
const { schema, artifacts } = change.workflowStatus;
console.log(` ${chalk.dim(`└─ [${schema}] ${artifacts}`)}`);
}
console.log(` ${chalk.dim(`└─ [${schema}]`)} ${artifacts}`);

});
}

Expand Down Expand Up @@ -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 });
Expand All @@ -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
Copy link

Choose a reason for hiding this comment

The 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 ArtifactGraph.fromSchema, an I/O error from detectCompleted, or a SchemaLoadError due to a misconfigured schema that the user should know about). A developer debugging a failing workflow status display would get no feedback at all.

Consider logging at debug/warn level for unexpected error types:

Suggested change
} catch (e) {
// Change doesn't have valid workflow metadata, skip workflow status
}
} catch (e) {
// Change doesn't have valid workflow metadata, skip workflow status
if (e instanceof Error && !(e.message.includes('not found') || e.message.includes('metadata'))) {
// Unexpected error — log it so it doesn't disappear silently
console.warn(chalk.dim(` [warn] Could not load workflow status for ${entry.name}: ${e.message}`));
}
}

active.push({ name: entry.name, progress, workflowStatus });
}
}
}
Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The 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., proposal ✓ specs → design tasks), but the code produces proposal✓ specs→ design tasks — the symbol is appended directly to the artifact id with no preceding space, and the space only appears between entries via .join(' ').

Suggested change
* Formats workflow artifact status into a compact string.
* Examples:
* - "proposal ✓ specs → design tasks"
* - "proposal ✓ specs ✓ design → tasks"
* - "proposal → specs design tasks"
*/
/**
* Formats workflow artifact status into a compact string.
* Examples:
* - "proposal✓ specs→ design tasks"
* - "proposal✓ specs✓ design→ tasks"
* - "proposal→ specs design tasks"
*/

Comment on lines +247 to +253
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Docstring examples don't match actual output format.

The examples show spaces before the symbols (proposal ✓), but the implementation places symbols immediately after the artifact id (proposal✓). The actual output would be "proposal✓ specs→ design tasks".

📝 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Formats workflow artifact status into a compact string.
* Examples:
* - "proposal ✓ specs → design tasks"
* - "proposal ✓ specs ✓ design → tasks"
* - "proposal → specs design tasks"
*/
/**
* Formats workflow artifact status into a compact string.
* Examples:
* - "proposal✓ specs→ design tasks"
* - "proposal✓ specs✓ design→ tasks"
* - "proposal→ specs design tasks"
*/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/view.ts` around lines 247 - 253, The docstring examples in
src/core/view.ts are inconsistent with the implementation: update the examples
to match the actual output format where symbols are attached directly to
artifact ids (e.g., "proposal✓ specs→ design tasks" rather than "proposal ✓
specs → design tasks"); locate the formatter (e.g., the function that renders
workflow artifact status, such as formatWorkflowArtifactStatus) and adjust the
three example lines in the comment block to show the real output
spacing/placement of symbols so the docs reflect the implementation.

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(' ');
}
}