Skip to content

Plan 009: Split commands.rs into per-domain modules #24

Description

@duyetbot

Plan 009: Split commands.rs into per-domain modules

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md — unless a reviewer dispatched you and told you they
maintain the index.

Drift check (run first): git diff --stat 61ee3c7..HEAD -- src/commands.rs src/lib.rs
If either changed since this plan was written, compare "Current state"
against live code; on mismatch, STOP.

Status

  • Priority: P3
  • Effort: L
  • Risk: MED (large mechanical move; compile-verified at each step)
  • Depends on: plans 002–007 ideally merged first (they edit commands.rs / upgrade.rs / auth.rs). Not strictly required — if not DONE, expect merge friction; note it in your report.
  • Category: tech-debt
  • Planned at: commit 61ee3c7, 2026-08-26

Why this matters

src/commands.rs holds every command implementation plus TUI plumbing in one
3,342-line file: login/auth, models, usage, whoami, config (including a
settings TUI), launch glue, logout/account, keys, launcher palette, menu.
Every new feature lands here; concurrent features collide; reviews wade
through unrelated context. The internal seams are clean (run_* functions
per command), so the split is mechanical — no behavior changes.

Current state

Function map from grep at 61ee3c7 (line ranges approximate):

Lines Domain Key items
33–233 TUI helpers tui_wants_dump, tui_menu_select/palette_select/settings_select + dump renderers
234–412 dispatch core cmd_kind, known_command, canonical_command, allowed_flags, assert_known_flags, wants_help, help_topic, shift_passthrough
413–495 auth cmds run_auth, run_auth_token, run_auth_switch
500–633 main dispatch run(), dispatch(), stub()
642–735 login config_path, persist_login, run_login
737–915 models model_pick_label, pick_*, set_model_slot, save_model_slot, apply_claude_alias_flags
916–1098 models cmd / usage / whoami run_models, run_usage, run_whoami
1099–2069 config TUI print_config_status, run_config_tui, settings frame/loop/edit rows, config_menu_loop_legacy
2070–2220 launch catalog_lookup_enabled, resolve_session_model, run_launch
2221–2392 logout/account run_logout, run_account_use, run_account
2393–2662 keys keys_credential, default_key_name, run_keys, resolve_latest_key, stored_api_key
2663–3100+ launcher/menu launcher_palette, run_menu, push_launch_entries, install_agent_dialog, persist_tool_command

Public surface consumed elsewhere: commands::run(argv, env) -> i32 is called
from src/main.rs; run_launch, launcher_palette, CreditsCache,
kick_credits_refresh, tui_dump_settings, SettingsState etc. are used by
src/tui/live.rs (verify with grep before moving anything:
grep -n "crate::commands::" src -r). Whatever those greps show must keep its
path or gain a re-export.

Repo conventions: modules are flat files under src/, pub fn surface kept
minimal, inline unit tests per module. lib.rs declares modules alphabetically
(see src/lib.rs).

Commands you will need

Purpose Command Expected on success
Compile check after every move cargo build --locked exit 0
Full suite cargo test --locked --all-targets all pass
Public-path audit grep -rn "crate::commands::" src tests list shrinks to intended re-exports

Scope

In scope:

  • src/commands.rs (becomes a thin dispatcher or directory module)
  • New files: src/cmd/ modules as laid out in Step 1
  • src/lib.rs (module declaration only)
  • Call-site fixes in other src files ONLY where a moved item's path changes
    (prefer keeping paths stable via re-exports)

Out of scope:

  • Any behavioral change, signature change, or "small cleanup while we're here".
  • Renaming functions. Moving text verbatim only.
  • src/tui/* internals.

Git workflow

  • Branch: advisor/009-split-commands-god-file
  • Commit per extracted module: e.g. refactor(cmd): extract config TUI from commands.rs
  • Do NOT push or open a PR.

Steps

Step 1: Create src/cmd/ with the domain modules

Create src/cmd/mod.rs plus these modules, each starting as an empty shell:

  • auth.rs (run_auth*, hint)
  • login.rs (persist_login, run_login)
  • models.rs (model pick/save/slot fns, run_models)
  • usage.rs (run_usage, run_whoami, print_config_status stays? NO — that belongs to config)
  • config_tui.rs (everything from lines 1099–2069)
  • launch.rs (catalog_lookup_enabled, resolve_session_model, run_launch)
  • account.rs (run_logout, run_account*)
  • keys.rs (keys_credential, default_key_name, run_keys, resolve_latest_key, key_pick_label, stored_api_key)
  • menu.rs (launcher palette, run_menu, push_launch_entries, install_agent_dialog, persist_tool_command, CreditsCache, kick_credits_refresh)
  • dispatch.rs (cmd_kind, known/canonical, allowed_flags, assert_known_flags, wants_help, help_topic, shift_passthrough, stub, tui_wants_dump)

Declare mod cmd; in src/lib.rs.

Verify: cargo build --locked → exit 0 (empty shells compile).

Step 2: Move code domain by domain, compiling between each

Order (least-coupled first): account → usage → models → login → auth → keys → launch → menu → config_tui → dispatch.

For each domain:

  1. Cut the functions verbatim into the new module file.
  2. Add use statements as demanded by the compiler (match existing imports
    at top of commands.rs).
  3. Fix cross-module references via use super::… or use crate::cmd::….
  4. Keep old names reachable where external callers exist: add
    pub use crate::cmd::menu::{launcher_palette, CreditsCache, …};
    style re-exports IN commands.rs for everything grep -rn "crate::commands::" src tests listed.
  5. cargo build --locked must pass before moving to the next domain.

Verify (after each domain): cargo test --locked --lib <matching-tests> or at minimum cargo build --locked.

Step 3: Shrink commands.rs to the public shim

After all moves, commands.rs should contain only: the run() entry,
dispatch() if still needed, and re-exports. Target ≤ 300 lines.

External call sites (src/tui/live.rs, src/main.rs) keep working through
the re-exports; optionally switch them to direct crate::cmd::… paths IF the
grep list is short (<10 sites) — otherwise leave re-exports.

Verify: wc -l src/commands.rs → ≤ 300.

Step 4: Full verification

Verify: cargo test --locked --all-targets → exit 0 (52 lib tests +
integration suites unchanged and green).
Also: cargo clippy --locked --all-targets → no NEW warnings beyond whatever
baseline exists at your HEAD.

Test plan

No new tests required — this is a pure move refactor. The existing suite
(integration tests exercise help output, dry-run spawn env, login flows
through the real binary) is the regression net. Unit tests embedded in moved
functions' modules travel WITH their code.

Done criteria

ALL must hold:

  • wc -l src/commands.rs ≤ 300.
  • ls src/cmd/ shows the ≥9 domain modules, none empty.
  • cargo test --locked --all-targets exits 0.
  • grep -rn "crate::commands::" src tests returns only intentional re-export consumers (list them in report).
  • git diff shows moves (high line churn, low added-line ratio); reviewer can spot-check any function body is byte-identical pre/post move.

STOP conditions

Stop and report if:

  • Any test fails after a pure verbatim move (means hidden coupling — identify it, do not paper over with logic changes).
  • The function map drifted so much that domains don't match (report actual layout).
  • You find shared private helpers used across would-be-separate domains in a way forcing circular imports — stop at that domain and report.

Maintenance notes

  • Future commands should land in their own src/cmd/<name>.rs; note that in
    a comment atop src/cmd/mod.rs.
  • Reviewer: diff-check three random moved functions byte-for-byte against the
    pre-move blob (git history has it).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions