Repository navigation
fix(gateway,code): the remaining findings of the independent review of 2.0.0 (D1, D2, D3, A1, A2, N1 to N5) - #473
Merged
Merged
Conversation
…f 2.0.0
D2. A ledger that fails after start stops model calls. The first append that
fails marks the ledger failed and closes the segment's file
(harness/gateway_ledger_health.py). `for_home` then returns None, which every
model interface already answers with 503 before any upstream call. At most
once a second the gateway reads the segment back: when it verifies and
begins with the entries in memory it appends again, starting with a
`ledger_gap` entry; when the tail is cut short a new segment records the
break. The segment is opened unbuffered so no failed line waits in a buffer.
Every model interface appends after the upstream answered, so the call that
meets the failure has reached the provider; the docs say so.
A1. `gateway_ledger.check` reads the head record before the segments, so
entries appended during a check are not reported as removed. `read_segment`
named the line before a cut line and dropped that intact entry; it names the
cut line.
N2. Only names that match segment-NNNNNN.jsonl exactly are segments. Other
`segment-*` files are named as ignored by `verify` and the start line.
A2. `gateway_key_gate.enter` reads the key's row again and takes the slot
under one lock, which `release` takes too.
D1. An unfinished stream's prompt and received text are each charged the
larger of the word count and a quarter of the characters, rounded up.
D3. The fallback from native calling to the text protocol starts only on an
error that says tools or function calling are unsupported
(harness/endpoints_tools_fallback.py), never on another 400 that mentions
them. The session records a `native_tools_fallback` entry and prints one
notice. The provider documentation read on 2026-11-14 gives no exact sentence
for this refusal, so the wording is a stated heuristic (KNOWN-LIMITS).
N1. The Lean scan resolves import aliases and reports more spawn APIs and an
env= that is not exactly lean_env(...) (tests/lean_spawn_scan.py, which also
lists what it cannot see).
N3. The request line and headers have one deadline, on every route, before
authentication; the body of a model call has one (four read timeouts).
N4, N5. KNOWN-LIMITS names /v1/embeddings among the routes outside the
ledger. `gateway-ledger show` cuts a field at 80 characters; `--full`.
CHANGELOG: the three sentences the review flagged are corrected ("cannot
open or write", the verify retry, "the other ways to start a child").
Tests (release_gate): tests/test_gateway_ledger_write_failure.py,
tests/test_gateway_round3_small_fixes.py,
tests/test_code_session_native_fallback_trigger.py, tests/test_lean_child_env.py.
On the unfixed tree 81 of 87 of these cases fail. One existing control
changes: the output estimate of its unfinished stream is 9 (a quarter of the
received characters), where the word count is 5.
Not run: any hosted provider, a real full disk, macOS.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…cord the gap Seen when the reviewer's probe was run against the branch: the gateway stopped inside the one-second retry interval, so the ledger held no `ledger_gap` entry for the append that had failed. `close` now calls `gateway_ledger_health.settle`, which resumes once if the disk allows it and logs what stays unrecorded if it does not. The ledger tests are two files now, each under 300 lines: tests/test_gateway_ledger_write_failure.py (D2) and tests/test_gateway_ledger_live_verify.py (A1, N2, and the cut-line count). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…no ledger_gap entry Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…error does not match The documented Claude API error for forced tool use (`tool_choice: type "tool" and "any" are not supported for this model`) matched the second phrase. The phrase now lists its subjects (tools, tool use, tool calling, function calling, the functions parameter) and the test table holds four documented errors that must not start the fallback. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The limits page said the gateway resumes within a second. It tries the ledger again when a call comes in, at most once a second, and once more when it stops. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…de the pause Reverting one fix at a time in a scratch copy showed that the A2 test did not notice when the row read and the slot release used two locks. It covered a call that starts while another request is paused between its row read and its admission, and not a call that ends in that pause. The second order is tested now: the call that ends keeps its slot until the paused request has been admitted or refused. Both reversions (two locks, no lock) make tests/test_gateway_admission_lock.py fail. The A2 tests moved to that file. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The admission-lock test let its stub provider answer after 0.6 seconds and needed the second request to have read the key's row by then. The provider now answers when the test says so, after the second request has read the row. The header-drip test allowed two seconds for a close that comes at half a second; it allows four, and drips for eight. Before the deadline existed the client was still connected when the dripping ended. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A read-only pass over this branch by another agent, given the code and the intended properties and not my conclusions, reported these. Each was checked against the code before it was changed, and each has a test that fails when the change is reverted in a scratch copy. - gateway: after a refusal that did not read the body (a 401, for one) the drain read the promised length with a buffered read(n), which waits for all n bytes. A client with no credential that sent a byte at a time held its thread for as long as it kept sending. The connection's reader now gives one read(n) a budget: the socket's own timeout while one is set (the drain sets two seconds), else four read timeouts. This also puts the body deadline on the routes that read their bodies in gateway.py. gateway_body_drain.py is pinned and unchanged. - code: whether a reply is held to the native rule was decided when the reply arrived, from the process-wide list of models that moved to the text protocol. Sessions in one process share that list, so a model moved by one session while another's reply was on its way made that reply a text-protocol reply, where a prose TOOL line runs. The decision is now what the request carried. - gateway ledger: the segment name pattern used \d, which matches digits of other scripts; such a file sorted last and was taken for the newest segment. ASCII digits only. - gateway ledger: a head record that could not be rewritten after an entry was written did not mark the ledger failed, so every later call reached the provider and was answered 500. It marks it now, and a resume reopens the head record. - code: the fallback wording matched "does not support tool_choice", "parallel tool calls are not supported", "tools are not supported with images" and any "tool choice requires". Those are about one way of using tools. The phrases are tighter and such errors are excluded. - code: the loop now runs the calls as parsed from the record (equal to the provider's objects by the check before it), so the executor shares no argument object with the remembered native reply. Not changed, and written down in the PR: a resume whose new segment cannot be removed after a failed start stays failed until a restart; a failed debit for an unfinished stream does not suspend the key; the admission lock is held during the key store read. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
HarperZ9
added a commit
that referenced
this pull request
Nov 14, 2026
Main holds the third review's fixes (#472, #473). CHANGELOG.md conflicted: main's Unreleased section against this branch's reordered 2.0.0 section. The release side is kept. The ten new bullets are carried with their pull request numbers, the three #470 sentences that main corrected are carried over, and the entries under "What changed by default" are updated. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
HarperZ9
added a commit
that referenced
this pull request
Nov 14, 2026
The review paragraph names the third review: one blocker (text inside a native tool call's arguments could run as a different call) and ten other items, fixed by #472 and #473. It says the reviewer's probe scripts were run again against the fixed code, and that #472 and #473 have had no further independent review. "What the security review changed" gains: the 503 for a ledger that stops taking writes while the gateway runs, the quarter-of-characters estimate for an unfinished stream, the head and body deadlines, the argument rule for native tool calls, and the notice when a model moves to the text protocol. Limits follow docs/KNOWN-LIMITS-2.0.0.md on main: connections are bounded by deadlines and not by a cap, /v1/embeddings is outside the ledger, the call that meets a ledger write failure has reached the provider, and the fallback rests on the wording of a provider's error. The changelog gains one entry for #471 (the Telos Toolkit plugin folder), which merged to main between #472 and #473 with no changelog entry of its own. It is in the repository tree and in no release asset; the entry says so. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.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.
The remaining findings of the independent review of the 2.0.0 fixes (B1 merged as #472). Each fix has a test that fails on the unfixed tree.
ledger_unavailablebefore any upstream call. When writes resume the gateway appends aledger_gapentry andverifypasses; a line left cut short is recorded as a break in a new segment. New moduleharness/gateway_ledger_health.py.tests/test_gateway_ledger_write_failure.pyverifyreads the head record before the segments, so a busy, intact ledger verifies.tests/test_gateway_ledger_live_verify.py, forced order and a writer threadverifyand the start line.tests/test_gateway_admission_lock.py, forced pause, both ordersestimated.native_tools_fallback. New moduleharness/endpoints_tools_fallback.py.tests/test_code_session_native_fallback_trigger.pyenv=that is not exactlylean_env(...).tests/lean_spawn_scan.pystates what it cannot see.tests/test_lean_child_env.pytests/test_gateway_round3_small_fixes.py/v1/embeddingsamong the routes outside the ledger.gateway-ledger showcuts a field at 80 characters;--fullprints it whole.tests/test_gateway_round3_small_fixes.pyWhat is stated, not fixed
ledger_gapentry; the log has the failure.Found by a second read of this branch
Before the merge another agent read this branch with the code and the intended properties in hand, not my conclusions. It is not an independent review: I started it and I judged its findings. Six of them held up against the code and are fixed here, each with a test that fails when the change is reverted:
read(n)a budget: two seconds in that drain, four read timeouts for a body. The body deadline therefore covers every route.gateway_body_drain.pyis pinned and unchanged. Checked with plain sockets on Windows and on Linux (WSL): closed at 2.1 and 2.0 s.\d, which matches digits of other scripts. ASCII digits only.tool_choice, parallel tool calls and tools with images. It is tighter.Not changed: a resume whose new segment cannot be removed after a failed start stays failed until a restart (it fails closed); a failed debit for an unfinished stream does not suspend the key (as before this branch); the admission lock is held during the key store read.
Corrected wording
CHANGELOG: "could not open or write its ledger" (the earlier fix covered open only), the sentence on the second read of
verify, and "the other ways to start a child". KNOWN-LIMITS: the word-estimate row, the read-timeout row, the first row ("a key holder" was narrower than the fact), and the intro, which now says the reviewer's probes were run again and that these fixes have had no further independent review.Evidence
7b7803c6f) the new test files, plus the new scan forms run through the old scan: 81 failed, 6 passed. The six that pass there are controls.tests/with noharness/beside it.gateway_ledger.py295 lines, no file over 300, no function over 50;gateway.py,gateway_body_drain.pyandlocal_loop.pyuntouched in this PR. An unimportablegateway_ledger_healthyields zero upstream requests (tested).Not run: any hosted provider, a real full disk (the failure is simulated at the segment's file), macOS. Local runs were targeted slices on Windows; the rest is left to CI.
🤖 Generated with Claude Code