feat(settings): surface approval history audit trail #2949
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| import debug from 'debug'; | ||
| import { useCallback, useEffect, useRef, useState } from 'react'; | ||
|
|
||
| import { useT } from '../../../lib/i18n/I18nContext'; | ||
| import { | ||
| type ApprovalAuditEntry, | ||
| type ApprovalDecision, | ||
| fetchRecentApprovalDecisions, | ||
| } from '../../../services/api/approvalApi'; | ||
| import SettingsHeader from '../components/SettingsHeader'; | ||
| import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; | ||
|
|
||
| const log = debug('ui:approval-history'); | ||
|
|
||
| /** Render a decided timestamp as a locale string; fall back to the raw value. */ | ||
| const formatDateTime = (value: string): string => { | ||
| const ts = Date.parse(value); | ||
| return Number.isNaN(ts) ? value : new Date(ts).toLocaleString(); | ||
| }; | ||
|
|
||
| /** Tailwind tone + i18n label key per decision variant. */ | ||
| const DECISION_TONE: Record<ApprovalDecision, string> = { | ||
| approve_once: 'bg-sage-50 text-sage ring-sage-200', | ||
| approve_always_for_tool: 'bg-sage-50 text-sage ring-sage-200', | ||
| deny: 'bg-coral-50 text-coral ring-coral-200', | ||
| }; | ||
|
|
||
| const DECISION_LABEL_KEY: Record<ApprovalDecision, string> = { | ||
| approve_once: 'settings.approvalHistory.decision.approveOnce', | ||
| approve_always_for_tool: 'settings.approvalHistory.decision.approveAlways', | ||
| deny: 'settings.approvalHistory.decision.deny', | ||
| }; | ||
|
|
||
| const ApprovalHistoryPanel = () => { | ||
| const { t } = useT(); | ||
| const { navigateBack, breadcrumbs } = useSettingsNavigation(); | ||
|
|
||
| const [entries, setEntries] = useState<ApprovalAuditEntry[]>([]); | ||
| const [isLoading, setIsLoading] = useState(true); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| // Monotonic guard so an out-of-order (slower) response can't clobber a | ||
| // fresher one when the user taps Refresh rapidly (last request wins). | ||
| const loadSeqRef = useRef(0); | ||
|
|
||
| // Runs the fetch and only ever calls setState AFTER the await, so it is safe | ||
| // to invoke straight from the mount effect without tripping | ||
| // react-hooks/set-state-in-effect. The synchronous spinner reset lives in the | ||
| // Refresh event handler below, where synchronous setState is expected. | ||
| const runLoad = useCallback( | ||
| async (seq: number) => { | ||
| log('load start %o', { seq }); | ||
| try { | ||
| const rows = await fetchRecentApprovalDecisions(); | ||
| if (seq !== loadSeqRef.current) { | ||
| log('stale response discarded %o', { seq, latest: loadSeqRef.current }); | ||
| return; | ||
| } | ||
| setEntries(rows); | ||
| setError(null); | ||
| log('load ok %o', { seq, count: rows.length }); | ||
| } catch (e) { | ||
| if (seq !== loadSeqRef.current) return; | ||
| // Never leak raw backend error text into the UI; localized fallback only. | ||
| log('load failed %o', e); | ||
| setError(t('settings.approvalHistory.errorGeneric')); | ||
| } finally { | ||
| if (seq === loadSeqRef.current) setIsLoading(false); | ||
| } | ||
| }, | ||
| [t] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| void runLoad(++loadSeqRef.current); | ||
| }, [runLoad]); | ||
|
|
||
| const handleRefresh = () => { | ||
| setIsLoading(true); | ||
| setError(null); | ||
| void runLoad(++loadSeqRef.current); | ||
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <SettingsHeader | ||
| title={t('settings.approvalHistory.title')} | ||
| showBackButton | ||
| onBack={navigateBack} | ||
| breadcrumbs={breadcrumbs} | ||
| /> | ||
|
|
||
| <div className="p-4 space-y-4" data-testid="approval-history-panel"> | ||
| <div className="flex items-center justify-between"> | ||
| <p className="text-xs text-ink-soft">{t('settings.approvalHistory.subtitle')}</p> | ||
| <button | ||
| type="button" | ||
| onClick={handleRefresh} | ||
| disabled={isLoading} | ||
| data-testid="approval-history-refresh" | ||
| className="rounded bg-primary-500 px-3 py-1 text-xs text-white hover:bg-primary-600 disabled:opacity-50"> | ||
| {t('settings.approvalHistory.refresh')} | ||
| </button> | ||
| </div> | ||
|
|
||
| {isLoading ? ( | ||
| <p className="text-sm text-ink-soft" data-testid="approval-history-loading"> | ||
| {t('settings.approvalHistory.loading')} | ||
| </p> | ||
| ) : error ? ( | ||
| <div className="space-y-2" data-testid="approval-history-error"> | ||
| <p className="text-sm text-coral">{error}</p> | ||
| <button | ||
| type="button" | ||
| onClick={handleRefresh} | ||
| className="text-xs text-primary-600 hover:underline"> | ||
| {t('settings.approvalHistory.retry')} | ||
| </button> | ||
| </div> | ||
| ) : entries.length === 0 ? ( | ||
| <p className="text-sm text-ink-soft" data-testid="approval-history-empty"> | ||
| {t('settings.approvalHistory.emptyState')} | ||
| </p> | ||
| ) : ( | ||
| <ul className="space-y-2" data-testid="approval-history-list"> | ||
| {entries.map(entry => ( | ||
| <li | ||
| key={entry.request_id} | ||
| className="rounded-lg border border-line p-3 space-y-1" | ||
| data-testid="approval-history-row"> | ||
| <div className="flex items-center justify-between gap-2"> | ||
| <span className="font-mono text-xs text-ink truncate">{entry.tool_name}</span> | ||
| <span | ||
| className={`inline-flex shrink-0 items-center rounded-full px-2 py-0.5 text-xs font-medium ring-1 ${DECISION_TONE[entry.decision]}`} | ||
| data-testid={`approval-history-decision-${entry.decision}`}> | ||
| {t(DECISION_LABEL_KEY[entry.decision])} | ||
| </span> | ||
| </div> | ||
| <p className="text-xs text-ink-soft">{entry.action_summary}</p> | ||
| <p className="text-[11px] text-ink-soft"> | ||
| {t('settings.approvalHistory.decidedAt').replace( | ||
| '{date}', | ||
| formatDateTime(entry.decided_at) | ||
| )} | ||
| </p> | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| </li> | ||
| ))} | ||
| </ul> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default ApprovalHistoryPanel; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| import { fireEvent, screen, waitFor } from '@testing-library/react'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { | ||
| type ApprovalAuditEntry, | ||
| fetchRecentApprovalDecisions, | ||
| } from '../../../../services/api/approvalApi'; | ||
| import { renderWithProviders } from '../../../../test/test-utils'; | ||
| import ApprovalHistoryPanel from '../ApprovalHistoryPanel'; | ||
|
|
||
| vi.mock('../../hooks/useSettingsNavigation', () => ({ | ||
| useSettingsNavigation: () => ({ | ||
| navigateBack: vi.fn(), | ||
| navigateToSettings: vi.fn(), | ||
| breadcrumbs: [], | ||
| }), | ||
| })); | ||
|
|
||
| vi.mock('../../../../services/api/approvalApi', () => ({ fetchRecentApprovalDecisions: vi.fn() })); | ||
|
|
||
| const mockFetch = vi.mocked(fetchRecentApprovalDecisions); | ||
|
|
||
| const auditRow = (overrides: Partial<ApprovalAuditEntry> = {}): ApprovalAuditEntry => ({ | ||
| request_id: 'req-1', | ||
| tool_name: 'shell', | ||
| action_summary: 'run ls -la', | ||
| args_redacted: {}, | ||
| session_id: 'sess-1', | ||
| created_at: '2026-05-29T10:00:00Z', | ||
| expires_at: null, | ||
| decided_at: '2026-05-29T10:00:05Z', | ||
| decision: 'approve_once', | ||
| ...overrides, | ||
| }); | ||
|
|
||
| describe('ApprovalHistoryPanel', () => { | ||
| beforeEach(() => { | ||
| mockFetch.mockReset(); | ||
| }); | ||
|
|
||
| it('renders the loaded list of decided approvals', async () => { | ||
| mockFetch.mockResolvedValueOnce([ | ||
| auditRow({ request_id: 'a', tool_name: 'shell', decision: 'approve_once' }), | ||
| auditRow({ request_id: 'b', tool_name: 'curl', decision: 'deny' }), | ||
| ]); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| await screen.findByTestId('approval-history-list'); | ||
| const rows = screen.getAllByTestId('approval-history-row'); | ||
| expect(rows).toHaveLength(2); | ||
| expect(screen.getByText('shell')).toBeInTheDocument(); | ||
| expect(screen.getByText('curl')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders a decision badge per row', async () => { | ||
| mockFetch.mockResolvedValueOnce([ | ||
| auditRow({ request_id: 'a', decision: 'approve_always_for_tool' }), | ||
| auditRow({ request_id: 'b', decision: 'deny' }), | ||
| ]); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| await screen.findByTestId('approval-history-list'); | ||
| expect( | ||
| screen.getByTestId('approval-history-decision-approve_always_for_tool') | ||
| ).toBeInTheDocument(); | ||
| expect(screen.getByTestId('approval-history-decision-deny')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders the empty state when there are no decisions', async () => { | ||
| mockFetch.mockResolvedValueOnce([]); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| await screen.findByTestId('approval-history-empty'); | ||
| expect(screen.queryByTestId('approval-history-list')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders a localized error state when the fetch rejects', async () => { | ||
| mockFetch.mockRejectedValueOnce(new Error('boom')); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| const err = await screen.findByTestId('approval-history-error'); | ||
| // Raw backend text must never leak into the UI. | ||
| expect(err.textContent).not.toContain('boom'); | ||
| }); | ||
|
|
||
| it('refetches when the Refresh button is clicked', async () => { | ||
| mockFetch.mockResolvedValue([auditRow()]); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| await screen.findByTestId('approval-history-list'); | ||
| expect(mockFetch).toHaveBeenCalledTimes(1); | ||
|
|
||
| fireEvent.click(screen.getByTestId('approval-history-refresh')); | ||
| await waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2)); | ||
| }); | ||
|
|
||
| it('replaces the list with the refreshed result', async () => { | ||
| // The reachable refresh behavior: a completed load is replaced by the rows | ||
| // from a subsequent refresh. (The `loadSeqRef` last-request-wins guard | ||
| // protects against *overlapping* in-flight loads, but that race is not | ||
| // reachable from the UI — the Refresh button is `disabled` while a load is | ||
| // pending, so two concurrent fetches can never be initiated. The guard | ||
| // stays as defense against React concurrent/StrictMode double-invocation.) | ||
| mockFetch | ||
|
Member
There was a problem hiding this comment. Addressed in The Refresh button is So the I renamed the test to |
||
| .mockResolvedValueOnce([auditRow({ request_id: 'old', tool_name: 'old-tool' })]) | ||
| .mockResolvedValueOnce([auditRow({ request_id: 'new', tool_name: 'new-tool' })]); | ||
|
|
||
| renderWithProviders(<ApprovalHistoryPanel />, { | ||
| initialEntries: ['/settings/approval-history'], | ||
| }); | ||
|
|
||
| await screen.findByText('old-tool'); | ||
|
|
||
| fireEvent.click(screen.getByTestId('approval-history-refresh')); | ||
|
|
||
| await screen.findByText('new-tool'); | ||
| expect(screen.queryByText('old-tool')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize the new breadcrumb label instead of hard-coding it.
agentAccessCrumb.labelis user-visible and should come fromuseT()to preserve locale parity.Suggested fix
As per coding guidelines: “Every user-visible string in
app/src/**… must useuseT()…; hard-coded literals … are not allowed.”🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring: every breadcrumb label in this hook is a hard-coded English literal today (
settingsCrumb'Settings',accountCrumb'Account',developerCrumb'Developer Options', etc.). Localizing only the newagentAccessCrumbwould be inconsistent; the whole breadcrumb set should move touseT()in one dedicated pass rather than piecemeal here. Tracking as a follow-up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@obchain, that's a fair point — applyinguseT()to onlyagentAccessCrumbwhile all sibling crumbs remain hard-coded English would be inconsistent and would create a confusing mixed state. A single pass across the whole breadcrumb set is the right approach.Would you like me to open a GitHub issue to track the breadcrumb-localization follow-up so it doesn't get lost?