Skip to content

finima-api: extract lib.rs so integration tests and bin targets can share real production code #101

Description

@pacphi

Background

finima-api (crates/finima-api/) is a binary-only crate: Cargo.toml has no [lib] section, and src/main.rs declares its module tree directly (mod config; mod error_response; mod handlers; mod metrics; mod router; mod state; mod storage; mod ws;, main.rs:1-8). Nothing in this crate — AppState, the axum handlers, the router-building logic, the config loader — is importable from anywhere outside main.rs itself.

This has two independent, compounding costs, both confirmed concretely rather than assumed:

1. No integration test in the crate can exercise real production code

crates/finima-api/tests/common/mod.rs:8-10 already states the workaround plainly:

Because finima-api is a binary crate (no lib.rs), the integration tests reconstruct the router from the public library crates (finima-db, finima-auth, finima-core, etc.) rather than importing from finima-api.

All three existing integration test files (auth_test.rs, authorization_test.rs, tier2_flow_persistence_test.rs) follow this pattern: they hand-roll local reimplementations of the real axum handlers and a local TestAppState that mirrors — but is not — the real AppState.

This was proven to have real teeth during a recent adversarial review of #31/#32/#33's implementation (PR #100). Two production regressions were injected as a mutation-testing oracle:

  • AppState::set_metrics's entire body replaced with () (state.rs:544)
  • The whole "confirm" match arm deleted from the real handlers::flows::update_flow (flows.rs:298)

All existing tests in the crate — including the ones specifically written to cover this code — kept passing. cargo-mutants, scoped to exactly the changed files, reported 0/27 and 0/3 mutants caught respectively. Re-running the identical mutation-testing commands after a follow-up remediation pass (which genuinely fixed several other issues on the same review — see PR #100) showed zero improvement, because the fix couldn't touch the actual production handler code the tests can't reach; it could only make the test-local reimplementation more elaborate. The mutation score for AppState::set_metrics and update_flow's confirm branch is currently pinned at exactly 0 and will stay there regardless of how many more tests are added to this file, because no test in the crate can call either function.

2. Five separate [[bin]] targets already duplicate-include just to get config loading

$ grep -rl '#\[path = "\.\./config\.rs"\]' crates/finima-api/src/bin/
crates/finima-api/src/bin/bootstrap_flows.rs
crates/finima-api/src/bin/normalize_directions.rs
crates/finima-api/src/bin/bootstrap_tier2.rs
crates/finima-api/src/bin/redetect_recurring.rs
crates/finima-api/src/bin/merchant_audit.rs

Each of these bins declares #[path = "../config.rs"] mod config; to re-include src/config.rs as a second copy of the same module, because there's no crate to use finima_api::config from. bootstrap_tier2.rs and bootstrap_flows.rs additionally each hand-duplicate a build_embedder_for_bin function that's a near-verbatim copy of state.rs's build_embedder (bootstrap_tier2.rs:41-73, bootstrap_flows.rs:32-64, state.rs:~594-660) — three independent copies of the same backend-selection logic that have to be kept in sync by hand. (One already drifted: the two bootstrap copies used the panicking CandleEmbedder::new() after state.rs's copy was fixed to use the non-panicking CandleEmbedder::load() — caught and fixed separately, but exactly the kind of drift a shared lib.rs would make structurally impossible.)

What's requested

Extract finima-api into a proper lib.rs + thin main.rs binary, following the standard pattern for testable Rust web services:

  1. Add crates/finima-api/src/lib.rs that declares the existing module tree (pub mod config; pub mod error_response; pub mod handlers; pub mod metrics; pub mod router; pub mod state; pub mod storage; pub mod ws; — or whatever subset needs to be pub vs. pub(crate)) and exposes whatever router/AppState-construction entry point(s) tests and other bins need (likely something like pub async fn build_app_state(config: AppConfig) -> Result<AppState, ...> and pub fn build_router(state: AppState) -> Router, mirroring what main.rs currently does inline).
  2. Add a [lib] section to crates/finima-api/Cargo.toml (name = "finima_api" or similar).
  3. Shrink main.rs to a thin entry point that calls into the new lib (config load → build_app_statebuild_router → bind + serve), removing the mod declarations it currently owns directly.
  4. Migrate the 6 [[bin]] targets (finima-api, merchant-audit, finima-normalize-directions, finima-redetect-recurring, bootstrap_flows, finima-generate-sample) plus the auto-discovered bootstrap_tier2 bin to use finima_api::config etc. instead of #[path = "../config.rs"], and to call the shared build_embedder/equivalent instead of maintaining their own copies.
  5. Migrate tests/auth_test.rs, tests/authorization_test.rs, and tests/tier2_flow_persistence_test.rs to build their router/state via the real finima_api::build_router/finima_api::state::AppState instead of their local reimplementations, and delete the now-redundant hand-rolled handler/router code in tests/common/mod.rs. Re-run the same scoped cargo-mutants commands from PR feat(tier2): observability gauges, E2E persistence test, staging rollout enablement #100's review (cargo mutants --file crates/finima-api/src/state.rs --features sona -- --test tier2_flow_persistence_test and the equivalent for handlers/flows.rs's update_flow) to confirm the mutation score genuinely moves off 0 — that's the acceptance bar for this issue, not just "tests still pass."

Why this is its own issue, not bundled into anything else

The blast radius touches every existing integration test in the crate plus 6+ binary targets — it's a deliberate, scoped architectural change that deserves review on its own merits, not something to slip in as a side effect of an unrelated feature PR. It was explicitly scoped out of the #31/#32/#33 remediation (PR #100) for exactly this reason.

Acceptance criteria

  • crates/finima-api/src/lib.rs exists; main.rs is reduced to a thin entry point
  • The 5 bins currently using #[path = "../config.rs"] (plus bootstrap_tier2, which is auto-discovered) import finima_api::config normally instead
  • bootstrap_tier2.rs/bootstrap_flows.rs's duplicated build_embedder_for_bin functions are removed in favor of calling the shared finima_api implementation
  • auth_test.rs, authorization_test.rs, tier2_flow_persistence_test.rs call the real finima_api router/AppState construction instead of hand-rolled local reimplementations
  • cargo mutants --file crates/finima-api/src/state.rs --features sona -- --test tier2_flow_persistence_test and the equivalent scoped to handlers/flows.rs's update_flow show a mutation score above 0 (baseline: 0/27 and 0/3 as of PR feat(tier2): observability gauges, E2E persistence test, staging rollout enablement #100) — the actual proof this issue is closed, not just that the crate compiles and existing tests still pass
  • Full CI gate (cargo fmt, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace both default and --features sona) stays green throughout

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions