Skip to content

fix(#377): re-voice mid-stream acp_summary as user messages (user-carrier) - #428

Merged
ranxianglei merged 3 commits into
masterfrom
2026-09-01_user-carrier-summary-377
Sep 1, 2026
Merged

ranxianglei merged 3 commits into
masterfrom
2026-09-01_user-carrier-summary-377

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem

Strict OpenAI-compatible backends (SGLang, Qwen3-family "system-first" chat template) require exactly one system message at index 0 and reject a mid-conversation or second system message with 400 {"message":"System message must be at the beginning"} (#377).

acp-kernel renders each compressed block's summary as role:"system" anchored mid-conversation. In proxy mode (plain client) the compress tool call is ephemeral (server-side loop, never in the client's re-sent history) and preflight blocks have no tool call at all — so the acp_summary system message is the only carrier, and ≥2 active blocks produce ≥2 system messages → 400.

Fix

Replace hoistMidSystemMessages (which hoisted mid-stream system messages to the head as separate system messages) with systemToUser — it converts the mid-stream acp_summary system/developer messages to role:"user", leaving them at their anchor position.

This satisfies both constraints at once:

  • SGLang compatibility — only one system message remains (the head, injected by injectOpenaiSystem); summaries ride on user messages, which are allowed anywhere.
  • Prefix-cache stability — the head system message (the cache anchor) stays byte-stable across compress turns, so a new block no longer invalidates the whole-conversation prefix. (Hoisting/merging to the head would have broken this.)

In plugin/launcher mode the summary carrier is the compress tool call (in the agent's own re-sent history), so the kernel's acp_summary is stripped by stripKernelSummaries and systemToUser is a no-op there.

Why user, not a forged tool call: a forged compress tool call would be the "pure" carrier, but in proxy mode it requires fabricating an assistant tool_calls + user tool_result pair by id, declaring the tool in the request, and handling preflight blocks that have no authentic call — far more invasive than re-voicing a standalone note. A summary is a stand-in for the folded history; re-voicing it as a user turn (clearly marked [Compressed conversation section]) is the accepted trade-off.

Changes

  • src/util.ts: hoistMidSystemMessages → systemToUser
  • src/server.ts, src/loop/adapter-openai.ts: call sites + two-mode comments
  • README.md: new "Two compression modes" section (carriers, why they differ, why user not system/tool-call, coexistence determination)
  • AGENTS.md: Key Design Decisions item 6 (two modes, different carriers) + Before Making Changes item 4 (every wire-touching change must be reasoned in BOTH modes)
  • tests: new system-to-user.test.ts (7 tests), updated e2e-mid-system.test.ts (summaries must ride on user messages, not leak into system), deleted hoist-mid-system.test.ts

Verification

Supersedes the abandoned 2026-09-01_merge-system-messages-377 branch (merge-to-system approach, rejected because it breaks prefix-cache stability).

…rier)

Replace hoistMidSystemMessages (hoisted mid-stream system messages to the head
as separate system messages) with systemToUser (converts them to role "user",
leaving them at their anchor position).

Why: strict OpenAI-compatible backends (SGLang) require exactly ONE system
message at index 0 and reject a mid-conversation or second system message with
400 "System message must be at the beginning" (#377). The kernel renders each
compressed block's summary as role "system" anchored mid-conversation; in proxy
mode (plain client) the compress tool call is ephemeral and preflight blocks
have none, so the acp_summary is the only carrier. Re-voicing it as a "user"
message satisfies SGLang's one-system rule AND keeps the head system message
(the prefix-cache anchor) byte-stable across compress turns — a new block no
longer invalidates the whole-conversation prefix.

In plugin/launcher mode the summary carrier is the compress tool call (in the
agent's own re-sent history), so the kernel's acp_summary is stripped by
stripKernelSummaries and systemToUser is a no-op there.

- src/util.ts: hoistMidSystemMessages -> systemToUser
- src/server.ts, src/loop/adapter-openai.ts: call sites + two-mode comments
- README.md: new "Two compression modes" section (carriers, why they differ,
  why user not system/tool-call, coexistence determination)
- AGENTS.md: Key Design Decisions item 6 (two modes) + Before Making Changes
  item 4 (every wire-touching change must be reasoned in BOTH modes)
- tests: new system-to-user.test.ts (7 tests), updated e2e-mid-system.test.ts
  (summaries must ride on user messages, not leak into system), deleted
  hoist-mid-system.test.ts
@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-01_user-carrier-summary-377 (499f314)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-428

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr428.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

Reviewed PR #428 (branch 2026-09-01_user-carrier-summary-377, commit 4f7e603). The fix is correct, and the verification is actually in better shape than the issue body claims.

Verification (clean npm ci from the committed lockfile):

  • tests/system-to-user.test.ts — 7/7 pass
  • tests/e2e-mid-system.test.ts — pass (real startServer vs mock upstream; asserts the summaries ride on user messages and leak zero into system/developer)
  • npm run build — success
  • npm test — 892/892 pass
  • npm run typecheck — clean, exit 0

The two "pre-existing" problems the issue body calls out no longer reproduce: the lockfile pins acp-kernel@0.0.47 (integrity-locked), and that artifact now ships invalidReasons, so a fresh npm ci gives a fully green tree — the compress-reject-reasons #362 test failure and the 6 src/compress-tool.ts/src/stream.ts type errors are both gone. CI (npm ci) will see the same.

Code review — logic is sound:

  • Both call sites run systemToUser before injectOpenaiSystem (src/server.ts:1536→1547, src/loop/adapter-openai.ts:175→176), so the head system is re-injected after the re-voicing → exactly one system at index 0, byte-stable for the prefix cache.
  • In proxy mode openaiToCore hoists the client's leading system out first (src/server.ts:1511), so by the time systemToUser runs it only ever sees the mid-conversation acp_summary messages and converts them to user in place — never touching the head.
  • Plugin mode: stripKernelSummaries removes the acp_summary (the tool call is the carrier), so systemToUser is a no-op — matches the documented two-mode model.
  • No leftover hoistMidSystemMessages references; tests/hoist-mid-system.test.ts deleted; README "Two compression modes" + AGENTS.md items 6 & 4 document the carrier distinction and the "reason in BOTH modes" rule.

One follow-up (human decision): PR #379 (2026-08-31_merge-system-messages, "merge all system/developer into one leading message") is the superseded merge-to-head approach this PR replaces. It should be closed in favor of #428 — I've left it open since closing/merging is human-only.

LGTM — ready to merge (merge is human-only per AGENTS.md).

The nudge (a trailing user message prompting compression) was not gated by
!pluginMode, so in plugin mode the proxy could inject a redundant nudge even
though the agent owns compression (its own overflow self-heal). Gate all three
nudge injection sites (anthropic/openai/responses) by !pluginMode, matching the
injectTools gating, so plugin mode is fully clean: no tool injection, no nudge
injection, summary carrier = the agent's own compress tool call.
README "Two compression modes" table: add a row for proxy-injected tools +
nudge (plugin mode: none; proxy mode: the 4 context tools + nudge). AGENTS.md
item 6: "suppresses tool injection" -> "suppresses tool + nudge injection".
@ranxianglei

Copy link
Copy Markdown
Owner Author

确认是否有回归问题

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

已确认,无回归问题。我做了 master 基线对比 + 全量测试(clean npm ci,acp-kernel 0.0.47):

全量测试

  • master 基线:893/893 通过,0 失败
  • 本 PR 分支:892/892 通过,0 失败
  • 没有任何一个在 master 上通过的测试在 PR 上失败

−1 个测试是有意为之,不是漏掉回归

本 bug 的专属回归测试通过

  • tests/e2e-mid-system.test.ts 驱动真实 startServer 代理对 mock upstream,断言新 wire 形态:summary 落在 user 消息上、system/developer 零泄漏、index 0 恰好一个 system

无残留引用

  • 代码/文档里剩余的 "hoist" 全部指 kernel openaiToCore 的前缀 hoist(kernel 0.0.37,仍在用),与被删除的 hoistMidSystemMessages 函数无关

一个说明(非回归,是本沙箱的覆盖限制)

  • AGENTS.md 要求请求管线改动合并前跑真实客户端 E2E tests/e2e/e2e-codex.test.ts,但它需要 codex 二进制 + 本地 Responses upstream(默认 127.0.0.1:8199)。本沙箱两者都不可用(codex 未安装、upstream 不可达),且被 ACP_TEST_E2E=1 门控。合并前建议由维护者或 manual-dispatch CI 跑一遍——这是环境限制,不是回归信号。

结论:无回归。 行为变更(hoist-to-head → 原地改写为 user)是有意设计且覆盖完整;唯一在本沙箱没跑到的只有需要本地 upstream 的真实客户端 e2e。

@ranxianglei

Copy link
Copy Markdown
Owner Author

新方案发到原issue

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 汇总本轮在 #428 上完成的工作:

  1. 代码评审 — systemToUser(src/util.ts)逻辑正确:两个调用点(src/server.ts:1536→1547、src/loop/adapter-openai.ts:175→176)都在 injectOpenaiSystem 之前执行,head system 在改写后重新注入 → index 0 恰好一条 system、字节稳定利于 prefix-cache;proxy 模式下 openaiToCore 先把 client 的 leading system 提走,所以 systemToUser 只碰 mid-stream 的 acp_summary。plugin 模式 stripKernelSummaries 剥掉 acp_summary,systemToUser 是 no-op。

  2. 验证(clean npm ci,acp-kernel 0.0.47)— system-to-user.test.ts 7/7、e2e-mid-system.test.ts 通过、全量 npm test 892/892、typecheck clean、build 成功。

  3. 回归确认 — master 基线 893/893,PR 分支 892/892,0 失败;净 −1 是删除的旧 hoist-mid-system.test.ts 里一个退化用例(全 system 列表),已被新通用用例覆盖,非漏覆盖。唯一本沙箱跑不到的是需要本地 upstream 的真实客户端 e2e(e2e-codex:codex 未装 + 127.0.0.1:8199 不可达),建议合并前由维护者/CI 跑一遍。

  4. 已把新方案同步到原 issue [Bug] ACP: ≥2 compressed blocks render as multiple system messages → SGLang (vLLM-compatible) upstreams return 400 #377,说明它取代 PR fix: merge all system/developer messages into one leading message (SGLang #377) #379(merge-to-system,会破坏 prefix-cache 稳定性),并建议关闭 PR fix: merge all system/developer messages into one leading message (SGLang #377) #379。

结论:PR #428 LGTM,无回归,可合并(合并为人工操作)。

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.

1 participant