fix: strip leaked bare ACP refs from completed assistant text (#431)

[ranxianglei]

Closes #431

Problem

With ACP enabled, assistant completions occasionally end with a bare ACP ref + garbage tail (e.g. m00057 <random multilingual text>). The fragment carries no tag, so the existing tag-only sanitizer in the experimental.text.complete hook passes it through; the suffix is persisted into OpenCode's DB and re-enters later model context and compression summaries.

Root cause layering (verified against v1.18.1 source):

1. Model side (intermittent trigger): at end of generation the model degenerates and echoes the ID-annotation pattern it saw on the last visible message. Refs are allocated monotonically during the next messages.transform, so the completing message does not yet have its ref — the echoed value is exactly what becomes its own ref (matches the issue evidence "the leaked reference matches the affected assistant message").
2. ACP side (fixed here): no defense against bare (tag-less) refs in text.complete.

Fix

Second sanitization pass in experimental.text.complete, exploiting a hard invariant: refs/block IDs are allocated strictly monotonically (messageIds.nextRef, prune.messages.nextBlockId are next-to-allocate). Any line-leading bare ref whose value is ≥ the next-to-allocate bound cannot be a legitimate citation of anything the model could have seen — it is by definition a future/hallucinated ID. Truncation runs from the first qualifying occurrence to end of string.

Conservative guards (legitimate prose survives):

• Line-leading only (^|\n + optional [ \t]) — embedded mentions like "see m00042" untouched
• Exact injected format only — lowercase m+4–5 digits or lowercase b+non-zero digits; uppercase product codes (M00057) untouched
• Existing refs (< bound) always kept; unknown ID space (registry miss / soft-cap eviction) → today's tag-only behavior, never a crash
• Tag stripping (paired/unpaired) unchanged

Note: the issue's suggested "look up the completing message's exact ref via input.messageID" is impossible at hook time — assignment happens on the next transform. The future-ref bound achieves the same containment with zero false positives by construction.

Changes

File	Change
lib/messages/utils.ts	New pure function stripLeakedTrailingRefs(text, { nextMessageRef, nextBlockRef })
lib/messages/index.ts	Barrel export
lib/hooks.ts	createTextCompleteHandler(registry, logger) — sync registry.get(sessionID) lookup, applies new strip, warn-log with removedChars + 200-char tail preview
index.ts	Pass registry + logger into factory (1 line)
tests/leaked-trailing-ref.test.ts	New regression suite (21 tests): issue repro shape, boundary equality, keeps existing/inline/uppercase/six-digit, bN variant, both-bounds-null no-op, first-match-cut, whole-output leak → "", CRLF, ws trim, handler-level combined strip / citation kept / registry-miss fallback / empty text
tests/hooks-permission.test.ts, tests/message-priority.test.ts	Existing handler tests updated for new factory signature
devlog/2026-09-17_bare-ref-leak/	REQ.md, WORKLOG.md, DESIGN.md

Verification

• npm run typecheck clean; npm run build clean
• Full suite: 1288 tests, 1286 pass, 2 fail — both failures are artifacts of my sandbox (read-only /tmp): tests/soft-block.test.ts import-time EACCES mkdir '/tmp/opencode-dcp-dangerous-*', and one toFile path assertion in tests/inactive-block-decompress.test.ts. Neither touches this change; expected green on CI.
• Bug-fail proof: old stripHallucinationsFromString(repro) returns input unchanged; new pipeline yields "Normal assistant progress message." for the [Bug]: Bare mNNNNN message refs leak into final assistant output and bypass sanitizer #431 repro shape.

Known limitation (documented in DESIGN.md §8)

An echo of an already-assigned ref (value < bound) is deliberately not stripped — that value could be a legitimate citation, and all reported evidence points at the unassigned self-ref case. If that variant appears in the wild, a stricter second pass can be added.

Housekeeping note (pre-existing, not introduced here)

Repo-wide npm run format:check currently fails on master files due to prettier version drift (package.json pins ^3.8.1; 3.9.x reformats differently). Kept this diff minimal instead of reformatting unrelated files; suggested as a separate housekeeping task.

[github-actions]

📦 Built Plugin Artifact

Branch: 2026-09-17_bare-ref-leak (35a9b63)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-432 --global

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

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-17_bare-ref-leak" --global

Option C — Download artifact

1. Download the artifact from the Actions run
2. Extract the tarball and install:

tar xzf opencode-acp-pr432.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist

3. Restart opencode to pick up changes.

---

This comment is automatically updated on each push.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 PR review pass 1 — code verified against source, local run reproduced

This is an agent-completed PR (ework-agent-pr marker), so I did not redo the work — I fetched the branch and audited it.

Branch state: f4096fc9, 1 commit, 10 files (+563/−9), based on current master (06efd39c). Diff is clean — every file maps directly to the #431 fix or its devlog; no stray changes, no version bump.

Design audit (verified against source, not just the description):

• Monotonicity invariant holds where it matters: allocateNextMessageRef (lib/message-ids.ts) sets nextRef = candidate + 1 after allocation (min 1, max 99999), so no live ref can be ≥ nextRef. Block IDs start at 1 — allocateBlockId (lib/compress/state.ts) resets corrupt/legacy counters to "allocate 1 next", so b0 can never exist; the regex's b([1-9]\d*) is therefore exact, not a coverage gap.
• registry.get() is genuinely synchronous (in-memory Map.get, lib/state/state.ts) — right tool for a hot per-completion hook vs getOrCreate's client fetch. Miss → tag-only fallback, no crash path.
• Pass ordering (tag strip → bare-ref strip) is actually load-bearing: removing a hallucinated paired tag can expose a line-leading bare ref, which the new pass then catches.
• Per-call regex rebuild avoids global-regex lastIndex state; empty-text early return; warn-log carries removedChars + 200-char tail preview (good for field diagnosis).
• Devlog complete: REQ/WORKLOG/DESIGN present, folder matches branch name, known limitation (already-assigned ref echoes not stripped) documented in DESIGN §8 with a concrete follow-up trigger.

Local verification (independent run): npm run typecheck clean; full suite 1288 tests / 1286 pass / 2 fail. Both failures are sandbox artifacts of this environment's read-only /tmp (tests/soft-block.test.ts import-time mkdir; one toFile path assertion in tests/inactive-block-decompress.test.ts) — exactly as the PR description predicted, and both green on CI.

CI at time of check: build ✓, test (22) ✓, test (24) ✓, pr-validation ✓, build-artifact ✓; e2e still in progress (hence mergeable_state: blocked).

One meta note: reviewing ACP from inside an ACP-enabled session means dcp-tag fixtures in test files get stripped from my tool-output views in transit — I confirmed via numeric probes that the fixtures are intact on disk (e.g. tests/regex-tag-leak.test.ts has 37/33 open/close tags on disk). No integrity issues found.

Continuing: second independent review pass + e2e confirmation, then a final verdict.

中文摘要：已拉取分支并对照源码核对了单调分配不变量、同步 registry 查询和剥离顺序，本地 typecheck 与测试复现结果与 PR 描述一致（2 个失败均为沙箱只读 /tmp 的环境问题），CI 除 e2e 运行中外全绿；代码可以进入合并等待。

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Final verdict: ✅ ready for merge — dual-agent review complete, full CI green on head b4ef2ded.

Follow-up since my first review pass:

1. Second independent review (separate agent, as required by the code/test review rules): verdict APPROVE, 4 LOW findings. Two were worth pinning with tests → fixed directly on the branch; two accepted as-is (no realistic failure path / cosmetic only).

2. Commits pushed to 2026-09-17_bare-ref-leak:

• b3c673c3 — two unit tests added to tests/leaked-trailing-ref.test.ts: pins the b0 exclusion (comment records the allocator invariant it relies on: block IDs start at 1, corrupt counters reset to 1, so b0 never exists) and covers the 4-digit mNNNN ref branch in both directions (stripped when ≥ bound, kept when < bound). Test-only change — zero lib/ behavior delta from review.
• b4ef2ded — WORKLOG correction: suite is now 1292 tests total (1288 baseline + 2 from ff24575f + 2 pins).
• Note: ff24575f (mixed below/future-bound sequence + non-integer bound guard) landed on the same file from a parallel session while I was pushing; rebased cleanly, no overlap. Post-rebase verification: 25/25 pass in that test file, typecheck clean.

3. CI on b4ef2ded: all 6 checks green — pr-validation, build, test (Node 22 + Node 24), e2e, build-artifact.

Findings accepted without change (for the record):

• input.sessionID dereference in the text-complete handler where the old handler ignored _input — the hook contract always supplies input, no realistic throw path.
• Cosmetic cast asymmetry between the nextMessageRef / nextBlockRef ternaries in lib/hooks.ts.

No blockers remain. Per project rules, merging is a human-only operation — please merge yourself when ready: #432

中文摘要：双 agent 独立复审通过（APPROVE，仅 4 个 LOW 级发现），已在 PR 分支直接补了 2 个测试钉住 b0 排除与 4 位 m 引用分支、修正 WORKLOG 测试计数；新 head b4ef2ded 全部 6 项 CI（含 e2e）均为绿，可以合并。