fix(review): apply 8-angle code-review findings from #13

An adversarial multi-angle review of #13 (line-by-line, removed-behavior,
cross-file tracing, reuse, simplification, efficiency, altitude,
conventions; each finding independently verified) surfaced defects that
merged before the fix sweep landed. This applies every confirmed finding.

Correctness (fail-closed integrity):
- boj client: a 200 whose body carries an estate error shape (`error` key
  or `success:false`) is now a FAILURE, not a verdict. The boj router
  unwraps cartridge failure envelopes into 200 bodies, so transport success
  alone was recording hard misses (e.g. echidna without ANTHROPIC_API_KEY)
  as "ok" — a direct fail-closed violation. [cross-file]
- App: an absent `repo_root` no longer silently defaults to the daemon's
  own working directory (which grafted whatever tree the server was
  launched from onto a fight for an unrelated slug). New
  `plan_ungrounded()` probes no filesystem at all, and every plan whose
  ground-truth is absent now says so with an explicit blocker. [A/B/C]
- boj client: repo_root is canonicalized before being sent to experts —
  they run in their own working directories and panic-attack's contract
  requires an absolute path; if canonicalization fails, the call is
  refused and recorded fail-closed instead of sending a relative path
  that would scan the wrong tree. [line-by-line]
- boj routing is now group-primary: the planner's chosen specialist is
  never rerouted by the obligation (previously a HypatiaFleet+Scan
  escalation would silently summon panic-attack instead of hypatia).
  [line-by-line]
- App: SQUABBLE_PORT that is malformed OR zero refuses to boot (exit 69)
  instead of silently binding the default/an ephemeral port the cartridge
  cannot target; startup prints the actually-bound address. [A + altitude]
- Justfile: `just test` now also compiles/tests the `boj` feature — the
  golden path previously never built the summon subsystem, so a break in
  it would ship silently. [cross-file]

Evidence fidelity (no truncation loss):
- Report gains typed `expert_verdicts: Vec<ExpertVerdict>` (serde default,
  back-compat): every expert call is recorded verbatim — check, cartridge,
  tool, ok (transport AND body), honest meaning, and the full untruncated
  verdict. The escalation's evidence string now carries only the short
  human narration; previously verdicts were spliced into prose and cut at
  400 chars, silently losing scan findings. [altitude]

API/robustness:
- App /api/v1/diagnose accepts both the bare Gate and a {gate: ...}
  wrapper, so callers generalising from /api/v1/fight's shape by analogy
  don't get an opaque 422. [altitude]

Docs/honesty (doctrine #10):
- context.rs no longer claims to live in squabble-cli (it moved to
  squabble-fight); cli main.rs doc now names fight/--summon and the
  client's network surface; app doc no longer claims a non-loopback
  "refusal" guard that never existed (the interface is hardcoded, and the
  doc now says exactly that); README.adoc no longer calls squabble-app a
  stub. [removed-behavior + cross-file]
- boj.rs documents the deliberate, feature-contained reversal of
  fetch.rs's shell-out-over-HTTP-client stance and why. [reuse]

Simplifications:
- structural_scan is a pure producer returning its findings (drops the
  clippy too_many_arguments allow); the summon cfg-pair collapsed to
  inline cfg at the call site; fight's USAGE is shared with the top-level
  help so the two cannot drift; FightRequest.repo_root is Option<PathBuf>.

Refuted in verification (not applied): the cartridge `domain:"ci"` claim
(laminar-mcp uses the identical value; siblings are already heterogeneous)
and all efficiency candidates (honest scale analysis: n≈5 escalations,
loopback, freshness-by-design).

Verified: 43 default + 11 boj tests green; fmt + clippy clean in both
configs; live re-check — ungrounded fight emits the honesty blocker, both
diagnose shapes accepted, SQUABBLE_PORT=0 exits 69.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SUURnQUnzTXYftzWFs5pfZ

Author: claude

Justfile

@@ -130,9 +130,12 @@ clean-all: clean
 # TEST & QUALITY
 # ═══════════════════════════════════════════════════════════════════════════════
 
-# Run all tests
+# Run all tests — including the feature-gated boj expert client, which the
+# plain --workspace run never compiles (the golden path must cover it or a
+# break in `--summon` ships silently).
 test *args:
     cargo test --workspace {{args}}
+    cargo test -p squabble-cli --features boj {{args}}
 
 # Run tests with verbose output
 test-verbose:

README.adoc

@@ -13,11 +13,14 @@ A standalone, per-repo CI/CD fighter that drives a stuck pipeline to a
 *legitimate* green — or improves red→yellow, or fails and hands the human a
 structured report. It *wins* the gate; it never cheats past it.
 
-NOTE: Status — *v0.1 in active development (updated 2026-07-01).* The engine and
+NOTE: Status — *v0.1 in active development (updated 2026-07-03).* The engine and
 its formal invariant are real and verified (see below); live gate fetch via `gh`
-landed in #6. Still open: the `squabble apply` plumbing (moves are proposed, not
-yet executed) and the GitHub App webhook server (honest stub pending the owner's
-hosting decision). See `docs/CHARTER.adoc` for the design and open owner decisions.
+landed in #6; `squabble fight` (classify each red as self-win | escalate |
+assign-owner), the `squabble-app` loopback HTTP backend, and the boj-server
+expert summoning (`--summon`, feature `boj`) landed in #13. Still open: the
+`squabble apply` plumbing (moves are proposed, not yet executed) and the GitHub
+App *webhook* leg (pending the owner's hosting decision). See
+`docs/CHARTER.adoc` for the design and open owner decisions.
 
 == Core principle: squabble ≠ bypass
 
@@ -36,8 +39,9 @@ check away, or any override are provably *not* paths to green.
 [cols="1,3"]
 |===
 | `crates/squabble-core` | The fight engine. Pure, detachable, no hypatia/estate dependency — `diagnose → propose move → apply → verify-satisfied`. The valuable reusable part.
-| `crates/squabble-cli`  | `squabble` — runs locally or as a CI step. `squabble diagnose <gate.json>` today; git/`gh` apply plumbing next.
-| `crates/squabble-app`  | GitHub App (axum webhook + octocrab). *Stub in v0.1.* Delivers the trigger button; calls the same `squabble-core`.
+| `crates/squabble-fight`| The shared host brain: a2ml ecosystem-context reader + workflow ground-truth + the fight planner. One planner for the CLI and the App, so they cannot drift.
+| `crates/squabble-cli`  | `squabble` — runs locally or as a CI step. `fetch`, `diagnose`, and `fight` (with `--summon` under the `boj` feature to put expert verdicts on the report); apply plumbing next.
+| `crates/squabble-app`  | The loopback HTTP backend (axum, `127.0.0.1:7741`): serves the shared planner to the boj-server `cicd-squabbler-mcp` cartridge. The GitHub App *webhook* leg (trigger button + octocrab) is still to come — no webhook route is registered.
 | `spark/`               | The formal heart. `Gate_Machine` + the proved theorem `green ⇔ (non-empty ∧ all passed)`.
 |===
 

crates/squabble-app/src/main.rs

@@ -11,14 +11,22 @@
 //! Surface (all JSON):
 //!
 //! * `GET  /health`          — liveness + version.
-//! * `POST /api/v1/diagnose` — body: a `Gate`; returns the pure engine's
-//!   `Diagnosis` (state + one conservative move per unsatisfied check).
+//! * `POST /api/v1/diagnose` — body: a `Gate` (`{checks:[...]}` — a
+//!   `{gate:{checks:[...]}}` wrapper is also accepted for symmetry with
+//!   `/api/v1/fight`); returns the pure engine's `Diagnosis`.
 //! * `POST /api/v1/fight`    — body: `{slug, gate, repo_root?}`; returns the
 //!   full `Outcome`/`Report` evidence manifest from the shared planner.
+//!   When `repo_root` is absent the fight runs **ungrounded**: no filesystem
+//!   is probed (deliberately not the daemon's own working directory, which
+//!   would mis-attribute evidence) and the report carries an explicit
+//!   "ground-truth absent" blocker.
 //!
-//! Binding is **loopback-only by design** (`127.0.0.1`, port from
-//! `SQUABBLE_PORT`, default 7741): this backend has no auth layer, and the
-//! boj gateway is co-located. It deliberately refuses to bind non-loopback.
+//! The bind address is hardcoded to `127.0.0.1` (only the port is
+//! configurable via `SQUABBLE_PORT`) because this backend has no auth layer
+//! and the boj gateway is co-located; no interface knob is offered, and none
+//! should be added without adding authentication first. A malformed or zero
+//! `SQUABBLE_PORT` refuses to boot (exit 69) rather than silently binding
+//! somewhere the cartridge is not pointing.
 //!
 //! The GitHub App *webhook* leg (`check_run.requested_action` — the
 //! "Squabble!" button — authenticated via octocrab) is still the next step
@@ -43,9 +51,18 @@ struct FightRequest {
     /// The gate: required checks + realised runs (from `squabble fetch` or a host).
     gate: Gate,
     /// Optional path to a local checkout; enables the a2ml context reader and
-    /// workflow ground-truthing. Absent/missing paths are fail-safe (empty
-    /// context, conservative classification).
-    repo_root: Option<String>,
+    /// workflow ground-truthing. Absent → the fight runs ungrounded (no
+    /// filesystem probes) and says so in the report.
+    repo_root: Option<PathBuf>,
+}
+
+/// Accepts both the bare-`Gate` body and a `{gate: ...}` wrapper, so callers
+/// who generalise from `/api/v1/fight`'s shape by analogy don't get a 422.
+#[derive(Debug, Deserialize)]
+#[serde(untagged)]
+enum DiagnoseRequest {
+    Bare(Gate),
+    Wrapped { gate: Gate },
 }
 
 async fn health() -> Json<serde_json::Value> {
@@ -57,44 +74,68 @@ async fn health() -> Json<serde_json::Value> {
     }))
 }
 
-async fn diagnose_handler(Json(gate): Json<Gate>) -> Json<Diagnosis> {
+async fn diagnose_handler(Json(req): Json<DiagnoseRequest>) -> Json<Diagnosis> {
+    let gate = match req {
+        DiagnoseRequest::Bare(g) | DiagnoseRequest::Wrapped { gate: g } => g,
+    };
     Json(diagnose(&gate))
 }
 
 async fn fight_handler(Json(req): Json<FightRequest>) -> Json<Outcome> {
-    let root = PathBuf::from(req.repo_root.unwrap_or_else(|| ".".to_string()));
-    let (_context, outcome) = squabble_fight::plan_at_root(&req.gate, &req.slug, &root);
+    let outcome = match req.repo_root {
+        Some(root) => squabble_fight::plan_at_root(&req.gate, &req.slug, &root).1,
+        // No checkout supplied: plan ungrounded rather than silently probing
+        // whatever directory this daemon happens to have been launched from.
+        None => squabble_fight::plan_ungrounded(&req.gate, &req.slug),
+    };
     Json(outcome)
 }
 
-fn port_from_env() -> u16 {
+/// Parse `SQUABBLE_PORT`. Fail-closed: a value that is set but malformed or
+/// zero (zero would bind an OS-assigned ephemeral port the cartridge cannot
+/// know about) refuses to boot instead of silently using the default.
+fn port_from_env() -> Result<u16, String> {
     match std::env::var("SQUABBLE_PORT") {
-        Ok(v) => v.parse().unwrap_or_else(|_| {
-            eprintln!("squabble-app: SQUABBLE_PORT=`{v}` is not a port; using {DEFAULT_PORT}");
-            DEFAULT_PORT
-        }),
-        Err(_) => DEFAULT_PORT,
+        Ok(v) => match v.parse::<u16>() {
+            Ok(0) => Err(
+                "SQUABBLE_PORT=0 would bind an ephemeral port the cartridge cannot target"
+                    .to_string(),
+            ),
+            Ok(p) => Ok(p),
+            Err(e) => Err(format!("SQUABBLE_PORT=`{v}` is not a port: {e}")),
+        },
+        Err(_) => Ok(DEFAULT_PORT),
     }
 }
 
 #[tokio::main]
 async fn main() {
+    let port = match port_from_env() {
+        Ok(p) => p,
+        Err(e) => {
+            eprintln!("squabble-app: {e} — refusing to boot (fail-closed)");
+            std::process::exit(69); // EX_UNAVAILABLE
+        }
+    };
+
     let app = Router::new()
         .route("/health", get(health))
         .route("/api/v1/diagnose", post(diagnose_handler))
         .route("/api/v1/fight", post(fight_handler));
 
-    // Loopback-only: no auth layer exists, so a wider bind is not offered.
-    let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, port_from_env()));
+    // Loopback-only: the interface is hardcoded, not configurable (no auth
+    // layer exists, so a wider bind is not offered).
+    let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, port));
     let listener = match tokio::net::TcpListener::bind(addr).await {
         Ok(l) => l,
         Err(e) => {
             eprintln!("squabble-app: cannot bind {addr}: {e}");
             std::process::exit(69); // EX_UNAVAILABLE — fail loudly, no fake OK.
         }
     };
+    let bound = listener.local_addr().unwrap_or(addr);
     println!(
-        "squabble-app {} listening on http://{addr} (loopback-only)",
+        "squabble-app {} listening on http://{bound} (loopback-only)",
         env!("CARGO_PKG_VERSION")
     );