fix(phase-3): normalize workflows and fix CLI command references#399
fix(phase-3): normalize workflows and fix CLI command references#399maystudios merged 3 commits intomainfrom
Conversation
Phase 3 workflow normalization (#218): - Fix label mismatch: all-progress and init.ts queried 'maxsim:phase' instead of 'type:phase' (the label actually created by create-phase) (#388) - Fix delete-comments to support comma-separated types for multi-type deletion (e.g. --type plan,context,research) (#389) - Add --plan-number flag to post-comment, allowing it to subsume post-plan-comment functionality (#390) - Replace set-status with move-issue in all workflow files, fix set-project --number to --project-number (#391) - Implement validate-structure and config-save-defaults commands, replace ghost refs (phase complete, install write-claude-md) (#392) - Replace ~15 direct gh CLI calls across 8 workflow files with maxsim-tools.cjs wrappers (#393) - Remove set-status alias from GITHUB_COMMANDS (#394) - Wire self-improvement.md into improve.md, verification-patterns.md into verify-phase.md (#395) - Remove stale hasPhaseFiles() from statusline hook — config-only approach, no local filesystem phase detection (#396) - Add template sync module (install/sync.ts) with hash-based one-way sync from bundled templates to .claude/ (#397) - Extend workflow validation test with CLI command reference checking and workflow file reference validation (#398) https://claude.ai/code/session_01EMccZLcggSgkXGywamtcuk
Use matchAll instead of while(regex.exec) to satisfy biome lint rules. https://claude.ai/code/session_01EMccZLcggSgkXGywamtcuk
Build-time version injection updated version.ts and config.json template. https://claude.ai/code/session_01EMccZLcggSgkXGywamtcuk
|
🎉 This PR is included in version 5.23.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Pull request overview
This PR continues Phase 3 “workflow normalization” by standardizing workflow CLI invocations, removing stale/ghost commands, and adding guardrails (tests + new commands) to prevent workflows from referencing non-existent CLI commands.
Changes:
- Normalize workflow templates to use
maxsim-tools.cjswrappers (e.g., replace directghcalls; replaceset-statuswithmove-issue; fixset-projectflag name). - Extend CLI capabilities (
validate-structure,config-save-defaults,post-comment --plan-number,delete-comments --typecomma lists) and remove theset-statusalias. - Add workflow validation coverage to check workflow CLI command references and referenced workflow files; update statusline behavior to be config-only.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/workflows/verify-phase.md | Adds reference doc link; switches sub-issue listing to github list-sub-issues. |
| templates/workflows/security-audit.md | Replaces gh calls with github create-issue / github get-issue. |
| templates/workflows/new-project.md | Updates init workflow CLI calls (project flag name, milestone creation wrapper, move-issue). |
| templates/workflows/new-milestone.md | Replaces set-status with move-issue and updates constraints text. |
| templates/workflows/init-existing.md | Updates init-existing CLI calls to wrappers and new flags. |
| templates/workflows/improve.md | Adds reference doc link; replaces gh issue create with wrapper. |
| templates/workflows/health.md | Updates structure validation command to validate-structure. |
| templates/workflows/go.md | Replaces gh issue list/edit guidance with wrapper commands. |
| templates/workflows/fix-loop.md | Replaces gh issue create with wrapper. |
| templates/workflows/execute.md | Replaces gh issue access/create and “phase complete” ghost command with supported wrappers. |
| templates/workflows/debug-loop.md | Replaces gh issue create/close with wrappers. |
| templates/templates/config.json | Bumps bundled template version to 5.23.0. |
| packages/cli/tests/unit/workflow-validation.test.ts | Adds workflow CLI command reference validation + workflow file reference validation. |
| packages/cli/tests/unit/statusline.test.ts | Updates tests for config-only statusline behavior (phase files no longer affect status). |
| packages/cli/src/install/sync.ts | Adds hash-based template sync implementation (new module). |
| packages/cli/src/hooks/maxsim-statusline.ts | Removes filesystem phase detection (hasPhaseFiles()), config-only status output. |
| packages/cli/src/core/version.ts | Bumps CLI version constant to 5.23.0. |
| packages/cli/src/commands/init.ts | Fixes phase label queries from maxsim:phase to canonical type:phase. |
| packages/cli/src/commands/github.ts | Adds --plan-number support to post-comment; enhances delete-comments; removes set-status alias; fixes phase label in all-progress. |
| packages/cli/src/commands/config.ts | Adds config-save-defaults and validate-structure commands. |
| package-lock.json | Updates lockfile metadata and bumps packages/cli version entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const commentType = getFlag(args, '--type'); | ||
| const planNumber = getIntFlag(args, '--plan-number'); | ||
| if (commentType) { | ||
| const header = formatCommentHeader({ type: commentType as Parameters<typeof formatCommentHeader>[0]['type'] }); | ||
| const meta: Record<string, unknown> = { type: commentType }; | ||
| if (planNumber !== undefined && !Number.isNaN(planNumber)) { | ||
| meta.plan = planNumber; | ||
| } | ||
| const header = formatCommentHeader(meta as Parameters<typeof formatCommentHeader>[0]); | ||
| body = `${header}\n${body}`; | ||
| } |
There was a problem hiding this comment.
--plan-number is parsed even when --type is not provided, but it has no effect because the metadata header is only prepended inside if (commentType). Consider either (a) erroring when --plan-number is provided without --type, or (b) treating --plan-number as implying --type plan, to avoid silently ignoring the flag.
| } | ||
|
|
||
| // Support comma-separated types (e.g. "plan,context,research") | ||
| const types = new Set(typeRaw.split(',').map((t) => t.trim()).filter(Boolean)); |
There was a problem hiding this comment.
If --type is provided as only commas/whitespace (e.g. --type ",,"), types becomes empty and the command reports Deleted 0 [] comments... rather than treating it as invalid input. Consider validating types.size > 0 and returning an error when no valid types are parsed.
| const types = new Set(typeRaw.split(',').map((t) => t.trim()).filter(Boolean)); | |
| const types = new Set(typeRaw.split(',').map((t) => t.trim()).filter(Boolean)); | |
| if (types.size === 0) { | |
| return cmdErr('--type must contain at least one non-empty comment type'); | |
| } |
| const second = m[2]; | ||
|
|
||
| if (NAMESPACE_COMMANDS[first]) { | ||
| if (second && !NAMESPACE_COMMANDS[first].has(second)) { |
There was a problem hiding this comment.
The test allows node ... maxsim-tools.cjs github (namespace with no subcommand) to pass validation because it only errors when second is present but unknown. This means a broken workflow invocation can slip through. Consider pushing an error when first is a known namespace and second is missing/undefined.
| if (second && !NAMESPACE_COMMANDS[first].has(second)) { | |
| if (!second) { | |
| errors.push(`${file}: missing ${first} subcommand`); | |
| } else if (!NAMESPACE_COMMANDS[first].has(second)) { |
| /** Files that should never be overwritten by sync (user-modified). */ | ||
| const EXCLUDED_FILES = new Set([ | ||
| 'config.json', | ||
| ]); |
There was a problem hiding this comment.
EXCLUDED_FILES skips any file named config.json in every synced directory. This will also skip syncing .claude/maxsim/templates/config.json (the bundled template config/version file), preventing template version updates from propagating. Consider narrowing the exclusion to the actual user config path (e.g. .claude/maxsim/config.json) or using path-based exclusions rather than filename-only.
| // Compare with existing destination | ||
| if (fs.existsSync(destPath)) { | ||
| const destHash = fileHash(destPath); | ||
| const srcHash = crypto.createHash('sha256').update(srcContent).digest('hex'); | ||
| if (srcHash === destHash) { | ||
| result.unchanged.push(relativePath); | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Write the updated file | ||
| fs.mkdirSync(path.dirname(destPath), { recursive: true }); | ||
| fs.writeFileSync(destPath, srcContent); | ||
| result.copied.push(relativePath); | ||
| } |
There was a problem hiding this comment.
syncDir overwrites destination files whenever the hash differs, but the PR description mentions “conflict prompts” / avoiding overwriting user-modified files. As implemented, only config.json is excluded and all other user edits in .claude/ will be silently overwritten. Consider adding a “conflict” mode (skip + record, or prompt via caller) when dest exists and differs, instead of always writing.
| export function syncTemplates( | ||
| projectDir: string, | ||
| templateVars?: Record<string, string>, | ||
| ): SyncResult { | ||
| const templatesDir = getTemplatesDir(); | ||
| const claudeDir = path.join(projectDir, '.claude'); | ||
|
|
||
| const result: SyncResult = { copied: [], skipped: [], unchanged: [] }; | ||
|
|
||
| const syncMappings = [ | ||
| { src: path.join(templatesDir, 'commands'), dest: path.join(claudeDir, 'commands') }, | ||
| { src: path.join(templatesDir, 'agents'), dest: path.join(claudeDir, 'agents') }, | ||
| { src: path.join(templatesDir, 'skills'), dest: path.join(claudeDir, 'skills') }, | ||
| { src: path.join(templatesDir, 'rules'), dest: path.join(claudeDir, 'rules') }, | ||
| { src: path.join(templatesDir, 'workflows'), dest: path.join(claudeDir, 'maxsim', 'workflows') }, | ||
| { src: path.join(templatesDir, 'references'), dest: path.join(claudeDir, 'maxsim', 'references') }, | ||
| { src: path.join(templatesDir, 'templates'), dest: path.join(claudeDir, 'maxsim', 'templates') }, | ||
| ]; | ||
|
|
||
| for (const { src, dest } of syncMappings) { | ||
| syncDir(src, dest, templateVars, result); | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
The new syncTemplates() entry point is not referenced anywhere else in the repo (no caller/CLI command wiring). If template sync is intended to be user-invokable (per PR description/acceptance criteria), consider exposing it via an install subcommand or invoking it from the installer/update flow so it actually runs.
| Use: node .claude/maxsim/bin/maxsim-tools.cjs github create-phase --phase-number N --title '{name}' --body '{body}' --milestone-number {MILESTONE_NUMBER} --project-number {PROJECT_NUMBER} | ||
|
|
||
| Set each issue status to 'To Do' on the board using: | ||
| node .claude/maxsim/bin/maxsim-tools.cjs github set-status --issue-number {N} --status 'To Do' | ||
| node .claude/maxsim/bin/maxsim-tools.cjs github move-issue --issue-number {N} --status 'To Do' |
There was a problem hiding this comment.
This step uses --issue-number {N}, but {N} is the phase number placeholder, not the GitHub issue number returned by github create-phase. move-issue expects an issue number, so this will move the wrong issue (or fail). Capture the created issue number(s) from create-phase output and use those values for --issue-number.
| Commands: | ||
| gh issue create --title 'Phase N: {name}' --label 'phase:{N}' --milestone {MILESTONE_NUMBER} --body '{body}' --repo {owner/repo} | ||
|
|
||
| After creating all issues, add each to the GitHub Project Board: | ||
| gh project item-add {PROJECT_NUMBER} --owner {OWNER} --url {issue_url} | ||
| node .claude/maxsim/bin/maxsim-tools.cjs github create-phase --phase-number N --title '{name}' --body '{body}' --milestone-number {MILESTONE_NUMBER} --project-number {PROJECT_NUMBER} | ||
|
|
||
| Set each issue status to 'To Do': | ||
| node .claude/maxsim/bin/maxsim-tools.cjs github set-status --issue-number {N} --status 'To Do' | ||
| node .claude/maxsim/bin/maxsim-tools.cjs github move-issue --issue-number {N} --status 'To Do' | ||
|
|
There was a problem hiding this comment.
This step uses --issue-number {N}, but {N} is the phase number placeholder, not the GitHub issue number returned by github create-phase. move-issue expects an issue number, so this will move the wrong issue (or fail). Capture the created issue number(s) from create-phase output and use those values for --issue-number.
Phase 3 workflow normalization (#218):
instead of 'type:phase' (the label actually created by create-phase) (Task 1.1: Fix label mismatch (type:phase vs maxsim:phase) #388)
deletion (e.g. --type plan,context,research) (Task 1.2: Fix delete-comments comma-separated type support #389)
post-plan-comment functionality (Task 1.3: Extend post-comment with --plan-number flag #390)
set-project --number to --project-number (Task 2.1: Fix workflow flag mismatches #391)
replace ghost refs (phase complete, install write-claude-md) (Task 2.2: Implement ghost commands / remove dead references #392)
maxsim-tools.cjs wrappers (Task 2.3: Replace gh CLI bypasses with MaxsimCLI commands #393)
into verify-phase.md (Task 3.2: Wire in orphan files + normalize init.md dispatch #395)
approach, no local filesystem phase detection (Task 4.1: Implement statusline cache infrastructure #396)
one-way sync from bundled templates to .claude/ (Task 5.1: Implement template sync mechanism #397)
and workflow file reference validation (Task 5.2: Extend workflow validation test + pre-commit hook #398)
https://claude.ai/code/session_01EMccZLcggSgkXGywamtcuk