feat(v2): the approval card reads a connector call as a verb and rows

frontend/design-system/preview/approval-readable.html

@@ -0,0 +1 @@
+<!doctype html><html><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>Approval arguments specimen</title></head><body style="margin:0"><div id="root"></div><script type="module" src="./approval-readable.tsx"></script></body></html>

frontend/design-system/preview/approval-readable.tsx

@@ -0,0 +1,45 @@
+import React from 'react';
+import axios from 'axios';
+import { createRoot } from 'react-dom/client';
+import { MemoryRouter } from 'react-router-dom';
+import { AuthContext } from '../../src/context/AuthContext';
+import i18n, { i18nReady } from '../../src/i18n';
+import V2ApprovalCard from '../../src/v2/components/V2ApprovalCard';
+import '../../src/v2/v2.css';
+// The approval card's rules live in the chat sheets V2App loads (S3).
+import '../../src/v2/chat/chat-message.css';
+import '../../src/v2/chat/chat-states.css';
+import '@fontsource/ibm-plex-sans/400.css';
+import '@fontsource/ibm-plex-sans/600.css';
+import '@fontsource/ibm-plex-mono/500.css';
+
+// Preview-only: answer the owner's pending-approvals read with fixture calls,
+// so the specimen renders the verb, rows and exact arguments without a server.
+const PENDING = { approvals: [
+  { approvalId: 'merge', messageId: 'merge', toolCall: { tool: 'github.merge_pull_request', canonicalArgs: {
+    owner: 'Team-Commonly', repo: 'demo-day', pullNumber: 4, mergeMethod: 'squash',
+    headSha: 'abc1234def5678abc1234def5678abc1234def56' } } },
+  { approvalId: 'inherited', messageId: 'inherited', toolCall: { tool: 'linear.create_issue', canonicalArgs: {
+    teamId: 'ENG', title: 'Approval rows wrap instead of widening the card on a phone',
+    description: 'Seen on a 390px screen.\nThe exact arguments stay one click away.', labels: ['ux', 'p1'], toString: 'own key' } } },
+] };
+axios.interceptors.request.use((config) => (String(config.url || '').startsWith('/api/approvals/pending')
+  ? { ...config, adapter: async (c) => ({ data: PENDING, status: 200, statusText: 'OK', headers: {}, config: c }) }
+  : config));
+
+async function show() {
+  await i18nReady;
+  await i18n.changeLanguage(new URLSearchParams(window.location.search).get('lang') || 'en');
+  createRoot(document.getElementById('root')!).render(
+    <AuthContext.Provider value={{currentUser:{_id:'viewer-1'},token:null} as never}>
+      <MemoryRouter><div className="v2-root" style={{height:'auto',minHeight:'100vh',padding:16,boxSizing:'border-box'}}>
+        <main style={{maxWidth:560,margin:'0 auto',display:'grid',gap:24}}>
+          {['merge','inherited'].map(id => <V2ApprovalCard key={id} authorLabel="Scout" time="now" message={{
+            id, pod_id:'pod-1', content:'Fixture approval', payload:{kind:'approval-card',approvalId:id,status:'flagged',ownerUserId:'viewer-1',actionType:'tool_call'},
+          } as never} />)}
+        </main>
+      </div></MemoryRouter>
+    </AuthContext.Provider>,
+  );
+}
+void show();

frontend/src/i18n/locales/en.json

@@ -1649,7 +1649,26 @@
       "failed": "Approved — tool call failed ({{code}}).",
       "refused": "Approved — tool call refused ({{code}}).",
       "pinChanged": "Approved, but the tool changed since this request. Ask again before running it.",
-      "expiredPending": "This tool-call approval has expired."
+      "expiredPending": "This tool-call approval has expired.",
+      "verb": {
+        "mergePullRequest": "Merge pull request #{{number}}",
+        "createIssue": "Open an issue",
+        "commentOnIssue": "Comment on issue #{{number}}",
+        "closeIssue": "Close issue #{{number}}"
+      },
+      "label": {
+        "repository": "Repository",
+        "pullRequest": "Pull request",
+        "issue": "Issue",
+        "head": "Head commit",
+        "mergeMethod": "Merge method",
+        "commitTitle": "Commit title",
+        "commitMessage": "Commit message",
+        "title": "Title",
+        "body": "Body",
+        "labels": "Labels"
+      },
+      "exactArgs": "Exact arguments"
     },
     "approve": "Approve",
     "decline": "Decline",

frontend/src/i18n/locales/zh-CN.json

@@ -1641,12 +1641,31 @@
       "failed": "已批准 — 工具调用失败（{{code}}）。",
       "refused": "已批准 — 工具调用被拒绝（{{code}}）。",
       "pinChanged": "已批准，但工具在请求后已变更。请重新发起请求后再执行。",
-      "expiredPending": "该工具调用批准已过期。"
+      "expiredPending": "该工具调用批准已过期。",
+      "verb": {
+        "mergePullRequest": "合并 PR #{{number}}",
+        "createIssue": "创建 Issue",
+        "commentOnIssue": "评论 Issue #{{number}}",
+        "closeIssue": "关闭 Issue #{{number}}"
+      },
+      "label": {
+        "repository": "仓库",
+        "pullRequest": "PR",
+        "issue": "Issue",
+        "head": "HEAD 提交",
+        "mergeMethod": "合并方式",
+        "commitTitle": "提交标题",
+        "commitMessage": "提交信息",
+        "title": "标题",
+        "body": "正文",
+        "labels": "标签"
+      },
+      "exactArgs": "原始参数"
     },
     "approve": "批准",
     "decline": "拒绝",
     "deciding": "处理中…",
-    "waitingForOwner": "等待工作区所有者决定。",
+    "waitingForOwner": "等待工作空间所有者决定。",
     "aged": "该提议已等待较久 — 请确认它仍然适用。",
     "approved": "✓ 已批准 — 已完成。",
     "approvedButFailed": "已批准，但执行失败：{{error}}",

frontend/src/v2/__tests__/V2ApprovalCardAction.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor, within } from '@testing-library/react';
 import { MemoryRouter, useLocation } from 'react-router-dom';
 import V2ApprovalCard from '../components/V2ApprovalCard';
 import en from '../../i18n/locales/en.json';
@@ -251,6 +251,16 @@ describe('tool-call approval details stay owner-scoped', () => {
     expect(screen.queryByText(/tool call refused/i)).not.toBeInTheDocument();
   });
 
+  test('arguments that are not an object leave the call unreadable and Approve disabled', async () => {
+    mockGet.mockResolvedValue({
+      approvals: [{ approvalId: 'tool-arr', messageId: 'm1', toolCall: { tool: 'github.merge_pull_request', canonicalArgs: [42, 'squash'] } }],
+    });
+    renderCard({ kind: 'approval-card', approvalId: 'tool-arr', status: 'flagged', ownerUserId: 'viewer-1', actionType: 'tool_call' });
+    await waitFor(() => expect(screen.getByTestId('approval-tool-call-state')).toHaveTextContent(en.approvalCard.toolCall.unavailable));
+    expect(screen.queryByTestId('approval-args')).not.toBeInTheDocument();
+    expect(screen.getByText(en.approvalCard.approve)).toBeDisabled();
+  });
+
   test('the owner fetches canonical args and must see them before approving', async () => {
     mockGet.mockResolvedValue({
       approvals: [{
@@ -279,6 +289,50 @@ describe('tool-call approval details stay owner-scoped', () => {
     expect(mockGet).toHaveBeenCalledWith('/api/approvals/pending?podId=pod-1');
   });
 
+  test('the owner reads the call as a verb and rows, with the exact arguments one click away', async () => {
+    const headSha = 'abc1234def5678abc1234def5678abc1234def56';
+    mockGet.mockResolvedValue({
+      approvals: [{
+        approvalId: 'tool-2',
+        messageId: 'm1',
+        toolCall: {
+          tool: 'github.merge_pull_request',
+          canonicalArgs: { pullNumber: 4, mergeMethod: 'squash', headSha, owner: 'Team-Commonly', repo: 'demo-day' },
+        },
+      }],
+    });
+
+    renderCard({
+      kind: 'approval-card',
+      approvalId: 'tool-2',
+      status: 'flagged',
+      ownerUserId: 'viewer-1',
+      actionType: 'tool_call',
+      summary: 'Merge the pull request',
+    });
+
+    await waitFor(() => expect(screen.getByTestId('approval-args')).toBeInTheDocument());
+    const action = screen.getByTestId('approval-action');
+    expect(action).toHaveTextContent('Merge pull request #4');
+    expect(action).toHaveTextContent('github.merge_pull_request');
+
+    const repoLink = within(screen.getByTestId('approval-arg-owner-repo')).getByRole('link', { name: 'Team-Commonly/demo-day' });
+    expect(repoLink).toHaveAttribute('href', 'https://github.com/Team-Commonly/demo-day');
+    expect(repoLink).toHaveAttribute('rel', 'noopener noreferrer');
+    expect(within(screen.getByTestId('approval-arg-pullNumber')).getByRole('link', { name: '#4' }))
+      .toHaveAttribute('href', 'https://github.com/Team-Commonly/demo-day/pull/4');
+    const head = screen.getByTestId('approval-arg-headSha');
+    expect(head).toHaveTextContent(en.approvalCard.toolCall.label.head);
+    expect(head).toHaveTextContent('abc1234');
+    expect(head.querySelector('dd')).toHaveAttribute('title', headSha);
+    expect(screen.getByTestId('approval-arg-mergeMethod')).toHaveTextContent('squash');
+
+    // The exact JSON is still on the card, behind a disclosure, never dropped.
+    expect(screen.getByText(en.approvalCard.toolCall.exactArgs).closest('details')).not.toBeNull();
+    expect(screen.getByTestId('approval-tool-call')).toHaveTextContent(`"headSha": "${headSha}"`);
+    expect(screen.getByText(en.approvalCard.approve)).toBeEnabled();
+  });
+
   test('a non-owner never fetches or renders canonical args', () => {
     renderCard({
       kind: 'approval-card',
@@ -330,3 +384,38 @@ describe('tool-call approval details stay owner-scoped', () => {
     expect(screen.getByText(en.approvalCard.decline)).toBeEnabled();
   });
 });
+
+test.each(['toString', 'constructor', 'hasOwnProperty', '__proto__', 'extra'])('canonical argument %s keeps the real consent card usable', async key => {
+  const canonicalArgs = JSON.parse(JSON.stringify({ title:'Pinned issue', [key]:'pinned-not-prose' }));
+  mockGet.mockResolvedValue({approvals:[{approvalId:'inherited-key',messageId:'m1',toolCall:{tool:'github.create_issue',canonicalArgs}}]});
+  renderCard({kind:'approval-card',approvalId:'inherited-key',status:'flagged',ownerUserId:'viewer-1',actionType:'tool_call'});
+  const row = await screen.findByTestId(`approval-arg-${key}`);
+  expect(within(row).getByText(key)).toBeInTheDocument();
+  expect(within(row).getByText('pinned-not-prose')).toBeInTheDocument();
+  expect(screen.getByTestId('approval-tool-call')).toHaveTextContent(`"${key}": "pinned-not-prose"`);
+  expect(screen.getByText(en.approvalCard.approve)).toBeEnabled();
+  expect(screen.getByText(en.approvalCard.decline)).toBeEnabled();
+});
+
+
+test.each([
+  ['github.merge_pull_request', { pullNumber: 42, issueNumber: 7 }, 'Merge pull request #42'],
+  ['github.comment_on_issue', { pullNumber: 7, issueNumber: 42 }, 'Comment on issue #42'],
+  ['github.close_issue', { pullNumber: 7, issueNumber: 42 }, 'Close issue #42'],
+  ['github.merge_pull_request', { pullNumber: 0, issueNumber: 7 }, null],
+  ['github.comment_on_issue', { pullNumber: 7, issueNumber: 0 }, null],
+  ['github.close_issue', { pullNumber: 7, issueNumber: 0 }, null],
+])('consent for %s uses only its required number (%j)', async (tool, canonicalArgs, expected) => {
+  mockGet.mockResolvedValue({ approvals: [{
+    approvalId: 'number-selection', messageId: 'm1', toolCall: { tool, canonicalArgs },
+  }] });
+  renderCard({ kind: 'approval-card', approvalId: 'number-selection', status: 'flagged',
+    ownerUserId: 'viewer-1', actionType: 'tool_call' });
+  await waitFor(() => expect(screen.getByTestId('approval-args')).toBeInTheDocument());
+  const action = screen.getByTestId('approval-action');
+  expect(action).toHaveTextContent(tool);
+  if (expected) expect(action).toHaveTextContent(expected);
+  else expect(action).not.toHaveTextContent(/#7/);
+  expect(screen.getByTestId('approval-arg-pullNumber')).toBeInTheDocument();
+  expect(screen.getByTestId('approval-arg-issueNumber')).toBeInTheDocument();
+});

frontend/src/v2/__tests__/toolCallView.test.ts

@@ -0,0 +1,137 @@
+import { toolCallView } from '../components/toolCallView';
+
+// The card is a consent surface: the readable view may relabel, but it must
+// show every pinned argument, and it may only link to github.com paths built
+// from validated values.
+const merge = {
+  pullNumber: 42,
+  mergeMethod: 'squash',
+  headSha: 'abc1234def5678abc1234def5678abc1234def56',
+  owner: 'Team-Commonly',
+  repo: 'demo-day',
+};
+
+const keysShown = (args: Record<string, unknown>, tool: string) => (
+  toolCallView(tool, args).rows.flatMap((row) => row.keys).sort()
+);
+
+describe('toolCallView', () => {
+  test('a merge reads as a verb with the repo, the PR, the pinned head and the method', () => {
+    const view = toolCallView('github.merge_pull_request', merge);
+    expect(view.verb).toEqual({ key: 'approvalCard.toolCall.verb.mergePullRequest', vars: { number: '42' } });
+    const byKey = Object.fromEntries(view.rows.map((row) => [row.keys.join('+'), row]));
+    expect(byKey['owner+repo']).toMatchObject({ value: 'Team-Commonly/demo-day', href: 'https://github.com/Team-Commonly/demo-day' });
+    expect(byKey.pullNumber).toMatchObject({ value: '#42', href: 'https://github.com/Team-Commonly/demo-day/pull/42' });
+    expect(byKey.headSha).toMatchObject({
+      value: 'abc1234',
+      title: merge.headSha,
+      kind: 'mono',
+      href: `https://github.com/Team-Commonly/demo-day/commit/${merge.headSha}`,
+    });
+    expect(byKey.mergeMethod).toMatchObject({ value: 'squash', labelKey: 'approvalCard.toolCall.label.mergeMethod' });
+  });
+
+  test('every pinned argument is shown, including ones the view has no name for', () => {
+    const args = { ...merge, commitTitle: 'Fix #3', commitMessage: 'line one\nline two', extra: { nested: true } };
+    expect(keysShown(args, 'github.merge_pull_request')).toEqual(Object.keys(args).sort());
+    const extra = toolCallView('github.merge_pull_request', args).rows.find((row) => row.keys[0] === 'extra');
+    expect(extra).toMatchObject({ label: 'extra', value: '{"nested":true}', kind: 'mono' });
+    expect(extra?.labelKey).toBeUndefined();
+  });
+
+  test('a hosted-MCP call keeps its tool name and shows its arguments as plain rows', () => {
+    const args = { parent: 'page-1', title: 'Weekly notes', properties: { status: 'draft' } };
+    const view = toolCallView('notion.create_page', args);
+    expect(view.verb).toBeNull();
+    expect(keysShown(args, 'notion.create_page')).toEqual(Object.keys(args).sort());
+    expect(view.rows.some((row) => row.href)).toBe(false);
+  });
+
+  test('a body stays one literal block, and labels read as a list', () => {
+    const view = toolCallView('github.create_issue', {
+      title: 'Bug', body: 'Steps:\n1. open\n2. <script>alert(1)</script>', labels: ['bug', 'p1'], owner: 'o', repo: 'r',
+    });
+    expect(view.verb).toEqual({ key: 'approvalCard.toolCall.verb.createIssue', vars: {} });
+    const body = view.rows.find((row) => row.keys[0] === 'body');
+    expect(body).toMatchObject({ kind: 'block', value: 'Steps:\n1. open\n2. <script>alert(1)</script>' });
+    expect(view.rows.find((row) => row.keys[0] === 'labels')).toMatchObject({ value: '["bug","p1"]', kind: 'text' });
+  });
+
+  test('no link is built from an owner, repo, number or SHA that fails validation', () => {
+    const view = toolCallView('github.merge_pull_request', {
+      owner: 'evil.example/../x', repo: 'demo-day', pullNumber: 4, headSha: 'not-a-sha',
+    });
+    expect(view.rows.every((row) => row.href === undefined)).toBe(true);
+    expect(view.rows.find((row) => row.keys[0] === 'headSha')).toMatchObject({ value: 'not-a-sha' });
+    const bad = toolCallView('github.close_issue', { owner: 'o', repo: 'r', issueNumber: '3; drop' });
+    expect(bad.rows.find((row) => row.keys[0] === 'issueNumber')).toMatchObject({ value: '3; drop' });
+    expect(bad.rows.find((row) => row.keys[0] === 'issueNumber')?.href).toBeUndefined();
+    // A numbered verb with no valid number falls back to the tool name rather than "#undefined".
+    expect(bad.verb).toBeNull();
+  });
+
+  test('only github tools get github links', () => {
+    const view = toolCallView('linear.create_issue', { owner: 'Team-Commonly', repo: 'demo-day', issueNumber: 3 });
+    expect(view.rows.every((row) => row.href === undefined)).toBe(true);
+  });
+
+  test('another tool keeps owner, repo, number and SHA keys as plain rows under their own names', () => {
+    const view = toolCallView('linear.create_issue', { owner: 'alice', repo: 'notes', issueNumber: 3, headSha: 'abc1234' });
+    expect(view.rows.map((row) => row.keys)).toEqual([['owner'], ['repo'], ['issueNumber'], ['headSha']]);
+    expect(view.rows.every((row) => row.labelKey === undefined)).toBe(true);
+    expect(view.rows.map((row) => row.value)).toEqual(['alice', 'notes', '3', 'abc1234']);
+  });
+
+  test('a multi-line string keeps its line breaks whatever its key', () => {
+    const view = toolCallView('linear.create_issue', { description: 'one\ntwo', team: 'ENG' });
+    expect(view.rows.find((row) => row.keys[0] === 'description')).toMatchObject({ kind: 'block' });
+    expect(view.rows.find((row) => row.keys[0] === 'team')).toMatchObject({ kind: 'text' });
+  });
+});
+
+test.each(['toString', 'constructor', 'hasOwnProperty', '__proto__', 'extra'])('unknown argument %s retains its raw label', key => {
+  const args = JSON.parse(JSON.stringify({ title:'Pinned title', [key]:'pinned-not-prose' }));
+  const view = toolCallView('github.create_issue', args);
+  const row = view.rows.find(candidate => candidate.keys.includes(key));
+  expect(row).toMatchObject({ label:key, value:'pinned-not-prose' });
+  expect(row?.labelKey).toBeUndefined();
+  expect(keysShown(args,'github.create_issue')).toEqual(Object.keys(args).sort());
+});
+
+test.each(['toString','constructor','__proto__'])('unknown tool %s has no inherited verb', tool => {
+  expect(toolCallView(tool,{issueNumber:1,extra:'kept'}).verb).toBeNull();
+});
+
+
+describe.each([
+  ['github.merge_pull_request', 'mergePullRequest', 'pullNumber', 'issueNumber'],
+  ['github.comment_on_issue', 'commentOnIssue', 'issueNumber', 'pullNumber'],
+  ['github.close_issue', 'closeIssue', 'issueNumber', 'pullNumber'],
+])('%s number selection', (tool, verb, required, irrelevant) => {
+  test('uses the required field when both numbers differ', () => {
+    expect(toolCallView(tool, { [required]: 42, [irrelevant]: 7 }).verb).toEqual({
+      key: `approvalCard.toolCall.verb.${verb}`, vars: { number: '42' },
+    });
+  });
+
+  test.each([undefined, 0, -1, 1.5, '42'])('invalid required number %s never uses the other field', invalid => {
+    expect(toolCallView(tool, { [required]: invalid, [irrelevant]: 7 }).verb).toBeNull();
+  });
+});
+
+
+test.each(['.', '..'])('dot segment %s never creates a GitHub link', segment => {
+  for (const args of [{ owner: segment, repo: 'valid-repo' }, { owner: 'valid-owner', repo: segment }]) {
+    const view = toolCallView('github.merge_pull_request', { ...args, pullNumber: 42, headSha: 'abc1234' });
+    expect(view.rows.every(row => row.href === undefined)).toBe(true);
+  }
+});
+
+test('label boundaries remain distinct when a label contains a comma', () => {
+  const labelsRow = (labels: string[]) => toolCallView('github.create_issue', { labels }).rows[0];
+  const comma = labelsRow(['security, urgent', 'p1']);
+  const separate = labelsRow(['security', 'urgent', 'p1']);
+  expect(comma.value).not.toBe(separate.value);
+  expect(JSON.parse(comma.value)).toEqual(['security, urgent', 'p1']);
+  expect(JSON.parse(separate.value)).toEqual(['security', 'urgent', 'p1']);
+});