AkurAI-Build

public
Latest change 2a1193541cf008c406be9f347e44150a58a66130 - docs: preserve the architecture review, backlog, and schema policy by Olafur Bui
# Architect Review 8/10 — Testing & Verification Architecture (bunfork)

## Summary
Test posture is bimodal: **db.rs unit coverage is excellent** (encrypted backup/restore round-trips, hard-link rejection, path-replacement detection, non-UTF8 orphan cleanup — src/db.rs:1206-1433), and **static mode has a thorough black-box integration matrix** (tests/static_http.rs:19-146: MPA/SPA/base-path, ETag/304, HEAD parity, traversal, 405, SIGTERM shutdown, no-runtime-state). But the **native serve mode — the authenticated API surface — has zero integration coverage**, there is no CI, and no property/fuzz testing on the security-critical parsers.

## Prioritized findings

**1. P0 — Native serve mode is never tested end-to-end.**
tests/ contains only static_http.rs. The vector API (`/api/vectors/{id}` PUT/DELETE, `/api/vectors/search` — src/server.rs:232-237), bearer auth (`authorize`, src/server.rs:1093-1111), same-origin mutation guard (src/server.rs:1113+), page rendering fallback, body limits, and compression layers are exercised only via piecemeal helper unit tests. Extract the router construction inlined in `serve()` (src/server.rs:231-245) into `fn build_app(state: AppState) -> Router` and reuse the existing `tower::ServiceExt::oneshot` pattern already proven at src/server.rs:1523-1560 (`static_status_routes` tests). This is the highest-leverage gap: auth is constant-time-compare security code with no direct test.

**2. P0 — No CI configuration.**
No `.github/`, no workflow YAML anywhere. `cargo test --locked` runs only when a human invokes `bunfork test`/`bunfork deploy` (src/main.rs:605-608, 686). Add a minimal CI: `cargo fmt --check`, `cargo clippy --locked --all-targets`, `cargo test --locked --all-targets` on Linux (tests are `#![cfg(unix)]`).

**3. P1 — Precompressed negotiation untested at the HTTP layer.**
`.br`/`.gz` selection logic exists (src/server.rs:521-601) and `encoding_quality` is unit-tested (src/server.rs:1641-1648), but all three fixtures set `"precompressed": true` while shipping **no** `.br`/`.gz` files (tests/fixtures/static/*/manifest.json). Content-Encoding selection, ETag-per-encoding, and Vary behavior are never integration-verified. Add precompressed variants to one fixture.

**4. P1 — No property-based/fuzz testing on manifest & path parsing.**
No proptest/quickcheck/cargo-fuzz anywhere (Cargo.toml dev-deps: only tower). The hand-written duplicate-key serde visitor (src/artifact.rs:660-676), `resolve()` percent-decode/traversal logic (unit-tested at src/artifact.rs:699-740 with only 2 hostile inputs), `decode_public_asset_path`, and `PageRouter::match_path` are exactly the attack-surface parsers that benefit from property tests (e.g., "no decoded path ever escapes root", "resolve never panics on arbitrary UTF-8").

**5. P1 — `bunfork test` conflates framework self-test with app verification.**
`test_project` (src/main.rs:605-608) shells `cargo test --locked` in cwd — it assumes a Rust toolchain and the bunfork source tree, so on a deployed target it fails or runs an unrelated project's tests. The valuable half is `smoke_test` (src/main.rs:572-603): toolchain-free route/migration/vector/backup verification — a genuinely good deployment health-probe pattern. Recommendation: make `bunfork test` run smoke checks only; leave `cargo test` to CI and `deploy`.

**6. P2 — CLI wiring untested.**
`migrate`, `backup`, `restore --yes`, `vector put/search/delete` (src/main.rs:341, 541-571), `build`, and `doctor` have no subprocess tests; only their underlying db/artifact functions are covered. One CLI smoke test would catch clap/arg-plumbing regressions.

**7. P2 — Heavyweight, self-built integration harness.**
static_http.rs carries ~200 lines of untested infrastructure: raw TCP HTTP/1.1 client, header folding, chunked decoder (tests/static_http.rs:333-449), sequential port allocator (line 15). Keep one process-level test (SIGTERM + no-runtime-state at lines 280-317 genuinely require a real process); move the header/body contract matrix in-process via oneshot after item 1's refactor to cut flake surface.

## Verdict
Unit/integration balance is inverted per mode: static = integration-only-strong, native = unit-only. Items 1-2 should land before any further feature work.