diff --git a/changes/409.fixed.md b/changes/409.fixed.md new file mode 100644 index 00000000..c1796d33 --- /dev/null +++ b/changes/409.fixed.md @@ -0,0 +1,2 @@ +**An approval card no longer says the same thing twice.** When Sentient asked to click a free "Subscribe" button, the +card showed "Signs up" both as its label and again in the line below. Now it says it once. diff --git a/desktop/src/features/chat/ApprovalCard.tsx b/desktop/src/features/chat/ApprovalCard.tsx index 893eb92c..738e9b39 100644 --- a/desktop/src/features/chat/ApprovalCard.tsx +++ b/desktop/src/features/chat/ApprovalCard.tsx @@ -30,7 +30,7 @@ import { cn, humanize, truncate } from '@/lib/utils' import { ActionPreview } from './ActionPreview' import { hostOf, refLabel } from './cards/bits' import { toolMeta } from './toolMeta' -import { resolveWording, type RiskLabel } from './approvalRisk' +import { approvalCardReason, resolveWording, type RiskLabel } from './approvalRisk' const STATUS_TEXT: Partial> = { allow: 'You allowed this once', @@ -188,6 +188,17 @@ export function ApprovalCard({ (c) => c !== 'allow' && c !== 'deny' && c !== 'allow_task' && !single ) + const reason = approvalCardReason(approval.reason, risk.label) + const smallPrint = + (command && 'It runs on this computer with your permissions, in this folder:') || + purpose || + reason || + (meta.done ? ( + <> + {meta.done} using {approval.name} + + ) : null) + return ( {approval.warning} )} -

- {(command && 'It runs on this computer with your permissions, in this folder:') || - purpose || - approval.reason?.trim() || ( - <> - {meta.done} using {approval.name} - - )} -

+ {smallPrint &&

{smallPrint}

} {command && (
diff --git a/desktop/src/features/chat/approvalRisk.ts b/desktop/src/features/chat/approvalRisk.ts index 7c5b1836..bc2b3215 100644 --- a/desktop/src/features/chat/approvalRisk.ts +++ b/desktop/src/features/chat/approvalRisk.ts @@ -299,3 +299,17 @@ export function resolveWording(approval: { if (!GENERIC_RISK_LABELS.has(wording.label)) return wording return engineWording(approval.riskLabel, approval.risk, wording.verb) ?? wording } + +/** + * Pick the small-print reason for an approval card (#409). + * Skips the reason when it matches the badge label after trimming and ignoring case, + * avoiding duplicate lines such as a [Signs up] badge followed by "Signs up" in small print. + */ +export function approvalCardReason(reason?: string | null, badgeLabel?: string | null): string | null { + const trimmed = reason?.trim() + if (!trimmed) return null + if (badgeLabel && trimmed.toLowerCase() === badgeLabel.trim().toLowerCase()) { + return null + } + return trimmed +} diff --git a/desktop/tests/approvalRisk.test.ts b/desktop/tests/approvalRisk.test.ts index 35682237..87bca6f4 100644 --- a/desktop/tests/approvalRisk.test.ts +++ b/desktop/tests/approvalRisk.test.ts @@ -3,7 +3,7 @@ // Run with `npm test` (no Electron needed). import assert from 'node:assert/strict' import { test } from 'node:test' -import { approvalRisk, engineWording, resolveWording } from '../src/features/chat/approvalRisk.ts' +import { approvalCardReason, approvalRisk, engineWording, resolveWording } from '../src/features/chat/approvalRisk.ts' const email = (name: string, body: string, reason = 'mcp_mailer: Send an email to one address') => ({ risk: 'send' as const, @@ -326,3 +326,28 @@ test('a priced subscribe button still reads as a purchase', () => { assert.equal(w.label, 'Purchase') assert.equal(w.verb, 'make a purchase') }) + +// #409: the small-print reason skips the reason when it matches the badge label, +// so a free Subscribe card doesn't repeat "Signs up" twice. +test('approval card small print skips the reason when it repeats the badge label (#409)', () => { + assert.equal(approvalCardReason('Signs up', 'Signs up'), null) + assert.equal(approvalCardReason(' signs up ', 'Signs up'), null) + assert.equal(approvalCardReason('Purchase', 'Purchase'), null) + assert.equal(approvalCardReason('Deletes', 'Deletes'), null) + assert.equal(approvalCardReason('Unlocks a door', 'Home security'), 'Unlocks a door') + assert.equal(approvalCardReason('Send payment', 'Purchase'), 'Send payment') + assert.equal(approvalCardReason('', 'Signs up'), null) + assert.equal(approvalCardReason(undefined, 'Signs up'), null) +}) + + +// #409 with #367 and #404: the badge comes from resolveWording and the reason is the +// tool's plain label, so a free Subscribe click's reason and badge are the same words. +test('a free Subscribe click shows "Signs up" once, a delete click keeps its own reason (#409)', () => { + const subscribe = { ...click('Signs up'), reason: 'Signs up' } + assert.equal(resolveWording(subscribe).label, 'Signs up') + assert.equal(approvalCardReason(subscribe.reason, resolveWording(subscribe).label), null) + const remove = { ...click('Deletes'), reason: 'Click "Delete account"' } + assert.equal(resolveWording(remove).label, 'Deletes') + assert.equal(approvalCardReason(remove.reason, resolveWording(remove).label), 'Click "Delete account"') +})