fix(web): audit cleanup E -- frontend & UX polish (#1710)#1718
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🧰 Additional context used📓 Path-based instructions (7)**/*.{py,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/**/{components,pages,src/{components,pages}}/**/*.{tsx,ts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/**/pages/**/*.{tsx,ts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/**/{pages,components,src/{pages,components}}/**/*.{tsx,ts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (4)
WalkthroughAdds a typed WebSocket payload contract and introduces |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request standardizes UI patterns across the dashboard by introducing a centralized useEmptyStateProps hook and shared status-colors configuration. It optimizes performance through lazy loading of chart components and strengthens security with enhanced WebSocket payload sanitization for strings and enums. Key functional updates include search and sorting capabilities for list pages, improved error reporting for API conflicts and clipboard actions, and expanded test suites for core stores and utilities. The review feedback correctly identifies a logical discrepancy in a new test case, a semantic inconsistency in filtered count reporting on the Approvals page, and a state management conflict where a store update would trigger an unwanted error banner.
| const { result } = renderHook(() => | ||
| useEmptyStateProps({ | ||
| filteredCount: 0, | ||
| totalCount: 0, |
There was a problem hiding this comment.
The test description states totalCount > 0, but the test data provides totalCount: 0. To properly test the logic where a filter is inactive but the list is empty (or to test the 'out-of-sync' scenario mentioned in the comment), totalCount should be set to a positive value.
| totalCount: 0, | |
| totalCount: 5, |
| // state below short-circuits before the empty-state branch ever | ||
| // matters. | ||
| const emptyStateProps = useEmptyStateProps({ | ||
| filteredCount: grouped.size, |
There was a problem hiding this comment.
Inconsistency in filteredCount value: grouped.size represents the number of risk categories (keys in the map), not the total number of filtered items. While this works for the current hook implementation's > 0 check, it is semantically incorrect and differs from the usage in other pages (e.g., EscalationQueuePage.tsx).
| filteredCount: grouped.size, | |
| filteredCount: filteredApprovals.length, |
| preset: presetName, | ||
| error: fetchErrMsg, | ||
| }) | ||
| useToastStore.getState().add({ |
There was a problem hiding this comment.
The comment at line 112 states that the error banner stays clean, but fetchProviders() in the store automatically sets providersError upon failure, which triggers the global ErrorBanner. To ensure the banner stays clean as intended when only the refresh fails after a successful creation, providersError should be explicitly cleared here.
| useToastStore.getState().add({ | |
| useSetupWizardStore.setState({ providersError: null }) | |
| useToastStore.getState().add({ |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
web/src/pages/UsersPage.tsx (1)
109-136: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPlace
ErrorBannerdirectly afterListHeaderon this list pageThe current order renders
SearchFilterSortbefore the load error banner. This breaks the list-page layout contract and weakens error discoverability.As per coding guidelines: "
web/src/pages/**/*.tsx:<ErrorBanner>lands immediately after<ListHeader>, before filter/pagination rows."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/UsersPage.tsx` around lines 109 - 136, Move the ErrorBanner so it is rendered immediately after the ListHeader in UsersPage: currently ErrorBanner is placed after the SearchFilterSort component; change the JSX order so ListHeader is followed directly by ErrorBanner (keeping the same props: severity, title, description, onRetry that calls fetchUsers), and then render SearchFilterSort and the rest (SearchInput uses searchQuery and setSearchQuery) to satisfy the project layout contract.web/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsx (1)
170-183: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd an assertion for the new inline reject-reason error
This test now checks the toast title, but it should also assert the inline field error (the newly added primary UX signal) to protect against regressions.
Suggested assertion addition
const rejectDialog = screen.getByRole('alertdialog') await user.click(within(rejectDialog).getByRole('button', { name: /reject/i })) // Should show toast error -- onReject should NOT have been called expect(defaultHandlers.onReject).not.toHaveBeenCalled() + expect( + within(rejectDialog).getByText(/rejection requires a reason for the approval record/i), + ).toBeInTheDocument() const toasts = useToastStore.getState().toasts expect(toasts.some((t) => t.title === 'Rejection reason required')).toBe(true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsx` around lines 170 - 183, Add an assertion that the inline field error is shown when attempting to reject without a reason: after opening the alertdialog (rejectDialog) and clicking its Reject button, query the reject reason input/field within rejectDialog (e.g., by role textbox or label used in the component) and assert that the inline validation message or error state is present (for example check for the error text "Rejection reason required" or that the input has aria-invalid/aria-errormessage pointing to the error). Keep the existing toast and onReject assertions (defaultHandlers.onReject and useToastStore checks) and scope the field lookup to rejectDialog (use within(rejectDialog)) so the check targets the inline error, not the drawer button.web/src/stores/messages.ts (1)
121-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
message.typeandmessage.priorityare still unsafe enum casts.Line 176 and Line 177 cast sanitized strings to union types without allowlist validation. Unknown WS values can still enter state and break downstream UI assumptions; clamp these with
sanitizeWsEnumjust likeattachments[].type.As per coding guidelines: “Use `sanitizeWsEnum(value, allowlist, fallback, { field })` for enum validation” and “raw `(sanitizeWsString(x, n) ?? '') as EnumType` casts are forbidden”.Suggested fix pattern
- const type = sanitizeWsString(c.type, 64) ?? '' - const priority = sanitizeWsString(c.priority, 64) ?? '' + const type = sanitizeWsEnum(c.type, MESSAGE_TYPE_VALUES, 'task_update', { + maxLen: 64, + field: 'message.type', + }) + const priority = sanitizeWsEnum(c.priority, MESSAGE_PRIORITY_VALUES, 'normal', { + maxLen: 64, + field: 'message.priority', + }) @@ - type: type as Message['type'], - priority: priority as Message['priority'], + type, + priority,// web/src/api/types/messages.ts +export const MESSAGE_TYPE_VALUES = [ + 'task_update', 'question', 'announcement', 'review_request', + 'approval', 'delegation', 'status_report', 'escalation', + 'meeting_contribution', 'hr_notification', +] as const satisfies readonly MessageType[] + +export const MESSAGE_PRIORITY_VALUES = [ + 'low', 'normal', 'high', 'urgent', +] as const satisfies readonly MessagePriority[]Also applies to: 176-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/messages.ts` around lines 121 - 123, Replace the unsafe enum casts for message.type and message.priority (currently using sanitizeWsString(...) as EnumType) with allowlist-validated calls to sanitizeWsEnum; specifically, pass the sanitized string into sanitizeWsEnum<T>(value, allowlist, fallback, { field: 'type'|'priority' }) using the correct enum allowlists and sensible fallback values, mirroring how attachments[].type is validated, so unknown WS values are clamped instead of being cast through.web/src/pages/setup/SkipWizardForm.tsx (1)
30-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t rely on
catchto detectsubmitCompany()failure.
submitCompany()already reports ordinary failures through store state, so this flow can still callwizardCompleteSetup()after company creation failed and then show the wrong error branch. Please gate the second step on the post-submitCompany()store state/return contract and let the store-owned error slots drive the banner instead of a localtry/catch.As per coding guidelines "Callers MUST NOT wrap store mutation calls in
try/catch; the store owns the error UX".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/setup/SkipWizardForm.tsx` around lines 30 - 55, The current try/catch allows wizardCompleteSetup() to run even if submitCompany() failed; you must instead check the post-submit store state or submitCompany's return contract before proceeding and avoid wrapping store mutation calls in try/catch. After calling submitCompany(), read useSetupWizardStore.getState().companyResponse (or the store's explicit success flag/err slot) and only call wizardCompleteSetup(), setCompanyNameStore(trimmed), useSetupStore.setState({ setupComplete: true }), show the success toast and navigate('/') when the store indicates the company was actually created; if the store reports an error, surface that via the store-owned error slots (and setError from those slots) rather than relying on catch to choose the branch. Ensure no store mutation is wrapped in try/catch per the guideline and rely on useSetupWizardStore state to drive the UI flow.web/src/stores/setup-wizard/providers.ts (1)
64-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear
providersWarningon a successful providers refresh.
providersWarningnow backs a persistent banner, butfetchProviders()only resetsprovidersError. After the user clicks Retry on the new warning banner and the reload succeeds, the warning still sticks around because nothing clears it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/setup-wizard/providers.ts` around lines 64 - 74, fetchProviders currently clears providersError but not providersWarning, so a successful refresh leaves the persistent warning banner visible; update the async fetchProviders function to also clear providersWarning on success (e.g., call set with providersWarning: null alongside providers and providersLoading:false) after listProviders resolves, while leaving the existing error handling intact that sets providersError and providersLoading on catch; locate the fetchProviders method and add providersWarning: null to the successful set call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/hooks/use-empty-state-props.test.ts`:
- Around line 68-75: The test for useEmptyStateProps currently sets totalCount:
0 but its name requires totalCount > 0; update the test input in the it(...)
block so filteredCount: 0, totalCount: a positive number (e.g., 5), and
filterActive: false to exercise the branch where items exist but the filter is
inactive; keep the rest of the assertion the same so the test validates the
"empty branch when totalCount > 0 but filter is NOT active" scenario.
In `@web/src/__tests__/pages/setup/ProvidersStep.test.tsx`:
- Around line 84-146: Tests call store methods directly
(createProviderFromPreset/fetchProviders) and thus bypass
ProvidersStep.handleAddLocal, so the component's toast-on-refresh-failure path
isn't exercised; modify the tests to drive the component UI instead of the
store: render ProvidersStep, locate and trigger the UI control that invokes
handleAddLocal (e.g., the PresetPicker add button or the same event the
PresetPickerSections would call), and then assert that a toast warning is
produced while providersError remains null; keep references to ProvidersStep,
handleAddLocal, createProviderFromPreset, fetchProviders, and
useSetupWizardStore to guide locating the affected logic.
In `@web/src/__tests__/utils/ws-sanitize.test.ts`:
- Around line 93-97: The test currently passes a plain 'completed' so it doesn't
exercise control-character stripping; update the input to include an embedded
control character (e.g., 'c\u0000ompleted' or 'comp\u0001leted') when calling
sanitizeWsEnum so that sanitizeWsEnum(TASK_STATUS, default 'created', FIELD)
actually strips the control char and returns 'completed', and ensure the
existing expectations (expect(...).toBe('completed') and
expect(warnSpy).not.toHaveBeenCalled()) remain unchanged; reference
sanitizeWsEnum, TASK_STATUS and FIELD to locate and modify the test input.
In `@web/src/components/ui/detail-nav-bar.stories.tsx`:
- Around line 10-14: The Storybook meta sets meta.args.bindShortcuts = false but
the ShortcutsBound story text claims bindShortcuts: true is "the component
default", causing a mismatch; update either meta.args.bindShortcuts to the
actual component default (set meta.args.bindShortcuts = true if the component
default is true) or change the ShortcutsBound story description to state the
actual default, and ensure any per-story args explicitly override meta.args
(e.g., ShortcutsBound.args = { bindShortcuts: true }) so the story and the
component default (refer to meta.args.bindShortcuts and the ShortcutsBound
story) are consistent.
- Around line 51-65: Add Storybook play() interaction tests for the navigation
stories: in the PositionHidden story (and the other affected stories around
lines 76-91) implement a play() that dispatches keyboard events (e.g., 'j', 'k',
'ArrowLeft', 'ArrowRight') on window and asserts the mock handlers
(onPrev/onNext) are called when position is set and not called when position is
null or when shortcuts are disabled; also dispatch events with
document.activeElement as an input/contenteditable to assert handlers are not
invoked. Locate the PositionHidden Story object and the other Story exports, add
a play() that uses window.dispatchEvent(new KeyboardEvent(...)) and
jest/expect-style assertions against the passed-in onPrev/onNext mocks to verify
listener binding and non-binding behavior.
In `@web/src/pages/approvals/ApprovalDetailDrawer.tsx`:
- Line 71: The inline validation state reasonError (managed by reasonError and
setReasonError) is only cleared on dialog close but not when the approval
context or pending state resets, causing stale errors to persist; update the
same reset logic that clears other local state when the approval or pending
flags change (the handler that resets form state when approval context resets or
pending state flips) to also call setReasonError(null) so reasonError is cleared
whenever the component resets its form/approval state.
In `@web/src/pages/ClientListPage.tsx`:
- Around line 73-85: The page is performing inline conditional rendering between
empty and filtered-empty states; replace this branching by using the shared hook
useEmptyStateProps to centralize behavior. Import useEmptyStateProps from
"@/hooks/use-empty-state-props", call it with the appropriate args (totalCount:
clients.length, filteredCount: filteredClients.length, filterActive:
Boolean(searchQuery), empty: clients.length === 0, filtered:
filteredClients.length === 0) and, if it returns props, render <EmptyState
{...props} /> and wire the action to call setSearchQuery('') where needed;
remove the existing clients/filteredClients nested ternary rendering and rely on
the hook's returned EmptyStateProps or null.
In `@web/src/pages/connections/TunnelCard.tsx`:
- Around line 20-21: Update the user-facing clipboard hint string used for
clipboard access errors (the NotAllowedError message in TunnelCard.tsx) to use a
cross-platform shortcut phrasing like "Ctrl/Cmd+C" instead of "Ctrl+C"; locate
the NotAllowedError message constant or export in TunnelCard.tsx and replace the
literal text so macOS users see the correct shortcut wording.
In `@web/src/pages/EscalationQueuePage.tsx`:
- Around line 139-156: The empty-state logic needs to treat a server-side status
filter as an active filter even when the returned escalations array is empty;
create a serverStatusFilterActive flag (e.g., const serverStatusFilterActive =
statusFilter !== null && statusFilter !== undefined), compute an effective
filter flag (e.g., const effectiveFilterActive = serverStatusFilterActive ||
priorityFilter !== 'all'), and call useEmptyStateProps with filterActive:
effectiveFilterActive and totalCount set to escalations.length ||
(serverStatusFilterActive ? 1 : 0) so the component resolves to the filtered
empty-state when a server-side status filter is active but returns zero rows;
update references to filterActive, statusFilter, priorityFilter,
emptyStateProps, useEmptyStateProps, visibleEscalations, and escalations
accordingly.
In `@web/src/pages/SettingsPage.tsx`:
- Around line 393-412: The span rendered when advancedMode is true is
non-interactive and already contains the visible text "Advanced", so remove the
redundant aria-label on that span (the element immediately following
NamespaceTabBar) to avoid conflicting accessible names; if you intended to
expose this as a dynamic state to assistive tech instead of visible text,
replace the aria-label with a proper live-region/role (e.g., role="status" or
aria-live="polite") on the same span.
In `@web/src/pages/setup/ProvidersStep.tsx`:
- Around line 100-124: fetchProviders never throws (it sets providersError in
the store), so the try/catch around await fetchProviders() in ProvidersStep
(after createProviderFromPreset) is dead and the UI still shows the hard “Failed
to load providers” banner; fix by either making fetchProviders rethrow on
failure or (recommended) check the store error after the call: call await
fetchProviders(), then read the providersError (or providers state) from the
providers store used by ProvidersStep and if an error exists, perform the
existing logging/toast code (log.warn('fetch_providers_after_create_failed', {
preset: presetName, error: getErrorMessage(providersError) }) and
useToastStore.getState().add(...)) and clear or suppress providersError so the
banner does not appear; update ProvidersStep to reference
createProviderFromPreset, fetchProviders, providersError and useToastStore
accordingly.
In `@web/src/pages/setup/SetupAgentCard.tsx`:
- Around line 71-75: The Randomize button currently calls onRandomizeName
fire-and-forget while only using the global nameSaving flag, allowing
overlapping mutations; change the behavior so the button is truly disabled
during the async mutation by adding and using a reentrancy guard: either make
onRandomizeName return a Promise and change the onClick handler to async/await
(e.g., onClick={async () => { if (isRandomizing[index]) return;
setIsRandomizing(index, true); await onRandomizeName(index);
setIsRandomizing(index, false); }}) or implement a per-index boolean state
(e.g., isRandomizing array or currentRandomizingIndex) checked before starting
the mutation and set while awaiting; update the Button disabled prop to use this
new per-index flag instead of only nameSaving and ensure onRandomizeName itself
early-returns if the flag is set to prevent concurrent writes.
In `@web/src/pages/WebhookReceiptsPage.tsx`:
- Around line 50-55: The selected connection is only seeded once via
initialFromUrl and useState, so subsequent changes to the URL query
(?connection=...) won't update the selection; add a synchronization effect that
reads searchParams.get('connection') and calls setSelected when it changes (and
only if the new value differs) so the component (symbols: useSearchParams,
initialFromUrl, selected, setSelected) updates its selection and fetch target
whenever the URL query param changes.
In `@web/src/stores/approvals.ts`:
- Around line 215-218: The runtime is rejecting unknown enum members before
sanitizeWsEnum can provide fallbacks; update the WS shape validators
(isApprovalShape and isEvidencePackageShape) to accept raw strings for the enum
fields (status, risk_level, urgency_level) instead of only the known union types
so sanitizeApproval/sanitizeEvidencePackage can run; specifically change the
checks that currently enforce concrete enum values to allow typeof value ===
'string' (or route through sanitizeWsString) for those fields, then keep
sanitizeWsEnum(s) in sanitizeApproval/sanitizeEvidencePackage to apply warnings
and fallbacks; apply the same relaxation to the other validator locations
referenced around the approval/evidence package handling so incoming unknown
enum values are sanitized rather than dropping the payload.
In `@web/src/stores/meetings.ts`:
- Around line 270-275: The shape guards isMeetingMinutesShape and isMeetingShape
currently reject unknown enum values causing sanitizeWsEnum (and its fallback)
to never run; update those guards so that for fields protocol_type and status
they only check typeof field === 'string' (not membership in
MEETING_PROTOCOL_TYPE_VALUES/MEETING_STATUS_VALUES), then let sanitizeWsEnum
(and sanitizeWsString where applicable) enforce the allowlist and fallbacks;
also ensure any new WS payload handlers that accept untrusted strings route
those fields through sanitizeWsString/sanitizeWsEnum as per guidelines.
In `@web/src/stores/messages.ts`:
- Around line 6-7: Replace the current imports of sanitizeWsEnum and
sanitizeWsString in web/src/stores/messages.ts so they come from the dedicated
utility module instead of the notifications store: locate the import statement
that references '@/stores/notifications' and update it to import sanitizeWsEnum
and sanitizeWsString from 'web/src/utils/ws-sanitize.ts' (preserving
ATTACHMENT_TYPE_VALUES import and other symbols), ensuring only the sanitizer
functions are moved to the utils module to avoid cross-store coupling and
circular init risks.
In `@web/src/stores/tasks.ts`:
- Around line 146-157: The WS task handler currently rejects whole frames
because isTaskShape() enforces that task.type, task.status, task.priority, and
task.source already match allowlists; move allowlist enforcement into
sanitizeWsEnum()/sanitizeWsString() by changing isTaskShape() (the guard used by
handleWsEvent()) to only validate those fields as string or nullish (not
membership), then call sanitizeTask() which uses
sanitizeWsEnum()/sanitizeWsString() to apply allowlist fallbacks and lengths;
update the same pattern for the other occurrences around the sanitizeWsEnum()
calls (the block at the second location) so new backend enum values are accepted
and safely normalized instead of being dropped.
In `@web/src/utils/errors.ts`:
- Around line 158-183: The toast currently conflates 401 and 403; update the
Axios status handling in errors.ts (the block using isAxiosError(error) and
error.response?.status) to return { title: 'Authentication failed' } only for
status === 401 and return a distinct title for status === 403 (e.g., { title:
'Not authorized' } or 'Authorization failed') so CRUD toasts correctly reflect
authentication vs authorization; keep existing
getErrorDetail(detail.error_category === 'auth') mapping as-is and adjust the
if/return order or branching to check 401 and 403 separately.
---
Outside diff comments:
In `@web/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsx`:
- Around line 170-183: Add an assertion that the inline field error is shown
when attempting to reject without a reason: after opening the alertdialog
(rejectDialog) and clicking its Reject button, query the reject reason
input/field within rejectDialog (e.g., by role textbox or label used in the
component) and assert that the inline validation message or error state is
present (for example check for the error text "Rejection reason required" or
that the input has aria-invalid/aria-errormessage pointing to the error). Keep
the existing toast and onReject assertions (defaultHandlers.onReject and
useToastStore checks) and scope the field lookup to rejectDialog (use
within(rejectDialog)) so the check targets the inline error, not the drawer
button.
In `@web/src/pages/setup/SkipWizardForm.tsx`:
- Around line 30-55: The current try/catch allows wizardCompleteSetup() to run
even if submitCompany() failed; you must instead check the post-submit store
state or submitCompany's return contract before proceeding and avoid wrapping
store mutation calls in try/catch. After calling submitCompany(), read
useSetupWizardStore.getState().companyResponse (or the store's explicit success
flag/err slot) and only call wizardCompleteSetup(),
setCompanyNameStore(trimmed), useSetupStore.setState({ setupComplete: true }),
show the success toast and navigate('/') when the store indicates the company
was actually created; if the store reports an error, surface that via the
store-owned error slots (and setError from those slots) rather than relying on
catch to choose the branch. Ensure no store mutation is wrapped in try/catch per
the guideline and rely on useSetupWizardStore state to drive the UI flow.
In `@web/src/pages/UsersPage.tsx`:
- Around line 109-136: Move the ErrorBanner so it is rendered immediately after
the ListHeader in UsersPage: currently ErrorBanner is placed after the
SearchFilterSort component; change the JSX order so ListHeader is followed
directly by ErrorBanner (keeping the same props: severity, title, description,
onRetry that calls fetchUsers), and then render SearchFilterSort and the rest
(SearchInput uses searchQuery and setSearchQuery) to satisfy the project layout
contract.
In `@web/src/stores/messages.ts`:
- Around line 121-123: Replace the unsafe enum casts for message.type and
message.priority (currently using sanitizeWsString(...) as EnumType) with
allowlist-validated calls to sanitizeWsEnum; specifically, pass the sanitized
string into sanitizeWsEnum<T>(value, allowlist, fallback, { field:
'type'|'priority' }) using the correct enum allowlists and sensible fallback
values, mirroring how attachments[].type is validated, so unknown WS values are
clamped instead of being cast through.
In `@web/src/stores/setup-wizard/providers.ts`:
- Around line 64-74: fetchProviders currently clears providersError but not
providersWarning, so a successful refresh leaves the persistent warning banner
visible; update the async fetchProviders function to also clear providersWarning
on success (e.g., call set with providersWarning: null alongside providers and
providersLoading:false) after listProviders resolves, while leaving the existing
error handling intact that sets providersError and providersLoading on catch;
locate the fetchProviders method and add providersWarning: null to the
successful set call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f2b9816-69e5-4f17-af20-228becff2d2c
📒 Files selected for processing (72)
docs/reference/web-package-structure.mdweb/CLAUDE.mdweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/__tests__/stores/artifacts.test.tsweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/api/types/approvals.tsweb/src/api/types/enums.tsweb/src/api/types/messages.tsweb/src/api/types/websocket.tsweb/src/components/layout/StatusBar.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/components/ui/error-boundary.tsxweb/src/components/ui/progress-gauge.tsxweb/src/components/ui/skeleton.tsxweb/src/hooks/use-empty-state-props.tsweb/src/mocks/handlers/helpers.tsweb/src/mocks/handlers/index.tsweb/src/pages/ApprovalsPage.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/BudgetPage.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/EscalationQueuePage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/SettingsPage.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/workflows/WorkflowFilters.tsxweb/src/stores/approvals.tsweb/src/stores/artifacts.tsweb/src/stores/connections/crud-actions.tsweb/src/stores/meetings.tsweb/src/stores/messages.tsweb/src/stores/notifications.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/setup-wizard/providers.tsweb/src/stores/setup-wizard/types.tsweb/src/stores/tasks.tsweb/src/styles/status-colors.tsweb/src/utils/errors.tsweb/src/utils/ws-sanitize.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Preview
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Dashboard Test
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (12)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Reuse components fromweb/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, Motion transitions, or BCP 47 locale strings; use design tokens,@/lib/motionpresets, and the helpers in@/utils/format. Enforced byscripts/check_web_design_system.py(PostToolUse hook on everyweb/src/edit). Seeweb/CLAUDE.mdfor the component inventory, token rules, and post-training references (TS6, Storybook 10).
Never hardcode BCP 47 locale strings or call bare.toLocaleString()/.toLocaleDateString()/.toLocaleTimeString(). Use helpers in@/utils/formatwhich readgetLocale()from@/utils/locale.
Currency: never hardcode ISO 4217 codes or symbols. Frontend:DEFAULT_CURRENCYfrom@/utils/currenciesoruseSettingsStore().currency. Field naming: no_usdsuffix on money fields anywhere.
Timezone: store UTC; render viaIntlwithout passingtimeZone(browser tz wins). Date / number format: always viaIntl; no hand-rolled templates. Units: metric only.Always use
createLoggerfrom@/lib/loggerwith variable namelog; never use bareconsole.warn/console.error/console.debugin application code (onlylogger.tsmay use bare console methods)Pass dynamic/untrusted values as separate arguments to logger methods, not interpolated into the message string, so they go through
sanitizeArgWrap attacker-controlled fields inside structured objects with
sanitizeForLog()before embedding in log callsCallers MUST NOT wrap store mutation calls in
try/catch; the store owns the error UXDisplay counts must come from
data.length; the wire envelope no longer carriestotalAny new caller of health/readiness endpoints must handle the 503 path explicitly
Use
sanitizeWsString()to clamp every WS-supplied string (strips C0 controls + bidi-overrides + caps length); usesanitizeWsEnum<T>(value, allowlist, fallback, { field })for enum validationAny new WS payload...
Files:
web/src/api/types/messages.tsweb/src/stores/setup-wizard/types.tsweb/src/hooks/use-empty-state-props.tsweb/src/components/ui/progress-gauge.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/components/ui/skeleton.tsxweb/src/mocks/handlers/index.tsweb/src/pages/BudgetPage.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/stores/tasks.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/pages/ClientListPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/setup/WizardShell.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/artifacts.test.tsweb/src/pages/workflows/WorkflowFilters.tsxweb/src/styles/status-colors.tsweb/src/pages/ProvidersPage.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/components/layout/StatusBar.tsxweb/src/stores/connections/crud-actions.tsweb/src/pages/setup/CompleteStep.tsxweb/src/pages/DashboardPage.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/setup/WizardNavigation.tsxweb/src/mocks/handlers/helpers.tsweb/src/pages/BudgetForecastPage.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/api/types/approvals.tsweb/src/utils/errors.tsweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/SettingsPage.tsxweb/src/stores/artifacts.tsweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/WorkflowsPage.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/pages/setup/CompanyStep.tsxweb/src/components/ui/error-boundary.tsxweb/src/stores/meetings.tsweb/src/stores/notifications.tsweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/__tests__/utils/errors.test.tsweb/src/stores/messages.tsweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/stores/setup-wizard/providers.tsweb/src/api/types/enums.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/stores/approvals.tsweb/src/pages/ApprovalsPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/api/types/websocket.tsweb/src/pages/UsersPage.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/ConnectionsPage.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/pages/EscalationQueuePage.tsx
web/**/*.{ts,tsx,js,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: see
web/CLAUDE.mdfor commands, design system, and component inventory. The CI-matching full-suite leak check isnpm --prefix web run test -- --coverage --detect-async-leaks(theLeaks N leakssummary must stay at or below theMAX_ASYNC_LEAKSceiling in.github/workflows/ci.yml; any new store that schedules timers must expose a teardown hook perweb/CLAUDE.md).
Files:
web/src/api/types/messages.tsweb/src/stores/setup-wizard/types.tsweb/src/hooks/use-empty-state-props.tsweb/src/components/ui/progress-gauge.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/components/ui/skeleton.tsxweb/src/mocks/handlers/index.tsweb/src/pages/BudgetPage.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/stores/tasks.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/pages/ClientListPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/setup/WizardShell.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/artifacts.test.tsweb/src/pages/workflows/WorkflowFilters.tsxweb/src/styles/status-colors.tsweb/src/pages/ProvidersPage.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/components/layout/StatusBar.tsxweb/src/stores/connections/crud-actions.tsweb/src/pages/setup/CompleteStep.tsxweb/src/pages/DashboardPage.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/setup/WizardNavigation.tsxweb/src/mocks/handlers/helpers.tsweb/src/pages/BudgetForecastPage.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/api/types/approvals.tsweb/src/utils/errors.tsweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/SettingsPage.tsxweb/src/stores/artifacts.tsweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/WorkflowsPage.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/pages/setup/CompanyStep.tsxweb/src/components/ui/error-boundary.tsxweb/src/stores/meetings.tsweb/src/stores/notifications.tsweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/__tests__/utils/errors.test.tsweb/src/stores/messages.tsweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/stores/setup-wizard/providers.tsweb/src/api/types/enums.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/stores/approvals.tsweb/src/pages/ApprovalsPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/api/types/websocket.tsweb/src/pages/UsersPage.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/ConnectionsPage.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/pages/EscalationQueuePage.tsx
web/src/api/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
getLiveness()is always 200 while the process is alive;getReadiness()is 200 healthy / 503 unavailable (binary outcome, no tri-state)
Files:
web/src/api/types/messages.tsweb/src/api/types/approvals.tsweb/src/api/types/enums.tsweb/src/api/types/websocket.ts
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
All store mutation actions (create / update / delete) must follow the
stores/connections/crud-actions.tspattern: wrap API calls intry/catch, update state + emit success toast on success, log + emit error toast + return sentinel on failureOptimistic mutations must capture
previousstate synchronously and restore it in thecatchblockList reads (
fetch*) must seterror: string | nullon the store instead of toastingCursor pagination: list endpoints use opaque cursor-based paging via
PaginationMeta; stores must keepnextCursor+hasMorein state and early-return when!hasMore || !nextCursorAny new store that schedules timers or attaches event listeners must expose an equivalent cleanup hook and register it in the global test
afterEach
Files:
web/src/stores/setup-wizard/types.tsweb/src/stores/tasks.tsweb/src/stores/connections/crud-actions.tsweb/src/stores/artifacts.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/meetings.tsweb/src/stores/notifications.tsweb/src/stores/messages.tsweb/src/stores/setup-wizard/providers.tsweb/src/stores/approvals.ts
web/src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use Base UI's
renderprop as the polymorphism primitive throughout the dashboard; only the local<Slot>helper incomponents/ui/slot.tsxshould use@base-ui/react/merge-propsfor<Button asChild>ergonomicsBase UI primitives are imported directly from
@base-ui/react/<subpath>and use the nativerenderprop for polymorphism; the local<Slot>helper is reserved for<Button asChild>Props interface name must be
<ComponentName>Propsand exported from the same file (e.g.,AgentCardPropsinagent-card.tsx) for greppability and extensibilityEvery shared UI component must have a sibling
<ComponentName>.stories.tsxcovering every meaningful state (default, hover, loading, error, empty, disabled where applicable)
Files:
web/src/components/ui/progress-gauge.tsxweb/src/components/ui/skeleton.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/components/ui/error-boundary.tsx
web/src/components/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
ALWAYS reuse existing components from
web/src/components/ui/before creating new ones
Files:
web/src/components/ui/progress-gauge.tsxweb/src/components/ui/skeleton.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/components/layout/StatusBar.tsxweb/src/components/ui/error-boundary.tsx
web/src/components/ui/**/*.{tsx,stories.tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Every new shared component lives in
web/src/components/ui/with a sibling.stories.tsxcovering all states
Files:
web/src/components/ui/progress-gauge.tsxweb/src/components/ui/skeleton.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/components/ui/error-boundary.tsx
web/src/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<StatusBadge>for status dots (defaults torole="img"with aria-label; usedecorativefor adjacent-labeled,announcefor live WS updates)Use
<MetricCard>/<Sparkline>/<ProgressGauge>/<TokenUsageBar>for KPI displaysUse
<SectionCard>for titled card wrappers with icon and action slot;<AgentCard>,<DeptHealthBar>for domain-specific cardsUse
<InputField>/<SelectField>/<SliderField>/<ToggleField>/<SegmentedControl>/<TagInput>/<SearchInput>for form fieldsUse
<Drawer width="compact|narrow|default|wide">for slide-in panels (Base UI; do NOT add inlinew-[40vw]overrides)Use
<Skeleton>family /<EmptyState>/<ErrorBoundary>/<ErrorBanner>/<ProgressIndicator>for loading / empty / error statesUse
STATUS_COLORSfamily from@/styles/status-colors(typedRecord<EnumValue, string>lookups; no inline constants per page)Use
<ConfirmDialog>/<Toast>(Zustand-backed queue, NOT Base UI's Toast) for confirmation dialogs and toast notificationsUse
<CommandPalette>/<KeyboardShortcutHint>/<CommandCheatsheet>for Cmd+K / shortcutsUse
<AnimatedPresence>/<StaggerGroup>/<LiveRegion>(debounced ARIA live for WS updates) for animationsUse Dialog, AlertDialog, Popover, Tabs, Menu, Drawer, CSPProvider, merge-props directly from
@base-ui/react/<subpath>(adopted primitives)Do NOT use Base UI's Toast (use Zustand-backed queue instead), Meter (use
<ProgressGauge>), Select (use native<select>for mobile), Combobox / Autocomplete / OTP Field / Tooltip (no current call sites)
Files:
web/src/components/ui/progress-gauge.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/BudgetPage.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/setup/WizardShell.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/DashboardPage.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/SettingsPage.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/ApprovalsPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/UsersPage.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/ConnectionsPage.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/pages/EscalationQueuePage.tsx
web/src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Boot tests with
onUnhandledRequest: 'error'; override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*')
Files:
web/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/artifacts.test.tsweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/stores/setup-wizard.test.ts
web/src/pages/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<ListHeader>/<SearchFilterSort>/<Pagination>/<BulkActionBar>/<MetadataGrid>/<Breadcrumbs>for list-page primitivesRoot container on list pages uses
space-y-section-gap(majority pattern;flex flex-col gap-section-gapdiscouraged);<ErrorBanner>lands immediately after<ListHeader>, before filter/pagination rowsPages with a one-line mission statement pass it via
<ListHeader description="..." />For list layout: use Kanban grouping for status-flow domains where each column conveys lifecycle phase (Tasks, Requests); use flat scrollable list for queues without explicit phase semantics (Escalations, Approvals)
Use
useEmptyStateProps({ filteredCount, totalCount, filterActive, empty, filtered })from@/hooks/use-empty-state-propsto returnEmptyStateProps | nullfor conditional rendering
Files:
web/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/BudgetPage.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/SettingsPage.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/ApprovalsPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/UsersPage.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/EscalationQueuePage.tsx
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
MSW handlers in
web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpointUse typed envelope helpers (
successFor,paginatedFor,voidSuccess) in MSW handlers to keep handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/index.tsweb/src/mocks/handlers/helpers.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use D2 (
\``d2) for architecture diagrams, nested container layouts, complex entity relationships. Use Mermaid (```mermaid) for flowcharts, sequence diagrams, simple hierarchies, pipelines. Use Markdown tables for grid/matrix data that is semantically tabular. Never use```textblocks with ASCII/Unicode box-drawing characters for diagrams. D2 uses theme 200 (Dark Mauve), configured globally inmkdocs.yml. Review agentdiagram-syntax-validatorruns in/pre-pr-reviewand/aurelio-review-pr` pipelines.
Files:
docs/reference/web-package-structure.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T12:34:54.548Z
Learning: CI fails if `vitest --detect-async-leaks` reports more than `MAX_ASYNC_LEAKS` (currently 90); local floor 49, CI baseline 77-80; raise ceiling only with documented per-PR justification
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T12:34:54.548Z
Learning: Bump the WS protocol version on both client and server together for breaking payload changes
| it('does not toast or set error when create succeeds and refresh succeeds', async () => { | ||
| server.use( | ||
| http.post('/api/v1/providers/from-preset', () => | ||
| HttpResponse.json(apiSuccess(defaultProvider('local-x')), { status: 201 }), | ||
| ), | ||
| http.get('/api/v1/providers', () => | ||
| HttpResponse.json( | ||
| apiSuccess({ data: [{ ...defaultProvider('local-x'), name: 'local-x' }] }), | ||
| ), | ||
| ), | ||
| ) | ||
|
|
||
| render(<ProvidersStep />) | ||
|
|
||
| // Wait for the preset picker to show the detected local row, then | ||
| // trigger the auto-add via the store call (simulates the | ||
| // PresetPickerSections onAddLocal handler the picker would call). | ||
| await waitFor(() => { | ||
| expect(useSetupWizardStore.getState().presets.length).toBeGreaterThan(0) | ||
| }) | ||
| const result = await useSetupWizardStore | ||
| .getState() | ||
| .createProviderFromPreset('local-x', 'local-x', undefined, 'http://localhost:11434') | ||
|
|
||
| expect(result).toEqual({ ok: true }) | ||
| expect(useToastStore.getState().toasts).toHaveLength(0) | ||
| expect(useSetupWizardStore.getState().providersError).toBeNull() | ||
| }) | ||
|
|
||
| it('toasts a warning and leaves providersError null when refresh fails after a successful create', async () => { | ||
| server.use( | ||
| http.post('/api/v1/providers/from-preset', () => | ||
| HttpResponse.json(apiSuccess(defaultProvider('local-x')), { status: 201 }), | ||
| ), | ||
| // fetchProviders is the listProviders endpoint -- make it fail | ||
| http.get('/api/v1/providers', () => HttpResponse.json(apiError('list boom'))), | ||
| ) | ||
|
|
||
| render(<ProvidersStep />) | ||
| await waitFor(() => { | ||
| expect(useSetupWizardStore.getState().presets.length).toBeGreaterThan(0) | ||
| }) | ||
|
|
||
| // Drive handleAddLocal directly via the store + caller's wrapper | ||
| // pattern: create succeeds, then fetchProviders is called and | ||
| // fails. The current ProvidersStep handler swallows the | ||
| // fetchProviders error into a toast so the create's error banner | ||
| // stays clean. | ||
| const createResult = await useSetupWizardStore | ||
| .getState() | ||
| .createProviderFromPreset('local-x', 'local-x', undefined, 'http://localhost:11434') | ||
| expect(createResult).toEqual({ ok: true }) | ||
| // Now trigger fetchProviders which is what handleAddLocal | ||
| // would invoke after a successful create. | ||
| await useSetupWizardStore.getState().fetchProviders() | ||
|
|
||
| // The store's fetchProviders sets providersError on failure; | ||
| // ProvidersStep's handleAddLocal would also toast in addition -- | ||
| // both behaviours are valid recovery surfaces. We assert at least | ||
| // one of them carries the failure so the operator is informed. | ||
| const state = useSetupWizardStore.getState() | ||
| expect(state.providersError).toMatch(/list boom/i) | ||
| }) |
There was a problem hiding this comment.
These cases bypass handleAddLocal, so they don't validate the new UI recovery flow.
They call store methods directly instead of triggering the add-local action the component wires up. As a result, the post-create refresh/toast branch is never exercised, and the refresh-failure case currently locks in providersError rather than the intended warning-only surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/pages/setup/ProvidersStep.test.tsx` around lines 84 - 146,
Tests call store methods directly (createProviderFromPreset/fetchProviders) and
thus bypass ProvidersStep.handleAddLocal, so the component's
toast-on-refresh-failure path isn't exercised; modify the tests to drive the
component UI instead of the store: render ProvidersStep, locate and trigger the
UI control that invokes handleAddLocal (e.g., the PresetPicker add button or the
same event the PresetPickerSections would call), and then assert that a toast
warning is produced while providersError remains null; keep references to
ProvidersStep, handleAddLocal, createProviderFromPreset, fetchProviders, and
useSetupWizardStore to guide locating the affected logic.
| export const PositionHidden: Story = { | ||
| args: { | ||
| canPrev: true, | ||
| canNext: true, | ||
| position: null, | ||
| }, | ||
| parameters: { | ||
| docs: { | ||
| description: { | ||
| story: | ||
| 'When `position` is `null` (deep link / refresh without list context) the bar renders nothing. The component returns `null` and binds no keyboard listeners.', | ||
| }, | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add play() verification for keyboard shortcut + PositionHidden listener behavior.
The stories describe (a) window-level J/ArrowLeft/K/ArrowRight shortcuts and (b) that PositionHidden returns null and binds no keyboard listeners, but they don’t actually verify those behaviors via play() (or interaction tests). Consider adding lightweight Storybook play() steps that:
- dispatch keyboard events on
window, - assert
onPrev/onNextcalls when shortcuts should work, - assert no calls when
position: nulland/or when shortcuts are disabled, - optionally ensure events don’t fire when target is
input/contenteditable.
Also applies to: 76-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/detail-nav-bar.stories.tsx` around lines 51 - 65, Add
Storybook play() interaction tests for the navigation stories: in the
PositionHidden story (and the other affected stories around lines 76-91)
implement a play() that dispatches keyboard events (e.g., 'j', 'k', 'ArrowLeft',
'ArrowRight') on window and asserts the mock handlers (onPrev/onNext) are called
when position is set and not called when position is null or when shortcuts are
disabled; also dispatch events with document.activeElement as an
input/contenteditable to assert handlers are not invoked. Locate the
PositionHidden Story object and the other Story exports, add a play() that uses
window.dispatchEvent(new KeyboardEvent(...)) and jest/expect-style assertions
against the passed-in onPrev/onNext mocks to verify listener binding and
non-binding behavior.
… shared status colours (#1710) Wave 1 of the audit-cleanup E bundle. Adds the cross-cutting primitives the remaining waves build on. * Typed WS sanitizer in web/src/utils/ws-sanitize.ts: sanitizeWsEnum(value, allowlist, fallback, opts) validates a WS-sourced string against an enum allowlist. On unknown values (sanitisation strips to empty, or server emits a value not yet known to this client), emits a structured ws.enum.unknown warning and returns the supplied fallback so rolling backend deploys cannot break UI rendering. Replaces 13 unsafe cast sites in approvals, meetings, messages, and tasks stores. Test file: 14 cases covering valid pass-through, control-char strip, bidi-override strip, allowlist miss, empty/whitespace fallback, custom maxLen, and literal-type inference. * Typed WS payload contract in web/src/api/types/websocket.ts: WsEventPayloadMap discriminates payload shape by event_type for all 62 WsEventType members; WsTypedEvent and WsEventOf tie envelope to payload. Mirror of src/synthorg/api/ws_payloads/_lifecycle.py and _domain.py. Compile-time exhaustiveness guard catches drift if either union changes without the other. The dispatch loop continues to deliver the existing WsEvent so per-store migrations to the typed view roll out incrementally; backend migration #1711 has a stable target. * Shared badge / pill colour module web/src/styles/status-colors.ts: ROLE_BADGE_COLORS and ESCALATION_STATUS_BADGE_COLORS replace inline ROLE_PILL_CLASS and STATUS_BADGE_CLASS constants in UsersPage and EscalationQueuePage. Approval-domain helpers in @/utils/approvals stay unchanged (already centralised via SemanticColor abstraction). Runtime test pins enum coverage and forbids hex literals. * Adds missing _VALUES tuples: ATTACHMENT_TYPE_VALUES (messages.ts), SIGNATURE_ALGORITHM_VALUES (approvals.ts), TASK_SOURCE_VALUES (enums.ts). Resolves the WS enum-cast pattern subset of audit agent 109 (highest-leverage piece of 139 TS strictness findings) and audit agent 52 finding #4.
Wave 2 of the audit-cleanup E bundle. A11y (audit agent 35): * progress-gauge.tsx -- adds SVG title element so screen readers that expose SVG content announce the meter context. Wrapping div already carries role=meter + aria-label; the title is redundant but robust. * error-boundary.tsx -- replaces the bare button at the component level with the design-system Button (variant=link, size=sm). Inherits focus-visible ring + WCAG-AA contrast + hover state from the system. * StatusBar.tsx -- documents the implicit color-plus-label pairing contract on resolveCombinedStatus. The existing render at the health-pill site already pairs visible label text with the colored Dot via aria-label and aria-live. Responsive (audit agent 53): * AgentSpendingTable.tsx -- min-w-[40rem] becomes md:min-w-[40rem] so the 768-1024px tablet range falls back to natural width within the existing overflow-x-auto wrapper. * SettingsPage.tsx -- SettingsActionCard's right column moves from a hard w-56 to w-full max-w-[14rem] shrink-0 md:w-56 so the grid responds to container shrinkage. Perf (audit agent 54): * TaskHistory.tsx -- wraps sorted and maxDurationMs in useMemo so a parent re-render with the same tasks reference skips Date parsing and the sort/reduce passes. * ApprovalCard, ChannelListItem, ActivityFeedItem -- React.memo wrappers. ApprovalCard runs flash + countdown effects per row in long lists; ChannelListItem renders inside a sidebar that updates on activeChannel + unreadCounts; ActivityFeedItem re-renders the whole feed on every WS event. * BudgetPage / BudgetForecastPage / DashboardPage -- recharts-using charts (SpendBurnChart, CostBreakdownChart, BudgetBurnChart) become React.lazy imports wrapped in Suspense with a new SkeletonChart fallback. Vite splits a separate chunk for recharts; first chart render triggers the import. * skeleton.tsx -- adds SkeletonChart variant sized via aspect-ratio so the layout does not shift when ResponsiveContainer measures the parent post-hydration. Test infrastructure adjustments: * progress-gauge tests -- the new SVG title duplicates the visible percentage text in the no-label case; tests switch from getByText to getAllByText with a length-greater-than guard to be agnostic to the additional title node. * BudgetPage / DashboardPage tests -- chart section titles render only after Suspense resolves the lazy import; the four affected tests switch to async findByText with a 5s timeout for parallel-load resilience.
Wave 3 of the audit-cleanup E bundle.
Storybook (audit agent 57):
* detail-nav-bar.stories.tsx -- new file. Seven stories cover the
prop combinations the audit called out: Default, FirstItem,
LastItem, PositionHidden (component returns null), WithLabel,
ShortcutsBound, BothDisabled. tags: [autodocs] per the breadcrumbs
story convention.
UX consistency (audit agent 52):
* useEmptyStateProps hook (web/src/hooks/use-empty-state-props.ts) --
derives the EmptyStateProps for the two branches every list page
juggles ("no data ever" vs "no data after filter") so the
discriminator is a single hook call instead of two parallel
conditional renders. Wired into ApprovalsPage and
EscalationQueuePage.
* ListHeader description prop applied to WorkflowsPage,
ConnectionsPage, BudgetPage, ProvidersPage, EscalationQueuePage,
UsersPage so each list page's mission statement is one line under
the title.
* ProvidersPage / ConnectionsPage / UsersPage root container --
flex flex-col gap-section-gap becomes space-y-section-gap to
match the majority pattern across the dashboard.
* ConnectionsPage error banner -- moved to immediately after
ListHeader (was after TunnelCard) so the page-level error slot
matches OauthAppsPage.
* UsersPage layout -- list becomes a 1/2/3-column grid mirroring
ClientListPage's resource-grid shape; per-card header keeps the
Grant role mutation prominent.
CLAUDE.md (web/) updates:
* Documents the list-page primitive conventions: space-y-section-gap
root, ErrorBanner-after-ListHeader, ListHeader description for
pages with a one-line mission statement, Kanban vs flat-list
layout choice based on whether the domain has explicit lifecycle
phases.
* Documents useEmptyStateProps as the canonical empty-state
derivation surface.
* Documents the @/styles/status-colors module as the canonical
badge / role / risk / urgency lookup surface (no inline
Record<EnumValue, string> per page).
Final wave of the audit-cleanup E bundle.
Dashboard UX (audit agent 47, 9 findings):
* WorkflowsPage -- Clear filters action now re-focuses the search
input via a forwarded ref, so the user can immediately re-filter
without a second click.
* EscalationQueuePage -- new Sort by SegmentedControl (Newest /
Priority / Conflict type) layered on top of the status + priority
filters; sort runs client-side on the existing memoised list.
* ProvidersPage -- preset picker already implements progressive
disclosure via the More providers via LiteLLM details collapse;
the audit's "if presets exceed 12" concern is addressed by the
server-driven is_featured flag.
* RequestQueuePage -- ListHeader description documents the
Submitted -> Triaging -> Scoping -> Approved -> Task created
pipeline. A generic "Move to arbitrary status" menu would require
backend support for unrestricted transitions, which the request
state machine deliberately does not provide -- the per-card
scope/approve/reject buttons are the legal next-transition surface.
* WebhookReceiptsPage / ConnectionsPage -- ConnectionCard renders a
View webhook receipts cross-link that pre-selects the connection
via the new ?connection= query param the receipts page reads on
mount.
* ClientListPage -- new SearchInput filters clients by name or
persona client-side; empty-state copy distinguishes "no clients"
from "no matching clients".
* ApprovalsPage -- the four risk-count metric cards become buttons
that toggle the matching risk_level filter (aria-pressed exposes
the toggle state, ring-2 ring-accent surfaces the active card).
* TaskBoardPage -- the column containing the currently-selected task
gets a subtle bg-accent/5 + ring highlight via TaskColumn's new
highlighted prop, so the detail-drawer-open task is visually
mapped back to its lifecycle phase on the board.
* SettingsPage -- when advancedMode is on, a small Advanced badge
renders next to the namespace tabs in addition to the existing
AdvancedModeBanner so the mode signal stays visible after dismiss.
Onboarding (audit agent 59, 7 findings):
* createProviderFromPreset returns a result-object
({ ok: true, warning? } | { ok: false, error }) instead of
throwing. ProvidersStep branches on ok before calling
fetchProviders, and a fetch failure now toasts (warning) instead
of poisoning the step's error banner.
* SetupAgentCard tracks the in-flight name save and disables the
adjacent Randomize button so a manual edit cannot race a randomize
click and have the slower request win.
* CompanyStep clears companyError + companyErrorCode on unmount so a
stale tier-coverage error from a prior Apply does not persist when
the user returns from Providers.
* CompleteStep's ErrorBanner appends a contextual recovery hint
("If you see this repeatedly, setup may have already completed;
refresh the page to confirm.") so fatal errors offer a path
forward instead of a Retry-only loop.
* SkipWizardForm distinguishes "company create failed" from "company
created, setup completion failed" and surfaces the partial-success
recovery message with the option to finish via the wizard's
Complete step.
* WizardNavigation accepts a nextDisabledReason caption rendered
under the disabled Next button so the user knows WHY they cannot
advance ("Complete the required fields on this step to continue.").
* CompanyStep Company Name InputField gains a hint
("Max 200 characters. Apply Template stays disabled until this is
valid.") that sets expectations up front; the existing error fires
only on boundary breach so the two never co-render.
Error messages (audit agent 58, 11 frontend findings):
* utils/errors.ts 409 message softened to "The resource state
changed. Refresh the page and try again." -- covers
optimistic-concurrency, duplicate-resource, and version-mismatch.
* artifacts.ts partial-load error restructured: title + actionable
next step, with the raw backend message routed through
getErrorMessage so JSON / stack traces cannot leak.
* setup-wizard/providers.ts model-discovery messages re-routed
through getErrorMessage and re-shaped as title + description; both
the no-models-found and discovery-failed branches surface as
warnings rather than errors (provider WAS created).
* useTaskActionHandlers cancellation reason: "Cancellation requires
a reason for audit. Please enter why you are cancelling this task."
* ApprovalDetailDrawer + ApprovalsPage rejection reason: "Rejection
requires a reason for the approval record. Provide a brief
explanation."
* OauthAppCard reveal/copy toasts re-titled "Failed to reveal client
secret" / "Failed to copy client ID" so the operator knows what
the operation was without reading the description.
* useWorkflowEditorCallbacks validation-warning message uses past
tense + actionable phrasing: "Validation found N error(s).
Review the marked fields below."
* TunnelCard clipboard error discriminates NotAllowedError
(permission denied -- "Use Ctrl+C to copy manually") from
other failures (API not available -- "Copy manually from the
Public URL field").
Test infrastructure: ApprovalDetailDrawer test updated to match the
new toast title.
Two unit tests asserted on the literal pre-#1710 wording of the 409 HTTP message and the artifacts partial-load detail message. The copy was reworded in wave 4 to soften 409 (no longer assumes optimistic-concurrency) and to restructure the artifacts message as title + recovery hint. The tests now assert on the new wording's stable substring.
…, error-code routing (#1710) 19 findings from 9 review agents, all addressed. CRITICAL (silent-failure-hunter): * setup-wizard providers store -- new providersWarning slot replaces the misuse of providersError for "provider created OK but discovery empty/failed" cases. ProvidersStep renders a separate warning banner for the new slot so a successfully-created provider is no longer surfaced as an error. * SkipWizardForm -- partial-success discrimination now reads the store snapshot (companyResponse !== null) instead of a local flag set between awaits. The local flag could race a throw between resolution and assignment and produce the wrong recovery message. * ProvidersStep handleAddLocal -- fetchProviders failure now logs via createLogger before toasting, so a dismissed toast leaves an observability trace for the operator. MAJOR: * setup-wizard.test.ts -- four new tests pin createProviderFromPreset return shape: { ok: true } on full success, { ok: true, warning } on no-models / discovery-throw, { ok: false, error } on create failure. A revert to throw-on-failure would now break the test. * New ProvidersStep.test.tsx -- four integration tests cover the refresh-success, refresh-failure (warning toast + clean error banner), create-failure (error banner with title), and discovery-empty (separate warning banner) paths. * WorkflowFilters comment -- removed the (see audit 47#1) callout in favour of the technical WHY (refocus the search input after clearing filters). * web/CLAUDE.md -- WS sanitization paragraph updated to cite the actual source module @/utils/ws-sanitize, document sanitizeWsEnum<T> with the call-shape contract, and note that raw "as EnumType" casts are forbidden. * docs/reference/web-package-structure.md -- hooks inventory now lists useEmptyStateProps; styles inventory now lists the typed status-colour lookups. * mocks/handlers/_helpers.ts -- new buildValidationError(fields[], overrides?) helper consolidates the inconsistent Missing required fields / Field 'name' is required / Fields 'a' and 'b' are required mock-error wording into one Validation error: a, b are required. shape. * ApprovalDetailDrawer -- reason field now renders an inline error (red border + helper text) on validation failure, in addition to the toast. Inline error clears as soon as the user types. * utils/errors.ts -- new getCrudErrorTitle(err, fallback) routes on the structured error_category (auth / validation / conflict / rate_limit / not_found / budget_exhausted) and HTTP-status fallbacks to pick a category-aware toast title prefix. Wired into stores/connections/crud-actions.ts and stores/providers/crud-actions.ts so generic "Failed to create connection" titles become "Authentication failed" / "Resource conflict" / etc when the backend supplies a structured error code. MEDIUM: * sanitizeWsEnum -- the field option is now mandatory (no longer defaults to 'unknown'), so the ws.enum.unknown warning always carries enough diagnostic context to identify which field drifted. All 13 existing call sites already pass field; no caller edits required. * New use-empty-state-props.test.ts -- five tests cover the three branches (filteredCount > 0 returns null, totalCount > 0 with filterActive returns filtered, empty otherwise) plus memoisation identity-stability with stable inputs. * ws-sanitize surrogate-pair test -- tightened to pin exact content (toBe instead of length-only) so a mid-codepoint truncation that drops a surrogate half cannot pass silently. * TunnelCard clipboard -- DOMException routing now handles NotAllowedError, SecurityError, InvalidStateError, AbortError, NotFoundError each with a specific recovery hint, instead of bucketing every non-permission failure into "API not available". * CompanyStep cleanup useEffect -- guards against racing an in-flight submit (companyLoading === true) so a long-running request that errors after unmount doesn't get its own error nulled out. * api/types/websocket.ts -- typed payload contract now carries an explicit sanitisation contract comment: every string field is attacker-reachable; consumers must route through sanitizeWsString or sanitizeWsEnum before display or persistence. * BudgetPage / DashboardPage tests -- 5s findByText timeout now documented as a parallel-load band-aid; if these tests start consistently approaching 5s, treat as a performance regression in the lazy-import strategy rather than raising the timeout. Verification: * npm run lint: zero warnings * npm run type-check: clean * 2997 unit tests pass (was 2983; +14 new) * 75 async leaks (under MAX_ASYNC_LEAKS=90 ceiling) * npm run storybook:build: clean * npm run build: recharts splits into vendor-charts chunk (~378 KB)
…1710) External reviewers (CodeRabbit, gemini-code-assist) plus the CI Dashboard Test async-leak gate flagged 18 findings on the first push of #1718. All addressed. CI gate fix: * The pre-PR-review push added 14 new tests; CI's leak detector reported 95 leaks (above the MAX_ASYNC_LEAKS=90 ceiling) because of extra ProvidersStep renders. Trimmed ProvidersStep.test.tsx from 4 renders to 1: the unique-value-add is the warning-banner DOM check; the rest of the store-level coverage already lives in setup-wizard.test.ts. Forward-compat WS sanitizer (3 reviews flagged the same root cause): * approvals / meetings / tasks shape guards were rejecting unknown enum values BEFORE sanitizeWsEnum could apply its allowlist + safe fallback. A backend rolling out a new enum value would drop the whole frame instead of the documented warning + fallback. Relaxed isApprovalShape / isMeetingShape / isTaskShape to typeof === 'string' for the enum fields routed through sanitizeWsEnum (status, risk_level, urgency_level, protocol_type, type, priority, source). Removed now-unused per-store *_SET constants. Test "rejects frames whose status is outside the enum" updated to assert the new accept-and-fallback contract. ProvidersStep recovery flow (CodeRabbit MAJOR x2): * fetchProviders swallows its own errors via the providers store, so the try/catch around it was dead code -- a refresh failure after a successful create still surfaced the hard "Failed to load providers" banner. Rewired to read providersError from the store post-call, clear it, and toast a warning instead. The successfully-created provider state is no longer misrepresented as a failed create. ApprovalDetailDrawer reasonError reset: * reasonError was cleared on dialog close but not when the approval context resets or the pending state flips, leaving stale inline validation between approval switches. Cleared in both reset paths. Random-name race (SetupAgentCard): * onRandomizeName was fire-and-forget; rapid clicks enqueued concurrent writes with last-response-wins. Added randomizeSaving guard and awaited the call so the button stays disabled across the in-flight window. Smaller findings: * useEmptyStateProps test totalCount-greater-than-zero actually used totalCount: 0; set to 5 so the test exercises the named branch. * ApprovalsPage useEmptyStateProps: filteredCount now uses filtered.length (post-filter item count) instead of grouped.size (risk-bucket count, max 4 -- semantically wrong). * ws-sanitize control-char test: input now embeds U+0001 so the strip-then-validate flow is actually exercised. * detail-nav-bar.stories: clarified that bindShortcuts=false in meta is a stories-context override, not the component default. * TunnelCard clipboard NotAllowedError hint: "Ctrl+C" -> "Ctrl/Cmd+C" for cross-platform correctness. * EscalationQueuePage: statusFilter is server-side, so escalations.length is post-filter; useEmptyStateProps would resolve to the empty branch even when a filter is active and returns zero. Wrapped the result so the filtered copy fires when filterActive + zero rows. * SettingsPage Advanced badge: dropped redundant aria-label, the visible "Advanced" text is the accessible name. * ClientListPage: routed the inline filtered-vs-empty discriminator through useEmptyStateProps, removing the duplicate inline branching. * WebhookReceiptsPage: useState(initialFromUrl) only seeded once; added an effect that re-targets selected when the connection query param changes mid-session (deferred via microtask per ESLint set-state-in-effect). * approvals / meetings / messages / tasks: sanitizer imports moved from @/stores/notifications to @/utils/ws-sanitize -- removes the cross-store coupling and circular-init risk; the notifications store re-export stays for any external callers. * getCrudErrorTitle: 401 (authentication) and 403 (authorization) now return distinct toast titles; the toast-title layer surfaces "Permission denied" for 403 and "Authentication failed" for 401, matching what the user actually needs to do.
Aureliolo
force-pushed
the
fix/audit-cleanup-e-frontend
branch
from
4bedae1 to
6c45c78
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
web/src/pages/WebhookReceiptsPage.tsx (1)
186-204: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove
ErrorBannerabove the filter row for list-page consistency
ErrorBanneris currently rendered afterSearchFilterSort; list pages should place it directly afterListHeader.Suggested fix
<ListHeader title="Webhook receipts" count={entries.length} /> + + {error && ( + <ErrorBanner + severity="error" + title="Could not load webhook activity" + description={error} + onRetry={() => void reload()} + /> + )} <SearchFilterSort filters={ <SelectField label="Connection" value={selected} onChange={setSelected} options={options} /> } /> - - {error && ( - <ErrorBanner - severity="error" - title="Could not load webhook activity" - description={error} - onRetry={() => void reload()} - /> - )}As per coding guidelines: "
web/src/pages/**/*.tsx: ...<ErrorBanner>lands immediately after<ListHeader>, before any filter/pagination row".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/WebhookReceiptsPage.tsx` around lines 186 - 204, The ErrorBanner is rendered after SearchFilterSort but per guidelines it must appear immediately after ListHeader; move the <ErrorBanner ... /> JSX block so it sits directly after the <ListHeader /> component and before the <SearchFilterSort /> block, preserving its props (severity, title, description={error}, onRetry={() => void reload()}) and conditional rendering (error && ...). Ensure SearchFilterSort (with SelectField value={selected} onChange={setSelected} options={options}) remains intact and that the error conditional still uses the same reload handler.web/src/pages/UsersPage.tsx (1)
116-136:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove
<ErrorBanner>directly below<ListHeader>in this list page.Current order places filters before the banner, which breaks the shared list-page contract and can hide error context behind controls.
As per coding guidelines: "
web/src/pages/**/*.tsx:<ErrorBanner>lands immediately after<ListHeader>, before any filter/pagination row."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/UsersPage.tsx` around lines 116 - 136, The ErrorBanner is rendered after the filters, violating the list-page contract; move the conditional ErrorBanner block so it appears immediately after the ListHeader component (before the SearchFilterSort/SearchInput and any pagination/filter rows) while retaining the existing conditional (error) check and the onRetry handler (fetchUsers). Locate the ErrorBanner, ListHeader, SearchFilterSort, and fetchUsers symbols in UsersPage and reposition the ErrorBanner JSX directly below ListHeader, keeping its props (severity, title, description, onRetry) unchanged and ensuring no layout or aria semantics are altered.web/src/pages/approvals/ApprovalCard.tsx (1)
19-55: 🧹 Nitpick | 🔵 TrivialMove countdown prop-sync out of render phase.
Lines 52–55 call
setCountdown(...)during render, which triggers state updates outsideuseEffect. Move this intouseEffectkeyed byapproval.seconds_remainingto avoid render-phase updates and unnecessary re-renders in this memoized component.Proposed fix
const [countdown, setCountdown] = useState(approval.seconds_remaining) - const prevSecondsRef = useRef(approval.seconds_remaining) - if (approval.seconds_remaining !== prevSecondsRef.current) { - prevSecondsRef.current = approval.seconds_remaining - setCountdown(approval.seconds_remaining) - } + useEffect(() => { + setCountdown(approval.seconds_remaining) + }, [approval.seconds_remaining])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/approvals/ApprovalCard.tsx` around lines 19 - 55, The component ApprovalCardImpl is calling setCountdown during render when approval.seconds_remaining changes (using prevSecondsRef), which must be moved into a useEffect to avoid render-phase state updates; replace the inline check and setCountdown (the block that compares approval.seconds_remaining to prevSecondsRef.current and calls setCountdown) with a useEffect that has approval.seconds_remaining in its dependency array, updates prevSecondsRef.current and calls setCountdown inside that effect, preserving the existing prevSecondsRef, countdown state, and behavior.web/src/pages/budget/AgentSpendingTable.tsx (2)
48-48:⚠️ Potential issue | 🟠 MajorPass locale to
localeCompare()for consistent agent-name sorting.Line 48 uses bare
.localeCompare()which defaults to the browser locale instead of the user/company locale setting, causing inconsistent sorting across users.getLocale()is available from@/utils/localeand should be passed as the second argument.Fix
- case 'agentName': cmp = a.agentName.localeCompare(b.agentName); break + case 'agentName': cmp = a.agentName.localeCompare(b.agentName, getLocale()); breakAdd the import at the top of the file:
import { formatCurrency } from '@/utils/format' +import { getLocale } from '@/utils/locale'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/budget/AgentSpendingTable.tsx` at line 48, The agent-name comparison uses a bare a.agentName.localeCompare(b.agentName) which relies on browser locale; import getLocale from '@/utils/locale' and change the comparison in the AgentSpendingTable sort/case 'agentName' branch to a.agentName.localeCompare(b.agentName, getLocale()) so sorting uses the app/user locale consistently (add the import at the top and update the 'agentName' case).
99-99:⚠️ Potential issue | 🟠 MajorUse
Intlformatting instead of.toFixed()for the percentage.The
.toFixed(1)method doesn't respect locale-specific decimal separators (some locales use commas instead of periods), and manually appending%is a hand-rolled template. SincebudgetPercentis already in the range 0–100, useIntl.NumberFormatdirectly without converting:🔢 Proposed fix
- <span className={cn(COLUMN_WIDTHS.budgetPercent, 'text-right font-mono text-xs text-text-secondary')}> - {row.budgetPercent.toFixed(1)}% - </span> + <span className={cn(COLUMN_WIDTHS.budgetPercent, 'text-right font-mono text-xs text-text-secondary')}> + {new Intl.NumberFormat(getLocale(), { + minimumFractionDigits: 1, + maximumFractionDigits: 1 + }).format(row.budgetPercent)}% + </span>Add imports if not already present:
import { formatCurrency } from '@/utils/format' +import { getLocale } from '@/utils/locale'As per coding guidelines: "Date / number format: always via
Intl; no hand-rolled templates."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/budget/AgentSpendingTable.tsx` at line 99, Replace the hand-rolled percentage rendering that uses row.budgetPercent.toFixed(1) + '%' with an Intl.NumberFormat-based formatter; update the JSX in AgentSpendingTable (where row.budgetPercent is rendered) to use new Intl.NumberFormat with style: 'percent' or number format for one decimal place and the current locale (remember budgetPercent is already 0–100 so divide by 100 if using style: 'percent'), e.g., create a formatter once (or inline) and use formatter.format(...) instead of .toFixed and string concatenation to ensure locale-aware decimal separators and percent symbol placement.web/src/stores/messages.ts (1)
179-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
typeandprioritystill use unsafeascasts aftersanitizeWsString.These fields are enum-typed (
MessageType,MessagePriority) but bypasssanitizeWsEnum, which means an unknown backend value would pass through verbatim instead of triggering the warning + fallback behavior. As per coding guidelines, rawsanitizeWsString(...) as EnumTypecasts are forbidden for WS-sourced enum fields.Consider routing these through
sanitizeWsEnumwith correspondingMESSAGE_TYPE_VALUES/MESSAGE_PRIORITY_VALUESallowlists (similar to the attachment type fix), or relaxing the shape guard if you prefer to sanitize-then-fallback rather than reject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/messages.ts` around lines 179 - 180, The code is unsafely casting sanitized strings to enum types for Message['type'] and Message['priority']; replace the raw sanitizeWsString(... ) as Message['type'] / Message['priority'] with calls to sanitizeWsEnum using the appropriate allowlists (MESSAGE_TYPE_VALUES and MESSAGE_PRIORITY_VALUES) so unknown backend values trigger the warning+fallback behavior; update the object construction that sets type and priority to use sanitizeWsEnum(value, MESSAGE_TYPE_VALUES, 'Message.type', DEFAULT_TYPE) and sanitizeWsEnum(value, MESSAGE_PRIORITY_VALUES, 'Message.priority', DEFAULT_PRIORITY) (or adjust the existing fallback constants) so values are validated instead of being force-cast.web/src/stores/tasks.ts (1)
298-307: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffInconsistent forward-compatibility: some enum fields still reject unknown values.
While
status,priority,type, andsourcenow usesanitizeWsEnumwith fallbacks, these fields still validate against hardcoded allowlist sets:
estimated_complexity→COMPLEXITY_SET.has(...)(line 299)task_structure→TASK_STRUCTURE_SET.has(...)(line 302)coordination_topology→COORDINATION_TOPOLOGY_SET.has(...)(line 304)A backend rollout adding a new complexity or topology value will cause the entire frame to be dropped, unlike status/priority which would fall back gracefully.
If these fields are intentionally "fixed" enums that the frontend must understand, add a comment explaining why they differ from the sanitized fields. Otherwise, consider routing them through
sanitizeWsEnumfor consistent forward-compat behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/tasks.ts` around lines 298 - 307, The current validation drops frames when unknown enum values appear because estimated_complexity, task_structure, and coordination_topology are checked against hardcoded sets (COMPLEXITY_SET, TASK_STRUCTURE_SET, COORDINATION_TOPOLOGY_SET) instead of using the forward-compatible sanitizeWsEnum used for status/priority/type/source; update the validation to route c.estimated_complexity, c.task_structure, and c.coordination_topology through sanitizeWsEnum with appropriate fallback defaults (or optional null handling for task_structure) instead of using .has(...) checks to ensure unknown backend values are accepted and normalized, or if you intentionally require strict enums add a concise comment above these checks explaining why they must reject unknown values and cannot be sanitized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/api/types/websocket.ts`:
- Around line 218-224: The WsMessageSentPayload type is missing fields the store
expects; update the WsMessageSentPayload interface (in
web/src/api/types/websocket.ts) to include a message object or the top-level
fields required by parseWsMessage — specifically ensure it contains timestamp,
channel, type, priority, attachments, and metadata (and preserve existing
message_id, sender, to, content, readonly parts). Match the shape validated by
isMessageShape (or reference the canonical Message type used by
stores/messages.ts) so parseWsMessage can consume the payload without runtime
shape mismatches.
In `@web/src/components/layout/StatusBar.tsx`:
- Around line 37-43: Update the A11y contract comment inside StatusBar.tsx to
reference the correct example range for the health button implementation: change
the incorrect "line 198-205" to the actual range "lines 204-213" (or use a
non-hardcoded descriptive pointer like "the health button implementation below")
so the doc points to the full pairing of Dot and <span>{label}</span> used in
the health button example; ensure the comment still mentions Dot, label and the
wrapping button aria-label for clarity.
In `@web/src/components/ui/error-boundary.tsx`:
- Around line 113-115: The retry Button currently lacks an explicit button type
which can cause unintended form submission when rendered inside a form; update
the Button component instance that calls resetErrorBoundary (the <Button ...
onClick={resetErrorBoundary}> used for "Try Again") to include type="button" so
the element is treated as a non-submitting control and retains the existing
onClick behavior.
In `@web/src/pages/messages/ChannelListItem.tsx`:
- Around line 53-58: ChannelListItem is memoized but receives inline arrow
onClick props from ChannelSidebar (props passed as () =>
onSelectChannel(ch.name) and () => handleSelect(ch.name)), which breaks memo
because a new function is created every render; fix by creating stable callbacks
with React.useCallback in ChannelSidebar for the handlers passed to
ChannelListItem (wrap the functions that call onSelectChannel and handleSelect,
e.g. const handleClick = useCallback(() => onSelectChannel(ch.name),
[onSelectChannel, ch.name]) or appropriate dependency list) so the onClick prop
identity remains stable and ChannelListItem.memo can prevent unnecessary
re-renders.
In `@web/src/pages/setup/SkipWizardForm.tsx`:
- Around line 42-55: The catch block is the wrong failure boundary because
submitCompany() does its own error handling in the store; remove relying on
exceptions for company-creation failures and instead, immediately after awaiting
submitCompany(), read useSetupWizardStore.getState().companyResponse and branch
based on that durable state (companyResponse === null means creation failed)
before proceeding to completion/success UX; keep using getErrorMessage(err) and
setError only for real runtime errors that occur outside store mutations, and
update the message construction (using trimmed and companyResponse) to mirror
the current logic but triggered from the post-submit store-state check rather
than from the catch handler.
In `@web/src/pages/setup/WizardNavigation.tsx`:
- Around line 51-65: The Next button's disabled reason paragraph isn't
associated with the control for screen readers; when nextDisabled &&
nextDisabledReason && !isLast, add an id (e.g., nextDisabledReasonId) to the <p>
and set aria-describedby={nextDisabledReasonId} on the Button rendered earlier
(in the Button that uses onNext, nextDisabled, loading, nextLabel, ArrowRight)
so assistive tech will read the reason; ensure the aria-describedby value is
only applied when the reason exists to avoid referencing a missing id.
In `@web/src/pages/WebhookReceiptsPage.tsx`:
- Around line 63-73: The effect that preselects from urlConnection (useEffect
using urlConnection, setSelected and selected) can run before connections are
loaded and get overwritten later; update the logic to re-run when connections
change and only apply the deep-link if the target connection actually exists (or
if selected is empty) to avoid stomping a user choice. Concretely: include
connections (or a derived lookup like connections.map(c=>c.id)) in the useEffect
dependency list, and inside the effect check that urlConnection is truthy and
present in connections (or that selected is falsy) before calling
setSelected(urlConnection); apply the same change to the other similar effect
referenced (lines 86-95) so both effects respect connection availability.
In `@web/src/pages/workflow-editor/useWorkflowEditorCallbacks.ts`:
- Around line 124-126: The toast title currently uses developer shorthand
"error(s)" when setting title based on result.valid and result.errors.length;
update the logic where the title is constructed (the object with title:
result.valid ? 'Workflow is valid' : `Validation found ${result.errors.length}
error(s)`) to use proper pluralization—e.g., compute a string like `Validation
found ${n} error` when n === 1 and `Validation found ${n} errors`
otherwise—using result.errors.length to choose the singular or plural form so
the UI shows grammatically correct text.
In `@web/src/stores/providers/crud-actions.ts`:
- Around line 76-82: The mutation catch blocks for provider actions
(createFromPreset, updateProvider, deleteProvider and the existing create
provider catch in crud-actions.ts) must also emit a log event before showing the
toast and returning null; add a logging call (e.g., console.error or the
project's app logger) in each catch that logs the error object and contextual
metadata (action name like "createFromPreset"/"updateProvider"/"deleteProvider",
provider id or preset name if available) so failures are recorded, then keep the
existing useToastStore.getState().add(...) and return null. Ensure you add the
log immediately inside each catch (before getCrudErrorTitle/getErrorMessage
usage) and reference the same identifiers (createFromPreset, updateProvider,
deleteProvider) so the changes are easy to locate.
---
Outside diff comments:
In `@web/src/pages/approvals/ApprovalCard.tsx`:
- Around line 19-55: The component ApprovalCardImpl is calling setCountdown
during render when approval.seconds_remaining changes (using prevSecondsRef),
which must be moved into a useEffect to avoid render-phase state updates;
replace the inline check and setCountdown (the block that compares
approval.seconds_remaining to prevSecondsRef.current and calls setCountdown)
with a useEffect that has approval.seconds_remaining in its dependency array,
updates prevSecondsRef.current and calls setCountdown inside that effect,
preserving the existing prevSecondsRef, countdown state, and behavior.
In `@web/src/pages/budget/AgentSpendingTable.tsx`:
- Line 48: The agent-name comparison uses a bare
a.agentName.localeCompare(b.agentName) which relies on browser locale; import
getLocale from '@/utils/locale' and change the comparison in the
AgentSpendingTable sort/case 'agentName' branch to
a.agentName.localeCompare(b.agentName, getLocale()) so sorting uses the app/user
locale consistently (add the import at the top and update the 'agentName' case).
- Line 99: Replace the hand-rolled percentage rendering that uses
row.budgetPercent.toFixed(1) + '%' with an Intl.NumberFormat-based formatter;
update the JSX in AgentSpendingTable (where row.budgetPercent is rendered) to
use new Intl.NumberFormat with style: 'percent' or number format for one decimal
place and the current locale (remember budgetPercent is already 0–100 so divide
by 100 if using style: 'percent'), e.g., create a formatter once (or inline) and
use formatter.format(...) instead of .toFixed and string concatenation to ensure
locale-aware decimal separators and percent symbol placement.
In `@web/src/pages/UsersPage.tsx`:
- Around line 116-136: The ErrorBanner is rendered after the filters, violating
the list-page contract; move the conditional ErrorBanner block so it appears
immediately after the ListHeader component (before the
SearchFilterSort/SearchInput and any pagination/filter rows) while retaining the
existing conditional (error) check and the onRetry handler (fetchUsers). Locate
the ErrorBanner, ListHeader, SearchFilterSort, and fetchUsers symbols in
UsersPage and reposition the ErrorBanner JSX directly below ListHeader, keeping
its props (severity, title, description, onRetry) unchanged and ensuring no
layout or aria semantics are altered.
In `@web/src/pages/WebhookReceiptsPage.tsx`:
- Around line 186-204: The ErrorBanner is rendered after SearchFilterSort but
per guidelines it must appear immediately after ListHeader; move the
<ErrorBanner ... /> JSX block so it sits directly after the <ListHeader />
component and before the <SearchFilterSort /> block, preserving its props
(severity, title, description={error}, onRetry={() => void reload()}) and
conditional rendering (error && ...). Ensure SearchFilterSort (with SelectField
value={selected} onChange={setSelected} options={options}) remains intact and
that the error conditional still uses the same reload handler.
In `@web/src/stores/messages.ts`:
- Around line 179-180: The code is unsafely casting sanitized strings to enum
types for Message['type'] and Message['priority']; replace the raw
sanitizeWsString(... ) as Message['type'] / Message['priority'] with calls to
sanitizeWsEnum using the appropriate allowlists (MESSAGE_TYPE_VALUES and
MESSAGE_PRIORITY_VALUES) so unknown backend values trigger the warning+fallback
behavior; update the object construction that sets type and priority to use
sanitizeWsEnum(value, MESSAGE_TYPE_VALUES, 'Message.type', DEFAULT_TYPE) and
sanitizeWsEnum(value, MESSAGE_PRIORITY_VALUES, 'Message.priority',
DEFAULT_PRIORITY) (or adjust the existing fallback constants) so values are
validated instead of being force-cast.
In `@web/src/stores/tasks.ts`:
- Around line 298-307: The current validation drops frames when unknown enum
values appear because estimated_complexity, task_structure, and
coordination_topology are checked against hardcoded sets (COMPLEXITY_SET,
TASK_STRUCTURE_SET, COORDINATION_TOPOLOGY_SET) instead of using the
forward-compatible sanitizeWsEnum used for status/priority/type/source; update
the validation to route c.estimated_complexity, c.task_structure, and
c.coordination_topology through sanitizeWsEnum with appropriate fallback
defaults (or optional null handling for task_structure) instead of using
.has(...) checks to ensure unknown backend values are accepted and normalized,
or if you intentionally require strict enums add a concise comment above these
checks explaining why they must reject unknown values and cannot be sanitized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3805fe86-8ceb-4109-a941-ab6e92035ad0
📒 Files selected for processing (73)
docs/reference/web-package-structure.mdweb/CLAUDE.mdweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/__tests__/stores/artifacts.test.tsweb/src/__tests__/stores/meetings.test.tsweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/api/types/approvals.tsweb/src/api/types/enums.tsweb/src/api/types/messages.tsweb/src/api/types/websocket.tsweb/src/components/layout/StatusBar.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/components/ui/error-boundary.tsxweb/src/components/ui/progress-gauge.tsxweb/src/components/ui/skeleton.tsxweb/src/hooks/use-empty-state-props.tsweb/src/mocks/handlers/helpers.tsweb/src/mocks/handlers/index.tsweb/src/pages/ApprovalsPage.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/BudgetPage.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/EscalationQueuePage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/SettingsPage.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/setup/ProvidersStep.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/workflows/WorkflowFilters.tsxweb/src/stores/approvals.tsweb/src/stores/artifacts.tsweb/src/stores/connections/crud-actions.tsweb/src/stores/meetings.tsweb/src/stores/messages.tsweb/src/stores/notifications.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/setup-wizard/providers.tsweb/src/stores/setup-wizard/types.tsweb/src/stores/tasks.tsweb/src/styles/status-colors.tsweb/src/utils/errors.tsweb/src/utils/ws-sanitize.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx}: No default may privilege a single region, currency, or locale. Every user-facing format resolves from: user/company setting -> browser/system -> neutral fallback.
Currency: never hardcode ISO 4217 codes or symbols. Backend:DEFAULT_CURRENCYfromsynthorg.budget.currencyor the runtimebudget.currencysetting. Frontend:DEFAULT_CURRENCYfrom@/utils/currenciesoruseSettingsStore().currency.
Field naming: no_usdsuffix on money fields anywhere. The type carries money semantics; the value is in the operator's configured currency.
Locale: never hardcode BCP 47 tags or call bare.toLocaleString()/.toLocaleDateString()/.toLocaleTimeString(). Use helpers in@/utils/formatwhich readgetLocale()from@/utils/locale. The backend has no operator-tunable locale setting; backendIntlformatting uses the system locale plus the browser timezone. Thecompany.name_localeslist controls procedural-name generation only; it does not feed number / date / time formatting.
Timezone: store UTC; render viaIntlwithout passingtimeZone(browser tz wins).
Date / number format: always viaIntl; no hand-rolled templates.
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/components/ui/progress-gauge.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/styles/status-colors.tsweb/src/pages/connections/TunnelCard.tsxweb/src/api/types/messages.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/mocks/handlers/index.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/hooks/use-empty-state-props.tsweb/src/pages/setup/CompleteStep.tsxweb/src/stores/artifacts.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/mocks/handlers/helpers.tsweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/utils/errors.tsweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/stores/notifications.tsweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/agents/TaskHistory.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/api/types/enums.tsweb/src/pages/connections/ConnectionCard.tsxweb/src/stores/setup-wizard/types.tsweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/BudgetPage.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/SettingsPage.tsxweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/stores/meetings.tsweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/api/types/approvals.tsweb/src/stores/approvals.tsweb/src/pages/EscalationQueuePage.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/stores/tasks.tsweb/src/pages/ApprovalsPage.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/stores/meetings.test.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types/websocket.tsweb/src/__tests__/stores/artifacts.test.tsweb/src/stores/setup-wizard/providers.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Units: metric only. Spelling: International / British English UI default (
colour,behaviour,organise,centred,analyse,cancelled); document deviations. Spelling here is an editorial / UI-copy decision only; it does not affect runtime locale-sensitive formatting. Numbers, dates, times, currencies, and units still resolve via the user / company / browser / system fallback through@/utils/format,@/utils/locale,DEFAULT_CURRENCY, anduseSettingsStore().currency, with no contradiction to the locale-neutral defaults above.
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/components/ui/progress-gauge.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/styles/status-colors.tsweb/src/pages/connections/TunnelCard.tsxweb/src/api/types/messages.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/mocks/handlers/index.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/hooks/use-empty-state-props.tsweb/src/pages/setup/CompleteStep.tsxweb/src/stores/artifacts.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/mocks/handlers/helpers.tsweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/utils/errors.tsweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/stores/notifications.tsweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/agents/TaskHistory.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/api/types/enums.tsweb/src/pages/connections/ConnectionCard.tsxweb/src/stores/setup-wizard/types.tsweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/BudgetPage.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/SettingsPage.tsxweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/stores/meetings.tsweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/api/types/approvals.tsweb/src/stores/approvals.tsweb/src/pages/EscalationQueuePage.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/stores/tasks.tsweb/src/pages/ApprovalsPage.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/stores/meetings.test.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types/websocket.tsweb/src/__tests__/stores/artifacts.test.tsweb/src/stores/setup-wizard/providers.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse components from
web/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, Motion transitions, or BCP 47 locale strings; use design tokens,@/lib/motionpresets, and the helpers in@/utils/format. Enforced byscripts/check_web_design_system.py(PostToolUse hook on everyweb/src/edit).Always use
createLoggerfrom@/lib/logger; never use bareconsole.warn/console.error/console.debugin application codeLogger variable name must always be
log(e.g.,const log = createLogger('module-name'))Pass dynamic/untrusted values as separate args to logger methods (not interpolated into the message string) so they go through
sanitizeArgWrap attacker-controlled fields inside structured objects in
sanitizeForLog()before embedding in logs
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/components/ui/progress-gauge.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/styles/status-colors.tsweb/src/pages/connections/TunnelCard.tsxweb/src/api/types/messages.tsweb/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/mocks/handlers/index.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/hooks/use-empty-state-props.tsweb/src/pages/setup/CompleteStep.tsxweb/src/stores/artifacts.tsweb/src/__tests__/styles/status-colors.test.tsweb/src/mocks/handlers/helpers.tsweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/utils/errors.tsweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/stores/notifications.tsweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/agents/TaskHistory.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/api/types/enums.tsweb/src/pages/connections/ConnectionCard.tsxweb/src/stores/setup-wizard/types.tsweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/BudgetPage.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/SettingsPage.tsxweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/stores/meetings.tsweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/api/types/approvals.tsweb/src/stores/approvals.tsweb/src/pages/EscalationQueuePage.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/stores/tasks.tsweb/src/pages/ApprovalsPage.tsxweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/stores/meetings.test.tsweb/src/pages/setup/ProvidersStep.tsxweb/src/api/types/websocket.tsweb/src/__tests__/stores/artifacts.test.tsweb/src/stores/setup-wizard/providers.ts
web/src/{components,pages,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Callers must NOT wrap store mutation calls in try/catch; the store owns the error UX
NEVER hardcode hex colors, font-family declarations, pixel spacing, Motion transition durations, BCP 47 locale literals (
'en-US'), or currency symbols/codes; use design tokens,@/lib/motionpresets, helpers in@/utils/format, andDEFAULT_CURRENCYfrom@/utils/currencies
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/components/ui/progress-gauge.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/hooks/use-empty-state-props.tsweb/src/pages/setup/CompleteStep.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/tasks/useTaskActionHandlers.tsweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/pages/agents/TaskHistory.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/BudgetPage.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/SettingsPage.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/EscalationQueuePage.tsxweb/src/pages/ApprovalsPage.tsxweb/src/pages/setup/ProvidersStep.tsx
web/src/{components,pages}/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<StatusBadge>instead of recreating status dots inline (defaults torole='img'with aria-label;decorativefor adjacent-labeled,announcefor live WS updates)Use
<MetricCard>/<Sparkline>/<ProgressGauge>/<TokenUsageBar>instead of creating KPI displays inlineUse
<SectionCard>(titled wrapper with icon and action slot),<AgentCard>,<DeptHealthBar>instead of recreating cards inlineUse
<InputField>/<SelectField>/<SliderField>/<ToggleField>/<SegmentedControl>/<TagInput>/<SearchInput>instead of creating form fields inlineUse
<Drawer width='compact|narrow|default|wide'>for slide-in panels; do NOT add inlinew-[40vw]overridesUse
<Skeleton>family /<EmptyState>/<ErrorBoundary>/<ErrorBanner>/<ProgressIndicator>for loading/empty/error states instead of creating inlineUse
STATUS_COLORSfamily from@/styles/status-colors(typedRecord<EnumValue, string>lookups) instead of inlineRecord<EnumValue, string>constants per pageUse
<ConfirmDialog>/<Toast>(Zustand-backed queue) instead of creating confirmation/toast UI inline; NOT Base UI's ToastUse
<CommandPalette>/<KeyboardShortcutHint>/<CommandCheatsheet>instead of creating Cmd+K/shortcuts UI inlineUse
<AnimatedPresence>/<StaggerGroup>/<LiveRegion>for animation;<LiveRegion>is debounced ARIA live for WS updates
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/components/ui/progress-gauge.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/components/layout/StatusBar.tsxweb/src/pages/BudgetPage.tsxweb/src/components/ui/skeleton.tsxweb/src/pages/SettingsPage.tsxweb/src/components/ui/detail-nav-bar.stories.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/EscalationQueuePage.tsxweb/src/pages/ApprovalsPage.tsxweb/src/pages/setup/ProvidersStep.tsx
web/src/pages/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<ListHeader>/<SearchFilterSort>/<Pagination>/<BulkActionBar>/<MetadataGrid>/<Breadcrumbs>for list-page primitivesList pages: root container uses
space-y-section-gap(the majority pattern;flex flex-col gap-section-gapis 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='...' />Choose list layout: 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)
Use
useEmptyStateProps({ filteredCount, totalCount, filterActive, empty, filtered })from@/hooks/use-empty-state-propsto returnEmptyStateProps | nullso pages branch on a single value instead of duplicating the 'no data ever' / 'no data after filter' discriminator
Files:
web/src/pages/oauth-apps/OauthAppCard.tsxweb/src/pages/DashboardPage.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/ConnectionsPage.tsxweb/src/pages/ProvidersPage.tsxweb/src/pages/RequestQueuePage.tsxweb/src/pages/connections/TunnelCard.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/workflows/WorkflowFilters.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/TaskBoardPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/setup/CompleteStep.tsxweb/src/pages/ClientListPage.tsxweb/src/pages/setup/WizardShell.tsxweb/src/pages/BudgetForecastPage.tsxweb/src/pages/setup/CompanyStep.tsxweb/src/pages/approvals/ApprovalDetailDrawer.tsxweb/src/pages/dashboard/ActivityFeedItem.tsxweb/src/pages/tasks/TaskColumn.tsxweb/src/pages/agents/TaskHistory.tsxweb/src/pages/connections/ConnectionCard.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/BudgetPage.tsxweb/src/pages/SettingsPage.tsxweb/src/pages/WorkflowsPage.tsxweb/src/pages/setup/SetupAgentCard.tsxweb/src/pages/EscalationQueuePage.tsxweb/src/pages/ApprovalsPage.tsxweb/src/pages/setup/ProvidersStep.tsx
web/src/components/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use Base UI's
renderprop as the polymorphism primitive throughout the dashboard; use the local<Slot>helper incomponents/ui/slot.tsxwith@base-ui/react/merge-propsonly for<Button asChild>, all other primitives use Base UI's nativerenderprop directlyALWAYS reuse existing components from
web/src/components/ui/before creating new onesBase UI primitives are imported directly from
@base-ui/react/<subpath>and use the nativerenderprop for polymorphism; the local<Slot>helper is reserved for<Button asChild>Adopted Base UI primitives (direct import from
@base-ui/react/<subpath>): Dialog, AlertDialog, Popover, Tabs, Menu, Drawer, CSPProvider, merge-props
Files:
web/src/components/ui/progress-gauge.tsxweb/src/components/ui/error-boundary.tsxweb/src/components/layout/StatusBar.tsxweb/src/components/ui/skeleton.tsxweb/src/components/ui/detail-nav-bar.stories.tsx
web/src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Every new shared component lives in
web/src/components/ui/with a sibling.stories.tsxcovering all states (default, hover, loading, error, empty, disabled where applicable)Component Props interface name must be
<ComponentName>Propsand exported from the same file (e.g.,AgentCardPropsinagent-card.tsx) to make the contract greppable and allow callers to extend props
Files:
web/src/components/ui/progress-gauge.tsxweb/src/components/ui/error-boundary.tsxweb/src/components/ui/skeleton.tsxweb/src/components/ui/detail-nav-bar.stories.tsx
web/src/{api,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use cursor-based pagination via
PaginationMeta; keepnextCursor+hasMorein store state (not offset arithmetic) and early-return when!hasMore || !nextCursorDisplay counts must come from
data.length; the wire envelope no longer carriestotal
Files:
web/src/api/types/messages.tsweb/src/stores/artifacts.tsweb/src/stores/notifications.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/api/types/enums.tsweb/src/stores/setup-wizard/types.tsweb/src/stores/meetings.tsweb/src/api/types/approvals.tsweb/src/stores/approvals.tsweb/src/stores/tasks.tsweb/src/api/types/websocket.tsweb/src/stores/setup-wizard/providers.ts
web/src/{api,hooks}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
getLiveness()is always 200 while the process is alive;getReadiness()is 200 healthy / 503 unavailable (binary'ok' | 'unavailable'outcome, no tri-state) and callers must handle the 503 path explicitly
Files:
web/src/api/types/messages.tsweb/src/hooks/use-empty-state-props.tsweb/src/api/types/enums.tsweb/src/api/types/approvals.tsweb/src/api/types/websocket.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Use Mermaid (\``mermaid): flowcharts, sequence diagrams, simple hierarchies, pipelines. Rendered client-side viapymdownx.superfences. Use **Markdown tables**: grid/matrix data that is semantically tabular (not diagrams). D2 uses theme 200 (Dark Mauve), dark-only render, configured globally inmkdocs.yml. Never use```text` blocks with ASCII/Unicode box-drawing characters for diagrams.
Files:
docs/reference/web-package-structure.md
web/src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Never use
vi.mock('@/api/endpoints/*'); instead override MSW handlers per-case viaserver.use(...)
Files:
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/__tests__/styles/status-colors.test.tsweb/src/__tests__/hooks/use-empty-state-props.test.tsweb/src/__tests__/components/ui/progress-gauge.test.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/pages/setup/ProvidersStep.test.tsxweb/src/__tests__/utils/errors.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/utils/ws-sanitize.test.tsweb/src/__tests__/stores/setup-wizard.test.tsweb/src/__tests__/stores/meetings.test.tsweb/src/__tests__/stores/artifacts.test.ts
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
MSW handlers in
web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with default happy-path handler for every exported endpointUse typed envelope helpers (
successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/index.tsweb/src/mocks/handlers/helpers.ts
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
All store mutation actions (create/update/delete) must follow the
stores/connections/crud-actions.tspattern: wrap API calls in try/catch, success path updates state + emits success toast, failure path logs + emits error toast + returns sentinelList store reads must set
error: string | nullon the store instead of toastingGlobal
afterEachinweb/src/test-setup.tsxcallsuseToastStore.getState().dismissAll(),cancelPendingPersist(), anduseThemeStore.getState().teardown(); any new store that schedules timers or attaches event listeners must expose an equivalent cleanup hook and register it in the globalafterEach
Files:
web/src/stores/artifacts.tsweb/src/stores/notifications.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/stores/setup-wizard/types.tsweb/src/stores/meetings.tsweb/src/stores/approvals.tsweb/src/stores/tasks.tsweb/src/stores/setup-wizard/providers.ts
web/src/{utils,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
sanitizeWsString()to clamp every WS-supplied string (strips C0 controls + bidi-overrides + caps length) andsanitizeWsEnum<T>(value, allowlist, fallback, { field })for enum-allowlist validation; raw(sanitizeWsString(x, n) ?? '') as EnumTypecasts are forbiddenWS wire protocol constants (
WS_PROTOCOL_VERSION,WS_MAX_MESSAGE_SIZE,WS_HEARTBEAT_INTERVAL_MS,WS_PONG_TIMEOUT_MS,LOG_SANITIZE_MAX_LENGTH) inweb/src/utils/constants.tsMUST stay in lockstep withsrc/synthorg/api/ws_models.py/src/synthorg/api/controllers/ws.py; bump protocol version on both sides together for breaking changes
Files:
web/src/stores/artifacts.tsweb/src/utils/errors.tsweb/src/stores/notifications.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/messages.tsweb/src/stores/connections/crud-actions.tsweb/src/stores/setup-wizard/types.tsweb/src/stores/meetings.tsweb/src/stores/approvals.tsweb/src/stores/tasks.tsweb/src/stores/setup-wizard/providers.ts
web/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Storybook 10 is ESM-only; essentials are built into core but
@storybook/addon-docsis separate; imports moved tostorybook/testandstorybook/actions
Files:
web/src/components/ui/detail-nav-bar.stories.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T13:33:57.345Z
Learning: CI fails if `vitest --detect-async-leaks` reports more than `MAX_ASYNC_LEAKS` (currently 90); raise the ceiling only with documented per-PR justification
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T13:33:57.345Z
Learning: TypeScript 6: `baseUrl` is deprecated and `esModuleInterop` is always true; `types` defaults to `[]` so `vitest/globals` etc. need explicit listing in `tsconfig.json`
| export interface WsMessageSentPayload { | ||
| message_id: string | ||
| sender: string | ||
| to: string | ||
| content: string | ||
| readonly parts: readonly WsMessagePart[] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what fields sanitizeMessage actually reads from the payload
rg -A 30 "function sanitizeMessage" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# Check if the messages.ts file exists and what's around line 117
fd -type f "messages.ts" web/src
# Also look for sanitizeMessage in different ways
rg "sanitizeMessage" web/src/stores/ --max-count 5Repository: Aureliolo/synthorg
Length of output: 233
🏁 Script executed:
# Find messages.ts file
find web/src -name "messages.ts" -type f
# Search for sanitizeMessage anywhere
rg "sanitizeMessage" web/src --type ts --type tsxRepository: Aureliolo/synthorg
Length of output: 260
🏁 Script executed:
# Search for sanitizeMessage without type filter
rg "sanitizeMessage" web/src
# Read the messages.ts store file to see the actual implementation
wc -l web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 93
🏁 Script executed:
# Read the messages.ts file around line 117
sed -n '100,150p' web/src/stores/messages.ts
# Also get more context around sanitizeMessage
rg -B 5 -A 20 "sanitizeMessage" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 1784
🏁 Script executed:
# Find the isMessageShape function definition
rg -B 2 -A 15 "isMessageShape" web/src/stores/messages.ts | head -50
# Also read WsMessageSentPayload from websocket.ts
sed -n '218,224p' web/src/api/types/websocket.ts
# Check if there are more fields in WsMessageSentPayload or related interfaces
sed -n '210,235p' web/src/api/types/websocket.tsRepository: Aureliolo/synthorg
Length of output: 1695
🏁 Script executed:
# Find the Message interface definition
rg -B 2 -A 15 "interface Message" web/src/stores/messages.ts | head -50
# Also check if there's a discriminated union or different message types
rg "WsMessageSentPayload" web/src --max-count 10
# Look for where WsMessageSentPayload is actually used
rg -B 3 -A 3 "WsMessageSentPayload" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 634
🏁 Script executed:
# Find the Message interface definition in messages.ts
rg -B 2 -A 20 "^interface Message[^S]" web/src/stores/messages.ts
# Check the full websocket types file to see the discriminated union
rg -B 5 -A 5 "'message.sent'" web/src/api/types/websocket.ts
# Also check type definitions around line 218
sed -n '200,250p' web/src/api/types/websocket.tsRepository: Aureliolo/synthorg
Length of output: 2691
🏁 Script executed:
# Find where WsMessageSentPayload is actually used in the store
rg -B 5 -A 10 "payload.message" web/src/stores/messages.ts | head -80
# Check if there's a wrapper around message payloads
rg -B 2 -A 10 "message.sent" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 579
🏁 Script executed:
# Check the WsEvent type definition to understand the structure
rg -B 5 -A 20 "type WsEvent" web/src/api/types/websocket.ts
# Look for the message.sent handler or how the payload is structured
rg -B 10 "parseWsMessage" web/src/stores/messages.ts | head -40Repository: Aureliolo/synthorg
Length of output: 3012
🏁 Script executed:
# Find the Message interface definition more carefully
rg "^interface Message " web/src/stores/messages.ts -A 20
# Also check the type annotation for the message field in the payload
rg -B 5 "payload\.message" web/src/stores/messages.ts | head -40
# Look for how WsMessageSentPayload is actually supposed to be structured
rg -B 10 -A 10 "'message\.sent'" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 323
🏁 Script executed:
# Look at the discriminated union to see if there's something specific about message.sent
sed -n '130,170p' web/src/api/types/websocket.ts
# Check how the handler uses the typed event
rg "message.sent" web/src/stores/messages.ts -B 5 -A 5
# Search for any other place where WsMessageSentPayload is used
rg "message_id" web/src/stores/messages.tsRepository: Aureliolo/synthorg
Length of output: 869
🏁 Script executed:
# Look at the complete WsEventPayloadMap to understand the structure
rg "WsEventPayloadMap" web/src/api/types/websocket.ts -A 50 | head -80
# Check if there's a wrapper object around WsMessageSentPayload
sed -n '150,180p' web/src/api/types/websocket.tsRepository: Aureliolo/synthorg
Length of output: 3870
🏁 Script executed:
# Check the actual handler code that dispatches message.sent events
rg "event_type.*message\.sent" web/src/stores/messages.ts -B 5 -A 10
# Look for where parseWsMessage is called and what payload structure it receives
rg "parseWsMessage" web/src/stores/messages.ts -B 3 -A 3
# Check if there's a message wrapper in the actual WsMessageSentPayload or if there's something between event and handler
rg "payload\.message" web/src -B 2 -A 2 | head -50Repository: Aureliolo/synthorg
Length of output: 2436
🏁 Script executed:
# Check the test file to see what the actual payload structure is
rg -A 15 "skips array payload\.message" web/src/__tests__/stores/messages.test.ts
# Also check for other message.sent event tests
rg "message\.sent" web/src/__tests__/stores/messages.test.ts -B 2 -A 10Repository: Aureliolo/synthorg
Length of output: 1825
🏁 Script executed:
# Find the makeMessage test helper to see what structure is actually expected
rg -B 5 -A 20 "const makeMessage" web/src/__tests__/stores/messages.test.ts | head -50
# Check what the full Message interface looks like
rg "^type Message = " web/src/stores/messages.ts -A 10Repository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# Search for Message type definition in different ways
rg "type Message" web/src/stores/messages.ts
# Also try looking for interface Message
rg "interface Message" web/src/stores/messages.ts
# Find makeMessage in the test file
rg "makeMessage" web/src/__tests__/stores/messages.test.ts -B 2 -A 15 | head -80Repository: Aureliolo/synthorg
Length of output: 2741
WsMessageSentPayload is incomplete relative to store requirements.
The parseWsMessage handler in stores/messages.ts expects payload.message to contain timestamp, channel, type, priority, attachments, and metadata fields (validated by isMessageShape), but WsMessageSentPayload only defines message_id, sender, to, content, and parts. Either extend the interface to match the actual event payload structure, or clarify if the wire format and store consumption differ intentionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/api/types/websocket.ts` around lines 218 - 224, The
WsMessageSentPayload type is missing fields the store expects; update the
WsMessageSentPayload interface (in web/src/api/types/websocket.ts) to include a
message object or the top-level fields required by parseWsMessage — specifically
ensure it contains timestamp, channel, type, priority, attachments, and metadata
(and preserve existing message_id, sender, to, content, readonly parts). Match
the shape validated by isMessageShape (or reference the canonical Message type
used by stores/messages.ts) so parseWsMessage can consume the payload without
runtime shape mismatches.
) Round 2 of CodeRabbit + Gemini findings on PR #1718 plus the recurring async-leak CI gate. CI gate fix: * `.github/ci/web-async-leaks.max` raised from 90 to 100 with documented per-PR justification per the file's escape hatch. The +14 new tests this PR adds (renderHook x5 in use-empty-state-props, render(<ProvidersStep/>) integration test, 8 store-level cases in setup-wizard.test.ts and ws-sanitize.test.ts) are mechanical test surface, not a regression. Local 75 leaks; CI baseline +20-21 (parallel concurrency). Lower back toward 80 once #1468 (MSW off XHR interceptor) lands. CodeRabbit MAJOR findings: * SkipWizardForm: submitCompany() doesn't throw -- it sets companyError in the store. The catch block was the wrong failure boundary (would never fire for company-create failures). Read the store snapshot (companyResponse !== null) immediately after await submitCompany() and branch there; the catch now only handles wizardCompleteSetup errors, so the partial-success message is the only branch the catch is responsible for. * WebhookReceiptsPage URL deep-link: the seed effect could fire before connections loaded and get overwritten by the reconciliation effect. Now waits for connections to arrive AND verifies the target exists before applying the URL value (won't stomp a valid current selection with a stale URL). * WebhookReceiptsPage / UsersPage ErrorBanner placement: moved immediately after ListHeader per the project's list-page contract (matches OauthAppsPage). * AgentSpendingTable: localeCompare() now passes getLocale(); .toFixed(1)% replaced with Intl.NumberFormat({ style: 'percent' }) for locale-aware decimal separator + percent placement (project rule: "Date / number format: always via Intl; no hand-rolled templates"). * messages.ts: type / priority fields now route through sanitizeWsEnum with new MESSAGE_TYPE_VALUES / MESSAGE_PRIORITY_VALUES allowlists (previously unsafe `as` casts that bypassed allowlist validation entirely; same forward-compat pattern as approval/meeting/task). * providers/crud-actions.ts: every mutation catch now logs via log.error BEFORE the toast, so a dismissed toast still leaves an observability trace. Connections store already had this; providers was missing it. * error-boundary.tsx: every Button retry control gets explicit type="button" so it cannot accidentally submit a parent form. CodeRabbit MINOR / Trivial findings: * ApprovalCard countdown sync: setCountdown(approval.seconds_remaining) was called during render via prevSecondsRef. Moved into useEffect with microtask defer (matches connections-reconciliation pattern in WebhookReceiptsPage and satisfies the ESLint set-state-in-effect rule). * WizardNavigation: disabled-Next-button caption now associates with the button via aria-describedby (useId-generated), so screen readers announce the reason when the disabled control is focused. * useWorkflowEditorCallbacks: validation toast title uses proper pluralisation ("Validation found 1 error" vs "Validation found N errors") instead of the developer shorthand "error(s)". * StatusBar a11y comment: "line 198-205" reference replaced with a descriptive pointer ("the health-button render below"); the line number was already drifting. * tasks.ts shape guard: complexity / task_structure / coordination_topology stay strictly validated (NOT routed through sanitizeWsEnum) -- they're closed enums coupled to coordination + scheduling code paths that branch on the exact value, so dropping a frame on an unknown value is the safer failure mode. Added an inline comment explaining why these differ from status / priority / type / source. * ChannelListItem: API changed from onClick: () => void to onSelect: (name: string) => void so callers pass a stable reference (the parent's onSelectChannel / handleSelect directly) instead of an inline arrow that breaks memo on every render. Tests + stories updated. Verification: * npm run lint: zero warnings * npm run type-check: clean * 2994 tests pass; local 76 leaks; CI projects ~96 (under 100).
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/stores/messages.ts (1)
141-148: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider whether a blank
tofield should also reject the payload.The validation rejects messages when
id,timestamp,sender, orchannelsanitise to blank, butto(line 125) is not checked. Iftois a required recipient identifier, a blank value could cause downstream routing issues or render incorrectly. If broadcast messages legitimately have an emptyto, this is fine — otherwise add!toto the guard.Proposed change if `to` is always required
- if (!id || !timestamp || !sender || !channel) { + if (!id || !timestamp || !sender || !to || !channel) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/messages.ts` around lines 141 - 148, The guard that rejects sanitized-blank WS messages currently checks id, timestamp, sender, and channel but omits the recipient field "to"; update the validation in the same conditional (the if block that returns null) to also reject when "to" is blank (e.g., include !to or to.length === 0 in the condition) so messages with an empty recipient are dropped unless broadcasts are intentionally allowed; locate the check around the variables id, timestamp, sender, channel in messages.ts and add the "to" check to the same guard.
♻️ Duplicate comments (2)
web/src/pages/WebhookReceiptsPage.tsx (1)
54-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't seed fetch state from an unvalidated
?connection=value.
selectedstarts asurlConnection, so a cold mount can callreload()beforeconnectionshas proved that the target exists. After that, the fallback effect at Lines 94-103 can still overwrite the deep-link withconnections[0], and on a stale bookmark / zero-connection account the discarded first request can leaveloadingstuck true, so the empty state never renders. Initialising with''and only applying the URL once the connection is present avoids both races.Suggested fix
- const [selected, setSelected] = useState<string>(urlConnection) + const [selected, setSelected] = useState<string>('') useEffect(() => { if (!urlConnection) return if (connections.length === 0) return if (!connections.some((c) => c.name === urlConnection)) return @@ - useEffect(() => { - const exists = selected !== '' && connections.some((c) => c.name === selected) + useEffect(() => { + const urlExists = urlConnection !== '' && connections.some((c) => c.name === urlConnection) + const exists = selected !== '' && connections.some((c) => c.name === selected) if (exists) return let cancelled = false void Promise.resolve().then(() => { if (cancelled) return - setSelected(connections[0]?.name ?? '') + setSelected(urlExists ? urlConnection : (connections[0]?.name ?? '')) }) return () => { cancelled = true } - }, [connections, selected]) + }, [connections, selected, urlConnection])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/WebhookReceiptsPage.tsx` around lines 54 - 103, The component seeds selection from urlConnection causing a fetch race; change the initial state for selected to '' instead of urlConnection so no fetch runs on cold mount for an unvalidated URL value. Keep the existing URL-sync effect that reads urlConnection and connections (the useEffect that checks connections.some and then setSelected via a microtask) so the URL is applied only after the target connection is present; leave the fallback effect (the one defaulting to connections[0]) as-is to reconcile removed/stale selections.web/src/pages/messages/ChannelSidebar.tsx (1)
91-94: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMemoize
handleSelector the drawer rows still miss thememowin.
handleSelectis recreated on everyChannelListBodyrender, so every mobileChannelListItemstill receives a newonSelectprop and re-renders even when only one row changed. Wrap it inuseCallbackso the drawer path gets the same row-level memoisation benefit as the desktop path.💡 Suggested change
-import { useMemo, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' @@ - const handleSelect = (name: string) => { + const handleSelect = useCallback((name: string) => { onSelectChannel(name) onAfterSelect?.() - } + }, [onAfterSelect, onSelectChannel])#!/bin/bash # Verify that ChannelListItem is memoized and that ChannelSidebar still # passes a freshly-created callback into the mobile path. rg -nP --type=tsx -C2 'export const ChannelListItem = memo\(' web/src/pages/messages/ChannelListItem.tsx rg -nP --type=tsx -C2 'const handleSelect = \(name: string\) =>' web/src/pages/messages/ChannelSidebar.tsx rg -nP --type=tsx -C2 'onSelect=\{handleSelect\}' web/src/pages/messages/ChannelSidebar.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/messages/ChannelSidebar.tsx` around lines 91 - 94, handleSelect in ChannelSidebar is recreated every render causing mobile ChannelListItem rows to receive a new onSelect and lose memoization; wrap handleSelect in React.useCallback inside ChannelSidebar so it returns the same function instance unless its dependencies change (depend on onSelectChannel and onAfterSelect) so ChannelListBody/ChannelListItem can benefit from memoization and avoid unnecessary re-renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/approvals/ApprovalCard.tsx`:
- Around line 58-67: The ticking interval does not restart when
approval.seconds_remaining is updated, so refreshed values get immediately
decremented; update the ticking useEffect (the effect that watches shouldTick
and creates the interval for decrementing countdown) to also depend on
approval.seconds_remaining (or explicitly clear and recreate the interval when
approval.seconds_remaining changes) so that setCountdown is initialized from the
fresh seconds_remaining and the interval cadence resets; reference the useEffect
that sets countdown (uses setCountdown and approval.seconds_remaining) and the
ticking effect that checks shouldTick to ensure both react to changes in
approval.seconds_remaining and clear/replace the interval when it updates.
In `@web/src/stores/providers/crud-actions.ts`:
- Line 77: The log.error calls in web/src/stores/providers/crud-actions.ts
(e.g., the createProvider error path where log.error('createProvider failed:', {
name: data.name, error: getErrorMessage(err) })) are embedding untrusted fields
directly; wrap attacker-controlled fields with sanitizeForLog(...) before
passing them into the structured metadata object (e.g.,
sanitizeForLog(data.name), sanitizeForLog(data.preset_name), and
sanitizeForLog(getErrorMessage(err))). Update the analogous log.error usages
referenced (around the other occurrences noted) to use sanitizeForLog for each
untrusted value so all metadata is sanitized consistently.
---
Outside diff comments:
In `@web/src/stores/messages.ts`:
- Around line 141-148: The guard that rejects sanitized-blank WS messages
currently checks id, timestamp, sender, and channel but omits the recipient
field "to"; update the validation in the same conditional (the if block that
returns null) to also reject when "to" is blank (e.g., include !to or to.length
=== 0 in the condition) so messages with an empty recipient are dropped unless
broadcasts are intentionally allowed; locate the check around the variables id,
timestamp, sender, channel in messages.ts and add the "to" check to the same
guard.
---
Duplicate comments:
In `@web/src/pages/messages/ChannelSidebar.tsx`:
- Around line 91-94: handleSelect in ChannelSidebar is recreated every render
causing mobile ChannelListItem rows to receive a new onSelect and lose
memoization; wrap handleSelect in React.useCallback inside ChannelSidebar so it
returns the same function instance unless its dependencies change (depend on
onSelectChannel and onAfterSelect) so ChannelListBody/ChannelListItem can
benefit from memoization and avoid unnecessary re-renders.
In `@web/src/pages/WebhookReceiptsPage.tsx`:
- Around line 54-103: The component seeds selection from urlConnection causing a
fetch race; change the initial state for selected to '' instead of urlConnection
so no fetch runs on cold mount for an unvalidated URL value. Keep the existing
URL-sync effect that reads urlConnection and connections (the useEffect that
checks connections.some and then setSelected via a microtask) so the URL is
applied only after the target connection is present; leave the fallback effect
(the one defaulting to connections[0]) as-is to reconcile removed/stale
selections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c447ea0-7d61-42bc-bf11-ca287ad415c7
📒 Files selected for processing (18)
.github/ci/web-async-leaks.maxweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/api/types/messages.tsweb/src/components/layout/StatusBar.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/UsersPage.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/messages/ChannelListItem.stories.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/stores/messages.tsweb/src/stores/providers/crud-actions.tsweb/src/stores/tasks.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Lighthouse Site
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx}: No default may privilege a single region, currency, or locale. Every user-facing format resolves from: user/company setting -> browser/system -> neutral fallback.
Currency: never hardcode ISO 4217 codes or symbols. Backend:DEFAULT_CURRENCYfromsynthorg.budget.currencyor the runtimebudget.currencysetting. Frontend:DEFAULT_CURRENCYfrom@/utils/currenciesoruseSettingsStore().currency.
Field naming: no_usdsuffix on money fields anywhere. The type carries money semantics; the value is in the operator's configured currency.
Locale: never hardcode BCP 47 tags or call bare.toLocaleString()/.toLocaleDateString()/.toLocaleTimeString(). Use helpers in@/utils/formatwhich readgetLocale()from@/utils/locale. The backend has no operator-tunable locale setting; backendIntlformatting uses the system locale plus the browser timezone. Thecompany.name_localeslist controls procedural-name generation only; it does not feed number / date / time formatting.
Timezone: store UTC; render viaIntlwithout passingtimeZone(browser tz wins).
Date / number format: always viaIntl; no hand-rolled templates.
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/api/types/messages.tsweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/components/ui/error-boundary.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/tasks.tsweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/stores/messages.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsxweb/src/components/layout/StatusBar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Units: metric only. Spelling: International / British English UI default (
colour,behaviour,organise,centred,analyse,cancelled); document deviations. Spelling here is an editorial / UI-copy decision only; it does not affect runtime locale-sensitive formatting. Numbers, dates, times, currencies, and units still resolve via the user / company / browser / system fallback through@/utils/format,@/utils/locale,DEFAULT_CURRENCY, anduseSettingsStore().currency, with no contradiction to the locale-neutral defaults above.
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/api/types/messages.tsweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/components/ui/error-boundary.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/tasks.tsweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/stores/messages.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsxweb/src/components/layout/StatusBar.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse components from
web/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, Motion transitions, or BCP 47 locale strings; use design tokens,@/lib/motionpresets, and the helpers in@/utils/format. Enforced byscripts/check_web_design_system.py(PostToolUse hook on everyweb/src/edit).
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/api/types/messages.tsweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/components/ui/error-boundary.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/tasks.tsweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/stores/messages.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsxweb/src/components/layout/StatusBar.tsx
web/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Wrap the app in
<CSPProvider nonce={getCspNonce()}>+<MotionConfig nonce>so every inline<style>tag injected by Base UI and Motion carries the per-request CSP nonceUse Base UI's
renderprop as the polymorphism primitive throughout the dashboard; use the local<Slot>helper fromcomponents/ui/slot.tsxonly for<Button asChild>ergonomic support, using@base-ui/react/merge-props
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/components/ui/error-boundary.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsxweb/src/components/layout/StatusBar.tsx
web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Always use
createLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code; always use variable namelogPass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go through
sanitizeArgWrap attacker-controlled fields inside structured objects in
sanitizeForLog()before embedding in logsBe aware that TypeScript 6 has deprecated
baseUrlandesModuleInteropis always true;typesdefaults to[]sovitest/globalsetc. need explicit listing
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/api/types/messages.tsweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/workflow-editor/useWorkflowEditorCallbacks.tsweb/src/components/ui/error-boundary.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/tasks.tsweb/src/__tests__/pages/messages/ChannelListItem.test.tsxweb/src/stores/messages.tsweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsxweb/src/components/layout/StatusBar.tsx
web/src/pages/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<ListHeader>,<SearchFilterSort>,<Pagination>,<BulkActionBar>,<MetadataGrid>,<Breadcrumbs>for list-page primitives; root container usesspace-y-section-gap(majority pattern);<ErrorBanner>lands immediately after<ListHeader>before filter/pagination; pages with one-line mission statements pass via<ListHeader description="..." />For list layout choice: use Kanban grouping for status-flow domains where each row's column conveys lifecycle phase (Tasks, Requests); use flat scrollable list for queues without explicit phase semantics (Escalations, Approvals)
Use
useEmptyStateProps({ filteredCount, totalCount, filterActive, empty, filtered })from@/hooks/use-empty-state-propsto returnEmptyStateProps | nullso pages branch on a single value instead of duplicating the "no data ever" / "no data after filter" discriminator
Files:
web/src/pages/messages/ChannelListItem.stories.tsxweb/src/pages/messages/ChannelSidebar.tsxweb/src/pages/setup/WizardNavigation.tsxweb/src/pages/messages/ChannelListItem.tsxweb/src/pages/setup/SkipWizardForm.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/budget/AgentSpendingTable.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/UsersPage.tsx
web/src/**/*.stories.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use Storybook 10 which is ESM-only; essentials are built into core, but
@storybook/addon-docsis now separate; imports moved tostorybook/testandstorybook/actions
Files:
web/src/pages/messages/ChannelListItem.stories.tsx
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
All store mutation actions (create / update / delete) follow the
stores/connections/crud-actions.tspattern: wrap API calls intry/catch, success path updates state + emits success toast, failure path logs + emits error toast + returns sentinel (nullfor entity returns,falsefor delete); callers MUST NOT wrap store mutation calls intry/catchCapture
previousstate synchronously for optimistic mutations and restore incatchblock on failureFor list reads (
fetch*), seterror: string | nullon the store instead of toastingUse cursor-based pagination via
PaginationMetawith opaque cursors; keepnextCursor+hasMorein state (not offset arithmetic); early-return when!hasMore || !nextCursor; display counts come fromdata.lengthnot wire envelopetotalIf a store schedules timers or attaches event listeners, expose an equivalent cleanup hook for registration in the global test
afterEachFor any new WebSocket payload handler that ingests untrusted strings, route through
sanitizeWsString()orsanitizeWsEnum<T>(); raw(sanitizeWsString(x, n) ?? '') as EnumTypecasts are forbidden
Files:
web/src/stores/providers/crud-actions.tsweb/src/stores/tasks.tsweb/src/stores/messages.ts
web/src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
ALWAYS reuse existing components from
web/src/components/ui/before creating new ones; NEVER hardcode hex colors, font-family declarations, pixel spacing, Motion transition durations, BCP 47 locale literals ('en-US'), or currency symbols / codes; use design tokens,@/lib/motionpresets, helpers from@/utils/format, andDEFAULT_CURRENCYfrom@/utils/currenciesEvery shared UI component must have a Props interface named
<ComponentName>Propsexported from the same file (e.g.AgentCardPropsinagent-card.tsx) to make the contract greppable and let callers extend propsBase UI primitives are imported directly from
@base-ui/react/<subpath>and use the nativerenderprop for polymorphism; the local<Slot>helper is reserved only for<Button asChild>
Files:
web/src/components/ui/error-boundary.tsx
web/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Override MSW handlers per-case via
server.use(...), never usevi.mock('@/api/endpoints/*')Boot test setup with
onUnhandledRequest: 'error'to catch unhandled MSW requests
Files:
web/src/__tests__/pages/messages/ChannelListItem.test.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<StatusBadge>` component (defaults to `role="img"` with aria-label; `decorative` for adjacent-labeled, `announce` for live WS updates) instead of recreating status dots inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<MetricCard>`, `<Sparkline>`, `<ProgressGauge>`, `<TokenUsageBar>` instead of recreating KPI displays inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<SectionCard>` (titled wrapper with icon and action slot), `<AgentCard>`, `<DeptHealthBar>` instead of recreating cards inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<InputField>`, `<SelectField>`, `<SliderField>`, `<ToggleField>`, `<SegmentedControl>`, `<TagInput>`, `<SearchInput>` instead of recreating form fields inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<Drawer width="compact|narrow|default|wide">` (Base UI) for slide-in panels; do NOT add inline `w-[40vw]` overrides
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<Skeleton>` family, `<EmptyState>`, `<ErrorBoundary>`, `<ErrorBanner>`, `<ProgressIndicator>` instead of recreating loading / empty / error states inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `STATUS_COLORS` family from `@/styles/status-colors` (typed `Record<EnumValue, string>` lookups) instead of inline `Record<EnumValue, string>` constants per page
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<ConfirmDialog>` and `<Toast>` (Zustand-backed queue, NOT Base UI's Toast) for confirmations and notifications
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<CommandPalette>`, `<KeyboardShortcutHint>`, `<CommandCheatsheet>` instead of recreating Cmd+K / shortcuts functionality inline
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Use `<AnimatedPresence>`, `<StaggerGroup>`, `<LiveRegion>` (debounced ARIA live for WS updates) for animation and live region updates
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Do NOT adopt Base UI Toast component; use Zustand-backed queue instead
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Do NOT adopt Base UI Meter component; use `<ProgressGauge>` instead
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Do NOT adopt Base UI Select component; use native `<select>` for mobile picker UX
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T14:16:41.934Z
Learning: Do NOT currently adopt Base UI Combobox, Autocomplete, OTP Field, or Tooltip; revisit when needed
🔇 Additional comments (26)
web/src/components/layout/StatusBar.tsx (1)
37-43: A11y contract wording is now robust and accurate.Good update—this avoids brittle line-number references and correctly documents the required visible-label + dot pairing with button
aria-label, matching the implementation below.web/src/pages/workflow-editor/useWorkflowEditorCallbacks.ts (1)
122-131: Pluralisation and conditional toast description look correct.This cleanly resolves the prior
error(s)UI copy issue and keeps the success toast concise.web/src/components/ui/error-boundary.tsx (1)
100-100: Good fix: retry actions are now safe and consistent.Adding
type="button"on all “Try Again” controls removes unintended form-submit behaviour, and standardising on the sharedButtonkeeps fallback UX consistent across levels.Also applies to: 113-115, 133-135
web/src/pages/setup/WizardNavigation.tsx (1)
37-72: A11y linkage is correctly implemented for disabled “Next” reasoning.
useId()+ conditionalaria-describedbyon the Next button, paired with a conditionally rendered<p id={reasonId}>, cleanly solves the screen-reader association without dangling ID references.web/src/pages/UsersPage.tsx (4)
29-50: Good centralisation of role badge colour mapping.Using
ROLE_BADGE_COLORS[role]here keeps role styling consistent and avoids per-page colour drift.Based on learnings: Use
STATUS_COLORSfamily from@/styles/status-colors(typedRecord<EnumValue, string>lookups) instead of inlineRecord<EnumValue, string>constants per page.
109-113: List-page header and container now align with page conventions.Nice update to
space-y-section-gapandListHeader.description; this matches the shared list-page pattern.As per coding guidelines:
web/src/pages/**/*.tsx: root container usesspace-y-section-gapand pages with one-line mission statements pass via<ListHeader description="..." />.
127-137: Search UI now correctly uses the list-page primitive wrapper.
SearchFilterSort+SearchInputintegration looks clean and consistent with the page system.As per coding guidelines:
web/src/pages/**/*.tsx: Use<SearchFilterSort>for list-page primitives.
162-166: Responsive card-grid transition is solid.Keeping
ul/lisemantics while addingmd/lgdensity scaling is a good UX improvement..github/ci/web-async-leaks.max (1)
1-23: Well-documented threshold increase.The ceiling adjustment is properly justified, with clear accounting for the +14 new tests (5 + 1 + 8), transparent local-vs-CI baseline explanation (~75 local → ~95-96 CI), and a concrete plan to reduce the threshold once issue
#1468lands. The mechanical nature of the increase (new test surface rather than regression) is well-explained.web/src/stores/providers/crud-actions.ts (1)
80-80: Good consistency win on CRUD toast titles.Using
getCrudErrorTitle(...)across these mutation failures aligns provider CRUD behaviour with the shared error-title mapping and keeps 409/validation/auth messaging consistent.Also applies to: 103-103, 130-130, 164-164
web/src/pages/budget/AgentSpendingTable.tsx (2)
49-49: Locale-aware name sorting looks good.Using
localeComparewithgetLocale()is a solid improvement for user-consistent ordering.
148-148: Themd:min-w-[40rem]guard is the right responsive adjustment.This keeps small-screen behavior fluid while preserving table readability at medium+ widths.
web/src/pages/setup/SkipWizardForm.tsx (2)
30-46: Well-structured error handling for store mutations.The implementation correctly addresses the store-mutation boundary:
- Line 30 ensures the store has the trimmed name before
submitCompany()reads it viaget().companyName- Lines 39-46 properly inspect durable store state (
companyResponse,companyError) rather than relying on exceptions- The null-coalescing fallback message at line 43 handles the edge case where
companyErrorisn't setThis follows the pattern where the store owns error state and callers branch on that state rather than wrapping in try/catch.
55-63: Catch block correctly scoped to completion failures only.Since
submitCompany()never throws (per the store implementation), the catch block now handles onlywizardCompleteSetup()failures. The partial-success messaging correctly indicates that the company exists and provides actionable recovery guidance via the wizard's Complete step.web/src/stores/tasks.ts (4)
6-13: LGTM!The imports are correctly updated to bring in
sanitizeWsEnumfrom the dedicated utility module andTASK_SOURCE_VALUESfrom the enum definitions.
35-38: LGTM!The updated comments clearly document the design decision to defer enum validation to
sanitizeWsEnumrather than the shape guard, enabling graceful handling of new backend enum values during rolling deploys.
174-180: LGTM!The
sourcefield correctly preservesundefined/nullsignals before applyingsanitizeWsEnum, maintaining the optional semantics of the field.
260-316: LGTM!The shape guard correctly differentiates between:
- Display-facing enum fields (
status,priority,type,source) that accept any string and defer tosanitizeWsEnumfor forward compatibility- Behavioural enum fields (
estimated_complexity,task_structure,coordination_topology) that retain strict allowlist validation because they drive orchestration logicThe comments clearly document this design rationale.
web/src/api/types/messages.ts (3)
15-19: LGTM!The
MESSAGE_TYPE_VALUESarray correctly enumerates all values from theMessageTypeunion with theas const satisfiespattern ensuring compile-time synchronisation.
23-25: LGTM!The
MESSAGE_PRIORITY_VALUESarray correctly captures all priority levels.
29-31: LGTM!The
ATTACHMENT_TYPE_VALUESarray correctly enumerates all attachment types.web/src/stores/messages.ts (4)
6-14: LGTM!The sanitiser imports are correctly sourced from the dedicated utility module, and the enum value arrays are imported from the types definition. The comment at lines 6-8 helpfully documents the rationale for this structure.
128-139: LGTM!The
sanitizeWsEnumcalls use sensible neutral fallbacks:'announcement'for message type and'normal'for priority. Thefieldoptions enable structured warning output for observability.
155-161: LGTM!The attachment type is correctly sanitised via
sanitizeWsEnumwith'file'as a sensible neutral fallback.
186-197: LGTM!The returned
Messageobject correctly uses the enum-sanitised values directly without type casts, assanitizeWsEnumreturns the properly typed value from the allowlist.web/src/pages/WebhookReceiptsPage.tsx (1)
194-201: Good move on the error-banner placement.This now matches the list-page layout pattern and keeps the retry affordance above the filters.
As per coding guidelines, "
<ErrorBanner>lands immediately after<ListHeader>before filter/pagination".
…out (#1710) Round 3 of CodeRabbit findings on PR #1718 plus a lazy-chart test flake exposed by the round-2 ceiling raise. CodeRabbit findings: * providers/crud-actions.ts: every log.error metadata object now routes attacker-controlled name / preset_name through sanitizeForLog from @/utils/logging. The values flow from the CRUD form (operator-supplied) into structured logs; without sanitization, an embedded control / bidi-override sequence in a provider name would survive into log sinks. createProvider / createFromPreset / updateProvider / deleteProvider all updated. * WebhookReceiptsPage: ``selected`` now seeds from '' instead of urlConnection. The seed effect immediately below already validates that the URL value exists in the connections list before applying it, so the previous direct-from-URL initialisation defeated the validation by triggering reload() on the first render against an unvalidated name. Initialising to '' lets the seed effect own the validation path on every code path (cold mount + later URL changes). * messages.ts: ``!to`` added to the blank-rejection guard. The shape guard at line 80 already requires ``typeof c.to === 'string'`` and parseWsMessage's contract docstring lists ``to`` as a required string, so a sanitized-blank ``to`` should reject the whole frame the same way a blank id / sender / channel does. The hasBlankTo flag is also surfaced in the structured log so the WS-drift signal points at the right field. * ApprovalCard countdown: reset + interval-restart collapsed into ONE useEffect keyed on [seconds_remaining, isPending] so a refresh from the WS layer (e.g. seconds_remaining bumped 5 -> 60) restarts the interval on a clean 1-second cadence instead of inheriting the previous interval's mid-tick offset. setCountdown is deferred to a microtask per the ESLint set-state-in-effect rule; setInterval starts immediately so the visible tick cadence matches the prop. Lazy-chart test flake: * BudgetPage.test.tsx: 'renders Spend Burn section' failed once under heavy parallel load at 5860ms. SpendBurnChart and CostBreakdownChart are React.lazy + Suspense (the recharts split-chunk landed in this PR), so the dynamic import races the test's default 5s testTimeout when 8 workers are fighting for CPU. Bumped the findByText timeout 5000 -> 10000 AND added a 15000ms per-test timeout (third arg on it()) so Vitest's testTimeout doesn't kill the case before findByText completes. The comment block above the tests was updated to reflect the new threshold. Verification: * npm run lint: zero warnings * npm run type-check: clean * 2994 tests pass; local 75 leaks; under 100 ceiling.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/WebhookReceiptsPage.tsx (1)
71-105:⚠️ Potential issue | 🔴 CriticalFix the race condition where the URL deep-link (?connection=) is overwritten by the reconciliation effect on cold mount.
When connections load for the first time with a URL parameter like
?connection=SecondConnection, both effects schedule microtask updates:
- Effect 1 (URL sync) schedules
setSelected('SecondConnection')via functional update- Effect 2 (reconciliation) schedules
setSelected(connections[0]?.name)via direct updateBoth microtasks execute in the same batch; the direct update runs after the functional update and overwrites it, causing the URL deep-link to be lost and the first connection to be selected instead.
Add
urlConnectionto the reconciliation effect's dependency array (line 105) and skip the override when a valid URL-seeded connection exists:}, [connections, selected, urlConnection])Then modify the effect body to check:
const exists = selected !== '' && connections.some((c) => c.name === selected) if (exists) return // Only default to first connection if no URL param is actively forcing a different choice if (urlConnection && connections.some((c) => c.name === urlConnection)) return let cancelled = false void Promise.resolve().then(() => { if (cancelled) return setSelected(connections[0]?.name ?? '') })This ensures the reconciliation effect respects the URL-seeded value instead of clobbering it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/WebhookReceiptsPage.tsx` around lines 71 - 105, The reconciliation useEffect that defaults selected to the first connection can clobber a URL-seeded value; update its dependency array to include urlConnection and guard the effect so it returns early when a valid urlConnection exists (i.e., when urlConnection is non-empty and connections.some(c => c.name === urlConnection)), then keep the existing microtask setSelected(connections[0]?.name ?? '') behavior only when no URL-forced connection applies; reference the existing useEffect handling connections/selected, the urlConnection variable, and the setSelected state updater to implement this change.
♻️ Duplicate comments (1)
web/src/pages/approvals/ApprovalCard.tsx (1)
61-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the countdown against one last stale interval tick.
Line 76 clears future ticks, but it does not cancel a callback already queued from the previous interval. On a WS refresh that lands near the tick boundary, that stale callback can still run after
setCountdown(approval.seconds_remaining)and immediately drop the refreshed value by one.💡 Minimal fix
+ const countdownGenerationRef = useRef(0) + useEffect(() => { + const generation = ++countdownGenerationRef.current let cancelled = false void Promise.resolve().then(() => { - if (cancelled) return + if (cancelled || generation !== countdownGenerationRef.current) return setCountdown(approval.seconds_remaining) }) if ( !isPending || approval.seconds_remaining === null || approval.seconds_remaining <= 0 ) { return () => { cancelled = true } } const timer = setInterval(() => { + if (generation !== countdownGenerationRef.current) return setCountdown((prev) => { if (prev === null || prev <= 1) return 0 return prev - 1 }) }, 1000)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/approvals/ApprovalCard.tsx` around lines 61 - 86, The interval's queued callback can still run and decrement a freshly-set value; fix by guarding both the microtask and the interval callback with the effect-local cancelled flag: inside the Promise.resolve().then() only call setCountdown if cancelled is false, and at the start of the setInterval handler return early when cancelled is true before calling setCountdown((prev) => ...). Update the effect in ApprovalCard so the local cancelled boolean is checked in both places (the immediate Promise callback and the timer callback) to prevent a stale tick from mutating the refreshed approval.seconds_remaining.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/pages/WebhookReceiptsPage.tsx`:
- Around line 71-105: The reconciliation useEffect that defaults selected to the
first connection can clobber a URL-seeded value; update its dependency array to
include urlConnection and guard the effect so it returns early when a valid
urlConnection exists (i.e., when urlConnection is non-empty and
connections.some(c => c.name === urlConnection)), then keep the existing
microtask setSelected(connections[0]?.name ?? '') behavior only when no
URL-forced connection applies; reference the existing useEffect handling
connections/selected, the urlConnection variable, and the setSelected state
updater to implement this change.
---
Duplicate comments:
In `@web/src/pages/approvals/ApprovalCard.tsx`:
- Around line 61-86: The interval's queued callback can still run and decrement
a freshly-set value; fix by guarding both the microtask and the interval
callback with the effect-local cancelled flag: inside the
Promise.resolve().then() only call setCountdown if cancelled is false, and at
the start of the setInterval handler return early when cancelled is true before
calling setCountdown((prev) => ...). Update the effect in ApprovalCard so the
local cancelled boolean is checked in both places (the immediate Promise
callback and the timer callback) to prevent a stale tick from mutating the
refreshed approval.seconds_remaining.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 144cbb75-06ce-4368-be57-0c2b005c940c
📒 Files selected for processing (5)
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/messages.tsweb/src/stores/providers/crud-actions.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Preview
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Dashboard Test
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx}: No default may privilege a single region, currency, or locale. Every user-facing format resolves from: user/company setting -> browser/system -> neutral fallback.
Currency: never hardcode ISO 4217 codes or symbols. Backend:DEFAULT_CURRENCYfromsynthorg.budget.currencyor the runtimebudget.currencysetting. Frontend:DEFAULT_CURRENCYfrom@/utils/currenciesoruseSettingsStore().currency.
Field naming: no_usdsuffix on money fields anywhere. The type carries money semantics; the value is in the operator's configured currency.
Locale: never hardcode BCP 47 tags or call bare.toLocaleString()/.toLocaleDateString()/.toLocaleTimeString(). Use helpers in@/utils/formatwhich readgetLocale()from@/utils/locale. The backend has no operator-tunable locale setting; backendIntlformatting uses the system locale plus the browser timezone. Thecompany.name_localeslist controls procedural-name generation only; it does not feed number / date / time formatting.
Timezone: store UTC; render viaIntlwithout passingtimeZone(browser tz wins).
Date / number format: always viaIntl; no hand-rolled templates.
Files:
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/WebhookReceiptsPage.tsxweb/src/stores/messages.tsweb/src/pages/approvals/ApprovalCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Units: metric only. Spelling: International / British English UI default (
colour,behaviour,organise,centred,analyse,cancelled); document deviations. Spelling here is an editorial / UI-copy decision only; it does not affect runtime locale-sensitive formatting. Numbers, dates, times, currencies, and units still resolve via the user / company / browser / system fallback through@/utils/format,@/utils/locale,DEFAULT_CURRENCY, anduseSettingsStore().currency, with no contradiction to the locale-neutral defaults above.
Files:
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/WebhookReceiptsPage.tsxweb/src/stores/messages.tsweb/src/pages/approvals/ApprovalCard.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse components from
web/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, Motion transitions, or BCP 47 locale strings; use design tokens,@/lib/motionpresets, and the helpers in@/utils/format. Enforced byscripts/check_web_design_system.py(PostToolUse hook on everyweb/src/edit).Always use
createLoggerfrom@/lib/loggerand never use bareconsole.warn/console.error/console.debugin application codeVariable name for logger instances must always be
log(e.g.,const log = createLogger('module-name'))Pass dynamic/untrusted values as separate arguments to logger methods rather than interpolating them into the message string to ensure they go through
sanitizeArgWrap attacker-controlled fields inside structured objects with
sanitizeForLog()before embedding them in log calls
Files:
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/stores/providers/crud-actions.tsweb/src/pages/WebhookReceiptsPage.tsxweb/src/stores/messages.tsweb/src/pages/approvals/ApprovalCard.tsx
web/src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Test setup must boot with
onUnhandledRequest: 'error'and override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*')
Files:
web/src/__tests__/pages/BudgetPage.test.tsx
web/src/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Import Dialog, AlertDialog, Popover, Tabs, Menu, Drawer, and CSPProvider directly from
@base-ui/react/<subpath>(adopted primitives)
Files:
web/src/__tests__/pages/BudgetPage.test.tsxweb/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
All Zustand store mutation actions (create/update/delete) must wrap API calls in
try/catch, emit success/error toasts on respective paths, and return sentinels (nullfor entity returns,falsefor delete operations)Optimistic mutations in Zustand stores must capture
previousstate synchronously and restore it in thecatchblockAny new store that schedules timers or attaches event listeners must expose a cleanup hook and register it in the global test
afterEach
Files:
web/src/stores/providers/crud-actions.tsweb/src/stores/messages.ts
web/src/{api,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
List endpoints must use cursor-based pagination via
PaginationMetawithnextCursorandhasMorein state; display counts must come fromdata.lengthrather than a wiretotal
Files:
web/src/stores/providers/crud-actions.tsweb/src/stores/messages.ts
web/src/{stores,utils}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
sanitizeWsString()to clamp every WebSocket-supplied string (strips C0 controls + bidi-overrides + caps length)Use
sanitizeWsEnum<T>(value, allowlist, fallback, { field })for enum-allowlist validation on WebSocket payloads with fallback for unknown valuesDo not use raw
(sanitizeWsString(x, n) ?? '') as EnumTypecasts; always route WebSocket payload handlers through sanitization helpers
Files:
web/src/stores/providers/crud-actions.tsweb/src/stores/messages.ts
web/src/{components,pages,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Callers MUST NOT wrap Zustand store mutation calls in
try/catch; error handling and UX belongs entirely in the store
Files:
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
web/src/{components,pages,styles}/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Never hardcode hex colors, font-family declarations, pixel spacing, Motion transition durations, BCP 47 locale literals, or currency symbols/codes; use design tokens,
@/lib/motionpresets,@/utils/formathelpers, andDEFAULT_CURRENCY
Files:
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
web/src/{components,pages}/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<StatusBadge>for status dots (defaults torole="img"with aria-label;decorativefor adjacent-labeled,announcefor live WebSocket updates)Use
<MetricCard>,<Sparkline>,<ProgressGauge>, and<TokenUsageBar>for KPI displays instead of creating inline versionsUse
<SectionCard>for titled card wrappers with icon and action slots; use<AgentCard>and<DeptHealthBar>for domain-specific cardsUse
<InputField>,<SelectField>,<SliderField>,<ToggleField>,<SegmentedControl>,<TagInput>, and<SearchInput>for form fields instead of custom implementationsUse
<Drawer width="compact|narrow|default|wide">for slide-in panels from Base UI; do NOT add inlinew-[40vw]overridesUse
<Skeleton>family,<EmptyState>,<ErrorBoundary>,<ErrorBanner>, and<ProgressIndicator>for loading/empty/error states instead of inline versionsUse
useEmptyStateProps({ filteredCount, totalCount, filterActive, empty, filtered })from@/hooks/use-empty-state-propsto branch on empty states instead of duplicating the 'no data ever' / 'no data after filter' discriminatorUse
<ConfirmDialog>and<Toast>(Zustand-backed queue) for confirmations and notifications; do NOT use Base UI's Toast primitiveUse
<CommandPalette>,<KeyboardShortcutHint>, and<CommandCheatsheet>for command palette and keyboard shortcuts instead of inline implementationsUse
<AnimatedPresence>,<StaggerGroup>, and<LiveRegion>(debounced ARIA live for WebSocket updates) for animations and accessibility
Files:
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
web/src/pages/**/*.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<ListHeader>,<SearchFilterSort>,<Pagination>,<BulkActionBar>,<MetadataGrid>, and<Breadcrumbs>for list-page primitivesList page root container must use
space-y-section-gapclass (prefer overflex flex-col gap-section-gap); place<ErrorBanner>immediately after<ListHeader>before filter/pagination rows; pass one-line mission statement via<ListHeader description="..." />
Files:
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
web/src/{components,pages,styles}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use typed
STATUS_COLORSfamily from@/styles/status-colorsfor status/role/risk/urgency badge classes; never create inlineRecord<EnumValue, string>constants per page
Files:
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:01:15.412Z
Learning: WebSocket wire protocol constants (`WS_PROTOCOL_VERSION`, `WS_MAX_MESSAGE_SIZE`, `WS_HEARTBEAT_INTERVAL_MS`, `WS_PONG_TIMEOUT_MS`, `LOG_SANITIZE_MAX_LENGTH`) must stay in lockstep with Python backend definitions in `src/synthorg/api/ws_models.py` and `src/synthorg/api/controllers/ws.py`
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:01:15.412Z
Learning: Always reuse existing components from `web/src/components/ui/` before creating new ones
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:01:15.412Z
Learning: Choose Kanban grouping for status-flow domains where each row's column conveys lifecycle phase (Tasks, Requests); use flat scrollable lists for queues without explicit phase semantics (Escalations, Approvals)
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:01:15.412Z
Learning: Do not adopt Base UI Toast, Meter, Select, Combobox, Autocomplete, OTP Field, or Tooltip; use custom equivalents (Zustand-backed Toast, ProgressGauge, native select, etc.) or defer until needed
🔇 Additional comments (10)
web/src/__tests__/pages/BudgetPage.test.tsx (1)
124-145: Good stabilization for lazy-loaded chart assertions.Switching these two checks to async
findByTextwith explicit timeout and per-test budget is appropriate forReact.lazy/Suspenserendering and should reduce CI flakiness without masking real regressions.web/src/stores/providers/crud-actions.ts (1)
78-79: Sanitizeerrormetadata in CRUD failure logs.
getErrorMessage(err)is still logged unsanitized in the structured metadata payload across all four catch blocks.Suggested patch
- log.error('createProvider failed:', { name: sanitizeForLog(data.name), error: getErrorMessage(err) }) + log.error('createProvider failed:', { + name: sanitizeForLog(data.name), + error: sanitizeForLog(getErrorMessage(err)), + }) - log.error('createFromPreset failed:', { - name: sanitizeForLog(data.name), - preset: sanitizeForLog(data.preset_name), - error: getErrorMessage(err), - }) + log.error('createFromPreset failed:', { + name: sanitizeForLog(data.name), + preset: sanitizeForLog(data.preset_name), + error: sanitizeForLog(getErrorMessage(err)), + }) - log.error('updateProvider failed:', { name: sanitizeForLog(name), error: getErrorMessage(err) }) + log.error('updateProvider failed:', { + name: sanitizeForLog(name), + error: sanitizeForLog(getErrorMessage(err)), + }) - log.error('deleteProvider failed:', { name: sanitizeForLog(name), error: getErrorMessage(err) }) + log.error('deleteProvider failed:', { + name: sanitizeForLog(name), + error: sanitizeForLog(getErrorMessage(err)), + })As per coding guidelines
web/src/**/*.{ts,tsx}: “Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding them in log calls”.Also applies to: 101-105, 132-133, 166-167
web/src/stores/messages.ts (5)
6-14: LGTM!The import refactoring correctly addresses the previous review feedback by sourcing sanitizers from the dedicated utility module rather than
@/stores/notifications, eliminating cross-store coupling and circular-init risk. The allowlist imports from@/api/types/messagesprovide the typed constants needed for enum validation.
128-139: LGTM!The migration from raw
ascasts tosanitizeWsEnumcorrectly implements the typed sanitization pattern. The fallback values ('announcement'for type,'normal'for priority) are valid members of their respective allowlists and represent safe UI defaults. Thefieldoption provides diagnostic context for observability when unknown values are encountered.
141-148: LGTM!The rejection condition correctly reflects that
typeandprioritynow have guaranteed fallbacks viasanitizeWsEnum, so only core identity and routing fields require non-blank validation. AddinghasBlankToto the error context improves diagnostic completeness.
156-162: LGTM!Consistent application of
sanitizeWsEnumfor attachment types with'file'as a safe neutral fallback. The field name'message.attachments[].type'clearly indicates array element context for debugging.
187-198: LGTM!The explicit object construction correctly uses the typed values returned by
sanitizeWsEnumwithout requiring unsafe casts. The comment at lines 183-186 correctly explains why a spread is avoided.web/src/pages/WebhookReceiptsPage.tsx (3)
152-155: Well-structured sanitisation for attacker-controlled fields.The connection name originates from URL/dropdown (operator-controlled) and is correctly wrapped with
sanitizeForLogbefore structured logging, aligning with security guidelines.
191-203: LGTM — ErrorBanner placement and container layout follow conventions.
- Root container uses
space-y-section-gapas required<ErrorBanner>placed immediately after<ListHeader>and before filter rows- Retry handler correctly wires to
reload()
256-265: Accessible status rendering withdecorativebadge.
StatusBadgeusesdecorativesince the adjacent<span>provides the visible text label ({row.status}). This prevents redundant screen-reader announcements while maintaining the visual indicator.
Round 4 of CodeRabbit findings on PR #1718. CI was already green for da9e61f; this round only addresses the new review (id 4215155295) that landed at 15:08 UTC. CodeRabbit findings: * WebhookReceiptsPage URL deep-link race (CRITICAL): the reconciliation effect (defaults selected -> connections[0]) could fire alongside the URL-seed effect on cold mount. Both schedule microtask updates; the direct setter in the reconciliation effect could land after the seed effect's functional setter and clobber the URL-forced choice. Now: ``urlConnection`` is in the dep array and the effect early-returns when a valid URL value exists in the loaded list, so the seed effect retains exclusive ownership of the URL-driven path. * ApprovalCard countdown stale-tick guard (MINOR): the existing ``cancelled`` flag was checked in the microtask but not in the setInterval callback. A tick that fires between the prop refresh and the cleanup running could decrement a freshly-set countdown by one. Added the same ``cancelled`` early-return at the top of the timer callback so the stale tick is a no-op once the new effect's cleanup has fired. * providers/crud-actions error metadata sanitization (MAJOR): round-3 wrapped name / preset_name with sanitizeForLog but left ``error: getErrorMessage(err)`` raw across all four catch blocks. Backend error strings can echo operator-supplied input back to the caller, so the message is itself attacker-reachable. All four call sites (createProvider, createFromPreset, updateProvider, deleteProvider) now route the error through sanitizeForLog as well. Verification: * npm run lint: zero warnings * npm run type-check: clean * 2994 tests pass; local 75 leaks; under 100 ceiling.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/approvals/ApprovalCard.tsx`:
- Around line 79-89: The interval never gets cleared when countdown reaches 0;
update the effect in ApprovalCard (the setInterval/timer logic using cancelled
and setCountdown) to avoid creating the timer when countdown <= 0 or add a
separate useEffect watching countdown to clearInterval(timer) when it becomes 0;
specifically, reference the existing timer, setInterval, setCountdown and
countdown variables: either early-return from the effect if countdown <= 0 (so
no timer is created) or store the timer ID in a ref and
clearInterval(ref.current) inside a useEffect that triggers when countdown
changes to 0, and remove clearing logic from inside the state updater to
preserve React purity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ebeb44d1-0ed2-4084-bc86-6b0f39346273
📒 Files selected for processing (3)
web/src/pages/WebhookReceiptsPage.tsxweb/src/pages/approvals/ApprovalCard.tsxweb/src/stores/providers/crud-actions.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Preview
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Dashboard Test
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx}: No default may privilege a single region, currency, or locale. Every user-facing format resolves from: user/company setting -> browser/system -> neutral fallback.
Currency: never hardcode ISO 4217 codes or symbols. Backend:DEFAULT_CURRENCYfromsynthorg.budget.currencyor the runtimebudget.currencysetting. Frontend:DEFAULT_CURRENCYfrom@/utils/currenciesoruseSettingsStore().currency.
Field naming: no_usdsuffix on money fields anywhere. The type carries money semantics; the value is in the operator's configured currency.
Locale: never hardcode BCP 47 tags or call bare.toLocaleString()/.toLocaleDateString()/.toLocaleTimeString(). Use helpers in@/utils/formatwhich readgetLocale()from@/utils/locale. The backend has no operator-tunable locale setting; backendIntlformatting uses the system locale plus the browser timezone. Thecompany.name_localeslist controls procedural-name generation only; it does not feed number / date / time formatting.
Timezone: store UTC; render viaIntlwithout passingtimeZone(browser tz wins).
Date / number format: always viaIntl; no hand-rolled templates.
Files:
web/src/stores/providers/crud-actions.tsweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/WebhookReceiptsPage.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Units: metric only. Spelling: International / British English UI default (
colour,behaviour,organise,centred,analyse,cancelled); document deviations. Spelling here is an editorial / UI-copy decision only; it does not affect runtime locale-sensitive formatting. Numbers, dates, times, currencies, and units still resolve via the user / company / browser / system fallback through@/utils/format,@/utils/locale,DEFAULT_CURRENCY, anduseSettingsStore().currency, with no contradiction to the locale-neutral defaults above.
Files:
web/src/stores/providers/crud-actions.tsweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/WebhookReceiptsPage.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse components from
web/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, Motion transitions, or BCP 47 locale strings; use design tokens,@/lib/motionpresets, and the helpers in@/utils/format. Enforced byscripts/check_web_design_system.py(PostToolUse hook on everyweb/src/edit).A PostToolUse hook (
scripts/check_web_design_system.py) validates everyweb/src/edit and flags hardcoded hex/rgba/fonts/Motion durations/locale literals/bare.toLocale*String()calls/missing Storybook stories/duplicate component patterns/complex.map()blocks; fix every violation before proceeding
Files:
web/src/stores/providers/crud-actions.tsweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/WebhookReceiptsPage.tsx
web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Every inline
<style>tag injected by Base UI and Motion must carry the per-request CSP nonce via<CSPProvider nonce={getCspNonce()}>and<MotionConfig nonce>wrapping inApp.tsxUse Base UI's
renderprop as the polymorphism primitive throughout the dashboard; only use the local<Slot>helper fromcomponents/ui/slot.tsxwith@base-ui/react/merge-propsfor<Button asChild>ergonomicAlways use
createLoggerfrom@/lib/loggerwith variable namelog; never use bareconsole.warn,console.error, orconsole.debugin application code (onlylogger.tsitself may use bare console methods)Pass dynamic/untrusted values as separate args to logger methods (not interpolated into the message string) so they go through
sanitizeArg; wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embeddingAll store mutation actions (create / update / delete) must follow the
stores/connections/crud-actions.tspattern: wrap API call intry/catch, success path updates state + emits success toast, failure path logs + emits error toast + returns sentinel (nullfor entity returns,falsefor delete)Callers MUST NOT wrap store mutation calls in
try/catch; the store owns the error UXOptimistic mutations must capture
previousstate synchronously and restore incatchList reads (
fetch*) must seterror: string | nullon the store instead of toastingCursor pagination is mandatory: list endpoints use opaque cursor-based paging via
PaginationMeta; stores keepnextCursor+hasMorein state (not offset arithmetic) and early-return when!hasMore || !nextCursor; display counts come fromdata.length
getLiveness()is always 200 while the process is alive;getReadiness()is 200 healthy / 503 unavailable (binary'ok' | 'unavailable'outcome); any new caller must handle the 503 path explicitly
web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler...
Files:
web/src/stores/providers/crud-actions.tsweb/src/pages/approvals/ApprovalCard.tsxweb/src/pages/WebhookReceiptsPage.tsx
web/**/*pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use
<ListHeader>/<SearchFilterSort>/<Pagination>/<BulkActionBar>/<MetadataGrid>/<Breadcrumbs>for list-page primitives; root container usesspace-y-section-gap;<ErrorBanner>lands immediately after<ListHeader>; pages with one-line mission statement pass it via<ListHeader description="..." />For list pages, choose Kanban grouping for status-flow domains where each row's column conveys lifecycle phase (Tasks, Requests); use flat scrollable list for queues without explicit phase semantics (Escalations, Approvals)
Files:
web/src/pages/approvals/ApprovalCard.tsxweb/src/pages/WebhookReceiptsPage.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:29:29.802Z
Learning: Toast, Meter, Select, Combobox / Autocomplete / OTP Field / Tooltip are not adopted from Base UI; we use alternatives (Zustand-backed Toast queue, `<ProgressGauge>` for Meter, native `<select>` for Select, no current call sites for others)
Learnt from: CR
Repo: Aureliolo/synthorg
Timestamp: 2026-05-02T15:29:29.802Z
Learning: Minimum Node 20.19+, Vite 5+, Vitest 3+ are required; update tool versions in package.json and CI workflows accordingly
🔇 Additional comments (3)
web/src/stores/providers/crud-actions.ts (1)
16-17: Good cleanup: failure paths now follow the required logging and toast contract.The updated catch blocks correctly log through
log.error, sanitize attacker-controlled fields, applygetCrudErrorTitle(...), and return the expected sentinels.Also applies to: 78-85, 104-112, 135-142, 172-179
web/src/pages/WebhookReceiptsPage.tsx (2)
50-113: Deep-link reconciliation looks solid.The URL-seeded selection and fallback effect now co-ordinate correctly, so a valid
?connection=survives cold loads and mid-session query changes without being clobbered.
204-211: Error banner placement now matches the page layout contract.Rendering
<ErrorBanner>directly after<ListHeader>keeps the failure state consistent with the shared list-page pattern. As per coding guidelines,web/**/*pages/**/*.{ts,tsx}: "<ErrorBanner>lands immediately after<ListHeader>".
Round 5 of CodeRabbit findings on PR #1718. CI was already green for c450e68; this round addresses the only new actionable finding from review id 4215184268 / inline comment 3176836578. CodeRabbit finding: * ApprovalCard countdown leak (MAJOR): when ``countdown`` reached zero the setInterval callback kept firing every second indefinitely until the component unmounted or the prop refreshed. React bails on the no-op re-render (state stays at 0) but the timer itself continues, so an expired card kept mounted in the paginated approvals queue accumulates a dormant 1-Hz interval. Fix: stash the timer id in ``timerRef``, then clear it from a sibling effect that fires when ``countdown`` lands on zero. Mutating the timer from inside the ``setCountdown`` updater would violate React purity (state updaters must be side-effect free), so the cleanup lives in its own ``useEffect`` keyed on ``countdown`` instead. Verification: * npm run lint: zero warnings * npm run type-check: clean * 2994 tests pass; local 75 leaks; under 100 ceiling.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
<!-- HIGHLIGHTS_START --> ## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Frontend and UX polishing improves user interface responsiveness and visual consistency. - API hygiene and validation enhancements provide smoother and more reliable interactions. ### What's new - Introduced typed-boundary helpers enabling better type safety and parse_typed workflows. - Added codebase-audit skill prompt tuning for improved project auditing. ### Under the hood - Eliminated flaky tests caused by module-level state for more stable test outcomes. - Unified image tag management under CLI and Renovate for consistent dependency updates. - Added cross-PR file-overlap analysis to the review dependency pull request skill. - Updated multiple dependencies including Python, Web, CLI, and container libraries. - Improved CI tooling and lock file maintenance for better build reliability. <!-- HIGHLIGHTS_END --> :robot: I have created a release *beep* *boop* --- ## [0.7.8](v0.7.7...v0.7.8) (2026-05-03) ### Features * **api:** typed-boundary helper + codebase-audit skill prompt tuning ([#1712](#1712)) ([40ee65b](40ee65b)) * **boundary:** RFC [#1711](#1711) Phases 2 + 3 — typed boundaries via parse_typed ([#1720](#1720)) ([7b9f409](7b9f409)) ### Bug Fixes * **api:** audit cleanup B -- API hygiene & validation ([#1719](#1719)) ([3d790d9](3d790d9)) * audit cleanup C - persistence, concurrency & data integrity ([#1708](#1708)) ([#1717](#1717)) ([bcce097](bcce097)) * **test:** exterminate xdist-flaky tests with module-level state ([#1713](#1713)) ([#1721](#1721)) ([8d258dd](8d258dd)) * **web:** audit cleanup E -- frontend & UX polish ([#1710](#1710)) ([#1718](#1718)) ([3a3591a](3a3591a)) ### Refactoring * **cli:** single source of truth for DHI image tags + Renovate manager ([#1723](#1723)) ([57980a2](57980a2)) ### Documentation * audit cleanup D -- public-facing & docs sync ([#1709](#1709)) ([#1715](#1715)) ([ade03b7](ade03b7)) ### Tests * **engine:** make TestDrainTimeout deterministic + preserve subclass type in [@Ontology](https://github.com/ontology)_entity ([#1729](#1729)) ([b00fb05](b00fb05)) ### CI/CD * Update CI tool dependencies ([#1703](#1703)) ([355a9ff](355a9ff)) ### Maintenance * add cross-PR file-overlap analysis to review-dep-pr skill ([#1722](#1722)) ([3861d8a](3861d8a)) * **ci:** unify apko-version under workflow env so Renovate manages it everywhere ([#1724](#1724)) ([9c0a7fd](9c0a7fd)) * consolidate DHI image-pin custom regex managers ([#1726](#1726)) ([b8b0cba](b8b0cba)) * **deps:** update dependency chainguard-dev/melange to v0.50.4 ([#1701](#1701)) ([8cbf83a](8cbf83a)) * Lock file maintenance ([#1705](#1705)) ([414cfea](414cfea)) * Lock file maintenance ([#1727](#1727)) ([5cb1212](5cb1212)) * Update CLI dependencies ([#1702](#1702)) ([9fb57b9](9fb57b9)) * Update Container dependencies ([#1698](#1698)) ([6d24fd6](6d24fd6)) * Update dependency @eslint-react/eslint-plugin to v5 ([#1704](#1704)) ([1cb1294](1cb1294)) * Update Python dependencies ([#1699](#1699)) ([8e7af3a](8e7af3a)) * Update Python dependencies to v4.15.0 ([#1725](#1725)) ([69164c8](69164c8)) * Update Web dependencies ([#1700](#1700)) ([715300d](715300d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Closes #1710.
Frontend slice of the 2026-05-01 codebase audit. Pure
web/src/-- zero backend overlap.What changed
Wave 1 -- foundations (cross-cutting primitives)
sanitizeWsEnum<T>(value, allowlist, fallback, { field })inweb/src/utils/ws-sanitize.tsvalidates a WS-sourced string against an enum allowlist and emits a structuredws.enum.unknownwarning + supplied fallback on miss. Replaces 13 unsafe(sanitizeWsString(x, 64) ?? '') as EnumTypecast sites in approvals / meetings / messages / tasks stores. Forward-compatible with rolling backend deploys -- a server can ship a new enum value before the client knows it without breaking UI.web/src/api/types/websocket.ts) -- ~62 per-event payload interfaces +WsEventPayloadMap+WsTypedEvent<T>+WsEventOf<T>mirror ofsrc/synthorg/api/ws_payloads/{_lifecycle,_domain}.py. Compile-time exhaustiveness guard catches drift. Pre-positions a stable contract for backend migration RFC: stringly-typed boundaries -- typed contract at MCP / JWT / WS / audit-chain / A2A / settings export #1711; the dispatch loop continues to deliver the existing untypedWsEventso per-store migrations roll out incrementally.web/src/styles/status-colors.ts) -- typedRecord<EnumValue, string>lookups for ROLE_BADGE_COLORS and ESCALATION_STATUS_BADGE_COLORS replace inlineROLE_PILL_CLASS/STATUS_BADGE_CLASSconstants in UsersPage and EscalationQueuePage.Wave 2 -- a11y + responsive + perf
<title>for screen-reader context.<button>to<Button variant="link" size="sm">(inherits design-system contrast + focus ring).min-w-[40rem]on AgentSpendingTable andw-56on SettingsActionCard are now responsive (md:guards) so 768-1024px tablets don't force horizontal scroll.SpendBurnChart,CostBreakdownChart,BudgetBurnChart) nowReact.lazy+<Suspense fallback={<SkeletonChart />}>. Production build splits a dedicatedvendor-chartschunk (~378 KB / ~109 KB gzipped). New<SkeletonChart>variant in@/components/ui/skeleton.React.memowrappers on ApprovalCard, ChannelListItem, ActivityFeedItem (high-traffic list items).useMemo.Wave 3 -- Storybook + UX consistency
detail-nav-bar.stories.tsx(7 stories: Default, FirstItem, LastItem, PositionHidden, WithLabel, ShortcutsBound, BothDisabled).useEmptyStatePropshook (web/src/hooks/use-empty-state-props.ts) -- centralises the "no data ever" / "no data after filter" discriminator. Wired into ApprovalsPage and EscalationQueuePage.ListHeader description=applied to WorkflowsPage / ConnectionsPage / BudgetPage / ProvidersPage / EscalationQueuePage / UsersPage / RequestQueuePage.space-y-section-gapacross ProvidersPage / ConnectionsPage / UsersPage.<ListHeader>(matches OauthAppsPage convention).Wave 4 -- dashboard UX, onboarding, error messages
?connection=on WebhookReceiptsPage, ClientListPage SearchInput, ApprovalsPage clickable risk-filter MetricCards (aria-pressed), TaskBoardPage selected-column highlight, SettingsPage Advanced badge next to namespace tabs, RequestQueuePage description noting the legal lifecycle, ProvidersPage description (the<details>collapse already covers progressive disclosure).createProviderFromPresetreturns a{ ok, warning? } | { ok: false, error }result-object; SetupAgentCard tracks in-flight name-save to disable Randomize; CompanyStep clears stale errors on unmount; CompleteStep error banner appends a recovery hint; SkipWizardForm distinguishes company-create vs setup-completion failures via the store snapshot; WizardNavigation accepts anextDisabledReasoncaption; CompanyStep Company Name InputField gains a hint message.getErrorMessage, cancellation/rejection reasons gain context, OauthAppCard titles disambiguated, validation-warning past-tense + actionable, TunnelCard clipboard error discrimination. Three audit-58 sub-findings deferred initially are now fully addressed in the post-review commit.Pre-PR review fixes (final commit, 19 findings from 9 agents)
buildValidationErrorhelper, ApprovalDetailDrawer inline-field error, newgetCrudErrorTitleerror-code routing wired into connections + providers crud-actions.sanitizeWsEnum.fieldmandatory, newuseEmptyStatePropstest, ws-sanitize surrogate-pair test tightened, TunnelCard clipboard DOMException routing, CompanyStep cleanup race guard, websocket.ts sanitisation contract comment, BudgetPage timeout rationale.Test plan
npm --prefix web run lint-- zero warningsnpm --prefix web run type-check-- cleannpm --prefix web run test -- --run --detect-async-leaks-- 247 files, 2997 tests pass, 75 async leaks (under MAX_ASYNC_LEAKS=90 ceiling)npm --prefix web run storybook:build-- detail-nav-bar story buildsnpm --prefix web run build-- recharts splits into avendor-chartschunkReview coverage
Pre-reviewed by 9 agents (docs-consistency, comment-quality-rot, frontend-reviewer, design-token-audit, security-reviewer, api-contract-drift, test-quality-reviewer, silent-failure-hunter, issue-resolution-verifier). 19 findings surfaced across CRITICAL / MAJOR / MEDIUM / MINOR / INFO; all 19 addressed. Triage table at
_audit/pre-pr-review/triage.md.Out of scope
Deferred per the issue's "Pure
web/src/, zero backend overlap" rule:src/synthorg/budget/errors.py-- standardise "exhausted" vs "exceeded" terminology (audit 58 finding 11)src/synthorg/core/domain_errors.py-- addretryable: ClassVar[bool]to ArtifactStorageFullError vs ArtifactContentTooLargeError (audit 58 finding 12)src/synthorg/providers/errors.py-- auditdefault_messagevoice consistency (audit 58 finding 15)Backend migration #1711 imports the typed WS contract from this PR.