[joshwapohlmann]

Problem

A model that imitates the render tag can write its ENTIRE turn, tool call included, where the opening tag's attributes are still open, so the attribute value is never closed and no closing tag is ever written. The span escapes every strip pattern at once: an attribute class stops at a <, and a value opened with a quote never terminates. The filter dropped the opening and released the payload, so the client received a completed turn made of the turn's own markup with no tool call in it, and the agent idled until it was nudged by hand.

Recorded bytes, architect session 01a0a0cb-4a90-74db-8e19-814e80cd4e20, 2026-09-16T17:29:09, model deepseek-v4-flash, stopReason: "stop", no tool call in the record. Line 1 is the echo head; everything after it is the tool call the model meant to make, written as text:

<acp tokens="1" text="text</｜｜DSML｜｜ parameter>
<parameter name="i">Rebuilding to test the source, not the build</｜｜DSML｜｜ parameter>
</invoke>
</｜｜DSML｜｜ calls>"
</｜｜DSML｜｜ parameter>
</invoke>
</｜｜DSML｜｜ calls>

The same session shows the sibling shape in the proxy log from 2026-09-14, where the model wrote a stray > after the opening's value. Both shapes reach the client today; both have to end as an empty turn.

Cause

src/loop/tag-echo-filter.ts matches render tags by pattern. Two independent properties of this echo defeat every pattern at once:

• The attribute list classes are [^<>]{0,256}, so they cannot cross a <. Where the model puts its own markup inside the still-open attribute, the opening has no terminating > at all.
• LONE_OPEN needs the value's closing quote only implicitly, and a span whose value is never closed is indistinguishable from prose by pattern alone.

The streaming filter therefore dropped the head and forwarded the payload, and stripAcpTags (the batch path) did the same, which is how orphan parameter/invoke/calls closers reached the client.

Fix

The wrapped shape is recognised by form rather than by a pattern, in both the streaming filter and stripAcpTags:

• BROKEN_ATTRS (/x3c<name>x5cs[^<>]{0,512}(?=x3c<)/) sees the opening whose attribute list runs into a <. A properly terminated opening cannot match it: the class ends at the > and no < is reachable from there.
• A lone opening that terminates but carries an ODD number of quotes is the same shape (a value was opened and never closed); a genuine opening is always balanced, tokens="245" type="text".
• The span is then swallowed WHOLE, to the first loose close tag or to the end of the turn, instead of dropping the head and releasing the payload.
• The swallow budget is per shape. SWALLOW_CAP stays 80 for a plain opening because [bug] #361 residual, form 3: turn whose entire text is a render-tag echo strips to an EMPTY message — agent records a marker-only turn, stalls until manual nudge (v0.1.92, post-#365) #644 requires an over-long tail there to be released as content; IMITATION_SWALLOW_CAP (4096) applies to the attested wrapped span, and past it the text is discarded while the swallow continues, so no part of an imitation can leak.

tests/tag-echo.test.ts gains four tests: the whole-span drop for both recorded shapes, the loose-close boundary keeping prose written after it, per-split-position equivalence between the streaming filter and stripAcpTags, and a push-by-push swallow that asserts nothing reaches the client.

Evidence

• node --import tsx --test tests/tag-echo.test.ts tests/marker-echo.test.ts: 68 pass. A/B on the full suite (1661 tests) with the filter reverted: exactly the three new behaviour tests fail there and pass with the change; the remaining failures are environmental (launcher, mcp shell, codex transport, plugin manifest, pi extension) and identical on both sides.
• A mock-upstream prover drives one plugin-mode turn whose only content is the wrapped echo: two upstream requests, the second carrying the continuation nudge, and the client receives a readable tool call. The same prover against the bundle without this change: one request, no tool call, the client gets the orphan markup with finish_reason: stop.

Half of one failure

This fix makes the client's turn EMPTY, which is the shape PR #871's degenerate-turn retry re-asks for. Neither half makes the turn usable alone: releasing the payload leaves unparseable markup, and swallowing it without a retry leaves an empty turn. Both are in the same local build.

Limits

A wrapped span longer than 4096 characters keeps being swallowed but is discarded rather than forwarded, which is deliberate: the span is the imitation's payload, never content. A plain opening with balanced quotes keeps the previous behaviour exactly, including #644's release rule. The span ends at the first loose close tag, so genuine prose written after the imitation survives.

[joshwapohlmann]

Live verification in the reporter's own session, on the locally installed build of this change, after the proxy was restarted onto it at 18:04:48Z.

Same hour-by-hour count from that machine's bili.log for 2026-09-16:

hour	strips	retries	[degenerate-turn] warnings
15:00Z	156	0	110
16:00Z	468	0	169
17:00Z	1498	0	404
18:00Z	98	2	0

The strips kept happening at the same rate after the restart and none of them produced the orphan-markup stall this PR is about: the wrapped span now ends as an empty turn (18:05:28.016Z, retry at .144Z, recovered by the retry's own request at 18:05:32.477Z), and the balanced pure-echo shape does the same through the same retry (18:09:37.309Z, no warning).

[degenerate-turn] is the residual-stall signature: a turn reached the client with no visible text and no tool call. It went from 110, 169 and 404 per hour on the previous build to zero in the hour the new build took over, on the same pipe and with the same echo pressure.

A second live session was asked to stand in for the model and re-trigger the original stall deliberately; it recovered.

Tested against the installed bundle before this went upstream: the attribute-wrapped shape and the pure echo both leave an empty turn that the retry recovers, and the batch path (stripAcpTags) matches the streaming filter at every split position.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review complete — I checked out your branch (c7c3a10) and ran the full verification locally. Verdict: the code and tests are solid; one sequencing question before merge (see §3).

1. What I verified (all on your head, base is current — 0 commits behind master)

Check	Result
npm run typecheck	✅ pass
tests/tag-echo.test.ts + tests/marker-echo.test.ts	✅ 68/68 (matches your claim)
Full suite, your head	1668 tests: 1665 pass, 1 fail, 2 skip
Full suite, master (A/B control)	1664 tests: 1661 pass, 1 fail, 2 skip

The single failure is identical on both sides: #857 D: upward self-heal trusts usage baselines only (tests/issue857-image-baseline.test.ts:276, expects { 'deepseek-flash': 140000 }, gets undefined). It's a self-contained localhost test, unrelated to tag echo — i.e. pre-existing on master, not introduced by this PR. That also explains the red CI test runs (ubuntu-24/22, windows-24): fork-PR logs aren't retrievable via API from here (404), but the local A/B shows your branch adds exactly 4 tests, all green, and changes nothing else. Side note for the tracker: that #857-D failure looks like a real pre-existing bug worth its own look — reporting here per repo rules rather than opening anything.

Diff hygiene: 3 files (tag-echo-filter.ts +108/-5, tests/tag-echo.test.ts +61, CHANGELOG.md +2), every line on-topic, no version/lock churn. ✅

2. Design & test review

• Shape-based attestation (attr list runs into <, or odd quote count in the opening) + whole-span swallow to first loose close / end-of-turn, applied identically in stripAcpTags and the streaming filter — matches the two recorded bytes-for-bytes shapes.
• Per-shape budgets keep the [bug] #361 residual, form 3: turn whose entire text is a render-tag echo strips to an EMPTY message — agent records a marker-only turn, stalls until manual nudge (v0.1.92, post-#365) #644 release rule for plain openings while wrapped spans discard past the 4096 cap instead of leaking. Correct call: releasing would have shipped the orphan payload.
• Tests are strong: fixture self-checks that it still contains tool-call markup (drift guard), both recorded shapes, loose-close boundary keeps trailing prose, streaming-vs-batch equivalence at every split position, and push-by-push swallowing with drop accounting. Exactly the right invariants given [bug] #644 follow-up (post-#646, v0.1.97): typo'd render-tag echo (<acip …) bypasses the strip regex and passes through as a 43-char degenerate end_turn turn — agent stalls mid-orchestration until manual nudge #673's streaming/batch divergence history.
• CHANGELOG entry reads accurately (I re-read it line by line against the code — the [bug] #361 residual, form 3: turn whose entire text is a render-tag echo strips to an EMPTY message — agent records a marker-only turn, stalls until manual nudge (v0.1.92, post-#365) #644/Plugin passthrough leaves the client an empty turn when the model echoes a render tag #870 citations check out).

3. Sequencing question — the #871 dependency

Your body (and the changelog entry) correctly flag it: swallowing makes the client turn empty, and the retry half lives in PR #871 (issue #870), which is still open. Note master's existing retry (#821/PR #822, forwardedVisible in src/loop/core.ts) requires sawThinking, so it does not cover a pure-echo turn with no reasoning — #871 is genuinely required for the loop to close, not just nice-to-have. Without it, a swallowed turn arrives empty and stalls (with the #673 [degenerate-turn] warning logged). That said, #872 alone is still a strict improvement over today (client gets a clean empty turn instead of orphan parameter/invoke/calls markup it can't parse), so it's mergeable either way — but I'd suggest merging #871 first, or the pair together, so the fix ships whole.

中文摘要：已在你分支上完整验证（typecheck、68/68 专项测试、全量套件与 master 的 A/B 对照均通过，CI 红是 master 上就存在的 #857-D 旧失败，与本 PR 无关）；代码设计合理、测试覆盖到位，可以合并——唯一建议是先合并或同时合并配套的 #871（空轮重试），让整条闭环一起上线。

[joshwapohlmann]

Agreed on the sequencing, and your mechanism is the reason it matters rather than a preference: master's retry gate cannot close this on its own.

I checked it on master before answering — the loop's single retry condition in src/loop/core.ts requires sawThinking alongside !forwardedVisible, zero assistant text and zero calls. It fires for a turn that produced reasoning and nothing visible; a pure-echo turn produced no reasoning at all, so it sits outside the gate. That is exactly the shape #872 produces: the swallow leaves an empty turn, and #871 is the pipe that re-asks for it. So #871 is not the nice-to-have half of a pair; it is the closure. Merging #872 alone is a strict improvement over orphan parameter/invoke/calls markup, as you say, but it moves the failure from unparseable markup to an empty turn without recovering the turn.

Practically: merge #871 first, or both together. Both are the maintainer's buttons, and #872's body already records the dependency, so I am stating it here so the order is not left to chance. If it lands the other way, nothing breaks that is not already broken — a swallowed turn still stalls behind the [degenerate-turn] warning until #871 follows, which is the state this project has been in all day.

One thing worth adding from the deployment side, since it bears on how the two are judged together: on the machine where these were developed, both halves are already running, so the closed loop has live evidence rather than only unit tests. The echo-swallow from #872 and the retry from #871 have been in the installed bundle since 18:04:48Z, and in that hour the [degenerate-turn] count went from 404 to zero with two retries recovering turns that would otherwise have stalled. That is the pair behaving as one mechanism, which is the argument for merging them as one story.

On the #857-D failure: agreed, and your A/B is what settles it — pre-existing on master, byte-identical on both sides, unrelated to this diff. Reporting it here rather than opening anything is the right call for this project; when someone does take it, it should arrive as a pull request, per the filing rule in this repository's own history. I am not taking it now: it is master's bug, not this PR's, and it is not in my queue.

Thanks for the diff-level re-read of the CHANGELOG citations and for checking the streaming-versus-batch equivalence at every split position. The batch path diverging from the streaming one is the failure this filter has produced before, so pinning equivalence is the test I would have written last and should have written first.

[ranxianglei]

继续

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

Picking this back up where the session crash cut off my reply to your sequencing note — agreed on everything in it, and your live numbers (degenerate-turn warnings 404 → 0 in the 18:00Z hour, two retries recovering turns that would otherwise have stalled) are exactly the pair-level evidence that makes "merge them as one story" the right call rather than a preference.

Since then master moved, so I re-verified this branch against it before leaving the verdict standing:

Re-verification on current master (90ad359)

Check	Result
Cherry-pick of both commits onto 90ad359	✅ clean, zero conflicts
npm run typecheck	✅ pass
tests/tag-echo.test.ts + tests/marker-echo.test.ts	✅ 68/68
Full suite, rebased head	✅ 1674 tests: 1672 pass, 0 fail, 2 skip
npm run build	✅ clean (dist index.js 2.92 MB)

Two things landed on master that change the picture from my first review:

1. The [Question] 非白名单上游（如 DeepSeek）图片按 b64/4 估算，实测高估约 100×；且该估算被写回 lastInputTokens 并自愈成 confirmed 窗口，使 #496 逃生门永久失效 #857-D failure is fixed upstream. 323d8c1 (in PR fix: cross-platform body-dump paths via XDG state dir (#864) #865, merged) aligns the self-heal baseline test with the post-[Question] deepseek-flash 被静态兜底表解析为 128k，真实窗口 1M —— 是有意设置，还是兜底表过期？ #852 DeepSeek window table — that was the single failure on both sides of the original A/B, so the rebased head is now fully green with zero failures. A rebase of the branch onto current master should clear the red CI test jobs without any line of this diff changing.
2. preflight: raise summary output cap to 32k, clamp via models.dev #874 (summary output cap 32k / models.dev ceilings) also landed; no interaction with the filter, confirmed by the full-suite run above.

State of the pair

• fix(plugin): retry a turn whose only visible output was a stripped tag echo (#870) #871 is still open and still moving: the unclosed-markup gate landed as a008628, the Responses-pipe twin of that hole is documented in-thread as deliberately out of scope, and the owner's 01:30Z "ci失败" report was forwarded into that thread for handling. Nothing on this side blocks on anything new.
• fix(loop): swallow a render tag wrapped around the model's turn #872 (this branch): code, tests, and CHANGELOG unchanged since review; the only action item is the mechanical rebase onto 90ad359, which I've proven conflict-free.

Order stands as you framed it: #871 first, or both together. Once #871 lands and this branch is rebased, it is ready for the human merge button.

中文摘要：会话崩溃后继续处理——已在当前 master（90ad359，含 #865/#874）上重新验证本分支：cherry-pick 零冲突、typecheck/68 专项测试/全量套件（1672 通过、0 失败）/构建全部通过；原先 A/B 对照中唯一的 #857-D 旧失败已被 master 上的 323d8c1 修复，rebase 后 CI 应转绿；结论不变——先合或同时合并 #871，本分支 rebase 后即可由人工合并。

[ranxianglei]

继续