Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes/409.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 13 additions & 10 deletions desktop/src/features/chat/ApprovalCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<ApprovalView['status'], string>> = {
allow: 'You allowed this once',
Expand Down Expand Up @@ -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 <span className="font-mono text-xs">{approval.name}</span>
</>
) : null)

return (
<motion.div
initial={{ opacity: 0, y: 4 }}
Expand Down Expand Up @@ -216,15 +227,7 @@ export function ApprovalCard({
<span className="selectable min-w-0 break-words">{approval.warning}</span>
</div>
)}
<p className="mt-0.5 text-sm text-fg-muted">
{(command && 'It runs on this computer with your permissions, in this folder:') ||
purpose ||
approval.reason?.trim() || (
<>
{meta.done} using <span className="font-mono text-xs">{approval.name}</span>
</>
)}
</p>
{smallPrint && <p className="mt-0.5 text-sm text-fg-muted">{smallPrint}</p>}
{command && (
<div className="mt-1 flex items-center gap-1 text-xs text-fg-subtle">
<IconFolder size={12} className="shrink-0" />
Expand Down
14 changes: 14 additions & 0 deletions desktop/src/features/chat/approvalRisk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
27 changes: 26 additions & 1 deletion desktop/tests/approvalRisk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"')
})
Loading