Three audit fixes in the streaming and context path - #779
Merged
Conversation
vpma.25 — request headers were serialized into the request BODY. rig serde-flattens additional_params into the JSON body and no provider promotes a body field to an HTTP header, so setting api_key shipped 'Authorization: Bearer <key>' to the endpoint as data, where it can be logged, while still failing to authenticate. Nothing in production set it (integration.rs passes None/empty on every spawn), so removing the injection changes no live behaviour. Not replaced with a real-header implementation: per-request headers belong at the HTTP client layer beside the transports that already own auth. The knob now warns once rather than silently doing nothing, which is how it survived. Three existing tests asserted the old behaviour — one asserted the credential IS in the body. They were written from the implementation's output, so the bug had become the contract. vpma.23 — a tool call whose name never arrived survived to the final message, counted as a tool call, flipped the turn to ToolUse, and made the loop dispatch a tool named "". Dropped at finalization, keyed on the empty NAME rather than open_tool_calls membership: some providers emit only deltas and never a complete event, so dropping by openness would discard good work. Counted in the log, not silently discarded. vpma.22 — the ExitWithSummary tier logged 'ending turn' and then fell through to prepareNextTurn with has_more_tool_calls untouched. When the summarizer fails or the breaker is already open at >80% context, the loop went round again against a context still over threshold and the next request could overflow — the exact case the tier exists to prevent. It now breaks, after the checkpoint reset and snip cleanup so neither is skipped. Closes dirge-vpma.22, dirge-vpma.23, dirge-vpma.25.
yogthos
pushed a commit
that referenced
this pull request
Aug 14, 2026
…r twice Reported live: the final answer rendered twice with the dim "repaired 0 input(s): ; 1 invalid" line between the copies. It looks like the old scavenged-tool-call duplicate but is unrelated — it is a render bug, and the repair line is what triggers it. Renderer::stream is handed the WHOLE accumulated response every time and replaces the open block at the buffer tail, but only while `streaming`. write_line seals that block, and the repair-stats summary is emitted once per run immediately before AgentEnd — only when a repair fired or an input was invalid, which is why the duplicate tracks the "N invalid" line. handle_done then re-streams the response to commit it as one reflowable markdown block, takes the sealed branch, and pushes a second block holding the whole answer. The renderer now remembers the committed part of the current logical stream and renders only the remainder, so a re-stream after a seal is a no-op and text arriving after one appends. end_stream() seals and clears that, wired where the response buffer is already cleared: done, tool_call, interjected, error, context_overflow, the reasoning marker and the notice path. Without it a turn repeating the previous answer verbatim would render as nothing — which is the exact case this was reported from, so there is a test for it. Also from the review of #779: land the vpma.22 regression test (the one fix that shipped without its discriminating test — verified here by mutation, red with the fix reverted), correct the LoopConfig api_key / get_api_key docs, which still advertise per-request OAuth resolution that vpma.25 removed, and drop a redundant rebind in rig_stream. Closes dirge-fw0p
yogthos
added a commit
that referenced
this pull request
Aug 15, 2026
…780) * Fix three diagnostics-and-defaults bugs in the stream and provider path The tool-call gap timeout reported the residual budget its last wait happened to get, not how long the provider was actually silent. When the gap window drained between iterations — an ordinary slow consumer, since the generator sits parked at its yield while the clock runs — the wait got the 1ms clamp and the message read "timed out after 0s ... the harness narrows to 60s" in a single sentence. as_secs truncating made even the normal case read 59s for a full 60s window. It now reports the measured stall, rounded. The gap budget also moves onto the runtime clock, which is the one that actually spends it; two clocks measuring one window can only disagree. The wire dump's reasoning flag was additional_params.is_some(), which is true for a tool_choice gate or a metadata map with thinking off, so turns got labelled reasoning-enabled while sending no reasoning params. The params and the flag are now the same decision. DeepSeek, GLM, Cerebras, OpenCode and Kimi each restated their default endpoint as an unwrap_or at client construction, duplicating the resolver's table. Four of the five were unreachable, so they were free to drift; and had the resolver ever stopped answering, the stale copy would have become the live endpoint silently. One table now, and a missing endpoint fails the build naming the provider instead of defaulting to rig's assumption — for the OpenAI-compatible clients, the wrong host holding the wrong key. Closes dirge-vpma.24, dirge-vpma.26, dirge-vpma.27 * Stop a notice between the last token and Done from printing the answer twice Reported live: the final answer rendered twice with the dim "repaired 0 input(s): ; 1 invalid" line between the copies. It looks like the old scavenged-tool-call duplicate but is unrelated — it is a render bug, and the repair line is what triggers it. Renderer::stream is handed the WHOLE accumulated response every time and replaces the open block at the buffer tail, but only while `streaming`. write_line seals that block, and the repair-stats summary is emitted once per run immediately before AgentEnd — only when a repair fired or an input was invalid, which is why the duplicate tracks the "N invalid" line. handle_done then re-streams the response to commit it as one reflowable markdown block, takes the sealed branch, and pushes a second block holding the whole answer. The renderer now remembers the committed part of the current logical stream and renders only the remainder, so a re-stream after a seal is a no-op and text arriving after one appends. end_stream() seals and clears that, wired where the response buffer is already cleared: done, tool_call, interjected, error, context_overflow, the reasoning marker and the notice path. Without it a turn repeating the previous answer verbatim would render as nothing — which is the exact case this was reported from, so there is a test for it. Also from the review of #779: land the vpma.22 regression test (the one fix that shipped without its discriminating test — verified here by mutation, red with the fix reverted), correct the LoopConfig api_key / get_api_key docs, which still advertise per-request OAuth resolution that vpma.25 removed, and drop a redundant rebind in rig_stream. Closes dirge-fw0p --------- Co-authored-by: Yogthos <yogthos@gmail.com>
yogthos
added a commit
that referenced
this pull request
Aug 15, 2026
* Add a loop trace, and fix three bugs it found --trace <path> writes a JSONL record per loop decision: turns, tool calls, token usage, the context manager's verdict, and every harness intervention attributed to the guard that sent it. scripts/loop-trace.py renders it. It taps the LoopEvent stream at the pump in integration.rs, the one point every event passes through on its way to every consumer, so there is no second set of call sites to keep in step with the first. describe() is exhaustive, so a new LoopEvent variant won't compile until it says how it traces. Interventions are named from the existing tag registry, so a guard added later is traced without anyone editing this file. Three bugs found by running it against a local model: A force-ended turn ended the whole run. The ExitWithSummary tier sets force_turn_end, which broke out of the inner loop — but the inner loop is the turn loop, so control fell to the finalization poll and the run stopped. A model over the threshold got one turn: its tool calls ran, results were appended, and the run ended before it ever saw them, silently. vpma.22 in #779 introduced this; the arm did nothing at all before that. Now it continues the run when the fold actually made room, and when it didn't, stops with a notice naming prompt_tokens and ctx_max. glm-5.3 matched nothing in the context-window table (glm-5.2 is listed at 1M) and fell to the 128k default, so a live session showed 226.9k/128.0k and "compaction soon" on a model with eight times that room. Every context tier divides by that number. Family prefixes now cover point releases, qwen3/qwen4 no longer inherit the original qwen's 32k, and a model neither lookup knows warns once instead of being guessed in silence. turns= on the gates line read 0 for any run that force-ends its turns: record_turn sat past the break. Moved to where the turn happens. The fold and force-summary log lines now carry prompt_tokens and ctx_max, which took solving a division by hand to recover. * Tell the model why a masked test run didn't count The verifier declines a build/test command whose exit status was piped or sequenced away — the zero belongs to tail, not pytest — and that is right. But it returned without recording why, so status() fell to the "nothing ran" branch and the model was told "you didn't run the tests or build to check it" after running pytest four times. Measured against a local model: it did not remove the pipe, because the sentence was visibly false. It re-ran the same shape twice more, then added `; echo "exit=$?"`, which reports echo's status. The claim gate fired on the consequence and the final answer asserted "exit status 0" — a number it never had. Three of fifteen turns, ending Unverified with a green suite. The verdict is unchanged: a masked pass still is not a green. Only the message changes — it now quotes the command, says the status belongs to the last stage of the pipe, and says what to run instead. * loop-trace.py: count turns from turn_end The summary counted assistant message records, which the trace stopped writing when the pre-stream placeholder was dropped — so it read 0 turns for a run that took 15. turn_end is one per completed turn and matches the gates line. * docs: how to read a loop trace * beads: close the trace findings, note what remains on tva8 * trace: heartbeat during a streaming turn A local model was observed thinking for thirteen minutes across 2892 tokens with nothing recorded between turn_start and the next event — which reads like a deadlock in a file whose stated purpose is explaining a run that hung. MessageUpdate now emits one throttled record per 10s saying what the turn is doing and how much it has produced. One record per delta would be the opposite failure, so the throttle is asserted both ways. * Per-provider context_window, and stop two gates disagreeing about pytest context_window (GH #772) is now settable per provider. It was top-level only, so with more than one provider configured — the common case — there was no way to correct one model's window without corrupting the others'. Correcting a local model's window this session needed a whole separate config directory for exactly that reason. Precedence is providers.<name>.context_window, then the top-level key, then the model table, then 128k. The loop's window override and the session's gauge now resolve from the same call, at the one point where provider and model are both final. It stays None when nothing is configured, so the loop keeps re-resolving from the table per run and a mid-session /model switch still tracks the new model. Separately: claim_gate::segment_kind took the first token of a segment as the command, so `python3 -m pytest -v` classified as `python3` and returned None. The verifier matches on any token, found `pytest`, and recorded the run green — so the model was told "no build/test command of the matching kind ran this run" one turn after running one correctly, having just been corrected by the verifier for running it wrong. Measured live. segment_kind now steps past an interpreter or runner prefix: python -m <module>, npx/bunx, poetry/uv/pipenv/rye/hatch run. `python script.py` and `python -c '…'` are deliberately left alone — the interpreter is running a script, not a known tool. Neither recogniser knew `unittest`; both do now. A test asserts the two agree on the commands where they overlap. * Context gauge: report what compaction compares The gauge divided total_estimated_tokens — a chars/4 heuristic plus per-tool-call overhead over the whole persisted transcript, tool results at full length — by the model's advertised window. The context manager compares the provider's prompt_tokens for the REQUEST, whose oversized results have been snipped and whose old turns are a summary, against effective_ctx_max. Reported live: 226.9k/128.0k, 100%, 'compaction soon', one compaction, on a run whose requests were never close. Session now keeps last_prompt_tokens, set from usage.input_tokens alone — the same field decide_after_usage divides by. Not the sum of the three input figures: that would be closer to the true prompt on Anthropic and double-count on DeepSeek, and either way put the gauge back to describing a fold that does not happen. The estimate stands in until the first response. Both the panel and the status bar read it. * docs: harness review, August 2026 * Correct the bead id on the masked-verification fix (g4lk, not hy4k) * Reword the masked-verification nudge on evidence from a second run Told to drop the `|` or `;`, one model produced a clean `pytest -v` and another produced `pytest -v; echo "EXIT=$?"` — which obeys the letter, tries to surface the status, and masks anyway. The nudge is now positional (the build/test command last, nothing after it) and names the three idioms that have actually shown up. Also records the long-horizon run in docs/harness-review-2026-08.md, including a cascade it exposed: a masked-but-green test run means verified_green never latches, so the progress monitor — which counts a green as one of its three progress events — fired the stall nudge twice at a model that had passed all 22 tests, the second time at 618.0s of a 618.1s run. Filed as hwk9.4 rather than patched: my first fix blamed edits_since_verify, which the stall nudge does not read and which an existing test deliberately pins the other way. * Stop rendering an intervention twice, and put boundary nudges on the stream Tracing what the front end receives, not just what the loop decided, showed the TUI rendering every harness intervention's body twice. The notice carries "harness intervention: {summary}\n{body}" because headless sees only it — --print renders SystemNotice and ignores UserMessage entirely — but the TUI gets both and renders the body from the message too. The notice now shows its summary line only in the TUI; the body stays on the message path, which is the copy dirge-m10x guarantees survives the next stream anchor. Headless output is unchanged. With that settled, boundary nudges emit MessageStart/MessageEnd like the finalization path. They were pushed straight into context with only a SystemNotice, so stall, budget, prologue, track-work, file-touch, safe-state, fast-verify and reflection nudges were absent from the message stream — the tally read nudge_progress_stall=2 while the trace recorded one intervention. Adding them before the render fix would have put the body on screen a third time. Also: the stall checkpoint stands down while the verifier holds a masked decline. Measured — a run that passed all 22 tests at 345s via `pytest … | tail -28` was told twice it had made no progress for three turns, the second time at 618.0s of a 618.1s run. The verify nudge owns that state and has the actionable message; the stall text offers "getting a green check" as the way out, which is what had just happened and could not be counted. The prologue checkpoint is not suppressed — it fires on a run that produced nothing, where a masked verification is not the explanation. * Breadcrumb tool schemas for a small context window dirge's opening request is large before the model has done anything: measured, 16,172 prompt tokens with the 34 built-in tools and 32,621 with MCP servers loaded. The second is larger than a 32k window in its entirety, so such a run could not take a single turn — and the only symptom was the context manager force-ending every one. On a window at or below 48k, each tool's description is trimmed to its first sentence and each parameter's to a short clause. Names, types, enums and required-ness are untouched: the model keeps everything it needs to form a well-formed call and loses the prose about when to prefer one tool over another. No tool is dropped — a model that cannot see a tool cannot ask for it, and that failure is silent and looks like incapability. Measured on the same task and model at a 32k window: 16,202 -> 12,249 prompt tokens, context peak 51% -> 39.5%, and the model still reached for list_symbols unprompted and answered correctly. config `compact_tool_schemas`: auto (default) / on / off. The decision is made at loop_tool_to_rig_definition, the one point every tool becomes a provider schema, and sized against the same window the session gauge and compaction use — passed in rather than re-derived. * 'exit 0' is a kind-agnostic claim, not a build claim Measured on deepseek: the model ran a clean `python3 -m pytest -q` and reported "Confirmed with a real exit status: 22 passed in 0.01s, exit 0." The claim gate fired, because "exit 0" sat in the build/lint list and no build had run. The verify nudge is what asks for the exit status when it declines a masked run — so one guard requested the number and another penalised the answer. "exit 0" / "exit code 0" / "exit status 0" are now satisfied by any observed verification. Kind-matching stays where it earns its keep: a build still cannot support "N passed" and a test run still cannot support "clippy clean", both pinned by a test. Found by running the same task on qwen, deepseek and glm to check the earlier fixes weren't shaped to one model. All three passed 22/22, and all three piped their test output through `tail` — masking is what models do, not a small-model quirk. --------- Co-authored-by: Yogthos <yogthos@gmail.com>
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.
Three from the Wavescope audit epic, all in the agent-loop area.
vpma.25 — a credential was serialized into the request body
merged_request_headersputAuthorization: Bearer <key>intoadditional_params. rig serde-flattens that into the JSON request body, and no provider promotes a body field to an HTTP header — so the mechanism (a) did not authenticate and (b) shipped the bearer to the endpoint as body content, where it can be logged.Nothing in production set it (
integration.rspassesapi_key: Noneand an empty header map on every spawn), so removing the injection changes no live behaviour. Deliberately not replaced with a real-header implementation: per-request headers belong at the HTTP client layer beside the transports that already own auth, not in the completion-request builder. The knob now warns once instead of silently doing nothing — silence is how it survived.Three existing tests asserted the old behaviour, one of them asserting that the credential is in the body. They were written down from the implementation's output, so the bug had become the contract.
vpma.23 — a nameless tool call reached dispatch
A delta-built call starts from a placeholder with an empty name. If the stream ended mid-assembly, the block survived with
name: "", counted as a tool call, flipped the turn toToolUse, and made the loop dispatch a tool named""— burning a turn on an error describing nothing.Dropped at finalization, keyed on the empty name rather than
open_tool_callsmembership: some providers emit only deltas and never a complete event, so their calls stay "open" for the whole stream and dropping by openness would discard perfectly good work. Counted in the log rather than silently discarded.vpma.22 —
ExitWithSummarynever ended the turnThe arm logged "forcing summary and ending turn" and then fell through to
prepareNextTurnwithhas_more_tool_callsuntouched. When the summarizer fails or the circuit breaker is already open at >80% context, the loop went round again against a context still over the threshold and the next request could overflow or 400 — the exact case this defence-in-depth tier exists to prevent.It now breaks, and does so at the bottom of the iteration so the checkpoint-schedule reset and the per-iteration snip cleanup still run.
Closes dirge-vpma.22, dirge-vpma.23, dirge-vpma.25.
Verified: 5484 tests,
cargo fmt --check, all four CI clippy configs.