fix(claude-bridge): rotate the session instead of 413-ing forever (#667) - #692
Open
vibechoom wants to merge 1 commit into
Open
fix(claude-bridge): rotate the session instead of 413-ing forever (#667)#692vibechoom wants to merge 1 commit into
vibechoom wants to merge 1 commit into
Conversation
A conversation on the subscription path rides one persisted claude session — that is what keeps its prompt prefix byte-stable and the cache warm, and why persisted-session beat `Attach::OneOff` on #584. It also means a plugin with a stable persona and no history rewriting (pet) maps to one session that grows every turn and that nothing prunes, until it returns `PromptTooLong` permanently, recoverable only by deleting session state by hand. Option 1 (archive-and-recreate on the typed overflow), with one correction: `hive-claude` addresses sessions by title, so "create a fresh one" cannot mean a second session under the same title — `--resume` would then pick between them arbitrarily, quite possibly the full one. A fresh addressable session needs a fresh title, so the replacement carries a generation (`…-g1`, `…-g2`). That is option 3's mechanism with option 1's trigger; generation 0 keeps rendering with no suffix, so every session already on disk still resolves. The delta rule is untouched: an ordinary turn resumes and sends only the delta, while a rotation names a session nothing has ever resumed, so the backend's existing create arm replays the whole transcript — persona prefix included. `prompt_for` is unchanged and every delta-rule test passes unedited. Rotation logs at `warn` (from/to/turns), because it is the one event that explains a cache-hit rate falling off a cliff — the observability trap #609/#634 argued out on the networkd/wifi side. A concurrent request joining an existing replacement logs `info`; an overflow on a title the bridge did not mint logs `error`. Single-flight does NOT cover the rotation race: it keys on the full transcript, so two different requests in one conversation resolve to one title and can both overflow. The rotation path therefore takes a process-wide async lock, held across the decision AND the replacement's first turn, so the second request joins rather than minting a rival. The retirement is recorded before the retry runs, so a rotating request that blows its budget still leaves the next one pointed at the fresh session. Nothing in CI can drive a real session to `PromptTooLong`, so the decision is a pure function (`session::rotation_for`) and it is that, plus the retirement bookkeeping, that the new tests pin. Closes #667. Refs #584, #666. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A conversation on the subscription path rides one persisted claude session — that is what keeps its prompt prefix byte-stable and the cache warm, and it is why persisted-session beat
Attach::OneOffon #584. It also means a plugin with a stable persona and no history rewriting (pet, exactly) maps to one session that grows every turn and that nothing prunes, until it returnsPromptTooLongpermanently, recoverable only by deleting session state by hand.Now it retires the full session and continues the conversation in a fresh one.
Which option, and one correction to it
Option 1 — archive-and-recreate, triggered by the typed overflow — with one thing the issue's framing did not anticipate:
hive-claudeaddresses sessions by title (Attach::Createis--name,Attach::Resumeis--resume). So "create a fresh one" cannot mean creating a second session under the same title:--resume <title>would then be picking between two, arbitrarily, and quite possibly the full one — the bug would survive its own fix. A fresh addressable session needs a fresh title.So the replacement title carries a generation:
hytte-bridge-<hash>→-g1→-g2. That is option 3's mechanism, but with option 1's trigger — the rotation is driven byError::PromptTooLong, never by a turn counter, so a session is only ever retired once it is genuinely full. Generation 0 renders with no suffix, so every session already on disk keeps resolving to the title it was created under.Option 2 (
InfiniteSession/CompactionPolicy) stays unadopted; a note on why is at the bottom.Which path sends what — the delta rule is untouched
Both arms are reached through the same
Subscription::respond, whose resume/create decision is unchanged:Resume(title)Resume(next)misses →Create(next)The rotation names a session nothing has ever resumed, so the backend's resume misses with
SessionNotFoundand its existing create arm replays the transcript — which is how the fresh session ends up the same character as the old one. No new prompt-building path;prompt_foris untouched and all four existing delta-rule tests pass unchanged.At most one rotation per request, by construction (the retry calls
respond, not the rotating wrapper). An overflow on the replacement means the client's own transcript does not fit, which no further rotation can fix, so it is reported as the 413 it is.What it logs
Rotation is
warn— visible under the unit's defaulthytte_claude_bridge=info— because it is the one event that explains a cache-hit rate falling off a cliff, which is the observability trap #609/#634 argued out on the networkd/wifi side:Two more:
infowhen a concurrent request finds the session already retired and joins the replacement, anderrorfor an overflow on a title the bridge did not mint (unrotatable — says so, and says the session needs deleting by hand). The old session is left on disk, not deleted.Concurrency: the existing serialisation does not cover this
Stated plainly because the answer is no. Single-flight is keyed on
transcript_key(messages)— the full transcript — so it only collapses byte-identical requests. pet's tick and a manual poke are two different keys that resolve to the same title, andPERMITSis 2, so both can be in flight against one session and both can overflow. Left alone, both would compute the same successor, both find it missing, and bothAttach::Createit: two rival sessions under one title.So the rotation path takes a process-wide
tokio::sync::Mutex, held across the decision and the replacement's first turn. It is never taken on the healthy path and a rotation happens about once per context window, so it costs nothing. Under it the second request re-resolves, sees the replacement recorded, and joins it —a_concurrent_retirement_joins_the_replacement_instead_of_minting_a_rivalpins that decision.The retirement is also recorded before the retry runs, synchronously. That matters more than it looks: an overflow is only discovered after claude has posted the oversized prompt, so the rotating request is the one most likely to blow the 8s budget — and a rotation that only became durable on success would be cancelled with it, leaving the bridge exactly as stuck as it is today. As written, that request may well 504 (pet falls back to a canned quip once), but the next one goes straight to the fresh session.
Also
Titlesgrew a second bounded map. Retirements cannot live in the existing prefix map:rememberwrites two entries per answered turn, so a retirement would be evicted within ~cap/2turns and the conversation would walk straight back into the session that had just overflowed — a rotation every ~128 quips, forever.retiredis written only by a rotation and wins on lookup. Pinned bya_retirement_survives_the_churn_of_later_turns.Reprompt's conversation is the bridge's own record, already bounded and head-pinned, so an overflow there means the client's transcript does not fit — unfixable by rotation, and it would spin the generation counter once per request if allowed to try.session::OVERFLOW_STATUSis used bymap_errorforPromptTooLongand by the rotation decision, and a test asserts it is the status that sentinel maps to and that no other sentinel maps to it — so a future error classified as 413 cannot start retiring healthy sessions. (http.rsalso answers 413 for an oversized request body, but that is raised while reading the request and never reaches the turn path.)Tests
The rotation decision is a pure function,
session::rotation_for(&Failure, &str) -> Rotation, precisely so it is testable without a subprocess: 8 new tests insession.rs(non-overflow keeps the session, first overflow →-g1, chaining across generations, 9 malformed/foreign titles rejected, retirement resolves + survives churn + stays bounded), 2 inbridge.rs(retirement points the conversation at a successor; a concurrent retirement joins rather than rivals), 2 inbackend.rs(the sentinel↔status link and its exclusivity; only the persisted backend rotates). 67 pass in the crate, 12 of them new; no existing test was edited, including the pinned title literal and all four delta-rule tests.Live-verify
Nothing in CI can reach this: there is no
claudebinary in the sandbox, and a genuine overflow takes ~10³ turns. What is not verified on real hardware:CLAUDE_BRIDGE_MODELat a small-window model, or hand-feed one conversation a few very large prompts until the journal shows thewarn. Thenclaude --resume hytte-bridge-<hash>-g1should exist and hold the persona, and the old title should still be there, untouched.cache_read_input_tokensshould return to non-zero within a couple of turns of the fresh session (hive-claude'stelemetry.rsparses it; the bridge does not surface it yet — hytte-claude-bridge: local OpenAI-compatible shim so the LLM plugins can ride a Claude subscription (#575 follow-up) #584 has the standing thread on that).Option 2 stays open on merit rather than on effort:
InfiniteSessionkeeps continuity across the boundary by running/compact, where this drops it. For pet's quips that is close to free and for caw's briefing it is irrelevant, but if a future consumer wants continuity, the trigger and the pin here are the same shape a compaction would hang off.Closes #667. Refs #584, #666.