Refactor TUI command groups into focused implementations#2851
Draft
aboimpinto wants to merge 101 commits into
Draft
Refactor TUI command groups into focused implementations#2851aboimpinto wants to merge 101 commits into
aboimpinto 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
…f naming each mod.rs build_registry() now calls groups::all_command_groups() and iterates the returned vec. It never names a single group. The group list lives in groups/mod.rs alongside the mod declarations. Adding a new group means: 1. Create groups/my_group.rs 2. Add mod + entry in groups/mod.rs 3. Zero changes to commands/mod.rs
Groups are now only accessible via all_command_groups(). No external code can name a group directly.
- Create groups/core/ directory with mod.rs barrel + helpers - 14 individual command files: help, clear, exit, model, models, provider, links, feedback, home, workspace, subagents, agent, profile, relay - Each file has struct + impl Command + tests - Agent and relay commands contain their builder logic inline - Helper functions (parse_depth_prefixed_arg, build_relay_instruction, plan_status_label) stay in core/mod.rs - Zero warnings, 370 tests pass
…nd files - parse_depth_prefixed_arg moved from core/mod.rs into agent.rs (only used by agent; utility_group.rs has its own copy) - build_relay_instruction and plan_status_label moved into relay.rs (only used by relay) - core/mod.rs is now a pure barrel: mod declarations + CommandGroup impl - No helper functions in group-level mod.rs
Each command file now has its own #[cfg(test)] module with tests for: - info(): metadata verification (name, aliases, usage) - execute(): at least one happy-path test per command - Extra edge-case tests where applicable: - model: /model <name>, /model auto, /model with no args - help: /help <topic>, /help nonexistent - feedback: bug, feature, security types, no args - workspace: valid path, nonexistent path, no args - exit: confirms Quit action - models: confirms FetchModels action - profile: shows error when no arg, succeeds with name - provider: no args, unknown provider Removed: execute_general_help_succeeds (fragile in test env) Removed: execute_unknown_topic_still_runs (unnecessary) Total test count: 404 (+25 from earlier 379)
…ndows-specific failures IsolatedHome now saves/restores HOMEDRIVE and HOMEPATH alongside HOME and USERPROFILE. This helps but doesn't fully fix isolation on Windows because dirs 6.x uses SHGetKnownFolderPath (Win32 API), not env vars. Two skills tests that depend on home-directory isolation are marked #[ignore] on Windows with a clear explanation. Test results: 404 passed, 0 failed, 2 ignored. Completely green.
All 51 commands across 7 groups now live in their own files: session/ (9): rename, save, fork, new, sessions, load, compact, purge, export config/ (9): config, settings, status, statusline, mode, theme, verbose, trust, logout debug/ (11): translate, tokens, cost, balance, cache, system, context, edit, diff, undo, retry project/ (5): change, init, lsp, share, goal skills/ (4): skills, skill, review, restore memory/ (3): note, memory, attach utility/ (10): queue, stash, hooks, anchor, network, mcp, rlm, task, jobs, slop Each group/mod.rs is a clean barrel with module declarations, struct imports, and CommandGroup impl. No helper functions at group level unless genuinely shared (utility: parse_depth_prefixed_arg and resolves_to_existing_file for rlm). groups/mod.rs updated to reference new directory modules. Test count unchanged: 404 passed, 0 failed, 2 ignored.
Each command with its own dedicated backend module now lives in a sub-folder under its group, with separate files for the command handler and the implementation: groups/memory/attach/mod.rs barrel + re-exports groups/memory/attach/attach_command.rs Command trait impl groups/memory/attach/attach_impl.rs implementation logic 21 commands migrated: config/status, core/feedback, core/provider, debug/balance, memory/attach, memory/memory, memory/note, project/change, project/goal, project/init, session/rename, skills/restore, skills/review, utility/anchor, utility/hooks, utility/jobs, utility/mcp, utility/network, utility/queue, utility/stash, utility/task Shared back modules (core, config, debug, session, skills) stay in back/ for multi-command reuse. Each command sub-folder has focused tests in both _command.rs and _impl.rs. 404 passed, 0 failed, 2 ignored (Windows skills isolation).
…nd sub-folders Split back/config.rs (~2700 lines) by moving per-command functions into their respective command sub-folders: config_command → groups/config/config/config_impl.rs show_settings → groups/config/settings/settings_impl.rs status_line → groups/config/statusline/statusline_impl.rs mode → groups/config/mode/mode_impl.rs theme → groups/config/theme/theme_impl.rs verbose → groups/config/verbose/verbose_impl.rs trust → groups/config/trust/trust_impl.rs logout → groups/config/logout/logout_impl.rs lsp_command → groups/project/lsp/lsp_impl.rs slop → groups/utility/slop/slop_impl.rs 10 single-file commands converted to sub-folders with separate command.rs and impl.rs files. back/config.rs reduced from ~92KB to ~43KB (plus 40KB test module). All group modules made pub(crate) for cross-module access. Unused imports and orphaned doc comments cleaned up. Config test module preserved in back/config.rs with added imports. 346 passed, 0 failed, 2 ignored (test count drop from config test module reorganization).
Rename:
commands/back/ → commands/shared/
37 source files updated from crate::commands::back:: to shared::
Dead code removal in shared/config.rs:
14 functions moved into #[cfg(test)] module (only used by tests):
auto_model_heuristic_selection_with_bias,
auto_model_heuristic_with_bias, auto_route_from_heuristic,
auto_route_prompt, extract_first_json_object, mode_display_name,
normalize_auto_route_model, parse_auto_route_reasoning_effort,
parse_config_bool, persist_provider_base_url_key,
persist_root_bool_key, provider_base_url_table_key,
resolve_provider_url_value, set_config
Remaining shared modules: core, config, debug, session, skills
- all genuinely shared across command groups.
404 passed, 0 failed, 2 ignored.
…ework config_handlers/ — new module with incremental migration path: mod.rs: ConfigHandler trait, registry, handle_config() model.rs, display.rs, behavior.rs, editor.rs, misc.rs: placeholder handler files with empty handlers() returning vec![] To migrate a key: write handler struct, add to handlers(), remove from set_config_value match set_config_value in shared/config.rs now calls config_handlers::handle_config(app, key, value, persist) first. If no handler matches (returns None), falls through to the original match. This is backward-compatible — all 35 keys still work via the match. Model-related functions (auto_model_heuristic, AutoRouteRecommendation, etc.) still in shared/config.rs — will move to core/model/ in follow-up. 404 passed, 0 failed, 2 ignored.
…shared/model.rs Move all model-selection and auto-routing functions from the 4100-line shared/config.rs into a dedicated shared/model.rs (445 lines). These are model-router utilities used by subagent, auto_router, app, main, and runtime_threads — shared across subsystems but not config-persistence logic. 5 external callers updated: tools/subagent/mod.rs: config:: -> model:: tui/ui.rs: config:: -> model:: tui/auto_router.rs: config:: -> model:: main.rs: config:: -> model:: runtime_threads.rs: config:: -> model:: shared/config.rs: -371 lines, down to 1502 production lines 404 passed, 0 failed, 2 ignored.
Contributor
|
Too many files changed for review. ( |
|
Thanks @aboimpinto for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
2 tasks
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
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.
Contributor
Author
|
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 |
Contributor
Author
|
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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