diff --git a/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs b/desktop/src/features/agents/ui/agentSessionToolClassifier.test.mjs index 01a73c76989..76615d710ba 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,94 @@ 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 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?.renderClass, "message"); + assert.equal(descriptor?.preview, null); +}); + +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..98e667efcfb 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,21 +514,60 @@ 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. 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. + * + * 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; + 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) { 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; @@ -534,8 +577,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 +595,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 +607,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 +619,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; }