mirror of
https://github.com/block/buzz.git
synced 2026-11-06 17:31:19 +00:00
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 <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
@@ -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 -",
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user