Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
13 changes: 12 additions & 1 deletion .github/ci/web-async-leaks.max
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@
# teardown landscape improves (issue #1468 will move MSW off a
# XHR interceptor, which should cut ~20-30 leaks).
#
# 2026-05-02 (#1710): raised 90 -> 100 to accommodate +14 new tests
# in the audit-cleanup-E PR (renderHook x5 in use-empty-state-props,
# render(<ProvidersStep/>) integration test, plus 8 new store-level
# cases in setup-wizard.test.ts and ws-sanitize.test.ts). Local count
# is 75; CI baseline is +20-21 above local due to parallel execution
# concurrency. The new tests are infrastructure for the typed-WS-
# sanitizer + setup-wizard recovery features that ship in this PR;
# trimming them would eliminate the core regression-protection of
# the new code paths. Lower back toward 80 once #1468 (MSW off XHR
# interceptor) lands.
#
# The CI step reads this file via:
# grep -v '^#' .github/ci/web-async-leaks.max | head -1
90
100
4 changes: 2 additions & 2 deletions docs/reference/web-package-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ On-demand reference for the React dashboard's directory layout. The short summar
web/src/
api/ # Axios client (`client.ts`), endpoint modules (`endpoints/`, 38 domains), and narrow-domain types under `types/` (27 files, no barrel `index.ts`; consumers import directly from `@/api/types/<domain>`)
components/ # React components: ui/ (shadcn primitives + SynthOrg core components), layout/ (app shell, sidebar with external link support, status bar); feature dirs added as pages are built
hooks/ # React hooks (auth, login lockout, WebSocket, polling, optimistic updates, command palette, flash effects, status transitions, page data composition, count animation, auto-scroll, roving tabindex, breakpoint detection, update tracking, animation presets, settings dirty state, settings keyboard shortcuts, communication edges, artifact / project data composition, useWorkflowsData, useBulkSelection)
hooks/ # React hooks (auth, login lockout, WebSocket, polling, optimistic updates, command palette, flash effects, status transitions, page data composition, count animation, auto-scroll, roving tabindex, breakpoint detection, update tracking, animation presets, settings dirty state, settings keyboard shortcuts, communication edges, artifact / project data composition, useWorkflowsData, useBulkSelection, useEmptyStateProps)
lib/ # Utilities (cn() class merging, semantic color mappers), Motion presets, CSP nonce reader, structured logger factory
mocks/ # MSW request handlers (handlers/) shared between Storybook stories and the Vitest suite; test-setup.tsx bootstraps them via setupServer(...defaultHandlers)
pages/ # Lazy-loaded page components (one per route); page-scoped sub-components in pages/<page-name>/ subdirs (e.g. tasks/, org-edit/, settings/, workflows/, fine-tuning/, training/)
router/ # React Router config, route constants (incl. DOCUMENTATION, an external link not SPA-routed), auth/setup guards
stores/ # Zustand stores (auth, WebSocket, toast, analytics, company, agents, approvals, budget, meetings, messages, tasks, settings, sinks, artifacts, projects, theme, workflows, fine-tuning, ceremony-policy, setup, training, per-domain stores). See "Store slicing patterns" below.
styles/ # Design tokens (--so-* CSS custom properties, single source of truth) and Tailwind theme bridge
styles/ # Design tokens (--so-* CSS custom properties, single source of truth), typed status-colour lookups (status-colors.ts: ROLE_BADGE_COLORS, ESCALATION_STATUS_BADGE_COLORS), and Tailwind theme bridge
utils/ # Constants, error handling, formatting, logging
__tests__/ # Vitest unit + property tests (mirrors src/ structure)
```
Expand Down
6 changes: 4 additions & 2 deletions web/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ All store **mutation** actions (create / update / delete) follow the `stores/con

**Async-leak ceiling (MANDATORY)**: CI fails if `vitest --detect-async-leaks` reports more than `MAX_ASYNC_LEAKS` (currently 90). Local floor is 49; CI baseline 77-80 (event-loop timing variance). Raise the ceiling only with documented per-PR justification; the structural floor is MSW 2.x + axios + tough-cookie and is tracked by #1468.

**WS payload sanitization**: `sanitizeWsString()` (from `web/src/stores/notifications.ts`) normalizes every string field received from WebSocket events. Any new WS payload handler that ingests untrusted strings MUST route through it.
**WS payload sanitization**: `sanitizeWsString()` and `sanitizeWsEnum()` live in `web/src/utils/ws-sanitize.ts` (pure helpers, re-exported from `@/stores/notifications`). `sanitizeWsString()` clamps every WS-supplied string (strips C0 controls + bidi-overrides + caps length). `sanitizeWsEnum<T>(value, allowlist, fallback, { field })` extends that with enum-allowlist validation: on unknown values it emits a structured `ws.enum.unknown` warning and returns the supplied fallback (must be a valid allowlist member), so a backend rolling out a new enum value cannot break UI rendering. Any new WS payload handler that ingests untrusted strings MUST route through one of these; raw `(sanitizeWsString(x, n) ?? '') as EnumType` casts are forbidden.

**WS wire protocol (MANDATORY)**: the client-server contract lives in `web/src/utils/constants.ts` (`WS_PROTOCOL_VERSION`, `WS_MAX_MESSAGE_SIZE`, `WS_HEARTBEAT_INTERVAL_MS`, `WS_PONG_TIMEOUT_MS`, `LOG_SANITIZE_MAX_LENGTH`) and MUST stay in lockstep with `src/synthorg/api/ws_models.py` / `src/synthorg/api/controllers/ws.py`. Bump the protocol version on both sides together for breaking payload changes.

Expand Down Expand Up @@ -95,7 +95,9 @@ See [docs/reference/web-design-system.md](../docs/reference/web-design-system.md
- Form fields -> `<InputField>` / `<SelectField>` / `<SliderField>` / `<ToggleField>` / `<SegmentedControl>` / `<TagInput>` / `<SearchInput>`.
- Slide-in panels -> `<Drawer width="compact|narrow|default|wide">` (Base UI; do NOT add inline `w-[40vw]` overrides).
- Loading / empty / error states -> `<Skeleton>` family / `<EmptyState>` / `<ErrorBoundary>` / `<ErrorBanner>` / `<ProgressIndicator>`.
- List-page primitives -> `<ListHeader>` / `<SearchFilterSort>` / `<Pagination>` / `<BulkActionBar>` / `<MetadataGrid>` / `<Breadcrumbs>`.
- List-page primitives -> `<ListHeader>` / `<SearchFilterSort>` / `<Pagination>` / `<BulkActionBar>` / `<MetadataGrid>` / `<Breadcrumbs>`. Page conventions: root container uses `space-y-section-gap` (the majority pattern -- `flex flex-col gap-section-gap` is equivalent but discouraged); `<ErrorBanner>` lands immediately after `<ListHeader>`, before any filter / pagination row; pages with a one-line mission statement pass it via `<ListHeader description="..." />`. List layout choice: use Kanban grouping for status-flow domains where each row's column conveys lifecycle phase (Tasks, Requests); use a flat scrollable list for queues without explicit phase semantics (Escalations, Approvals).
- Empty-state derivation -> `useEmptyStateProps({ filteredCount, totalCount, filterActive, empty, filtered })` from `@/hooks/use-empty-state-props` returns `EmptyStateProps | null` so the page branches on a single value instead of duplicating the "no data ever" / "no data after filter" discriminator.
- Status / role / risk / urgency badge classes -> `STATUS_COLORS` family from `@/styles/status-colors` (typed `Record<EnumValue, string>` lookups; no inline `Record<EnumValue, string>` constants per page).
- Confirmation / toasts -> `<ConfirmDialog>` / `<Toast>` (Zustand-backed queue, NOT Base UI's Toast).
- Cmd+K / shortcuts -> `<CommandPalette>` / `<KeyboardShortcutHint>` / `<CommandCheatsheet>`.
- Animation -> `<AnimatedPresence>` / `<StaggerGroup>` / `<LiveRegion>` (debounced ARIA live for WS updates).
Expand Down
34 changes: 22 additions & 12 deletions web/src/__tests__/components/ui/progress-gauge.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,22 @@ import { render, screen } from '@testing-library/react'
import * as fc from 'fast-check'
import { ProgressGauge } from '@/components/ui/progress-gauge'

// The circular variant renders the percentage twice -- once as visible
// SVG ``<text>`` and once inside the SVG ``<title>`` element that
// screen readers announce. Tests use ``getAllByText`` to accept either
// match without coupling to the DOM shape.
function expectPercentageText(value: string) {
const matches = screen.getAllByText(value)
expect(matches.length).toBeGreaterThan(0)
}

describe.each<['circular' | 'linear']>([
['circular'],
['linear'],
])('ProgressGauge shared behavior (variant: %s)', (variant) => {
it('renders the percentage value', () => {
render(<ProgressGauge value={75} variant={variant} />)
expect(screen.getByText('75%')).toBeInTheDocument()
expectPercentageText('75%')
})

it('renders the label when provided', () => {
Expand All @@ -18,17 +27,17 @@ describe.each<['circular' | 'linear']>([

it('clamps value to 0 minimum', () => {
render(<ProgressGauge value={-10} variant={variant} />)
expect(screen.getByText('0%')).toBeInTheDocument()
expectPercentageText('0%')
})

it('clamps value to max', () => {
render(<ProgressGauge value={150} max={100} variant={variant} />)
expect(screen.getByText('100%')).toBeInTheDocument()
expectPercentageText('100%')
})

it('computes percentage from custom max', () => {
render(<ProgressGauge value={50} max={200} variant={variant} />)
expect(screen.getByText('25%')).toBeInTheDocument()
expectPercentageText('25%')
})

it('has accessible role and aria attributes', () => {
Expand All @@ -48,34 +57,34 @@ describe.each<['circular' | 'linear']>([

it('handles max=0 without NaN', () => {
render(<ProgressGauge value={50} max={0} variant={variant} />)
expect(screen.getByText('100%')).toBeInTheDocument()
expectPercentageText('100%')
})

it('handles negative max by clamping to 1', () => {
render(<ProgressGauge value={50} max={-50} variant={variant} />)
// safeMax becomes Math.max(-50, 1) = 1, clampedValue = min(50, 1) = 1, percentage = 100%
expect(screen.getByText('100%')).toBeInTheDocument()
expectPercentageText('100%')
})

it('handles NaN max as 1', () => {
render(<ProgressGauge value={50} max={NaN} variant={variant} />)
// safeMax becomes 1, clampedValue = min(50, 1) = 1, percentage = 100%
expect(screen.getByText('100%')).toBeInTheDocument()
expectPercentageText('100%')
})

it('treats NaN value as 0%', () => {
render(<ProgressGauge value={NaN} variant={variant} />)
expect(screen.getByText('0%')).toBeInTheDocument()
expectPercentageText('0%')
})

it('treats Infinity as 0%', () => {
render(<ProgressGauge value={Infinity} variant={variant} />)
expect(screen.getByText('0%')).toBeInTheDocument()
expectPercentageText('0%')
})

it('treats -Infinity as 0%', () => {
render(<ProgressGauge value={-Infinity} variant={variant} />)
expect(screen.getByText('0%')).toBeInTheDocument()
expectPercentageText('0%')
})

it('always clamps percentage between 0 and 100 (property)', () => {
Expand All @@ -87,8 +96,9 @@ describe.each<['circular' | 'linear']>([
const { unmount } = render(
<ProgressGauge value={value} max={max} variant={variant} />,
)
const text = screen.getByText(/%$/)
const percentage = parseInt(text.textContent ?? '0')
const matches = screen.getAllByText(/%$/)
expect(matches.length).toBeGreaterThan(0)
const percentage = parseInt(matches[0]!.textContent ?? '0')
expect(percentage).toBeGreaterThanOrEqual(0)
expect(percentage).toBeLessThanOrEqual(100)
unmount()
Expand Down
109 changes: 109 additions & 0 deletions web/src/__tests__/hooks/use-empty-state-props.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { renderHook } from '@testing-library/react'
import { Inbox, Search } from 'lucide-react'
import { useEmptyStateProps } from '@/hooks/use-empty-state-props'

const empty = {
title: 'No items',
description: 'Create your first item to get started.',
} as const

const filtered = {
title: 'No matching items',
description: 'Adjust your filters.',
action: { label: 'Clear filters', onClick: () => {} },
} as const

describe('useEmptyStateProps', () => {
it('returns null when filteredCount > 0', () => {
const { result } = renderHook(() =>
useEmptyStateProps({
filteredCount: 5,
totalCount: 10,
filterActive: true,
empty,
filtered,
}),
)
expect(result.current).toBeNull()
})

it('returns the empty branch when totalCount is 0', () => {
const { result } = renderHook(() =>
useEmptyStateProps({
filteredCount: 0,
totalCount: 0,
filterActive: false,
icon: Inbox,
empty,
filtered,
}),
)
expect(result.current).toEqual({
icon: Inbox,
title: empty.title,
description: empty.description,
action: undefined,
})
})

it('returns the filtered branch when totalCount > 0 and filter is active', () => {
const { result } = renderHook(() =>
useEmptyStateProps({
filteredCount: 0,
totalCount: 12,
filterActive: true,
icon: Search,
empty,
filtered,
}),
)
expect(result.current).toEqual({
icon: Search,
title: filtered.title,
description: filtered.description,
action: filtered.action,
})
})

it('returns the empty branch when totalCount > 0 but filter is NOT active', () => {
// Logical edge: filteredCount === 0 but filter inactive means the
// visible view is empty even though the pool has items
// (e.g. an out-of-sync render). The hook treats this as the
// "empty" branch, not "filtered", because the user did NOT
// apply a filter to cause the empty state.
const { result } = renderHook(() =>
useEmptyStateProps({
filteredCount: 0,
totalCount: 5,
filterActive: false,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
empty,
filtered,
}),
)
expect(result.current?.title).toBe(empty.title)
})

it('returns the same reference when inputs are stable across renders', () => {
const { result, rerender } = renderHook(
(input: Parameters<typeof useEmptyStateProps>[0]) => useEmptyStateProps(input),
{
initialProps: {
filteredCount: 0,
totalCount: 0,
filterActive: false,
empty,
filtered,
},
},
)
const first = result.current
rerender({
filteredCount: 0,
totalCount: 0,
filterActive: false,
empty,
filtered,
})
expect(result.current).toBe(first)
})
})
25 changes: 19 additions & 6 deletions web/src/__tests__/pages/BudgetPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,28 @@ describe('BudgetPage', () => {
expect(screen.getByText('Budget Status')).toBeInTheDocument()
})

it('renders Spend Burn section', () => {
// SpendBurnChart and CostBreakdownChart are React.lazy-loaded so
// the recharts bundle defers to first chart render; the test must
// await the Suspense boundary to resolve before the section title
// is in the DOM. The 10000ms timeout (vs the default 1000ms) is a
// headroom band-aid for heavy parallel test load -- under normal
// sequential runs the dynamic import resolves in well under 1s.
// If these tests start consistently approaching 10s, treat that
// as a real performance regression in the lazy-import strategy
// rather than raising the timeout further.
it('renders Spend Burn section', async () => {
renderBudget()
expect(screen.getByText('Spend Burn')).toBeInTheDocument()
})
expect(
await screen.findByText('Spend Burn', undefined, { timeout: 10000 }),
).toBeInTheDocument()
}, 15000)

it('renders Cost Breakdown section', () => {
it('renders Cost Breakdown section', async () => {
renderBudget()
expect(screen.getByText('Cost Breakdown')).toBeInTheDocument()
})
expect(
await screen.findByText('Cost Breakdown', undefined, { timeout: 10000 }),
).toBeInTheDocument()
}, 15000)

it('renders Cost Categories section', () => {
renderBudget()
Expand Down
10 changes: 8 additions & 2 deletions web/src/__tests__/pages/DashboardPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,15 @@ describe('DashboardPage', () => {
expect(screen.getByText('Activity')).toBeInTheDocument()
})

it('renders Budget Burn section', () => {
// BudgetBurnChart is React.lazy-loaded so the recharts bundle defers
// to first chart render; the test must await the Suspense boundary.
// The 5000ms timeout (vs default 1000ms) accommodates heavy parallel
// test loads where the dynamic import takes longer to resolve.
it('renders Budget Burn section', async () => {
renderDashboard()
expect(screen.getByText('Budget Burn')).toBeInTheDocument()
expect(
await screen.findByText('Budget Burn', undefined, { timeout: 5000 }),
).toBeInTheDocument()
})

it('shows error banner when error is set', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe('ApprovalDetailDrawer', () => {
// Should show toast error -- onReject should NOT have been called
expect(defaultHandlers.onReject).not.toHaveBeenCalled()
const toasts = useToastStore.getState().toasts
expect(toasts.some((t) => t.title === 'Please provide a rejection reason')).toBe(true)
expect(toasts.some((t) => t.title === 'Rejection reason required')).toBe(true)
})

it('renders conditional fields for decided approvals', () => {
Expand Down
11 changes: 6 additions & 5 deletions web/src/__tests__/pages/messages/ChannelListItem.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('ChannelListItem', () => {
channel: makeChannel('#engineering'),
active: false,
unreadCount: 0,
onClick: vi.fn(),
onSelect: vi.fn(),
}

it('renders channel name', () => {
Expand Down Expand Up @@ -62,11 +62,12 @@ describe('ChannelListItem', () => {
expect(screen.getByText('#all-hands')).toBeInTheDocument()
})

it('calls onClick when clicked', async () => {
it('calls onSelect with the channel name when clicked', async () => {
const user = userEvent.setup()
const onClick = vi.fn()
render(<ChannelListItem {...defaultProps} onClick={onClick} />)
const onSelect = vi.fn()
render(<ChannelListItem {...defaultProps} onSelect={onSelect} />)
await user.click(screen.getByRole('button'))
expect(onClick).toHaveBeenCalledTimes(1)
expect(onSelect).toHaveBeenCalledTimes(1)
expect(onSelect).toHaveBeenCalledWith('#engineering')
})
})
Loading
Loading