fix(#766): retire nav_local with the goal — a zone change no longer publishes the previous zone's fine-tier verdict

crates/eqoxide-command/src/nav.rs

@@ -161,11 +161,12 @@ impl CommandState {
         // throwaway `probe_route_len` field: E0027 in `eqoxide-ipc`, `eqoxide-command` compiled
         // clean, and `request_stop` published the field beside `state: "idle"`.
         //
-        // Behaviour-identical: `retire_to_idle` writes a strict subset of what this list writes for
-        // `idle` (it deliberately keeps `local`, which is cleared just below), and both `idle` call
-        // sites already pass `goal: None`.
+        // Behaviour-identical: `retire_to_idle` writes exactly what this list writes for `idle`, and
+        // both `idle` call sites already pass `goal: None`. (#766 removed the last difference — the
+        // explicit `s.local = None;` that used to sit here because `retire_to_idle` kept `local`.
+        // `retire_to_idle` now retires `local` itself, so keeping the line would only mean two
+        // writers for one field and a comment that has to explain the overlap.)
         if new_state == "idle" {
-            s.local = None;
             s.retire_to_idle(reason);
             return s.goal_id;
         }
@@ -687,4 +688,75 @@ mod tests {
             "the newer goal's coordinates must not be cleared by the older request's backstop");
         assert_eq!(s.reason, None);
     }
+
+    /// **#766 (agent-honesty): the FINE tier's verdict is a per-goal fact, so every command-side
+    /// route to `idle` must retire it too.**
+    ///
+    /// #766 was reported against the zone change, but as with #732 the defect was one writer up:
+    /// `NavStatus::retire_to_idle` deliberately KEPT `local`, so of the six documented routes to
+    /// `idle` the two that reached it without a `Walker::clear_local_plan` on the same tick leaked
+    /// it — the zone change (pinned in `eqoxide-nav`/`eqoxide-net`) and this crate's
+    /// `zone_cross_dropped_unhandled` backstop. `stopped` and `goto_cancelled` did NOT leak it,
+    /// because `stamp_new_goal` carried its own explicit `s.local = None;` line; this PR deleted
+    /// that line as redundant once `retire_to_idle` owns the field, so those two routes now depend
+    /// on the shared writer and are pinned here for that reason.
+    ///
+    /// `no_way_through` is the fixture verdict because `eqoxide_http::observe` filters `threaded`
+    /// out — an unhealthy verdict is the only kind that reaches an agent at all.
+    ///
+    /// **Mutation check:** delete `*local = None;` from `NavStatus::retire_to_idle`
+    /// (`crates/eqoxide-ipc/src/lib.rs`) → all three cases go RED. Restore the deleted
+    /// `s.local = None;` in `stamp_new_goal` on top of that mutation and the first two go green
+    /// again while `zone_cross_unhandled` stays RED — which is the split this test exists to show.
+    #[test]
+    fn every_command_side_retirement_retires_the_fine_tiers_verdict_766() {
+        let stale = || eqoxide_ipc::NavLocal {
+            state: "no_way_through".into(), reason: "search_closed".into(),
+            stuck_ticks: 7, plan_us: 1234,
+        };
+        // The two `stamp_new_goal` routes. NOTE the ordering: `request_goto` itself goes through
+        // `stamp_new_goal`'s NON-idle branch, which clears `local` — so the verdict is planted
+        // AFTER it, or the post-condition would be met before the code under test ever runs.
+        for (label, act) in [
+            ("stop",        Box::new(|cs: &CommandState| { cs.request_stop(); })        as Box<dyn Fn(&CommandState)>),
+            ("cancel_goto", Box::new(|cs: &CommandState| { cs.request_cancel_goto(); }) as Box<dyn Fn(&CommandState)>),
+        ] {
+            let cs = CommandState::default();
+            cs.request_goto((2216.0, 579.0, -113.0));
+            cs.nav.nav_state.lock().unwrap().local = Some(stale());
+            assert_eq!(cs.nav.nav_state.lock().unwrap().local, Some(stale()),
+                "{label}: PREMISE — the fine tier's verdict really is loaded, so the assertion \
+                 below cannot pass on `NavStatus::default()`");
+
+            act(&cs);
+
+            let s = cs.nav.nav_state.lock().unwrap();
+            assert_eq!(s.state, "idle", "{label}: the route ends at idle");
+            assert_eq!(s.local, None,
+                "{label}: #766 — the fine tier's last word is its verdict on threading toward the \
+                 goal that just ended. Beside `idle` it is a live-looking answer about work that is \
+                 over, and after a zone change it describes a corridor in a world we have left.");
+        }
+
+        // The #725 backstop: a one-shot cross drained and never answered. Same ordering hazard —
+        // `request_zone_cross` also stamps through the non-idle branch — so the verdict is planted
+        // while the ticket is HELD, which is also exactly when the fine tier would have produced
+        // one (the walker keeps ticking while the drain runs). Planting it does not touch
+        // `state`/`reason`/`goal_id`, so the ticket's `moved` guard still sees an unanswered request.
+        let cs = CommandState::default();
+        cs.request_zone_cross(30);
+        let ticket = cs.take_zone_cross().expect("the request was queued");
+        cs.nav.nav_state.lock().unwrap().local = Some(stale());
+        assert_eq!(cs.nav.nav_state.lock().unwrap().local, Some(stale()),
+            "zone_cross_unhandled: PREMISE — the verdict is loaded before the backstop fires");
+        drop(ticket);
+
+        let s = cs.nav.nav_state.lock().unwrap();
+        assert_eq!(s.state, "idle");
+        assert_eq!(s.reason.as_deref(), Some(NAV_REASON_ZONE_CROSS_UNHANDLED),
+            "PREMISE: the backstop really did fire — otherwise the `local` assertion below would be \
+             about a row nothing retired");
+        assert_eq!(s.local, None,
+            "#766: the sixth route to `idle` retires the fine verdict like the other five");
+    }
 }

crates/eqoxide-http/src/observe.rs

@@ -914,6 +914,26 @@ async fn get_debug(State(s): State<HttpState>) -> Json<serde_json::Value> {
         // clock" with no way to ask which, and `nav_state` said a confident `navigating` throughout.
         // The clock is gone; the ambiguity went with it.
         "nav_local": nav_local,
+        // WORKER-scoped fine-planner liveness (#766 review B3; scope corrected from "session" by
+        // round-6 review B12 — the latch is cleared by `Walker::new` as it spawns a replacement, and
+        // it reads as session-scoped from outside only because exactly one fine worker is built per
+        // process — a premise nothing in the tree pins, #787. `docs/http-api.md` keeps the
+        // agent-facing "session-scoped" name and says why).
+        // `nav_local` above is a PER-GOAL
+        // verdict and #766 retires it with the goal — correct for `no_way_through` / `exhausted`, and
+        // wrong for `planner_dead`, which is a latched client fault, not a statement about a goal.
+        // Carried here it survives every retirement, so an agent BETWEEN goals — which is when it
+        // polls this endpoint to decide what to do next — can still see that its steering has
+        // degraded to the coarse 8u route and that nothing on any nav route recovers it (a claim
+        // about WRITERS, which is what the tree guarantees; "permanently" was strictly stronger and
+        // false in the same breath as this block's own "the lifetime is the WORKER's" — round-6
+        // review B14). `planner_dead` still appears in `nav_local`
+        // while a route is committed; this is the channel that does not vanish when the route does.
+        //
+        // Always present, in BOTH states, unlike the `null`-when-healthy fields around it: checking
+        // your own health needs a readable "alive", not merely the absence of "dead" — which is
+        // indistinguishable from an older client that never had the field.
+        "nav_local_planner_dead": nav.local_planner_dead,
         // The agent-honesty payload behind a terminal `no_path` (#378 Phase 2). `null` when there is
         // nothing to report. `goal` (if present) is the DEFINITIVE "your goal itself cannot be stood
         // at"; `frontier` is the hazard at the search's CLOSEST APPROACH — one blocking fact, named
@@ -2943,6 +2963,114 @@ mod tests {
              `idle` to the goal it asked for; only the per-goal FACTS are retired");
     }
 
+    /// **#766 (agent-honesty) — the OBSERVER half of the fine tier's retirement.**
+    ///
+    /// The sibling of the test above, for the field #732 left standing. `nav_local` is read off the
+    /// same cloned `NavStatus` and passed through exactly one filter —
+    /// `.filter(|l| l.state != "threaded")` — so an UNHEALTHY verdict reaches the response body
+    /// verbatim, and an unhealthy verdict is the only kind an agent can ever see. (Kept whole on one
+    /// line: a code span broken across a `///` wrap renders the break inside the span and is
+    /// un-greppable — #773, round-6 review B15.)
+    /// That makes `no_way_through` the right fixture
+    /// and makes the first half of this test a real premise: it asserts that the planted verdict
+    /// genuinely publishes, so the `null` below is the retirement's doing and not the filter's.
+    ///
+    /// Mutation check: delete `*local = None;` from `NavStatus::retire_to_idle`
+    /// (`crates/eqoxide-ipc/src/lib.rs`) → the `nav_local` assertion goes RED with the previous
+    /// zone's `no_way_through` object in the diff.
+    #[tokio::test]
+    async fn debug_publishes_no_nav_local_once_the_goal_is_retired_to_idle_766() {
+        let state = empty_state();
+        {
+            let mut s = state.nav.nav_state.lock().unwrap();
+            s.goal_id = 4;
+            s.state   = "navigating".into();
+            s.local   = Some(eqoxide_ipc::NavLocal {
+                state: "no_way_through".into(), reason: "search_closed".into(),
+                stuck_ticks: 7, plan_us: 1234,
+            });
+        }
+        let v = debug_json(state.clone()).await;
+        assert_eq!(v["nav_local"]["state"], serde_json::json!("no_way_through"),
+            "PREMISE: an un-retired UNHEALTHY fine verdict is published — the harm is reachable by a \
+             reader of GET /v1/observe/debug, not merely resident in memory");
+        assert_eq!(v["nav_local"]["stuck_ticks"], serde_json::json!(7),
+            "PREMISE: and the whole object comes through, not just a state string");
+
+        // The zone-change retirement, which is where #766 was reported.
+        state.nav.nav_state.lock().unwrap().retire_to_idle(Some("zoned"));
+
+        let v = debug_json(state).await;
+        assert_eq!(v["player"]["nav_state"], serde_json::json!("idle"));
+        assert_eq!(v["player"]["nav_reason"], serde_json::json!("zoned"));
+        assert_eq!(v["nav_local"], serde_json::json!(null),
+            "#766: `no_way_through` beside `idle`/`zoned` describes a corridor in the zone the \
+             reader has LEFT, computed against a collision grid that no longer exists — the fine \
+             tier's verdict is about threading toward a goal, so it retires with the goal");
+    }
+
+    /// **#766 review B3 — the worker-scoped fault must NOT retire with the goal.**
+    ///
+    /// The counterweight to the test above, and the reason this PR did not simply narrow a sentence.
+    /// `planner_dead` is the third publishable `nav_local.state`, and it is not a verdict about a
+    /// goal: it is a latched client fault meaning steering has degraded to the coarse 8 u route
+    /// with nothing on any nav route to recover it. Retiring `nav_local` on every `idle` — which is #766's whole point — therefore hid
+    /// it from an agent BETWEEN goals, which is precisely when an agent polls this endpoint. So the
+    /// fault moved to its own field and this measures the split at the JSON surface, where an agent
+    /// actually reads it: after the same retirement, the per-goal verdict is `null` and the
+    /// worker-scoped fault is still `true`. (The field's lifetime is the fine WORKER's, not the
+    /// session's — round-6 review B12. Nothing in this test turns on the difference; it retires a
+    /// goal, and retiring a goal does not replace a worker. What it would catch is a clear placed on
+    /// a retirement route, which is the defect it was written for.)
+    ///
+    /// It also pins the always-present shape. A `null`-when-healthy field would make "alive"
+    /// indistinguishable from "this client is too old to have the field", and a health check you
+    /// cannot distinguish from a missing feature is not a health check.
+    ///
+    /// Mutation check, RUN: clear `local_planner_dead` in `NavStatus::retire_to_idle` instead of
+    /// keeping it → the final assertion here goes RED (`246 passed; 1 failed` in this crate), and
+    /// `eqoxide-nav` goes red separately on its own row-level assertion. Named by assertion, not by
+    /// line number, deliberately (review B8): a line locator drifts on the next edit above it. The
+    /// always-present shape is pinned by the first assertion but NOT mutation-checked — omitting a
+    /// key from a `json!` literal is a shape change, not a behaviour one, and I did not run it.
+    #[tokio::test]
+    async fn debug_keeps_publishing_a_dead_fine_planner_after_the_goal_is_retired_766() {
+        let state = empty_state();
+        let v = debug_json(state.clone()).await;
+        assert_eq!(v["nav_local_planner_dead"], serde_json::json!(false),
+            "PREMISE: liveness is published in BOTH states — a healthy client says so out loud, so \
+             the `true` below is a change this test caused and not a key that only ever appears \
+             when something is wrong");
+
+        {
+            let mut s = state.nav.nav_state.lock().unwrap();
+            s.goal_id = 4;
+            s.state   = "navigating".into();
+            s.local   = Some(eqoxide_ipc::NavLocal {
+                state: "planner_dead".into(), reason: "local_planner_dead".into(),
+                stuck_ticks: 0, plan_us: 0,
+            });
+            s.local_planner_dead = true;   // what `Walker::latch_local_planner_liveness` writes
+        }
+        let v = debug_json(state.clone()).await;
+        assert_eq!(v["nav_local"]["state"], serde_json::json!("planner_dead"),
+            "PREMISE: while a route is committed the fault is visible in BOTH channels, so the \
+             assertions below measure which one survives rather than which one exists");
+        assert_eq!(v["nav_local_planner_dead"], serde_json::json!(true));
+
+        state.nav.nav_state.lock().unwrap().retire_to_idle(Some("zoned"));
+
+        let v = debug_json(state).await;
+        assert_eq!(v["nav_local"], serde_json::json!(null),
+            "#766 is unchanged by B3: the per-goal channel still retires with the goal. The \
+             liveness field is an addition, not a hole in that guarantee");
+        assert_eq!(v["nav_local_planner_dead"], serde_json::json!(true),
+            "#766 B3: the worker thread does not come back — recovering it needs a client restart — \
+             so an agent between goals must still be able to read that its steering has degraded. \
+             Clearing this on retirement would report a recovery that never happened, which is the \
+             agent-honesty defect class #766 exists to close, not one it may create");
+    }
+
     /// #471 (agent-honesty): the server placed two Mobs (consecutive spawn_ids, e.g. 526/527) at a
     /// byte-identical position; the wire disambiguates their names with a numeric suffix
     /// ("Geeda"/"Geeda00"), so in the name-keyed roster they survive as TWO entries. The observe