Skip to content

feat: conditionally protect acp_rule pairs by stream-derived rule liveness - #378

Open
ranxianglei wants to merge 2 commits into
masterfrom
2026-09-24_rule-pair-protection
Open

ranxianglei wants to merge 2 commits into
masterfrom
2026-09-24_rule-pair-protection

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #377

What changed

acp_rule moves out of ALWAYS_PROTECTED_TOOLS into a new conditional protection, derived from the visible message stream itself:

  • collectRulePairProtection(messages) walks acp_rule tool-results in order. Result text forms a closed partition with host executors (billion-context executeRule is the reference implementation): Recorded ruleN: … adds the id to a live set, Removed ruleN: … removes it, Cleared N rule(s). clears it.
  • A record pair (call + result, paired by toolCallId; results without a projected toolName pair via the call-side name) is protected iff its id survives the walk. Kernel contract: ids are never re-issued, so final-set membership equals per-event liveness.
  • Every other acp_rule exchange — removals, clears, listings, failures — becomes ordinary foldable content. They are event traces, not persistent reminders; pinning them forever would recreate the accumulation problem this feature exists to fix.
  • Fail-closed: a result starting Recorded with an unparseable id keeps its pair protected (format drift must never silently fold a live rule).

Wired at all five protection consumers: buildCompressibleRanges (fold gate), assignRefsNode (BLOCKED refs, outside the config gate like media), filterProtectedToolMessages, CCR storeLargeResults skip-store check, and absorb (isAbsorbCandidate + applyAbsorb refusal). Exported from index for hosts/tests.

Why stream-derived and not a state.rules lookup

In plugin mode the agent owns compression: folding runs in the agent's own kernel copy whose state has no rules table (rule records are written server-side by the host executor). A state lookup would misjudge live rules as foldable there. The message stream is identical in both modes, so the derivation is mode-agnostic with zero cross-process sync.

Behavior-change disclosure (owner-approved)

  • Old: every acp_rule pair hard-protected forever (name-based).
  • New: record pairs protected while their rule is live per the stream; once removed/cleared they become foldable at the next natural fold. Delete/clear/list/failure pairs were also permanently protected before and are now ordinary content.
  • No-op safe: without any Removed/Cleared events in the history, every record pair stays protected — behaviorally identical to today. This release alone changes nothing until hosts actually delete rules (billion-context deletes ship in feat: acp_rule delete/clear — wire kernel removeRule/clearRules (#1177) billion-context#1178 / issue #1177).
  • Cache-safe (the owner's condition): zero byte change at deletion time — request bytes only change when a natural fold later absorbs a dead pair, which rewrites history anyway. With CCR v2 (billion-context#1179) such folded pairs additionally become retrievable via acp_retrieve.
  • The §7.3 protected-tool filtering contract is intentionally modified for acp_rule only; compress stays unconditionally protected (it carries block refs). This was explicitly approved by the owner in the #1177 thread: 「内核你可以搞,只要是不破坏缓存就可以搞」.

Tests

New tests/rule-pair-protection.test.ts (11 tests): unit coverage of liveness walk (live / removed / cleared / re-record-after-remove / malformed fail-closed / listings+errors inert / toolName pairing variants / empty), plus integration through buildCompressibleRanges proving the gate (dead pair compressible, live pair excluded) and no-op safety (record-only history fully protected). Updated the one stale assertion in tests/rules.test.ts.

Pre-flight on rebased head (master 312eb11): typecheck clean · full suite 1040/1040 pass · build clean.

…eness

acp_rule moves out of ALWAYS_PROTECTED_TOOLS into collectRulePairProtection: a record pair stays protected iff its id survives an ordered walk of acp_rule tool-results (Recorded adds, Removed deletes, Cleared clears). All other acp_rule exchanges become ordinary foldable content. Fail-closed for malformed 'Recorded ' results. No-op safe without removal/clear events; zero byte change until a natural fold (billion-context#1177, owner-approved semantic change).
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-24_rule-pair-protection (2f1e50a)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-378

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr378.tgz
npm install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review: mergeable — direction sound, no regressions found, diff clean. I pushed one test-only commit while reviewing (below). Merge is yours: #378

① Original issue (#377) — direction check

Verified the claimed root cause: pre-change, acp_rule sat in ALWAYS_PROTECTED_TOOLS, so every historical pair was pinned by tool name forever — deleted rules' pairs could never fold again. That is exactly the accumulation problem the feature exists to fix, and stream-derived liveness is the right layer: the message stream is identical in plugin mode and server mode, so no state sync is needed. Owner approval for the contract modification is on record in the #1177 thread. Direction: pass.

② Regression inventory (existing behaviors touched)

  • ALWAYS_PROTECTED_TOOLS — acp_rule removed, compress kept unconditionally (it carries block refs). Intentional, disclosed, approved. ✓
  • buildCompressibleRanges — acp_rule pairs now conditionally excluded. Record-only history (no Removed/Cleared events) yields byte-identical range decisions to today; the no-op-safety test pins exactly this. ✓
  • assignRefsNode BLOCKED refs — rule-protected halves get BLOCKED outside the config gate, consistent with how media payloads are handled. ✓
  • filterProtectedToolMessages / CCR storeLargeResults / absorb (isAbsorbCandidate + applyAbsorb) / crush — same single predicate (collectRulePairProtection + isMessageRuleProtected) at all five sites; grep confirms there is no sixth consumer. Single source of truth, no divergent semantics. ✓
  • Fail-closed — a Recorded result with an unparseable id keeps its pair protected; format drift can never silently fold a live rule. ✓
  • id-immutability reliance — final-set membership equals per-event liveness because ids are never re-issued (§2 principle 6). Correct dependency, correctly noted. ✓
  • Cache safety — protection only influences range selection; nothing rewrites live bytes at deletion time. Request bytes first change when a natural fold absorbs a dead pair, which rewrites history anyway. Matches the disclosure. ✓

③ Code

Two-pass walk (liveness, then live-record collection) is O(n) over results; pairing via the call-side toolName handles realistic projections where results carry toolCallId but no name; regexes are anchored; no new deps; index exports are additive. Two notes (non-blocking):

  • Limitation: if CCR/crush ever rewrote a dead removal/clear event's text, the walk would lose the event and the old record pair would stay protected — over-protection (fail-safe), and unreachable at defaults: absorb.minToolTokens is 4000 while a maxRuleChars=300 result is ~75 tokens, and both features are off by default. Worth remembering if a host enables CCR with very large custom rules.
  • Wire contract: Recorded is now coupled to the billion-context executor's output format. A future executor rename away from the Recorded prefix would under-protect (pair becomes foldable) — mitigated by the fact that live rule content is re-injected in the system prompt every turn regardless.

④ Tests

The 11 shipped tests have teeth: exact protected-range assertions (m00002–m00003, everything else compressible incl. the dead pair and delete trace), no tautologies. Gap I closed directly: the CCR skip-store guard and the applyAbsorb refusal were wired but unpinned — a regression there would let a live record result get placeholderized (breaking liveness derivation itself) or absorbed. Added 2 tests in commit 2f1e50a (test(rule-protection): pin CCR skip-store and absorb refusal guards): live result stays inline while the dead pair is stored; live target refused, dead target proceeds. Full suite locally: 1042/1042 pass, typecheck clean, file prettier-clean. (Local format:check repo-wide noise is pre-existing prettier-version drift — ^3.3.0 resolves to 3.9.6 and master fails identically; CI does not run format:check.)

⑤ Diff cleanliness

Single concern across 11 files (+390/−21 before my test commit), no unrelated churn; doc comments updated where the codebase actually made claims (types.ts, rules.ts, protected.ts). DESIGN.md et al. make no acp_rule-protection claims, so no doc drift.

Post-merge sequencing (§7.4)

This modifies a §7.3 contract (protected-tool filtering), so it ships by the manual cross-repo path: release acp-kernel first, verify npm view acp-kernel version, then bump adapters — billion-context deletes land in ranxianglei/billion-context#1178. Until hosts delete anything, behavior is identical to today.

CI was green on 1bc8aa7; re-running on 2f1e50a (test-only). I will not merge — that's yours once CI is green.

一句话中文摘要:审核通过——流派生规则活性保护方向正确、五处消费点接线一致、缓存安全且无回归;我直接补了 CCR 跳过存储与 absorb 拒绝两个缺失的守护测试(commit 2f1e50a,全套 1042/1042 过),可以合并。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 CI on new head 2f1e50a is fully green: test (22), test (24), build-artifact, pr-validation — all success. The PR is ready whenever you are: #378

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 审查完成,结论:可以合并(人工操作)。完整审查记录见原 issue 线程: #377 — 方向关/回归清点/代码/测试有效性/diff 整洁度五项全过;本地前置验证 typecheck · 1042/1042 · build 全绿,CI 绿;format:check 的 118 文件失败经核实为 master 既有状态(非本 PR 引入)。触及内核契约 #6,按 §7.4 须人工合并。

@ranxianglei

Copy link
Copy Markdown
Owner Author

暂时搁置(维护者决定)。

理由:按消息流文本协议(Recorded/Removed/Cleared)推导存活状态是补丁式做法——rule 的增删改查需要一套综合设计先行:

  • 生命周期所有权:存活规则的 source of truth 应该是谁——宿主执行器状态、内核会话状态(类似 CCR envelope),还是流推导?内核嗅探宿主输出字符串是脆弱耦合(格式漂移即失效)。
  • 完整 CRUD 语义:record 重复提交是更新还是复制?规则跨会话吗?与 native compaction / ref 重置如何交互?
  • 保护语义只是这个设计的一个下游结论,不该先于设计落地。

先出设计(issue 或 RFC),再回来重做本 PR。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 收到,已按维护者决定执行搁置:

  1. 设计 issue 已立:acp-kernel#390 [RFC] acp_rule lifecycle & CRUD design — 覆盖搁置理由中的三个问题(生命周期所有权 / 完整 CRUD 语义 / 保护语义作为下游结论),附 A(流推导)/B(内核会话表)/C(宿主声明 typed state)方案对比。我的建议:C 为主机制(typed snapshot/diff 进 CompressionState,事实源保持在宿主执行器,内核不再嗅探结果字符串),叠加 B 式乐观本地更新避免压缩决策等往返,A 降级为默认关闭的 fallback 或移除;另列了两个待你拍板的语义点(重复 record = 幂等更新?rule id 是否沿用永不复用纪律?)。状态已标「owner 拍板前不实现」。
  2. PR feat: conditionally protect acp_rule pairs by stream-derived rule liveness #378 保留原样:branch 2026-09-24_rule-pair-protection(head 2f1e50a,含 13 个测试的完整参考实现)不删不关,供重做时对照。一个说明:我尝试过把 PR 转 draft 以防误合并,但本环境的 GitHub API 代理未暴露 draft 字段(REST 静默忽略、GraphQL 报 argumentNotAccepted),无法代操作——如需,请在 web UI 一键转 draft。
  3. Protection of acp_rule invocation pairs should be conditioned on rule liveness: historical pairs of deleted rules should be collapsible. | acp_rule 调用对保护应随规则存活性条件化:已删除规则的历史 pair 应可折叠 #377 保持 open,已加评论指向 [RFC] acp_rule lifecycle & CRUD design: source of truth, full semantics, protection as downstream conclusion #390,原始问题不因搁置丢失。

重做时的起点建议:以 #390 的方案拍板为准,PR #378 中可复用的部分主要是五处消费点的接线骨架与测试结构,liveness walk 本体预计整体替换。

一句话中文摘要:已按决定搁置——立了 RFC #390(建议宿主声明 typed state 为主、内核乐观镜像为辅),PR 代码保留为重做参考,draft 切换受环境 API 限制需人工操作。

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

Labels

None yet

Projects

None yet

1 participant