Skip to content

test: fail when CONFIGURATION.md documents an acp.json key missing from KNOWN whitelist (#354) - #355

Open
ranxianglei wants to merge 1 commit into
masterfrom
2026-09-10_config-doc-consistency-test
Open

test: fail when CONFIGURATION.md documents an acp.json key missing from KNOWN whitelist (#354)#355
ranxianglei wants to merge 1 commit into
masterfrom
2026-09-10_config-doc-consistency-test

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

What

New regression test tests/user-config-doc-consistency.test.ts — the consistency test proposed in #354 (教训/建议).

Why

#354: repetitionGuard is documented in CONFIGURATION.md as acp.json-configurable (#308) but was missing from the KNOWN whitelist in src/user-config.ts, so user-written values were silently dropped with no log. No existing test could catch doc↔whitelist drift. This class of bug recurs every time a new config key lands in docs before (or without) the whitelist entry.

How it works

  • Parses the ## Parameter Reference section of CONFIGURATION.md; collects keys from every parameter table whose header row starts with | Key (the status-legend and environment-variable tables are excluded automatically); top-level key = prefix before the first ..
  • Behavioral assertion (no internals exported): writes all parsed top-level keys into a temp .pi/acp.json (value true each — pickKnown does no type validation) under an isolated HOME, calls loadUserConfig, asserts every key survives. Tests the user-visible "no silent drop" contract directly; immune to internal refactors.
  • Non-vacuity guard: asserts ≥9 documented top-level keys and presence of core stable keys (enabled, debug, autoUpdate, delegate, compress, prompts) so a broken parser cannot make the test pass vacuously.
  • Scope: English CONFIGURATION.md main tables only (as proposed in [bug] acp.json 的 repetitionGuard 键被静默丢弃(user-config KNOWN 白名单缺失)— 已修复于 #352 #354). zh-CN can be extended symmetrically later.

Verification

⚠️ CI will be RED until PR #352 merges — that failure is the test doing its job (catching the live bug). Recommended merge order: #352 first, then this PR goes green immediately.

Fixes #354

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

📦 Built Extension Artifact

Branch: 2026-09-10_config-doc-consistency-test (1e05947)

Option A — Install from npm PR tag (recommended)

pi install npm:billion-context-pi@pr-355

Each push to this PR publishes a new version under the pr-355 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pi-pr355.tgz
pi install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

Reviewed PR #355 independently (checked out the head 1e05947, installed deps, ran everything locally). Verdict: clean, correct, ready to merge — but only AFTER #352 merges.

Verification (run on the PR head)

Root cause confirmed (independent of the issue's analysis)

repetitionGuard is a real, implemented feature — src/config.ts:243 (repetitionGuard?: boolean | RepetitionGuardConfig) with logic in src/tool-guardrails.ts (#308) — yet it's absent from the KNOWN set in src/user-config.ts. pickKnown drops it silently and loadUserConfig has no unknown-key warn path, so "repetitionGuard": false in acp.json silently runs defaults. Genuine silent-drop bug; #352 adds both repetitionGuard + degenerationGuard to KNOWN.

Diff cleanliness

Single new file tests/user-config-doc-consistency.test.ts (+79). Zero src/ changes, no lockfile/version churn. Every line is on-topic. No rebase needed (parent = current master 6c10dab).

Duplicate screening

Not a duplicate. Searched issues + PRs (all states) for consistency/whitelist/doc-drift — no existing item. Only this new test reads CONFIGURATION.md; tests/user-config.test.ts exercises loadUserConfig with hardcoded fixtures and never parses the doc, so there's no overlapping coverage. This is the unique implementation of the consistency test proposed in #354's 教训/建议.

Design assessment

  • Behavioral assertion (writes parsed keys into a temp .pi/acp.json under an isolated HOME, calls loadUserConfig, asserts survival) — no internals exported, immune to refactors.
  • Non-vacuity guard (≥9 top-level keys + required core keys enabled/debug/autoUpdate/delegate/compress/prompts) stops a broken parser from passing vacuously.
  • Parser hand-traced: correctly excludes the status-legend and env-var tables (their headers don't start with | Key); scoped to English main tables per [bug] acp.json 的 repetitionGuard 键被静默丢弃(user-config KNOWN 白名单缺失)— 已修复于 #352 #354.
  • Follows the same isolated-HOME / mkdtemp / cleanup conventions as the existing config tests.
  • Minor, non-blocking: the (config as Record<string, unknown>) cast indexes by arbitrary string key — idiomatic, not as any/@ts-ignore, so it complies with the no-as-any rule. Left as-is.

No defects found; I made no code changes (nothing warranted a commit).

⚠️ Merge order (blocking)

CI on this PR stays red until #352 merges — and that red is the test doing its job. Please merge #352 first; this PR goes green immediately after. I can't merge either (human-only). Once #352 lands, #355 can be merged and #354 closes via Fixes #354.


一句话中文总结:独立核验了该回归测试——在 master(未含 #352)上恰好以 “repetitionGuard 被静默丢弃” 精确复现 #354、在 #352 上转绿、全量 657 例仅此一例失败且 typecheck/build 干净;diff 只新增一个测试文件、无杂项、与既有测试无重复覆盖,设计合理无需改动;唯一前提是必须先合 #352 再合本 PR(合并仅人工操作,我不代合)。

@ranxianglei ranxianglei mentioned this pull request Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] acp.json 的 repetitionGuard 键被静默丢弃(user-config KNOWN 白名单缺失)— 已修复于 #352

1 participant