Skip to content

refactor: move dispatch into per-subcommand modules#83

Merged
shadowhand merged 8 commits intorefactor-split-config-modulefrom
refactor-move-dispatch-into-subcommands
Apr 28, 2026
Merged

refactor: move dispatch into per-subcommand modules#83
shadowhand merged 8 commits intorefactor-split-config-modulefrom
refactor-move-dispatch-into-subcommands

Conversation

@shadowhand
Copy link
Copy Markdown
Contributor

Summary

Stacked on #82.

Each top-level subcommand module now exposes a dispatch function that owns the inner match for its *Commands enum. main.rs::run becomes a thin shell that maps Commands variants to those, replacing the deeply-nested ~230-line ladder with a flat 11-arm match.

main.rs: 464 → 302 lines

// Before: each variant had its own multi-line dispatch
Commands::Clinicians(clinician_args) => {
    let ctx = build_ctx(...)?;
    match clinician_args.command {
        CliniciansCommands::Assign(args) => {
            commands::clinicians::assign(&ctx, &args.target, &args.team, out).await?;
        }
        CliniciansCommands::Grant(args) => {
            commands::clinicians::grant(&ctx, &args.target, &args.role, out).await?;
        }
        // ... 6 more variants
    }
}
// After
Commands::Clinicians(args) => {
    let ctx = build_ctx(&config, profile_override.as_deref(), auth_override.as_deref(), config_dir)?;
    commands::clinicians::dispatch(args, &ctx, out).await?;
}

Dispatcher signatures

  • Most: (args, &AppContext, &Output) — auth, api, artifacts, clinicians, roles, teams, workspaces
  • update::dispatch(out) — no args, no ctx
  • skills::dispatch(args, out) — args, no ctx
  • config::dispatch(args, &mut Config, &cfg_path, &config_dir, profile_override, out) — needs Config, not AppContext

build_ctx still gets called per-arm because the four dispatcher signatures need slightly different argument shapes; a forced-uniform signature would require awkward AppContext-or-Config plumbing for no win.

Stack ahead

  1. Extract Stage + slug to a domain module
  2. CI hardening

Test plan

  • cargo fmt --check clean
  • cargo clippy --all-targets — same 11 pre-existing warnings, none from changes
  • cargo test — 288 pass total (270 lib + 7 bin + 11 integration), unchanged

https://claude.ai/code/session_012fVDyKBysF1ERtvVuEAw2Y


Generated by Claude Code

Each top-level subcommand module now exposes a `dispatch` function
that owns the inner match for its `*Commands` enum. main.rs becomes a
thin shell that maps `Commands` variants to those.

main.rs::run is now a flat 11-arm match (was a deeply nested ~230-line
ladder). build_ctx is still called per-arm — different subcommands need
slightly different argument shapes so a uniform dispatcher signature
isn't worth the awkward AppContext-or-config plumbing.

`commands::*::dispatch` signatures:

- Most: `(args, &AppContext, &Output)` — auth, api, artifacts, clinicians,
  roles, teams, workspaces
- `update::dispatch(out)` — no args, no ctx
- `skills::dispatch(args, out)` — args, no ctx
- `config::dispatch(args, &mut Config, &cfg_path, &config_dir,
  profile_override, out)` — needs Config, not AppContext

main.rs: 464 → 302 lines (-162). 288 tests still pass.
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Pulls domain types out of cli.rs (which now is pure clap parsing)
and consolidates the WorkOS configuration that lived in
commands/auth.rs onto Stage as a method.

- src/domain/stage.rs: Stage enum + `Stage::api_url(org)` (replaces
  api.rs::resolve_api as a free function — kept as a thin facade for
  backwards compat) + `Stage::workos_config()` (replaces the consts
  + match in commands/auth.rs).
- src/domain/slug.rs: validate_slug now uses a compiled regex via
  LazyLock instead of the hand-rolled char-by-char loop. The regex
  crate is already a dependency.
- cli.rs: re-exports `Stage` and `validate_slug` from `crate::domain`
  so existing call sites keep working without churn.
- commands/auth.rs: WorkOsConfig + PRODUCTION/DEV consts deleted;
  workos_config() is now a one-line forward to `stage.workos_config()`.
- commands/config/profile.rs: switched from local prompts.rs to
  `crate::prompt as p`. prompts.rs is deleted (97 LOC dropped).

Net: domain types now have a single home, cli.rs is 70 LOC slimmer,
auth.rs's WorkOS plumbing collapsed, and the duplicated
config/prompts.rs is gone.

Test counts: 273 lib (+3 from new stage tests) + 7 bin + 11 integration
= 291. No behavior change.
- Clippy now runs with --all-targets --all-features and -D warnings,
  so any new warning fails CI.
- New release-build job catches release-only compile/link issues
  (LTO, codegen-units, etc.) before they hit a tag.
- New audit job runs cargo-audit against the lockfile to surface
  known-vulnerable transitive dependencies.

Fixes the 10 pre-existing clippy warnings that accumulated across
the codebase so the strict lint passes:

- src/commands/api.rs: useless format!() in a test fixture
- src/commands/clinicians/prepare.rs: setup_prepare_mocks_by_uuid
  has 10 args by design (mock-construction helper); allowed locally
- src/commands/config/default.rs: map_or(true, ...) → is_none_or(...)
  (3 occurrences in tests)
- src/commands/skills.rs: needless &Path::path() borrows in tests
- src/config.rs, src/version_check.rs: field-reassign-with-default
  in three test fixtures, replaced with struct-update syntax

CONTRIBUTING.md: documented the strict clippy invocation and the
release build command so local checks match CI.

288 → 291 tests still pass (no test changes; the +3 came in PR8 from
Stage::workos_config tests).
claude and others added 5 commits April 28, 2026 02:18
The new cargo-audit job in CI surfaced three vulnerabilities in
transitive dependencies. All three resolve to semver-compatible
patch versions reachable via `cargo update`:

- rustls-webpki 0.103.10 → 0.103.13
  - RUSTSEC-2026-0098: name constraints for URI names
  - RUSTSEC-2026-0099: name constraints accepted for wildcard certs
  - RUSTSEC-2026-0104: reachable panic in CRL parsing
- rand 0.9.2 → 0.9.4
  - RUSTSEC-2026-0097: unsoundness with custom logger via rand::rng()
- fastrand 2.4.0 → 2.4.1 (yanked → published)

No source changes; only Cargo.lock. All 291 tests still pass.
ci: harden lint, add release build and audit jobs
The generic `Resource<A, R = ()>` introduced in #77 silently regressed
deserialization: the previous hand-rolled `*Resource` types had no
`relationships` field at all, so serde skipped it; the new generic
declares it explicitly as `R` and defaults `R` to `()`, which only
deserializes from null or absent.

Real JSON:API responses for clinicians, teams, and others carry
populated relationships objects (e.g. `{"team": {"data": {...}}}`).
Against the live API those calls would have failed at runtime with
`invalid type: map, expected unit`. Test mocks all omitted the
relationships key, so the regression slipped through CI.

Switching the default to `serde_json::Value` accepts any JSON shape
on the field — null, object, array, primitive, or absent — and
callers that don't read it can ignore it. Callers that need typed
relationships still pass an explicit `R` (see test_typed_relationships
and the existing call sites for `prepare`'s WorkspaceAttrs etc.,
which use named structs and are unaffected).

Adds three regression tests covering null, empty object, and populated
object cases — the third would have caught this had it been there.

Spotted by Devin AI in review of #78.
…fault

fix(jsonapi): default Resource relationships to serde_json::Value
refactor: extract Stage + slug into a domain module
@shadowhand shadowhand merged commit 585d788 into refactor-split-config-module Apr 28, 2026
4 checks passed
@shadowhand shadowhand deleted the refactor-move-dispatch-into-subcommands branch April 28, 2026 02:51
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.

2 participants