Skip to content

feat(execution): wire Execute→Review→Simplify→Review pipeline into executor workflow#20

Open
maystudios wants to merge 1 commit intomainfrom
worktree-agent-a9428419
Open

feat(execution): wire Execute→Review→Simplify→Review pipeline into executor workflow#20
maystudios wants to merge 1 commit intomainfrom
worktree-agent-a9428419

Conversation

@maystudios
Copy link
Owner

Summary

  • Add post-task simplification pass (duplication, dead code, complexity checks) to execute-plan workflow and executor agent
  • Add post-wave 2-stage code review gate (spec compliance + code quality) with retry limits
  • Update executor agent flow to Execute → Simplify → Verify → Commit cycle with progress tracking table (EXEC-02) and task-based context loading (EXEC-03)
  • Add skill_paths to ExecutePhaseContext in init.ts so orchestrators know where to find skills
  • Update skill path references from .agents/skills/ to ~/.claude/skills/ across executor and orchestrator
  • Add wave review aggregation and progress tables to execute-phase orchestrator

Test plan

  • Unit tests pass (npm test — 54 tests, 2 suites)
  • Build succeeds (npm run build)
  • Lint passes (biome check)
  • Pack test confirms dist contents

🤖 Generated with Claude Code

…ecutor workflow

- Add post-task simplification pass to execute-plan workflow (duplication, dead code, complexity)
- Add post-wave 2-stage code review gate (spec compliance + code quality)
- Update executor agent with Execute→Simplify→Verify→Commit cycle
- Add orchestrator status tracking table (EXEC-02) with stage progression
- Add task-based context loading instructions (EXEC-03)
- Update skill paths from .agents/skills/ to ~/.claude/skills/
- Add skill_paths to ExecutePhaseContext in init.ts
- Add progress table and wave review aggregation to execute-phase orchestrator

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 2, 2026 04:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the execute-phase / executor workflow templates to add a post-task simplification step and a post-wave two-stage review gate, while also migrating skill directory references to ~/.claude/skills/ and exposing skill path info via the CLI init context.

Changes:

  • Add a per-task “simplify” pass (duplication/dead code/complexity) before committing, with re-verification.
  • Add a post-wave, two-stage review gate (spec compliance + code quality) and progress/review tables in execution reporting.
  • Update skill directory references and add skill_paths to ExecutePhaseContext output from cmdInitExecutePhase.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
templates/workflows/execute-plan.md Adds simplify pass and a post-wave code review gate to the execute-plan workflow template.
templates/workflows/execute-phase.md Updates orchestrator wave flow to include a wave review step, progress tables, and skill path reference changes.
templates/agents/maxsim-executor.md Updates executor guidance for task-scoped context loading, Execute→Simplify→Verify→Commit cycle, progress tracking, and skill path references.
packages/cli/src/core/init.ts Extends the execute-phase init context with a skill path field.
Comments suppressed due to low confidence (1)

templates/agents/maxsim-executor.md:33

  • This section calls them “Project skills” but instructs checking ~/.claude/skills/ first and only then .claude/skills/ as a fallback. That ordering conflicts with later text about project skills overriding built-in/global skills. Clarify precedence (typically project-local overrides global) and align the check order + wording accordingly.
**Project skills:** Check `~/.claude/skills/` directory if it exists (also check `.claude/skills/` in the project root as a fallback):
1. List available skills (subdirectories)
2. Read `SKILL.md` for each skill (lightweight index ~130 lines)
3. Load specific `rules/*.md` files as needed during implementation
4. Do NOT load full `AGENTS.md` files (100KB+ context cost)
5. Follow skill rules relevant to your current task

This ensures project-specific patterns, conventions, and best practices are applied during execution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +184 to +185
# Get all files changed in this wave
WAVE_FIRST_COMMIT=$(git log --oneline --all --grep="{phase}-{first_plan_in_wave}" --reverse | head -1 | cut -d' ' -f1)
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The suggested command uses --grep="{phase}-{first_plan_in_wave}", but first_plan_in_wave isn’t defined anywhere else in this workflow template (unlike {phase}-{plan} above). As written, this is likely to be copy-pasted and fail or produce an empty commit hash. Consider computing the first plan ID for the wave explicitly (or using the collected plan IDs/commit hashes from the wave) and reference that variable here.

Suggested change
# Get all files changed in this wave
WAVE_FIRST_COMMIT=$(git log --oneline --all --grep="{phase}-{first_plan_in_wave}" --reverse | head -1 | cut -d' ' -f1)
# Get all files changed in this wave.
# Assumes $WAVE_FIRST_COMMIT was recorded when starting the wave (the first commit for this wave).

Copilot uses AI. Check for mistakes.
- .planning/config.json (Config, if exists)
- ./CLAUDE.md (Project instructions, if exists — follow project-specific guidelines and coding conventions)
- .agents/skills/ (Project skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)
- ~/.claude/skills/ (Skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

<files_to_read> now lists only ~/.claude/skills/, but the executor template also mentions a project-local .claude/skills/ fallback/override. If project-local skills are supported, the orchestrator’s startup context list should include them as well so executors discover the correct skill set consistently.

Suggested change
- ~/.claude/skills/ (Skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)
- ./.claude/skills/ (Project-local skills, if exists — override or extend global skills; list skills and read SKILL.md for each)
- ~/.claude/skills/ (Global skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +90
1. Call `skill-context` or read the plan to get the task's file list
2. Use the `Read` tool to load only those specific files
3. If the task has no `Files:` field, load files referenced in the task description
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

task_context_loading instructs “Call skill-context” to get a task’s file list, but there’s no other reference in the repo to a skill-context command/tool. If this is meant to be a MAXSIM tool, it should be named consistently with other documented commands (e.g., maxsim-tools.cjs ...) or replaced with explicit instructions to parse the Files: field from the plan.

Suggested change
1. Call `skill-context` or read the plan to get the task's file list
2. Use the `Read` tool to load only those specific files
3. If the task has no `Files:` field, load files referenced in the task description
1. Read the plan (e.g., `PLAN.md`) and parse the current task's `Files:` field to get the task's file list
2. Use the `Read` tool to load only those specific files
3. If the task has no `Files:` field, load files explicitly referenced in the task description

Copilot uses AI. Check for mistakes.
state_path: string;
roadmap_path: string;
config_path: string;
skill_paths: string;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

ExecutePhaseContext is exported from the published CLI package; adding a new required field (skill_paths) is a breaking TypeScript change for any external consumers that construct this type. Consider making it optional (or adding a new optional field) to preserve compatibility, especially since it’s not yet used elsewhere in the codebase.

Suggested change
skill_paths: string;
skill_paths?: string;

Copilot uses AI. Check for mistakes.
Comment on lines 397 to 438
@@ -431,6 +434,7 @@ export function cmdInitExecutePhase(cwd: string, phase: string | undefined, raw:
state_path: '.planning/STATE.md',
roadmap_path: '.planning/ROADMAP.md',
config_path: '.planning/config.json',
skill_paths: skillPaths,
};
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

skill_paths is set to a single global directory ($HOME/.claude/skills). The templates also mention a project-local .claude/skills fallback/override and the field name is plural, suggesting multiple lookup locations. Consider outputting an ordered list of skill search paths (project-local first, then home) or renaming the field to skill_path if only one location is intended.

Copilot uses AI. Check for mistakes.
Comment on lines 152 to 156
- .planning/STATE.md (State)
- .planning/config.json (Config, if exists)
- ./CLAUDE.md (Project instructions, if exists — follow project-specific guidelines and coding conventions)
- .agents/skills/ (Project skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)
- ~/.claude/skills/ (Skills, if exists — list skills, read SKILL.md for each, follow relevant rules during implementation)
</files_to_read>
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

PR description says skill path references were updated “across executor and orchestrator”, but there are still multiple templates referencing .agents/skills/ (e.g., templates/workflows/plan-phase.md, templates/workflows/quick.md, and several agent templates like maxsim-planner.md). If the skills directory move is intended repo-wide, those remaining references will keep pointing at the old location and create inconsistent behavior/documentation.

Copilot uses AI. Check for mistakes.
Comment on lines +300 to +301
WAVE_FIRST_COMMIT=$(echo "${TASK_COMMITS[0]}" | awk '{print $NF}')
git diff ${WAVE_FIRST_COMMIT}^..HEAD --name-only
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

<wave_code_review> says it reviews only the current wave, but WAVE_FIRST_COMMIT is derived from TASK_COMMITS[0]. Since TASK_COMMITS is only ever appended to in this template and never reset per wave, this diff range will start from the very first task of the entire run, not the wave. Consider resetting the commit list at wave start or tracking a per-wave first commit hash explicitly so the review diff is scoped correctly.

Suggested change
WAVE_FIRST_COMMIT=$(echo "${TASK_COMMITS[0]}" | awk '{print $NF}')
git diff ${WAVE_FIRST_COMMIT}^..HEAD --name-only
# WAVE_FIRST_COMMIT must be set to the first commit hash of this wave at wave start, e.g.:
# WAVE_FIRST_COMMIT=$(git rev-parse --short HEAD)
git diff "${WAVE_FIRST_COMMIT}^"..HEAD --name-only

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants