Skip to content

feat(rules): acp_rule persistent rules — state field, helpers, hard protection - #283

Closed
ranxianglei wants to merge 2 commits into
masterfrom
2026-09-13_acp-rule-tool
Closed

ranxianglei wants to merge 2 commits into
masterfrom
2026-09-13_acp-rule-tool

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #282 (跨仓功能 billion-context-pi#433 的内核侧实现)

变更

  • src/types.ts: RuleRecord {id, text} + CompressionState.rules?(可选字段,旧状态向后兼容)
  • src/rules.ts(新): RULE_TOOL_NAMEDEFAULT_RULE_LIMITS {maxRules:50, maxRuleChars:300}listRulesallocateRuleIdaddRule(非空/超长/去重/条数上限校验)、removeRuleclearRulesformatRulesForPrompt(系统提示词注入渲染)、RULES_USAGE_PROMPT(模型使用说明:用户反复强调的教训/要求记住的行为/自己撞到的大坑,简短原则性记录)
  • src/protected.ts: ALWAYS_PROTECTED_TOOLS 增加 "acp_rule" —— 调用+结果默认永久受保护,压缩范围硬排除(#433 第 1 点)
  • src/state.ts: createInitialState 初始化 rules: []
  • src/index.ts: barrel 导出
  • tests/rules.test.ts(新): 10 个测试,含压缩排除保护集成测试(仿 protected-content.test.ts)

设计说明

  • 功能开关放适配器侧(pi acp.json / billion-context 三级配置,默认关闭);内核不加 Config 字段,保持正交(#433 第 4 点「普通工具」)
  • 保护无条件生效:功能关闭时不存在 acp_rule 消息,无副作用

验证

  • npm run typecheck
  • npm test:748/748 通过

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-09-13_acp-rule-tool (2bf84ee)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-283

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr283.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

PR #283 reviewed — the acp_rule contribution is clean and green.

Why the GitHub "Files changed" view looks bigger than the description (stacking, not scope creep):
Commit 55201e7 — the actual acp_rule work — touches exactly the 6 files listed in your description:

  • src/rules.ts (+106, new), src/types.ts (+12), src/protected.ts (+8/−2), src/state.ts (+1), src/index.ts (+12), tests/rules.test.ts (+167, new)

It does not touch package.json, package-lock.json, or src/report.ts. Those show up in the diff-vs-master only because this branch is cut from the v0.0.69 release head, which rides on two still-unmerged base PRs:

Confirmed neither is in master yet: master head f0de5f0 is still v0.0.68, and neither 92f37e0 (surface-meta) nor 6f108b4 (release) is an ancestor of origin/master.

Verification (ran locally on head 55201e7):

  • npm run typecheck — clean
  • npm test748/748 pass, 0 fail (including the acp_rule call+result compression-exclusion integration test)
  • npm run build — success

Code quality: solid. Immutable updates, legacy-state tolerance (state.rules ?? [] throughout), monotonic non-reusing ids (allocateRuleId = max+1, consistent with the never-reuse-ids principle), frozen DEFAULT_RULE_LIMITS, discriminated-union AddRuleResult, no as any / no @ts-ignore. Unconditional protection via ALWAYS_PROTECTED_TOOLS += "acp_rule" matches your design note (feature-off ⇒ no acp_rule messages exist ⇒ no side effect).

Recommendation — land the base first, no rebase:
Merging order so this PR's diff-vs-master collapses to just the acp_rule commit on its own:

  1. feat(report): host-declared surface meta in status reports #280 (surface meta)
  2. release v0.0.69 #279 (release v0.0.69)
  3. feat(rules): acp_rule persistent rules — state field, helpers, hard protection #283 (this one)

Once #280 + #279 are in master, GitHub will show #283 as acp_rule-only with zero extra work. Do not rebase #283 onto master — that would strip the release stacking and is the wrong move for a feature slated to ride on top of v0.0.69.

Minor, non-blocking: addRule dedup is case-sensitive exact-match after trim, so "Remember X" and "remember x" would coexist. Fine unless you want case-insensitive de-duplication.

I can't merge PRs — agent rule forbids it. Please merge yourself, in the order above.

中文摘要:已核对——本 PR 的 acp_rule 提交(55201e7)只改了描述里那 6 个文件,typecheck / 测试 748 全过 / 构建三项全绿;diff 里多出的版本号与 ACTIVE SURFACE 改动来自叠在下方、尚未合并的两个基座 PR(#280#279),并非跑题混入。建议按 #280#279#283 的顺序合并(基座落地后本 PR 相对 master 的 diff 会自动收敛为纯 acp_rule),无需 rebase;合并操作需人工执行。

…g.rules

Per owner feedback on billion-context-pi#433: acp_rule stays an ordinary
tool with a single optional 'rule' argument — value records a rule,
omitted lists them. No remove/clear in the tool surface, no system prompt
injection: usage guidance lives only in the tool description. Rules are
hard-protected from compression by default (call/result pairs stay in
context as the durable record).

- rules.ts: drop RULES_USAGE_PROMPT/formatRulesForPrompt; add RULE_TOOL,
  RULE_TOOL_OPENAI, RULE_TOOL_RESPONSES schemas, rulesEnabled(),
  resolveRuleLimits(); rename formatRulesForPrompt -> formatRulesList
- types.ts: RuleFeatureConfig + Config.rules (absent/disabled = off)
- config.ts: validateConfig checks for rules.maxRules/maxRuleChars
@ranxianglei

Copy link
Copy Markdown
Owner Author

This PR is superseded by #332 (branch 2026-09-20_acp-rule-reconcile), which reconciles this implementation with #284 against the only concrete consumer code that exists today (billion-context#750, port of billion-context-pi#433).

What survived from this PR into #332: the Config.rules limits layer (resolveRuleLimits(config) merging {maxRules, maxRuleChars} over frozen defaults {50, 300}), validateConfig range checks, the optional Config.rules field.

What #332 took from #284 instead: monotonic nextRuleId with never-reuse semantics + hand-crafted-state guard, persistence carry through state-merge/sync/cloneState, state-mutating helpers returning {ok:true;rule}|{ok:false;error} (matches #750's executeRule, which discards returned state), plus formatRulesList for tool-result rendering.

Recommend closing in favor of #332. Pre-flight there: typecheck PASS · 861/861 tests · build PASS.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

Verified every claim in the supersession note against branch 2026-09-20_acp-rule-reconcile @ c81aee7, then closed this PR in favor of #332 (closed, not merged).

What I checked

  1. Diff hygienec81aee7 sits directly on current master (post-v0.0.79), so unlike feat(rules): acp_rule persistent rules — state field, helpers, hard protection #283 there is no base stacking: diff-vs-master is exactly 10 files, all rule-scoped (src/rules.ts new +145, tests/rules.test.ts new +212, plus small carries in types/config/protected/state/sync/compress/state-merge/index). No version bump, no unrelated churn.
  2. Attribution claims — all accurate:
    • From feat(rules): acp_rule persistent rules — state field, helpers, hard protection #283: resolveRuleLimits(config) merging {maxRules, maxRuleChars} over frozen DEFAULT_RULE_LIMITS {50, 300} (src/rules.ts:24-27,44-49); validateConfig range checks rejecting < 1 (src/config.ts); optional Config.rules?: RulesConfig field.
    • From feat(rules): acp_rule persistent rule records with hard compression protection #284: monotonic nextRuleId never-reuse with hand-crafted-state backfill (allocateRuleId = max(state.nextRuleId ?? 1, highestRuleNumber(state) + 1), src/rules.ts:66-68; counter only ever increments in addRule, remove/clear don't touch it); persistence carry through cloneState (src/compress.ts:1565), syncBlocks (src/sync.ts:32), mergeCompressionState (src/persist/state-merge.ts:20) with fresh-state fallbacks; in-place-mutating helpers returning {ok:true;rule}|{ok:false;error}; formatRulesList.
    • Hard protection: ALWAYS_PROTECTED_TOOLS = ["compress", "acp_rule"] (src/protected.ts).
  3. Pre-flight re-run locally on c81aee7 (real Node v22): npm run typecheck PASS · npm test 861/861 pass, 0 fail · npm run build PASS. Matches your reported numbers exactly. CI also already green there — artifact comment posted for c81aee7, acp-kernel@pr-332 tag published.
  4. Consumer alignment (the load-bearing claim) — pulled billion-context#750 head (aefe017, branch 2026-09-13_acp-rule-tool) and read src/rules-feature.ts directly: it imports exactly addRule, formatRulesList, listRules, resolveRuleLimits from "acp-kernel" (lines 2–7) and calls addRule(state, rule, resolveRuleLimits(ctx.config)) discarding any returned state (line 54) — which is precisely why feat(rules): acp_rule persistent rules — reconciled API (state, limits, hard protection) #332's in-place mutation choice is correct and an immutable-only helper would have silently dropped rules. All four imported names are exported from feat(rules): acp_rule persistent rules — reconciled API (state, limits, hard protection) #332's barrel; the port compiles as-is once the kernel pin bumps.

Result: #283 closed unmerged, superseded by #332. Note #284 is still open and needs the same close treatment when you get to it.

中文摘要:已逐条核实 #332(c81aee7)——diff 干净(10 个文件全在 rules 范围内、无版本混改)、"取自 #283 / 取自 #284" 的归属描述全部属实、本地重跑 typecheck/861 测试/构建三项全绿、且直接拉取 billion-context#750 源码确认其导入与调用形态与 #332 API 完全吻合;已按建议将本 PR 关闭(未合并),以 #332 为准。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

acp_rule 持久规则:状态字段、辅助函数与硬保护(跨仓功能内核侧)

1 participant