serve: cold-reset the next request after a mid-generation failure

[ghazni101]

Summary

A daemon generation error that reports rolled_back: false poisons the gateway's session flag, so the next request cold-resets instead of resuming a mid-turn slot session.

Re-scope after review (fivetide, 2026-10-08): the originally observed trigger (unsafe multi_slot terminal: open_think) is gone on current beta — 325f31cc1 made an unclosed <think> a reasoning-only stop with the session closed at commit, and every post-admission error path in slots.rs already closes the session before emitting. Mid-generation failures that cannot attest a rollback still exist on beta (class internal for a multi-slot engine death after produced tokens, class generation for unsafe terminals, class gpu for vision faults), so the guard stays as defense-in-depth for daemon regressions, with the classification corrected per review.

Which surface(s) does this touch?

• serve — crates/hipfire-cli/src/serve (gateway)

Changes

• Classifier (poisons_session_state): poison on any typed daemon error without an attested rollback EXCEPT the pre-generation-by-wire-contract classes (validation, malformed, context_length, unsupported, overload, cancel, transport), each mapped to its beta pre-generation sites in the comment. The old deny-list exempted internal and transient; both can carry unattested mid-generation state and now poison. internal is beta's engine-death class and not even a hipfire_client::error_class constant — that desync is exactly why the deny-list shape was wrong.
• Scope disclosures (per review): the poison flag is process-global by design — the gateway has no per-conversation session key; the daemon owns slot lifecycle. The follow-up reset is refused by the daemon while any multi-slot session is active or queued, so under concurrency a poison costs the poisoned request's own attempt plus one full reload — never a reset under a live slot. The failure→flag race can let one request start a turn early; on beta the failed session is already closed daemon-side, so this is only reachable under a future daemon regression. Documented on the field and at both use sites.
• clear_resident on failed reset is now unconditional (previously forced retries only) — undisclosed in the old head, now stated: an unattested reset leaves the same unknown-state situation either way.

Test plan

• cargo test -p hipfire-cli — 345/345, including the poison-classifier table test and a fake-daemon HTTP wire test that drives the real serve lowering and asserts the wire log: an internal/rolled_back=false failure is followed by exactly one reset before the next generate; a validation refusal by none.
• scripts/check-crate-maps.py --check green; scripts/leanup-ratchets.sh green.
• GPU repro on current beta — not runnable on the author's host (no ROCm userspace; static evidence says the original trigger is unreachable). Maintainer hardware run welcome; per the review's own analysis the stale-slot mechanism it exercised no longer exists on beta.

Single commit on current beta, hipfire-cli only. The unrelated hipfire-isa/peacemaker-ir toolchain skips (beta 639907cc3 landed equivalents), quant_mq test rewrite, hipfire-xdna clippy allow, and the daemon main.rs helper move from the old head are dropped — along with the merge conflicts they caused.

[ghazni101]

Empirical evidence (gfx1101, ROCm/HIP 7.15 container, qwen3.5-4b.mq4, multi-slot 2×81920):

Reproduced the trigger deterministically — any request whose budget dies inside the think span:

POST {multi-turn convo, max_tokens: 8}
→ 500 "[generation retryable=false rolled_back=false] unsafe multi_slot terminal: open_think"

This build's serve log, per failure:

[hipfire] generation failed without rollback ([generation rolled_back=false] unsafe multi_slot terminal: open_think); session state poisoned — next request cold-resets

…followed by the cold reset; subsequent requests run a real generation rather than resuming the mid-turn session. Negative case verified: a gateway-side validation 400 (oversized explicit max_tokens) does NOT poison — no log line, no reset.

Note on reproduction scope: with this model/config the poisoned slot's retained state surfaces as a fast repeat open_think failure rather than verbatim prompt echo; the mechanism (resume vs cold-reset on rolled_back=false) is verified in log + timing asymmetry, and classification is unit-tested. serve_harness.py battery (standard route, thinking off): 5/5 turns, finish=stop, 0 runaway/empty/attractor — battery-806.json.

[fivetide]

Review notes (gfx1151 / Strix Halo, ROCm 7.2)

Recommendation: request-changes (reproduce on current beta first; narrow the classifier; drop unrelated hunks).

• 🟠 medium — the trigger may already be gone on beta. 325f31cc1 made an unclosed <think> a reasoning-only stop: on beta crates/hipfire-daemon/src/slots.rs:1561-1566 open_think is no longer an unsafe terminal, the session is closed at commit instead. Even at this PR's base, the unsafe-terminal branch already called close_session before emitting the error. Please re-run your max_tokens: 8 repro on current beta; if the follow-up still echoes the failed turn, the stale state is somewhere else (gateway prefix cache?) and that is what should be fixed.
• 🟠 medium crates/hipfire-cli/src/serve/complete.rs:2154-2169 — internal is classed as "never touched the engine", but beta slots.rs:1503 emits internal with rolled_back=false for "multi_slot engine terminated without completing request … N tokens were produced". That is a mid-generation failure. Prefer an allow-list of pre-generation classes, or have the daemon report rolled_back honestly, rather than a deny-list that has to be kept in sync with every daemon error site.
• 🟠 medium complete.rs:2690 — the flag is process-global and set after the runtime lock was released for generation. Under multi-slot, (a) a concurrent request can pass the needs_session_reset check before the flag lands, and (b) one failed request cold-resets the engine under every other in-flight slot. Needs either a per-session poison or a statement that cold reset is safe with live slots.
• 🟡 low complete.rs:2240 — clear_resident on reset failure is now unconditional (previously only for force_reset). Probably right, but it is an undisclosed behavior change; mention it.
• 🟡 low scope — the hipfire-isa / peacemaker-ir toolchain skips, the quant_mq test rewrite (weakens an assertion), the hipfire-xdna clippy allow, and moving no_model_loaded_suffix are unrelated. Beta 639907cc3 already landed equivalent skips, and these hunks are now the merge conflicts.

_{Agent-assisted review; head e5aba2a. Evidence: static read of the diff; local validation on gfx1151 follows in a separate comment where applicable.}

[ghazni101]

Closing: the triggering failure mode no longer exists on beta, verified empirically on the hardware class where it was found.

gfx1101 / RX 7700 XT, ROCm 7.2.4 container, qwen3.5-4b.mq4 md5 712b69f8…, multi-slot 2×81920, kv q8, thinking on. The max_tokens: 8 repro (budget dies inside the think span) on beta e3beb4414: turn 1 is a reasoning-only length stop (325f31cc1 semantics, session closed at commit), turn 2 answers its own prompt with no echo; serve log shows zero unsafe multi_slot terminal / open_think. The old rolled_back=false slot-poison mechanism is unreachable.

What the review surfaced that's still real (daemon-side, follow-up material): slots.rs:1494 (engine-died path) calls engine.close directly, bypassing close_session, leaking pending_tools.by_session; close_session (slots.rs:607) swallows close results. If gateway-side guarding is ever wanted, the durable shape is the daemon reporting rolled_back honestly at every emit site, not a gateway class deny-list.

Thanks @fivetide — the "reproduce on current beta first" call was right. Clean head (307ce098f, hipfire-cli only) stays on the fork branch if this ever needs resurrecting.

[fivetide]

Validation on gfx1151 (Strix Halo, ROCm 7.2): the trigger does not reproduce on current beta

Beta 639907cc3 CLI + daemon, isolated HIPFIRE_HOME, serve.multi_slot=true (2 slots × 8192), reasoning.mode=on, qwen3.5-9b.mq4. Your repro shape:

1. 3-message conversation, max_tokens: 8 → HTTP 200, finish_reason=length, content="", reasoning_content="Here's a thinking process that leads to". No unsafe multi_slot terminal: open_think error (beta 325f31cc1 turned it into a reasoning-only stop and closes the session at commit).
2. Follow-up turn ("What is my name?"), twice → finish=stop, content "Your name is Ada.". The reasoning is about the new question; there's no trace of the failed turn's B-tree prompt.

So on beta the observed corruption needs a different trigger. The remaining unsafe multi_slot terminal reason is a malformed or truncated tool call, which I didn't exercise. If you can reproduce the echo on current beta (e.g. with a truncated tool call), please post that repro. Without one, I'd narrow this PR to the classifier/locking fixes from my note above, or close it.

_{Agent-assisted; beta 639907c; PR head e5aba2a not built (conflicts with beta).}