Repository navigation
[PR] fix(#5140): name the failed raw-tail reconstruction gate instead of failing closed silently - #5188
Merged
Conversation
…ailing closed silently The #5144 cold-restored snapshot path fails closed with a bare SNAPSHOT_UNAVAILABLE when any of its five reconstruction gates misses. On master CI the #5140 regression subtest has been red shard-after-shard with zero signal about WHICH gate fired — the five null returns were indistinguishable from the server log and from the client-visible error. Every miss now records its gate (empty-lastPassIds / replicasUnavailable / empty-tailSeqs / null-tailViewRows / gap-tailRow / unparseable-row / bad-row-shape / length-mismatch / id-mismatch / cap-exceeded), logs one warn line per session+gate per 10s, and appends [reconstruction: <gate>] to the thrown SNAPSHOT_UNAVAILABLE message so a 409 body says exactly why the tail could not be rebuilt. Behavior is unchanged — same statuses, same codes, same base messages; pure observability ahead of the root fix.
ranxianglei
changed the title
Contributor
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.7003-pr.5188.753d764This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5188.tgz
npm install -g packageInstall instructions are refreshed on each push. |
This was referenced Oct 30, 2026
…d sidecar; recover an unlucky first open from the save path (#5188) Two independent one-way degradations made the unified store vanish under CI load and turned #5144's fork raw-tail rebuild into hard 409s: 1. The 3-strike disable latch counted every ingest error equally. Under machine load sqlite surfaces resource-class errors (locks held past the 5s busy_timeout, momentary disk-full, fd spikes) that settle on their own; three ordinary hits latched the sidecar legacy-only for the process lifetime. noteIngestFailure now classifies via isTransientContentionError and only structural/operator-evidence errors (corruption, readonly, NOTADB, internal invariants) count toward the latch. lastError stays fresh either way; an already-latched sidecar stops re-warning on every save. 2. A FIRST synchronous open that failed transiently had no retry path at all: maybeOpen warns once and the process runs legacy-only forever. The save path is now the 'still wanted' signal: SessionStore.tryRecoverSidecar retries maybeOpen at most once per 30 s cooldown window (fixed constant, no config surface); on success the unified surfaces resume WITHOUT a restart (status wire clears via recordSidecarInstance inside maybeOpen). In-process probe evidence from the failing CI shard shape: '[recon-debug] ... gateB-replicasUnavailable sid=fork-parent-* disj=disabled: lastError=SQLITE_BUSY: database is busy' (loop-A-r2/r3, tmp/ci-repro/).
Owner
Author
|
(ai: sglang-gpu0/qwen3.8-27b) Root fix landed on this branch (commit 2 + master sync): probes captured the exact failing state ( |
…-path gate suffix (#5188) Review of this PR found that appending [reconstruction: <gate>] to EVERY unavailable-snapshot refusal breaks the pinned D-B contract (tests/plugin-fork.test.ts "retention cap fails closed"): a capped session refuses for CAP policy, yet the suffix names the incidental reconstruction gate hit first (e.g. replicasUnavailable) — misleading attribution plus a red exact-string assertion. - forkSnapshot appends the gate suffix only to the generic "raw snapshot unavailable" base; the capped base stays byte-exact (D-B contract untouched). - New regression test pins the generic path: a fresh conversation's 409 body carries "[reconstruction: empty-lastPassIds]".
…ilure cannot strand cold-restore snapshots (#5188) CI evidence on cd4b9cd: all three flaked shards' 409 bodies now name the gate ([reconstruction: replicasUnavailable]), proving the residual #5140 flake is the open-failure half of sidecar availability, not the latch half. The earlier recovery fired only from the save path on a 30 s cooldown stamped at construction; #5140's cold-restore snapshot request arrives milliseconds after construction, before any save, and kept failing closed. - no constructor-time cooldown stamp: the first retry is immediately eligible - replicasAvailable() makes one best-effort reopen per 100 ms read-check slot while the sidecar is missing (the save path keeps its 30 s pacing) - sidecarDownReason() names which half is down (sidecar-missing vs sidecar-disabled) in the fork raw-tail gate's warn line and 409 suffix - tests: dual-pace gating pinned both directions plus a deterministic #5140 timeline (ctor fail -> resource clears -> a plain read recovers, no save involved); negative control verified red on the pre-fix source
…behind-guard round 3)
…5188) The #5198 batched view verifier (master) only fetches messages for the tail seqs it needs; this suite's fixture carries none, so the old getMessages seam stopped firing and the three latch integration tests saw zero failures. getBlocks is the verify leg's unconditional first read-back — same catch, same classification path under test.
…HARD_MAX_BYTES #4788 D2 added the per-session resident hard cap (config field sessions.hardMaxBytes / env BILI_SESSION_RESIDENT_HARD_MAX_BYTES) but never registered the surface: no YAML seed entry, no LIFECYCLE.md row, and a hand-edited EN-only CONFIGURATION.md row that diverged from the generator. That tripped config-surface-guard, docs-coverage, lifecycle-ledger-guard and docs-sync on master. Add the seed entry (restoring the author's original EN wording + the #4792 decision ref + honest dynamic default), the matching ZH rows, and the LIFECYCLE.md §2 ledger entry, then regenerate the config docs so the artifact matches the seed again.
…k test The #4666 suite's terminal-stamp asserts were moved to v9 when the schema advanced (commit 'expect the v9 stamp in the #4666 migration suite'), but the concurrent-boots lock test's final assert was missed and still expected v8. With MYSQL_SCHEMA_VERSION now 9 (#4796 zero-ref index — index-only, no row change), the migrator legitimately stamps 9 after the lock frees, so 'migration completed after the lock freed' asserted 9!=='8'. Bump to 9 to match the three sibling asserts; the lock-serialization discriminators (mid-flight stamp 7, rowCount 8, dedup collapse) are untouched.
…hs (#5188 round 3) #5140's flake survived rounds 1-2 because the cold-restore GET is the LAST consumer touch in its timeline: the ctor open dies on a shard-load spike, the resource clears milliseconds later, but no read-check slot fires before the GET arrives, so replicasAvailable() still answers false at request time. The two user-visible 409 paths (GET snapshot / POST fork) now wait a bounded moment (750 ms budget, 50 ms poll; recovery attempts stay paced by the existing 100 ms read-check slot) for that self-recovery before racing it — only when the sidecar is merely missing and reconstruction is actually needed; latched- disabled and healthy paths return immediately, unchanged. Complements the branch-side fix d4d4c82 (contention-suite fault seam moved to getBlocks after #5198's batched view verifier made the old getMessages seam stop firing for tail-less fixtures — the verified root cause of the suite reds on run 41007166321: GH Actions checks out refs/pull/N/merge, so the run picked up #5198 from master even though the PR head predated it).
Owner
Author
(ai: vllm-qwen/qwen3.8-27b) |
Owner
Author
(ai: vllm-qwen/qwen3.8-27b) 结论先行:本轮 5 道红检查(mysql-contract + 4 道静态文档门)全部继承自 master,不是本 PR 的代码;修复集已在 ci-fixes@ 七问:
收工判定:等 CI 重跑——预期 4 道静态门 + mysql 转绿、分片恢复运行;若 #5140 子测试再红,按决策单 #5215 走,不再自动处理。继承红状态已按纪律报至 #4599。 |
Contributor
📦 Merged Build PublishedThis merge to master was published as npm install -g billion-context@0.0.930-master.44067a5 # this exact build
npm install -g billion-context@master # always the newest merged build
|
Owner
Author
(ai: vllm-qwen/qwen3.8-27b) |
ranxianglei
pushed a commit
that referenced
this pull request
Oct 30, 2026
…pen into the no-degradation policy - 3 text conflicts in src/storage/shadow.ts resolved to OUR side: the incoming pre-policy breaker latch (MAX_CONSECUTIVE_FAILURES) and its transient-contention helper stay withdrawn — noteIngestFailure keeps warning per unique error and NEVER disables. - #5188 demand-driven sidecar reopen is kept but constrained by the final form: tryRecoverSidecar() now (a) skips forever after a proven-corrupt alarm boot (hadIntegrityQuickCheckFailure — one loud scan, never a re-scan storm of the same bytes) and (b) wraps maybeOpen in try/catch because under the owner chain MaybeOpen THROWS for transient failures instead of returning null (save/evict keep their never-throw contract). - tests/issue5140-contention-not-disabling.test.ts and tests/issue5188-sidecar-recoverable-open.test.ts re-pinned to the terminal semantics (no self-disable ever; no-engine ask REFUSES to start; corrupt index alarms once and hooks never retry). - tests/plugin-fork.test.ts: the broken-dir fixture now pins the refusal (constructor throws EEXIST) because construction-time silent degradation is withdrawn; the corrupt-index boot serves forks via envelopes; process-proxy restart closes our own side first (one writer per sessions dir at a time is THE #5046 policy).
ranxianglei
pushed a commit
to adlternative/billion-context
that referenced
this pull request
Oct 30, 2026
…k-snapshot recon gates) into PR branch (ranxianglei#4599)
ranxianglei
pushed a commit
that referenced
this pull request
Oct 30, 2026
…t — merge-guard sync Master advanced 09a5b5b -> 44067a5 (+12 commits: persist.ts concurrency #5140/#5188, plugin.ts fork, storage/shadow.ts; 3 test files). Based on current server head 962a04c (concurrent worker's ci-fixes sync built on r18 3ff7f12). Clean auto-merge, ZERO conflicts; delta touches NO PR-area file. PR change intact: zero-fold calibrated-fit predicate at src/server.ts:4087 (#4930) + tests/issue4930-zero-fold-calibrated-fit.test.ts. No lockfile/version change. Validated: typecheck clean; targeted PR/preflight/yield suites 48/48 green.
ranxianglei
pushed a commit
that referenced
this pull request
Oct 30, 2026
Behind-guard sync: merges master's #5188 (fork/snapshot sidecar recovery + reconstruction-reason error naming) into the #4266 thinking-gate-lift branch. Single conflict in forkSnapshot resolved as the union of two orthogonal changes: keep master's reconMissReason [reconstruction: why] error naming AND keep #4900's lifted thinking hard gate (signed thinking projects to comparable reasoning messages; only publicSnapshotTextComparable===false still refuses). Targeted tests green: plugin-fork, issue4266-fork-thinking-gate, issue5188-sidecar-recoverable-open, issue5140-cold-raw-tail-fork (71 pass); typecheck clean.
ranxianglei
pushed a commit
that referenced
this pull request
Oct 30, 2026
- plugin-fork: pin the #5140 cold-restore snapshot test to unified-full — under the global legacy test baseline its sidecar never opens, so the index-reconstruction gate can never pass (409 replicasUnavailable sidecar-missing) - plugin-fork: pin the new broken-dir refuse-to-start assertion to full mode; legacy never touches sqlite at all so the ctor cannot throw there - multi-instance-live: spawn children with BILI_STORAGE_UNIFIED=full despite the global legacy baseline pin, else instance A holds no index.db.lock and B has nothing to refuse over (boot-refusal needs a live foreign writer) - export-jsonl: mysql-without-DSN now refuses at open time — assert on the actual refusal message instead of the pre-refusal #3388 tag - env-only.yaml: sync BILI_STORAGE_QUICK_CHECK seed with the final alarm-only policy wording already shipped in CONFIGURATION.md/.zh-CN (docs-sync gate)
This was referenced Oct 30, 2026
[PR] fix: panel "saved $" replay cost priced at input tier instead of cache-write tier (#4956)
#4963
Open
Closed
[PR] Stop advertising compress tools on plugin rounds that don't declare them (pi round-1 fix)
#5009
Open
Merged
ranxianglei
pushed a commit
that referenced
this pull request
Nov 3, 2026
…rage stamp Review follow-up on 0964efe: inside the !replicasAvailable() branch, liveStorageStamp === "legacy" is always true (the stamp reads legacy whenever the sidecar is absent/disabled), so the previous ternary's else branch was dead code and every unified-configured instance with a down sidecar (3-strike latch, corrupt-index boot-without-index) got stamped "durable storage is legacy-only" — mislabeling exactly the recoverable/ latched cases F4 exists to tell apart. Discriminate on the constructor-time resolution instead: new SessionStore.configuredUnified getter (enabled AND knob tier non-legacy AND no encryption hard-block; the block flag is now stored as a ctor-time field instead of a local const). Structurally legacy-only instances keep the actionable config message; unified-configured instances whose sidecar is down right now keep the #5188 sidecarDownReason() vocabulary. Tests: three store-level pins appended to tests/issue5595-fork-empty-tool-args.test.ts (legacy knob / full with a 3-strike-latched sidecar via the #4942 seam / encryption hard-block), including the conflation assertion itself (live stamp reads "legacy" while configuredUnified stays true).
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.
(ai: sglang-gpu0/qwen3.8-27b)
What was broken
Since #5144 merged, its own #5140 regression subtest (
HTTP a cold-restored non-retained parent serves its intact raw tail immediately) has been red in every failing CI shard: the snapshot endpoint answers409 SNAPSHOT_UNAVAILABLEwhere the test expects the raw-tail rebuild. The failure path is five null-return gates deep, and since #5089 the harness children's stderr tee is muted after the log-file leg proves alive — so CI showed a bare409 !== 200with zero signal about WHICH gate fired. Reproduced locally under the full shard-3 load (green standalone ⇒ load-correlated, not a plain code break).What this does
Commit 1 — name the gate. Every reconstruction miss now names its gate (
empty-lastPassIds/replicasUnavailable/empty-tailSeqs/null-tailViewRows/gap-tailRow/unparseable-row/bad-row-shape/length-mismatch/id-mismatch/cap-exceeded): one warn per session+gate per 10 s, and the thrown error carries[reconstruction: <gate>]so a 409 body says exactly why.Commit 2 — heal the two one-way degradations the probes proved were behind it. In-process probes on the failing shard shape captured the exact state at failure time:
gateB-replicasUnavailable … disj=disabled: lastError=SQLITE_BUSY: database is busy. Two mechanisms had each turned a TRANSIENT condition into a permanent one:noteIngestFailurenow classifies viaisTransientContentionError; only structural/operator-evidence errors (corruption, NOTADB, readonly-database, internal invariants) count toward the latch. Bidirectional test matrix pins both classes through the realrecord()machinery.maybeOpenonce per 30 s cooldown window (SessionStore.tryRecoverSidecar, fixed constant); when the resource returns, unified surfaces resume in place and the status wire clears viarecordSidecarInstance.Behavior changes (disclosed)
lastError/status and log once as first-failure warns). Persistent lock pathology will therefore stay "unified-but-flaky" instead of tripping legacy-only; structural breakage latches exactly as before.Config surface
None new / none changed (the reopen cooldown is a fixed constant + a test seam, mirroring
setSqliteEnginesUnavailableForTests).Boundary (deliberately not changed)
Read-path failures at cold load (
UnifiedStore.load) still return null fail-SOFT without striking: open-time corruption already gets the #4175 quarantine-and-reopen-fresh pass, and making load errors latch would change every cold-start error surface — tracked as follow-up scope, out of this PR's blast radius.Verification
tsc clean on this head. New suites:
tests/issue5140-contention-not-disabling.test.ts(classification truth table + strike semantics through real record() — 5/5),tests/issue5188-sidecar-recoverable-open.test.ts(recovery-without-restart + cooldown gating, both directions — 2/2). Neighbors green locally: issue5103 startup-recovery-visibility, acp-status-storage-degraded, issue5140-cold-raw-tail-fork, web-storage-banner, issue-split-brain-surface, issue4796-gc-batched-sweep, issue4990-web-single-store. The authoritative full-suite verdict is the CI lanes on this head — if the flaky red fires again, it now prints the gate name AND the store self-heals the states that used to cause it.Refs #5140 (regression test + failure shape), #5144 (introducing change).
Reviewer note — behavior changes (added by reviewing agent; see issue #5188 thread)
Per the behavior-change disclosure rule, full old→new for BOTH commits, plus two reviewer fix commits:
2d6d1b0(corrects a regression this PR's own first commit introduced against the pinned D-B contract) and28d538c(second round, driven by CI evidence — see below).shadow.tsnoteIngestFailure)stats.disabled); unified surfaces fall back to legacy foreverstats.lastError+ per-read fallback warns only. Breakage-class failures (CORRUPT / NOTADB / readonly / missing-row) still latch at exactly 3 strikes, unchangedpersist.ts)SHADOW_REOPEN_COOLDOWN_MS) and the read path (replicasAvailable()) once per 100 ms slot (SHADOW_REOPEN_READ_CHECK_MS) while the sidecar is missing; NO constructor-time cooldown stamp — the first retry is immediately eligible on first need; one info log on recovery; ctor still warns once at failed-opencd4b9cd5f: all three flaked shards' 409 bodies named[reconstruction: replicasUnavailable])[reconstruction: <gate>]suffix naming the fired gate — GENERIC base only; the capped base stays byte-exact (reviewer correction: a suffix under the cap would misattribute the refusal reason and broke the pinned exact-string D-B test). ThereplicasUnavailablegate additionally names WHICH half is down:sidecar-missing(transient, recoverable without restart) vssidecar-disabled(structural latch, stays down by design)Status codes, response shapes, and both base error strings are unchanged. No config surface added or changed.
Second-round review fix (
28d538c). After the ci-fixes merge (cd4b9cd5f), CI ran the full matrix: the #5140 subtest still flaked in 3 shards — but now every 409 body NAMED ITS GATE (commit 1 working as designed):[reconstruction: replicasUnavailable]in all three. That localized the residual flake to the open-failure half of sidecar availability: the constructor's failed sync open stamped a 30 s cooldown and only the SAVE path ever retried, while #5140's timeline is ctor-fail → resource clears in ms → pure READ (cold-restore snapshot request) arrives before any save. Fix: no constructor-time stamp (first retry immediately eligible) + a demand-driven best-effort reopen insidereplicasAvailable()paced by its own 100 ms read-check slot (steady state costs two field reads; bounded ≤10 attempts/s while degraded). Pinned by a deterministic test reproducing #5140's exact timeline with production pacing (ctor fail → resource returns → a plain read recovers, no save anywhere), plus dual-pace gating asserted both directions; negative control verified red on the pre-fix source.