Skip to content

fix: keep quality-gate rejection concise — strip internal details, log full diagnostics (#444) - #447

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-20_quality-rejection-no-leak
Sep 20, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-20_quality-rejection-no-leak

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #444

Problem

When the pre-commit compression quality gate rejects a compression, the tool-error message returned to the model leaked internal implementation detail into context:

  • Full compression rules are already in your system prompt. → reveals internal prompt architecture
  • add "acknowledgeRisk": true to bypass the quality gate → frames a documented param as an internal bypass/backdoor
  • Gate layer: / rougeF1: / top20Recall: lines + threshold config inside Reason → algorithm internals

In long sessions this error text becomes part of the model's context, so every rejection burns extra tokens on content that is already in the system prompt.

Note: this is the residual leak that survived PR #397 (which removed the giant embedded HOW_TO_COMPRESS_RULES block but left these finer-grained lines).

Fix

lib/compress/quality-gate/rejection.ts

  • Model-facing message now carries only why-it-failed + how-to-retry.
  • New coreReason() strips algorithm threshold/config detail from the reason string (handles both L1 colon form and L2 parenthetical form).
  • Removed the system prompt, bypass, Gate layer/rougeF1/top20Recall, redundant Ratio, and re-evaluates automatically lines.
  • New optional logger?: RejectionDiagnosticsLogger param: full diagnostics (reason with thresholds, layer, tokens, chars, ratio, retention, rougeF1, top20Recall) are written to the ACP debug log instead of the message.

lib/compress/range.ts

  • Passes ctx.logger into buildQualityRejectionError so diagnostics land in logs.

Before (~780 chars) → After (~430 chars):

⚠️ COMPRESSION REJECTED — QUALITY GATE FAILURE

Range: m00006–m00010
Reason: Summary too short
Original: ~36234 tokens · Summary: 670 chars · Retention: 0.46%

Retry: write a more complete summary preserving critical details (file paths, decisions, exact values, errors), then call compress again on the same range. If you are confident the summary is adequate despite the metrics, pass "acknowledgeRisk": true.

E2E markers COMPRESSION REJECTED / QUALITY GATE FAILURE are preserved (the fake-LLM server greps for them).

Tests

tests/quality-gate-enforcement.test.ts: +4 cases (no-leak assertions, coreReason stripping table, diagnostics-to-log capture, no-logger guard). Target suite 19/19 pass. Mutation-checked per §5.7.3 (reverting the fix makes the 3 new leak tests fail).

Verification

  • npm run typecheck ✅ · npm run build ✅
  • Full suite: 1294 pass / 2 fail — the 2 failures (tests/soft-block.test.ts, tests/inactive-block-decompress.test.ts) are pre-existing & environmental: they mkdir '/tmp/...' which is read-only in this sandbox; confirmed identical on clean HEAD.

…g full diagnostics (#444)

The pre-commit quality gate rejection message leaked internal implementation
detail into model context: the 'Full compression rules are already in your
system prompt' line, an 'acknowledgeRisk ... to bypass' framing, and raw gate
algorithm internals (Gate layer / rougeF1 / top20Recall / threshold config).

Fix:
- rejection.ts: model-facing message now carries only why-it-failed + how-to-retry
  (core reason with threshold/config detail stripped via coreReason()). Full
  diagnostics are written to the ACP debug log via a new optional logger param.
- range.ts: pass ctx.logger into buildQualityRejectionError so diagnostics land in logs.
- tests: 4 new cases asserting no-leak, coreReason stripping, and diagnostics-to-log.

E2E markers COMPRESSION REJECTED / QUALITY GATE FAILURE preserved.
@github-actions

Copy link
Copy Markdown
Contributor

📦 Built Plugin Artifact

Branch: 2026-09-20_quality-rejection-no-leak (426d728)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-447 --global

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

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-20_quality-rejection-no-leak" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr447.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 PR #447 verified — ready for human merge. (This is the agent PR for issue #444; per the ework-agent-pr flow I audited rather than redid the work.)

Verification performed (branch 2026-09-20_quality-rejection-no-leak, head 426d728d, single commit on top of master 2cdce3ff):

Check Result
CI (pr-validation, test 22, test 24, build, build-artifact, e2e) ✅ all 6 green
npm run typecheck ✅
npm run build ✅
Target suite tests/quality-gate-enforcement.test.ts ✅ 19/19
Full suite ✅ 1294 pass / 2 fail — both failures (tests/soft-block.test.ts, inactive-block-decompress E2E) reproduced identically on clean master via a throwaway worktree → pre-existing & environmental (read-only /tmp in sandbox), not caused by this PR
Mutation check (§5.7.3) ✅ reverting only lib/compress/quality-gate/rejection.ts to master makes exactly the 3 new leak tests fail (16/19); restored afterwards

Code review findings:

  • Diff hygiene: 5 files, every line on-topic (rejection.ts, range.ts, tests, devlog REQ+WORKLOG). No version/CHANGELOG churn on a non-release branch. Devlog present per §5.1.2.
  • coreReason() (lib/compress/quality-gate/rejection.ts:58-64): correctly cuts at the first of : / ( covering both L1 colon form and L2 parenthetical form; no-delimiter reasons pass through whole; empty result falls back to "unknown". Verified against the 4-case table in the tests.
  • Logger injection: RejectionDiagnosticsLogger is a structural interface (warn(message, data?)) satisfied by Logger.warn(message: string, data?: any) — no casts, no runtime dependency on the logger class, optional param so the no-logger path can't throw (covered by test). Single production call site (lib/compress/range.ts:296) updated to pass ctx.logger; diagnostics (full reason incl. thresholds, layer, tokens, ratio, retention, rougeF1, top20Recall) go to the ACP log instead of model context.
  • E2E markers preserved: COMPRESSION REJECTED / QUALITY GATE FAILURE still in the message (asserted by tests), so fake-LLM detection is unaffected.
  • Behavioral note: acknowledgeRisk remains mentioned in the retry line, reworded from "bypass" framing to a documented-param phrasing — consistent with the issue's stated intent.

No small fixes were needed — nothing actionable found during review. Per §5.1.1.2 I will not merge; please merge from your side once you're happy: #447

中文摘要:质量门禁拒绝消息瘦身(去掉系统提示词引用、bypass 措辞、Gate layer/rougeF1/top20Recall 等算法内部细节,完整诊断改写入 ACP 日志)——已逐行核对代码、本地复跑 typecheck/build/全量测试并做了变异验证(回退修复后 3 个新泄漏测试确实失败),2 个失败用例在干净 master 上同样失败属沙箱环境问题,与 PR 无关;可以合并。

@ranxianglei
ranxianglei merged commit 988ed73 into master Sep 20, 2026
6 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 30, 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]: 压缩 Quality Gate 失败时仍会向模型暴露内部压缩规则,并可能导致不必要的上下文消耗

1 participant