Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,49 @@ All source changes require review by **at least 2 separate agents** before merge
- `test:` test changes
- `docs:` documentation
- `release:` version bump

## 7. Review & Auto-Merge Discipline

> Distilled from a full-history audit of this repo + siblings ([billion-context#801](https://github.com/ranxianglei/billion-context/issues/801)). Full cited material: [AUTO-MERGE-GUARDRAILS.md](./AUTO-MERGE-GUARDRAILS.md). This is the CORE library: it OWNS the compression-block model and the wire-artifact format every adapter depends on, so its auto-merge surface is the narrowest of the three repos. Complements §6 Code Review (≥2 agents) by defining when a bugfix may skip the human round-trip.

### 7.1 Before you start
- **Duplicate screening first** (open AND closed). Link existing work, don't start parallel work.
- **One issue = one scope.** Never bundle unrelated changes or mass whitespace/reformatting.
- **Open a PR, never just push a branch.**

### 7.2 Review discipline
- **Rebase to CURRENT master** before claiming mergeable; after rebase re-run typecheck + test + build + format:check.
- **Pipeline nodes are shared hotspots** — if another open PR rewrites the same node/module, resolve by union of intent, then prove with tests.
- **Done = evidence**; tests stay pure (no I/O / network / mocks of kernel internals).
- **Deterministic tests** — no env/port luck.

### 7.3 Kernel-owned contracts (first-class; ALWAYS human-gated)
The invariants every adapter relies on. Changing ANY of these is cross-repo + prefix-cache-affecting by construction and MUST stay human (and follow cross-repo sequencing — kernel ships first):
1. **Message-id / ref immutability** (§2 principle 6) — no slot reclamation/recycling; widen `REF_WIDTH`. A re-issued number silently misattributes on decompress.
2. **Wire-artifact format** — the ACP compression tag wrapping `mNNNNN`, the `acp_summary` structure, block refs, and rendered-tag token counts being SNAPSHOTS frozen at first render.
3. **Single-owner content** (§2 principle 3) — `assignRefsNode` is the sole writer of message content.
4. **Lossless round-trip** (wire codec) — `deriveMessageId` stability, empty `reasoning_content` preserved (#289), raw control chars survive (#274), unparseable bodies pass through untransformed.
5. **Tool call/result atomicity** in prune/compress/fold (#286/#287/#293).
6. **Protected-tool filtering** (hard-excluded from compression — Bug 39) and soft recent-zone semantics (excluded but does not fail the range).
7. **The four load-bearing Prompts rules** (overriding one requires `acknowledgeRisk`).
8. **Pipeline node ordering** (the canonical 9-node sequence) and nudge invariants (growth-gating, tier-trigger gating #237/#162, baseline reset on compress).

Plus structural constraints that stay human: zero runtime deps (§2 principle 1 — no new dep without sign-off), platform-agnostic (§2 principle 5 — no host API / file I/O / network creep), and the state schema (explicit in/out; changing its shape is cross-repo).

### 7.4 Auto-merge gate (narrowest of the three repos)
A bugfix may **auto-merge** only if ALL hold:
1. Scoped to ONE pipeline node or standalone module; no change to node ordering or pipeline shape.
2. A pure regression test reproduces the bug and now passes.
3. Green on the rebased head (typecheck + test + build + format:check).
4. Touches NO §7.3 contract or structural constraint.
5. Pure `fix:` — no new capability surface.
6. Clean diff: no unrelated changes, no mass whitespace/reformat, no generated/lock churn.
7. References its issue via `Fixes #N`.

**Must stay human:** any §7.3 contract, any structural constraint, wire/message-shape changes, config/schema, state format, cross-repo dependencies, feat/refactor/architecture, security, or any default-value change (a product decision). Because a contract change is cross-repo by construction, it ALSO follows the manual cross-repo rule: release acp-kernel first, verify `npm view acp-kernel version`, then bump the adapters.

### 7.5 Reviewer focus — the "重灾区"
Of merged PRs that drew review, ≈25% needed a 2nd+ round. Dominant drivers:
1. **Stale-base / concurrent-file churn** on shared pipeline nodes.
2. **Incomplete first pass** — the repro passes but round-trip fidelity, tool-pair atomicity, or a doc-vs-default drift is missed.
Also check explicitly: does the fix preserve lossless round-trip and tool-pair atomicity? Do docs still match the actual code defaults (#163)?
83 changes: 83 additions & 0 deletions AUTO-MERGE-GUARDRAILS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# acp-kernel — Auto-Merge Guardrails & Review Discipline

> Evidence base for §7 of this repo's `AGENTS.md`. Derived from a full-history audit of this
> repo plus its siblings (`billion-context`, `billion-context-pi`), filed under
> [billion-context#801](https://github.com/ranxianglei/billion-context/issues/801).
> **This is the CORE library** — it OWNS the compression-block model and the wire-artifact format
> every adapter depends on, so its auto-merge surface is deliberately the **narrowest** of the
> three repos.
> **Positioning:** the authoritative rule / auto-merge-gate / reviewer-checklist text lives in
> [`AGENTS.md` §7](./AGENTS.md#7-review--auto-merge-discipline) (loaded every session — the single
> source of truth). This file is **evidence-only**: measured baseline + the incident that proves
> each invariant load-bearing + blind-spot analysis. Rules/gate/checklist are NOT restated here,
> to avoid drift. Cross-repo changes stay manual/human.

## 1. Baseline facts (measured from this repo's history)

- 295 tracked items (220 PR / 75 issues); **175 merged, 0 closed-unmerged, 21 open**; 16 carry
the `ework-agent-pr` marker. Authorship cannot separate AI from human (owner PAT) — classify by
the `[bot] 🏷` comment prefix + PR marker instead.
- **Second-round ("重灾区") rate: of 20 merged PRs that drew human review, 5 (≈25%) needed ≥2
human review touches** (billion-context ≈27%, billion-context-pi ≈42%).
- Zero runtime deps + pure pipeline = small per-file blast radius but **high fan-out**: every
adapter links the kernel inline, so any contract change is cross-repo by construction.

## 2. Rules humans actually enforce (evidence)

> Rule text lives in `AGENTS.md` §7.1–§7.3; below is *why* each exists.

### 2.A Already codified in AGENTS.md
§2 Key Design Principles (esp. #3 single-owner content, #6 message-id immutability), §4 Git
Safety, §5 Release Workflow, §6 Code Review ("at least 2 separate agents").

### 2.B Implicit rules observed in review threads
- **Never split a tool call from its result** in prune/compress/fold
([#286](https://github.com/ranxianglei/acp-kernel/issues/286) /
[#287](https://github.com/ranxianglei/acp-kernel/pull/287) /
[#293](https://github.com/ranxianglei/acp-kernel/pull/293)) — a fold that separates them corrupts
the transcript the model re-sends.
- **Lossless round-trip is a guarantee, not best-effort.** Empty `reasoning_content` survives the
core round-trip ([#289](https://github.com/ranxianglei/acp-kernel/issues/289)); raw control chars
in compound JSON don't damage it ([#274](https://github.com/ranxianglei/acp-kernel/pull/274)); a
body `detectWireFormat` cannot parse MUST pass through untransformed.
- **Docs must match code defaults.** [#163](https://github.com/ranxianglei/acp-kernel/issues/163)
`NudgeConfig` docstring drifted from the actual defaults — nudge thresholds are load-bearing, so
a lying docstring misleads every adapter author.
- **Block state must be stable across turns** (keep host-expanded blocks deactivated
[#181](https://github.com/ranxianglei/acp-kernel/pull/181)); tier-distillation nudges are
count-triggered but gated to the nudge band ([#237](https://github.com/ranxianglei/acp-kernel/issues/237) /
[#162](https://github.com/ranxianglei/acp-kernel/pull/162)); the growth baseline resets on a
successful compress (no feedback-loop re-fire).
- **Capacity pressure resolves by widening `REF_WIDTH`, never recycling** (PR #176 ref-reclamation
was reverted in #191).

## 3. What AI cannot reliably self-judge (blind spots)

### 3.1 Kernel-owned contracts — the incident proving each is load-bearing
> Definitions live in `AGENTS.md` §7.3 (always human-gated). Below is only *why* each is
> non-negotiable:
1. **Message-id / ref immutability** — a re-issued number silently misattributes on decompress
(**wrong content, not an error**); #176 was reverted for exactly this.
2. **Wire-artifact format** (tag / `acp_summary` / token-snapshot freezing) — changing shape breaks
every adapter plus the prefix cache at once.
3. **Single-owner content** — a second writer of message content breaks determinism.
4. **Lossless round-trip** — see #289 (empty `reasoning_content`) and #274 (control chars).
5. **Tool call/result atomicity** — see #286/#287/#293.
6. **Protected-tool filtering** (Bug 39) — dropping protected calls out of compression changes what
the model sees.
7. **Load-bearing Prompts rules** — overriding one silently degrades summary quality across sessions.
8. **Pipeline node ordering + nudge invariants** — reordering nodes or un-gating tier triggers
reintroduces feedback-loop over-compression.

### 3.2 Structural constraints that stay human
Zero runtime deps (§2 principle 1 — a new dep lands inside every adapter's inline bundle),
platform-agnostic (§2 principle 5 — no host API / file I/O / network creep into the core), and the
state schema (explicit in/out; changing its shape is cross-repo).

## Appendix
- **Gate + must-stay-human list + reviewer checklist:** see `AGENTS.md` §7.4 / §7.5 (single source
of truth). Because a contract change is cross-repo by construction, it also follows the manual
cross-repo rule: release acp-kernel first, verify `npm view acp-kernel version`, then bump adapters.
- **Release ordering:** acp-kernel ships FIRST in the chain (adapters pin exact versions and bundle
inline). Sibling consumption duties live in `billion-context` / `billion-context-pi` `AGENTS.md §7`.
- **Owner decision (#801):** rules merged into `AGENTS.md §7`; cross-repo stays manual for now.
Loading