From 9dc2ec80d0a6e72a6633df2573cba8c69bf91602 Mon Sep 17 00:00:00 2001 From: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Date: Tue, 25 Aug 2026 18:27:31 -0700 Subject: [PATCH] fix(desktop): keep escape-form substitutions conservative across shells Codex found a hole in the previous commit. Backslash-escaping earned literal credit -- `--content "literal \$MESSAGE"` was displayed as sent text -- on the assumption that a backslash escape is portable. It is not, and `BUZZ_SHELL` explicitly supports the shells where it is not (`buzz-dev-mcp/src/shell.rs` dispatches `-Command` for powershell/pwsh and `/C` for cmd). Verified by running PowerShell 7 rather than reasoning about it: 'Fixed `labelForStatus` in the rail' -> Fixed `labelForStatus` in the rail 'Costs $5 per run, not $MESSAGE' -> Costs $5 per run, not $MESSAGE "literal \$MESSAGE" -> literal \ (variable EXPANDED) So single quoting is literal in PowerShell too, and the fix this PR exists for is portable. Backslash is not an escape character there at all: the shell keeps the backslash and still expands the variable, which is exactly the #2201 failure the guard prevents, and the old character test returned null for it. Escaped `$` and backtick therefore stay flagged as substitutions even though bash reads them as literal. Only single quoting is trusted, because only single quoting means the same thing in every shell an operator can select. The affected test now asserts null and says why. The bash-argv probe still agrees on all nine command lines, with the escape case marked as intentionally more conservative than bash. Mutating the new guard away fails exactly one test -- the new one -- so it is not vacuous. Desktop suite 5516 passing / 0 failing; typecheck, check and the file-size ratchet clean. Refs #6834 Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- .../ui/agentSessionToolClassifier.test.mjs | 8 +++-- .../agents/ui/agentSessionToolClassifier.ts | 29 ++++++++++++++----- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs b/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs index 2ca919a427..76615d710b 100644 --- a/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs @@ -149,12 +149,16 @@ test("parseBuzzCliCommand preserves single-quoted literals in the --content=valu assert.equal(descriptor?.preview, "use `pnpm test`"); }); -test("parseBuzzCliCommand preserves backslash-escaped substitution characters", () => { +test("parseBuzzCliCommand rejects backslash-escaped substitution characters as non-portable", () => { + // bash treats `\$MESSAGE` as literal text, but PowerShell keeps the backslash + // and expands the variable anyway, so the escaped form cannot be trusted as a + // literal. Only single quoting is literal across the shells BUZZ_SHELL allows. const descriptor = parseBuzzCliCommand( 'buzz messages send --channel agents --content "literal \\$MESSAGE and \\`code\\`"', ); - assert.equal(descriptor?.preview, "literal $MESSAGE and `code`"); + assert.equal(descriptor?.renderClass, "message"); + assert.equal(descriptor?.preview, null); }); test("parseBuzzCliCommand still rejects an unquoted substitution in --content", () => { diff --git a/desktop/src/features/agents/ui/agentSessionToolClassifier.ts b/desktop/src/features/agents/ui/agentSessionToolClassifier.ts index 475a89726a..98e667efcf 100644 --- a/desktop/src/features/agents/ui/agentSessionToolClassifier.ts +++ b/desktop/src/features/agents/ui/agentSessionToolClassifier.ts @@ -519,15 +519,21 @@ function findBuzzCommand(tokens: ShellToken[]): BuzzCommandRange | null { * * `hasSubstitution` is what quote context buys us: `$` and a backtick mean * expansion when they are unquoted or inside double quotes, and mean nothing at - * all inside single quotes or after a backslash. Without this distinction a - * markdown code span in `--content 'use `pnpm test`'` is indistinguishable from - * a real `--content "$(cat file)"`, and both have to be discarded. + * all inside single quotes. Without this distinction a markdown code span in + * `--content 'use `pnpm test`'` is indistinguishable from a real + * `--content "$(cat file)"`, and both have to be discarded. * - * The `$` rule is deliberately conservative — any unescaped `$` outside single - * quotes flags the token, even where bash would leave it literal (a trailing - * `$`, or `$` before a space). Displaying an unexpanded shell expression as if - * it were the sent text is the failure this guard exists to prevent, so it errs - * towards dropping the preview. + * Only single quoting is trusted, because only single quoting is literal in + * every shell `BUZZ_SHELL` supports. A backslash escape is not: PowerShell + * keeps the backslash and still expands `"literal \$MESSAGE"`. So escaped + * substitution characters stay flagged even though bash would treat them as + * literal text. + * + * The `$` rule is deliberately conservative in the same direction — any `$` + * outside single quotes flags the token, even where bash would leave it literal + * (a trailing `$`, or `$` before a space). Displaying an unexpanded shell + * expression as if it were the sent text is the failure this guard exists to + * prevent, so it errs towards dropping the preview. */ export type ShellToken = { value: string; @@ -555,6 +561,13 @@ export function tokenizeShellCommandTokens(command: string): ShellToken[] { for (const char of command) { if (escaping) { + // The value is built as bash would build it, but a backslash escape does + // not earn substitution credit: backslash is not an escape character in + // PowerShell (`"literal \$MESSAGE"` keeps the backslash AND expands the + // variable) or cmd, and `BUZZ_SHELL` explicitly supports both. Only + // single quoting is literal across all of them, so escape forms stay + // conservative and keep #2201's behaviour. + if (isSubstitutionChar(char)) hasSubstitution = true; current += char; escaping = false; continue;