Skip to content

Conversation

@janhesters
Copy link
Collaborator

@janhesters janhesters commented Oct 15, 2025

We might want to put this as /task instead.

I think even better would be to separate the slash commands. One for the /task part of the task creator, and one for the /execute command. WDYT?


Note

Moves the Task Creator guide from ai/rules/task-creator.mdc into ai/commands/execute.md, expanding it and deleting the original rules file.

  • Commands:
    • ai/commands/execute.md: Replaces minimal execute doc with a full Task Creator guide, including Context Gathering, Requirements Analysis, Agent Orchestration, Task Planning, Execution Protocol, Task Plan Template, Completed Epic docs, Constraints, createTask pipeline, and Commands (/help, /task, /list).
  • Rules:
    • Removes ai/rules/task-creator.mdc (content migrated).

Written by Cursor Bugbot for commit d6ac04c. This will update automatically on new commits. Configure here.

@janhesters janhesters self-assigned this Oct 15, 2025
@ericelliott
Copy link
Collaborator

@cursor 🙏 review

@cursor
Copy link

cursor bot commented Oct 17, 2025

🔬 Code Review

I've completed a thorough review of the PR "refactor(commands): migrate execute sudolang program to slash command". Here are my findings:

Summary of Changes

  • Moved: ai/rules/task-creator.mdcai/commands/execute.md
  • Removed: YAML frontmatter metadata (description and alwaysApply fields)
  • Changed: File extension from .mdc to .md

🚨 Critical Issues

1. Misalignment Between Command Structure and Implementation

The PR creates a structural inconsistency:

  • please.mdc defines TWO commands (lines 36-37):

    • ✅ /task - plan and execute a task epic
    • ⚙️ /execute - execute a task epic (execution only)
  • Current file structure:

    • ai/commands/task.md - stub that says "use the task creator to plan and execute"
    • ai/commands/execute.md - contains the full Task Creator implementation (both planning AND execution)
  • Problem: execute.md contains the complete planning + execution workflow, but it should only contain execution logic per its command definition. The Commands section within execute.md (line 135) lists /task as a command, creating circular confusion.

📋 Recommendations

Option A: Merge into Single /task Command (Simpler)

  1. Rename execute.mdtask.md (overwrite the stub)
  2. Remove /execute command from please.mdc
  3. Keep the full Task Creator content in task.md
  4. This aligns with the PR author's suggestion: "We might want to put this as /task instead"

Option B: Separate Planning and Execution (More Granular)

  1. Split execute.md into two files:
    • task.md - contains planning workflow (createPlan() function)
    • execute.md - contains execution workflow (executePlan() function)
  2. Update the Commands sections in each file accordingly
  3. This implements the PR author's alternate suggestion: "separate the slash commands. One for the /task part...and one for the /execute command"

My Recommendation: Option A is preferable because:

  • Simpler mental model for users
  • The workflow already requires user approval between planning and execution
  • Having separate commands adds unnecessary friction
  • The createTask() function shows they're meant to work together: createPlan |> awaitAproval |> executePlan

✅ What's Working Well

  1. Content preservation: All functionality from task-creator.mdc is preserved
  2. No orphaned references: No files reference the old task-creator.mdc path
  3. Clean migration: The content itself is unchanged and follows existing patterns
  4. Constraints section: Properly maintained with clear guidelines

🎯 Action Items

Required:

  1. Resolve command structure - Choose Option A or B above and implement it
  2. Update file location - Ensure command files align with their definitions in please.mdc
  3. Fix Commands section - Update line 135 in the file to reflect the correct command name

Nice to have:
4. Consider adding the YAML frontmatter back for consistency with other .md command files (if they use it)
5. Verify references to @agent-orchestrator.mdc (line 31) are still valid since this file has moved

Conclusion

The refactoring direction is sound, but the execution is incomplete. The command structure needs clarification before merging. I recommend Option A (merge into /task) as it simplifies the user experience while maintaining all functionality. The PR author's instinct that "We might want to put this as /task instead" is correct.

Would you like me to implement Option A or Option B to complete this refactoring?


Open in Cursor Open in Web

Learn more about Cursor Agents

@ericelliott ericelliott requested a review from Copilot October 17, 2025 00:05
Copy link
Contributor

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 refactors the task execution workflow by migrating content from a rule file to a slash command. The content from task-creator.mdc has been moved to ai/commands/execute.md, transforming it from an always-applicable rule into an explicit command that users invoke.

Key Changes:

  • Removed the entire task-creator.mdc rule file
  • Replaced minimal command documentation in execute.md with full task creator implementation
  • Changed from automatic rule application to explicit command invocation

Reviewed Changes

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

File Description
ai/rules/task-creator.mdc Completely removed the task creator rule file and all its content
ai/commands/execute.md Replaced brief command description with full task creator implementation from the deleted rule file

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Commands {
/help
/task - create a task/epic
/list [(tasks|epics) = tasks]- list all tasks in the epic
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

Missing space before the dash in command description. Should be '/list [(tasks|epics) = tasks] - list all tasks in the epic'.

Copilot uses AI. Check for mistakes.
@janhesters
Copy link
Collaborator Author

🧩 Refactor Request: Split Task Creator into Two Slash Commands

Hey @cursor 👋

Let’s implement Option B (separate planning and execution) from your review summary.

✅ Goal

Split the Sudolang Task Creator program into two slash commands:

  1. /task → for planning (createPlan workflow)
  2. /execute → for execution (executePlan workflow)

🧱 Required Changes

1. File Structure

  • Move planning logic (createPlan, planTask, context gathering, etc.) into
    ai/commands/task.md
  • Move execution logic (executePlan, validation, reporting, etc.) into
    ai/commands/execute.md

2. Command Declarations

  • Update the Commands section at the bottom of each file:

    • task.md: only list /task
    • execute.md: only list /execute

3. Update please.mdc

  • Ensure both commands are defined consistently:

    /task - plan a task or epic
    /execute - execute a previously planned task or epic

4. Code Adjustments

  • Remove circular references between the two files (e.g., /execute listed inside task.md)

  • Make sure createTask() correctly chains:

    createPlan |> awaitApproval |> executePlan
    

    where /task handles planning and /execute handles execution.

🧭 Acceptance Criteria

  • /task runs full planning flow only (no execution)
  • /execute runs execution flow only
  • please.mdc reflects both commands cleanly
  • No broken references or duplicated logic between files

Commands {
/help
/task - create a task/epic
/list [(tasks|epics) = tasks]- list all tasks in the epic
Copy link

Choose a reason for hiding this comment

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

Bug: Command Naming Conflicts with File Purpose

The execute.md file defines the /task command for creating tasks. This conflicts with the file's name and the PR's goal of separating task planning from task execution.

Fix in Cursor Fix in Web

@ericelliott
Copy link
Collaborator

I'm good with separating /task and /execute - in fact, /execute feels like the wrong word. maybe /run [task] ?

@janhesters janhesters force-pushed the 2025-10-15-refactor-execute-command branch from d6ac04c to a06b885 Compare November 3, 2025 10:28
@janhesters
Copy link
Collaborator Author

@ericelliott

I'm good with separating /task and /execute - in fact, /execute feels like the wrong word. maybe /run [task] ?

Done 👍

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.

3 participants