Repository navigation
[PR] fix(wiring): feed the loop lane's egress sends into the 429 governor - #4772
ranxianglei wants to merge 16 commits into
Conversation
…ment #4316 PR3/3 (restampRefsToLedger) re-homes citation numbers once on every full-mode cold load; goldens pinning exact ref numbers are born-red. Re-pin the 7 storage-lane tests (engine-proofs x6 + orphan-recovery x1) modulo that renumbering — zero production code touched. - golden-storage-drive.ts: per-turn citation-normalized digest (shaCiteNorm) and state digest modulo stripOneTimeAlignment (stateDigestAligned); the deep-sort helper is hoisted into golden-canonical.ts for sharing. - corrupt-index-child.ts: new stripOneTimeAlignment normalizer — strips the refLedgerAligned stamp and re-homes refs onto canonical raw-id order so a clean full-mode read and the legacy fallback load compare equal. - storage-engine-proofs.test.ts: wire legs assert shaCiteNorm equality vs the legacy restart leg and the committed goldens; state legs assert stateDigestAligned; rollback leg now spells BILI_STORAGE_UNIFIED=legacy explicitly (#3818 flip: absent resolves to full, deleting the env would vacate the legacy path). - storage-orphan-recovery.test.ts: pins the content-addressable byRaw->byRef round-trip instead of a counter-era m00003 literal. - golden/storage-render/*.json regenerated with shaCiteNorm/stateDigestAligned. Converges the master-red storage-lane family (#4599 plan A, owner directive).
…dger parity masks The #4316 one-time renumbering restamped messageRefs but left state.tokenSnapshot keyed by the OLD refs, so post-restart bodies carried stale per-message token counts (1.3K vs 1.4K) and the full-mode-restart wire drifted off the legacy baseline (#4599). persist.ts: two-pass remap (stage moves, delete old keys, then write new keys) so entries whose new ref collides with another move's old ref survive; fires only when superseded refs exist. Aligned normalizer (corrupt-index-child.ts): the cacheLedger's segment bookkeeping (be/bl/bs, unfinalized tail line), the derived agg block, and per-line miss-attribution (prov/seam) are restart experience, not content — no-restart and restart legs legitimately differ there. Masked in the aligned view only; raw digests keep full accounting. Goldens regenerated (stateDigest + aligned). storage-engine-proofs 12/12, golden-storage-render 5/5 locally.
…persisted state (#4599) The fork-survival test asserted immediately after the compress tool response, but the fold lands in the live session state asynchronously (#3573 replay semantics). Slow CI runners persisted the pre-fold state ('0 block(s) (compressions=0)' cold load) and the deepEqual diverged. Bounded 5s wait on the in-memory fold; fast machines exit on the first iteration. Product semantics unchanged.
The #4316 cold-load alignment reads the unified sidecar; its ingest of just-flushed records completes asynchronously. Slow runners compared the pre-ingest state and saw refLedgerAligned missing while the fold itself had landed. Poll (bounded 5s) until the loaded state carries the flag; fast machines exit on the first probe.
Git-for-Windows autocrlf checks the bundled .md posts out with CRLF; the exact-match '---' fences then failed to parse and every post lost its title/excerpt/date frontmatter (Windows CI: 'bundled meet-bili post found' assertion). Normalize CRLF to LF inside parseFrontmatter when the fence is CRLF-shaped; LF content is byte-identical as before. Regression test with a CRLF fixture.
… storage mode (#4599) The #4316 cold-load alignment sets refLedgerAligned only when the unified store is available; config-clean machines (CI runners) run legacy mode where it is an honest skip. The test asserted the flag unconditionally, so every shard carrying this file was born-red on CI while unified-config machines passed. Expectation now mirrors store.replicasAvailable(); verified 49/49 in BOTH modes locally.
…4743) The upstream 429 governor (upstreamThrottleLocalReject / noteUpstreamThrottle429 / noteUpstreamThrottleResolved) was wired only on the relay forwarding path. Three loop-lane chokepoints bypassed it, so a 429-storming upstream never armed the egress hold on the lane most prone to storms: - src/compress-loop-responses.ts — Responses-wire compress loop round-2+ resends - src/preflight.ts — preflight refold summary calls - src/reasoning-guard.ts — reasoning-guard continuation rounds Each now reports its verdict to the observe pair: a 429 arms the egress hold for the next client turn; a settled non-429 clears it. Observe-only (no local gate), matching the main-loop fix in #4725 — a mid-loop local reject would change re-request behavior under load. The observation split follows each site's fetch primitive: fetchWithRetry sites report at the settled boundary (a 429 exhausting the budget throws UpstreamHttpError(429)); fetchWithTimeout sites report per response, like relay. Adds tests/issue4743-loop-lane-429-observe.test.ts — three wiring pins that drive the real code path against a 429 upstream and assert the hold armed; they fail without the fix (verified: 0/3 pass on the unpatched tree). Closes #4743
(ai: vllm-qwen/qwen3.8-27b) 这是 agent PR( 查重:不重复。 与 #4725(主循环,现为空壳 PR)、#4752(degenerate-retry,在途)同属 #4743 覆盖家族;本 PR 覆盖的三个文件(responses-wire compress loop / preflight refold / reasoning-guard rounds)与它们互不相交,无其他重叠条目。 标签: bug / scope/wiring / p2(wiring 遗漏;#4204 已覆盖主转发路径,此处是 429 风暴下 loop 车道放大问题,非整体不可用) 本地核验(PR head e7e38d4,base = 当前 master,落后 0 commit):
review 中发现一个小偏差,稍后补独立标注 commit:compress-loop 站点在 fetchWithRetry 抛出非 429 的 UpstreamHttpError(如 fail-fast 400)时没有上报 resolved——而 governor 契约(#4204 模块头注释「ANY resolved non-429 … clears the hold」)以及 relay 和 #4752 的写法都是任何已决定的非 429 都清 hold。一行对齐 + 一行测试钉住。 |
… non-429 (#4743 review fix) fetchWithRetry throws UpstreamHttpError for terminal non-ok verdicts; the boundary observation armed on 429 but stayed silent on e.g. a fail-fast 400, whereas the #4204 contract clears the hold on ANY resolved non-429 (the endpoint demonstrably processed a request) — relay parity, same as the sibling #4752 site. Transport deaths remain unobserved (circuit-breaker territory, #3148). The responses-wire pin gains a re-arm -> fail-fast-400 phase asserting the hold lifts (verified failing without the fix line).
(ai: vllm-qwen/qwen3.8-27b) #4772 审核结论(agent 自报 PR,审核模式:不重做,核对代码 + 盯 CI)。 做了什么
八问
状态与判定
|
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g [email protected]This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr4772.tgz
npm install -g packageInstall instructions are refreshed on each push. |
… per-response (#4743 review fix) The per-response arm added by this PR in the preflight walk self-blocked: the fits-case forward runs AFTER preflight and passes the relay gate, so a summary-probe 429 armed the hold that then answered the SAME request's own forward locally — violating the #301 contract (payload fits + preflight 429 → still forwarded; CI red on tests/preflight-fail-fast.test.ts, reproduced locally: 429 !== 200). Fix: the walk reports resolved per response (settled non-429 clears); the arm moves to the caller's withhold verdict in server.ts (kind === upstream && status === 429 → noteUpstreamThrottle429 with null retry-after). Nothing is forwarded on a withheld request, so the armed hold only answers the NEXT turn's relay forward locally — exactly the #4743 ask. Doomed-413 exits stay un-armed (they spend no summary call / different evidence). Pins: new e2e in tests/preflight-fail-fast.test.ts (503 withhold arms → next small turn gets local 429 stage:forward, zero egress; FAILS without the arm line — verified). The issue4743 preflight pin now asserts the anti-self-block (no arm from a 429 probe alone) + cleared-on-200.
(ai: vllm-qwen/qwen3.8-27b) Review update — found and fixed a self-block regression in this PR's preflight site. My earlier "mergeable once CI green" verdict is superseded: CI went red exactly where predicted, and it was this PR's code, not inherited master red. Fix is pushed as Mechanism. The per-response 429 arm added in the preflight walk ( Fix (
Pins. New e2e in Structured 八问:
行为变更披露(vs 本 PR 第一版,非 vs master):preflight 摘要 429 不再在请求中途武装 hold,改在 withhold 判决时武装。相对 master 的净效果:fits 场景行为不变(照常转发);withhold 场景从「不武装」变为「武装下一轮闸门」——后者即本 PR 的功能本身。 判定:代码审核完成(两轮 review fixes 均已落地: |
…mspec wrap on win32 (#4599)
The ci-check lane's expectation change (c496cfb) exposed a real Windows bug: stripInheritedProxy builds inlineEnv as a spread copy of process.env, which keeps the original key casing (ComSpec) — Windows env vars are case-insensitive but plain objects are not. planClientSpawn's exact env.COMSPEC probe then missed it and silently degraded every wrapped spawn to the bare-name 'cmd.exe' fallback (verified by repro on a real win32 box: childEnv.COMSPEC=undefined while process.env.COMSPEC was set). Fix: probe COMSPEC/ComSpec/comspec in planClientSpawn and planDshSpawn. Verified on real Windows (Node 22): launcher-opencode-inline + fix-679 + dsh-refresh 37/37; host tsc clean, 35 pass / 2 win32-skips.
…OMSPEC case-probe c496cfb re-derived the win32 expectation from ambient process.env, which still mismatches production: Node's process.env member access is case-insensitive on Windows while the child env handed to the planner is a plain object whose key keeps the OS casing (ComSpec), so the exact env.COMSPEC probe fell through to the bare-name fallback. The assertion now derives from the child's OWN captured env/options and probes COMSPEC/ComSpec/comspec exactly like planClientSpawn does post-dfeea8a11 — self-contained under any concurrent-test env churn in single-process CI shards — and additionally pins windowsVerbatimArguments=true on the wrap.
Summary
The upstream 429 governor (
upstreamThrottleLocalReject/noteUpstreamThrottle429/noteUpstreamThrottleResolved, #4204) was wired only on the relay forwarding path. Three loop-lane egress chokepoints bypassed it, so when the upstream continuously returns 429 for compress requests, the governor never arms on the lane most prone to storms. This wires them in.What changed
Each site now reports its verdict to the governor's observe pair — a 429 arms the egress hold for the next client turn; a settled non-429 clears it:
Observe-only, no local gate — matching the main-loop fix in #4725. A mid-loop local reject would change re-request behavior under load; the issue's ask ("stop amplifying") is met by arming the hold so the NEXT turn's relay forward answers locally instead of storming again.
The observation split follows each site's fetch primitive:
fetchWithRetryresolves only ok responses and throwsUpstreamHttpError(status)on a terminal non-ok, so those sites report at the boundary;fetchWithTimeouthands back the raw response, so those sites report per response exactly as the relay path does.Coverage map (#4743 family)
These three together close the loop-lane coverage for #4743; they touch disjoint files.
Tests
tests/issue4743-loop-lane-429-observe.test.ts — three wiring pins that drive the real code path against a 429 upstream and assert the hold armed (and cleared on a later 200). Unlike the #4725 governor-contract pin, these FAIL on the unpatched tree (verified locally: 0/3 pass without the fix, 3/3 with). Targeted regression suites for the touched paths (reasoning-guard, preflight-transport-recovery, preflight-transient-empty-retry, responses-guard): 25/25 green locally.
Closes #4743
Review fixes (agent review, #4772 thread)
50b2bf859— loop-lane boundary report: a terminal NON-429UpstreamHttpError(e.g. fail-fast 400) now reportsresolvedto the governor, matching the Provider 429 (concurrency-style limit): turn re-driven every few seconds for minutes, keeps the account pinned at the limit #4204 contract ("ANY resolved non-429 … clears the hold") and the relay / [PR] fix: review-findings wave — 7 fixes from the #4725 pre-release audit (P1×2 + P2×3 + P3×2) #4752 sites; transport deaths stay unobserved ([bug] 上游在会话中途不可用时,bili 把一次故障放大成四重代价:固定节奏 20 连击全部冷发、输出预算被静默钳制 83 次(最狠 65536→1360)、紧急收缩与 nudge 停摆均不可见 #3148 territory). The responses-wire pin gains a re-arm → fail-fast-400 phase asserting the hold lifts (verified failing without the fix line).c1e0b726b— self-block regression found in review and fixed. The per-response 429 arm in the preflight walk violated the fix: honor anthropic-beta context-1m to size the context window #301 contract: the fits-case forward runs AFTER preflight and passes the relay gate, so a summary-probe 429 armed the hold that then answered the SAME request's own forward locally (CI red: tests/preflight-fail-fast.test.ts fix: honor anthropic-beta context-1m to size the context window #301 e2e, reproduced locally429 !== 200). Fix: the walk reports resolved per response only; the arm moves to the caller's withhold verdict in server.ts (kind === "upstream" && status === 429→ note with null retry-after). Nothing is forwarded on a withheld request, so the armed hold answers only the NEXT turn's relay forward locally — the [wiring] 429 governor is invisible to the loop lane (compress round-2+ resends do not go through gate/observe) #4743 ask is preserved. Doomed-413 exits stay un-armed. New e2e pin (503 withhold arms → next small turn gets local 429 stage:forward, zero egress) verified failing without the fix line; issue4743 preflight pin now asserts the anti-self-block direction.Behavior change vs this PR's first revision (not vs master): preflight summary 429 no longer arms the hold mid-request; it arms at the withhold verdict instead. Net effect on master behavior: unchanged for the fits-case (still forwards); the withhold-case now arms where master did not (that is the feature).