From 9477f97c1608ea12016fb565c2091ce45845600a Mon Sep 17 00:00:00 2001 From: ework-agent Date: Tue, 8 Sep 2026 23:44:40 +0800 Subject: [PATCH 1/4] feat: strip reasoning from protected-exempt historical messages Request-time pass (stripProtectedReasoning) that reclaims the never-compressible reasoning floor: reasoning parts on compress/skill assistant messages in CLOSED historical turns are dropped before the request is sent, while the current (possibly-open) round is always preserved. - lib/messages/reasoning-strip.ts: new pass, 3 gates (turn-closure via getLastUserMessage, protected-tool selector, size threshold > 2048 chars) - lib/hooks.ts: wired after hideConsumedCompressCalls, guarded by kill-switch - lib/config.ts: compress.stripProtectedReasoning (bool, default true) + compress.stripProtectedReasoningThreshold (number, default 2048); excluded from CompressOverridableConfig (global-only) - lib/config-validation.ts: registered both keys in VALID_CONFIG_KEYS + validateConfigTypes - dcp.schema.json: schema properties + default - tests: +19 tests (unit + hook-level kill-switch, mutation-verified) No provider gate (owner decision: handle reactively). No persisted-state or internal-tag changes. 1096/1096 tests pass. Fixes #368 --- dcp.schema.json | 15 +- .../DESIGN.md | 106 +++++++ .../REQ.md | 69 +++++ .../WORKLOG.md | 93 ++++++ lib/config-validation.ts | 26 ++ lib/config.ts | 27 +- lib/hooks.ts | 13 + lib/messages/index.ts | 2 +- lib/messages/reasoning-strip.ts | 90 ++++++ tests/e2e-message-transform.test.ts | 82 +++++- tests/reasoning-strip.test.ts | 272 +++++++++++++++++- 11 files changed, 786 insertions(+), 9 deletions(-) create mode 100644 devlog/2026-09-07_strip-protected-reasoning/DESIGN.md create mode 100644 devlog/2026-09-07_strip-protected-reasoning/REQ.md create mode 100644 devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md diff --git a/dcp.schema.json b/dcp.schema.json index e2584946..a694a3d6 100644 --- a/dcp.schema.json +++ b/dcp.schema.json @@ -568,6 +568,17 @@ "type": "boolean", "default": true, "description": "Always protect the most recent user message from compression." + }, + "stripProtectedReasoning": { + "type": "boolean", + "default": true, + "description": "Strip reasoning parts from protected-exempt (compress/skill) messages in closed historical turns. The current (possibly-open) round is never touched." + }, + "stripProtectedReasoningThreshold": { + "type": "number", + "default": 2048, + "minimum": 0, + "description": "Minimum total reasoning length (chars) on a protected-exempt historical message before its reasoning is stripped." } }, "default": { @@ -592,7 +603,9 @@ "lastSegmentSoftBlock": true, "preserveRecentMessages": 20, "preserveRecentTokens": 20000, - "preserveLastUserMessage": true + "preserveLastUserMessage": true, + "stripProtectedReasoning": true, + "stripProtectedReasoningThreshold": 2048 } }, "gc": { diff --git a/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md b/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md new file mode 100644 index 00000000..d5acd04d --- /dev/null +++ b/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md @@ -0,0 +1,106 @@ +# DESIGN - Strip reasoning from protected-exempt historical messages + +- Task ID: `2026-09-07_strip-protected-reasoning` +- Home Repo: `opencode-acp` +- Created: 2026-09-07 +- Status: Final (owner decision 2026-09-07: **no provider gate**, default-on, threshold 2048 chars — see §8) + +## 1. Problem Statement + +- **What problem are we solving?** The `reasoning` parts on `compress`/`skill`-carrying assistant messages form a permanently-incompressible context floor: protection is message-granular, so the whole message (reasoning included) is excluded from compression and re-sent every turn. Each round adds ~9 KB of unreclaimable reasoning. +- **Why now?** Measured ~83.5% of the never-covered residual in a real long session (#368); it is a monotonic feedback loop that degrades long-session usability. + +## 2. Goals & Non-Goals + +- **Goals**: + - Reclaim the reasoning floor at request time with zero loss of user-visible / compression-critical data. + - Never break providers that require reasoning replay — enforced by the **turn-closure gate** (the active round is always preserved). No provider gate (owner decision); residual cross-turn risk handled reactively via the kill-switch. + - Keep the sent prefix cache-stable within a turn. +- **Non-Goals**: + - Part-granular protection rework (separate effort). + - The three secondary findings A/B/C (separate issues). + - Any persistent/DB write. + +## 3. Current Architecture + +The `experimental.chat.messages.transform` pipeline (`lib/hooks.ts`) runs, in order (relevant span): + +``` +:256 prune() +:257 truncateLargeToolOutputs() +:258 hideConsumedCompressCalls() ← splices reasoning-only leftovers (reasoning is "structural") +:259 assignMessageRefs() +:262 injectCompressNudges(prePruneTokens) +:290 injectMessageIds() +:291 hideFailedCompressCalls() +:292 stripStaleMetadata() +:293 dropEmptyMessages() +:294 postTokens +``` + +Key facts (code-verified @ v1.14.27): +- `filterProtectedToolMessages` (`lib/compress/protected-content.ts:188-234`, `:202`) excludes the **whole** message (reasoning included) from every selection. +- `compress`/`skill` are default `compress.protectedTools` (`lib/config.ts:158`); `compress` is in `FORCE_COMPRESS_PROTECTED` (`:167`, force-appended `:476`). +- Excluded messages never enter `byMessageId`, so `prune` (`lib/messages/prune.ts:60-66`) re-sends them every turn. +- `lib/compress/parts.ts:1` `STRUCTURAL_PART_TYPES = ["step-start","step-finish","reasoning"]` → `hasMeaningfulContent()` is false on a reasoning-only leftover, so `hideConsumedCompressCalls` (`lib/compress/hide-consumed.ts:121-124`) **splices** consumed-block messages whose only remaining content is reasoning. **Therefore the only reasoning with no reclaim path is on (a) live-block compress calls and (b) skill-carrying messages** — exactly the floor this pass targets. +- `getLastUserMessage` (`lib/messages/query.ts:10`) returns the last user-role message that is **not** synthetic and **not** all-`ignored` (tool-result user msgs are ignored) — i.e. the last **genuine** user input. This is the turn boundary and is the same mechanism `stripStaleMetadata` already relies on. + +## 4. Proposed Architecture + +- **Overview**: A single request-time pass inserted **after** `hideConsumedCompressCalls` (operates on the minimal surviving set; never touches about-to-be-spliced messages) and **before** `assignMessageRefs`. + +``` +for each assistant message m at index i: + if i >= lastGenuineUserIndex: # Gate 1: current open round → KEEP + continue + if not hasProtectedToolPart(m): # Gate 2: selector = protected tool call (compress/skill) + continue + if reasoningLength(m) <= threshold: # Gate 3: size threshold → small reasoning untouched + continue + m.parts = m.parts.filter(p => p.type !== "reasoning") # drop reasoning parts only, keep tool call +``` + +- **Key components**: + - **Pass function** in `lib/messages/reasoning-strip.ts` (new export, name distinct from `stripStaleMetadata`; e.g. `stripProtectedReasoning`). + - **Gate 1 — turn-closure**: `lastGenuineUserIndex = index of getLastUserMessage(messages)`. All assistant messages at/after it are the current (possibly-open) round → reasoning kept. Only messages strictly before are candidates. If `getLastUserMessage` returns `null` → strip nothing (fail-safe). + - **Gate 2 — selector** (narrow, per operator): `m` contains a **protected tool part** (`compress`/`skill`). Only these are the "floor" — normal historical messages' reasoning is already reclaimed by compression, so we do **not** target arbitrary large-reasoning messages. (Simplified from the earlier "compress part OR all-non-structural-are-protected" predicate; confirm with owner.) + - **Gate 3 — size threshold** (operator proposal, 2026-09-08): only strip when the message's total `reasoning` content length **exceeds a configurable threshold** (default ~2 KB, unit pending). Small reasoning is left untouched → zero prefix churn for those messages, and the per-message decision is stable (length doesn't change). Captures ~all of the floor (measured mean 9,418 B, max 28,067 B per message). + - **~~Gate 4 — provider policy~~ (removed per owner decision 2026-09-07).** No provider gate is implemented ("provider 先不管 有问题再说"). The turn-closure gate (Gate 1) is the safety mechanism; the global kill-switch is the self-service mitigation; a provider gate would be added reactively if a real breakage is reported. (`state.modelProviderID` remains available at the call site — `lib/hooks.ts:92,104,197,215` — should a gate be added later.) + - **Action**: rebuild `m.parts` without `reasoning` parts (keep the tool call + any other non-reasoning parts). Message `info.id`/order unchanged → no effect on `mNNNNN` refs or downstream passes. +- **Data flow**: pure in-memory mutation of the per-request array. **No state/DB writes.** Deterministic → idempotent. +- **API / interface changes**: new config keys under `compress.*` (see §4 of REQ). No change to persisted state format, exported tool APIs, or internal `dcp` tags. + +## 5. Design Decisions & Rationale + +| Decision | Options Considered | Chosen | Why | +|----------|--------------------|--------|-----| +| Gate axis | (a) provider-only; (b) turn-closure; (c) both | **(b) turn-closure** (+ provider gate §8) | Turn-closure keeps the mitigation active on high-thinking models while never touching the open round (the only case where replay actually matters). Provider-only would forfeit the biggest contributor. | +| Placement | before `hideConsumedCompressCalls`; after it | **after** (`:258`→`:259`) | Operates on the minimal surviving set; never processes messages about to be spliced. | +| What to strip | whole message; reasoning parts only | **reasoning parts only** | Dropping the whole message kills the live summary (it lives only in the compress-call body, `state.ts:55-63`). Reasoning has no value once the summary is finalized. | +| Predicate scope | any protected msg; only protected-exempt msgs | **only protected-exempt** (compress part, or all-non-structural-are-protected) | Narrow; never touches user-visible text or normal messages. | +| Provider policy | allowlist; blocklist; none | **none (owner decision 2026-09-07)** | Owner: "provider 先不管 有问题再说". Turn-closure gate + global kill-switch are the safeguards; a gate is added reactively only on a real breakage. | +| Naming | `stripExemptReasoning`; other | **distinct from `stripStaleMetadata`** | Avoids conceptual collision in `reasoning-strip.ts`. | +| Strip trigger | uniform (all protected msgs); size-gated | **size-gated** (threshold, default ~2 KB) | Operator proposal: floor is dominated by large reasoning (mean 9.4 KB), so size-gating captures ~all benefit while leaving small reasoning untouched → smaller cache-invalidation surface + a stable per-message decision. | + +## 6. Impact Analysis + +- **Backward compatibility**: additive config with safe defaults; no persisted-state or internal-tag changes. +- **Performance**: one O(n) pass per request (n = message count); negligible vs existing pipeline steps. +- **Cache**: prefix is byte-identical within a turn (strip set is fixed by the stable `lastGenuineUserIndex`). Invalidation is bounded to (a) turn-boundary shifts (≈ one prior turn's messages) and (b) a one-time rebuild on first enablement. +- **Security**: none (no new network/credential surface). +- **Dependencies**: none new. + +## 7. Migration Plan + +- **Steps**: + 1) Ship the pass + config behind the kill-switch. + 2) Default per owner decision (§8). +- **Feature flags / gradual rollout**: `compress.stripProtectedReasoning` (kill-switch) + provider-policy key. Can ship default-off (opt-in) if the owner prefers a burn-in release (the `qualityGate` precedent). + +## 8. Open Questions (RESOLVED — owner decision 2026-09-07) + +- [x] **Provider policy** — **none** ("provider 先不管 有问题再说"). Turn-closure gate + global kill-switch are the safeguards; a provider gate is added reactively only if a real breakage is reported. +- [x] **Size-threshold default + unit** — **2048 chars** (owner: "阈值按照你的推荐"). Unit = characters (matches the `part.text.length` measurement; cheap, no tokenizer). +- [x] **Selector** — target = protected tool-call messages (`compress`/`skill`), NOT all large-reasoning messages. Confirmed. +- [x] **Default on/off** — **default-on** with the global kill-switch `stripProtectedReasoning: false`. +- [x] **Provider-ID matching** — moot (no provider gate). diff --git a/devlog/2026-09-07_strip-protected-reasoning/REQ.md b/devlog/2026-09-07_strip-protected-reasoning/REQ.md new file mode 100644 index 00000000..ab038e05 --- /dev/null +++ b/devlog/2026-09-07_strip-protected-reasoning/REQ.md @@ -0,0 +1,69 @@ +# REQ - Strip reasoning from protected-exempt historical messages + +- Task ID: `2026-09-07_strip-protected-reasoning` +- Home Repo: `opencode-acp` +- Created: 2026-09-07 +- Status: InProgress (design settled; **owner decision 2026-09-07: no provider gate — "provider 先不管 有问题再说"**; see §3 + DESIGN.md §8) +- Priority: P1 +- Owner: ework-daemon (agent) / ranxianglei +- References: https://github.com/ranxianglei/opencode-acp/issues/368 + +## 1. Background & Problem Statement + +- **Context**: On long sessions, ACP force-protects `compress` and `skill` messages. Protection is **message-granular** (`lib/compress/protected-content.ts:202` `removedMessageIds.add(messageId)`), so the *whole* assistant message — including its `reasoning` parts — is excluded from every compression selection. +- **Current behavior (symptom)**: Those excluded messages never enter the compression index, so `prune` (`lib/messages/prune.ts:60-66`) re-sends them **every turn**, reasoning riding along. Each compress/skill round therefore adds one permanently-exempt message carrying ~9 KB of reasoning (measured mean 9,418 B, max 28,067 B/msg in the audited session). The floor grows monotonically with every compression — a feedback loop. In the forensically-audited session (#368), `reasoning` parts were **~83.5% of the never-covered residual** (446 KB of 563 KB rode on `compress`-part messages). +- **Expected behavior**: Reclaim that reasoning floor at request time **without** losing user-visible or compression-critical data, **without** breaking providers that require reasoning replay, and with **bounded** prefix-cache impact. +- **Impact**: A monotonically-growing incompressible base makes context fill faster → triggers more compression → raises the floor further. + +## 2. Reproduction (if applicable) + +- **Environment**: opencode-acp 1.14.27 (branch base = `origin/master` @ v1.14.27, matching the reporter's audit baseline). +- **Minimal reproduction steps**: + 1) Long session with repeated `compress` calls (or `skill` usage) on a high-thinking model. + 2) Read-only audit over `opencode.db` (message/part) joined against the ACP registry JSON (`prune.messages.byMessageId`, `blocksById[*].effectiveMessageIds/compressCallId/active`): count bytes of parts in messages never covered by any block, grouped by protected-tool co-occurrence of the parent message. +- **Relevant configuration**: default `compress.protectedTools = ["skill","compress"]`; `compress` additionally in `FORCE_COMPRESS_PROTECTED` (`lib/config.ts:167`). + +> The exact session numbers (83%, ~9 KB/round) come from the reporter's audit and are not independently reproducible here; the **mechanism** producing the floor is fully code-verified. + +## 3. Constraints & Non-Goals + +- **Constraints**: + - **Request-time transform only.** ACP is a plugin with the `experimental.chat.messages.transform` hook; it can only rewrite the per-request message array. It **cannot** modify opencode's stored messages, and (per reporter) must avoid persistent/DB writes. + - **Provider safety (owner decision: no gate).** Some upstreams may require reasoning to be replayed complete (operator: **GPT/OpenAI** requires complete thinking). The owner chose **not** to add a provider gate ("provider 先不管 有问题再说") — the **turn-closure gate** is the safety mechanism (only closed historical rounds are ever touched; the active round is always preserved). Residual risk: a provider that validates thinking-signatures *across* user-turn boundaries could 400 on a stripped historical block; this is handled **reactively** (kill-switch `stripProtectedReasoning: false`, or a provider gate added later if a real breakage is reported). The kill-switch is the self-service mitigation. + - **Turn safety.** Never strip the **current open round's** reasoning (Anthropic thinking-signature / Gemini `thought_signature` replay on the active tool round). + - **Cache stability.** The sent prefix must be byte-identical across consecutive requests *within a turn* so prompt caching keeps hitting; invalidation must be bounded (turn-boundary shifts + one-time enablement rebuild only). + - **Surgical.** Only drop `reasoning` parts; never touch user-visible `text`; message identity/order unchanged (no effect on `mNNNNN` ref assignment). +- **Non-Goals** (explicitly out of scope): + - Part-granular protection rework (protect the tool *part* but let the *reasoning part* compress normally) — larger, changes selection semantics; reporter judged it non-minimal; **separate effort**. + - The three secondary findings from #368 (A: display estimator excludes reasoning; B: orphaned `byMessageId` entries; C: `rewriteCompressInput` full-consumption leak) — **filed as separate issues**. + +## 4. Acceptance Criteria (must be testable) + +- **Correctness**: + - [ ] `reasoning` parts are removed from assistant messages that are (a) **before** the last genuine user message, (b) contain a **protected tool part** (`compress`/`skill`), and (c) whose total `reasoning` length **exceeds the configured threshold**. (No provider gate — owner decision.) + - [ ] `reasoning` is **never** removed from any assistant message at/after the last genuine user message (the current round). + - [ ] Messages whose total `reasoning` length is **at or below** the threshold are left untouched (no prefix change). + - [ ] Messages carrying user-visible `text` are not modified (only `reasoning` parts dropped). + - [ ] Kill-switch `stripProtectedReasoning: false` disables the pass entirely (no-op) — verified at the **hook level** (full transform handler), not just the pure function. + - [ ] No DB/state writes; request-time only (idempotent, no persisted mutation). + - [ ] Fail-safe: if no genuine user message is found, the pass strips nothing. +- **Performance / Stability**: + - [ ] For a fixed message array within a turn, the transformed prefix is byte-identical across repeated calls (cache-stable). +- **Regression**: + - [ ] New/modified test cases added and passing, per §5.7: multi-turn (≥2 `inject`-style calls sharing state), side-effect assertions, `preserveRecentMessages > 0`, full growth cycle; plus Docker E2E per §5.7.2. + - [ ] `npm run build`, `npm run typecheck`, full `npm run test` all green. + +## 5. Proposed Approach (optional) + +- **Affected modules & entry files**: + - `lib/messages/reasoning-strip.ts` — add the new pass function (distinct name from existing `stripStaleMetadata`). + - `lib/hooks.ts` — wire the pass **after** `hideConsumedCompressCalls` (`:258`), **before** `assignMessageRefs` (`:259`). + - `lib/config.ts` + `dcp.schema.json` + `lib/config-validation.ts` — new config keys: `compress.stripProtectedReasoning` (bool, kill-switch, default `true`) + `compress.stripProtectedReasoningThreshold` (number, default `2048` chars). Both registered in `VALID_CONFIG_KEYS` + `validateConfigTypes`; excluded from `CompressOverridableConfig` (global-only, not per-provider overridable). + - `tests/reasoning-strip.test.ts` — unit tests for the pass; `tests/e2e-message-transform.test.ts` — hook-level kill-switch test. +- **Risks**: + - Provider semantics (no provider gate per owner decision; mitigated by the turn-closure gate + global kill-switch; residual cross-turn thinking-signature validation risk handled reactively). + - Cache invalidation (mitigated: turn-stable prefix; bounded to boundary shifts + one-time enablement rebuild). +- **Rollback strategy**: config kill-switch (`stripProtectedReasoning: false`) for immediate disable; revert the commit for full rollback. +- **RESOLVED (owner decision 2026-09-07)** — see DESIGN.md §8: + - **No provider gate** ("provider 先不管 有问题再说"). Turn-closure gate is the safety mechanism; the global kill-switch is the self-service mitigation; a provider gate is added reactively only if a real breakage is reported. + - Ships **default-on** with the global kill-switch `stripProtectedReasoning: false` + threshold `2048` (owner: "阈值按照你的推荐"). diff --git a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md new file mode 100644 index 00000000..9ef94bd4 --- /dev/null +++ b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md @@ -0,0 +1,93 @@ +# WORKLOG - Strip reasoning from protected-exempt historical messages + +- Task ID: `2026-09-07_strip-protected-reasoning` +- Home Repo: `opencode-acp` +- Status: InProgress (implementation + dual-agent review fixes complete; awaiting commit + PR) +- Updated: 2026-09-08 23:05 + +## 1. Summary + +- **What was done**: Implemented a request-time pass `stripProtectedReasoning` that strips `reasoning` parts from protected-exempt (compress/skill) messages in CLOSED historical turns, wired into the message-transform pipeline, with two new config keys (kill-switch + size threshold) and a full unit-test suite. +- **Why**: Reclaim the monotonically-growing, never-compressible reasoning floor (~83.5% of measured residual in #368) without breaking reasoning-replay providers and with bounded cache impact. +- **Behavior / compatibility changes**: Yes — additive request-time transform + additive config keys; no persisted-state/internal-tag changes. +- **Risk level**: Low-Medium — mitigated by turn-closure gate (current round never touched) + size threshold (small reasoning untouched) + kill-switch. Provider gate intentionally omitted per owner decision ("provider 先不管 有问题再说" — handle reactively). + +## 2. Change Log + +### Commits + +| Commit | Description | +|--------|-------------| +| _pending_ | implementation (see Key Files) | + +### Key Files + +- `devlog/2026-09-07_strip-protected-reasoning/REQ.md` — ticket. +- `devlog/2026-09-07_strip-protected-reasoning/DESIGN.md` — design. +- `lib/messages/reasoning-strip.ts` — new `stripProtectedReasoning(messages, protectedTools, threshold): number` pass (3 gates). +- `lib/messages/index.ts` — barrel export. +- `lib/hooks.ts` — wired between `hideConsumedCompressCalls` and `assignMessageRefs`, guarded by kill-switch. +- `lib/config.ts` — `compress.stripProtectedReasoning` (bool, default true) + `compress.stripProtectedReasoningThreshold` (number, default 2048): interface + DEFAULT_CONFIG + mergeCompress + excluded from `CompressOverridableConfig` (global-only, not per-provider overridable). +- `lib/config-validation.ts` — registered both keys in `VALID_CONFIG_KEYS` + `validateConfigTypes` (bool / non-negative finite number). +- `dcp.schema.json` — schema properties + default. +- `tests/reasoning-strip.test.ts` — 18 pass tests (turn-closure, selector, size-threshold, boundary/edge) + 3 config-merge tests + `protectedToolMsg` helper. +- `tests/e2e-message-transform.test.ts` — hook-level kill-switch test (flag=false preserves / flag=true strips); `buildConfig`/`setupPipeline` extended (backward-compatible) to accept config overrides. + +## 3. Design & Implementation Notes + +- **Entry point / key function**: `stripProtectedReasoning` in `lib/messages/reasoning-strip.ts`, wired in `lib/hooks.ts` after `hideConsumedCompressCalls` before `assignMessageRefs`. +- **Three gates** (all must hold to strip a message's reasoning): + 1. **turn-closure**: message index strictly `< lastUserIndex` (index of `getLastUserMessage`). The current, possibly-open round is never touched. + 2. **selector**: message contains a tool part whose `part.tool` ∈ `config.compress.protectedTools`. + 3. **size**: total reasoning length (sum of `part.text.length` over reasoning parts) `> threshold` (default 2048). +- **Action**: `msg.parts = parts.filter(p => p.type !== "reasoning")` — drops reasoning only; tool call + other parts preserved. Returns count removed. +- **No provider gate** (per owner): closed-turn reasoning is stripped for all providers; current round always kept. +- **Deterministic / cache-stable**: within a turn the output is byte-stable; the boundary shifts only when a new user turn starts (which invalidates the prefix cache anyway). + +## 4. Testing & Verification + +### Build & Test Commands + +```sh +npm run build +npm run typecheck +node --import tsx --test tests/reasoning-strip.test.ts +npm test +``` + +### Test Coverage + +- New/modified test files: `tests/reasoning-strip.test.ts` (+18), `tests/e2e-message-transform.test.ts` (+1 kill-switch). +- Test count: 1096 total, 0 failures (was 1077 before this change). +- Key scenarios verified: turn-closure (current round kept), selector (non-protected tool untouched), size threshold (`<= threshold` kept, `== threshold` kept [strict `>`], custom threshold), reasoning-only vs tool+reasoning, idempotency (2nd call removes 0), multi-turn growth cycle (closed turns stripped, current kept), summing across multiple reasoning parts, no-user-msg no-op, empty-protectedTools no-op, **synthetic-user boundary** (anchors on last genuine user msg), **first-user no-op** (`lastUserIndex<=0`), config merge (default / kill-switch / custom threshold), **hook-level kill-switch** (e2e: flag=false preserves / flag=true strips). + +### Results + +- **PASS/FAIL**: PASS — typecheck clean, build clean, 1096/1096 tests pass. +- **Key logs/data**: `tests/reasoning-strip.test.ts` 25/25 in-file (7 pre-existing `stripStaleMetadata` + 18 new). + +### Dual-Agent Review (2026-09-08, both via `task`+`general`) + +Both reviewers returned REQUEST-CHANGES; core logic / tests / pipeline integration confirmed clean. All findings addressed: +- **Code**: (MAJOR-2) registered new keys in `config-validation.ts` `VALID_CONFIG_KEYS` + `validateConfigTypes` (was a real "Unknown keys" TUI-toast bug); (MAJOR-3) excluded keys from `CompressOverridableConfig` (dead per-provider override); (MINOR-3) `Array.isArray(message.parts)` guard; (MAJOR-1/MINOR-1) REQ/DESIGN updated for the owner no-gate decision. +- **Test**: (F1 MAJOR) added the hook-level kill-switch e2e test — **mutation-verified** (replacing the guard with `if(true)` makes it fail); (F3) synthetic-user boundary test; (F4) first-user no-op test; (F6) test-name precision fix. +- **Skipped (with rationale)**: provider gate (owner declined — "provider 先不管 有问题再说"); F2 idempotency test (idempotent by construction); F5 fixture `time` field (NIT, consistent with existing style). + +## 5. Risk Assessment & Rollback + +- **Risk points**: provider reasoning-replay semantics (mitigated: current round never touched; historical closed-turn reasoning is the standard-strippable case); one-time cache rebuild on enablement (bounded, not continuous); correctness of the protected-tool selector (matches `config.compress.protectedTools`). +- **Rollback method**: + - Config: set `compress.stripProtectedReasoning: false` (immediate no-op). + - Revert commit(s): _pending sha_. +- **Compatibility notes**: additive config keys only; no persisted-state or internal-tag changes. + +## 6. Lessons Learned + +- Prettier version drift: `npm ci` installs Prettier 3.9.5 (lockfile) vs the ~3.8.x the repo was formatted with; `format:check` flags 425 pre-existing files. CI does NOT gate on format. New code matches the de-facto committed convention (single-line part literals, 4-space, no semi); pre-existing lines left untouched to avoid diff noise. + +## 7. Follow-ups (separate issues, source marker `来源: #368 ...`) + +- [ ] #368 secondary finding **A** — display/`acp_status` estimator excludes reasoning (`lib/messages/inject/utils.ts:586` `estimateContextComposition` counts only text+tool; real usage includes reasoning per `lib/token-utils.ts:19,44`). +- [ ] #368 secondary finding **B** — orphaned `byMessageId` entries with emptied `activeBlockIds` stay visible forever (`lib/messages/prune.ts:60-66`). +- [ ] #368 secondary finding **C** — `rewriteCompressInput` full-consumption leak (`lib/compress/hide-consumed.ts:42` `kept.length === 0 → return null`). +- [ ] File the main #368 issue to `ranxianglei/billion-context` and `ranxianglei/billion-context-pi` (owner request). diff --git a/lib/config-validation.ts b/lib/config-validation.ts index 33c1e56b..aa0a375e 100644 --- a/lib/config-validation.ts +++ b/lib/config-validation.ts @@ -49,6 +49,8 @@ export const VALID_CONFIG_KEYS = new Set([ "compress.preserveRecentMessages", "compress.preserveRecentTokens", "compress.preserveLastUserMessage", + "compress.stripProtectedReasoning", + "compress.stripProtectedReasoningThreshold", "gc", "gc.algorithm", "gc.promotionThreshold", @@ -539,6 +541,30 @@ export function validateConfigTypes(config: Record): ValidationErro }) } + if ( + compress.stripProtectedReasoning !== undefined && + typeof compress.stripProtectedReasoning !== "boolean" + ) { + errors.push({ + key: "compress.stripProtectedReasoning", + expected: "boolean", + actual: typeof compress.stripProtectedReasoning, + }) + } + + if ( + compress.stripProtectedReasoningThreshold !== undefined && + (typeof compress.stripProtectedReasoningThreshold !== "number" || + !Number.isFinite(compress.stripProtectedReasoningThreshold) || + compress.stripProtectedReasoningThreshold < 0) + ) { + errors.push({ + key: "compress.stripProtectedReasoningThreshold", + expected: "number (>= 0)", + actual: JSON.stringify(compress.stripProtectedReasoningThreshold), + }) + } + if ( typeof compress.iterationNudgeThreshold === "number" && compress.iterationNudgeThreshold < 1 diff --git a/lib/config.ts b/lib/config.ts index df3bfeef..874d7e74 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -21,7 +21,15 @@ type Permission = "ask" | "allow" | "deny" */ export type CompressOverridableConfig = Omit< CompressConfig, - "permission" | "minContextLimit" | "modelMaxLimits" | "modelMinLimits" | "providers" + | "permission" + | "minContextLimit" + | "modelMaxLimits" + | "modelMinLimits" + | "providers" + // Global-only: the strip pass reads raw global config, so a per-provider form + // would be a silent no-op. Excluded to keep the overridable surface honest. + | "stripProtectedReasoning" + | "stripProtectedReasoningThreshold" > /** Per-model / per-provider override object (all overridable fields optional). */ @@ -80,6 +88,19 @@ export interface CompressConfig { preserveRecentTokens?: number /** Always protect the most recent user message (default: true). */ preserveLastUserMessage?: boolean + /** + * Strip `reasoning` parts from protected-exempt (compress/skill) messages in + * CLOSED historical turns — they are re-sent every turn and otherwise form a + * never-reclaimable incompressible floor. The current (possibly-open) round is + * never touched. Default: true. + */ + stripProtectedReasoning?: boolean + /** + * Minimum total reasoning length (chars) on a protected-exempt historical + * message before its reasoning is stripped. Small reasoning is left untouched + * to avoid prefix-cache churn. Default: 2048. + */ + stripProtectedReasoningThreshold?: number } export interface Commands { @@ -257,6 +278,8 @@ const defaultConfig: PluginConfig = { preserveRecentMessages: 5, preserveRecentTokens: 5000, preserveLastUserMessage: true, + stripProtectedReasoning: true, + stripProtectedReasoningThreshold: 2048, }, gc: { algorithm: "truncate", @@ -488,6 +511,8 @@ export function mergeCompress( preserveRecentMessages: override.preserveRecentMessages ?? base.preserveRecentMessages, preserveRecentTokens: override.preserveRecentTokens ?? base.preserveRecentTokens, preserveLastUserMessage: override.preserveLastUserMessage ?? base.preserveLastUserMessage, + stripProtectedReasoning: override.stripProtectedReasoning ?? base.stripProtectedReasoning, + stripProtectedReasoningThreshold: override.stripProtectedReasoningThreshold ?? base.stripProtectedReasoningThreshold, } } diff --git a/lib/hooks.ts b/lib/hooks.ts index 6c96e227..36ade58a 100644 --- a/lib/hooks.ts +++ b/lib/hooks.ts @@ -11,6 +11,7 @@ import { prune, stripHallucinations, stripHallucinationsFromString, + stripProtectedReasoning, stripStaleMetadata, syncCompressionBlocks, computeInputBudget, @@ -256,6 +257,18 @@ export function createChatMessageTransformHandler( prune(state, logger, config, output.messages) truncateLargeToolOutputs(state, config, logger, output.messages) hideConsumedCompressCalls(state, output.messages) + if (config.compress.stripProtectedReasoning !== false) { + const removedReasoning = stripProtectedReasoning( + output.messages, + config.compress.protectedTools, + config.compress.stripProtectedReasoningThreshold ?? 2048, + ) + if (removedReasoning > 0) { + logger.debug("stripProtectedReasoning: removed reasoning parts from historical protected messages", { + removed: removedReasoning, + }) + } + } assignMessageRefs(state, output.messages) const compressionPriorities = buildPriorityMap(config, state, output.messages) prompts.reload() diff --git a/lib/messages/index.ts b/lib/messages/index.ts index 32b61d7c..22a000b1 100644 --- a/lib/messages/index.ts +++ b/lib/messages/index.ts @@ -3,6 +3,6 @@ export { syncCompressionBlocks } from "./sync" export { injectCompressNudges } from "./inject/inject" export { computeInputBudget } from "./inject/utils" export { injectMessageIds } from "./inject/inject" -export { stripStaleMetadata } from "./reasoning-strip" +export { stripProtectedReasoning, stripStaleMetadata } from "./reasoning-strip" export { buildPriorityMap } from "./priority" export { buildToolIdList, stripHallucinations, stripHallucinationsFromString, hasContent, dropEmptyMessages } from "./utils" diff --git a/lib/messages/reasoning-strip.ts b/lib/messages/reasoning-strip.ts index 9872e1d0..4da6390a 100644 --- a/lib/messages/reasoning-strip.ts +++ b/lib/messages/reasoning-strip.ts @@ -41,3 +41,93 @@ export function stripStaleMetadata(messages: WithParts[]): void { }) }) } + +/** + * Strip `reasoning` parts from protected-exempt HISTORICAL assistant messages. + * + * Protected (compress/skill) messages are excluded from every compression + * selection at message granularity (`filterProtectedToolMessages`), so they are + * re-sent every turn with their `reasoning` riding along — a monotonically + * growing, never-reclaimable floor in the incompressible baseline. + * + * This request-time pass removes ONLY the `reasoning` parts from such messages + * when ALL of the following gates hold (see devlog DESIGN.md): + * 1. turn-closure: the message is strictly before the last genuine user + * message. The current, possibly-open round is never touched — providers + * may require replaying the active round's thinking. + * 2. selector: the message contains a protected tool part (compress/skill). + * 3. size: the message's total reasoning length exceeds `threshold` chars. + * Small reasoning is left untouched → zero prefix churn for it. + * + * The tool call and every non-reasoning part are preserved. No state/DB writes; + * deterministic (prefix-cache-stable within a turn). + * + * @returns the number of `reasoning` parts removed. + */ +export function stripProtectedReasoning( + messages: WithParts[], + protectedTools: string[], + threshold: number, +): number { + if (protectedTools.length === 0) { + return 0 + } + + const lastUserMessage = getLastUserMessage(messages) + if (!lastUserMessage || lastUserMessage.info.role !== "user") { + return 0 + } + + // Index of the last genuine user message = start of the current round. + // Only messages strictly before it belong to closed historical turns. + let lastUserIndex = -1 + for (let i = messages.length - 1; i >= 0; i--) { + if (messages[i] === lastUserMessage) { + lastUserIndex = i + break + } + } + if (lastUserIndex <= 0) { + return 0 + } + + const protectedSet = new Set(protectedTools) + let removed = 0 + + for (let i = 0; i < lastUserIndex; i++) { + const message = messages[i] + if (!message || message.info.role !== "assistant") { + continue + } + + const parts = Array.isArray(message.parts) ? message.parts : [] + let hasProtectedTool = false + let reasoningLength = 0 + + for (const part of parts) { + if (part.type === "tool") { + if (protectedSet.has(part.tool)) { + hasProtectedTool = true + } + } else if (part.type === "reasoning") { + reasoningLength += part.text.length + } + } + + if (!hasProtectedTool) { + continue + } + if (reasoningLength <= threshold) { + continue + } + + const filtered = parts.filter((part) => part.type !== "reasoning") + if (filtered.length === parts.length) { + continue + } + message.parts = filtered + removed += parts.length - filtered.length + } + + return removed +} diff --git a/tests/e2e-message-transform.test.ts b/tests/e2e-message-transform.test.ts index 58f7009a..91b6c6cb 100644 --- a/tests/e2e-message-transform.test.ts +++ b/tests/e2e-message-transform.test.ts @@ -15,7 +15,7 @@ import assert from "node:assert/strict" import test, { beforeEach } from "node:test" -import type { PluginConfig } from "../lib/config" +import type { PluginConfig, CompressConfig } from "../lib/config" import { createChatMessageTransformHandler } from "../lib/hooks" import { Logger } from "../lib/logger" import { createSessionState, saveSessionState, type WithParts, type SessionState } from "../lib/state" @@ -29,7 +29,11 @@ import { createTestRegistry } from "./registry-stub" const SID = "session-e2e-1" -function buildConfig(overrides: Partial = {}): PluginConfig { +function buildConfig( + overrides: { compress?: Partial; gc?: Partial } & Partial< + Omit + > = {}, +): PluginConfig { const base: PluginConfig = { enabled: true, autoUpdate: true, @@ -62,7 +66,12 @@ function buildConfig(overrides: Partial = {}): PluginConfig { batchCleanup: { lowThreshold: "60%", highThreshold: "75%", forceThreshold: "90%" }, }, } - return { ...base, ...overrides } + return { + ...base, + ...overrides, + compress: { ...base.compress, ...(overrides.compress ?? {}) }, + gc: { ...base.gc, ...(overrides.gc ?? {}) }, + } } let msgCounter = 0 @@ -163,7 +172,10 @@ function createMockPrompts() { } } -function setupPipeline(stateOverrides: Partial = {}) { +function setupPipeline( + stateOverrides: Partial = {}, + configOverrides: Parameters[0] = {}, +) { const tempDir = mkdtempSync(join(tmpdir(), "acp-e2e-")) process.env.XDG_DATA_HOME = tempDir process.env.XDG_CONFIG_HOME = tempDir @@ -173,7 +185,7 @@ function setupPipeline(stateOverrides: Partial = {}) { Object.assign(state, stateOverrides) const logger = new Logger(false) - const config = buildConfig() + const config = buildConfig(configOverrides) const client = createMockClient() const prompts = createMockPrompts() const hostPermissions = { global: undefined, agents: {} } @@ -854,3 +866,63 @@ test("normal agent request (build) is still fully processed", async () => { "build: messages should be processed (suffix may be appended)", ) }) + +// ─── Test: stripProtectedReasoning kill-switch (hook-level guard) ─────────── +// The guard lives in lib/hooks.ts (`config.compress.stripProtectedReasoning !== +// false`), not in the pure function, so it must be exercised through the full +// transform handler. Regression: with the guard replaced by `if (true)`, the +// flag=false case below would strip reasoning and fail. + +test("kill-switch: stripProtectedReasoning=false preserves historical reasoning through the handler", async () => { + const big = "x".repeat(3000) + const mkProtected = (id: string): WithParts => + makeAssistantMessage(id, "summary text", [ + { type: "reasoning", text: big, id: `${id}-reason`, sessionID: SID, messageID: id }, + { + type: "tool", + tool: "compress", + callID: `${id}-call`, + id: `${id}-tool`, + sessionID: SID, + messageID: id, + state: { status: "completed", output: "ok", input: {} }, + }, + ]) + const buildMessages = (): WithParts[] => [ + makeUserMessage("u1", "do it"), + mkProtected("a1"), + makeUserMessage("u2", "next"), + mkProtected("a2"), + ] + + // Kill-switch ON (disabled): the historical protected message's reasoning must survive. + { + const { handler } = setupPipeline( + {}, + { compress: { protectedTools: ["compress"], stripProtectedReasoning: false } }, + ) + const messages = buildMessages() + const output = { messages } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + const a2 = output.messages.find((m) => m.info.id === "a2")! + assert.ok(a1.parts.some((p) => p.type === "reasoning"), "disabled: historical a1 reasoning preserved") + assert.ok(a2.parts.some((p) => p.type === "reasoning"), "disabled: current a2 reasoning preserved") + } + + // Kill-switch OFF (enabled, the default): historical reasoning stripped, current round kept. + { + const { handler } = setupPipeline( + {}, + { compress: { protectedTools: ["compress"], stripProtectedReasoning: true } }, + ) + const messages = buildMessages() + const output = { messages } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + const a2 = output.messages.find((m) => m.info.id === "a2")! + assert.ok(!a1.parts.some((p) => p.type === "reasoning"), "enabled: historical a1 reasoning stripped") + assert.ok(a1.parts.some((p) => p.type === "tool"), "enabled: a1 tool call preserved") + assert.ok(a2.parts.some((p) => p.type === "reasoning"), "enabled: current a2 reasoning preserved") + } +}) diff --git a/tests/reasoning-strip.test.ts b/tests/reasoning-strip.test.ts index 6db6fbd3..f0c46b08 100644 --- a/tests/reasoning-strip.test.ts +++ b/tests/reasoning-strip.test.ts @@ -1,6 +1,7 @@ import assert from "node:assert/strict" import test from "node:test" -import { stripStaleMetadata } from "../lib/messages/reasoning-strip" +import { stripProtectedReasoning, stripStaleMetadata } from "../lib/messages/reasoning-strip" +import { mergeCompress, type CompressConfig } from "../lib/config" import type { WithParts } from "../lib/state" const SID = "ses-reasoning-strip" @@ -42,6 +43,22 @@ function assistantMsg( } } +function protectedToolMsg(id: string, reasoningText: string, tool = "compress"): WithParts { + return { + info: { + id, + role: "assistant", + sessionID: SID, + agent: "assistant", + time: { created: 2 }, + } as WithParts["info"], + parts: [ + { id: `${id}-reason`, messageID: id, sessionID: SID, type: "reasoning", text: reasoningText }, + { id: `${id}-tool`, messageID: id, sessionID: SID, type: "tool", tool, callID: `${id}-call`, state: { status: "completed", output: "ok" } }, + ], + } +} + test("stripStaleMetadata is a no-op when no user message exists", () => { const messages: WithParts[] = [assistantMsg("a1", "model-a", "prov-a")] stripStaleMetadata(messages) @@ -113,3 +130,256 @@ test("stripStaleMetadata only considers the last user message's model", () => { stripStaleMetadata(messages) assert.ok(!("metadata" in messages[1]!.parts[0]!), "a1 metadata stripped (u2 has different model)") }) + +const PROTECTED = ["compress", "skill"] + +test("stripProtectedReasoning strips reasoning from a historical compress message above threshold", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 1) + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "reasoning removed") + assert.ok(messages[1]!.parts.some((p) => p.type === "tool"), "tool call preserved") +}) + +test("stripProtectedReasoning never touches the current round (after the last user message)", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + protectedToolMsg("a2", big), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 1, "only the historical message stripped") + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "historical a1 stripped") + assert.ok(messages[3]!.parts.some((p) => p.type === "reasoning"), "current-round a2 preserved") +}) + +test("stripProtectedReasoning no-op when the last user message is first (no history before it)", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 0, "no historical messages before the only user message") + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "a1 (current round) preserved") +}) + +test("stripProtectedReasoning anchors on the last GENUINE user message (skips synthetic)", () => { + const big = "x".repeat(3000) + const synthetic: WithParts = { + info: { + id: "msg_acp_recap_1", + role: "user", + sessionID: SID, + agent: "assistant", + model: { modelID: "claude-4", providerID: "anthropic" }, + time: { created: 3 }, + } as WithParts["info"], + parts: [{ id: "syn-p", messageID: "msg_acp_recap_1", sessionID: SID, type: "text", text: "recap" }], + } + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + protectedToolMsg("a2", big), + synthetic, + protectedToolMsg("a3", big), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 1, "only a1 (before the last genuine user message u1) stripped") + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "a1 stripped") + assert.ok(messages[3]!.parts.some((p) => p.type === "reasoning"), "a2 (after u1, before synthetic) preserved") + assert.ok(messages[5]!.parts.some((p) => p.type === "reasoning"), "a3 (current round) preserved") +}) + +test("stripProtectedReasoning leaves small reasoning untouched (<= threshold)", () => { + const small = "x".repeat(100) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", small), + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 0) + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "small reasoning preserved") +}) + +test("stripProtectedReasoning boundary: reasoning == threshold is NOT stripped (strict >)", () => { + const exact = "x".repeat(2048) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", exact), + userMsg("u1", "claude-4", "anthropic"), + ] + assert.equal(stripProtectedReasoning(messages, PROTECTED, 2048), 0) +}) + +test("stripProtectedReasoning ignores non-protected tools (bash)", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big, "bash"), + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 0) + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "bash reasoning preserved") +}) + +test("stripProtectedReasoning no-op when no user message exists", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [protectedToolMsg("a1", big)] + assert.equal(stripProtectedReasoning(messages, PROTECTED, 2048), 0) + assert.ok(messages[0]!.parts.some((p) => p.type === "reasoning")) +}) + +test("stripProtectedReasoning no-op when protectedTools is empty", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + ] + assert.equal(stripProtectedReasoning(messages, [], 2048), 0) +}) + +test("stripProtectedReasoning preserves the tool call and non-reasoning parts", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + { + info: { id: "a1", role: "assistant", sessionID: SID, agent: "assistant", time: { created: 2 } } as WithParts["info"], + parts: [ + { id: "a1-text", messageID: "a1", sessionID: SID, type: "text", text: "here is the summary" }, + { id: "a1-reason", messageID: "a1", sessionID: SID, type: "reasoning", text: big }, + { id: "a1-tool", messageID: "a1", sessionID: SID, type: "tool", tool: "compress", callID: "c1", state: { status: "completed", output: "ok" } }, + ], + }, + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 1) + assert.deepEqual(messages[1]!.parts.map((p) => p.type), ["text", "tool"], "text + tool preserved, reasoning removed") +}) + +test("stripProtectedReasoning is idempotent (second pass removes nothing)", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + ] + const first = stripProtectedReasoning(messages, PROTECTED, 2048) + const second = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(first, 1) + assert.equal(second, 0, "idempotent — nothing left to remove") +}) + +test("stripProtectedReasoning strips all qualifying historical messages (compress + skill)", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big, "compress"), + protectedToolMsg("a2", big, "skill"), + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 2) + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "a1 stripped") + assert.ok(!messages[2]!.parts.some((p) => p.type === "reasoning"), "a2 stripped") +}) + +test("stripProtectedReasoning multi-turn growth cycle: closed turns stripped, current round kept", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + protectedToolMsg("a2", big), + userMsg("u2", "claude-4", "anthropic"), + protectedToolMsg("a3", big), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 2, "a1 and a2 (closed turns) stripped") + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "a1 stripped") + assert.ok(!messages[3]!.parts.some((p) => p.type === "reasoning"), "a2 stripped") + assert.ok(messages[5]!.parts.some((p) => p.type === "reasoning"), "a3 (current round) preserved") +}) + +test("stripProtectedReasoning respects a custom threshold", () => { + const mid = "x".repeat(5000) + const mk = (): WithParts[] => [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", mid), + userMsg("u1", "claude-4", "anthropic"), + ] + assert.equal(stripProtectedReasoning(mk(), PROTECTED, 2048), 1, "stripped at 2048") + assert.equal(stripProtectedReasoning(mk(), PROTECTED, 10000), 0, "not stripped at 10000") +}) + +test("stripProtectedReasoning sums reasoning length across multiple reasoning parts", () => { + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + { + info: { id: "a1", role: "assistant", sessionID: SID, agent: "assistant", time: { created: 2 } } as WithParts["info"], + parts: [ + { id: "a1-r1", messageID: "a1", sessionID: SID, type: "reasoning", text: "x".repeat(1500) }, + { id: "a1-r2", messageID: "a1", sessionID: SID, type: "reasoning", text: "x".repeat(1500) }, + { id: "a1-tool", messageID: "a1", sessionID: SID, type: "tool", tool: "compress", callID: "c1", state: { status: "completed", output: "ok" } }, + ], + }, + userMsg("u1", "claude-4", "anthropic"), + ] + const removed = stripProtectedReasoning(messages, PROTECTED, 2048) + assert.equal(removed, 2, "both reasoning parts removed (sum 3000 > 2048)") + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning")) +}) + +const cfgBase: CompressConfig = { + permission: "allow", + showCompression: true, + summaryBuffer: true, + maxContextLimit: "55%", + minContextLimit: "45%", + nudgeFrequency: 5, + minNudgeContextPercent: 15, + iterationNudgeThreshold: 15, + nudgeForce: "soft", + protectedTools: ["skill", "compress"], + protectTags: false, + protectUserMessages: false, + maxSummaryLengthHard: 20000, + minCompressRange: 5000, + minNudgeGrowthRatio: 0.45, + minNudgeGrowthFloor: 5000, + emergencyThresholdPercent: "98%", + maxVisibleSegments: 50, + keepEmbedMaxChars: 2000, + stripProtectedReasoning: true, + stripProtectedReasoningThreshold: 2048, +} + +test("config: stripProtectedReasoning keys survive a no-op merge (defaults preserved)", () => { + const merged = mergeCompress(cfgBase, {}) + assert.equal(merged.stripProtectedReasoning, true) + assert.equal(merged.stripProtectedReasoningThreshold, 2048) +}) + +test("config: kill-switch override stripProtectedReasoning=false wins", () => { + const merged = mergeCompress(cfgBase, { stripProtectedReasoning: false }) + assert.equal(merged.stripProtectedReasoning, false) + assert.equal(merged.stripProtectedReasoningThreshold, 2048, "threshold preserved") +}) + +test("config: custom threshold override wins, flag preserved", () => { + const merged = mergeCompress(cfgBase, { stripProtectedReasoningThreshold: 5000 }) + assert.equal(merged.stripProtectedReasoningThreshold, 5000) + assert.equal(merged.stripProtectedReasoning, true, "flag preserved") +}) From e06adae98c18b5f2ce756c5443fefaae61229e05 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Tue, 8 Sep 2026 23:52:43 +0800 Subject: [PATCH 2/4] docs: record PR #370 + filed issues in WORKLOG --- .../2026-09-07_strip-protected-reasoning/WORKLOG.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md index 9ef94bd4..8d31aa2a 100644 --- a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md +++ b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md @@ -18,7 +18,7 @@ | Commit | Description | |--------|-------------| -| _pending_ | implementation (see Key Files) | +| `9477f97` | implementation (see Key Files) — PR #370 | ### Key Files @@ -78,7 +78,7 @@ Both reviewers returned REQUEST-CHANGES; core logic / tests / pipeline integrati - **Risk points**: provider reasoning-replay semantics (mitigated: current round never touched; historical closed-turn reasoning is the standard-strippable case); one-time cache rebuild on enablement (bounded, not continuous); correctness of the protected-tool selector (matches `config.compress.protectedTools`). - **Rollback method**: - Config: set `compress.stripProtectedReasoning: false` (immediate no-op). - - Revert commit(s): _pending sha_. + - Revert commit(s): `9477f97` (PR #370). - **Compatibility notes**: additive config keys only; no persisted-state or internal-tag changes. ## 6. Lessons Learned @@ -87,7 +87,8 @@ Both reviewers returned REQUEST-CHANGES; core logic / tests / pipeline integrati ## 7. Follow-ups (separate issues, source marker `来源: #368 ...`) -- [ ] #368 secondary finding **A** — display/`acp_status` estimator excludes reasoning (`lib/messages/inject/utils.ts:586` `estimateContextComposition` counts only text+tool; real usage includes reasoning per `lib/token-utils.ts:19,44`). -- [ ] #368 secondary finding **B** — orphaned `byMessageId` entries with emptied `activeBlockIds` stay visible forever (`lib/messages/prune.ts:60-66`). -- [ ] #368 secondary finding **C** — `rewriteCompressInput` full-consumption leak (`lib/compress/hide-consumed.ts:42` `kept.length === 0 → return null`). -- [ ] File the main #368 issue to `ranxianglei/billion-context` and `ranxianglei/billion-context-pi` (owner request). +- [x] #368 secondary finding **A** → filed as **#371** (display/`acp_status` estimator excludes reasoning; `lib/messages/inject/utils.ts:586`). +- [x] #368 secondary finding **B** → filed as **#372** (orphaned `byMessageId` entries stay visible; `lib/messages/prune.ts:60-66`). +- [x] #368 secondary finding **C** → filed as **#373** (`rewriteCompressInput` full-consumption leak; `lib/compress/hide-consumed.ts:42`). +- [x] Main #368 filed to `ranxianglei/billion-context` (**#651**) and `ranxianglei/billion-context-pi` (**#336**) (owner request). +- [x] PR opened: **#370** (awaiting human merge). From 0675c12cee15078f564dda8c8b2fb3efa74593b6 Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Wed, 9 Sep 2026 11:23:36 +0800 Subject: [PATCH 3/4] feat: gate stripProtectedReasoning by provider allowlist + session size (review #368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the 2026-09-08 review session (owner-approved), three changes: - Provider allowlist gate (fail-closed): stripProtectedReasoningProviders (default [anthropic, gemini], '*' = all, case-insensitive substring, entries trimmed; undefined/unmatched provider strips nothing). hooks.ts resolves the CURRENT request's provider via requestModel?.providerID ?? state.modelProviderID (requestModel hoisted from the last user message's info.model). - Session activation gate: stripProtectedReasoningMinMessages (default 100, integer >= 0; 0 = always) — short sessions keep a byte-stable prefix. - Threshold default 2048 -> 0 (size is cache-noise; activation gate is the cache lever). Tests: 1124 pass (was 1112). Mutations verified: provider gate (5 fails), activation gate (2), hook options dropped (2), hook fallbacks dropped (1). Review round 2: dual-agent APPROVE; findings addressed (trim, config validation incl. Number.isInteger, hook-fallback e2e, tight boundaries). --- dcp.schema.json | 20 +- .../DESIGN.md | 27 ++- .../REQ.md | 19 +- .../WORKLOG.md | 60 +++-- lib/config-validation.ts | 29 +++ lib/config.ts | 33 ++- lib/hooks.ts | 23 +- lib/messages/reasoning-strip.ts | 53 ++++- tests/config-validation.test.ts | 73 ++++++ tests/e2e-message-transform.test.ts | 224 ++++++++++++++++++ tests/reasoning-strip.test.ts | 188 ++++++++++++++- 11 files changed, 699 insertions(+), 50 deletions(-) diff --git a/dcp.schema.json b/dcp.schema.json index a694a3d6..8b2b9613 100644 --- a/dcp.schema.json +++ b/dcp.schema.json @@ -576,9 +576,21 @@ }, "stripProtectedReasoningThreshold": { "type": "number", - "default": 2048, + "default": 0, "minimum": 0, - "description": "Minimum total reasoning length (chars) on a protected-exempt historical message before its reasoning is stripped." + "description": "Minimum total reasoning length (chars) on a protected-exempt historical message before its reasoning is stripped. Default 0 strips regardless of size; cache protection comes from stripProtectedReasoningMinMessages." + }, + "stripProtectedReasoningProviders": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "default": ["anthropic", "gemini"], + "description": "Provider allowlist for stripProtectedReasoning (case-insensitive substring match on the provider id; '*' = all providers). Fail-closed: unknown provider or empty list strips nothing. Closed-turn thinking stripping is only documented-safe for Anthropic/Gemini; some GPT-family gateways reject incomplete historical thinking." + }, + "stripProtectedReasoningMinMessages": { + "type": "integer", + "default": 100, + "minimum": 0, + "description": "Activation gate: stripProtectedReasoning only runs when the request carries at least this many messages. Short sessions keep a byte-stable prefix; the reclaimed floor only matters on long sessions. 0 disables the gate." } }, "default": { @@ -605,7 +617,9 @@ "preserveRecentTokens": 20000, "preserveLastUserMessage": true, "stripProtectedReasoning": true, - "stripProtectedReasoningThreshold": 2048 + "stripProtectedReasoningThreshold": 0, + "stripProtectedReasoningProviders": ["anthropic", "gemini"], + "stripProtectedReasoningMinMessages": 100 } }, "gc": { diff --git a/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md b/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md index d5acd04d..79b024c2 100644 --- a/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md +++ b/devlog/2026-09-07_strip-protected-reasoning/DESIGN.md @@ -3,7 +3,7 @@ - Task ID: `2026-09-07_strip-protected-reasoning` - Home Repo: `opencode-acp` - Created: 2026-09-07 -- Status: Final (owner decision 2026-09-07: **no provider gate**, default-on, threshold 2048 chars — see §8) +- Status: Final (updated 2026-09-08 per review: **provider allowlist gate + activation gate added, threshold default 0** — see §8) ## 1. Problem Statement @@ -14,7 +14,7 @@ - **Goals**: - Reclaim the reasoning floor at request time with zero loss of user-visible / compression-critical data. - - Never break providers that require reasoning replay — enforced by the **turn-closure gate** (the active round is always preserved). No provider gate (owner decision); residual cross-turn risk handled reactively via the kill-switch. + - Never break providers that require reasoning replay — enforced by the **provider allowlist gate** (fail-closed; review update 2026-09-08) + the **turn-closure gate** (the active round is always preserved). - Keep the sent prefix cache-stable within a turn. - **Non-Goals**: - Part-granular protection rework (separate effort). @@ -50,12 +50,17 @@ Key facts (code-verified @ v1.14.27): - **Overview**: A single request-time pass inserted **after** `hideConsumedCompressCalls` (operates on the minimal surviving set; never touches about-to-be-spliced messages) and **before** `assignMessageRefs`. ``` +# request-level gates (evaluated once per request, before per-message iteration): +# Gate 0: provider allowlist — FAIL-CLOSED. allowedProviders !== undefined, "*" not in it, +# and providerID undefined or not substring-matched → strip nothing. +# Gate 0.5: session activation — minMessages > 0 and messages.length < minMessages → strip nothing. + for each assistant message m at index i: if i >= lastGenuineUserIndex: # Gate 1: current open round → KEEP continue if not hasProtectedToolPart(m): # Gate 2: selector = protected tool call (compress/skill) continue - if reasoningLength(m) <= threshold: # Gate 3: size threshold → small reasoning untouched + if reasoningLength(m) <= threshold: # Gate 3: size threshold (default 0 = strip all sizes) continue m.parts = m.parts.filter(p => p.type !== "reasoning") # drop reasoning parts only, keep tool call ``` @@ -64,8 +69,9 @@ for each assistant message m at index i: - **Pass function** in `lib/messages/reasoning-strip.ts` (new export, name distinct from `stripStaleMetadata`; e.g. `stripProtectedReasoning`). - **Gate 1 — turn-closure**: `lastGenuineUserIndex = index of getLastUserMessage(messages)`. All assistant messages at/after it are the current (possibly-open) round → reasoning kept. Only messages strictly before are candidates. If `getLastUserMessage` returns `null` → strip nothing (fail-safe). - **Gate 2 — selector** (narrow, per operator): `m` contains a **protected tool part** (`compress`/`skill`). Only these are the "floor" — normal historical messages' reasoning is already reclaimed by compression, so we do **not** target arbitrary large-reasoning messages. (Simplified from the earlier "compress part OR all-non-structural-are-protected" predicate; confirm with owner.) - - **Gate 3 — size threshold** (operator proposal, 2026-09-08): only strip when the message's total `reasoning` content length **exceeds a configurable threshold** (default ~2 KB, unit pending). Small reasoning is left untouched → zero prefix churn for those messages, and the per-message decision is stable (length doesn't change). Captures ~all of the floor (measured mean 9,418 B, max 28,067 B per message). - - **~~Gate 4 — provider policy~~ (removed per owner decision 2026-09-07).** No provider gate is implemented ("provider 先不管 有问题再说"). The turn-closure gate (Gate 1) is the safety mechanism; the global kill-switch is the self-service mitigation; a provider gate would be added reactively if a real breakage is reported. (`state.modelProviderID` remains available at the call site — `lib/hooks.ts:92,104,197,215` — should a gate be added later.) + - **Gate 3 — size threshold** (default `0` per review 2026-09-08): only strip when the message's total `reasoning` content length **exceeds the threshold**. Review rationale for 0: prefix-cache invalidation propagates from the **first divergent message** — as soon as one message in a turn is stripped, everything after it re-caches, so sparing small messages saves nothing once any large one is stripped. The measured mean is 9,418 B (max 28,067 B), far above any sensible threshold, so the gate is kept only as an operator knob; the **activation gate (Gate 5) is the cache lever**. + - **Gate 4 — provider allowlist (ADDED per review 2026-09-08).** `compress.stripProtectedReasoningProviders: string[]`, default `["anthropic","gemini"]`; `"*"` = all providers. Matching is case-insensitive substring (`providerID.toLowerCase().includes(entry.toLowerCase())`). **Fail-closed**: `modelProviderID` undefined (older opencode builds / missing model info) or unmatched → the whole pass is a no-op, because GPT-family gateways may reject incomplete historical thinking. The call site passes `state.modelProviderID` (`lib/hooks.ts`). Explicit `[]` = strip for no provider (full opt-out at the strip level, distinct from the kill-switch which disables the pass entirely). + - **Gate 5 — session activation (ADDED per review 2026-09-08).** `compress.stripProtectedReasoningMinMessages: number`, default `100`; `0` = always active. Below the threshold the pass is a byte-stable no-op, so short sessions (where the floor does not matter and any prefix churn is pure cost) never pay cache invalidation. This absorbs the issue-thread "turn-count-gated handling of ancient content" idea, scoped to the strip pass instead of compression itself. - **Action**: rebuild `m.parts` without `reasoning` parts (keep the tool call + any other non-reasoning parts). Message `info.id`/order unchanged → no effect on `mNNNNN` refs or downstream passes. - **Data flow**: pure in-memory mutation of the per-request array. **No state/DB writes.** Deterministic → idempotent. - **API / interface changes**: new config keys under `compress.*` (see §4 of REQ). No change to persisted state format, exported tool APIs, or internal `dcp` tags. @@ -78,9 +84,9 @@ for each assistant message m at index i: | Placement | before `hideConsumedCompressCalls`; after it | **after** (`:258`→`:259`) | Operates on the minimal surviving set; never processes messages about to be spliced. | | What to strip | whole message; reasoning parts only | **reasoning parts only** | Dropping the whole message kills the live summary (it lives only in the compress-call body, `state.ts:55-63`). Reasoning has no value once the summary is finalized. | | Predicate scope | any protected msg; only protected-exempt msgs | **only protected-exempt** (compress part, or all-non-structural-are-protected) | Narrow; never touches user-visible text or normal messages. | -| Provider policy | allowlist; blocklist; none | **none (owner decision 2026-09-07)** | Owner: "provider 先不管 有问题再说". Turn-closure gate + global kill-switch are the safeguards; a gate is added reactively only on a real breakage. | +| Provider policy | allowlist; blocklist; none | **allowlist, fail-closed** (review 2026-09-08) | Owner initially chose none ("provider 先不管 有问题再说"); the PR #370 review showed GPT-family gateways may 400 on incomplete historical thinking and recommended fail-closed `["anthropic","gemini"]` + `"*"` escape hatch; owner approved direct application ("直接修改pr"). | | Naming | `stripExemptReasoning`; other | **distinct from `stripStaleMetadata`** | Avoids conceptual collision in `reasoning-strip.ts`. | -| Strip trigger | uniform (all protected msgs); size-gated | **size-gated** (threshold, default ~2 KB) | Operator proposal: floor is dominated by large reasoning (mean 9.4 KB), so size-gating captures ~all benefit while leaving small reasoning untouched → smaller cache-invalidation surface + a stable per-message decision. | +| Strip trigger | uniform; size-gated; session-activation-gated | **session-activation-gated** (`minMessages: 100`) + size threshold default 0 | Review: per-message size is cache-noise (invalidation propagates from the first stripped message); a request-level activation gate bounds churn to sessions large enough to have a floor. | ## 6. Impact Analysis @@ -99,8 +105,9 @@ for each assistant message m at index i: ## 8. Open Questions (RESOLVED — owner decision 2026-09-07) -- [x] **Provider policy** — **none** ("provider 先不管 有问题再说"). Turn-closure gate + global kill-switch are the safeguards; a provider gate is added reactively only if a real breakage is reported. -- [x] **Size-threshold default + unit** — **2048 chars** (owner: "阈值按照你的推荐"). Unit = characters (matches the `part.text.length` measurement; cheap, no tokenizer). +- [x] **Provider policy** — ~~none~~ → **allowlist, fail-closed** (updated 2026-09-08 review session; owner: "直接修改pr"). `["anthropic","gemini"]` default, `"*"` = all, case-insensitive substring; undefined/unmatched provider → no-op. +- [x] **Size-threshold default + unit** — ~~2048~~ → **0 chars** (updated 2026-09-08 review: cache-noise; activation gate is the lever). Unit = characters (matches `part.text.length`; cheap, no tokenizer). +- [x] **Session activation** — **ADDED 2026-09-08 review**: `stripProtectedReasoningMinMessages: 100` (0 = always). Small sessions keep a byte-stable prefix. - [x] **Selector** — target = protected tool-call messages (`compress`/`skill`), NOT all large-reasoning messages. Confirmed. - [x] **Default on/off** — **default-on** with the global kill-switch `stripProtectedReasoning: false`. -- [x] **Provider-ID matching** — moot (no provider gate). +- [x] **Provider-ID matching** — case-insensitive substring against allowlist entries; `"*"` short-circuits. diff --git a/devlog/2026-09-07_strip-protected-reasoning/REQ.md b/devlog/2026-09-07_strip-protected-reasoning/REQ.md index ab038e05..41f500fe 100644 --- a/devlog/2026-09-07_strip-protected-reasoning/REQ.md +++ b/devlog/2026-09-07_strip-protected-reasoning/REQ.md @@ -3,7 +3,7 @@ - Task ID: `2026-09-07_strip-protected-reasoning` - Home Repo: `opencode-acp` - Created: 2026-09-07 -- Status: InProgress (design settled; **owner decision 2026-09-07: no provider gate — "provider 先不管 有问题再说"**; see §3 + DESIGN.md §8) +- Status: InProgress (**updated 2026-09-08 per review**: provider gate + activation gate added, threshold default 2048→0 — supersedes the 2026-09-07 "no provider gate" decision; see §3 + DESIGN.md §8) - Priority: P1 - Owner: ework-daemon (agent) / ranxianglei - References: https://github.com/ranxianglei/opencode-acp/issues/368 @@ -29,7 +29,7 @@ - **Constraints**: - **Request-time transform only.** ACP is a plugin with the `experimental.chat.messages.transform` hook; it can only rewrite the per-request message array. It **cannot** modify opencode's stored messages, and (per reporter) must avoid persistent/DB writes. - - **Provider safety (owner decision: no gate).** Some upstreams may require reasoning to be replayed complete (operator: **GPT/OpenAI** requires complete thinking). The owner chose **not** to add a provider gate ("provider 先不管 有问题再说") — the **turn-closure gate** is the safety mechanism (only closed historical rounds are ever touched; the active round is always preserved). Residual risk: a provider that validates thinking-signatures *across* user-turn boundaries could 400 on a stripped historical block; this is handled **reactively** (kill-switch `stripProtectedReasoning: false`, or a provider gate added later if a real breakage is reported). The kill-switch is the self-service mitigation. + - **Provider safety (review update 2026-09-08: allowlist gate, fail-closed).** Some upstreams may require reasoning to be replayed complete (operator: **GPT/OpenAI** requires complete thinking). The initial design shipped without a provider gate (owner 2026-09-07: "provider 先不管 有问题再说"); the independent review of PR #370 recommended a **provider allowlist** (fail-closed: unknown/unmatched/undefined provider → strip nothing) and the owner approved applying the review to the PR directly ("直接修改pr", 2026-09-08). Defaults: `["anthropic","gemini"]`, `"*"` = all providers, case-insensitive substring match. The **turn-closure gate** remains the primary safety mechanism (the active round is always preserved); kill-switch `stripProtectedReasoning: false` remains the self-service mitigation. - **Turn safety.** Never strip the **current open round's** reasoning (Anthropic thinking-signature / Gemini `thought_signature` replay on the active tool round). - **Cache stability.** The sent prefix must be byte-identical across consecutive requests *within a turn* so prompt caching keeps hitting; invalidation must be bounded (turn-boundary shifts + one-time enablement rebuild only). - **Surgical.** Only drop `reasoning` parts; never touch user-visible `text`; message identity/order unchanged (no effect on `mNNNNN` ref assignment). @@ -40,9 +40,10 @@ ## 4. Acceptance Criteria (must be testable) - **Correctness**: - - [ ] `reasoning` parts are removed from assistant messages that are (a) **before** the last genuine user message, (b) contain a **protected tool part** (`compress`/`skill`), and (c) whose total `reasoning` length **exceeds the configured threshold**. (No provider gate — owner decision.) + - [ ] `reasoning` parts are removed from assistant messages only when **all five gates** hold: (1) **provider allowlist** (`stripProtectedReasoningProviders`, default `["anthropic","gemini"]`; fail-closed on unknown/undefined provider; `"*"` = all; case-insensitive substring), (2) **session activation** (`stripProtectedReasoningMinMessages`, default 100; `0` = always; strips only when `messages.length >= minMessages`), (3) message is **before** the last genuine user message, (4) contains a **protected tool part** (`compress`/`skill`), (5) total `reasoning` length **exceeds the threshold** (default 0 = strip regardless of size). + - [ ] Provider gate is **fail-closed**: undefined/unmatched `modelProviderID` → no stripping (verified at hook level). + - [ ] Below `minMessages` the pass is a byte-stable no-op (small-session prefix cache untouched). - [ ] `reasoning` is **never** removed from any assistant message at/after the last genuine user message (the current round). - - [ ] Messages whose total `reasoning` length is **at or below** the threshold are left untouched (no prefix change). - [ ] Messages carrying user-visible `text` are not modified (only `reasoning` parts dropped). - [ ] Kill-switch `stripProtectedReasoning: false` disables the pass entirely (no-op) — verified at the **hook level** (full transform handler), not just the pure function. - [ ] No DB/state writes; request-time only (idempotent, no persisted mutation). @@ -58,12 +59,14 @@ - **Affected modules & entry files**: - `lib/messages/reasoning-strip.ts` — add the new pass function (distinct name from existing `stripStaleMetadata`). - `lib/hooks.ts` — wire the pass **after** `hideConsumedCompressCalls` (`:258`), **before** `assignMessageRefs` (`:259`). - - `lib/config.ts` + `dcp.schema.json` + `lib/config-validation.ts` — new config keys: `compress.stripProtectedReasoning` (bool, kill-switch, default `true`) + `compress.stripProtectedReasoningThreshold` (number, default `2048` chars). Both registered in `VALID_CONFIG_KEYS` + `validateConfigTypes`; excluded from `CompressOverridableConfig` (global-only, not per-provider overridable). + - `lib/config.ts` + `dcp.schema.json` + `lib/config-validation.ts` — config keys: `compress.stripProtectedReasoning` (bool, kill-switch, default `true`), `compress.stripProtectedReasoningThreshold` (number, default `0` chars — review: cache invalidation propagates from the first divergent message, so per-message size gating saves nothing; the activation gate is the cache lever), `compress.stripProtectedReasoningProviders` (string[], default `["anthropic","gemini"]`, `"*"` = all, case-insensitive substring; explicit `[]` = strip for no provider), `compress.stripProtectedReasoningMinMessages` (integer ≥ 0, default `100`; `0` = always active; fractional rejected by validation). All registered in `VALID_CONFIG_KEYS` + `validateConfigTypes`; threshold/providers/minMessages excluded from `CompressOverridableConfig` (global-only, not per-provider overridable). - `tests/reasoning-strip.test.ts` — unit tests for the pass; `tests/e2e-message-transform.test.ts` — hook-level kill-switch test. - **Risks**: - Provider semantics (no provider gate per owner decision; mitigated by the turn-closure gate + global kill-switch; residual cross-turn thinking-signature validation risk handled reactively). - Cache invalidation (mitigated: turn-stable prefix; bounded to boundary shifts + one-time enablement rebuild). - **Rollback strategy**: config kill-switch (`stripProtectedReasoning: false`) for immediate disable; revert the commit for full rollback. -- **RESOLVED (owner decision 2026-09-07)** — see DESIGN.md §8: - - **No provider gate** ("provider 先不管 有问题再说"). Turn-closure gate is the safety mechanism; the global kill-switch is the self-service mitigation; a provider gate is added reactively only if a real breakage is reported. - - Ships **default-on** with the global kill-switch `stripProtectedReasoning: false` + threshold `2048` (owner: "阈值按照你的推荐"). +- **RESOLVED (owner decision 2026-09-07; updated 2026-09-08 review session)** — see DESIGN.md §8: + - **Provider gate: ADDED** (review update). Initial decision was no gate ("provider 先不管 有问题再说"); the independent PR #370 review recommended a fail-closed allowlist and the owner approved direct application to the PR ("直接修改pr"). Default `["anthropic","gemini"]`. + - Ships **default-on** with the global kill-switch `stripProtectedReasoning: false`. + - **Threshold default 2048→0** (review: per-message size is noise for prefix-cache purposes — invalidation propagates from the first divergent message; the activation gate is the cache lever). + - **Activation gate ADDED** (`stripProtectedReasoningMinMessages: 100`; absorbs the issue-thread "turn-count-gated handling of ancient content" idea — same intent, scoped to the strip pass instead of compression itself). diff --git a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md index 8d31aa2a..6ec0ad35 100644 --- a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md +++ b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md @@ -2,15 +2,16 @@ - Task ID: `2026-09-07_strip-protected-reasoning` - Home Repo: `opencode-acp` -- Status: InProgress (implementation + dual-agent review fixes complete; awaiting commit + PR) -- Updated: 2026-09-08 23:05 +- Status: InProgress (review-session gate additions implemented + mutation-verified; awaiting dual-agent review of the new changes, push, human merge of PR #370) +- Updated: 2026-09-09 ## 1. Summary -- **What was done**: Implemented a request-time pass `stripProtectedReasoning` that strips `reasoning` parts from protected-exempt (compress/skill) messages in CLOSED historical turns, wired into the message-transform pipeline, with two new config keys (kill-switch + size threshold) and a full unit-test suite. +- **What was done**: Implemented a request-time pass `stripProtectedReasoning` that strips `reasoning` parts from protected-exempt (compress/skill) messages in CLOSED historical turns, wired into the message-transform pipeline, with four new config keys (kill-switch + size threshold + provider allowlist + session activation gate) and a full unit-test suite. +- **2026-09-08 review session**: independent review of PR #370 against issue #368 recommended (a) a fail-closed provider allowlist (GPT-family gateways may 400 on incomplete historical thinking), (b) a session activation gate as the cache lever, (c) threshold default 2048→0 (per-message size is cache-noise: invalidation propagates from the first stripped message). Owner approved direct application ("直接修改pr"); all three applied to PR #370's branch. - **Why**: Reclaim the monotonically-growing, never-compressible reasoning floor (~83.5% of measured residual in #368) without breaking reasoning-replay providers and with bounded cache impact. - **Behavior / compatibility changes**: Yes — additive request-time transform + additive config keys; no persisted-state/internal-tag changes. -- **Risk level**: Low-Medium — mitigated by turn-closure gate (current round never touched) + size threshold (small reasoning untouched) + kill-switch. Provider gate intentionally omitted per owner decision ("provider 先不管 有问题再说" — handle reactively). +- **Risk level**: Low — mitigated by provider allowlist (fail-closed) + turn-closure gate (current round never touched) + activation gate (small sessions byte-stable) + kill-switch. ## 2. Change Log @@ -19,6 +20,7 @@ | Commit | Description | |--------|-------------| | `9477f97` | implementation (see Key Files) — PR #370 | +| (pending) | review additions: provider allowlist gate + activation gate + threshold default 0 + 4 config keys registered + tests (mutation-verified) | ### Key Files @@ -27,21 +29,25 @@ - `lib/messages/reasoning-strip.ts` — new `stripProtectedReasoning(messages, protectedTools, threshold): number` pass (3 gates). - `lib/messages/index.ts` — barrel export. - `lib/hooks.ts` — wired between `hideConsumedCompressCalls` and `assignMessageRefs`, guarded by kill-switch. -- `lib/config.ts` — `compress.stripProtectedReasoning` (bool, default true) + `compress.stripProtectedReasoningThreshold` (number, default 2048): interface + DEFAULT_CONFIG + mergeCompress + excluded from `CompressOverridableConfig` (global-only, not per-provider overridable). -- `lib/config-validation.ts` — registered both keys in `VALID_CONFIG_KEYS` + `validateConfigTypes` (bool / non-negative finite number). -- `dcp.schema.json` — schema properties + default. -- `tests/reasoning-strip.test.ts` — 18 pass tests (turn-closure, selector, size-threshold, boundary/edge) + 3 config-merge tests + `protectedToolMsg` helper. -- `tests/e2e-message-transform.test.ts` — hook-level kill-switch test (flag=false preserves / flag=true strips); `buildConfig`/`setupPipeline` extended (backward-compatible) to accept config overrides. +- `lib/messages/reasoning-strip.ts` — (review) `stripProtectedReasoning` gained optional 4th param `options?: {providerID?, allowedProviders?, minMessages?}`; Gate 4 provider allowlist (fail-closed, `"*"`, case-insensitive substring, empty list = strip nothing) + Gate 5 activation (`minMessages > 0 && messages.length < minMessages` → no-op). Omitted options = ungated (pure-function callers unchanged). +- `lib/config.ts` — `compress.stripProtectedReasoning` (bool, default true) + `compress.stripProtectedReasoningThreshold` (number, default **0** after review) + `compress.stripProtectedReasoningProviders` (string[], default `["anthropic","gemini"]`) + `compress.stripProtectedReasoningMinMessages` (number, default **100**): interface + DEFAULT_CONFIG + mergeCompress (providers: explicit array replaces, even `[]`) + excluded from `CompressOverridableConfig` (global-only). +- `lib/config-validation.ts` — registered all four keys in `VALID_CONFIG_KEYS` + `validateConfigTypes` (bool / non-negative finite number / string[] of non-empty strings / non-negative finite number). +- `lib/hooks.ts` — call site passes `threshold ?? 0` + `{providerID: state.modelProviderID, allowedProviders, minMessages}`. +- `dcp.schema.json` — schema properties + defaults for all four keys. +- `tests/reasoning-strip.test.ts` — 18 pass tests (turn-closure, selector, size-threshold, boundary/edge) + 5 config-merge tests + `protectedToolMsg` helper + (review) 14 gate tests: provider match / fail-closed no-match / fail-closed undefined providerID / empty allowlist / `"*"` incl. undefined / case-insensitive substring / omitted-options ungated / activation below-min / at-min (`>=`) / min 0 disables / combined gates / default threshold 0. +- `tests/e2e-message-transform.test.ts` — hook-level kill-switch test (flag=false preserves / flag=true strips) + (review) hook-level provider-gate test (undefined→kept, "openai"→kept, "anthropic"→stripped) + activation-gate test (3-msg fixture, min 100 → kept); `buildConfig` compress base carries permissive strip values (`providers:["*"]`, `minMessages:0`, `threshold:0`) so fixture-sized cases exercise the strip. ## 3. Design & Implementation Notes - **Entry point / key function**: `stripProtectedReasoning` in `lib/messages/reasoning-strip.ts`, wired in `lib/hooks.ts` after `hideConsumedCompressCalls` before `assignMessageRefs`. -- **Three gates** (all must hold to strip a message's reasoning): +- **Five gates** (request-level 4 & 5 run first; all must hold to strip a message's reasoning): + 0. **provider allowlist** (review): `allowedProviders !== undefined` → empty list strips nothing; else unless `"*"` present, `providerID` must case-insensitively substring-match an entry. **Fail-closed on undefined `providerID`**. + ½. **activation** (review): `minMessages > 0 && messages.length < minMessages` → no-op (byte-stable prefix for small sessions). 1. **turn-closure**: message index strictly `< lastUserIndex` (index of `getLastUserMessage`). The current, possibly-open round is never touched. 2. **selector**: message contains a tool part whose `part.tool` ∈ `config.compress.protectedTools`. - 3. **size**: total reasoning length (sum of `part.text.length` over reasoning parts) `> threshold` (default 2048). + 3. **size**: total reasoning length (sum of `part.text.length` over reasoning parts) `> threshold` (default 0 = strip regardless of size). - **Action**: `msg.parts = parts.filter(p => p.type !== "reasoning")` — drops reasoning only; tool call + other parts preserved. Returns count removed. -- **No provider gate** (per owner): closed-turn reasoning is stripped for all providers; current round always kept. +- **Provider gate** (review update): only `anthropic`/`gemini` (default) strip historical reasoning; everyone else — including unknown provider IDs — is untouched (fail-closed). Current round always kept. - **Deterministic / cache-stable**: within a turn the output is byte-stable; the boundary shifts only when a new user turn starts (which invalidates the prefix cache anyway). ## 4. Testing & Verification @@ -57,14 +63,19 @@ npm test ### Test Coverage -- New/modified test files: `tests/reasoning-strip.test.ts` (+18), `tests/e2e-message-transform.test.ts` (+1 kill-switch). -- Test count: 1096 total, 0 failures (was 1077 before this change). +- New/modified test files: `tests/reasoning-strip.test.ts` (+18 pass tests, +2 merge tests, +14 gate tests), `tests/e2e-message-transform.test.ts` (+1 kill-switch, +2 gate tests). +- Test count: 1112 total, 0 failures (was 1096 before the review additions; 1077 before the PR). - Key scenarios verified: turn-closure (current round kept), selector (non-protected tool untouched), size threshold (`<= threshold` kept, `== threshold` kept [strict `>`], custom threshold), reasoning-only vs tool+reasoning, idempotency (2nd call removes 0), multi-turn growth cycle (closed turns stripped, current kept), summing across multiple reasoning parts, no-user-msg no-op, empty-protectedTools no-op, **synthetic-user boundary** (anchors on last genuine user msg), **first-user no-op** (`lastUserIndex<=0`), config merge (default / kill-switch / custom threshold), **hook-level kill-switch** (e2e: flag=false preserves / flag=true strips). +### Mutation verification (§5.7.3, review additions) + +- Provider gate: replacing `if (allowedProviders !== undefined)` with `if (false && ...)` → 4 tests fail (fail-closed no-match, fail-closed undefined, empty allowlist, combined gates). Restored; suite green. +- Activation gate: replacing the `minMessages` condition with `false && ...` → 2 tests fail (unit "below minMessages strips nothing"; e2e "below minMessages the handler preserves reasoning"). Restored; full suite 1112/1112 green. + ### Results -- **PASS/FAIL**: PASS — typecheck clean, build clean, 1096/1096 tests pass. -- **Key logs/data**: `tests/reasoning-strip.test.ts` 25/25 in-file (7 pre-existing `stripStaleMetadata` + 18 new). +- **PASS/FAIL**: PASS — typecheck clean, build clean, 1112/1112 tests pass. +- **Key logs/data**: `tests/reasoning-strip.test.ts` 39/39 in-file; `tests/e2e-message-transform.test.ts` 17/17 in-file. ### Dual-Agent Review (2026-09-08, both via `task`+`general`) @@ -73,9 +84,23 @@ Both reviewers returned REQUEST-CHANGES; core logic / tests / pipeline integrati - **Test**: (F1 MAJOR) added the hook-level kill-switch e2e test — **mutation-verified** (replacing the guard with `if(true)` makes it fail); (F3) synthetic-user boundary test; (F4) first-user no-op test; (F6) test-name precision fix. - **Skipped (with rationale)**: provider gate (owner declined — "provider 先不管 有问题再说"); F2 idempotency test (idempotent by construction); F5 fixture `time` field (NIT, consistent with existing style). +### Dual-Agent Review — Round 2 (2026-09-08/09, second pass on the review-session additions) + +After applying the provider/activation gates, a second dual-agent review (source + test focus) returned **APPROVE + APPROVE** (no MAJOR). Findings and dispositions: +- **R1-MINOR-2 (fixed)**: hooks.ts passed `state.modelProviderID` — undefined on the first request of a fresh session. Now `requestModel?.providerID ?? state.modelProviderID` (`requestModel` hoisted above the if/else at ~:187 from the last user message's `info.model`), preferring this request's provider metadata. +- **R1-MINOR-3 (fixed)**: allowlist entries now `.trim()`-ed at match time (both the substring match and the `"*"` check) — padded `" anthropic "` / `" * "` configs work. Pinned by unit test. +- **R1-MINOR-1/4/5/6**: doc comment clarified (undefined = gate disabled vs [] = strip nothing); duplicated default literals kept (now pinned by tests, see below); `as unknown[]` cast kept (narrow, local); internal `dcp` naming unchanged. No action needed. +- **R2-F1 (fixed)**: added 9 `tests/config-validation.test.ts` cases for the two new keys (valid/empty-list OK, wrong type, non-string entries, empty-string entries, negative, **fractional rejected** — validation tightened from `Number.isFinite` to `Number.isInteger`, schema type → `integer`). +- **R2-F2 (fixed)**: new e2e `gate fallbacks` test — config with undefined gate fields must behave as DEFAULT_CONFIG (openai kept; short anthropic session kept; 101-message anthropic session strips). **Mutation-verified**: removing the hooks.ts `??` fallbacks makes it fail (reviewer 2's Mutation C previously survived with 0 failures). +- **R2-F3 (follow-up, pre-existing)**: e2e `buildConfig` omits 8 required `CompressConfig` fields + has a phantom `mode` — compiles only because `tsconfig.json` excludes tests from typecheck. Candidate follow-up: add `tests/**/*` to tsconfig include + complete buildConfig (may surface pre-existing errors — separate PR). +- **R2-NIT 4/6/7 (fixed)**: tight boundary test (`minMessages: 4` vs 3-message fixture → no-op) + presence assertions (`!parts.some(reasoning)` after strip, `parts.some(reasoning)` when kept) + integer validation (above). +- **R2-NIT 5 (moot)**: provider-gate e2e rewritten to drive provider via last-user-message `info.model` metadata (4 sub-cases incl. metadata-vs-cached-state precedence and the `?? state` fallback), removing the hidden dependency reviewer flagged. +- **Extra (found while rewriting e2e)**: `stripStaleMetadata` (`lib/messages/reasoning-strip.ts:15`) dereferences `lastUserMessage.info.model.modelID` without optional chaining — crashes if a user message lacks `info.model`. Pre-existing, out of scope; noted to owner. +- **Verification**: 1124/1124 tests pass (was 1112 at PR open; +1 padded-entries, +1 tight boundary, +9 config-validation, +1 fallback e2e), typecheck + build clean. All 4 mutations re-verified: provider gate → 5 fails; activation gate → 2 fails; hook options dropped → 2 e2e fails; hook fallbacks dropped → 1 e2e fail. + ## 5. Risk Assessment & Rollback -- **Risk points**: provider reasoning-replay semantics (mitigated: current round never touched; historical closed-turn reasoning is the standard-strippable case); one-time cache rebuild on enablement (bounded, not continuous); correctness of the protected-tool selector (matches `config.compress.protectedTools`). +- **Risk points**: provider reasoning-replay semantics (mitigated: allowlist fail-closed + current round never touched); one-time cache rebuild on enablement (bounded, not continuous; activation gate keeps small sessions untouched); correctness of the protected-tool selector (matches `config.compress.protectedTools`). - **Rollback method**: - Config: set `compress.stripProtectedReasoning: false` (immediate no-op). - Revert commit(s): `9477f97` (PR #370). @@ -92,3 +117,4 @@ Both reviewers returned REQUEST-CHANGES; core logic / tests / pipeline integrati - [x] #368 secondary finding **C** → filed as **#373** (`rewriteCompressInput` full-consumption leak; `lib/compress/hide-consumed.ts:42`). - [x] Main #368 filed to `ranxianglei/billion-context` (**#651**) and `ranxianglei/billion-context-pi` (**#336**) (owner request). - [x] PR opened: **#370** (awaiting human merge). +- [x] 2026-09-08 review session applied to PR #370: provider allowlist + activation gate + threshold 0 (owner: "直接修改pr"). diff --git a/lib/config-validation.ts b/lib/config-validation.ts index aa0a375e..630bc050 100644 --- a/lib/config-validation.ts +++ b/lib/config-validation.ts @@ -51,6 +51,8 @@ export const VALID_CONFIG_KEYS = new Set([ "compress.preserveLastUserMessage", "compress.stripProtectedReasoning", "compress.stripProtectedReasoningThreshold", + "compress.stripProtectedReasoningProviders", + "compress.stripProtectedReasoningMinMessages", "gc", "gc.algorithm", "gc.promotionThreshold", @@ -565,6 +567,33 @@ export function validateConfigTypes(config: Record): ValidationErro }) } + if ( + compress.stripProtectedReasoningProviders !== undefined && + (!Array.isArray(compress.stripProtectedReasoningProviders) || + !(compress.stripProtectedReasoningProviders as unknown[]).every( + (entry) => typeof entry === "string" && entry.trim() !== "", + )) + ) { + errors.push({ + key: "compress.stripProtectedReasoningProviders", + expected: "string[] (non-empty strings; \"*\" = all providers)", + actual: JSON.stringify(compress.stripProtectedReasoningProviders), + }) + } + + if ( + compress.stripProtectedReasoningMinMessages !== undefined && + (typeof compress.stripProtectedReasoningMinMessages !== "number" || + !Number.isInteger(compress.stripProtectedReasoningMinMessages) || + compress.stripProtectedReasoningMinMessages < 0) + ) { + errors.push({ + key: "compress.stripProtectedReasoningMinMessages", + expected: "integer (>= 0)", + actual: JSON.stringify(compress.stripProtectedReasoningMinMessages), + }) + } + if ( typeof compress.iterationNudgeThreshold === "number" && compress.iterationNudgeThreshold < 1 diff --git a/lib/config.ts b/lib/config.ts index 874d7e74..34d9fa43 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -30,6 +30,8 @@ export type CompressOverridableConfig = Omit< // would be a silent no-op. Excluded to keep the overridable surface honest. | "stripProtectedReasoning" | "stripProtectedReasoningThreshold" + | "stripProtectedReasoningProviders" + | "stripProtectedReasoningMinMessages" > /** Per-model / per-provider override object (all overridable fields optional). */ @@ -97,10 +99,29 @@ export interface CompressConfig { stripProtectedReasoning?: boolean /** * Minimum total reasoning length (chars) on a protected-exempt historical - * message before its reasoning is stripped. Small reasoning is left untouched - * to avoid prefix-cache churn. Default: 2048. + * message before its reasoning is stripped. Default: 0 — strip regardless + * of size. Cache protection comes from the activation gate + * (`stripProtectedReasoningMinMessages`), not the per-message size: prefix + * invalidation propagates from the first divergent message, so sparing + * small-reasoning messages rarely avoids a fork (issue #368 review). */ stripProtectedReasoningThreshold?: number + /** + * Provider allowlist for `stripProtectedReasoning` (case-insensitive + * substring match on the request's provider id; `"*"` = all providers). + * FAIL-CLOSED: an unknown provider id or an explicit `[]` strips nothing — + * closed-turn thinking stripping is only documented-safe for a known set + * (Anthropic, Gemini); some upstreams (GPT-family gateways) reject + * incomplete historical thinking. Default: ["anthropic", "gemini"]. + */ + stripProtectedReasoningProviders?: string[] + /** + * Activation gate: `stripProtectedReasoning` only runs when the request + * carries at least this many messages. Short sessions keep a byte-stable + * prefix for free; the reclaimed floor only matters on long sessions. + * Default: 100. Set 0 to disable the gate. + */ + stripProtectedReasoningMinMessages?: number } export interface Commands { @@ -279,7 +300,9 @@ const defaultConfig: PluginConfig = { preserveRecentTokens: 5000, preserveLastUserMessage: true, stripProtectedReasoning: true, - stripProtectedReasoningThreshold: 2048, + stripProtectedReasoningThreshold: 0, + stripProtectedReasoningProviders: ["anthropic", "gemini"], + stripProtectedReasoningMinMessages: 100, }, gc: { algorithm: "truncate", @@ -513,6 +536,10 @@ export function mergeCompress( preserveLastUserMessage: override.preserveLastUserMessage ?? base.preserveLastUserMessage, stripProtectedReasoning: override.stripProtectedReasoning ?? base.stripProtectedReasoning, stripProtectedReasoningThreshold: override.stripProtectedReasoningThreshold ?? base.stripProtectedReasoningThreshold, + stripProtectedReasoningProviders: Array.isArray(override.stripProtectedReasoningProviders) + ? [...override.stripProtectedReasoningProviders] + : base.stripProtectedReasoningProviders, + stripProtectedReasoningMinMessages: override.stripProtectedReasoningMinMessages ?? base.stripProtectedReasoningMinMessages, } } diff --git a/lib/hooks.ts b/lib/hooks.ts index 36ade58a..ce79561a 100644 --- a/lib/hooks.ts +++ b/lib/hooks.ts @@ -164,6 +164,12 @@ export function createChatMessageTransformHandler( } const lastUserMessage = getLastUserMessage(messages) + // Model named on this request's last user message. Hoisted so the + // stripProtectedReasoning call below can gate on the CURRENT request's + // provider even before state.modelProviderID is populated. + const requestModel = ( + lastUserMessage?.info as { model?: { providerID?: string; modelID?: string } } | undefined + )?.model let state: SessionState if (!lastUserMessage) { // Ephemeral state: no session to resolve, but keep running @@ -185,9 +191,6 @@ export function createChatMessageTransformHandler( // value still reflects the previous model. Reconcile it from the // catalog entry for the model named on this request's user message // before any consumer (filters, GC, nudge thresholds) reads it. - const requestModel = ( - lastUserMessage.info as { model?: { providerID?: string; modelID?: string } } - ).model const requestModelLimit = registry.resolveModelLimit( requestModel?.providerID, requestModel?.modelID, @@ -261,7 +264,19 @@ export function createChatMessageTransformHandler( const removedReasoning = stripProtectedReasoning( output.messages, config.compress.protectedTools, - config.compress.stripProtectedReasoningThreshold ?? 2048, + config.compress.stripProtectedReasoningThreshold ?? 0, + { + // Prefer this request's provider from the user-message + // model metadata; fall back to the cached identity pair + // (undefined on the first request of a fresh session → + // fail-closed). + providerID: requestModel?.providerID ?? state.modelProviderID, + allowedProviders: config.compress.stripProtectedReasoningProviders ?? [ + "anthropic", + "gemini", + ], + minMessages: config.compress.stripProtectedReasoningMinMessages ?? 100, + }, ) if (removedReasoning > 0) { logger.debug("stripProtectedReasoning: removed reasoning parts from historical protected messages", { diff --git a/lib/messages/reasoning-strip.ts b/lib/messages/reasoning-strip.ts index 4da6390a..6edc2db9 100644 --- a/lib/messages/reasoning-strip.ts +++ b/lib/messages/reasoning-strip.ts @@ -56,23 +56,72 @@ export function stripStaleMetadata(messages: WithParts[]): void { * message. The current, possibly-open round is never touched — providers * may require replaying the active round's thinking. * 2. selector: the message contains a protected tool part (compress/skill). - * 3. size: the message's total reasoning length exceeds `threshold` chars. - * Small reasoning is left untouched → zero prefix churn for it. + * 3. size: the message's total reasoning length exceeds `threshold` chars + * (default 0 — strip regardless of size; the cache-protective gate is the + * session-size activation gate below, not the per-message size). + * 4. provider allowlist: only strip when the current request's provider is + * on `allowedProviders` (case-insensitive substring; `"*"` = all). + * FAIL-CLOSED: unknown/undefined provider or empty list strips nothing. + * Closed-turn stripping is only *documented-safe* for a known set of + * providers (Anthropic, Gemini); some upstreams (GPT-family via certain + * gateways) reject incomplete historical thinking — so unknown providers + * must not be touched (issue #368 review). + * 5. activation: only strip when the request carries at least `minMessages` + * messages (default 100). Small sessions keep byte-stable prefixes for + * free; the floor this pass reclaims only matters on long sessions. * * The tool call and every non-reasoning part are preserved. No state/DB writes; * deterministic (prefix-cache-stable within a turn). * * @returns the number of `reasoning` parts removed. */ +export interface StripProtectedReasoningOptions { + /** Provider id of the current request (e.g. "anthropic"). Undefined = unknown → fail-closed. */ + providerID?: string + /** Allowlist entries (case-insensitive substring match). `"*"` = all providers. Undefined = gate disabled (legacy callers); empty list = strip nothing (fail-closed). */ + allowedProviders?: string[] + /** Activation gate: strip only when `messages.length >= minMessages`. 0/undefined = always. */ + minMessages?: number +} + export function stripProtectedReasoning( messages: WithParts[], protectedTools: string[], threshold: number, + options?: StripProtectedReasoningOptions, ): number { if (protectedTools.length === 0) { return 0 } + // Gate 4 — provider allowlist (fail-closed). An explicitly provided list + // gates the pass: no entry matches (or provider unknown / list empty) → + // strip nothing. `"*"` opts in for every provider. + const allowedProviders = options?.allowedProviders + if (allowedProviders !== undefined) { + if (allowedProviders.length === 0) { + return 0 + } + if (!allowedProviders.some((entry) => entry.trim() === "*")) { + const providerID = options?.providerID + if ( + providerID === undefined || + !allowedProviders.some((entry) => + providerID.toLowerCase().includes(entry.trim().toLowerCase()), + ) + ) { + return 0 + } + } + } + + // Gate 5 — session-size activation. Below the floor the pass is a no-op so + // short sessions never pay any prefix-cache churn for it. + const minMessages = options?.minMessages + if (minMessages !== undefined && minMessages > 0 && messages.length < minMessages) { + return 0 + } + const lastUserMessage = getLastUserMessage(messages) if (!lastUserMessage || lastUserMessage.info.role !== "user") { return 0 diff --git a/tests/config-validation.test.ts b/tests/config-validation.test.ts index 57275e70..ab0f8a77 100644 --- a/tests/config-validation.test.ts +++ b/tests/config-validation.test.ts @@ -223,6 +223,79 @@ test("validateConfigTypes rejects negative compress.preserveRecentTokens", () => assert.equal(result[0].key, "compress.preserveRecentTokens") }) +test("validateConfigTypes accepts valid compress.stripProtectedReasoningProviders (#368)", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningProviders: ["anthropic", "gemini", "*"] }, + }) + assert.equal(result.length, 0) +}) + +test("validateConfigTypes accepts empty compress.stripProtectedReasoningProviders (#368)", () => { + // Explicit empty array is a legal value ("strip for no provider"), not an error. + const result = validateConfigTypes({ + compress: { stripProtectedReasoningProviders: [] }, + }) + assert.equal(result.length, 0) +}) + +test("validateConfigTypes catches wrong type for compress.stripProtectedReasoningProviders", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningProviders: "anthropic" }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningProviders") + assert.equal(result[0].expected, 'string[] (non-empty strings; "*" = all providers)') +}) + +test("validateConfigTypes catches non-string entries in stripProtectedReasoningProviders", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningProviders: ["anthropic", 123] }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningProviders") +}) + +test("validateConfigTypes rejects empty-string entries in stripProtectedReasoningProviders", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningProviders: ["anthropic", ""] }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningProviders") +}) + +test("validateConfigTypes accepts valid compress.stripProtectedReasoningMinMessages (#368)", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningMinMessages: 100 }, + }) + assert.equal(result.length, 0) +}) + +test("validateConfigTypes catches wrong type for compress.stripProtectedReasoningMinMessages", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningMinMessages: "100" }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningMinMessages") + assert.equal(result[0].expected, "integer (>= 0)") +}) + +test("validateConfigTypes rejects negative compress.stripProtectedReasoningMinMessages", () => { + const result = validateConfigTypes({ + compress: { stripProtectedReasoningMinMessages: -1 }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningMinMessages") +}) + +test("validateConfigTypes rejects fractional compress.stripProtectedReasoningMinMessages", () => { + // Message counts are integers; 2.5 would act as a fractional floor. + const result = validateConfigTypes({ + compress: { stripProtectedReasoningMinMessages: 2.5 }, + }) + assert.equal(result.length, 1) + assert.equal(result[0].key, "compress.stripProtectedReasoningMinMessages") +}) + test("validateConfigTypes catches wrong type for compress.preserveLastUserMessage", () => { const result = validateConfigTypes({ compress: { preserveLastUserMessage: 1 }, diff --git a/tests/e2e-message-transform.test.ts b/tests/e2e-message-transform.test.ts index 91b6c6cb..aa8d0740 100644 --- a/tests/e2e-message-transform.test.ts +++ b/tests/e2e-message-transform.test.ts @@ -56,6 +56,12 @@ function buildConfig( protectedTools: ["task"], protectTags: false, protectUserMessages: false, + // Permissive strip-gate values so fixture-sized e2e cases exercise the + // strip itself; the gates get dedicated e2e tests below. + stripProtectedReasoning: true, + stripProtectedReasoningThreshold: 0, + stripProtectedReasoningProviders: ["*"], + stripProtectedReasoningMinMessages: 0, }, gc: { algorithm: "truncate", @@ -926,3 +932,221 @@ test("kill-switch: stripProtectedReasoning=false preserves historical reasoning assert.ok(a2.parts.some((p) => p.type === "reasoning"), "enabled: current a2 reasoning preserved") } }) + +// ─── Test: provider allowlist gate (hook-level, fail-closed) ─────────────── +// The gate reads state.modelProviderID in lib/hooks.ts. Unknown provider must +// never be stripped (issue #368 review: GPT-family gateways may reject +// incomplete historical thinking). Regression: removing the provider check in +// stripProtectedReasoning makes the preserved-cases below fail. + +test("provider gate: unknown provider is fail-closed, allowlisted provider strips", async () => { + const big = "x".repeat(3000) + const mkProtected = (id: string): WithParts => + makeAssistantMessage(id, "summary text", [ + { type: "reasoning", text: big, id: `${id}-reason`, sessionID: SID, messageID: id }, + { + type: "tool", + tool: "compress", + callID: `${id}-call`, + id: `${id}-tool`, + sessionID: SID, + messageID: id, + state: { status: "completed", output: "ok", input: {} }, + }, + ]) + // User message with controllable model metadata. hooks.ts resolves the + // request provider as `requestModel?.providerID ?? state.modelProviderID` + // (this request's metadata first, cached identity pair as fallback). + const userWithModel = ( + id: string, + text: string, + model?: { providerID: string | undefined; modelID: string }, + ): WithParts => { + const msg = makeUserMessage(id, text) + ;(msg.info as { model?: { providerID: string; modelID: string } }).model = model + return msg + } + const buildMessages = (lastUserModel?: { providerID: string | undefined; modelID: string }): WithParts[] => [ + makeUserMessage("u1", "do it"), + mkProtected("a1"), + userWithModel("u2", "next", lastUserModel), + ] + const gates = { + compress: { + protectedTools: ["compress"], + stripProtectedReasoningProviders: ["anthropic", "gemini"] as string[], + stripProtectedReasoningMinMessages: 0, + }, + } + + // Unknown provider (fixture default "test-provider", not allowlisted, no + // cached state) → fail-closed, no strip. (The metadata-AND-state-both- + // undefined case is covered at the unit level; stripStaleMetadata assumes + // info.model is present on user messages, so the fixture keeps it.) + { + const { handler } = setupPipeline({}, gates) + const output = { messages: buildMessages({ providerID: "test-provider", modelID: "test-model" }) } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok(a1.parts.some((p) => p.type === "reasoning"), "unknown provider: fail-closed, reasoning kept") + } + + // This request's metadata names a non-allowlisted provider → no strip + // (metadata wins over the cached state below). + { + const { handler } = setupPipeline({ modelProviderID: "anthropic" }, gates) + const output = { messages: buildMessages({ providerID: "openai", modelID: "gpt-test" }) } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok( + a1.parts.some((p) => p.type === "reasoning"), + "openai: not allowlisted, reasoning kept (request metadata preferred)", + ) + } + + // This request's metadata names an allowlisted provider → strip. + { + const { handler } = setupPipeline({}, gates) + const output = { messages: buildMessages({ providerID: "anthropic", modelID: "claude-test" }) } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok(!a1.parts.some((p) => p.type === "reasoning"), "anthropic: allowlisted, reasoning stripped") + assert.ok(a1.parts.some((p) => p.type === "tool"), "anthropic: tool call preserved") + } + + // Request metadata carries no providerID (e.g. some gateways) but the + // cached identity pair is allowlisted → the `?? state.modelProviderID` + // fallback alone gates the strip. + // (info.model itself must stay present: stripStaleMetadata assumes it.) + { + const { handler } = setupPipeline({ modelProviderID: "gemini" }, gates) + const output = { + messages: buildMessages({ providerID: undefined as unknown as string, modelID: "test-model" }), + } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok(!a1.parts.some((p) => p.type === "reasoning"), "fallback to cached gemini: stripped") + } +}) + +// ─── Test: session-size activation gate (hook-level) ─────────────────────── + +test("activation gate: below minMessages the handler preserves reasoning", async () => { + const big = "x".repeat(3000) + const mkProtected = (id: string): WithParts => + makeAssistantMessage(id, "summary text", [ + { type: "reasoning", text: big, id: `${id}-reason`, sessionID: SID, messageID: id }, + { + type: "tool", + tool: "compress", + callID: `${id}-call`, + id: `${id}-tool`, + sessionID: SID, + messageID: id, + state: { status: "completed", output: "ok", input: {} }, + }, + ]) + const { handler } = setupPipeline( + {}, + { compress: { protectedTools: ["compress"], stripProtectedReasoningMinMessages: 100 } }, + ) + // Allowlisted provider via request metadata so ONLY the activation gate + // can block the strip (mutation coverage for the minMessages check). + const lastUser = makeUserMessage("u2", "next") + ;(lastUser.info as { model?: { providerID: string; modelID: string } }).model = { + providerID: "anthropic", + modelID: "claude-test", + } + const output = { + messages: [makeUserMessage("u1", "do it"), mkProtected("a1"), lastUser], + } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok( + a1.parts.some((p) => p.type === "reasoning"), + "3-message session < minMessages 100: prefix must stay byte-stable", + ) +}) + +// ─── Test: hook-level fallback defaults (config fields absent) ───────────── + +test("gate fallbacks: absent config fields fall back to defaults (anthropic/gemini, 100, 0)", async () => { + const big = "x".repeat(3000) + const mkProtected = (id: string): WithParts => + makeAssistantMessage(id, "summary text", [ + { type: "reasoning", text: big, id: `${id}-reason`, sessionID: SID, messageID: id }, + { + type: "tool", + tool: "compress", + callID: `${id}-call`, + id: `${id}-tool`, + sessionID: SID, + messageID: id, + state: { status: "completed", output: "ok", input: {} }, + }, + ]) + const userWithModel = ( + id: string, + text: string, + providerID: string, + ): WithParts => { + const msg = makeUserMessage(id, text) + ;(msg.info as { model?: { providerID: string; modelID: string } }).model = { + providerID, + modelID: "test-model", + } + return msg + } + // Undefined gate fields simulate configs from before these keys existed + // (older installs / partial overrides): hooks.ts must apply the same + // fail-closed defaults as DEFAULT_CONFIG (providers [anthropic, gemini], + // minMessages 100, threshold 0). Spread with explicit undefined values so + // buildConfig does NOT fill in its permissive ["*"]/0 base. + const gates = { + compress: { + protectedTools: ["compress"], + stripProtectedReasoningThreshold: undefined, + stripProtectedReasoningProviders: undefined as unknown as string[], + stripProtectedReasoningMinMessages: undefined as unknown as number, + }, + } + + // Fallback providers: openai (via request metadata) is NOT allowlisted. + { + const { handler } = setupPipeline({}, gates) + const output = { + messages: [makeUserMessage("u1", "do it"), mkProtected("a1"), userWithModel("u2", "next", "openai")], + } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok(a1.parts.some((p) => p.type === "reasoning"), "fallback providers: openai kept") + } + + // Fallback minMessages: anthropic short session (< 100) is a no-op. + { + const { handler } = setupPipeline({}, gates) + const output = { + messages: [makeUserMessage("u1", "do it"), mkProtected("a1"), userWithModel("u2", "next", "anthropic")], + } + await handler({}, output) + const a1 = output.messages.find((m) => m.info.id === "a1")! + assert.ok(a1.parts.some((p) => p.type === "reasoning"), "fallback minMessages: short anthropic session kept") + } + + // Both fallbacks satisfied: long (101-message) allowlisted session strips. + // NOTE: assert the LAST protected message — hideConsumedCompressCalls runs + // before the strip and hides all but the last 2 orphaned compress calls, + // so earlier ones (a1…) have already lost their tool parts and are no + // longer selector-eligible. a99 keeps its tool part and its reasoning. + { + const { handler } = setupPipeline({}, gates) + const messages: WithParts[] = [makeUserMessage("u1", "do it")] + for (let i = 1; i <= 99; i++) messages.push(mkProtected(`a${i}`)) + messages.push(userWithModel("u2", "next", "anthropic")) + const output = { messages } + await handler({}, output) + const a99 = output.messages.find((m) => m.info.id === "a99")! + assert.ok(a99.parts.some((p) => p.type === "tool"), "precondition: a99 kept its tool part") + assert.ok(!a99.parts.some((p) => p.type === "reasoning"), "fallbacks satisfied: long anthropic session strips") + } +}) diff --git a/tests/reasoning-strip.test.ts b/tests/reasoning-strip.test.ts index f0c46b08..31dcc6b3 100644 --- a/tests/reasoning-strip.test.ts +++ b/tests/reasoning-strip.test.ts @@ -363,19 +363,23 @@ const cfgBase: CompressConfig = { maxVisibleSegments: 50, keepEmbedMaxChars: 2000, stripProtectedReasoning: true, - stripProtectedReasoningThreshold: 2048, + stripProtectedReasoningThreshold: 0, + stripProtectedReasoningProviders: ["anthropic", "gemini"], + stripProtectedReasoningMinMessages: 100, } test("config: stripProtectedReasoning keys survive a no-op merge (defaults preserved)", () => { const merged = mergeCompress(cfgBase, {}) assert.equal(merged.stripProtectedReasoning, true) - assert.equal(merged.stripProtectedReasoningThreshold, 2048) + assert.equal(merged.stripProtectedReasoningThreshold, 0) + assert.deepEqual(merged.stripProtectedReasoningProviders, ["anthropic", "gemini"]) + assert.equal(merged.stripProtectedReasoningMinMessages, 100) }) test("config: kill-switch override stripProtectedReasoning=false wins", () => { const merged = mergeCompress(cfgBase, { stripProtectedReasoning: false }) assert.equal(merged.stripProtectedReasoning, false) - assert.equal(merged.stripProtectedReasoningThreshold, 2048, "threshold preserved") + assert.equal(merged.stripProtectedReasoningThreshold, 0, "threshold preserved") }) test("config: custom threshold override wins, flag preserved", () => { @@ -383,3 +387,181 @@ test("config: custom threshold override wins, flag preserved", () => { assert.equal(merged.stripProtectedReasoningThreshold, 5000) assert.equal(merged.stripProtectedReasoning, true, "flag preserved") }) + +test("config: explicit providers array replaces the default list (even empty)", () => { + const replaced = mergeCompress(cfgBase, { stripProtectedReasoningProviders: ["zhipu"] }) + assert.deepEqual(replaced.stripProtectedReasoningProviders, ["zhipu"]) + const emptied = mergeCompress(cfgBase, { stripProtectedReasoningProviders: [] }) + assert.deepEqual(emptied.stripProtectedReasoningProviders, [], "explicit [] = strip for no provider") +}) + +test("config: minMessages override wins", () => { + const merged = mergeCompress(cfgBase, { stripProtectedReasoningMinMessages: 0 }) + assert.equal(merged.stripProtectedReasoningMinMessages, 0) +}) + +// ─── Gate 4: provider allowlist (fail-closed) ─────────────────────────────── + +const OPT_ANTHROPIC = { allowedProviders: ["anthropic", "gemini"] } + +function stripFixture(): WithParts[] { + const big = "x".repeat(3000) + return [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", big), + userMsg("u1", "claude-4", "anthropic"), + ] +} + +test("provider gate: strips when providerID matches an allowlist entry", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + ...OPT_ANTHROPIC, + providerID: "anthropic", + }) + assert.equal(removed, 1) + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "stripped for allowlisted provider") +}) + +test("provider gate: fail-closed when providerID does not match", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + ...OPT_ANTHROPIC, + providerID: "openai", + }) + assert.equal(removed, 0) + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "preserved for non-allowlisted provider") +}) + +test("provider gate: fail-closed when providerID is undefined", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, OPT_ANTHROPIC) + assert.equal(removed, 0, "unknown provider must never be stripped (fail-closed)") + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning")) +}) + +test("provider gate: empty allowlist strips nothing", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: [], + providerID: "anthropic", + }) + assert.equal(removed, 0) + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "part itself is still present") +}) + +test("provider gate: '*' strips for any provider, including undefined", () => { + for (const providerID of ["zhipu", "openai", undefined]) { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: ["*"], + providerID, + }) + assert.equal(removed, 1, `stripped for providerID=${String(providerID)}`) + } +}) + +test("provider gate: matching is case-insensitive substring", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: ["Anthropic"], + providerID: "ANTHROPIC-claude", + }) + assert.equal(removed, 1, "substring + case-insensitive entry matches") +}) + +test("provider gate: padded allowlist entries (and padded '*') are trimmed at match time", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: [" anthropic \t"], + providerID: "anthropic-claude", + }) + assert.equal(removed, 1, "padded entry still matches") + + const wildcard = stripFixture() + assert.equal( + stripProtectedReasoning(wildcard, PROTECTED, 0, { + allowedProviders: [" * "], + providerID: undefined, + }), + 1, + "padded '*' still short-circuits the gate", + ) +}) + +test("provider gate: omitted options keeps legacy ungated behavior (pure-function callers)", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0) + assert.equal(removed, 1, "no options → no provider/activation gate") +}) + +// ─── Gate 5: session-size activation ─────────────────────────────────────── + +test("activation gate: below minMessages strips nothing", () => { + const messages = stripFixture() + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: ["*"], + minMessages: 100, + }) + assert.equal(removed, 0, "3-message fixture < 100 → no-op") + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "small session prefix untouched") +}) + +test("activation gate: at or above minMessages strips", () => { + const messages = stripFixture() + assert.equal( + stripProtectedReasoning(messages, PROTECTED, 0, { allowedProviders: ["*"], minMessages: 3 }), + 1, + "messages.length == minMessages (>=) → strips", + ) + assert.ok(!messages[1]!.parts.some((p) => p.type === "reasoning"), "reasoning part actually removed") +}) + +test("activation gate: tight boundary — one below minMessages strips nothing", () => { + // 3-message fixture, minMessages 4: the only difference from the >= case + // above is the boundary itself. Catches < vs <= mutants precisely. + const messages = stripFixture() + assert.equal( + stripProtectedReasoning(messages, PROTECTED, 0, { allowedProviders: ["*"], minMessages: 4 }), + 0, + "messages.length == minMessages - 1 → no-op", + ) + assert.ok(messages[1]!.parts.some((p) => p.type === "reasoning"), "small-session prefix untouched") +}) + +test("activation gate: 0 disables the gate", () => { + const messages = stripFixture() + assert.equal( + stripProtectedReasoning(messages, PROTECTED, 0, { allowedProviders: ["*"], minMessages: 0 }), + 1, + ) +}) + +test("combined gates: provider mismatch wins even on a large session", () => { + const big = "x".repeat(3000) + const messages: WithParts[] = [userMsg("u0", "gpt-5", "openai")] + for (let i = 0; i < 150; i++) { + messages.push(protectedToolMsg(`a${i}`, big)) + } + messages.push(userMsg("u1", "gpt-5", "openai")) + const removed = stripProtectedReasoning(messages, PROTECTED, 0, { + allowedProviders: ["anthropic", "gemini"], + providerID: "openai", + minMessages: 100, + }) + assert.equal(removed, 0, "fail-closed provider gate blocks the strip regardless of session size") +}) + +test("default threshold 0 strips small historical reasoning too (activation gate is the cache lever)", () => { + const small = "x".repeat(100) + const messages: WithParts[] = [ + userMsg("u0", "claude-4", "anthropic"), + protectedToolMsg("a1", small), + userMsg("u1", "claude-4", "anthropic"), + ] + assert.equal( + stripProtectedReasoning(messages, PROTECTED, 0, { allowedProviders: ["*"], minMessages: 0 }), + 1, + "threshold 0 = strip regardless of reasoning size", + ) +}) From 70c21ba44bae84357d14b03e9af3f07156ab8998 Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Wed, 9 Sep 2026 11:46:45 +0800 Subject: [PATCH 4/4] docs: document stripProtectedReasoning config keys (EN + zh-CN) Adds the 4 new compress.* keys to CONFIGURATION.md / CONFIGURATION.zh-CN.md (reference sections) and README.md / README.zh-CN.md (example config blocks): stripProtectedReasoning, ...Threshold, ...Providers, ...MinMessages. --- CONFIGURATION.md | 24 +++++++++++++++++++ CONFIGURATION.zh-CN.md | 24 +++++++++++++++++++ README.md | 7 ++++++ README.zh-CN.md | 7 ++++++ .../WORKLOG.md | 1 + 5 files changed, 63 insertions(+) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 590e9f58..ad2568df 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -335,6 +335,30 @@ In this example, for `anthropic/claude-sonnet-4-6`: the floor is 30%, the over-m - **Status:** ACTIVE - **Description:** Always protect the most recent user message from compression, regardless of `preserveRecentMessages` or `preserveRecentTokens`. +#### `compress.stripProtectedReasoning` +- **Type:** `boolean` +- **Default:** `true` +- **Status:** ACTIVE +- **Description:** Kill-switch for closed-turn thinking stripping (#368). Protected-exempt messages (e.g. `compress`/`skill` tool calls) are excluded from compression at message granularity, so their `reasoning` parts are re-sent on every request and form a permanently incompressible floor. When enabled (default), ACP drops the `reasoning` parts from those messages in **closed historical turns** (strictly before the last genuine user message) at request time. The active round is never touched; no persisted state is modified. Set to `false` to disable entirely. + +#### `compress.stripProtectedReasoningThreshold` +- **Type:** `number` +- **Default:** `0` +- **Status:** ACTIVE +- **Description:** Minimum total `reasoning` length (chars) on a protected-exempt historical message before its reasoning is stripped. `0` (default) strips regardless of size — per-message size is prefix-cache noise (invalidation propagates from the first stripped message); cache stability is controlled by `stripProtectedReasoningMinMessages` instead. + +#### `compress.stripProtectedReasoningProviders` +- **Type:** `string[]` +- **Default:** `["anthropic", "gemini"]` +- **Status:** ACTIVE +- **Description:** Provider allowlist for closed-turn thinking stripping (case-insensitive substring match on the provider id; `"*"` = all providers; entries are trimmed). **Fail-closed**: an unknown, undefined, or non-matching provider strips nothing, as does an explicit `[]` (strip for no provider). Closed-turn thinking stripping is only documented-safe for Anthropic/Gemini; some GPT-family gateways reject requests whose historical thinking blocks are incomplete. + +#### `compress.stripProtectedReasoningMinMessages` +- **Type:** `integer` (≥ 0) +- **Default:** `100` +- **Status:** ACTIVE +- **Description:** Activation gate: closed-turn thinking stripping only runs when the request carries at least this many messages. Short sessions keep a byte-stable prefix (no cache churn); the reclaimed floor only matters on long sessions. `0` disables the gate. Fractional values are rejected by validation. + --- ### `gc` (Generation & Cleanup) diff --git a/CONFIGURATION.zh-CN.md b/CONFIGURATION.zh-CN.md index 9a7d4486..7e9431cf 100644 --- a/CONFIGURATION.zh-CN.md +++ b/CONFIGURATION.zh-CN.md @@ -335,6 +335,30 @@ ACP 从最多三层配置文件中读取（后加载的覆盖先加载的）： - **状态：** ACTIVE - **说明：** 始终保护最近一条用户消息不被压缩，无论 `preserveRecentMessages` 或 `preserveRecentTokens` 如何设置。 +#### `compress.stripProtectedReasoning` +- **类型：** `boolean` +- **默认值：** `true` +- **状态：** ACTIVE +- **说明：** 闭轮思考剥离的总开关（#368）。受保护豁免消息（如 `compress`/`skill` 工具调用）以消息粒度被排除在压缩之外，其 `reasoning` 部分随每轮请求重复发送，形成永久不可压缩的上下文底座。启用时（默认），ACP 在请求时丢弃这些消息在**已关闭历史轮次**中（严格位于最后一条真实用户消息之前）的 `reasoning` 部分。当前活跃轮次永不受影响；不修改任何持久化状态。设为 `false` 可完全禁用。 + +#### `compress.stripProtectedReasoningThreshold` +- **类型：** `number` +- **默认值：** `0` +- **状态：** ACTIVE +- **说明：** 受保护豁免历史消息上 `reasoning` 总长度（字符）达到该阈值后才剥离。默认 `0` 表示无论大小都剥离 —— 单条消息的大小对前缀缓存只是噪声（失效会从第一条被改写的消息向后传播）；缓存稳定性由 `stripProtectedReasoningMinMessages` 控制。 + +#### `compress.stripProtectedReasoningProviders` +- **类型：** `string[]` +- **默认值：** `["anthropic", "gemini"]` +- **状态：** ACTIVE +- **说明：** 闭轮思考剥离的提供方白名单（对 provider id 大小写不敏感的子串匹配；`"*"` = 所有提供方；条目会做 trim）。**失败关闭（fail-closed）**：未知、未定义或不匹配的提供方一律不剥离；显式 `[]` 同样不剥离任何提供方。闭轮思考剥离仅在 Anthropic/Gemini 上验证过安全性；部分 GPT 系网关会拒绝历史思考块不完整的请求。 + +#### `compress.stripProtectedReasoningMinMessages` +- **类型：** `integer`（≥ 0） +- **默认值：** `100` +- **状态：** ACTIVE +- **说明：** 激活门：仅当请求携带的消息数达到该值时才运行闭轮思考剥离。短会话保持字节稳定的前缀（无缓存扰动）；被回收的底座只在长会话中才有意义。`0` 禁用该门。小数会被校验拒绝。 + --- ### `gc`（生成与清理） diff --git a/README.md b/README.md index d859840b..a36417da 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,13 @@ Each level overrides the previous, so project settings take priority over global // Preserve your messages during compression. // Warning: large copy-pasted prompts will never be compressed away "protectUserMessages": false, + // Closed-turn thinking stripping on protected-exempt messages (#368). + // "reasoning" parts of compress/skill tool calls in closed historical + // turns are dropped at request time; the active turn is never touched. + "stripProtectedReasoning": true, + "stripProtectedReasoningThreshold": 0, + "stripProtectedReasoningProviders": ["anthropic", "gemini"], + "stripProtectedReasoningMinMessages": 100 }, // Garbage collection — hardcoded 100% fallback only "gc": { diff --git a/README.zh-CN.md b/README.zh-CN.md index 54c38a6e..2e6a5d9d 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -334,6 +334,13 @@ ACP 使用自己的配置文件，按以下顺序搜索： // Preserve your messages during compression. // Warning: large copy-pasted prompts will never be compressed away "protectUserMessages": false, + // 受保护豁免消息的闭轮思考剥离（#368）。 + // 请求时丢弃已关闭历史轮次中 compress/skill 工具调用的 + // "reasoning" 部分；当前活跃轮次永不受影响。 + "stripProtectedReasoning": true, + "stripProtectedReasoningThreshold": 0, + "stripProtectedReasoningProviders": ["anthropic", "gemini"], + "stripProtectedReasoningMinMessages": 100 }, // 垃圾回收与批量清理 "gc": { diff --git a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md index 6ec0ad35..db926aae 100644 --- a/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md +++ b/devlog/2026-09-07_strip-protected-reasoning/WORKLOG.md @@ -118,3 +118,4 @@ After applying the provider/activation gates, a second dual-agent review (source - [x] Main #368 filed to `ranxianglei/billion-context` (**#651**) and `ranxianglei/billion-context-pi` (**#336**) (owner request). - [x] PR opened: **#370** (awaiting human merge). - [x] 2026-09-08 review session applied to PR #370: provider allowlist + activation gate + threshold 0 (owner: "直接修改pr"). +- [x] 2026-09-09 config docs: added the 4 `stripProtectedReasoning` keys to `CONFIGURATION.md`, `CONFIGURATION.zh-CN.md` (reference sections) and `README.md`, `README.zh-CN.md` (example config blocks), both languages (user: "配置文件文档没改 中英文的").