fix(#817): serve player.hold from GET /v1/observe/debug

[djhenry]

Closes #817.

The defect

player.hold was computed by the movement controller, mirrored into GameState on every net
tick by ActionLoop::stream_position (that function's own rustdoc: "Runs every tick"), from a
ControllerView snapshot the render thread republishes on every rendered frame — so the mirror
is as fresh as the last published frame, not as fresh as the last net tick — and covered by
unit tests since #724 landed — and reached no HTTP
response body anywhere in the workspace. PlayerState is an internal projection; get_debug
in crates/eqoxide-http/src/observe.rs hand-builds its player object with a serde_json::json!
literal and patches extras in via player.insert(...) — that mechanism is how levitating,
run_mode, and afloat_stall reach a client. Nothing did that for hold. An agent that greps
/v1/observe/debug for hold found nothing and could only conclude it was not stuck — a confident
falsehood produced by a 200.

The fix

One player.insert("hold".into(), serde_json::json!(player_hold)) in observe::get_debug,
next to the existing levitating/run_mode/afloat_stall inserts, following the exact same
"always present, explicit null when healthy" contract. crates/eqoxide-http/src/observe.rs,
crates/eqoxide-http/src/lib.rs.

⚠️ Live observation, not a test

Per the issue's acceptance bar: the workspace suite was already green with the field unreachable,
so green tests are not evidence here, and a test that does serde_json::to_value(&player_state)
proves nothing about what a client can actually receive.

Launched the built PR binary on my allocated character/port (config-fayverify.yaml,
API port 8796) and hit the real, running GET /v1/observe/debug. The character turned out to
already be genuinely embedded in world geometry (unrelated pre-existing state, not staged for this
PR), so this is an authentic non-null hold from a live client, not a synthetic fixture:

{
  "camera": {
    "azimuth_deg": 90.0,
    "elevation_deg": 20.0002384185791,
    "focus": [-2190.5, 902.125, 2.75],
    "mode": "auto_follow",
    "radius": 80.0
  },
  "common_assets_failed": null,
  "frame_profile": {
    "egui_ms": 0.0, "frame_ms": 0.0, "render_ms": 0.0, "scene_ms": 0.0,
    "smooth_ms": 0.0, "submit_ms": 0.0, "total_ms": 0.0, "update_ms": 0.0
  },
  "last_consider": null,
  "model_sync_dead": null,
  "nav_blocked_by": null,
  "nav_declined_pads": null,
  "nav_goal": null,
  "nav_goal_id": 0,
  "nav_local": null,
  "nav_local_planner_dead": false,
  "nav_support": null,
  "nav_tier": null,
  "nav_tight": null,
  "net_thread_dead": null,
  "player": {
    "afloat_stall": null,
    "casting": null,
    "class": "Ranger",
    "coin_verified": true,
    "connected": true,
    "crossing_pending_ms": null,
    "currency": { "copper": 0, "gold": 0, "platinum": 0, "silver": 0 },
    "dead": false,
    "died_ago_secs": null,
    "guild": "",
    "guild_id": 0,
    "guild_rank": 0,
    "heading_ccw": 180.0,
    "heading_cw": 180.0,
    "hold": {
      "detail": "the character is EMBEDDED in world geometry. The client's push-out search found nowhere it can legally stand, and it has no recovery position to fall back to (a position discontinuity — a summon, a large server correction — supersedes that history, #724). Physics is frozen: every movement command will be accepted and produce NO motion, in any direction. This will not clear on its own — the client keeps streaming this position and the server agrees with it, so no further correction is coming. Ask a GM to move the character (#goto/#summon), or zone out.",
      "held_secs": 41.995487213134766,
      "reason": "embedded_no_recovery"
    },
    "hp": 97,
    "hp_max": 97,
    "hp_pct": 100.0,
    "killed_by": null,
    "last_cast": null,
    "last_packet_age_ms": 1304,
    "last_send_error": null,
    "last_send_error_age_ms": null,
    "last_world_response_ms": 1304,
    "level": 4,
    "levitating": false,
    "link_age_ms": 1304,
    "mana": 60,
    "mana_max": 60,
    "mana_pct": 100.0,
    "name": "<redacted>",
    "nav_reason": "zoned",
    "nav_state": "idle",
    "pos": [-2190.5, 902.125, 2.75],
    "position_provisional": false,
    "race": "ELF",
    "reliable_abandoned": 0,
    "run_mode": true,
    "send_deferred": 0,
    "send_failures": 0,
    "send_failures_unretried": 0,
    "send_starved": false,
    "send_wouldblock_rescued": 0,
    "server_corrections": 0,
    "session_drop": null,
    "snapshot_age_ms": 5,
    "spawn_id": 433,
    "target_attitude": null,
    "target_con": null,
    "target_hp_pct": null,
    "target_id": null,
    "target_level": null,
    "target_name": null,
    "world_responsive": true,
    "xp_pct": 100.0,
    "zone": "steamfont",
    "zone_in_failed": false
  },
  "zone_assets": {
    "collision_loaded": true,
    "detail": "terrain meshes are uploaded and the collision grid is built: what the client reports about this zone's geometry is the real zone.",
    "player_zone": "steamfont",
    "reason": null,
    "state": "ready",
    "status": null,
    "terrain_meshes": 24,
    "zone": "steamfont"
  },
  "zone_cross_best_effort": null,
  "zone_cross_stopped": null
}

player.hold is present, non-null, and correct: reason: "embedded_no_recovery", held_secs
advancing (measured 41.995487213134766s at capture time — I polled twice a few seconds apart and
watched it advance in wall-clock proportion, confirming the render loop is live and stepping, not
a frozen stale value), and detail naming the actual predicament. player now carries 57 keys
(was 55 in the issue's original measurement — the +2 is hold plus afloat_stall, which landed on
the same review chain slightly after that count was taken). Client exited cleanly via
POST /v1/lifecycle/exit; port 8796 confirmed free afterward.

What this establishes and what it does not: this proves hold is reachable, present, and
correctly valued through the real router on a real running client, for the embedded_no_recovery
case observed live. It does not itself fix, and is not claimed to fix, the fact that this
particular character is currently stuck — that is unrelated pre-existing state on my allocated
test character and is out of scope for this issue.

Test changes

• New hold_reaches_the_debug_json_817 in crates/eqoxide-http/src/observe.rs, modeled directly
  on afloat_stall_reaches_the_debug_json_801: drives the real router via debug_json (not
  serde_json::to_value on the struct), checks the key is present-and-null when healthy, and
  checks both ControllerHoldReason variants round-trip correctly (reason, held_secs,
  detail).
• Rewrote the tail of afloat_stall_reaches_the_debug_json_801. Corrected in review round 2 —
  the original wording here inverted which assertion was vacuous. The check that had been
  measured vacuous (fix(#801): publish the trapped-swimmer stall to the agent, not to the log #810 round 2) was the ORIGINAL assert!(v["player"]["hold"].is_null()): an
  absent key and an explicit null both read as Value::Null through serde_json, and hold was
  not a key in this body at all, so that assertion could never fail. What fix(#801): publish the trapped-swimmer stall to the agent, not to the log #810 replaced it with —
  assert!(!player.contains_key("hold"), …) — was a deliberate, non-vacuous tripwire set to go
  RED exactly when player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817 landed; its message said so verbatim ("this test deliberately trips on it,
  because the claim it exists to pin only becomes checkable now"). It worked as designed, and this
  PR is what trips it. Replaced with contains_key("hold") (must be present) + is_null() (a
  stall alone must not read as a hold).

Mutation check (remote builder, -p eqoxide-http --lib, restored from an md5sum-verified copy
between runs):

• Control: 270 passed; 0 failed (up from the pre-PR baseline of 269 passed — the new test).
• Deleted the player.insert("hold".into(), …) line from get_debug → 268 passed, 2 failed:
  hold_reaches_the_debug_json_817 (at the contains_key assertion, exactly as designed) and
  afloat_stall_reaches_the_debug_json_801 (at its rewritten disjointness check, which also now
  depends on hold being served). Restored the file and verified the restore against the
  pre-mutation md5sum before re-running.

Full workspace suite

rbuild <worktree> test --workspace --locked --no-fail-fast:

• Finished \test` profile [unoptimized + debuginfo] target(s) in 24m 04s` (compile sentinel,
  not the run time — the run itself was fast; the box had a cold cache for this worktree).
• running [0-9]+ tests? headers: 55. test result: lines: 55. Equal — no lost binary.
• Reconciled by name: 1839 passed + 0 failed + 47 ignored = 1886, matching the sum of every
  test result: line.
• Zero test result: FAILED lines; zero error[ lines.

Sweep — other PlayerState fields with the same shape

Checked every field on PlayerState against what get_debug and the other /v1/observe/* and
/v1/trainer/* routes actually read off it:

Field	Reached via
mem_spells	GET /v1/observe/spells
skills	GET /v1/observe/skills
trainer_open, trainer_skills	GET /v1/trainer/list (open/skills)
book_text	GET /v1/observe/item_text
last_consider	top-level last_consider in GET /v1/observe/debug
everything else in the player.insert/literal list	GET /v1/observe/debug's player object

Found no other field with the "maintained + tested + unreachable" shape. hold was the one
gap. Not expanding this PR's scope beyond it.

Prose audit — doc/comment claims that hold is observable/served

Grepped the tracked tree for player.hold, player_hold, PlayerHoldView, and GET /v1/observe
near hold. Found and fixed:

• crates/eqoxide-http/src/lib.rs — PlayerState::hold's doc claimed "No skip_serializing_if:
  the key is ALWAYS present" without qualifying serialization of what — true only of a direct
  serialization of the struct, which nothing in the tree does. Rewrote to state the distinction
  explicitly and point at this PR's insert as what actually makes the key reachable.
• crates/eqoxide-http/src/lib.rs — PlayerHoldView's doc named player.hold (#724) without
  saying it was served anywhere. Added the "served as player.hold by GET /v1/observe/debug"
  sentence, matching the pattern already used on PlayerAfloatStallView.
• src/movement.rs — two comments cited the route as bare GET /v1/observe, which is not a
  registered route (confirmed 404 in the issue). Corrected both to GET /v1/observe/debug.
• docs/http-api.md — the hold section's "The key is always present" sentence had no route
  scoping (afloat_stall's equivalent sentence does). Scoped it to GET /v1/observe/debug's
  player object, matching the afloat_stall pattern, and noted player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817 as what made it true.
• docs/http-api.md — two afloat_stall caveats explicitly said "hold is not currently served
  by any handler — player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817" / "not served by any handler today — player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817". Both are now stale (that's
  what this PR fixes); updated them to state the current, corrected relationship instead of
  deleting the history outright.

I did not find any doc/comment asserting hold was reachable that turned out to still be false
after this PR — the remaining player.hold/player_hold references I checked (in
crates/eqoxide-core/src/game_state.rs, crates/eqoxide-ipc/src/lib.rs,
crates/eqoxide-net/src/action_loop.rs, src/zone_in.rs) either cite the correct
GET /v1/observe/debug route already or don't make a reachability claim at all (internal
plumbing comments).

Not established by this PR

• This does not audit fields outside PlayerState for the same shape (out of scope per the
  issue).
• This does not fix the fact that my test character was found already stuck
  (embedded_no_recovery) — that is pre-existing state on my allocated account, unrelated to this
  change, and not investigated further here.

Do not merge — awaiting independent review per the agent-fleet process.

[djhenry]

Paused at wind-down — the first independent review never got underway.

A reviewer was dispatched against 4f19b5b and was stopped very early, before producing
any finding. This PR has had no independent review at all. It needs a full fresh
adversarial pass, not a resumption.

The branch is clean and unchanged; a reviewer mutation left in a separate scratch worktree
(observe.rs, the player.insert("hold", …) line deleted) has been reverted, so nothing
is contaminated.

[djhenry]

BLOCKING FINDINGS — first independent review of 4f19b5b

Full first-pass adversarial review; this PR had had no independent review of any kind. I did not
write it.

The code change is right, and I could not break it. The one-line player.insert is correct, the
regression test is genuinely at the API boundary, and I reproduced your mutation check independently.
One blocking finding, and it is a false mechanism claim about the publication path — introduced by
this PR, in two tracked files, and it misstates precisely the staleness property that makes this
field safe to trust.

---

BLOCKING

B1 — "mirrored into GameState every controller-stepped frame" is false, in two tracked files, and it hides the field's real staleness mode

Introduced text:

• crates/eqoxide-http/src/lib.rs:248-249 — "player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817 found this field computed, mirrored into
  GameState every controller-stepped frame, and covered by tests…"
• crates/eqoxide-http/src/observe.rs:3194-3196 (rustdoc of hold_reaches_the_debug_json_817) —
  "…mirrored into GameState every controller-stepped frame (ActionLoop::stream_position)…"

The second is the worse one, because it names the function — and that function contradicts it.
ActionLoop::stream_position is a net-thread function, and its own rustdoc, three lines above
the write, says (crates/eqoxide-net/src/action_loop.rs:2917-2921):

Stream the render controller's authoritative position to the server at native cadence
(design §2/§3.4). Runs every tick (not gated by the 150 ms planner).

and the write itself (:2923, :2934) is a copy of a published snapshot, not a controller step:

let view = *self.controller.controller_view.lock().unwrap();
…
(gs.player_hold, gs.player_afloat_stall) = view.disclosures();

The tree already says this correctly in two other places, both of which the new sentences now
contradict:

• src/movement.rs:3598 — "re-mirrored into gs.player_hold on every net tick for the whole
  ~10 s load."
• the inline comment at the write site itself — "the view is republished every render frame, so
  this write is also the clear."

So there are two cadences — the render thread republishes the view per rendered frame, the net
thread copies it per net tick — and the new sentences collapse them into one that matches neither.

Why this is not cosmetic

It is exactly the property the honesty story for this field rests on. "Mirrored every
controller-stepped frame" tells a reader the served value can be at most one controller step old.
What actually happens is that the net thread re-copies whatever the render thread last published,
every tick — so when the render loop idles, gs.player_hold keeps being re-written with a stale view
and looks freshly maintained while describing a frame of unbounded age.

That is not a hypothetical: it is the mode PlayerHoldView's own rustdoc (lib.rs:578-588) devotes a
bullet to ("While the render loop is idle it does not render at all, so nothing recomputes") and
that docs/http-api.md:395-397 gives a detection procedure for. A reader who believes the new
sentence has no reason to think that bullet applies. The new prose weakens a correct existing
disclosure, which is the inverse of what #817 is for.

Suggested replacement (both sites)

mirrored into GameState on every net tick by ActionLoop::stream_position, from a
ControllerView snapshot the render thread republishes on every rendered frame — so the mirror is
as fresh as the last published frame, not as fresh as the last net tick

The same sentence appears in the PR body ("mirrored into GameState on every controller-stepped
frame") and should be corrected there too, since it is the handoff a future agent reads.

---

NON-BLOCKING

N1 — the code comment's absent-vs-null claim is stronger than this PR's own measurement supports

crates/eqoxide-http/src/observe.rs, the new comment above the insert:

an omitted key would read as "this client is too old to report the state", not "no hold in force".

Stated flatly, that is not true of the ordinary access path, and this PR contains the measurement
that refutes it — the test comment ~30 lines below records #810 round 2's finding that serde_json
returns Value::Null for an absent key, which is why the old assert!(v["player"]["hold"].is_null())
was vacuous and why your new test had to use contains_key. For an agent doing
resp["player"]["hold"] — the obvious thing to write — an omitted key reads as exactly "no hold
in force". Only a presence check or a raw-text grep distinguishes them.

docs/http-api.md:384-386 gets this right ("an agent that greps that response for hold"). The
code comment should carry the same scoping — one clause: "…to an agent that checks key presence or
greps the raw body; note that v["player"]["hold"] cannot tell an absent key from an explicit null,
which is what made the original is_null() assertion vacuous."

N2 — the doc now asserts twice that /v1/observe/state does not exist, while a comment in the file this PR edits still cites it

New text at docs/http-api.md:387-388 (mirroring the pre-existing afloat_stall sentence at :471):

there is no bare GET /v1/observe or /v1/observe/state route to carry it

But crates/eqoxide-http/src/observe.rs:1397 still tells a reader:

the SAME convention as heading_ccw on GET /v1/observe/state

Pre-existing, and outside your stated grep (you searched GET /v1/observe near hold, and this
is a camera-yaw comment), so I am not charging it to this PR. But the assertion is now made twice in
the doc against a live stale citation, and it is the same defect class as the GET /v1/observe →
GET /v1/observe/debug corrections you did make in src/movement.rs. Worth a follow-up issue rather
than scope creep here.

N3 — the PR body misdescribes which assertion was vacuous (body only; the tracked comment is right)

Body:

Rewrote the tail of afloat_stall_reaches_the_debug_json_801, which had asserted
!player.contains_key("hold") — a check … had already measured vacuous

That inverts it. The vacuous check was the original assert!(v["player"]["hold"].is_null()).
!contains_key("hold") was the deliberate non-vacuous tripwire #810 installed to go red exactly
when #817 landed — its removed message says so verbatim ("this test deliberately trips on it,
because the claim it exists to pin only becomes checkable now"). It worked as designed. The test's
own in-tree comment describes this correctly; only the PR body is wrong. Flagging it because the body
is what the next agent reads.

---

What I verified, and could not break

• Mutation check — reproduced independently. Deleted the player.insert("hold".into(), …) line
  from get_debug on the remote builder, test -p eqoxide-http --locked --no-fail-fast:
  test result: FAILED. 268 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out, failing at
  observe.rs:3212 (hold_reaches_the_debug_json_817) and observe.rs:3180
  (afloat_stall_reaches_the_debug_json_801). Exactly your 268/2. One Compiling eqoxide-http
  line, so not a cached artifact. File restored from a cp -p snapshot and touched; md5
  56e9c3ad… matches pre-mutation, git status clean, diff vs 4f19b5b empty. No git stash, no
  git restore.
• The test is genuinely at the boundary. debug_json (observe.rs:2130-2135) does
  router().with_state(state) → oneshot(Request::get("/debug")) → to_bytes →
  serde_json::from_slice. Real router, real request, real bytes — not serde_json::to_value on the
  struct. That is the thing player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817 said a test must do, and it does it. Both ControllerHoldReason
  variants are exercised.
• "Always present" holds structurally. The insert sits inside
  if let Some(player) = out.get_mut("player").and_then(|p| p.as_object_mut()), and out is built
  from a json! literal that always contains a player object, so the branch is always taken. There
  is no early return in get_debug that skips it. serde_json::json!(None::<PlayerHoldView>) renders
  an explicit null. Confirmed at source and by the empty_state() leg of the new test asserting
  contains_key + is_null through the router.
• Live writer, and it is also the clear. (gs.player_hold, gs.player_afloat_stall) = view.disclosures(); is an unconditional destructuring assignment, so a None view withdraws the
  hold on the next tick — it cannot latch. Second clear in GameState::begin_zone_in
  (game_state.rs:1424). This is not the connected/last_packet_age_ms are published ONLY by the render loop, which sleeps when the world dies — a dead connection reports connected:true forever #343 no-live-writer shape. (B1 is about how the cadence
  is described, not about the writer's existence.)
• Doc blast radius on eqoxide-net — covered, and I checked it rather than trusting it.
  crates/eqoxide-net/src/action_loop.rs:4840 does
  include_str!("../../../docs/http-api.md") and parses the idle nav_state row. Your doc edits
  are at :384-390, :439 and :486-490; the pinned nav_state table is at :265-270, untouched.
  Ran test -p eqoxide-http -p eqoxide-net --locked --no-fail-fast on 4f19b5b:
  Finished `test` profile [unoptimized + debuginfo] target(s) in 34.79s; 270 passed; 0 failed
  (http), 383 passed; 0 failed (net), plus two empty targets; 4 ^running [0-9]+ tests?$
  headers vs 4 test result: lines (equal — no lost binary); 0 FAILED.
• Doc shape matches what is served. docs/http-api.md:344-350 documents exactly
  {reason, held_secs, detail}; PlayerHoldView serialises exactly those three with no renames, and
  your live capture shows the same three. This is not the Two docs tell an agent to read pos_up; the API serves pos as an array and has no such key #822 pos_up shape — I looked for it
  specifically. docs/http-api.md:440 even carries the pos_east/pos_north/pos_up disclaimer from
  that fix, still accurate.
• Your PlayerState sweep — reproduced, no new gap. Extracted all 44 pub fields from
  PlayerState and checked each name against every quoted JSON key in crates/eqoxide-http/src.
  Eleven names never appear: position_provisional_since, mem_spells, trainer_open,
  trainer_skills, player_id, coin, cur_hp, max_hp, cur_mana, max_mana, book_text.
  Every one is either renamed at the boundary (player_id → spawn_id at observe.rs:1013;
  position_provisional_since → position_provisional at :684; cur_hp/max_hp → hp/hp_max;
  coin → currency) or served by another route exactly as your table says. hold really was the
  only one with the maintained-tested-unreachable shape.

---

What I could NOT establish — stated rather than skipped

• I did not run the full workspace suite. I ran the two crates in the blast radius
  (eqoxide-http, eqoxide-net) because that is where every finding of mine lives. So I am not
  independently confirming your 55/55, 1839 + 0 + 47 = 1886 workspace figures — I did not measure
  them and will not repeat them as though I had.
• No independent live client run. Your capture is internally consistent (the detail string
  matches PlayerState::from_game_state's EmbeddedNoRecovery arm verbatim, and the three keys match
  PlayerHoldView), but I did not reproduce it on my own character and port, so the E2E leg of the
  gate is not discharged by me.
• One open honesty question I could not settle, and it is worth its own issue.
  PlayerHoldView's rustdoc (lib.rs:585-588, pre-existing, untouched here) claims a universal:
  "a held body cannot be freed without a stepped frame either, so idling cannot manufacture a false
  hold… a non-null hold is never stale-because-idle in the sense of describing a predicament
  the body has left." But the documented remedy for a hold is a GM #summon, which arrives as a
  server position correction on the net thread, while the hold is only cleared at the top of
  CharacterController::step on the render thread. If the render loop can be idle when a summon
  lands, the body is freed and hold keeps reporting "the character is EMBEDDED… ask a GM to move
  the character" — which would falsify the "never". Reasoned from source, not run — I did not
  build a repro and I am not claiming the bug exists. It is pre-existing prose this PR does not touch,
  so it is not a finding against fix(#817): serve player.hold from GET /v1/observe/debug #844; but player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817 is what makes those sentences reachable by an agent
  for the first time, and per this project's rule a passing live run cannot discharge a "never"
  anyway. Recommend filing it.

---

Summary. One blocking finding, B1 — a false mechanism claim in crates/eqoxide-http/src/lib.rs
and crates/eqoxide-http/src/observe.rs that misstates the mirror's cadence and thereby understates
the field's staleness mode, contradicting action_loop.rs:2917-2921, src/movement.rs:3598 and the
write site's own comment. Two-site prose fix; no code change needed. N1–N3 non-blocking. The fix
itself, the test, and the mutation discrimination are all sound and I could not break any of them.

Not approving, not merging — posting as a comment (the fleet shares one identity and self-approval is
blocked).

[djhenry]

Round 2 applied — head ad822f5

Merged origin/main (7cd9652) in first; no rebase, no force-push. Comment-only, as you found:
the code was not the problem.

Mechanical proof that nothing but comments changed

$ git diff -U0 HEAD~1 | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' \
    | sed -E 's/^[+-]//' | grep -vE '^[[:space:]]*(///|//!|//)' | grep -cvE '^[[:space:]]*$'
0

Zero added-or-removed lines that are not a ////// comment or blank. Files:
crates/eqoxide-http/src/lib.rs (+9/-2) and crates/eqoxide-http/src/observe.rs (+26/-8).

B1 — the cadence claim

Verified your evidence at source before changing anything: action_loop.rs:2918 — "Runs every
tick (not gated by the 150 ms planner)"; :2923/:2934 — the write is
(gs.player_hold, gs.player_afloat_stall) = view.disclosures(); over
*self.controller.controller_view.lock().unwrap(); the write site's own comment — "the view is
republished every render frame"; publish_disclosures is called from src/app.rs:1424 and
:1758, both on the app/render side. Two cadences, confirmed.

Both sites now read (lib.rs, and the hold_reaches_the_debug_json_817 rustdoc):

mirrored into GameState on every net tick by ActionLoop::stream_position (that function's
own rustdoc: "Runs every tick"), from a ControllerView snapshot the render thread
republishes on every rendered frame — so the mirror is as fresh as the last published frame,
not as fresh as the last net tick

The lib.rs one additionally says outright that an idle render loop leaves the net thread
re-copying a stale view, and points at PlayerHoldView's staleness bullet "which this sentence
must not be read as softening" — because your finding was that the new prose weakened a correct
existing disclosure, and a replacement that merely stops being false would still leave the reader
with no pointer to the disclosure it had been undercutting.

Same correction applied to the get_debug insert-site comment, which said "every tick" — not
false, but the only remaining site not agreeing with src/movement.rs:3598's "every net tick". It
now says "every NET tick (from a view the render thread republishes per rendered frame)".

The PR body carried the same sentence and has been corrected too.

N1 — absent vs null

The comment now scopes the claim to what this PR measured: an omitted key reads as "this client is
too old to report the state" only to a reader that checks key presence or greps the raw body,
because serde_json returns Value::Null for an absent key exactly as for an explicit one — which
is what made the original is_null() assertion vacuous (#810 round 2) and why the test has to use
contains_key. It names docs/http-api.md as saying it the same way.

N2 → the PR body's inverted vacuity claim

You are right and I confirmed it from the diff rather than from the prose. git show 4f19b5b
removes:

-        assert!(!player.contains_key("hold"),
-             this test deliberately trips on it, because the claim it exists to pin only becomes \

So !contains_key was #810's deliberate tripwire, set to go red exactly when #817 landed, and
it worked. The vacuous one was the original assert!(v["player"]["hold"].is_null()). The body now
says that, explicitly labelled as a round-2 correction so the squash-merged commit message carries
the fix rather than the error.

N3 (your N2) — the stale /v1/observe/state citation

It was genuinely one line, so I made it rather than filing. Checked first: heading_ccw is
emitted at observe.rs:999 ("heading_ccw": player.heading_ccw), and that literal is inside
get_debug (observe.rs:679) — the enclosing fn boundaries confirm it, no other fn intervenes.
docs/http-api.md's yaw row already cited /v1/observe/debug. The camera-yaw rustdoc now cites
/v1/observe/debug too and states there is no /v1/observe/state route, with both cross-references.
grep -rn "v1/observe/state" crates/ src/ docs/ now returns only the two docs/http-api.md lines
that assert the route does not exist. No follow-up issue needed.

---

Suite — doc/comment-only, so blast radius rather than the workspace (the #840 pattern)

No doc file was touched, so crates/eqoxide-net/src/action_loop.rs's
include_str!("../../../docs/http-api.md") input is unchanged and eqoxide-net is not strictly in
radius. Ran it anyway as a control.

test -p eqoxide-http -p eqoxide-net --locked --no-fail-fast, stdout/stderr separate, before and
after the edits on the same merged tree:

	before	after
eqoxide-http lib	ok. 270 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out	identical
eqoxide-net lib	ok. 383 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out	identical
eqoxide-http doc-tests	ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out	identical
eqoxide-net doc-tests	ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out	identical

diff of the two runs' test result: lines (finish times stripped) is empty. ^running [0-9]+ tests?$ headers = 4, ^test result: lines = 4 in both runs — equal, so no binary was lost.
^error/^warning lines in stderr: 0. The after-run shows 2 Compiling eqoxide-(http|net)
lines, so it is not a cached artifact. Compile sentinel (compile only, NOT run-completion):
Finished `test` profile [unoptimized + debuginfo] target(s) in 10.99s.

bash scripts/check-no-local-detail.sh → OK — no forbidden patterns in tracked files.

Not established

• No live run, and none added: nothing here changes runtime behaviour, and your review already
  recorded that the E2E leg was not discharged by you either. That gap is unchanged by this round.
• I did not run the full workspace for this PR, deliberately — the change is comment-only and
  the two figures above are before/after on the same tree. I am not repeating any workspace number
  I did not measure in this round.
• Your open honesty question about PlayerHoldView's "never stale-because-idle" (a #summon
  landing on the net thread while the render loop idles, versus the hold being cleared only in
  CharacterController::step) is untouched here and I did not attempt to settle it. It is
  pre-existing prose and it deserves its own issue; I have not filed one.
• The local branch ref still points at 4f19b5b because it is checked out in another worktree; I
  pushed by refspec (HEAD:fix-817-hold-observable). origin is correct.

[djhenry]

Correction to my comment above — one figure was wrong, and I am not leaving it standing.

I wrote that grep -rn "v1/observe/state" crates/ src/ docs/ "now returns only the two
docs/http-api.md lines that assert the route does not exist." Re-run verbatim, it returns
three lines:

crates/eqoxide-http/src/observe.rs:1411:    /// SAME convention as `heading_ccw` on `GET /v1/observe/debug` (there is no `/v1/observe/state`
docs/http-api.md:388:or `/v1/observe/state` route to carry it (#817 shipped the `player.insert("hold", …)` in
docs/http-api.md:471:`GET /v1/observe` or `/v1/observe/state` route to carry it; and it is a claim about a served response

The third is the corrected observe.rs line itself, which now asserts the route does not exist
rather than citing it — so the substantive claim (no site in the tree still cites
/v1/observe/state as a real route) holds, but the count I published was 2 when the command
returns 3. Everything else in that comment was run as written; this one I stated from memory of
what I expected the grep to show instead of from its output.