Refactor command dispatch from monolithic match to modular strategy pattern

[aboimpinto]

Refactor command dispatch and command implementation ownership from monolithic files into focused command modules.

Problem

The command area had grown into a mix of central dispatch, large implementation files, and a commands::shared tree that was not truly shared. Several public methods were kept alive only because tests referenced them, not because any command handler or production path called them. That made it hard to see which command owned which behavior and made dead code look like reusable API.

Current work plan

• Part 1: Refactor command groups, remove false commands::shared ownership, move real non-command shared behavior outside commands, and localize command implementations. Draft PR: Refactor TUI command groups into focused implementations #2851
• Part 2: Refactor user_commands separately.

Part 1 summary

The first PR focuses on the command structure itself:

• Reorganized commands toward the focused folder shape: <command>_command.rs, <command>_impl.rs, and mod.rs.
• Removed the commands::shared tree because it was not a real shared command layer anymore.
• Deleted dead-code paths and public methods that were only referenced by tests.
• Removed placeholder config handler modules and tests that only protected unused public APIs.
• Moved real cross-module behavior out of commands into neutral TUI modules:
  • config persistence
  • config action handling
  • model routing / auto model selection
  • share export/upload behavior
  • conversation reset state
• Kept command handlers as the command entry points. UI and runtime code now call neutral modules instead of reaching into command internals.

Why this split matters

This keeps the first PR focused on command ownership and dead-code cleanup. The remaining user_commands work is still important, but it is a separate concern and should be reviewed as its own follow-up instead of being mixed into the broader command-group restructuring.

Paulo Aboim Pinto

[github-actions]

Thanks @aboimpinto for the report.

This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this issue is staying open. When enforcement is enabled, issues from contributors who are not listed in .github/APPROVED_CONTRIBUTORS will be closed automatically.

Please read CONTRIBUTING.md for the expected issue shape. A maintainer can grant issue access by commenting /lgtmi on an issue.

[aboimpinto]

Hi @Hmbown, I want to avoid guessing and producing a stack of PRs that still does not match what you would accept for #2791.

#2851 is currently the proof/reference branch for the command-strategy refactor. I understand from the harvest note in #2722 that the direct PR is too broad to harvest into v0.9 as-is, even though the branch can be made green locally.

Before I restart the work against the v0.9 cutline, can you confirm which migration shape you would consider acceptable?

Options I see:

1. Atomic migration: one command-architecture PR that moves the command responsibility out of the monoliths in one pass, with a parity harness, migration map, and focused commits. This avoids a long-lived hybrid command system, but it is close to the shape you already marked as too broad in Refactor TUI command groups into focused implementations #2851.

2. Partial command-by-command migration: introduce a temporary dual loader and migrate one command per PR. This is very reviewable per PR, but it creates a lot of work, many stacked PRs, and a real risk of leaving the repo halfway between two command systems if the migration stops.

3. Partial group-by-group migration: introduce a temporary dual loader and migrate one command group per PR, for example config, core, debug, memory, project, session, skills, and utility. This is roughly 8 stacked PRs. It is less extreme than command-by-command, but still creates a temporary hybrid that needs explicit completion pressure.

4. Another shape you would prefer.

5. Leave Refactor TUI command groups into focused implementations #2851 as design/proof only and do not pursue this refactor for v0.9.

My concern with options 2 or 3 is not the implementation difficulty. It is the architecture risk: if we stop halfway, CodeWhale ends up with old and new command systems, two places to add commands, and unclear ownership. I can work that way if you prefer, but I would want the dual-loader bridge to be explicitly temporary, with a completion checklist and a linked old-loader removal PR/issue.

Also, should new work for this be based on codex/v0.9.0-stewardship, or should final PRs still target main while being validated against the v0.9 stewardship branch?

Paulo Aboim Pinto

[aboimpinto]

@Hmbown
Follow-up after comparing the current CodeWhale shape, #2851, and Claw Code's Rust implementation.

I think there is a better middle path than the options I listed above.

Claw Code appears to use a dedicated Rust commands crate, which is a useful boundary, but it still keeps the slash-command implementation centralized in a large commands/src/lib.rs rather than focused files per command. That is an important signal: the useful idea is not necessarily "one file per command". The useful idea is a clearer command ownership boundary.

The reason I am still interested in this refactor is not file size by itself. Large files are manageable with search tools. The practical problem is responsibility blur: when command behavior, shared command plumbing, UI-facing helpers, config/model/share behavior, and tests all cross through the same broad files, refactors become error-prone. It is easy to rename or move one method and miss another path because ownership is not obvious. That matters especially because agents like Codex are doing a lot of this work for us; clear responsibility boundaries reduce agent mistakes.

So I would like to refine the proposed path:

1. Add the v0.9 command parity harness first.
2. Move toward a dedicated command boundary with group-owned files, not immediately to per-command files.

For example:

commands/
  mod.rs
  registry.rs       // command specs, aliases, duplicate checks
  parse.rs          // slash parsing
  help.rs           // help/palette rendering
  groups/
    core.rs
    config.rs
    session.rs
    skills.rs
    project.rs
    memory.rs
    utility.rs
    debug.rs

This is closer to a Claw-Code-style stepping stone, but with clearer responsibility boundaries inside the command area.

It avoids both extremes:

• not the current broad monolith/responsibility blur;
• not a huge per-command file explosion;
• not a command-by-command dual-loader migration that might stop halfway and leave two command systems behind.

If a group file later becomes too large or blurry, we can split only that group further. For example, groups/config.rs could later become groups/config/{mod.rs,trust.rs,mode.rs,theme.rs} if the responsibility boundary requires it. But the first target would be group ownership, not one file per slash command.

Would this be a more acceptable migration target for #2791?

In short: parity harness first, then a command-boundary/group-owned refactor against the v0.9 cutline, with per-command files deferred unless a specific group actually needs that extra split.

Paulo Aboim Pinto

[Hmbown]

@Hmbown
Follow-up after comparing the current CodeWhale shape, #2851, and Claw Code's Rust implementation.

I think there is a better middle path than the options I listed above.

Claw Code appears to use a dedicated Rust commands crate, which is a useful boundary, but it still keeps the slash-command implementation centralized in a large commands/src/lib.rs rather than focused files per command. That is an important signal: the useful idea is not necessarily "one file per command". The useful idea is a clearer command ownership boundary.

The reason I am still interested in this refactor is not file size by itself. Large files are manageable with search tools. The practical problem is responsibility blur: when command behavior, shared command plumbing, UI-facing helpers, config/model/share behavior, and tests all cross through the same broad files, refactors become error-prone. It is easy to rename or move one method and miss another path because ownership is not obvious. That matters especially because agents like Codex are doing a lot of this work for us; clear responsibility boundaries reduce agent mistakes.

So I would like to refine the proposed path:

1. Add the v0.9 command parity harness first.
2. Move toward a dedicated command boundary with group-owned files, not immediately to per-command files.

For example:

commands/
  mod.rs
  registry.rs       // command specs, aliases, duplicate checks
  parse.rs          // slash parsing
  help.rs           // help/palette rendering
  groups/
    core.rs
    config.rs
    session.rs
    skills.rs
    project.rs
    memory.rs
    utility.rs
    debug.rs

This is closer to a Claw-Code-style stepping stone, but with clearer responsibility boundaries inside the command area.

It avoids both extremes:

• not the current broad monolith/responsibility blur;
• not a huge per-command file explosion;
• not a command-by-command dual-loader migration that might stop halfway and leave two command systems behind.

If a group file later becomes too large or blurry, we can split only that group further. For example, groups/config.rs could later become groups/config/{mod.rs,trust.rs,mode.rs,theme.rs} if the responsibility boundary requires it. But the first target would be group ownership, not one file per slash command.

Would this be a more acceptable migration target for #2791?

In short: parity harness first, then a command-boundary/group-owned refactor against the v0.9 cutline, with per-command files deferred unless a specific group actually needs that extra split.

Paulo Aboim Pinto

I think this is a wonderful idea. I apologize for the back and forth!!

[Hmbown]

yeah exactly — and to make this actionable: I think the middle path you proposed is the right target.

I really do want us to take your work seriously here and aim to get it in directly, not just harvest around it. #2851 is big, but it is also clearly real work, and you have been thoughtful about keeping it draft, getting CI green, and asking what shape would actually be acceptable instead of guessing.

For v0.9, I think the best next step is:

1. add the command parity harness first;
2. then move toward the group-owned command boundary you sketched;
3. avoid a command-by-command temporary dual-loader unless the harness proves we need that.

Please keep #2851 as the proof/reference branch. For the next mergeable step, I would open the parity harness against codex/v0.9.0-stewardship so we can review and land that foundation directly. After that, we can take the group-boundary refactor in one PR or a small stack, depending on what the diff looks like once the harness is protecting behavior.

The goal is not to make you redo work for no reason. The goal is to get the architecture boundary right while still giving us something we can safely merge and validate during the v0.9 cutline.

Thank you again for pushing on this — the responsibility-boundary framing is the part that clicked for me.

[aboimpinto]

Created the staged tracker and first mergeable layer:

• EPIC tracker: EPIC: staged command-boundary refactor for #2791 #2870
• Layer 1 draft PR: Layer 1: clean command support boundaries #2871, targeting codex/v0.9.0-stewardship
• Reference/proof PR remains Refactor TUI command groups into focused implementations #2851

Layer 1 is intentionally limited to command-surface cleanup and neutral extraction for shared config persistence/model routing. It does not restructure command folders, add a dual loader, or touch user_commands architecture. The remaining layers are tracked in #2870.

Paulo Aboim Pinto