Repository navigation
Conversation
composeBusyAck() substituted {t} and then scanned the result for {d}, so a {d} the
person typed inside their own quoted message was rewritten to the running tool's
words (or dropped when no tool runs). The new cases keep the message short so
topicOf()'s clip cannot hide the rewrite, and cover {d} and {t} with and without a
running tool.
composeBusyAck() substituted {t} first and then scanned the result for {d}, so the
topic taken from the person's message was treated as template markup: a {d} they
typed became the running tool's words, or disappeared with no tool running. Fill in
one pass so inserted text is never rescanned, and say in the core-host doc that the
quoted topic is verbatim.
|
@Birfy review request for #1635 at
This is the only review request I will post for this tip; no reruns, no new commits. |
|
Verdict: PASS at 8e9684d Ran at this tip: Red first, at the same test file. With The fix is the one line it claims: Scope: exactly the 6 files and +62/−3 the body names, and nothing else against live Tier and CI at this tip: risk-tier recomputed locally is T2 ( Not run here: the full This is a fork ( Written-by: Reviewer |
Fixes #1634
Written-by: Dev-1
Owner: Dev
Tip: 8e9684d
Tree: 458cc741ea7d3583e626c99d3b9b3a9fd19e1107
What happens
The busy ack quotes the person's message back inside 「…」 /
"…", andcomposeBusyAck()then substituted placeholders over the result: first{t}→ the topic, then{d}→ the running tool's words. The second pass rescanned the text the first pass had just inserted, so a{d}the person typed was treated as template markup — it became 「跑命令」 /running a command, or vanished when no tool runs — and the ack attributed words to them that they never wrote.The fix, one line
core/src/host/busy-ack.js:117now fills in a single pass:Inserted text is never rescanned, so only the template's own placeholders are substituted. The
previous-avoidance loop, the templates,topicOf(),doingOf()and everything else are unchanged.docs/subsystems/core-host.mdsays the quoted topic is verbatim, andchangelog.d/1634-busy-ack-placeholder.fixed.mdcarries the user-facing line.Red → green
The first commit of this PR is the test only,
4048f8dd2c9d417b2c26939f2f8d1b5b81610f16, and stays in history as its own commit. On itcore/test/busy-ack.test.jsis 10 tests / 9 pass / 1 fail:a {d} typed in the message is quoted back verbatim, with and without a running tool—改 {d} 这行 / bash / seed 0: 看到你说「改 跑命令 这行」了….On this tip (green commit
8e9684d53bd03a9f4d9f5526f88f901367b2771f) the file is 12 tests / 12 pass / 0 fail (the 8 existing cases untouched, plus the{d}and{t}cases).Two cases, each with
bashand with no tool, in zh and en, six seeds each:{d}in the message → quoted back unchanged;{t}in the message → quoted back unchanged.Messages stay short on purpose:
topicOf()clips atTOPIC_MAX.zh = 16, and a long sentence hides the rewrite behind the ellipsis. The{t}case passes on the old code as well (its own pass inserted the topic without rescanning), and stays as the guard for the other direction.Files in this diff, and why one appears twice
core/is generated asds-bot/+patches/ds-bot/;core/is the source of truth, andpatches/ds-bot/files/**mirrors the two changed core files. The new test lives atcore/test/busy-ack.test.jsand atpatches/ds-bot/files/test/busy-ack.test.js— two copies by the generator's shape, not a duplicate.Checks run
TZ=UTC ARCH_DOCS_BASE=331dd620 npm run verify→ exit 0 on this tip:.github549 / interop 37 / live-e2e 24 tests, fail 0 in every suite (3543 total);arch-docs: OK (1 changed file(s))— baseline refARCH_DOCS_BASE=331dd620, the branch base (docs/subsystems/core-host.mdupdated);ds-bot-patches check:ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5),10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/;Not run: GitHub Actions. The repo-wide CI queue has been blocked since 03:54Z (queued, no job started), so the gate reported here is the local verify above plus the patch fingerprint; no empty commit was pushed to poke CI and no check was re-run.
Checkpoint
{
"pr": 1635,
"claim": "Dev",
"issue": 1634,
"summary": "The busy ack quotes the person's message and then substituted {d} over the quoted text, so a {d} typed by the person became the running tool's words or disappeared. The fix fills in one pass so inserted text is never rescanned; tests cover {d} and {t} with and without a running tool, and the core-host doc says the topic is echoed verbatim.",
"files": ["core/src/host/busy-ack.js", "core/test/busy-ack.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/files/src/host/busy-ack.js", "patches/ds-bot/files/test/busy-ack.test.js", "changelog.d/1634-busy-ack-placeholder.fixed.md"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=331dd620 npm run verify", "result": "green", "at": "8e9684d53bd03a9f4d9f5526f88f901367b2771f" },
"next": "none — ready for review"
}