Skip to content

[PR] fix(ci): clear the three inherited master-red gates — two unused exports + the lost chat-pack seed - #5347

Closed
Louie0806 wants to merge 3 commits into
ranxianglei:masterfrom
Louie0806:2026-10-31_master-red-gates
Closed

Louie0806 wants to merge 3 commits into
ranxianglei:masterfrom
Louie0806:2026-10-31_master-red-gates

Conversation

@Louie0806

Copy link
Copy Markdown
Contributor

(ai: deepseek/deepseek-v41-flash)

Closes #5346
Refs #5317

What

Master has been carrying three inherited required-check failures. None of them come from any in-flight PR — every PR inherits them and goes red for reasons unrelated to its own diff.

Gate Finding Introduced by
unused-exports INTERNAL_ONLY src/pack-install.ts: PackInstallOptions b9218e857 (#5340)
unused-exports INTERNAL_ONLY src/codex-models.ts: CodexWindowSource 062948078 (#5305), tracked as #5317
docs-sync seed drift on compress.promptPack f096cb3fb (#5319)

Why each is master-red on its own

PackInstallOptions — installPackFromNpm is the only consumer and it lives in the same file; there is no cross-file import in src/ or tests/. The gate classifies it INTERNAL_ONLY precisely because total === own.

CodexWindowSource — already filed as #5317 and fixed identically in #5318. Included here so one PR clears the whole gate.

docs-sync — #5319 shipped a third builtin (src/chat-pack.ts:61-66, source: "shipped:chat") and a new resolution tier (src/compress-settings.ts:171-184), and hand-edited CONFIGURATION.md / CONFIGURATION.zh-CN.md to match — but never updated the generator seed at website/config-reference/compress.yaml. The seed still lists two builtins, so node tools/gen-config-docs.mjs generate rewrites the docs backwards on every run and git diff --exit-code fails.

This reproduces on pristine dc10c2d0b and on 485c5eaff with a clean tree:

git worktree add /tmp/m 485c5eaff -q && cd /tmp/m
node tools/gen-config-docs.mjs generate
git diff --stat
#  CONFIGURATION.md        | 2 +-
#  CONFIGURATION.zh-CN.md  | 2 +-

Fix

  1. src/pack-install.ts — drop export on PackInstallOptions; matches existing precedent (DiffOptions, OpencodeV2SetupOptions, SelfRegisterOptions are all unexported param types).
  2. src/codex-models.ts — drop export on CodexWindowSource (same change as [PR] fix(#5317): drop the internal-only CodexWindowSource export (master unused-exports gate) #5318).
  3. website/config-reference/compress.yaml — restore "chat" to the builtin list and shipped pack to the resolution chain, then commit the generator output.

Both type demotions are behavior-neutral: the gate's own classification is INTERNAL_ONLY = zero external references.

Verification (all run locally)

  • node scripts/check-unused-exports.mjs → clean (6 waived), exit 0
  • node tools/gen-config-docs.mjs generate && git diff --exit-code -- CONFIGURATION.md CONFIGURATION.zh-CN.md website/main.js website/docs/index.html → exit 0
  • Generator is a fixed point: running generate twice in a row produces a byte-identical tree
  • config-surface-guard / lifecycle-ledger-guard / check-test-import-graph → exit 0
  • npx tsc --noEmit → clean
  • npm run build → OK; check-dist-import-annotations + check-dist-external both OK
  • tests/pack-install.test.ts 6/6, tests/chat-pack.test.ts + tests/codex-models*.test.ts 15/15

Negative controls (this is the part that matters — the fix must be the thing that flips the gate):

Revert Gate result
both source demotions unused-export exit 1, both findings return
the seed row only docs-sync exit 1, 2-line diff returns

Scope note

Deliberately not touching anything else. This is untangling the inherited red so that real PRs become reviewable again — #5195 has been open about exactly this ("Master CI has been massively red since #5169"). Same-shape fixes were done before (#4533, #4530, #4524, #4534, #4520) but each closed individually without a standing guard.

The broader question — whether docs-sync/docs-coverage need a pre-merge guard so hand-editing generated docs can never land again — is left to the owner; this PR only restores the fixed point.

…lost config-docs seed (ranxianglei#5317, ranxianglei#5319)

The master branch has carried three inherited required-check failures:

1. unused-exports: INTERNAL_ONLY src/codex-models.ts CodexWindowSource
   (ranxianglei#5305 added the type with only same-file uses).
2. unused-exports: INTERNAL_ONLY src/pack-install.ts PackInstallOptions
   (ranxianglei#5340 added the param-type interface; installPackFromNpm is the only
   consumer and it lives in the same file).
3. docs-sync: ranxianglei#5319 shipped a 'chat' prompt pack and hand-edited
   CONFIGURATION.md/.zh-CN.md, but never updated the generator seed at
   website/config-reference/compress.yaml. The generator therefore rewrote
   the docs back to the pre-ranxianglei#5319 text on every run.

Both type demotions are behavior-neutral (the gate classifies them
INTERNAL_ONLY: no cross-file import exists). The seed row now carries the
'chat' builtin and the 'shipped pack' resolution tier, matching the code
in src/chat-settings.ts and src/compress-settings.ts.

Verified: unused-exports clean (6 waived), docs-sync generate is a fixed
point (no diff after commit), config-surface-guard / lifecycle-ledger-guard /
test-import-graph all exit 0, tsc --noEmit clean, npm run build OK.
@ranxianglei ranxianglei changed the title fix(ci): clear the three inherited master-red gates — two unused exports + the lost chat-pack seed [PR] fix(ci): clear the three inherited master-red gates — two unused exports + the lost chat-pack seed Oct 31, 2026
… shim + a stale ranxianglei#5103 assertion (ranxianglei#5340, ranxianglei#4125)

Two required-check failures reproduce on pristine master 485c5ea (and pass
on dc10c2d), so they are inherited, not introduced by the ranxianglei#5175 branches.
Both are fixed here, each with a negative control.

1. `family gate: no raw negative string slices outside the allowlist`
   (tests/text-safe.test.ts) — the gate is red because src/pack-install.ts
   spawns `npm` with a bare `execFileSync("npm", …)`. On Windows `npm` is
   `npm.cmd`, which CreateProcess cannot execute, so every Windows shard
   reported `npm was not found on PATH` and the whole pack-install suite
   failed there (`not ok 1547`, `not ok 1548`). Route it through
   `planDshSpawn`, the same cmd.exe /d /s /c planner the launcher, pi and
   dsh lanes already use. Verified: win32 -> `cmd.exe /d /s /c "npm …"`
   with windowsVerbatimArguments; linux -> unchanged bare `npm`.

2. That `.slice(-3)` is an ARRAY receiver (`split("\n")`), so it was never
   the ranxianglei#816/ranxianglei#1615 family bug — the gate only inspects the receiver text.
   Allowlist the exact expression rather than loosening the gate.

3. `ranxianglei#5103: explicit attachUrl target found but build-incompatible — refusal
   named with reason and remedy` pinned the literal remedy sentence
   "...converges to the same version". ranxianglei#4125 (fc53fd4) reworded it
   because a strictly newer same-lane holder is now adopted rather than
   deadlocked around, dropping "to the same version". Pin the stable
   fragment ("restart the host once its copy converges") so the assertion
   tracks the remedy instead of its old prose.

Negative controls: restoring the old `execFileSync` line / the old
assertion string makes the two suites red again (17/1 and 4/1); with the
fixes they are 18/0 and 5/0. Battery: pack-install 6, text-safe 18,
issue5103 5, dsh-native 71, dsh-refresh 19, win-cmd-argv 28 = 147 pass /
0 fail; `npx tsc --noEmit` clean; the four static gates exit 0.
@Louie0806

Copy link
Copy Markdown
Contributor Author

(ai: deepseek/deepseek-v41-flash)

Follow-up on the two required-check failures that were still red on 485c5eaff. Both reproduce on pristine master and pass on dc10c2d0b, so they are inherited rather than introduced by the #5175 line. Cleared in f5ddd362b and merged here as 1b0726e9e→ correct 1b0726e9e.

failing test root cause commit
family gate: no raw negative string slices outside the allowlist (tests/text-safe.test.ts) src/pack-install.ts:110 spawned npm with a bare execFileSync("npm", …). On Windows npm is npm.cmd, which CreateProcess cannot execute → every Windows shard reported npm was not found on PATH and the whole pack-install suite died there (not ok 1547, not ok 1548). b9218e857 (#5340)
#5103: explicit attachUrl target found but build-incompatible — refusal named with reason and remedy (tests/issue5103-startup-recovery-visibility.test.ts) The assertion pinned the literal remedy "...converges to the same version". A strictly newer same-lane holder is now adopted rather than deadlocked around, so that clause is gone from the message. fc53fd466 (#4125)

Fix for (1): route the spawn through planDshSpawn, the same cmd.exe /d /s /c planner launcher / pi-channel / dsh-channel already use. Measured plan shapes:

win32 -> {"command":"cmd.exe","args":["/d","/s","/c","\"npm install --prefix C:\\tmp --no-save x\""],"windowsVerbatimArguments":true}
linux -> {"command":"npm","args":["install","x"]}

So the Linux/CI path is byte-identical to before. The pre-existing if (/ENOENT/.test(err.message)) throw new Error("npm was not found on PATH") guard stays reachable: a genuine missing npm now surfaces as spawnSync res.error with code: ENOENT, which we rethrow into that handler.

Note on (1)'s gate hit specifically: the offender .slice(-3) is on stderr.trim().split("\n") — an array receiver, so it was never the #816/#1615 family bug the gate exists to catch. The gate only inspects the receiver text, so the honest fix is an exact-expression allowlist entry (same shape as the existing events.slice(-10) / lines.slice(-12) entries), not loosening the gate.

Fix for (2): pin the stable fragment restart the host once its copy converges so the assertion tracks the remedy rather than its old prose.

Negative controls (both reverted individually, confirmed red again):

  • restore the bare execFileSync line → text-safe 17 pass / 1 fail
  • restore the old assertion string → issue5103 4 pass / 1 fail
  • with both fixes → text-safe 18/0, issue5103 5/0

Battery on 1b0726e9e: pack-install 6, text-safe 18, issue5103 5, issue5175-drift-name-leak 34, store 29, dsh-native 71, dsh-refresh 19, win-cmd-argv 28 — all pass, 0 fail. npx tsc --noEmit clean; check-unused-exports, config-surface-guard, lifecycle-ledger-guard, check-test-import-graph all exit 0.

The store.test.ts "flushAll retries a dirty content store" failure did not reproduce locally (29/29, and the single test 12× in a row) — leaving that one as environmental/flaky unless it recurs.

… the next turn into a hard failure

Two defects in the ranxianglei#4788 phase-2 discharge path, both surfaced by the CI
matrix running under the `legacy` storage baseline:

1. `contentStoreLanded()` proved "the next touch will find data" with
   `existsSync(p)` alone. A PATH is not a COPY: a directory (or socket)
   sitting at the sidecar name satisfies existsSync, so the store was folded
   away while nothing readable was left behind. Require a regular file —
   that is exactly what `readContentStoreLys` can read back, so the proof
   now implies the reload instead of merely implying a name.

2. `adoptContentStore` read `prev.byRef` to compute the delta. When `prev` is a
   discharged lazy Proxy whose copy vanished, that read triggers `ensure()`
   and throws "could not be re-materialized from the store — refusing the
   thinned view" — a stale path turning an ordinary turn into a hard
   failure. Adoption is append-only and the incoming store already carries
   every ref (kernel processTurn re-ingests history), so a thinned prev is
   not a source of truth: treat it as absent.

Repro (deterministic, CI parity): the matrix preload pins
`BILIE_STORAGE_UNIFIED=legacy` (tests/preload-storage-baseline.mjs:16) and
`tests/store.test.ts` flips the sidecar file into a DIRECTORY right before
the second adopt. Pre-fix:

    BILI_STORAGE_UNIFIED=legacy ... --test tests/store.test.ts
    not ok 10 - flushAll retries a dirty content store after a transient write failure
    error: '[persist] resident content store for ccr-flushall-* could not be
            re-materialized from the store — refusing the thinned view'

Post-fix: 29/29 in all three modes (legacy, shadow, full). Negative
controls: revert (1) alone -> 28/1 reproduced; revert (2) alone -> 28/1
reproduced; both present -> 29/0. `BILI_SESSION_CONTENT_STORE_PIN=1` also masks
the symptom, which is why this stayed invisible on machines that set it.

Battery (legacy baseline, CI parity): store 29, memory-contentstore-lazy 8,
persist-lazy-narrow-reads 9, issue4788-resident-minimal 4, persist-tail 7,
persist-stats-roundtrip 1, proxy-persist 23, storage-ccr 3 = 84 pass /
0 fail. `npx tsc --noEmit` clean; the four static gates exit 0.
@ranxianglei

Copy link
Copy Markdown
Owner

关单说明:本 PR 修「master 三红灯门」的三处内容已被 #5327(f9cec9f)以等价或更优方式交付:①codex-models 去导出;②pack-install 裸负切片改 safePrefix(而非加白名单——#5327 后该字面量不存在,此处的 text-safe 白名单条目已成死条目,有门禁会因死白名单反红);③issue5103 测试钉。唯一残留:compress.yaml 的 promptPack "chat" builtin 文档笔 + 对应网站生成物,建议基于新 master 开 docs-only 小 PR 收编,勿在此分支上继续修(它的测试分片红正源于上述残留改动与新 master 的组合)。—— 由 #5327 交付方代为关闭。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants