AkurAI-Build

public
Latest change 2a1193541cf008c406be9f347e44150a58a66130 - docs: preserve the architecture review, backlog, and schema policy by Olafur Bui
# Review 7/10 — CLI & Developer Experience (bunfork)

Scope: `src/main.rs` (1120 lines, read fully), `README.md`, spot-verified against `src/db.rs` and `src/server.rs`. Overall: the clap structure is clean, secrets handling has excellent error hints, and `dev --static` honestly warns about its frozen artifact (server.rs:267). The gaps below are prioritized.

## Findings

**1. P1 — `dev` reloads pages but not `public/` assets.**
`render_page` rescans `app/pages` per request in dev (server.rs:970–973), but `public/` is preloaded once before bind (server.rs:196, `preload_public_files`) and never re-read. Editing CSS/JS under `public/` during `bunfork dev` silently serves stale bytes until restart — worse than "reload, not HMR"; it's no reload at all for assets. Fix: when `development` is true, re-scan (or re-hash lazily) `public/` per request in the `/assets` handler (server.rs:804), mirroring the pages path. Alternatively, document the limitation in the `dev` help text (main.rs:76) and README table row for `dev`.

**2. P1 — `--json` is inconsistent across commands.**
`admit` and `doctor` accept `--json` (main.rs:166, 178); `build` performs the same admission but hardcodes human output (`print_admission(&manifest, false)`, main.rs:436); `vector search` always emits JSON (main.rs:559) while `vector put/delete` emit prose; `keygen`/`migrate`/`backup`/`restore`/`deploy` have no machine-readable mode. CI pipelines scripting `build` or `backup` must scrape prose. Fix: promote `--json` to a global flag or add it uniformly, starting with `build` (one-line change) and `backup` (emit `{ok, path}`).

**3. P1 — `doctor --json` hides the failure reason and exit codes are undifferentiated.**
On failure, the JSON report says only `"artifact validation failed"` (main.rs:466–480); the actual cause goes to stderr via anyhow, and the process exits 1 — the same code as an I/O error or a clap misuse (clap uses 2). A JSON consumer cannot distinguish "manifest mismatch" from "manifest file unreadable". Fix: include `error.to_string()` (the chain) in the `DoctorCheck.message`, and use distinct exit codes (e.g., 1 = operational error, 3 = verification failed) via `std::process::exit` after reporting.

**4. P1 — Onboarding needs four commands and missing-DB errors lack a hint.**
Quick start is `keygen` → `keygen --out .bunfork.token` → `migrate` → `dev`. Running `dev` before `migrate` fails with `inspect database data/bunfork.db: No such file or directory` (db.rs:77–78 → validate_existing_file), with no "run `bunfork migrate`" hint — inconsistent with the excellent missing-secret hint that suggests the exact keygen command (db.rs:428–433). Fix: add the migrate hint to `open_existing`, and consider a `bunfork init` that runs keygen×2 + migrate idempotently.

**5. P2 — `restore --yes` is validated at runtime, not by clap.**
`yes: bool` (main.rs:94–95) is checked with `ensure!(*yes, ...)` (main.rs:331), so omission exits 1 instead of producing a clap usage error (exit 2) with generated help. Minor, but it breaks the usage/error-code distinction the rest of the CLI gets for free. Fix: `#[arg(long, required = true)]` on an `ArgAction::SetTrue` flag, or keep runtime but align exit codes per finding 3.

**6. P2 — `test` and `deploy` require a source checkout + cargo toolchain.**
Both shell out to `cargo test --locked` / `cargo build --release` (main.rs:606, 700, 703) and `deploy` anchors on `fs::canonicalize(".")` (main.rs:618). Running `bunfork test` from a deployed `bunfork-dist/` fails with cargo errors, not a scoped message. Fix: detect absence of `Cargo.toml` in CWD and fail with "run from the bunfork source root".

**7. P2 — Global flags accepted where meaningless.**
`--tenant/--model/--database/--key-file` are `global = true` (main.rs:35–52) and silently accepted on `keygen`, `admit`, `doctor`, and `serve --static`, which use none of them. Low harm, but `serve --static --tenant x` implying tenant scoping is misleading. Also note README says globals "precede the command"; clap global args work after the subcommand too — worth documenting since trailing flags are the more common muscle memory.

## What's already good
- File-over-env secret precedence is implemented exactly as documented (db.rs:414–437) with a self-healing error message.
- `build`'s env allowlist + refusal to forward `BUNFORK_DB_KEY`/`BUNFORK_API_TOKEN` (main.rs:409–413) is exemplary.
- `--static`/`--manifest` and `--artifact`/`--static-manifest` mutual `requires` constraints (main.rs:121–123, 219–223) fail fast at parse time.