Repository navigation
[PR] fix(dsh): make unresolvable client-build detection explicit instead of silent - #5309
ranxianglei wants to merge 1 commit into
Conversation
(ai: vllm-qwen/qwen3.8-27b) 查重:不重复。 这是 #4545(「高于已测版本」软提示,已合入 master)的后续修复——修的是该特性在 DSH 桌面版(asar 布局)结构性哑火的缺口;无并列 PR/issue 实现同样的「显式 unknown」提示。 标签(拟): 结构化八问
证据
合并前需处理的非代码项
判定:代码 merits 上可合——逻辑正确、改动最小、双向测试与文档到位、无披露外回归;待上面三项(尤其第 1 项的 issue 关联纠偏)清理后由 owner 合并。 |
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.9076-pr.5309.4da48d0This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5309.tgz
npm install -g packageInstall instructions are refreshed on each push. |
6566c7c to
e61c54f
Compare
(ai: vllm-qwen/qwen3.8-27b) Conflict resolved — PR rebased onto current master. 查重: 不适用(本次为既有 PR 的冲突修复运行,未新立项)。 What happened:
Verification (targeted, local — full-suite verdict belongs to remote CI on the pushed head):
Behavior-change flag (the rebase adds nothing beyond landing the PR's own intent): unresolvable dsh client builds now emit ONE explicit "could not detect" boot note instead of staying silent (old: silent → new: explicit note; — 八问 —
判定:冲突已清除,PR 现为 mergeable(blocked = 等新 head 的 CI 与评审);若新 head CI 再红,由本 lane 下一轮跟进。 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Conflict-fix run COMPLETE (auto-fix lane attempt 1/3) — confirming outcome; full detail in my previous comment above.
— 八问(简版)—
判定:冲突已清除、已推送、已验证(定向级);剩余风险仅为新 head 的全量 CI 结果——若红,由本 lane 下一轮跟进。 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) No action needed this run — the blocker is already resolved; verified against LIVE sources just now. This forward appears to have been generated from the pre-rebase
So: conflicts resolved (previous run), CI green (verified now), PR waiting only on review/merge. If a future run sees a NEW dirty state or red check on a newer head, that will be fresh work — this one is clean. — 八问(简版)—
判定:本轮空转收工;PR #5309 现状 = 无冲突 + CI 全绿,仅待人工评审/合并。 |
|
(ai: vllm-qwen/qwen3.8-27b) Review verdict: APPROVE (posted as a comment — GitHub rejects a formal APPROVE on the author's own PR) — independent end-to-end review of the rebased head Correctness. The fix does what it claims: Regression walk (existing behaviors, read pre-change code):
No wire/message-shape, threshold, timing, or cache/retry/cursor impact; boot diagnostic only, both compression modes N/A. Sole consumer in src/ is dsh-native.ts; nothing outside the module parses the note lines. Tests. Real teeth, bidirectional gates per #3617: the new test pins msg≠null + content substrings ([dsh], package name, "Could not detect", the knob env, CN half) plus a negative assert against the other note's string; knob-off Docs / config surface. Closes reference (resolves the earlier thread concern). GitHub issue #5302 IS the dsh-asar detection issue ("dsh desktop: newer-than-tested build warning never fires because version lookup cannot see packages inside app.asar") — House conventions. Single commit, 八问(中文结构化结论)
判定:可合并。 代码正确、最小、测试有牙(本地 10/10 + CI 全绿)、文档再生成幂等零漂移、行为变更已按规范披露旧值→新值。本轮未发现需顺手修的小问题。A/B 方案跟踪在 #5302,合并将自动关闭该 issue——由 owner 定夺。 |
Review lane done (review-worker-r1). PR #5309 head e61c54f: verdict APPROVE — posted on GitHub as a comment (formal APPROVE blocked: PAT == PR author, GitHub rejects self-approval) → #5309 (comment). Independent end-to-end pass: correctness verified at src/agent/client-build-note.ts:45-50 + dsh-native.ts:2801; full regression walk (knob-off/at-cap/newer/unparseable/throwing paths all unchanged, only delta = disclosed silent→warn for unresolvable+on); tests have teeth, re-run locally 10/10 + CI green at this head (30 checks); docs regeneration idempotent zero-drift + both guards green; |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) CI-engineer hand-off: all mechanical blockers cleared — PR is merge-ready, only the human merge step remains. Status of every gate, verified live this run:
One merge-time note carried over from the review (owner's call, non-blocking): merging will auto-close #5302 ( Merge when ready: #5309 (human-only step — I can't merge PRs). — 八问(简版)—
判定:CI 侧收工;球在 owner 脚下——合并即闭环(或先决定 #5302 去留)。 |
Closes #5302
What
The newer-than-tested client-build soft warning (#4545) structurally NEVER fires on DSH desktop (app.asar layout):
walkUpNodeModuleVersioncannot see the host package bundled inside app.asar, sofound === undefinedreturned null by design ("absence of evidence ≠ broken build"). The safety net was silently dead exactly where it matters most.This implements option C from the issue (minimal, zero layout assumptions): an UNRESOLVED version with the check enabled now emits ONE explicit bilingual boot note — "Could not detect the running @deepseek-ai/dsh version in this install layout … the newer-than-tested build check is INACTIVE here … set BILI_DSH_MAX_TESTED_VERSION="" to silence" — instead of staying silent. UNPARSEABLE versions keep failing open to silence; the two failure modes are now distinguished and documented everywhere. The knob silences everything, including the new note.
Behavior change (disclosed old → new)
[native-client]record + console banner). Web/profile layouts unaffected (the version resolves there); knob-off users stay fully silent; still soft by design — never gates or blocks traffic.Config surface
NONE. No new key/env var/flag — the existing
BILI_DSH_MAX_TESTED_VERSIONis merely referenced inside the note text as the escape hatch.Tests
tests/client-build-note.test.ts: the two pins encoding the old contract updated; bidirectional gates added (#3617): unresolvable + check-on → explicit note (lane label, npm id, EN/CN content, escape-hatch knob named, NOT the strictly-newer string); knob off (empty / whitespace) → null; explicitly raised cap → same unknown outcome; emit-channel test asserts both firing directions plus knob-off suppression in BOTH directions. Local: targeted suite 10/10 green, full typecheck clean. Docs: seed (website/config-reference/env-only.yaml) updated and artifacts regenerated withtools/gen-config-docs.mjs generate(no drift); LIFECYCLE.md ledger row updated.Checklist items awaiting owner input (managed inside #5302, non-blocking)