refactor: promote crate to library + add integration tests#79
Merged
shadowhand merged 16 commits intorefactor-migrate-resource-commands-to-apiclientfrom Apr 28, 2026
Merged
Conversation
Splits the crate into: - `src/lib.rs` — `pub mod` declarations for every top-level module, giving integration tests (and any future external consumers) access to internals. - `src/main.rs` — thin binary that imports from `rw::*`. The dispatch shell stays here for now; PR7 will move it into per-subcommand modules. Adds `tests/cli.rs` with 11 end-to-end tests using `assert_cmd` + `tempfile`. They cover argument parsing, error reporting for invalid input, and the basic dispatch paths the unit tests can't reach. The new test cache is seeded with the current `CARGO_PKG_VERSION` so `version_check::check_and_update` short-circuits on cache hit and never makes a live GitHub HEAD request during tests. Removes the `#![allow(dead_code)]` attributes on `http`, `jsonapi`, and `prompt` — once the modules are `pub` in the library crate, dead-code analysis no longer fires on the unused-internally items. Test counts: - Before: 277 unit - After: 270 lib + 7 bin + 11 integration = 288 total
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
The file was 2985 lines (largest in the crate). Now 11 files in
`commands/clinicians/`, each focused on one concern:
- `mod.rs` — module wiring + re-exports
- `data.rs` — JSON:API deserialization types (Clinician*)
- `output.rs` — output types (Prepare/Grant/Assign/etc.)
- `client.rs` — legacy reqwest helpers shared across commands
(resolve_team, fetch_clinician_*, apply_auth, etc.)
- `testing.rs` — `#[cfg(test)]` shared fixtures (TestAuthGuard,
*_response builders) used by every command's tests
- `assign.rs` — `pub fn assign` + `patch_clinician_team` + tests
- `enable.rs` — `pub fn enable` / `disable` + `set_enabled` + tests
- `grant.rs` — `pub fn grant` + `patch_clinician_role` + tests
- `prepare.rs` — `pub fn prepare` + workspace helpers + tests
- `register.rs` — `pub fn register` + tests
- `show.rs` — `pub fn show` + tests
- `update.rs` — `pub fn update` + validation + tests
No behavior change. All 288 tests still pass (270 lib + 7 bin + 11
integration). Largest remaining file is `prepare.rs` at 832 lines, of
which most is the elaborate multi-mock test setup; `update.rs` is 557
for the same reason. Production code per file is now well under 200
lines each.
The cross-command helpers in `client.rs` retain their raw-reqwest
signatures rather than taking an `&ApiClient`. They will fold into
the ApiClient migration in the next PR.
3 tasks
Folds the clinician-side raw-reqwest helpers into ApiClient calls and switches every command to deserialize via `jsonapi::Single` / `jsonapi::List`. Removes 347 lines of HTTP/parse boilerplate. Per-file changes: - `data.rs`: dropped `ClinicianResource` / `ClinicianListResponse` / `ClinicianSingleResponse`; added a `Clinician` type alias for `Resource<ClinicianAttributes>`. - `client.rs` (177 → 75 LOC): every helper now takes `&ApiClient` instead of `(&Client, &str, &str)`. `apply_auth` deleted (ApiClient attaches auth itself). - `commands::roles::resolve_role`: signature changed to `(&ApiClient, &str)`. The "legacy helper" docstring is removed. - `grant.rs`, `assign.rs`, `enable.rs`, `show.rs`, `update.rs`, `register.rs`, `prepare.rs`: each command now opens with `let api = ApiClient::new(ctx).await?;` and uses `api.get/patch/post` in place of the inline reqwest pipeline. Per-command `patch_clinician_*` helpers deleted; bodies inlined as `serde_json::json!()` literals at the call site. - `prepare.rs`: workspace POSTs now use `api.post_void`. 288 tests still pass (270 lib + 7 bin + 11 integration), unchanged.
The file was 1424 lines with three orthogonal subcommand groups
(profile, updates, default) and five interactive prompt helpers all
mixed together. Now five files under commands/config/:
- mod.rs (15) — module wiring + re-exports
- prompts.rs (97) — local prompt_* helpers; will be replaced by
crate::prompt in a follow-up
- profile.rs (779) — list/show/use/set/rm/add/auth + their output
types + tests
- updates.rs (126) — show/enable/disable + output types + tests
- default.rs (474) — set/get/list/rm + key validation + output
types + tests
profile.rs is the largest by virtue of having seven subcommands and
the bulk of the test code. updates.rs and default.rs are now sized
appropriately for their scope.
288 tests still pass (270 lib + 7 bin + 11 integration), unchanged.
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.
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).
4 tasks
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
…subcommands refactor: move dispatch into per-subcommand modules
refactor: split commands/config.rs into a module
…to-apiclient refactor: migrate commands::clinicians to ApiClient
shadowhand
merged commit 4725090
into
refactor-migrate-resource-commands-to-apiclient
4 checks passed
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
Stacked on #78.
Splits the crate into a library + binary:
src/lib.rsdeclares every modulepubso integration tests undertests/can reach internals.src/main.rsbecomes a thin binary that imports fromrw::*. The dispatch shell stays here for now (PR7 collapses it).Adds
tests/cli.rswith 11 end-to-end tests usingassert_cmd+tempfile. They spawn the actual binary with--config-dir <tmpdir>and a pre-seeded version_check cache (so no live GitHub HEAD requests). Coverage:--version/--helpsucceed--config-dirvalidationauth login --jsonrejected (interactive)--authwithauth loginrejectedauth statuson unauthenticated profileSide benefits
Now that
http,jsonapi, andpromptarepubin a library crate, dead-code analysis no longer fires on items that aren't called yet. The temporary#![allow(dead_code)]attributes added in #77 are removed.Test counts
Stack ahead
commands/clinicians.rsinto a moduleclinicianstoApiClientcommands/config.rsinto a moduleStage+ slug to a domain moduleTest plan
cargo fmt --checkcleancargo clippy --all-targets— same 11 pre-existing warnings as beforecargo test— 288 pass totalhttps://claude.ai/code/session_012fVDyKBysF1ERtvVuEAw2Y
Generated by Claude Code