From 60642ca33384b735e70484a0880b59e7e0c61129 Mon Sep 17 00:00:00 2001 From: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Date: Tue, 25 Aug 2026 17:47:46 -0700 Subject: [PATCH 1/2] fix(desktop): keep quoted literals in agent-activity send previews An agent posting ordinary markdown lost the text of its own message in the activity view. `--content 'Fixed `labelForStatus` in the rail'` renders with no message in it, while the same send in plain prose renders fine. The cause is that the guard added by #2201 tests characters, not meaning. It rejects any `--content` containing `$` or a backtick so that `--content "$(cat file)"` and `"$MESSAGE"` can never be displayed as if the unexpanded shell expression were the sent text. That intent is right and is kept. But `tokenizeShellCommand` discards quoting, so by the time the guard runs a single-quoted literal backtick -- ordinary markdown the shell never touches -- is indistinguishable from a real substitution, and both are dropped. Since agents write markdown by default, the rejected case is the common one. Quote context is therefore preserved through tokenization. Each token now carries `hasSubstitution`, set only where the shell could actually expand something: `$` and a backtick unquoted or inside double quotes. Inside single quotes, or after a backslash, they are literal text and the preview survives. The guard reads that flag instead of scanning characters, so it still rejects every substitution #2201 was about -- `$(cat file)`, `$MESSAGE`, bare and prefixed, and a token that concatenates a quoted literal with an unquoted variable -- and those four tests continue to pin it. The `$` rule stays deliberately conservative: any unescaped `$` outside single quotes flags the token, including places bash would leave it literal. Showing a shell expression as the sent message is the failure being prevented, so it errs towards dropping the preview. Verified against bash rather than against a reading of it: for nine command lines, `bash` was asked for the real argv entry it passes as `--content`. Every preserved preview is byte-identical to that argv, and every case where the shell changed the text yields `null`. Three mutants, each producing a distinct failure set: restoring the character test fails the four new quoted-literal tests; making substitution tracking never fire fails eight, four of them #2201's own; and making single quotes stop protecting fails four. `tokenizeShellCommand` is kept as a string-returning wrapper so #2201's tokenizer test still exercises the same path. Not addressed here, and still open in #6834: the rail's missing event-fetch fallback (part b, gated on a design call) and the latent dot/underscore `isBuzzMessageSend` mismatch, which has no user-visible path on main. 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 | 85 ++++++++++++++ .../agents/ui/agentSessionToolClassifier.ts | 108 +++++++++++++----- 2 files changed, 164 insertions(+), 29 deletions(-) diff --git a/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs b/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs index 01a73c76989..2ca919a4273 100644 --- a/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs @@ -5,6 +5,7 @@ import { classifyTool, parseBuzzCliCommand, tokenizeShellCommand, + tokenizeShellCommandTokens, } from "./agentSessionToolClassifier.ts"; test("tokenizeShellCommand preserves quoted strings and command separators", () => { @@ -105,6 +106,90 @@ test("parseBuzzCliCommand preserves --content=inline for sends", () => { assert.equal(descriptor?.preview, "Acknowledged"); }); +test("tokenizeShellCommandTokens flags only tokens that can be substituted", () => { + const tokens = tokenizeShellCommandTokens( + "buzz messages send --content 'a `literal` $HOME' --channel \"$CHANNEL\"", + ); + const flagged = tokens.map((token) => [token.value, token.hasSubstitution]); + + assert.deepEqual(flagged, [ + ["buzz", false], + ["messages", false], + ["send", false], + ["--content", false], + ["a `literal` $HOME", false], + ["--channel", false], + ["$CHANNEL", true], + ]); +}); + +test("parseBuzzCliCommand preserves single-quoted literal backticks in --content", () => { + const descriptor = parseBuzzCliCommand( + "buzz messages send --channel agents --content 'Fixed `labelForStatus` in the rail'", + ); + + assert.equal(descriptor?.renderClass, "message"); + assert.equal(descriptor?.preview, "Fixed `labelForStatus` in the rail"); + assert.equal(descriptor?.object, "Fixed `labelForStatus` in the rail"); +}); + +test("parseBuzzCliCommand preserves a single-quoted literal dollar in --content", () => { + const descriptor = parseBuzzCliCommand( + "buzz messages send --channel agents --content 'Costs $5 per run, not $MESSAGE'", + ); + + assert.equal(descriptor?.preview, "Costs $5 per run, not $MESSAGE"); +}); + +test("parseBuzzCliCommand preserves single-quoted literals in the --content=value form", () => { + const descriptor = parseBuzzCliCommand( + "buzz messages send --channel agents --content='use `pnpm test`'", + ); + + assert.equal(descriptor?.preview, "use `pnpm test`"); +}); + +test("parseBuzzCliCommand preserves backslash-escaped substitution characters", () => { + const descriptor = parseBuzzCliCommand( + 'buzz messages send --channel agents --content "literal \\$MESSAGE and \\`code\\`"', + ); + + assert.equal(descriptor?.preview, "literal $MESSAGE and `code`"); +}); + +test("parseBuzzCliCommand still rejects an unquoted substitution in --content", () => { + for (const command of [ + "buzz messages send --channel agents --content $MESSAGE", + "buzz messages send --channel agents --content `cat /tmp/f`", + "buzz messages send --channel agents --content=$MESSAGE", + ]) { + const descriptor = parseBuzzCliCommand(command); + assert.equal(descriptor?.renderClass, "message"); + assert.equal( + descriptor?.preview, + null, + `send preview leaked a substitution for: ${command}`, + ); + } +}); + +test("parseBuzzCliCommand rejects --content that mixes a literal with a substitution", () => { + const descriptor = parseBuzzCliCommand( + "buzz messages send --channel agents --content 'a `literal` '\"$MESSAGE\"", + ); + + assert.equal(descriptor?.renderClass, "message"); + assert.equal(descriptor?.preview, null); +}); + +test("parseBuzzCliCommand keeps double-quoted backticks rejected as command substitution", () => { + const descriptor = parseBuzzCliCommand( + 'buzz messages send --channel agents --content "run `cat /tmp/f` now"', + ); + + assert.equal(descriptor?.preview, null); +}); + test("parseBuzzCliCommand never surfaces --channel as preview for sends", () => { const commands = [ "printf 'msg' | buzz messages send --channel my-uuid --content -", diff --git a/desktop/src/features/agents/ui/agentSessionToolClassifier.ts b/desktop/src/features/agents/ui/agentSessionToolClassifier.ts index 10fb74b1457..475a89726a5 100644 --- a/desktop/src/features/agents/ui/agentSessionToolClassifier.ts +++ b/desktop/src/features/agents/ui/agentSessionToolClassifier.ts @@ -356,12 +356,12 @@ function classifyDeveloperToolName(value: string | null | undefined) { export function parseBuzzCliCommand( command: string, ): AgentActivityDescriptor | null { - const tokens = tokenizeShellCommand(command); + const tokens = tokenizeShellCommandTokens(command); const range = findBuzzCommand(tokens); if (!range) return null; - const group = tokens[range.groupIndex]; - const verb = tokens[range.verbIndex] ?? "run"; + const group = tokens[range.groupIndex].value; + const verb = tokens[range.verbIndex]?.value ?? "run"; const operation = `${group}.${verb}`; const isSend = group === "messages" && verb === "send"; const preview = isSend @@ -451,17 +451,20 @@ function buzzCliTone(group: string, verb: string): AgentActivityTone { } function extractBuzzCliInlineContent( - tokens: string[], + tokens: ShellToken[], range: BuzzCommandRange, ): string | null { const content = getFlagValue(tokens, range.verbIndex + 1, "--content"); - if (!content || content === "-") return null; - if (content.includes("$") || content.includes("`")) return null; - return content; + if (!content || content.value === "-") return null; + // Quoting decides this, not the characters: a single-quoted or escaped `$` / + // backtick is literal text the shell never touched, so it is safe to show. + // Anything the shell could have expanded is dropped, as #2201 intended. + if (content.hasSubstitution) return null; + return content.value; } function extractBuzzCliObjectPreview( - tokens: string[], + tokens: ShellToken[], range: BuzzCommandRange, ): string | null { const flagPreview = @@ -470,11 +473,11 @@ function extractBuzzCliObjectPreview( getFlagValue(tokens, range.verbIndex + 1, "--query") ?? getFlagValue(tokens, range.verbIndex + 1, "--name") ?? getFlagValue(tokens, range.verbIndex + 1, "--file"); - if (flagPreview) return flagPreview; + if (flagPreview) return flagPreview.value; const next = tokens[range.verbIndex + 1]; - return next && !isCommandSeparator(next) && !next.startsWith("-") - ? next + return next && !isCommandSeparator(next.value) && !next.value.startsWith("-") + ? next.value : null; } @@ -484,24 +487,25 @@ type BuzzCommandRange = { verbIndex: number; }; -function findBuzzCommand(tokens: string[]): BuzzCommandRange | null { +function findBuzzCommand(tokens: ShellToken[]): BuzzCommandRange | null { for (let i = 0; i < tokens.length; i++) { - if (!isBuzzExecutable(tokens[i])) continue; + if (!isBuzzExecutable(tokens[i].value)) continue; for (let j = i + 1; j < tokens.length; j++) { - if (isCommandSeparator(tokens[j])) break; - if (tokens[j].startsWith("-")) { + const token = tokens[j].value; + if (isCommandSeparator(token)) break; + if (token.startsWith("-")) { if ( - !tokens[j].includes("=") && - tokens[j + 1]?.startsWith("-") === false + !token.includes("=") && + tokens[j + 1]?.value.startsWith("-") === false ) { j += 1; } continue; } - if (!BUZZ_CLI_GROUPS.has(tokens[j])) continue; + if (!BUZZ_CLI_GROUPS.has(token)) continue; const verbIndex = j + 1; - if (!tokens[verbIndex] || isCommandSeparator(tokens[verbIndex])) { + if (!tokens[verbIndex] || isCommandSeparator(tokens[verbIndex].value)) { return null; } return { buzzIndex: i, groupIndex: j, verbIndex }; @@ -510,17 +514,43 @@ function findBuzzCommand(tokens: string[]): BuzzCommandRange | null { return null; } +/** + * A token plus whether the shell could have substituted anything into it. + * + * `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. + * + * 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. + */ +export type ShellToken = { + value: string; + hasSubstitution: boolean; +}; + export function tokenizeShellCommand(command: string): string[] { - const tokens: string[] = []; + return tokenizeShellCommandTokens(command).map((token) => token.value); +} + +export function tokenizeShellCommandTokens(command: string): ShellToken[] { + const tokens: ShellToken[] = []; let current = ""; + let hasSubstitution = false; let quote: "'" | '"' | null = null; let escaping = false; const pushCurrent = () => { if (current.length > 0) { - tokens.push(current); + tokens.push({ value: current, hasSubstitution }); current = ""; } + hasSubstitution = false; }; for (const char of command) { @@ -534,8 +564,12 @@ export function tokenizeShellCommand(command: string): string[] { continue; } if (quote) { - if (char === quote) quote = null; - else current += char; + if (char === quote) { + quote = null; + } else { + if (quote === '"' && isSubstitutionChar(char)) hasSubstitution = true; + current += char; + } continue; } if (char === "'" || char === '"') { @@ -548,9 +582,10 @@ export function tokenizeShellCommand(command: string): string[] { } if (char === "|" || char === ";" || char === "&") { pushCurrent(); - tokens.push(char); + tokens.push({ value: char, hasSubstitution: false }); continue; } + if (isSubstitutionChar(char)) hasSubstitution = true; current += char; } @@ -559,6 +594,10 @@ export function tokenizeShellCommand(command: string): string[] { return tokens; } +function isSubstitutionChar(char: string) { + return char === "$" || char === "`"; +} + function isBuzzExecutable(token: string) { return token === "buzz" || token.split(/[\\/]/).pop() === "buzz"; } @@ -567,16 +606,27 @@ function isCommandSeparator(token: string) { return token === "|" || token === ";" || token === "&"; } -function getFlagValue(tokens: string[], start: number, flag: string) { +function getFlagValue( + tokens: ShellToken[], + start: number, + flag: string, +): ShellToken | null { for (let i = start; i < tokens.length; i++) { const token = tokens[i]; - if (isCommandSeparator(token)) return null; - if (token === flag) { - return tokens[i + 1] && !isCommandSeparator(tokens[i + 1]) + if (isCommandSeparator(token.value)) return null; + if (token.value === flag) { + return tokens[i + 1] && !isCommandSeparator(tokens[i + 1].value) ? tokens[i + 1] : null; } - if (token.startsWith(`${flag}=`)) return token.slice(flag.length + 1); + if (token.value.startsWith(`${flag}=`)) { + // The substitution flag is per token, so `--content=$X` carries it here + // too; slicing the value off must not lose it. + return { + value: token.value.slice(flag.length + 1), + hasSubstitution: token.hasSubstitution, + }; + } } return null; } 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 2/2] 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 2ca919a4273..76615d710ba 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 475a89726a5..98e667efcfb 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;