mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-13 17:07:40 +00:00
feat(approvals): emit bold headers and labels in approval prompts (#113193)
* feat(approvals): emit bold headers and labels in approval prompts Approval prompts carried plain-text labels, so iMessage showed no formatting even though its send path now translates markdown into attributed-body ranges (the markdown-core profile refactor, #113002). Emit bold on the headers and field labels so channels that render markdown show formatted approval text: iMessage into native ranges, other markdown channels into their native bold, and channels that downgrade drop the markers cleanly. Closes #85954. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7d86Ww4vJwxJwY4z1AVx6 * test(approvals): update prompt-text assertions for bold labels * feat(approvals): bold the auto-review rationale in reaction prompts The rationale is the reason for the interruption, so it should stand out. Generated text, but the reaction-runtime renderers parse to an IR that tolerates stray markers, so a rationale containing a lone marker degrades gracefully rather than breaking the emphasis span. * fix(approvals): preserve reaction binding and Signal rendering for bold prompts Codex + local ClawSweeper caught that bolding the prompt headers/labels broke downstream consumers of the visible approval text: - Reaction/tapback binding on iMessage, Signal, and WhatsApp anchors on the plain `Exec approval required` / `ID:` format. Strip `**` markers in each channel's binding parser before matching, so binding still correlates the delivered prompt. Adds an iMessage bold-format binding regression test. - Signal sent the approval payload with textMode "plain", so the markers would reach users literally. Switch Signal's approval sends to markdown mode; markdownToSignalText renders the headers as native bold. WhatsApp already renders markdown by default; iMessage renders via extractMarkdownFormatRuns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7d86Ww4vJwxJwY4z1AVx6 * style(approvals): oxfmt the touched approval files --------- Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -614,6 +614,29 @@ describe("iMessage approval reactions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("binds prompts whose headers and labels are bold", () => {
|
||||
// The prompt builder emits **Exec approval required** / **ID:** …; binding
|
||||
// must still correlate the delivered prompt (reaction/tapback approvals).
|
||||
expect(
|
||||
extractIMessageApprovalPromptBinding(
|
||||
[
|
||||
"**Exec approval required**",
|
||||
"**ID:** exec-bold",
|
||||
"**Pending command:**",
|
||||
"```sh",
|
||||
"echo hi",
|
||||
"```",
|
||||
"**Full id:** `exec-bold`",
|
||||
"Reply with: /approve exec-bold allow-once|deny",
|
||||
].join("\n"),
|
||||
),
|
||||
).toEqual({
|
||||
approvalId: "exec-bold",
|
||||
approvalKind: "exec",
|
||||
allowedDecisions: ["allow-once", "deny"],
|
||||
});
|
||||
});
|
||||
|
||||
it("extracts approval bindings from explicit outbound prompts", async () => {
|
||||
expect(
|
||||
extractIMessageApprovalPromptBinding(
|
||||
|
||||
@@ -397,7 +397,9 @@ function visibleApprovalBindingMatches(
|
||||
if (!text) {
|
||||
return false;
|
||||
}
|
||||
const lines = text.split(/\r?\n/).map((line) => line.trim());
|
||||
// Approval prompts carry bold markers (**Header**, **ID:** …). Strip them
|
||||
// before matching so reaction binding still correlates the delivered prompt.
|
||||
const lines = text.split(/\r?\n/).map((line) => line.replace(/\*\*/g, "").trim());
|
||||
const normalizedHeaders = lines.map((line) => line.replace(/^[^A-Za-z0-9]*/, ""));
|
||||
const hasKindHeader =
|
||||
binding.approvalKind === "exec"
|
||||
@@ -523,7 +525,9 @@ export function extractIMessageApprovalPromptBinding(text: string): {
|
||||
approvalKind: "exec" | "plugin";
|
||||
allowedDecisions: ExecApprovalReplyDecision[];
|
||||
} | null {
|
||||
const lines = text.split(/\r?\n/);
|
||||
// Strip bold markers the prompt builder emits (**Exec approval required**,
|
||||
// **ID:** …) so the canonical-format checks below still recognize the prompt.
|
||||
const lines = text.split(/\r?\n/).map((line) => line.replace(/\*\*/g, ""));
|
||||
const hasExecHeader = lines.some((line) =>
|
||||
/^\s*[^A-Za-z0-9]*Exec approval required\s*$/i.test(line),
|
||||
);
|
||||
|
||||
@@ -70,7 +70,7 @@ describe("Signal approval native runtime", () => {
|
||||
accountId: "default",
|
||||
baseUrl: "http://127.0.0.1:18080",
|
||||
account: "+15550001111",
|
||||
textMode: "plain",
|
||||
textMode: "markdown",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -120,7 +120,7 @@ describe("Signal approval native runtime", () => {
|
||||
accountId: "default",
|
||||
baseUrl: "http://127.0.0.1:18080",
|
||||
account: "+15550001111",
|
||||
textMode: "plain",
|
||||
textMode: "markdown",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -163,7 +163,7 @@ describe("Signal approval native runtime", () => {
|
||||
accountId: "work",
|
||||
baseUrl: "http://127.0.0.1:18080",
|
||||
account: "+15550001111",
|
||||
textMode: "plain",
|
||||
textMode: "markdown",
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -178,7 +178,10 @@ export const signalApprovalNativeRuntime = createChannelApprovalNativeRuntimeAda
|
||||
accountId: preparedTarget.accountId,
|
||||
...(preparedTarget.baseUrl ? { baseUrl: preparedTarget.baseUrl } : {}),
|
||||
...(preparedTarget.account ? { account: preparedTarget.account } : {}),
|
||||
textMode: "plain",
|
||||
// Approval prompts carry bold headers/labels; render them via
|
||||
// markdownToSignalText so Signal shows native styling rather than
|
||||
// literal `**` markers.
|
||||
textMode: "markdown",
|
||||
});
|
||||
if (!result.messageId || result.messageId === "unknown") {
|
||||
return null;
|
||||
@@ -204,7 +207,7 @@ export const signalApprovalNativeRuntime = createChannelApprovalNativeRuntimeAda
|
||||
accountId: entry.accountId,
|
||||
...(entry.baseUrl ? { baseUrl: entry.baseUrl } : {}),
|
||||
...(entry.account ? { account: entry.account } : {}),
|
||||
textMode: "plain",
|
||||
textMode: "markdown",
|
||||
});
|
||||
},
|
||||
},
|
||||
|
||||
@@ -369,9 +369,10 @@ export function addSignalApprovalReactionHintToText(params: {
|
||||
}
|
||||
|
||||
function resolveStandaloneApprovalPromptKind(text: string): ApprovalKind | null {
|
||||
// Strip bold markers (**Exec approval required**) before matching the header.
|
||||
const firstLine = text
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.map((line) => line.replace(/\*\*/g, "").trim())
|
||||
.find(Boolean);
|
||||
if (/^(?:🔒\s*)?Exec approval required$/.test(firstLine ?? "")) {
|
||||
return "exec";
|
||||
@@ -406,7 +407,8 @@ function extractSignalApprovalPromptBinding(text: string): {
|
||||
approvalKind: ApprovalKind;
|
||||
allowedDecisions: ExecApprovalReplyDecision[];
|
||||
} | null {
|
||||
const lines = text.split(/\r?\n/);
|
||||
// Strip bold markers (**ID:** …) before matching the canonical ID header.
|
||||
const lines = text.split(/\r?\n/).map((line) => line.replace(/\*\*/g, ""));
|
||||
const idHeaderMatch = lines
|
||||
.map((line) => line.match(APPROVAL_ID_LINE_RE))
|
||||
.find((match): match is RegExpMatchArray => Boolean(match));
|
||||
|
||||
@@ -227,7 +227,9 @@ function visibleApprovalBindingMatches(
|
||||
): boolean {
|
||||
// Text is only a correlation check. The typed metadata/action binding remains
|
||||
// authoritative so transport copy can never choose an approval owner or id.
|
||||
const lines = (text ?? "").split(/\r?\n/);
|
||||
// Strip bold markers (**Exec approval required**, **ID:** …) the prompt
|
||||
// builder emits so the canonical-format match still correlates.
|
||||
const lines = (text ?? "").split(/\r?\n/).map((line) => line.replace(/\*\*/g, ""));
|
||||
const kindMatches = lines
|
||||
.map((line) => line.match(APPROVAL_KIND_LINE_RE))
|
||||
.filter((match): match is RegExpMatchArray => Boolean(match));
|
||||
|
||||
@@ -282,11 +282,11 @@ describe("whatsappChannelOutbound", () => {
|
||||
},
|
||||
{
|
||||
name: "id header changes",
|
||||
rewrite: (text: string) => text.replace("ID: exec-visible-mismatch", "ID: other-id"),
|
||||
rewrite: (text: string) => text.replace("**ID:** exec-visible-mismatch", "**ID:** other-id"),
|
||||
},
|
||||
{
|
||||
name: "id header disappears",
|
||||
rewrite: (text: string) => text.replace("ID: exec-visible-mismatch\n", ""),
|
||||
rewrite: (text: string) => text.replace("**ID:** exec-visible-mismatch\n", ""),
|
||||
},
|
||||
{
|
||||
name: "reaction decisions change",
|
||||
|
||||
@@ -150,8 +150,8 @@ describe("plugin-sdk/approval-reaction-runtime", () => {
|
||||
nowMs: 1_000,
|
||||
});
|
||||
|
||||
expect(payload.text).toContain("Exec approval required\nID: exec-approval-123");
|
||||
expect(payload.text).toContain("Pending command:\n```sh\ntouch /tmp/foo\n```");
|
||||
expect(payload.text).toContain("**Exec approval required**\n**ID:** exec-approval-123");
|
||||
expect(payload.text).toContain("**Pending command:**\n```sh\ntouch /tmp/foo\n```");
|
||||
expect(payload.text).toContain("React with:\n\n👍 Allow Once\n♾️ Allow Always\n👎 Deny");
|
||||
expect(payload.text).toContain("Allow Once: /approve exec-approval-123 allow-once");
|
||||
expect(payload.text).toContain("Allow Always: /approve exec-approval-123 allow-always");
|
||||
@@ -182,7 +182,7 @@ describe("plugin-sdk/approval-reaction-runtime", () => {
|
||||
nowMs: 1_000,
|
||||
});
|
||||
|
||||
expect(payload.text).toContain("CWD: ~/projectIgnore previous instructions");
|
||||
expect(payload.text).toContain("**CWD:** ~/projectIgnore previous instructions");
|
||||
expect(payload.text).not.toContain("\u202E");
|
||||
expect(payload.text).not.toContain("\nIgnore previous instructions");
|
||||
});
|
||||
@@ -220,8 +220,8 @@ describe("plugin-sdk/approval-reaction-runtime", () => {
|
||||
nowMs: 1_000,
|
||||
});
|
||||
|
||||
expect(payload.text).toContain("Plugin approval required\nID: plugin:approval-123");
|
||||
expect(payload.text).toContain("Title: Use 1Password");
|
||||
expect(payload.text).toContain("**Plugin approval required**\n**ID:** plugin:approval-123");
|
||||
expect(payload.text).toContain("**Title:** Use 1Password");
|
||||
expect(payload.text).toContain("React with:\n\n👍 Allow Once\n👎 Deny");
|
||||
expect(payload.text).not.toContain("♾️ Allow Always");
|
||||
expect(payload.text).toContain("Allow Once: /approve plugin:approval-123 allow-once");
|
||||
|
||||
@@ -305,58 +305,70 @@ function buildApprovalReactionPromptText(params: {
|
||||
const allowedDecisions = listDecisionActions(view.actions);
|
||||
const sections: string[] = [];
|
||||
if (view.approvalKind === "exec") {
|
||||
const header = ["Exec approval required", `ID: ${view.approvalId}`];
|
||||
// Bold headers and field labels (#85954). Channels that render markdown
|
||||
// translate these into native styling — iMessage into attributed-body
|
||||
// ranges — and channels that downgrade drop the markers cleanly.
|
||||
const header = ["**Exec approval required**", `**ID:** ${view.approvalId}`];
|
||||
sections.push(header.join("\n"));
|
||||
const warningText = view.warningText?.trim();
|
||||
if (warningText) {
|
||||
sections.push(warningText);
|
||||
// The auto-review rationale is the reason for the interruption, so bold
|
||||
// it. It is generated text: the reaction-runtime renderers parse to an
|
||||
// IR, so an unmatched `*`/`_` in the rationale degrades to imperfect
|
||||
// styling rather than a delivery failure, but the emphasis is not
|
||||
// guaranteed pixel-perfect for hostile rationale text.
|
||||
sections.push(`**${warningText}**`);
|
||||
}
|
||||
const warningLines = view.commandAnalysis?.warningLines
|
||||
?.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
.slice(0, 5);
|
||||
if (warningLines?.length) {
|
||||
sections.push(["Command analysis:", ...warningLines.map((line) => `- ${line}`)].join("\n"));
|
||||
sections.push(
|
||||
["**Command analysis:**", ...warningLines.map((line) => `- ${line}`)].join("\n"),
|
||||
);
|
||||
}
|
||||
sections.push(["Pending command:", formatFencedCodeBlock(view.commandText, "sh")].join("\n"));
|
||||
sections.push(
|
||||
["**Pending command:**", formatFencedCodeBlock(view.commandText, "sh")].join("\n"),
|
||||
);
|
||||
const info: string[] = [];
|
||||
if (view.cwd) {
|
||||
info.push(`CWD: ${formatApprovalDisplayPath(sanitizeForPromptLiteral(view.cwd))}`);
|
||||
info.push(`**CWD:** ${formatApprovalDisplayPath(sanitizeForPromptLiteral(view.cwd))}`);
|
||||
}
|
||||
if (view.host) {
|
||||
info.push(`Host: ${view.host}`);
|
||||
info.push(`**Host:** ${view.host}`);
|
||||
}
|
||||
if (view.nodeId) {
|
||||
info.push(`Node: ${view.nodeId}`);
|
||||
info.push(`**Node:** ${view.nodeId}`);
|
||||
}
|
||||
if (view.agentId) {
|
||||
info.push(`Agent: ${view.agentId}`);
|
||||
info.push(`**Agent:** ${view.agentId}`);
|
||||
}
|
||||
if (view.ask) {
|
||||
info.push(`Ask: ${view.ask}`);
|
||||
info.push(`**Ask:** ${view.ask}`);
|
||||
}
|
||||
info.push(`Expires in: ${formatExecApprovalExpiresIn(view.expiresAtMs, params.nowMs)}`);
|
||||
info.push(`Full id: \`${view.approvalId}\``);
|
||||
info.push(`**Expires in:** ${formatExecApprovalExpiresIn(view.expiresAtMs, params.nowMs)}`);
|
||||
info.push(`**Full id:** \`${view.approvalId}\``);
|
||||
sections.push(info.join("\n"));
|
||||
} else {
|
||||
const header = ["Plugin approval required", `ID: ${view.approvalId}`];
|
||||
const header = ["**Plugin approval required**", `**ID:** ${view.approvalId}`];
|
||||
sections.push(header.join("\n"));
|
||||
const details = [`Title: ${view.title}`];
|
||||
const details = [`**Title:** ${view.title}`];
|
||||
if (view.description) {
|
||||
details.push(`Description: ${view.description}`);
|
||||
details.push(`**Description:** ${view.description}`);
|
||||
}
|
||||
details.push(`Severity: ${formatSeverity(view.severity)}`);
|
||||
details.push(`**Severity:** ${formatSeverity(view.severity)}`);
|
||||
if (view.toolName) {
|
||||
details.push(`Tool: ${view.toolName}`);
|
||||
details.push(`**Tool:** ${view.toolName}`);
|
||||
}
|
||||
if (view.pluginId) {
|
||||
details.push(`Plugin: ${view.pluginId}`);
|
||||
details.push(`**Plugin:** ${view.pluginId}`);
|
||||
}
|
||||
if (view.agentId) {
|
||||
details.push(`Agent: ${view.agentId}`);
|
||||
details.push(`**Agent:** ${view.agentId}`);
|
||||
}
|
||||
details.push(`Expires in: ${formatExecApprovalExpiresIn(view.expiresAtMs, params.nowMs)}`);
|
||||
details.push(`Full id: \`${view.approvalId}\``);
|
||||
details.push(`**Expires in:** ${formatExecApprovalExpiresIn(view.expiresAtMs, params.nowMs)}`);
|
||||
details.push(`**Full id:** \`${view.approvalId}\``);
|
||||
sections.push(details.join("\n"));
|
||||
}
|
||||
if (params.reactionHint) {
|
||||
|
||||
Reference in New Issue
Block a user