Repository navigation
[PR] fix: validate global cacheKeepalive block at config load (#4200) - #4233
ranxianglei wants to merge 6 commits into
Conversation
…heKeepalive in web whole-file save (#4200 follow-up)
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.6191-pr.4233.f2ee720This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr4233.tgz
npm install -g packageInstall instructions are refreshed on each push. |
(ai: vllm-qwen/qwen3.8-27b) 查重:不重复。 检索 open+closed issue 与近期 PR,无其他条目认领「全局 cacheKeepalive 块的文件加载校验」;本 PR 是 #4200 的唯一工作项(#4200 线程内此前的分析楼层正是产出本 PR 的接单工作,不是并行认领)。 标签:bug / p3 / scope/wiring(+pr) —— 配置面静默失效缺陷,仅手工编辑配置文件可触发(web 保存路径本来就拒这些值),失效后果是整块回落默认值加一条错误日志、无数据影响,判 p3;改动面在请求管道的配置加载与 web 保存路径,scope/wiring。已打。 审核结论(独立复核,未引用 #4200 线程已有结论)判定:rebase 后可合。 代码审查通过;CI 两条红腿判定与本机红风暴同源、非本 PR 引入(证据见第 5 问),建议 rerun-failed-jobs 转绿后合并。分支落后 master 120 个提交,GitHub 报 mergeable=true 无冲突,合并前需先 rebase。 八问:
行为变更披露(有意):旧 = 非法全局 cacheKeepalive 被静默忽略(逐字段回落默认)、其余配置正常工作;web 保存一个 → 请求死掉无响应。新 = 非法全局 cacheKeepalive → 整个配置文件在加载时被拒绝(一条点名字段的错误日志,所有键回落默认/env 直到修好);web 保存一个 → 干净 400 点名字段。合法文件字节级不受影响;因 web UI 本就拒绝这些值,加载侧变化只会被手工编辑的文件命中。 审核中发现的小问题(留痕): 合并判定:代码审查通过,rebase 后可合;CI 两条红腿判定为进行中的仓库级红风暴(master 同样红、已知机制修复均未合入),rerun-failed-jobs 或 rebase 重新触发转绿后即可合并。合并动作留给你。 |
(ai: vllm-qwen/qwen3.8-27b) 查重: 不重复。 检索 open+closed issues/PRs(cacheKeepalive 标题精确 + validation 关键词组合):仅有源 issue #4200 与本 PR,无其他在途条目覆盖「全局块加载期校验」或「web 保存 400」这两个面。 标签: 独立审核(未复用 #4200 的结论;重读了 diff、
测试有效性:7 个测试断言都有牙(deepEqual 到 CI 现状(合并关键):42 个 check run 中 40 绿/skip,两个红 lane 已查日志归因:
判定:可合。 方向正确、diff 最小(3 文件 +178/-1,每行都与目的相关)、遵循仓库惯例、测试有牙、行为变更已按政策披露(旧→新+原因)。建议 rerun 上述两个红 job(均与本 diff 无关),转绿后合并即可。本次纯审核,我未推送任何 commit——diff 本身没有需要顺手修的缺陷。 |
…help-merge sweep)
…help-merge sweep)
master window commits 293ae65/de5e03659 bumped kernel/package.json to 0.0.165 without regenerating the lockfiles; lock-sync re-resolution then rewrites the stale recorded versions (root 0.0.163, kernel 0.0.164) and fails. Pure recorded-version sync via npm install --package-lock-only; no dependency or semantic change.
What & why
The GLOBAL
cacheKeepaliveblock at the config root was never validated on file load: it was only listed inKNOWN_TOP_LEVEL_KEYS, so a hand-edited~/.config/billion-context/billion-context.jsoncontaining e.g.{"cacheKeepalive": {"ttlFactor": "high"}}loaded clean while the identical typo underproviders.<url>.cacheKeepaliverejected loudly — same typo behaving differently by nesting level (config-surface silent-failure class). The consumer (resolveCacheKeepalive) then silently dropped the bad field to its default.Fix (commit 1)
loadConfigFilenow runs the same strict #1909-stance parser (parseCacheKeepaliveSettings) on the global block — validation only, well-formed blocks pass through as-is, mirroringparseRouteEntry. An invalid block throws into the file-load catch: one loud error naming the exact field, and the file is treated as unreadable ({}) — exactly how a malformed JSON file is handled today.nullstays "absent" per the existing web-save contract (src/web/api.tsskips null for this block).Follow-up (commit 2, small fix found while working)
The web whole-file save path called that same throwing parser without a try/catch: saving a bad global block via the web UI produced an unhandled rejection (error log, client's PUT gets no response) instead of the clean 400 every other field gets. Now caught → 400 with the parser's message. The file was never written before either, so no data impact.
Behavior change (intended, disclosed per policy)
cacheKeepalive→ silently ignored (per-field fallback to defaults), rest of the config kept working; web save of one → request dies without a response.cacheKeepalive→ whole config file rejected at load (one error log naming the field; all keys fall back to defaults/env until fixed); web save of one → clean 400 naming the field.Valid files are byte-for-byte unaffected. The web UI already refused these values on save, so only hand-edited files hit the load-side change.
Tests
New
tests/issue4200-global-cachekeepalive-validation.test.ts(7 tests): issue repro (badttlFactor→{}+ field-named error, logged once across repeated reads, cleared by fixing the file), unknown-key rejection, route-level symmetry pin, valid block pass-through reaching the resolver, null=absent contract, web save 400-with-field-name + file untouched, web save of a valid block still succeeds. Targeted suites green locally on the rebased head (this file + cache-keepalive + issue3572-jsonc-config + issue3572-web-jsonc-config + config-file-cache: 57/57); typecheck + build clean; full-suite verdict from CI.Closes #4200