Refactor TUI command groups into focused implementations#2851
Refactor TUI command groups into focused implementations#2851aboimpinto wants to merge 101 commits into
Conversation
…d, clear on turn end
…nel, works during active turns)
…ing...' initially
…ssage flow through as normal user message
… non-'continue' messages
…the user retypes fresh
…s model from trying alternatives)
…ally stops the turn
…of separate line)
…roke); other messages cancel + consume
…tale 100% from previous command)
…ded (pausable/paused flags extend the cancel window)
… prevent 'Command resumed' event overriding cancel status
… sync in dispatch_user_message; direct flag clear in cancel path
… app.pausable is true (new pausable command after cancel)
…x mock engine handle missing shared_paused
…Paused / Request was Resumed / Request was Cancelled
…ds race where late engine status overwrites cancel/resume; add guard in Event::Status handler; update test to verify guard
…in next turn; add test
…prompted to continue the goal
…stead of continuing
There was a problem hiding this comment.
Code Review
This pull request modularizes the command system in crates/tui by organizing commands into logical groups under a registry-based architecture using Command and CommandGroup traits. It also introduces a pausable workflow feature with workspace rollback support via git stash, and refactors shell invocation handling. The review feedback identifies several instances where the unstable let_chains feature is used (in config_impl.rs, trust_impl.rs, user_commands.rs, shell_invocation.rs, and slash_menu.rs), and provides actionable suggestions to rewrite these using standard nested if let blocks or is_some_and to ensure compatibility with stable Rust compilers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed the Gemini Code Assist note about No code change was made for that item because this repository declares The local validation for this branch passed with:
Paulo Aboim Pinto |
|
Follow-up after the first review/CI pass:
Latest CI run is green across Lint, macOS, Windows, Ubuntu, mobile smoke, version drift, and GitGuardian. Paulo Aboim Pinto |
|
@aboimpinto — checking in on this one now that Layer 3 (#2888) is merged. Confirming the plan we're operating on: this PR stays open as the reference/proof PR for the layered re-landing series (#2871 → #2878 → #2888 → Layer 4+), and we close it once the final layer lands — none of it merges directly. One question: the pause/resume/cancel lifecycle work tangled into this branch looks separable from the command-group extraction. Would you be up for carving it out into its own layered PR (against Layer 4 (group-owned command modules) is green-lit over on #2888. Thanks for the discipline on this series — it's the model we want for big refactors. |
|
/lgtm |
|
v0.8.59 release triage: deferring this PR out of the release-stabilization lane. It is not being rejected; it is just not required to unblock 0.8.59 now that the current release blockers have narrow fixes on |
Harvested from PR #2851 by @aboimpinto Co-authored-by: Paulo Aboim Pinto <aboimpinto@gmail.com> Co-authored-by: Hunter Bown <hmbown@gmail.com>
Summary
This is the first part of the command-strategy refactor from #2791. The goal of this PR is to stop treating large command implementation files as shared code and put command behavior next to the command groups that own it.
This PR does not intentionally remove command features. It restructures the implementation surface, removes dead command plumbing, and deletes tests that only protected public methods that production code no longer called.
What changed
<command>_command.rsfor command registration/handler metadata<command>_impl.rsfor the command behaviormod.rsfor the command module wiringcommands::sharedtree. It was not truly shared command code anymore; most modules were either command-specific implementation pipework, placeholders, or APIs that were only exercised by tests.commandsinto neutral TUI modules:config_actions.rsfor config value handlingconfig_persistence.rsfor config persistence used by commands and UI codemodel_routing.rsfor model selection and auto-routing used outside command handlersconversation_state.rsfor conversation reset behavior used outside command handlersshare_export.rsfor async share/export behavior used by UI and the/sharecommandcommandsinternals.Why
The old structure made files look shared when they were not shared. That created a lot of accidental pipework: command handlers called command impl files, those impl files called
commands/shared/*, and some UI code also reached into command modules directly. That made ownership hard to reason about and kept public methods alive only because tests referenced them.This PR moves command-specific behavior back under the command groups and extracts the small amount of real cross-module behavior into neutral modules outside
commands.Scope
This PR covers the command-group restructuring work. The remaining
user_commandsrefactor is intentionally left for the follow-up tracked in #2791.Notes on tests and dead code
A large part of the cleanup was removing tests that were testing public methods no production path called anymore. Those methods were not command behavior; they were dead APIs kept alive by the test suite. This PR keeps tests around command-owned behavior and neutral module behavior, but does not preserve tests whose only value was protecting removed dead-code methods.
Validation
cargo fmt --package codewhale-tuigit diff --checkcargo check -p codewhale-tui --binscargo check -p codewhale-tui --testsRefs #2791
Paulo Aboim Pinto